1342158 - Delay known websense users from updating to 52

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

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

We'd like to delay websense users from updating to 52 once we release. rhelmer suggested shipping that in tree, but allowing us to turn it off a week or so after 52 release to let them update to 52.  This is to allow the websense team time to land their fix and release it to their users so they won't automatically hit the startup crash.

Comment 1

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

Felipe is this something you can take on?

Comment 2

[:Felipe Gomes (needinfo for replies!)]

I believe this is what rstrong worked on bug 1329692, which just landed on beta. If I'm not misunderstanding, then it's just a matter of configuring the Balrog rules when 52 is released.

Comment 3

[Robert Strong (they/them - no direct email)]

Felipe, that landed on beta for 51 so wouldn't the aushelper system add-on need to be converted into an xpi and whatever other steps that need to be performed and then pushed to 51 or have a 51.0.2 release that includes this code for the aushelper system add-on?

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Ah right, so the code is there, but I got confused with the version numbers.. Yeah, we'd need to pack it into an xpi and ship it through GoFaster

Comment 5

[:Felipe Gomes (needinfo for replies!)]

But does this crash not occur on 51 but occurs on 52?

Comment 6

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

This is the crash we expect to happen because it'll be a newly signed build.

Comment 7

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

Marco, here's your bug. Please feel free to chime in!

Comment 8

[Marco Castelluccio [:marco]]

We already know that the latest version of WebSense won't work with Firefox 52 (for binary compatibility reasons). They release a new version shortly after we release ours and their users update manually, so it's going to take a while to get all WebSense users updated.

Since we know which WebSense versions are compatible with which Firefox versions, we don't have to block updates only for a week or so, we could also block them indefinitely for incompatible versions of WebSense.

Comment 9

[Marco Castelluccio [:marco]]

