Closed Bug 985627 Opened 10 years ago Closed 10 years ago

temporarily turn off malware blocklist and allowlists

Categories

(Core :: DOM: Security, defect)

Product:
Core
Component:
DOM: Security
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla28
Tracking Flags:
Tracking Status
firefox28 + ---

People

(Reporter: mmc, Assigned: Felipe)

References

Details

Attachments

(3 files, 1 obsolete file)

Temporarily turn off malware allowlist and blocklist (
1.52 KB, patch
gcp
: review+
Details | Diff | Splinter Review
Hotfix to temporarily turn off malware blocklist and whitelist
5.19 KB, patch
MattN
: review+
Details | Diff | Splinter Review
hotfix-v20140319.01.xpi unsigned
11.74 KB, application/x-xpinstall
Details
Assignee: nobody → mmc
Attachment #8393703 - Flags: review?(gpascutto)
Attachment #8393703 - Flags: review?(gpascutto) → review+
Does this disable application reputation checking entirely for this release?

This patch works if we do a re-spin, but if we try a hotfix add-on you'd need something different.
Blocks: 985623
(In reply to Daniel Veditz [:dveditz] from comment #2)
> Does this disable application reputation checking entirely for this release?

This list isn't consulted until FF 30.

> This patch works if we do a re-spin, but if we try a hotfix add-on you'd
> need something different.

Are there instructions for this anywhere?

Thanks,
Monica
Felipe, Matt: Gavin said you might be able to help converting this to a hotfix.

Thanks,
Monica
Flags: needinfo?(felipc)
Flags: needinfo?(MattN+bmo)
Sounds like we're going to want to hotfix this.

My understanding:
- this change is required only in Firefox 28, on all platforms
- to simplify things, we can just unconditionally set the relevant prefs from attachment 8393703 [details] [diff] [review] to blank (user-set value). When we want to re-enable this feature later, that will require re-naming the prefs so that existing profiles with the user-set value start working again.
QA Contact: mwobensmith
>- this change is required only in Firefox 28, on all platforms

Only on Windows. The pref is empty on all other platforms, IIRC.
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #5)
> - to simplify things, we can just unconditionally set the relevant prefs
> from attachment 8393703 [details] [diff] [review] to blank (user-set value).
> When we want to re-enable this feature later, that will require re-naming
> the prefs so that existing profiles with the user-set value start working
> again.

Monica, does that sound acceptable?
Flags: needinfo?(mmc)
Flags: needinfo?(felipc)
Flags: needinfo?(MattN+bmo)
I can create a hotfix version of this, it should be easy.

Note that even after we release a hotfix, every user will briefly have an unhotfixed version 28 when they just got upgraded to it.
If this is a problem we would probably need a 28.0.1 release in addition to the hotfix.
(In reply to :Felipe Gomes from comment #8)
> I can create a hotfix version of this, it should be easy.
> 
> Note that even after we release a hotfix, every user will briefly have an
> unhotfixed version 28 when they just got upgraded to it.
> If this is a problem we would probably need a 28.0.1 release in addition to
> the hotfix.

Can we do a hotfix for this to 27.0.1 users in order to circumvent that?
Flags: needinfo?(felipc)
Or both 27.0.1 and 28.0, to catch the 10% who have already upgraded
This hotfix sets both prefs to an empty string, on all platforms, and targets Firefox 27.0 - 28.*.

(Untested)
Flags: needinfo?(felipc)
(In reply to Gian-Carlo Pascutto (:gcp) from comment #6)
> >- this change is required only in Firefox 28, on all platforms
> 
> Only on Windows. The pref is empty on all other platforms, IIRC.

goog-badbinurl-shavar is present on all platforms.
Flags: needinfo?(mmc)
Better, now tested.  I tested locally on Mac that it works on 27, the upgrade from 27->28 won't reset the prefs, and it works for users already on 28.
Attachment #8393734 - Attachment is obsolete: true
Unsigned xpi if someone from QA wants to start testing it
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #7)
> (In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #5)
> > - to simplify things, we can just unconditionally set the relevant prefs
> > from attachment 8393703 [details] [diff] [review] to blank (user-set value).
> > When we want to re-enable this feature later, that will require re-naming
> > the prefs so that existing profiles with the user-set value start working
> > again.
> 
> Monica, does that sound acceptable?

I see -- this is ok, but is there any way to clean up unused prefs?
Yes, a migration step in migrateUI could do this for Firefox e.g. https://mxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#1450
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
Comment on attachment 8393741 [details] [diff] [review]
Hotfix to temporarily turn off malware blocklist and whitelist

Review of attachment 8393741 [details] [diff] [review]:
-----------------------------------------------------------------

The hotfix stuff looks fine to me and the pref changes align with the product patch in the bug.

::: v20140319.01/bootstrap.js
@@ +49,5 @@
>   */
>  function shouldHotfixApp() {
>    // Ensure that this is the correct version in case compatibility checking is overridden.
> +  if (Services.vc.compare(Services.appinfo.version, "27.0") < 0 ||
> +      Services.vc.compare(Services.appinfo.version, "28.*") > 0) {

I think we'd only want to target 28.0.0 if we are also going to land the product fix on the release branch.

::: v20140319.01/install.rdf
@@ +18,5 @@
>        <Description>
>          <!-- Firefox -->
>          <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
> +        <em:minVersion>27.0</em:minVersion>
> +        <em:maxVersion>28.*</em:maxVersion>

Ditto
Attachment #8393741 - Flags: review+
https://hg.mozilla.org/releases/firefox-hotfixes/rev/844b1d518c93

Pushed with 28.* per IRC. I can push a simple follow-up to change that if needed.
Depends on: 985689
Filed bug 985689 for the xpi signing
Could you record here why we don't have 27.0, from the point of avoiding a surge of traffic between updating to 28.0 and getting the hotfix applied ?
Setting myself as QA Contact to test once this is ready. Please need-info me when this is ready to test.
QA Contact: mwobensmith → anthony.s.hughes
The XPI has been signed and is now up on staging. I think it's ready to test.
Flags: needinfo?(anthony.s.hughes)
(In reply to Nick Thomas [:nthomas] from comment #20)
> Could you record here why we don't have 27.0, from the point of avoiding a
> surge of traffic between updating to 28.0 and getting the hotfix applied ?

Nick, 27.0.1 was mentioned in some comments in the bug, but the hotfix actually targets 27.0
Apart from changing the values of the two prefs is there something else I can be checking? In other words, how can I test that the hotfix is having the desired effect on safebrowsing pings? Is this testing even necessary? Thanks.
Flags: needinfo?(anthony.s.hughes) → needinfo?(mmc)
(In reply to :Felipe Gomes from comment #24)
Ah sorry, I should've checked the patch more carefully.
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #25)
> Apart from changing the values of the two prefs is there something else I
> can be checking? In other words, how can I test that the hotfix is having
> the desired effect on safebrowsing pings? Is this testing even necessary?
> Thanks.

Hi Anthony,

I have tested manually on Linux and Mac but would appreciate another set of eyes to make sure that updates aren't broken.

0) Use windows, or a clean profile on any platform, because only Windows updates timestamps properly in the case of not receiving safebrowsing updates.
1) Apply the hotfix
2) Wait 45 minutes (the maximum length of time we go without receiving safebrowsing updates)
3) Check in the Windows profile folder (*not* the roaming profile) that goog-phish-shavar* and goog-malware-shavar* have been updated in the last 45 minutes. On Mac that profile directory is ~/Library/Caches/Firefox/Profiles/<profile_name>/safebrowsing. On Linux, it's in ~/.cache/mozilla/firefox/<profile_name>/safebrowsing.

Thanks,
Monica
Thanks Monica, I'll do that now.
Flags: needinfo?(mmc)
>3) Check in the Windows profile folder (*not* the roaming profile) that goog-phish-shavar* and goog-malware-shavar* have been updated in the last 45 minutes. On Mac that profile directory is ~/Library/Caches/Firefox/Profiles/<profile_name>/safebrowsing. On Linux, it's in ~/.cache/mozilla/firefox/<profile_name>/safebrowsing.

...and check that this directory DOES NOT have goog-badbinurl-shavar and goog-downloadwhite-digest256 files.
To explain more clearly: on a clean profile, you should not be getting those files. On a not-clean profile, they should not be updating, unlike goog-phish-* and goog-malware-*.
(In reply to Gian-Carlo Pascutto (:gcp) from comment #30)
> To explain more clearly: on a clean profile, you should not be getting those
> files. On a not-clean profile, they should not be updating, unlike
> goog-phish-* and goog-malware-*.

Yeah, thanks for clarifying. It is probably safer to blow these away in the safebrowsing folder before testing, to avoid confusion.
I've finished testing this on staging and did not encounter any issues. In all cases goog-badbinurl-shavar and goog-downloadwhite-digest256 did not exist, goog-phish-* and goog-malware-* were update after ~30 minutes (+/- 5 minutes). You can see more details about exactly what I tested here:
https://wiki.mozilla.org/Releases/Firefox_28/Test_Plan#Staging

If you need more coverage please let me know.
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #32)
> I've finished testing this on staging and did not encounter any issues. In
> all cases goog-badbinurl-shavar and goog-downloadwhite-digest256 did not
> exist, goog-phish-* and goog-malware-* were update after ~30 minutes (+/- 5
> minutes). You can see more details about exactly what I tested here:
> https://wiki.mozilla.org/Releases/Firefox_28/Test_Plan#Staging
> 
> If you need more coverage please let me know.

Sounds perfect! Thank you so much, and especially for moving the documentation to a more stable location.
Be advised that if we plan to push this live tomorrow or Friday that I'm largely unavailable for testing due to my attendance at GDC. Juan Becerra should be able to help test in my absence.
Assignee: mmc → felipc
Lukas, where do we go from here? I heard there might be another chemspill next week. Do I try to get https://bugzilla.mozilla.org/attachment.cgi?id=8393703 into that, in case users don't get the hotfix?

Do we know when the hotfix rolls out to the people who have updated to 28, so I can ask Google to verify on their side?
Flags: needinfo?(lsblakk)
Monica - we'll push the hotfix today and get testing on it, that gives time to ensure it's working before the weekend.  There's not any chemspill on the horizon, so we'll track the issue in case something we don't know about yet surfaces so it could ride along if that's deemed useful (it might not be, given the hotfix).
Flags: needinfo?(lsblakk)
Jorge - please put this hotfix into production

Setting ni? on Juan to help with testing this once it's live in production.
Flags: needinfo?(jorge)
Flags: needinfo?(jbecerra)
The new version is up on prod and ready to test.
Flags: needinfo?(jorge)
I've tested several combinations of the scenarios described in the link in comment #32, and it looks like the hotfix is working according plan.
Flags: needinfo?(jbecerra)
Seems like we should call this FIXED now?
Yep
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Target Milestone: mozilla31 → mozilla28
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: