Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Description • 8 years ago

While doing some investigation for bug 1272228 I noticed that none of the current Marionette tests which are getting reported as Tier-1 on Treeherder make use of MOZ_DISABLE_NONLOCAL_CONNECTIONS (bug 995417) to disallow usage of remote network connections. If that environment variable is set the browser should crash under those use cases.

Other test harnesses handle that variable in their runner script, but Marionette doesn't have support for it at all.

So I see two options here:

1) We get this support added to Marionette. Maybe with an optional argument to get it disabled. Harnesses which depend on Marionette (e.g. firefox-ui-test) could modify it on their behalf.

2) We have to update each Taskcluster task and buildbot job, so that those set the environment variable globally for those jobs in case of Tier-1 jobs.

Not sure what's preferred here but I feel we should fix that ASAP to ensure that upcoming Marionette tests do not introduce lots of orange.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 1 • 7 years ago

it looks like for reftest and mochitest we have removed MOZ_DISABLE_NONLOCAL_CONNECTIONS.  So right now only talos and xpcshell run with MOZ_DISABLE_NONLOCAL_CONNECTIONS=1.  I assume there is more backstory here and if we determined it is ok to run with MOZ_DISABLE_NONLOCAL_CONNECTIONS=0, I would like to learn more.

As a note, this is required for webextensions which are not signed, so right now I have to sign new webextensions in talos and one of the major reasons of using marionette was to avoid signing them!

:ahal, do you have more history on the MOZ_DISABLE_NONLOCAL_CONNECTIONS environment variable for reftest/mochitest?

	Andrew Halberstadt [:ahal]
	Comment 2 • 7 years ago

Mochitest and reftest have the variable set here:
https://dxr.mozilla.org/mozilla-central/source/testing/mozbase/mozrunner/mozrunner/utils.py#143

Marionette also uses mozrunner, maybe it just needs to call that function?

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 3 • 7 years ago

That's correct, but we cannot always do it. For Marionette based harnesses we have different tier levels as described in comment 0. So only for those tests which absolutely do not require remote access this env variable has to be set. Using the `test_environment` function is another question, which needs investigation.

	Andreas Tolfsen ❲:ato❳
	Comment 4 • 7 years ago

Is it possible to set per-job environment variables?  If it was possible to configure these at that level, we would be able to control crash-on-network-access-assertion at a more finely grained level than at the Marionette harness level.

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 5 • 7 years ago

I think I was led here accidentally as marionette jobs don't support MOZ_DISABLE_NONLOCAL_CONNECTIONS whereas I am trying to connect to firefox -marionette when setting MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 in the environment and that isn't working.  That does work for mochitest/reftest though.

	Andreas Tolfsen ❲:ato❳
	Comment 6 • 7 years ago

(In reply to Joel Maher ( :jmaher) from comment #5)

> I think I was led here accidentally as marionette jobs don't support
> MOZ_DISABLE_NONLOCAL_CONNECTIONS whereas I am trying to connect to
> firefox -marionette when setting MOZ_DISABLE_NONLOCAL_CONNECTIONS=1
> in the environment and that isn't working.  That does work for
> mochitest/reftest though.

If I enable MOZ_DISABLE_NONLOCAL_CONNECTIONS with the default set of
Marionette unit tests, it fails with the following error message:

> FATAL ERROR: Non-local network connections are disabled and a connection attempt to tiles.services.mozilla.com (35.167.184.4) was made.

I think mochitest and reftest instrument Firefox in some way such that
the tile service is not contacted.  The tile service URL appears to be
configurable through the browser.newtabpage.directory.source preference,
and it is possible that needs to be cleared.

Ideally there would be another preference that turns this feature off,
and maybe browser.newtabpage.enabled or browser.newtabpage.enhanced does
this.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 7 • 7 years ago

Yes, and there might be still other preferences which we have to set and are used to define remote endpoints for various Firefox services.

This is bug is still not a priority, so I haven't done any work yet.

	Andreas Tolfsen ❲:ato❳
	Comment 8 • 6 years ago

I found some more prefs that need to be set:

> "browser.newtabpage.directory.source": "",
> "browser.ping-centre.production.endpoint": "",
> "browser.newtabpage.activity-stream.telemetry.ping.endpoint": "",
> "geo.wifi.uri": "",
> "browser.search.geoip.url": "",
> "browser.safebrowsing.provider.mozilla.gethashURL": "",
> "browser.safebrowsing.provider.mozilla.updateURL": "",
> "browser.aboutHomeSnippets.updateUrl": "",
> "app.update.url": "",
> "extensions.systemAddon.update.url": "",
> "media.gmp-manager.certs.1.commonName": "",
> "media.gmp-manager.certs.2.commonName": "",
> "media.gmp-manager.url": "",
> "browser.newtabpage.activity-stream.tippyTop.service.endpoint": "",
> "extensions.shield-recipe-client.api_url": "localhost",

This gets the tests mostly running.

I also wasn’t able to disable downloading of the OpenH264 codec from
ciscobinary.openh264.org.

	Andreas Tolfsen ❲:ato❳
	Comment 9 • 6 years ago

> 17:54:28  <jesup>	ato: you can change media.gmp-manager.url.override (see all.js in the source)
> 17:54:55  <jesup>	that can block contacting the update check
> 17:55:28  <jesup>	ato: some prefs are dynamic, especially ones related to plugins

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 10 • 6 years ago

(In reply to Andreas Tolfsen ‹:ato› from comment #8)
> I found some more prefs that need to be set:

Yes, please have a look at bug 1371576 which is a dependency for this flag to be added.

	Pulsebot
	Comment 13 • 2 years ago

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d260e67d087c
Force MOZ_DISABLE_NONLOCAL_CONNECTIONS in Marionette-based tests r=webdriver-reviewers,whimboo,chutten

	Pulsebot
	Comment 15 • 2 years ago

Backout by nfay@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6b8ce155bfa6
Backed out changeset d260e67d087c for causing Bug 1753649 CLOSED TREE

	Pulsebot
	Comment 21 • 2 years ago

Pushed by jdescottes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/66941397c141
Force MOZ_DISABLE_NONLOCAL_CONNECTIONS in Marionette-based tests r=webdriver-reviewers,whimboo,chutten
https://hg.mozilla.org/integration/autoland/rev/dce3e334a5d9
Check MOZ_DISABLE_NONLOCAL_CONNECTIONS from services/settings/Utils.jsm r=Gijs