Open Bug 1729327 Opened 3 years ago Updated 3 years ago

Mochitests crash when MOZ_DISABLE_NONLOCAL_CONNECTIONS is set to 0

Categories

(Core :: XPConnect, task, P5)

Product:
Core
Component:
XPConnect
Type:
task

Tracking

()

People

(Reporter: jdescottes, Unassigned)

References

Details

It is currently impossible to use non-local connections when running Browser Mochitests (eg opening a website).

From reading Bug 1049688, I understand that this should be possible locally by setting MOZ_DISABLE_NONLOCAL_CONNECTIONS=0. Of course it shouldn't be used for real tests, but locally it can be a great tool to run various scenarios against live websites. But in current central, trying to run a browser mochitest with this setting will crash.

The issue is around the isInAutomation check:

inline bool IsInAutomation() {
  if (!ReadOnlyPage::sInstance.mTurnOffAllSecurityPref) {
    return false;
  }
  MOZ_RELEASE_ASSERT(AreNonLocalConnectionsDisabled());
  return true;
}

When you enable non-local connections, this method will fail on the assert. To avoid this you can pass security.turn_off_all_security_so_that_viruses_can_take_over_this_computer to true. But in this case, IsInAutomation will start returning false and there are a few spots which will call Cu.crashIfNotInAutomation(); when you try to start a mochitest.

So for reference, simply commenting those calls to Cu.crashIfNotInAutomation(); and then running a test with MOZ_DISABLE_NONLOCAL_CONNECTIONS=0 ./mach test path/to/test --setpref security.turn_off_all_security_so_that_viruses_can_take_over_this_computer=true allows you to run mochitests which can load real webpages.

It would be nice if isInAutomation didn't assert on AreNonLocalConnectionsDisabled() so that we can enable connections while still keeping the isInAutomation flag.

See bug 984012. security.turn_off_all_security_so_that_viruses_can_take_over_this_computer=true enabled really danger test-only features in the past. If we have no such danger features anymore, we could loosen the release-mode assert.

See Also: → 984012

AreNonLocalConnectionsDisabled() is part of our defense in depth for exploits, so we can't just remove it.

That said, it does look like there's some code in xpc::ReadOnlyPage::Init() that is carefully set up to let you disable these checks if things are set up correctly, so I'm not sure why it isn't working.

Component: Mochitest → XPConnect
Product: Testing → Core
Version: Default → unspecified

Ah, I see. I was confused. xpc::ReadOnlyPage::Init() won't set the take_over pref to true if non-local connections are allowed, presumably because the IsInAutomation() assert mentioned in comment 0 will always crash with that combination.

Seems like this (doing local development in which a browser-chrome mochitest does network loads) is a niche-enough use-case that we'd be better served by having the developer just locally patch firefox to remove the check. Let me know if I'm off-base there.

The issue would be that people who normally use artifact builds will have to do a full local build. But given that I haven't seen any other complaints about this, and it has probably been like this for at least a year, maybe that's not a huge issue, if we can't easily fix this without increasing the vulnerability of our release builds.

(In reply to Bobby Holley (:bholley) from comment #4)

Seems like this (doing local development in which a browser-chrome mochitest does network loads) is a niche-enough use-case that we'd be better served by having the developer just locally patch firefox to remove the check. Let me know if I'm off-base there.

Patching Firefox locally is fine. I would say the issue is that it's hard to find what to update to make it work, especially with artifact builds.
So it would be great to have a single thing to update (which I imagined was not too far from supporting the env variable).

To give a rough idea of my journey to get to a workaround here:

  • search the error message on searchfox
  • found MOZ_DISABLE_NONLOCAL_CONNECTIONS
  • found comments such as https://searchfox.org/mozilla-central/source/testing/mochitest/runtestsremote.py#287-291 which suggest that MOZ_DISABLE_NONLOCAL_CONNECTIONS can be set to 0 temporarily
  • tried setting MOZ_DISABLE_NONLOCAL_CONNECTIONS to 0
  • got a different crash from isInAutomation
  • passed the pref security.turn_off_all_security_so_that_viruses_can_take_over_this_computer=true
  • got other crashes due to calls to Cu.crashIfNotInAutomation();
  • commented out the Cu.crashIfNotInAutomation(); calls

I don't really have a strong opinion about whether we should support this better or not.
If not we can probably just update the comments around MOZ_DISABLE_NONLOCAL_CONNECTIONS, and I can still refer to this bug if/when I forget how to do this :)

Changing severity to S? because of <rationale>.
(testing only)

Severity: -- → N/A
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.