1365732 - Create a one off system addon to disable captive portal detection for FF52

Status: RESOLVED INVALID

(Core :: Networking, defect, P5)

Description

[Benson Wong [:mostlygeek]]

Captive Portal detection polling logic is too aggressive. This was fixed in Bug 1359697, but users have reported (bug 1363651) that it is causing network infrastructure problems. On our side, detectportal.mozilla.org is seeing huge traffic levels. 

After discussing with :lizzard we decided to: 

-- snip snip --
- System add-on to turn the feature off for 52.x
- Uplift the patch now to m-r and include it in the 53.0.3 dot release
- Make sure the patch gets into 52.2.0esr  and beta.  
- Optionally, we could include the patch in 52.1.2esr
-- snip snip --

This bug tracks the work for a one off system addon to disable the feature on FF52.

Comment 1

[Benson Wong [:mostlygeek]]

Opened a pull request for the system addon: https://github.com/mozilla/one-off-system-add-ons/pull/40

Comment 2

[Benson Wong [:mostlygeek]]

Attachment: xpi for signing

r+'d from :rhelmer

Some initial testing: 
 - download FF52.0.1 for OSX 
 - confirmed that: network.captive-portal-service.enabled == true
 - use about:debugging to install the attached XPI
 - confirmed that: network.captive-portal-service.enabled == false

:jason || :wezhou could you guys sign this extension for me please?

Comment 3

[:wezhou]

Attachment: signed.attachment.8868797.zip

Attachment 8868797 [details] signed.

Please test it.

Thanks!

Comment 4

[Robert Helmer [:rhelmer]]

I've just tried this using the test instructions in https://gist.github.com/rhelmer/b392e97d0b6ff81c06c205c760050c2b on 52.0.2 macOS and it flips the `network.captive-portal-service.enabled` to `false` as expected.

Allowing app upgrade to 53.0.2 removes the "[e-mail]" extension as expected, and `network.captive-portal-service.enabled` is set to the default (for that release) of `true`.

Comment 5

[Benson Wong [:mostlygeek]]

I followed :rhelmer's direction in comment #4 and had the same results. 

- using a new FF profile on FF52.0.2 on OSX
- the system addon ([e-mail]) was installed locally
- the network.captive-portal-service.enabled was set to false

Comment 6

[Adrian Florinescu [:aflorinescu]]

I've tried to verify the bug. I couldn't actually managed to update the sys-addons using the steps from https://gist.github.com/rhelmer/b392e97d0b6ff81c06c205c760050c2b, I wonder if there's something I'm doing wrong. Here's what I did:

