Marionette driven tests are missing support for MOZ_DISABLE_NONLOCAL_CONNECTIONS

NEW
Unassigned

Status

defect
P3
normal
3 years ago
6 months ago

People

(Reporter: whimboo, Unassigned)

Tracking

(Depends on 2 bugs)

49 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

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.
Depends on: 1299441
Depends on: 1313599
Depends on: 1320643
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?
Flags: needinfo?(ahalberstadt)
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?
Flags: needinfo?(ahalberstadt)
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.
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.
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.
(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.
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.
Blocks: 1361002
No longer blocks: 1361002
Depends on: 1371576
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.
Priority: -- → P3
> 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
(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.
Duplicate of this bug: 1371576
You need to log in before you can comment on or make changes to this bug.