Closed
Bug 985627
Opened 11 years ago
Closed 11 years ago
temporarily turn off malware blocklist and allowlists
Categories
(Core :: DOM: Security, defect)
Core
DOM: Security
Tracking
()
RESOLVED
FIXED
mozilla28
Tracking | Status | |
---|---|---|
firefox28 | + | --- |
People
(Reporter: mmc, Assigned: Felipe)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
gcp
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
MattN
:
review+
|
Details | Diff | Splinter Review |
(deleted),
application/x-xpinstall
|
Details |
See Bug 985623.
Reporter | ||
Updated•11 years ago
|
Assignee: nobody → mmc
Reporter | ||
Comment 1•11 years ago
|
||
Reporter | ||
Updated•11 years ago
|
Attachment #8393703 -
Flags: review?(gpascutto)
Updated•11 years ago
|
Attachment #8393703 -
Flags: review?(gpascutto) → review+
Comment 2•11 years ago
|
||
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
Reporter | ||
Comment 3•11 years ago
|
||
(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
Reporter | ||
Comment 4•11 years ago
|
||
Felipe, Matt: Gavin said you might be able to help converting this to a hotfix.
Thanks,
Monica
Flags: needinfo?(felipc)
Flags: needinfo?(MattN+bmo)
Comment 5•11 years ago
|
||
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.
Updated•11 years ago
|
QA Contact: mwobensmith
Comment 6•11 years ago
|
||
>- this change is required only in Firefox 28, on all platforms
Only on Windows. The pref is empty on all other platforms, IIRC.
Comment 7•11 years ago
|
||
(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)
Assignee | ||
Comment 8•11 years ago
|
||
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.
Comment 9•11 years ago
|
||
(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)
Comment 10•11 years ago
|
||
Or both 27.0.1 and 28.0, to catch the 10% who have already upgraded
Assignee | ||
Comment 11•11 years ago
|
||
This hotfix sets both prefs to an empty string, on all platforms, and targets Firefox 27.0 - 28.*.
(Untested)
Flags: needinfo?(felipc)
Reporter | ||
Comment 12•11 years ago
|
||
(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)
Assignee | ||
Comment 13•11 years ago
|
||
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
Assignee | ||
Comment 14•11 years ago
|
||
Unsigned xpi if someone from QA wants to start testing it
Reporter | ||
Comment 15•11 years ago
|
||
(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?
Comment 16•11 years ago
|
||
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 17•11 years ago
|
||
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+
Assignee | ||
Comment 18•11 years ago
|
||
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.
Assignee | ||
Comment 19•11 years ago
|
||
Filed bug 985689 for the xpi signing
Comment 20•11 years ago
|
||
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 ?
Comment 21•11 years ago
|
||
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
Comment 22•11 years ago
|
||
The XPI has been signed and is now up on staging. I think it's ready to test.
Flags: needinfo?(anthony.s.hughes)
Assignee | ||
Comment 23•11 years ago
|
||
For easy reference, test instructions here: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/Hotfix
Assignee | ||
Comment 24•11 years ago
|
||
(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
Comment 25•11 years ago
|
||
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)
Comment 26•11 years ago
|
||
(In reply to :Felipe Gomes from comment #24)
Ah sorry, I should've checked the patch more carefully.
Reporter | ||
Comment 27•11 years ago
|
||
(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
Comment 29•11 years ago
|
||
>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.
Comment 30•11 years ago
|
||
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-*.
Reporter | ||
Comment 31•11 years ago
|
||
(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.
Comment 32•11 years ago
|
||
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.
Reporter | ||
Comment 33•11 years ago
|
||
(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.
Comment 34•11 years ago
|
||
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 | ||
Updated•11 years ago
|
Assignee: mmc → felipc
Reporter | ||
Comment 35•11 years ago
|
||
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)
Comment 36•11 years ago
|
||
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)
Updated•11 years ago
|
tracking-firefox28:
--- → +
Comment 37•11 years ago
|
||
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)
Comment 39•11 years ago
|
||
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)
Comment 40•11 years ago
|
||
Seems like we should call this FIXED now?
Assignee | ||
Comment 41•11 years ago
|
||
Yep
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Target Milestone: --- → mozilla31
Updated•11 years ago
|
Target Milestone: mozilla31 → mozilla28
You need to log in
before you can comment on or make changes to this bug.
Description
•