According to this dashboard (https://docs.google.com/spreadsheets/d/18n8NEimajNpnGRHdQsrrnyJ-Ufgvmpz-AXCUpVaFRMk/edit#gid=0) Firefox 52 is not crashing with WebSense, which is quite unexpected.

Florin, can you confirm that? Could you also force a Firefox crash, so that we can see if WebSense is injecting itself in Beta?

Comment 10

[David Durst [:ddurst]]

I want to clarify here that we're doing what we expect:

- rstrong's patch for the endpoint version check (bug 1329692) landed in 52
- because that is NOT in 51, we want to pull that logic into a system add-on and deploy it to 51 (so that we can discretely control those users' cadence of updating to 52)

[The other option is a dot release for 51, but unless I'm mistaken, we've decided that number of clients don't warrant that.]

Liz, is that all correct?

Comment 11

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

Yes that sounds accurate, we would like a system add-on for 51 to deploy, say, early next week, so that we can control when the websense users update to 52.   No time for a dot release (which takes a lot more work and would likely be more risky)

Comment 12

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

Attachment: https://github.com/mozilla/one-off-system-add-ons/pull/17

Adding the link to the GitHub PR just created.

We need

* Signed XPI posted to this bug
* QA verification posted to this bug
* RelMan approval posted to this bug

Ref: https://wiki.mozilla.org/Firefox/Go_Faster/System_Add-ons/Process#QA_and_Code_Review

Comment 13

[Alex Thayer [:alexical] (she/her)]

Had a mid-air collision and didn't notice. Adding a needinfo on rstrong to get feedback on the actual code.

@rstrong: I just pulled out the telemetry stuff and adjusted the REPLACE_KEY matching to account for the case where the Bug1296630 code had already executed. Does that seem reasonable?

Should I be contacting anyone specifically for each of those bullet points?

Comment 14

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

(In reply to Doug Thayer [:dthayer] from comment #13)
> Should I be contacting anyone specifically for each of those bullet points?

> * Signed XPI posted to this bug
After you are r+'d, package the XPI and attach to the bug. You can NI :jason and ask him to sign it.

> * QA verification posted to this bug
NI rares_bologa and he'll help with QA verification against the signed XPI (include any relevant QA instructions). Although I see florin is pinged here, he may be able to help you too.

> * RelMan approval posted to this bug
Since Liz already commented here, she can probably post approval for you.

Examples of all of this can be found in the docs linked above.

Comment 15

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Marco Castelluccio [:marco] from comment #9)
> According to this dashboard
> (https://docs.google.com/spreadsheets/d/18n8NEimajNpnGRHdQsrrnyJ-Ufgvmpz-
> AXCUpVaFRMk/edit#gid=0) Firefox 52 is not crashing with WebSense, which is
> quite unexpected.
> 
> Florin, can you confirm that? Could you also force a Firefox crash, so that
> we can see if WebSense is injecting itself in Beta?

As the gdoc shows we didn't get any crashes with newer versions of the endpoint (note that there are some intermediate versions that reportedly crash, but we don't have them available). Fx 48+ consistently crash with Forcepoint v8.2 (and some older versions as Alex discovered yesterday).

Alex, when you get some time, could you test with the latest Firefox 52 and the latest Endpoint version, force a crash and post the link so Marco can have a look?

(In reply to Cory Price [:ckprice] from comment #14)
> (In reply to Doug Thayer [:dthayer] from comment #13)
> > * QA verification posted to this bug
> NI rares_bologa and he'll help with QA verification against the signed XPI
> (include any relevant QA instructions). Although I see florin is pinged
> here, he may be able to help you too.

Alex should be able to help here as well. The steps for verification that I see here would be:
- install older release versions (46, 47, 48... - depends to which versions we want this system add-on to apply)
- force an add-on update check (the Fire Timer add-on can do that) - or manually install the signed xpi if the sys-addon is not available yet on the channel
- see that we get the system add-on installed
- check the app.update.url for the websense version string - should show only if Websense is detected
- force a Firefox update and see that we correctly update only if the Websense version is not in the blocklist

Questions:
1. We already have a hotfix that applies to Fx < 49.0, adds a (websense) or (nowebsense) parameter to the update URL, and basically does the following:
   - Fx < 48.0
      * updates to latest release only if there is NO Websense (qipcap.dll) detected
      * updates to 47.0.2 if Websense (qipcap.dll) is detected
   - Fx 48.0+
      * updates to latest release regardless of whether Websense is detected or not
Will we keep this hotfix? I'd think not, but if we do I think we may get some conflicts.
2. What Firefox versions would we want this sys-addon to apply to?

Comment 16

[Robert Strong (they/them - no direct email)]

(In reply to Doug Thayer [:dthayer] from comment #13)
> Had a mid-air collision and didn't notice. Adding a needinfo on rstrong to
> get feedback on the actual code.
> 
> @rstrong: I just pulled out the telemetry stuff and adjusted the REPLACE_KEY
> matching to account for the case where the Bug1296630 code had already
> executed. Does that seem reasonable?
That sounds reasonable but it would be good to add back the calls to TelemetryLog for the cpu microcode portion and to add new TelemetryLog calls to the places you removed the telemetry code since the Histograms won't exist in 51 for either.

Comment 17

[Alexandru Simonca [:asimonca], Release Desktop QA]

>Alex, when you get some time, could you test with the latest Firefox 52 and the latest Endpoint version, force a crash and post the link so Marco can have a look?

I've tested the latest Endpoint version (8.3.2509) with Firefox 52.0b9 (unbranded tinderbox build) and crashed the browser using "crashme-new.xpi" add-on. I've used an unbranded tinderbox build because the .xpi for "crashme" was unsigned. Here is the crash report link: https://crash-stats.mozilla.com/report/index/5819d553-024a-46ba-bea0-f50032170224

> Alex should be able to help here as well. The steps for verification that I see here would be:
> - install older release versions (46, 47, 48... - depends to which versions we want this system add-on to apply)
> - force an add-on update check (the Fire Timer add-on can do that) - or manually install the signed xpi if the sys-addon is not available yet on the channel
> - see that we get the system add-on installed
> - check the app.update.url for the websense version string - should show only if Websense is detected
> - force a Firefox update and see that we correctly update only if the Websense version is not in the blocklist

I could not test this yet. Firefox says the add-on could not be installed because it has not been verified. I have downloaded the .xpi file straight from the Github link provided in the attachment. I tried using a tinderbox build here too, but the problem with that approach is that I can't get it to update even if I change the "channel-prefs.js" file to use the "release" channel updates. 
Even if I couldn't update the tinderbox build, I did manage to install the websense-detect.xpi file and that works as expected and the Websense Enpoint version is reflected in the "app.update.url" preff string.

Cory, am I doing something wrong with the .xpi file? Do I have to do anything else to it after I download it from the link?

Comment 18

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #17)
> Cory, am I doing something wrong with the .xpi file? Do I have to do
> anything else to it after I download it from the link?
Yeah, it has to be signed[0]. But it looks like Robert recommended some changes in comment 16 (NI :dthayer). The XPI should be final before we ask Ops to sign it.

[0] Example bug 1339662 comment 3

Comment 19

[Alex Thayer [:alexical] (she/her)]

I added the TelemetryLog.Log calls in. I kept the CPU microcode log calls identical to stay synced with the old code, but I could see that being a problem if we want to differentiate between them? Let me know what you think.

Also, I forgot to ask for clarification on what versions this should apply to. Right now it's 48.0-51.*. Let me know if we should adjust that.

Comment 20

[Robert Strong (they/them - no direct email)]

I think them being in sync making analysis easier than having them out of sync. It should be possible to differentiate them using other values in telemetry. I would think that the add-ons version number will need to be something along the lines of 1.5 since there is already an AUSHelper version 1 and the new one is version 2. Felipe will likely know better than I whether this should be done.

I think it would be a good thing to go with 47.0.2 through 51.*. 47.0.2 has the CPU instruction set detection and along with this 47.0.2 should be able to update to latest if they have the minimum CPU instruction set and nowebsense or a version of websense that is compatible.

Jim and Ben, are both of you OK with this being deployed to 47.0.2 through 51.*?

Comment 21

[Robert Strong (they/them - no direct email)]

Felipe, this would be with a different add-on ID. Do you think that will be a problem? Would it be best to just go with the add-on ID you used for websense?

Comment 22

[Robert Strong (they/them - no direct email)]

Specifically in relation to comment #20?

Comment 23

[Marco Castelluccio [:marco]]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #17)
> >Alex, when you get some time, could you test with the latest Firefox 52 and the latest Endpoint version, force a crash and post the link so Marco can have a look?
> 
> I've tested the latest Endpoint version (8.3.2509) with Firefox 52.0b9
> (unbranded tinderbox build) and crashed the browser using "crashme-new.xpi"
> add-on. I've used an unbranded tinderbox build because the .xpi for
> "crashme" was unsigned. Here is the crash report link:
> https://crash-stats.mozilla.com/report/index/5819d553-024a-46ba-bea0-
> f50032170224

WebSense is injecting itself in this case.
Could you also test a 32 bit build?

Comment 24

[:Felipe Gomes (needinfo for replies!)]

All things being equal, if we're re-shipping this to 47.0.2 through 51.0 I think using the same add-on id is better.. Unless you have any reservations about it..

Comment 25

[Robert Strong (they/them - no direct email)]

I'm fine with that.

With that in mind I think the add-on should just include the websense code.

Comment 26

[Alex Thayer [:alexical] (she/her)]

Just include the websense code as in exclude the CPU code? Is there a guaranteed ordering for the startup calls? I just don't want to modify the %OS_VERSION% section such that the CPU code in 51 doesn't run correctly. I.e., if it becomes "%OS_VERSION%(nowebsense)/", then it won't match |REPLACE_KEY + "/"|.

Comment 27

[Robert Strong (they/them - no direct email)]

Yep... I didn't think about that.

Ideally it would be Felipe's websense add-on ID for versions that don't have the AUSHelper and it would be the AUSHelper add-on ID for versions with the AUSHelper (this would include the CPU microcode info). Would that extra work be reasonable?

Comment 28

[Alex Thayer [:alexical] (she/her)]

It wouldn't be too much work for me. Just so I'm sure I understand:

Create two add-ons in the PR:

- One with id: "[e-mail]"
  * Should be from 47.0.2-49.*
  * Should exclude the CPU code
  * Should be version 2.0
- One with id: "[e-mail]"
  * Should be from 50.0-51.*
  * Should include the CPU code
  * Should be version 1.5

In either case I'm thinking we should match on /%OS_VERSION%(?:\(\w+Bug1296630v1\))?(?:\(websense[0-9.-]*\)|\(nowebsense\))?\//. I.e: we should overwrite what's there every time we restart or load the add-on, because we're interested in updating once the Websense version changes, right?

Comment 29

[Robert Strong (they/them - no direct email)]

(In reply to Doug Thayer [:dthayer] from comment #28)
> It wouldn't be too much work for me. Just so I'm sure I understand:
> 
> Create two add-ons in the PR:
> 
> - One with id: "[e-mail]"
>   * Should be from 47.0.2-49.*
>   * Should exclude the CPU code
>   * Should be version 2.0
> - One with id: "[e-mail]"
>   * Should be from 50.0-51.*
>   * Should include the CPU code
>   * Should be version 1.5
> 
> In either case I'm thinking we should match on
> /%OS_VERSION%(?:\(\w+Bug1296630v1\))?(?:\(websense[0-9.-
> ]*\)|\(nowebsense\))?\//. I.e: we should overwrite what's there every time
> we restart or load the add-on, because we're interested in updating once the
> Websense version changes, right?
That looks correct. Yes the default pref values should be set on every startup and this should already be the case.

Comment 30

[Alex Thayer [:alexical] (she/her)]

@rstrong: Just pushed a commit with it split out - should be ready for review.

Comment 31

[[e-mail] (:bhearsum)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #20)
> I think them being in sync making analysis easier than having them out of
> sync. It should be possible to differentiate them using other values in
> telemetry. I would think that the add-ons version number will need to be
> something along the lines of 1.5 since there is already an AUSHelper version
> 1 and the new one is version 2. Felipe will likely know better than I
> whether this should be done.
> 
> I think it would be a good thing to go with 47.0.2 through 51.*. 47.0.2 has
> the CPU instruction set detection and along with this 47.0.2 should be able
> to update to latest if they have the minimum CPU instruction set and
> nowebsense or a version of websense that is compatible.
> 
> Jim and Ben, are both of you OK with this being deployed to 47.0.2 through
> 51.*?

Based on my read, it looks like we're deploying a SystemAddon that will put values like '(noBug1296630v1)' in OS_VERSION, so we can block certain classes of users based on them. If that's the case, should be fine.

Comment 32

[Robert Strong (they/them - no direct email)]

r+'d but it looks like I don't have reviewer privs on that repo and it would be a good thing to have someone that is more familiar with the system add-on process review this anyways.

Comment 33

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #15)
> 1. We already have a hotfix that applies to Fx < 49.0, adds a (websense) or
> (nowebsense) parameter to the update URL, and basically does the following:
>    - Fx < 48.0
>       * updates to latest release only if there is NO Websense (qipcap.dll)
> detected
>       * updates to 47.0.2 if Websense (qipcap.dll) is detected
>    - Fx 48.0+
>       * updates to latest release regardless of whether Websense is detected
> or not
> Will we keep this hotfix? I'd think not, but if we do I think we may get
> some conflicts.

To re-iterate here: what are we going to do with the existing Hotfix that detects whether or not Websense is installed based on the presence of the qipcap.dll file? Will we remove it once we release the new system add-on here?

The existing Hotfix appears as "Firefox Hotfix 20160826.01", with [e-mail] ID.

Comment 34

[Alexandru Simonca [:asimonca], Release Desktop QA]

(In reply to Marco Castelluccio [:marco] from comment #23)

> WebSense is injecting itself in this case.
> Could you also test a 32 bit build?


Here is the crash report for the 32 bit build. I used the tinderbox unbranded build of Fx 52.0b10 32 bit and "crashme" add-on. I have the 32 bit Websense Endpoint 8.3.2509 installed.
https://crash-stats.mozilla.com/report/index/c39d197d-c2fa-4623-9083-ac9ab2170227

Comment 35

[Alex Thayer [:alexical] (she/her)]

Jason, could you create signed XPIs for the two addons in this PR?

https://github.com/mozilla/one-off-system-add-ons/pull/17

Links:
https://github.com/squarewave/one-off-system-add-ons/raw/4a3693e03a51afa1eafd1613238e4708b7450c7a/addons/aushelper/aushelper.xpi
https://github.com/squarewave/one-off-system-add-ons/raw/4a3693e03a51afa1eafd1613238e4708b7450c7a/addons/websensehelper/websensehelper.xpi

Thanks!

Comment 36

[Marco Castelluccio [:marco]]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #34)
> (In reply to Marco Castelluccio [:marco] from comment #23)
> 
> > WebSense is injecting itself in this case.
> > Could you also test a 32 bit build?
> 
> 
> Here is the crash report for the 32 bit build. I used the tinderbox
> unbranded build of Fx 52.0b10 32 bit and "crashme" add-on. I have the 32 bit
> Websense Endpoint 8.3.2509 installed.
> https://crash-stats.mozilla.com/report/index/c39d197d-c2fa-4623-9083-
> ac9ab2170227

It's injected here as well. I'm not sure why there's no crash, perhaps we can ask Forcepoint if they have changed something in how they inject.

Comment 37

[Jason Thomas [:jason]]

Attachment: websensehelper.xpi signed

Comment 38

[Jason Thomas [:jason]]

Attachment: aushelper.xpi signed

Please see signed add-ons.

Comment 39

[Jim Mathies [:jimm]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #20)
> I think them being in sync making analysis easier than having them out of
> sync. It should be possible to differentiate them using other values in
> telemetry. I would think that the add-ons version number will need to be
> something along the lines of 1.5 since there is already an AUSHelper version
> 1 and the new one is version 2. Felipe will likely know better than I
> whether this should be done.
> 
> I think it would be a good thing to go with 47.0.2 through 51.*. 47.0.2 has
> the CPU instruction set detection and along with this 47.0.2 should be able
> to update to latest if they have the minimum CPU instruction set and
> nowebsense or a version of websense that is compatible.
> 
> Jim and Ben, are both of you OK with this being deployed to 47.0.2 through
> 51.*?

Yes, we definitely want the help up 47 and 48 users to get the new version check. Some of those will likely be allowed to upgrade as a result.

Comment 40

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #39)
> (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from
> comment #20)
> > I think them being in sync making analysis easier than having them out of
> > sync. It should be possible to differentiate them using other values in
> > telemetry. I would think that the add-ons version number will need to be
> > something along the lines of 1.5 since there is already an AUSHelper version
> > 1 and the new one is version 2. Felipe will likely know better than I
> > whether this should be done.
> > 
> > I think it would be a good thing to go with 47.0.2 through 51.*. 47.0.2 has
> > the CPU instruction set detection and along with this 47.0.2 should be able
> > to update to latest if they have the minimum CPU instruction set and
> > nowebsense or a version of websense that is compatible.
> > 
> > Jim and Ben, are both of you OK with this being deployed to 47.0.2 through
> > 51.*?
> 
> Yes, we definitely want the help up 47 and 48 users to get the new version
> check. Some of those will likely be allowed to upgrade as a result.

/help/held

Comment 41

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

(In reply to Doug Thayer [:dthayer] from comment #28)
> It wouldn't be too much work for me. Just so I'm sure I understand:
> 
> Create two add-ons in the PR:
> 
> - One with id: "[e-mail]"
>   * Should be from 47.0.2-49.*
>   * Should exclude the CPU code
>   * Should be version 2.0
> - One with id: "[e-mail]"
>   * Should be from 50.0-51.*
>   * Should include the CPU code
>   * Should be version 1.5
Is this still relevant with respect to Balrog rules? And is there any reason we can't say 47.* to 49.*?

> In either case I'm thinking we should match on
> /%OS_VERSION%(?:\(\w+Bug1296630v1\))?(?:\(websense[0-9.-
> ]*\)|\(nowebsense\))?\//. I.e: we should overwrite what's there every time
> we restart or load the add-on, because we're interested in updating once the
> Websense version changes, right?
And you are doing this targeting within the add-on correct? Considering the complexity of this deployment, it would be preferable if Balrog didn't have to do any filtering other than by version number.

Just a note re: websensehelper, we're currently shipping a v1.0[0] to 47.* (but not to 48.* as the filename implies). This would be updated to v2.0.

Also, pinging a couple folks to see if we can get QA to verify the signed add-ons.

[0] https://ftp.mozilla.org/pub/system-addons/websensehelper/[e-mail]

Comment 42

[[e-mail] (:bhearsum)]

(In reply to Cory Price [:ckprice] from comment #41)
> (In reply to Doug Thayer [:dthayer] from comment #28)
> > It wouldn't be too much work for me. Just so I'm sure I understand:
> > 
> > Create two add-ons in the PR:
> > 
> > - One with id: "[e-mail]"
> >   * Should be from 47.0.2-49.*
> >   * Should exclude the CPU code
> >   * Should be version 2.0
> > - One with id: "[e-mail]"
> >   * Should be from 50.0-51.*
> >   * Should include the CPU code
> >   * Should be version 1.5
> Is this still relevant with respect to Balrog rules? And is there any reason
> we can't say 47.* to 49.*?
> 
> > In either case I'm thinking we should match on
> > /%OS_VERSION%(?:\(\w+Bug1296630v1\))?(?:\(websense[0-9.-
> > ]*\)|\(nowebsense\))?\//. I.e: we should overwrite what's there every time
> > we restart or load the add-on, because we're interested in updating once the
> > Websense version changes, right?
> And you are doing this targeting within the add-on correct? Considering the
> complexity of this deployment, it would be preferable if Balrog didn't have
> to do any filtering other than by version number.

We can't match on regexes in Balrog. We can only do simple substring matches in OS_VERSION. I'm happy to help come up with an effective rule set.

Comment 43

[Alex Thayer [:alexical] (she/her)]

(In reply to Cory Price [:ckprice] from comment #41)
> Is this still relevant with respect to Balrog rules? And is there any reason
> we can't say 47.* to 49.*?

Is what specifically still relevant? I don't think there would be any reason why we couldn't do from 47.*, but if we want to QA all of the versions then that's two more to QA, and I don't know exactly what it gets us.

> And you are doing this targeting within the add-on correct? Considering the
> complexity of this deployment, it would be preferable if Balrog didn't have
> to do any filtering other than by version number.

Yeah, this is all referring to work the add-on is doing just to make sure it's not undoing the work of existing update URL modifying add-ons.

> Also, pinging a couple folks to see if we can get QA to verify the signed
> add-ons.

I'm not sure if QA normally inspects the telemetry component of these things, but if so we should probably wait on https://github.com/mozilla/one-off-system-add-ons/pull/18 and re-sign.

Some details on PR#18 (synopsis from IRC conversations):
This adds telemetry logging on success, and whenever the app.update.url prefs are modified, so we can better diagnose if we see problems like we did with 47.0.2. :chutten did some analysis of the telemetry logs for 47.0.2 and found that there were no logs of errors when using the qipcap.dll method, and yet we still weren't seeing (websense) or (nowebsense) in the URLs. This is a big mystery, so we want to make sure we're prepared if something similar happens this time.

Adding a NI on rstrong to validate that this is all correct, regarding 47.* and whether our plan with the two add-ons is still current, since I'm still a bit fuzzy on all the history of this stuff.

Comment 44

[Alex Thayer [:alexical] (she/her)]

(In reply to Ben Hearsum (:bhearsum) from comment #42)
> We can't match on regexes in Balrog. We can only do simple substring matches
> in OS_VERSION. I'm happy to help come up with an effective rule set.

Sorry for not being more clear! The regexes are just in the add-on, to make sure that it correctly modifies the update URL which may have already been modified by previous versions of websensehelper/aushelper. Substring matching on the OS_VERSION section should be just fine.

Comment 45

[Robert Strong (they/them - no direct email)]

That sounds correct.

Some background... a websense add-on was deployed awhile back and for 47 through 49 that add-on will be pushed. As of 50 we have the aushelper add-on which also include the addition of the currently unused microcode info and that will be used for 50 through 51. 52 has essentially the same add-on as the the 50 through 51 with the addition of telemetry histograms that should make it easier to analyze what is going on in relation to the add-on.

As far as balrog app update rules are concerned they will all make the same change to the update url in relation to websense. 50 and above will also include the currently unused microcode info.

Comment 46

[Alex Thayer [:alexical] (she/her)]

Hey Jason, sorry for asking for this twice, but could we get those two addons signed again? We had to make a few adjustments.

Links:
https://github.com/mozilla/one-off-system-add-ons/raw/master/addons/websensehelper/websensehelper.xpi
https://github.com/mozilla/one-off-system-add-ons/raw/master/addons/aushelper/aushelper.xpi

Comment 47

[Jason Thomas [:jason]]

Attachment: aushelper.xpi signed

Comment 48

[Jason Thomas [:jason]]

Attachment: websensehelper.xpi signed

Please see attached.

Comment 49

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Are these latest signed xpis also available on any sysaddon channel?

Comment 50

[Alexandru Simonca [:asimonca], Release Desktop QA]

I have tested with Websense v 8.3.2509 by manually installing the xpi files to following versions of Firefox and then updating the browser through "beta-cdntest" update channel:
47.0.2 - updated to 48.0 and Websense Helper v2.0
48.0 - updated to 52 and Websense Helper v2.0
48.0.1 - updated to 52 and Websense Helper v2.0
48.0.2 - updated to 52 and Websense Helper v2.0
49.0 - updated to 52 and Websense Helper v2.0
49.0.1 - updated to 52 and Websense Helper v2.0
49.0.2 - updated to 52 and Websense Helper v2.0
50.0 - updated to 52, Websense Helper v2.0, Aus Helper v1.5
50.0.1 - updated to 52, Websense Helper v2.0, Aus Helper v1.5
50.0.2 - updated to 52, Websense Helper v2.0, Aus Helper v1.5
50.1.0 - updated to 52, Websense Helper v2.0, Aus Helper v1.5
51.0 - updated to 52, Websense Helper v2.0, Aus Helper v1.5
51.0.1 - updated to 52, Websense Helper v2.0, Aus Helper v1.5

I have also tested with Websense v 8.2.2324. I have installed "websensehelper.xpi" to Firefox 47.0.2 and forced an update from "About Firefox". The update was NOT blocked and upon restarting Firefox I experienced a start-up CRASH. Since this is a known bad Websense version, I think the update should have been blocked.

The "app.update.url" value for this case was https://aus5.mozilla.org/update/6/%PRODUCT%/%VERSION%/%BUILD_ID%/%BUILD_TARGET%/%LOCALE%/%CHANNEL%/%OS_VERSION%(websense-8.2.2324)/%SYSTEM_CAPABILITIES%/%DISTRIBUTION%/%DISTRIBUTION_VERSION%/update.xml

We cannot test Firefox 48+ with Websense v 8.2.2324, because they crash on startup and so we cannot install the system addon

Comment 51

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

I'd like to meet and talk this over today if possible - I'll send out a meeting invitation. 
My original intention was just to delay all the 51 users with websense from updating for a week or so. This is more ambitious (but awesome).

Also, looks like we need another bug here for releng to make changes to the update rules. But, we might not know exactly what we want those update rules to be until we see the results from shipping this (this week) plus testing with the 52 RC2 build. With that testing we can figure out if any of the websense versions can be let through (with update rules).  But the assumption i'm starting off with is, we release 52 while delaying all the websense users from update. Then whatever versions we know don't crash, we can let through by modifying the update rules after the release.  Is that an accurate summary of the situation?

Comment 52

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #50)
> I have also tested with Websense v 8.2.2324. I have installed
> "websensehelper.xpi" to Firefox 47.0.2 and forced an update from "About
> Firefox". The update was NOT blocked and upon restarting Firefox I
> experienced a start-up CRASH. Since this is a known bad Websense version, I
> think the update should have been blocked.

My guess is we just don't have the update rules in place on beta-cdntest.

With regards to the previous comment from Liz I thought we want to block only known bad Websense versions, so I guess we'd need some clarification there.

Comment 53

[[e-mail] (:bhearsum)]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #52)
> (In reply to Alexandru Simonca, QA (:asimonca) from comment #50)
> > I have also tested with Websense v 8.2.2324. I have installed
> > "websensehelper.xpi" to Firefox 47.0.2 and forced an update from "About
> > Firefox". The update was NOT blocked and upon restarting Firefox I
> > experienced a start-up CRASH. Since this is a known bad Websense version, I
> > think the update should have been blocked.
> 
> My guess is we just don't have the update rules in place on beta-cdntest.

Yeah, the rules are only in place on release-cdntest. I plan to tweak them there once we get further clarification. I wasn't planning on implementing them for Beta, because I doubt we have a notable amount of WebSense users there.

Comment 54

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

From talking with Marco again, we want to block users of all versions of Websense from update until Forcepoint has a chance to ship their updates (for all their actively maintained versions). 

I filed bug 1343643 for the details of what we should do with the update rules.

Comment 55

[Jim Mathies [:jimm]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #54)
> From talking with Marco again, we want to block users of all versions of
> Websense from update until Forcepoint has a chance to ship their updates
> (for all their actively maintained versions). 

This is not what I was thinking. Marco, can you clarify your reasoning here? I thought the plan was to allow all users with newer endpoints to update, and hold any user who has 8.2.2324 or lower. The main goal would be get users who have compat versions of websense + firefox updated to the latest firefox release.

Comment 56

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #55)
> (In reply to Liz Henry (:lizzard) (needinfo? me) from comment #54)
> > From talking with Marco again, we want to block users of all versions of
> > Websense from update until Forcepoint has a chance to ship their updates
> > (for all their actively maintained versions). 
> 
> This is not what I was thinking. Marco, can you clarify your reasoning here?
> I thought the plan was to allow all users with newer endpoints to update,
> and hold any user who has 8.2.2324 or lower. The main goal would be get
> users who have compat versions of websense + firefox updated to the latest
> firefox release.

Liz and I spoke on irc. She clarified that the plan here was to temporarily hold firefox updates for websense users on each new firefox release. During the temporary hold (1 week or so) additional release testing with current websense endpoints would occur. If this is the plan, and we still aim to get compat websense users updated off older versions of firefox soonish, sgtm.

Comment 57

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #50)
> sgtm.
So, can someone run down what the add-ons/targeting is? Comment 28 had a nice breakdown as an example.

Comment 58

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

Hmm, not sure happened with that comment^

There were two things,

1. Asking :dthayer to comment on the results in comment 50.

2. Asking someone (doug, jim or liz) to breakdown the targeting.

Comment 59

[Alex Thayer [:alexical] (she/her)]

Responding to my initial interpretation in case it's useful:

(In reply to Cory Price [:ckprice] from comment #57)
> (In reply to Alexandru Simonca, QA (:asimonca) from comment #50)
> > sgtm.
> So, can someone run down what the add-ons/targeting is? Comment 28 had a
> nice breakdown as an example.

Comment 28 is still current:

- "[e-mail]"
  * Should be from 47.0.2-49.*
  * Should exclude the CPU code (URLs will look like .../%OS_VERSION%(websense-x.y.z)/...)
  * Should be version 2.0
- "[e-mail]"
  * Should be from 50.0-51.*
  * Should include the CPU code (URLs will look like .../%OS_VERSION%(noBug1296630v1)(websense-x.y.z)/...)
  * Should be version 1.5

Since at the start we will be delaying all websense users, Balrog will just need to match on the substring "(websense" (without a closing paren) or "(nowebsense)". @lizzard: Since I'm not sure it's been answered, what should Balrog do with update URLs which contain neither substring? Since despite our efforts we still don't know what caused the previous add-on to fail to add "(websense)" / "(nowebsense)" to some update URLs, there's no way we can be sure that this add-on will succeed. Do we want to treat the unknown cases as nowebsense?

(In reply to Cory Price [:ckprice] from comment #58)
> Hmm, not sure happened with that comment^
> 
> There were two things,
> 
> 1. Asking :dthayer to comment on the results in comment 50.
> 
> 2. Asking someone (doug, jim or liz) to breakdown the targeting.

Regarding comment 50, I believe per https://docs.google.com/spreadsheets/d/18n8NEimajNpnGRHdQsrrnyJ-Ufgvmpz-AXCUpVaFRMk/edit#gid=0 we should be testing with version 8.2.2424 for 48+.

Comment 60

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

(In reply to Doug Thayer [:dthayer] from comment #59)
> Since at the start we will be delaying all websense users, Balrog will just
> need to match on the substring "(websense" (without a closing paren) or
> "(nowebsense)". @lizzard: Since I'm not sure it's been answered, what should
> Balrog do with update URLs which contain neither substring? Since despite
> our efforts we still don't know what caused the previous add-on to fail to
> add "(websense)" / "(nowebsense)" to some update URLs, there's no way we can
> be sure that this add-on will succeed. Do we want to treat the unknown cases
> as nowebsense?
Hmm, it was my understanding that the add-on was going to do some of this targeting (see comment 42 and comment 43). I don't now if Balrog rules can do this, and even if it can, we have other add-ons deploying to these same versions that may prohibit us from doing anything other than a simple version targeting.

NI bhearsum in case I'm missing something.

Comment 61

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

Actually, I'm going to clear Ben's NI for now. Looking again I think we can do this targeting. I will assume that the version/add-ons Doug described is correct and get something up on test.

Comment 62

[Robert Strong (they/them - no direct email)]

As discussed with bhearsum in the bug that created the system add-on for the next release, when websense is present the add-on will send the (websense-X.X.X) where X.X.X is the version number and (nowebsense) when it isn't present. balrog will have rules for app update that will update all clients that have (nowebsense) in the update url to the latest. The clients with (websense-X.X.X) will be updated to the latest if the (websense-X.X.X) matches specific websense versions that have been tested as ok to update to the latest version. For this bug delaying should be extremely similar to these same rules with the exception that anything that matches (websense-X.X.X) will not be updated during this delay.

(In reply to Cory Price [:ckprice] from comment #60)
> (In reply to Doug Thayer [:dthayer] from comment #59)
> > Since at the start we will be delaying all websense users, Balrog will just
> > need to match on the substring "(websense" (without a closing paren) or
> > "(nowebsense)". @lizzard: Since I'm not sure it's been answered, what should
> > Balrog do with update URLs which contain neither substring? Since despite
> > our efforts we still don't know what caused the previous add-on to fail to
> > add "(websense)" / "(nowebsense)" to some update URLs, there's no way we can
> > be sure that this add-on will succeed. Do we want to treat the unknown cases
> > as nowebsense?
> Hmm, it was my understanding that the add-on was going to do some of this
> targeting (see comment 42 and comment 43). I don't now if Balrog rules can
> do this, and even if it can, we have other add-ons deploying to these same
> versions that may prohibit us from doing anything other than a simple
> version targeting.
How would other add-ons deploying to these same versions prohibit balrog's app update rules in any way given that the other add-ons don't modify the app update url and hence won't affect balrog's app update rules?

Are you referring to balrog's system add-on deployment rules? If so, how would this be any different than deploying one system add-on to a version range and another system add-on to another version range as well as how would that be a problem?

Comment 63

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

Let's leave the "websense status unknown" users for now and keep them where they are. We can let them through with update rules and watch for crashes (later).

Comment 64

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

I'm going to go ahead and put this up on the test channel based on comment 59. Per discussion with dthayer on IRC the system add-on targeting doesn't have to do any OS_VERSION matching. Look out for an email tonight on release-drivers.

Comment 65

[[e-mail] (:bhearsum)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #62)
> As discussed with bhearsum in the bug that created the system add-on for the
> next release, when websense is present the add-on will send the
> (websense-X.X.X) where X.X.X is the version number and (nowebsense) when it
> isn't present. balrog will have rules for app update that will update all
> clients that have (nowebsense) in the update url to the latest. The clients
> with (websense-X.X.X) will be updated to the latest if the (websense-X.X.X)
> matches specific websense versions that have been tested as ok to update to
> the latest version. For this bug delaying should be extremely similar to
> these same rules with the exception that anything that matches
> (websense-X.X.X) will not be updated during this delay.

I don't think there's any use in having any "(nowebsense)" rules. Unless we're willing to not update people that haven't received the SystemAddon, we'll still need to have a rule that doesn't specify "(nowebsense)" or "(websense" to make sure those folks can update, and anyone without the SystemAddon would end up matching it as well.

As long as we deploy the SystemAddon ahead of shipping Firefox 52 I think what I've implemented in https://bugzilla.mozilla.org/show_bug.cgi?id=1343643 should be fine:
* Rules for blacklisting known-bad versions (eg: 8.2.2324)
* A rule to hold all known websense users at 51.0.1 for the first week

Should a few of us meet up today to sort this out? It seems like everybody is coming at this with different ideas, and it would be good to get on the same page.

Comment 66

[Alexandru Simonca [:asimonca], Release Desktop QA]

I have tested the "release-sysaddon" channel updates for Websense. I have noted my results in the following etherpad.
https://public.etherpad-mozilla.org/p/websense_fix

Please ni? me for any questions.

Comment 67

[[e-mail] (:bhearsum)]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #66)
> I have tested the "release-sysaddon" channel updates for Websense. I have
> noted my results in the following etherpad.
> https://public.etherpad-mozilla.org/p/websense_fix
> 
> Please ni? me for any questions.

I see that all of the nowebsense tests have failed. It looks like this is because we just built Firefox-52.0, which means the release-cdntest channel is temporarily going to have 404s. Very sorry about that, we hit some bad timing. I just moved these rules to release-localtest, which should work fine.

So, it might be a good idea to rerun the tests with "release-localtest" as the second channel in step 9.

Comment 68

[Alexandru Simonca [:asimonca], Release Desktop QA]

I got to these results by setting each of the release builds starting from 47.0.2 to 51.0.1 on the "release-sysaddon" channel in the "channel-prefs.js" file and I used a code snippet in the Browser Console to force the add-on updates. Then I changed the update channel to "release-cdntest" to test if the builds got an update and what effect would this have on the add-ons.

When there is NO Websense installed:
- The "app.update.url" preff contains the "(nowebsense)" string for every build
- Builds from 47.0.2 to 49.0.2 get an add-on update for "Websense Helper" to v2.0
- Builds from 50.0 to 51.0.1 get an add-on update for "Application Update Service Helper" from v1.0 to v1.5
- NONE of the builds were updated. The browser tries to find and download an update but all I got was an "Update failed." message.

When Websense 8.3.2509 is installed:
- The "app.update.url" preff contains the "(websense)(websense-8.3.2509)" string for every build.
- Builds from 47.0.2 to 49.0.2 get an add-on update for "Websense Helper" to v2.0
- Builds from 50.0 to 51.0.1 get an add-on update for "Application Update Service Helper" updated from v1.0 to v1.5 after the add-on update on "release-sysaddon"
- All of the builds got updated on "release-cdntest" to 51.0.1
- Builds from 50.0 to 51.0.1 get a downgrade of the "Application Update Service Helper" add-on from v1.5 to v1.0 after a browser update.

Comment 69

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #68)
> When Websense 8.3.2509 is installed:
> - The "app.update.url" preff contains the "(websense)(websense-8.3.2509)"
> string for every build.
> - Builds from 47.0.2 to 49.0.2 get an add-on update for "Websense Helper" to
> v2.0
> - Builds from 50.0 to 51.0.1 get an add-on update for "Application Update
> Service Helper" updated from v1.0 to v1.5 after the add-on update on
> "release-sysaddon"
> - All of the builds got updated on "release-cdntest" to 51.0.1
> - Builds from 50.0 to 51.0.1 get a downgrade of the "Application Update
> Service Helper" add-on from v1.5 to v1.0 after a browser update.

With regards to this case, I think the expected result was NOT to offer an update during the temporary 1 week hold of all Websense users(correct me if I'm wrong). Or was this not setup on release-cdntest?

Also note the double websense parameter: "(websense)(websense-8.3.2509)". The first one comes from the Hotfix that currently exists, that just searches for the qipcap.dll file, to determine if Websense is installed.

Comment 70

[[e-mail] (:bhearsum)]

(In reply to Florin Mezei, QA (:FlorinMezei) from comment #69)
> (In reply to Alexandru Simonca, QA (:asimonca) from comment #68)
> > When Websense 8.3.2509 is installed:
> > - The "app.update.url" preff contains the "(websense)(websense-8.3.2509)"
> > string for every build.
> > - Builds from 47.0.2 to 49.0.2 get an add-on update for "Websense Helper" to
> > v2.0
> > - Builds from 50.0 to 51.0.1 get an add-on update for "Application Update
> > Service Helper" updated from v1.0 to v1.5 after the add-on update on
> > "release-sysaddon"
> > - All of the builds got updated on "release-cdntest" to 51.0.1
> > - Builds from 50.0 to 51.0.1 get a downgrade of the "Application Update
> > Service Helper" add-on from v1.5 to v1.0 after a browser update.
> 
> With regards to this case, I think the expected result was NOT to offer an
> update during the temporary 1 week hold of all Websense users(correct me if
> I'm wrong). Or was this not setup on release-cdntest?

My understanding was that we didn't want them to get 52.0, so I configured the rules to hold them at 51.0.1. Unless we have concerns about websense compatibility with 51.0.1, I don't think we should offer them no update at all.

Comment 71

[Robert Strong (they/them - no direct email)]

Filed bug 1343982 to create a hotfix add-on to remove the websense hotfix add-on. This won't affect the websense system add-on and systems that get this hotfix won't have the additional values in the url caused by the websense hotfix add-on.

Comment 72

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

So, given the above reports, are we good to deploy this system add-on to the release channel?

Comment 73

[[e-mail] (:bhearsum)]

(In reply to Cory Price [:ckprice] from comment #72)
> So, given the above reports, are we good to deploy this system add-on to the
> release channel?

I don't see any reason not to below the SystemAddon. We haven't settled on the Balrog rules we'll need to block WebSense from getting app updates, but as long as we know the correct information is being set in the update URL (and it appears to be), we can do that after the SystemAddon is rolled out.

Comment 74

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum (:bhearsum) from comment #73)
> (In reply to Cory Price [:ckprice] from comment #72)
> > So, given the above reports, are we good to deploy this system add-on to the
> > release channel?
> 
> I don't see any reason not to below the SystemAddon. We haven't settled on
> the Balrog rules we'll need to block WebSense from getting app updates, but
> as long as we know the correct information is being set in the update URL
> (and it appears to be), we can do that after the SystemAddon is rolled out.

Erm, I meant "deploy" not "below".

Comment 75

[Alex Thayer [:alexical] (she/her)]

Everything regarding the system add-on looks good to me.

Comment 76

[Robert Strong (they/them - no direct email)]

In bug 1343982 it was discovered that the websense hotfix add-on was marked as compatible with 52.* though it wasthought it was only compatible with 48.*. Please make sure that the websense system add-on is only pushed to and compatible with 47.0.2-49.* and that the aushelper system add-on is only compatible with 50.0-51.*. Thanks!

Comment 77

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

OK, sounds like we are ready to ship this. Thanks everyone for diving into this so deeply.

Comment 78

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

websense/aushelper system add-ons are on release \o/

Comment 79

[Alexandru Simonca [:asimonca], Release Desktop QA]

I have tested the System Add-ons on the "release" update channel and the browser update on the "release-localtest" update channel. 
With or without Websense installed I got the same results as in Comment #68 for the System Add-ons. Everything looks good there. More detailed results in the etherpad: https://public.etherpad-mozilla.org/p/websense_fix_0303

As for the update rules on "release-localtest", I have tested:

No Websense
- 47.0-win64 (en-US) - updated to 47.0.2 but I went to "About Firefox" again after updating and it updated again to 52.0
- 49.0-win64 (en-US) - updated to 52.0

Websense 8.2.2324
- 47.0-win64 (en-US) - updated the browser to 47.0.2
- 47.0.2-win64 (en-US) - "Firefox is up to date"

Websense 8.3.2509
- 47.0-win64 (en-US) - updated to 47.0.2
- 49.0-win64 (en-US) - updated to 51.0.1

Comment 80

[[e-mail] (:bhearsum)]

(In reply to Alexandru Simonca, QA (:asimonca) from comment #79)
> I have tested the System Add-ons on the "release" update channel and the
> browser update on the "release-localtest" update channel. 
> With or without Websense installed I got the same results as in Comment #68
> for the System Add-ons. Everything looks good there. More detailed results
> in the etherpad: https://public.etherpad-mozilla.org/p/websense_fix_0303
> 
> As for the update rules on "release-localtest", I have tested:
> 
> No Websense
> - 47.0-win64 (en-US) - updated to 47.0.2 but I went to "About Firefox" again
> after updating and it updated again to 52.0
> - 49.0-win64 (en-US) - updated to 52.0
> 
> Websense 8.2.2324
> - 47.0-win64 (en-US) - updated the browser to 47.0.2
> - 47.0.2-win64 (en-US) - "Firefox is up to date"
> 
> Websense 8.3.2509
> - 47.0-win64 (en-US) - updated to 47.0.2
> - 49.0-win64 (en-US) - updated to 51.0.1

Excellent, this is exactly as expected! I'm going to move these rules to the release channel now that they've been verified.

Comment 81

[Alex Thayer [:alexical] (she/her)]

Felipe just noticed an error in the new websensehelper saying that PREF_DEFAULTS_RESET_TOPIC is not defined. After we change the update URL pref we add an observer to watch for other systems resetting the default prefs:

```
Services.obs.addObserver(observer, PREF_DEFAULTS_RESET_TOPIC, false);
```

This fails because PREF_DEFAULTS_RESET_TOPIC isn't defined. This is the second-to-last line in the add-on, and it's very ancillary functionality. Its only purpose is to help us diagnose if we successfully set the app.update.url pref, but don't see websense values in the URLs coming into Balrog.

If it's trivial to push a new version out, then I think we should, but I don't think important enough to spend much time on it, especially since we should be able to get the same information from aushelper which doesn't have this problem.

Thoughts? I'm not sure who should be making this call, so please ni? whoever should.


TLDR: Felipe noticed an error in one of our two add-ons, but it only affects our ability to log an extreme edge case through telemetry. Need someone to make the call as to whether it's worth it to push a new version of that add-on or not.

Comment 82

[[e-mail] (:bhearsum)]

(In reply to Doug Thayer [:dthayer] from comment #81)
> Felipe just noticed an error in the new websensehelper saying that
> PREF_DEFAULTS_RESET_TOPIC is not defined. After we change the update URL
> pref we add an observer to watch for other systems resetting the default
> prefs:
> 
> ```
> Services.obs.addObserver(observer, PREF_DEFAULTS_RESET_TOPIC, false);
> ```
> 
> This fails because PREF_DEFAULTS_RESET_TOPIC isn't defined. This is the
> second-to-last line in the add-on, and it's very ancillary functionality.
> Its only purpose is to help us diagnose if we successfully set the
> app.update.url pref, but don't see websense values in the URLs coming into
> Balrog.
> 
> If it's trivial to push a new version out, then I think we should, but I
> don't think important enough to spend much time on it, especially since we
> should be able to get the same information from aushelper which doesn't have
> this problem.
> 
> Thoughts? I'm not sure who should be making this call, so please ni? whoever
> should.
> 
> 
> TLDR: Felipe noticed an error in one of our two add-ons, but it only affects
> our ability to log an extreme edge case through telemetry. Need someone to
> make the call as to whether it's worth it to push a new version of that
> add-on or not.

As long as we have evidence that the update URLs are being changed properly, there's no reason from an ability-to-block-updates standpoint that we'd need to fix this.

Comment 83

[Cory Price [:ckprice] (bugmail disabled, NI me!)]

(In reply to Ben Hearsum (:bhearsum) from comment #82)
> (In reply to Doug Thayer [:dthayer] from comment #81)
> > Thoughts? I'm not sure who should be making this call, so please ni? whoever
> > should.
> As long as we have evidence that the update URLs are being changed properly,
> there's no reason from an ability-to-block-updates standpoint that we'd need
> to fix this.
This, and it sounds like it's not a user-facing issue.

Comment 84

[[e-mail] (:bhearsum)]

Now that the new WebSense System Addon has been out for a bit, and we're blocking specific bad versions from updating, does anyone have a reason I can't remove the Rule that blocks queries with "(websense)" from updating past 47.0.2? There shouldn't be anybody still sending this AFAIK.

Comment 85

[Alexandru Simonca [:asimonca], Release Desktop QA]

I have re-tested the update rules set for Websense. I did this using the "release" update channel. Here is a TL;DR version of the results:

For Websense 8.2.2324

47.0-win64 (en-US)

   - the "app.update.url" preff does not contain the "(websense)" string after forcing an update of the add-ons.
   - no extra add-on was installed
   - Updating the browser on "release" will update the browser to 47.0.2

47.0.2-win64 (en-US)

   - the "app.update.url" preff contains the "(websense-8.2.2324)" string
   - "Websense Helper" gets updated from v1.0 to v2.0
   - Trying to update the browser on "release" will give the message "Firefox is up to date"

For Websense 8.3.2509

47.0.2-win64 (en-US)

   - the "app.update.url" preff contains the "(websense)" string
   - Updating the browser on "release" is stopped. "Firefox is up to date" message is displayed in "About Firefox"

49.0-win64 (en-US)

   - the "app.update.url" preff contains the "(websense-8.3.2509)" string
   - Updating the browser on "release" takes the build to 51.0.1. Another update is found in "About Firefox" and it takes the build to 52.0

For NO Websense installed

47.0-win64 (en-US)

   - the "app.update.url" preff does not contain the "(websense)" string after forcing an update of the add-ons      
   - no extra add-on was installed
   - Updating the browser on "release" takes the build to 47.0.2. Going to "About Firefox" after the update triggers another update and takes the browser build to 52.0.
   - After the update to 47.0.2 the "app.update.url" contains the string "(nowebsense)"

49.0-win64 (en-US)

   - the "app.update.url" preff value contains the string "(nowebsense)"
   - Updating the browser on "release" takes the build to 52.0

You can check this etherpad [1] for more detailed information.
[1] - https://public.etherpad-mozilla.org/p/websense_fix_releasechannel

Comment 86

[Robert Strong (they/them - no direct email)]

(In reply to Ben Hearsum (:bhearsum) from comment #84)
> Now that the new WebSense System Addon has been out for a bit, and we're
> blocking specific bad versions from updating, does anyone have a reason I
> can't remove the Rule that blocks queries with "(websense)" from updating
> past 47.0.2? There shouldn't be anybody still sending this AFAIK.

Anyone that has not run 47.0.2 since the system add-on was deployed and has websense will still send that until after they have received the new system add-on. Also, system add-ons are not restartless on 47.0.2

Comment 87

[[e-mail] (:bhearsum)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #86)
> (In reply to Ben Hearsum (:bhearsum) from comment #84)
> > Now that the new WebSense System Addon has been out for a bit, and we're
> > blocking specific bad versions from updating, does anyone have a reason I
> > can't remove the Rule that blocks queries with "(websense)" from updating
> > past 47.0.2? There shouldn't be anybody still sending this AFAIK.
> 
> Anyone that has not run 47.0.2 since the system add-on was deployed and has
> websense will still send that until after they have received the new system
> add-on. Also, system add-ons are not restartless on 47.0.2

How long do we need to wait to feel comfortable that these people have the new SystemAddon? Given that most WebSense users are business', and we've gone through at least one weekend, it seems unlikely that there's still a notable number of people on the old WebSense System Addon?

Comment 88

[Robert Strong (they/them - no direct email)]

(In reply to Ben Hearsum (:bhearsum) from comment #87)
> (In reply to Robert Strong [:rstrong] (use needinfo to contact me) from
> comment #86)
> > (In reply to Ben Hearsum (:bhearsum) from comment #84)
> > > Now that the new WebSense System Addon has been out for a bit, and we're
> > > blocking specific bad versions from updating, does anyone have a reason I
> > > can't remove the Rule that blocks queries with "(websense)" from updating
> > > past 47.0.2? There shouldn't be anybody still sending this AFAIK.
> > 
> > Anyone that has not run 47.0.2 since the system add-on was deployed and has
> > websense will still send that until after they have received the new system
> > add-on. Also, system add-ons are not restartless on 47.0.2
> 
> How long do we need to wait to feel comfortable that these people have the
> new SystemAddon? Given that most WebSense users are business', and we've
> gone through at least one weekend, it seems unlikely that there's still a
> notable number of people on the old WebSense System Addon?

If a Windows system is not sending the value then they either don't have the system add-on or for all practical purposes don't have the system add-on. It shouldn't be a case of whether we are comfortable or not since that state answers the question. In relation to the websense system add-on being successful which was seen with the original system add-on there are plans to investigate that question further with the latest system which doesn't use the same method as the original system add-on. I also mentioned 47.0.2 system add-ons not being restartless since a client that hasn't run for awhile and launches will not have the websense system add-on activated until the client restarts.

In my opinion it should be fine to at least wait until the analysis of the latest websense system add-on but the final call should be done by release drivers based on the number of users affected along with the above information, etc.

Comment 89

[[e-mail] (:bhearsum)]

I think this bug is done? We:
* Are holding users with known bad versions of WebSense on their last supported release
* Are holding users whose WebSense status is unknown on their current release
* Are allowing users who we know do NOT have WebSense to update the latest release
* Allowed a percantage of users with non-blacklisted versions of WebSense to update to latest, and decided things looked good enough to allow the rest of these users to update - so all users with a non-blacked listed version of WebSense are now updating to the latest realese.

Comment 90

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum (:bhearsum) from comment #89)
> I think this bug is done? We:
> * Are holding users with known bad versions of WebSense on their last
> supported release
> * Are holding users whose WebSense status is unknown on their current release
> * Are allowing users who we know do NOT have WebSense to update the latest
> release
> * Allowed a percantage of users with non-blacklisted versions of WebSense to
> update to latest, and decided things looked good enough to allow the rest of
> these users to update - so all users with a non-blacked listed version of
> WebSense are now updating to the latest realese.

And for posterity, we will be removing most of this as part of shipping 53.0. We will continue to hold known bad versions on their last supported release, but we will not delay updates for other WebSense versions nor hold back anyone with an unknown WebSense status.