1. Modified update.xml (https://people-mozilla.org/~aflorinescu/CPdissable_addon/update.xml)
2. Set the add-on to this location: https://people-mozilla.org/~aflorinescu/CPdissable_addon/[e-mail]
3. Opened about:config and modified the extensions.systemAddon.update.url to point to https://people-mozilla.org/~aflorinescu/CPdissable_addon/update.xml
4. From about:config also flipped the extensions.logging.enabled and devtools.chrome.enabled prefs to true.
5. Ran the snipper to update the sys addons: Cu.import("resource://gre/modules/AddonManager.jsm"); AddonManagerPrivate.backgroundUpdateCheck();

Benson, could you please advise where I'm lacking with my understanding of the above steps?

Comment 7

[Robert Helmer [:rhelmer]]

(In reply to Adrian Florinescu [:AdrianSV] from comment #6)
> I've tried to verify the bug. I couldn't actually managed to update the
> sys-addons using the steps from
> https://gist.github.com/rhelmer/b392e97d0b6ff81c06c205c760050c2b, I wonder
> if there's something I'm doing wrong. Here's what I did:
> 
> 1. Modified update.xml
> (https://people-mozilla.org/~aflorinescu/CPdissable_addon/update.xml)

The hashValue (abcdef123) and size (1234) don't look correct.

> 2. Set the add-on to this location:
> https://people-mozilla.org/~aflorinescu/CPdissable_addon/disable-
> [e-mail]
> 3. Opened about:config and modified the extensions.systemAddon.update.url to
> point to https://people-mozilla.org/~aflorinescu/CPdissable_addon/update.xml
> 4. From about:config also flipped the extensions.logging.enabled and
> devtools.chrome.enabled prefs to true.
> 5. Ran the snipper to update the sys addons:
> Cu.import("resource://gre/modules/AddonManager.jsm");
> AddonManagerPrivate.backgroundUpdateCheck();
> 
> Benson, could you please advise where I'm lacking with my understanding of
> the above steps?

Do you see any output in the Browser Console? I'd expect the overall update to fail, and the logs to print the correct values for the above two fields.

Comment 8

[Adrian Florinescu [:aflorinescu]]

Thanks Robert for looking into this: 
This is the browser console output for the profile that i've set on FF 52.0 : https://pastebin.mozilla.org/9022409

The hashValue and size are defaulted from the example (don't know where to take the hash from?) and I couldn't find anything helpful from my point of view in the browser console. Maybe I don't know what I'm looking for?

Comment 9

[Robert Helmer [:rhelmer]]

(In reply to Adrian Florinescu [:AdrianSV] from comment #8)
> Thanks Robert for looking into this: 
> This is the browser console output for the profile that i've set on FF 52.0
> : https://pastebin.mozilla.org/9022409
> 
> The hashValue and size are defaulted from the example (don't know where to
> take the hash from?) and I couldn't find anything helpful from my point of
> view in the browser console. Maybe I don't know what I'm looking for?

There are a ton of errors in here that look like it's related to work going on in Nightly. What version of Firefox is this and is it a Mozilla build or Ubuntu?

I also don't see anything in this logs related to the update, it may take a minute after running the `AddonManagerPrivate.backgroundUpdateCheck` snippet before the update process completes.

Immediately after running the above output similar to this should be logged:

1495474301007	addons.manager	DEBUG	Background update check beginning

When the update process is complete, you should see something like:

1495474353911	addons.manager	DEBUG	Background update check complete

Comment 10

[Adrian Florinescu [:aflorinescu]]

Apologies for the profile errors - i created the profile on nightly indeed, redid the steps, on a clean 52 profile. I'm running Mozilla FF 52.0 Build ID 	20170302120751. Here are the browser console logs: https://pastebin.mozilla.org/9022418

Comment 11

[Robert Helmer [:rhelmer]]

(In reply to Adrian Florinescu [:AdrianSV] from comment #10)
> Apologies for the profile errors - i created the profile on nightly indeed,
> redid the steps, on a clean 52 profile. I'm running Mozilla FF 52.0 Build ID
> 20170302120751. Here are the browser console logs:
> https://pastebin.mozilla.org/9022418

Hm. I don't see the update attempt for system add-ons happening - are both app.update.enabled and app.update.auto prefs set to true?

Also this is the Mozilla build and not the build that comes with Ubuntu?

Comment 12

[Adrian Florinescu [:aflorinescu]]

(In reply to Robert Helmer [:rhelmer] from comment #11)
> (In reply to Adrian Florinescu [:AdrianSV] from comment #10)
> > Apologies for the profile errors - i created the profile on nightly indeed,
> > redid the steps, on a clean 52 profile. I'm running Mozilla FF 52.0 Build ID
> > 20170302120751. Here are the browser console logs:
> > https://pastebin.mozilla.org/9022418
> 
> Hm. I don't see the update attempt for system add-ons happening - are both
> app.update.enabled and app.update.auto prefs set to true?
> 
> Also this is the Mozilla build and not the build that comes with Ubuntu?

We have continued the above on IRC. 

And we've got to the point where we figured I had the autoupdate on manual. As soon as I've set it to true, I got the [e-mail] add-on. Following up I've set the right size and Sha for it, afterwards still got stuck on the following:

1495478847300	addons.productaddons	INFO	sending request to: https://people-mozilla.org/~aflorinescu/CPdissable_addon/update.xml
1495478847936	addons.xpi	WARN	Add-on [e-mail] is not correctly signed.
1495478847936	addons.xpi	WARN	System add-on [e-mail] isn't compatible with the application.
1495478847937	addons.manager	WARN	Failed to update system addons: Error: Rejecting updated system add-on set that either could not be downloaded or contained unusable add-ons. (resource://gre/modules/addons/XPIProvider.jsm:3228:13) JS Stack trace: this.XPIProvider.updateSystemAddons<@XPIProvider.jsm:3228:13

I've confirmed that I have the signed version from comment 3. I will check tomorrow if this is related to Ubuntu by doing the same steps on a different OS.

Comment 13

[Adrian Florinescu [:aflorinescu]]

Still no success making this work (comment 12). Same result on a Windows machine. I think the best and fastest way to move forward here (although I'm still curious what are the the necessary steps to successfully set-up the above environment) is to have this landed as a system-add-on on `release-sysaddon` channel.  (https://wiki.mozilla.org/Firefox/Go_Faster/System_Add-ons/Process#Verification_of_Test_Channel)

Comment 14

[Benson Wong [:mostlygeek]]

The size and hash in your xml file [1] are wrong. 

Try mine: https://people-mozilla.org/~bwong/bug_1365732/update.xml. 
I tested this with FF52.0.1 and it worked. Also upgrading to 53.0.3 removed the system addon. 

The problem with your XML file is that the hash and the file size are wrong. This is how I installed it on people.m.o:

 > cd public_html
 > mkdir bug_1365732; cd bug_1365732
 > curl -L 'https://bugzilla.mozilla.org/attachment.cgi?id=8868799' -o '[e-mail]'
 > ls -l; sha512sum '[e-mail]'
 > use above values in the XML file

[1] https://people-mozilla.org/~aflorinescu/CPdissable_addon/update.xml

Comment 15

[Adrian Florinescu [:aflorinescu]]

Nice. Thanks Benson, it works from your link. I was taking the hash and the size from the browser console, from the expected. Seems it's not reliable. I shall proceed verifying this then.

Comment 16

[Adrian Florinescu [:aflorinescu]]

Verified on Ubuntu 16.04, Windows 10 x64 and Mac OSX 10.12 for release versions 52.0, 52.0.1, 52.0.2:

Following remarks:
1. (release channel) If you install the add-on, (will set pref. network.captive-portal-service.enabled to false), then you set it manually to true, restarting to FF 52.* won't re-trigger the add-on to set the pref. back to false. However,  this scenario involves a small % of our users, usually the users that are stuck on 52.* and auto-update doesn't work for them. AFAIK, if auto-update is true and it doesn't work, there are chances that neither the system add-on won't reach these users.  
From my point of view, there should be a very small niche of user population which is stuck on 52.* with auto-update on - which doesn't work and on which we shall be able to actually deliver the system add-on. I'm speculating quite a bit on the above, but there was the OutOfDateNotification System add-on which was supposed to target these kind of stuck users(orphans) and as far as I know with not that much of success.  

2. As we tested this manually, this add-on can affect 52.* ESR as well. My question here is, will/should the system add-on target ESR as well as the release channel?

Comment 17

[Benson Wong [:mostlygeek]]

:lizzard 

Following up to Adrian's points: 

1. Should we bother releasing this addon? If people are stuck on 52 due to auto-update not working they probably won't get this system addon either. 

2. I believe we released a 52ESR patch? Those people should already have the bug fixed. 

Also, I asked ops about changes in traffic. Seems with 53.0.3 going out we reduced the traffic to detectportal.firefox.com by 75% as of Monday. So it's already had a large effect. We should see that continue to drop as people continue to upgrade.

Comment 18

[Adrian Florinescu [:aflorinescu]]

(In reply to Benson Wong [:mostlygeek] from comment #17)
> 2. I believe we released a 52ESR patch? Those people should already have the
> bug fixed. 
The pref. network.captive-portal-service.enabled is still defaulted on true on the last ESR version: Version 	52.1.2 Build ID 20170517122419.

Comment 19

[Benson Wong [:mostlygeek]]

(In reply to Adrian Florinescu [:AdrianSV] from comment #18)

> The pref. network.captive-portal-service.enabled is still defaulted on true
> on the last ESR version: Version 	52.1.2 Build ID 20170517122419.


That's right. The logic was fixed so it stops polling.

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Most users on the release channel have moved to 53.x by now. If we're OK with the current level of server traffic, then I agree with Benson in comment 17 that we may not need the add-on. Let's follow up by email.

Comment 21

[Firefox Bug Husbandry Bot]

Bulk priority update: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 22

[Jason Duell]

Where are we at with this bug? Is this INVALID at this point?