Google is planning a change in the protocol for SafeBrowsing: "Currently, we say that cached full hash results may be used as long as the client has successfully updated within the last 45 minutes. This point implicitly assumes that we'll send all of the subs that the client needs anytime they update. We would like to give ourselves more flexibility to rate-limit the subs based on current load. To do this, we'd need to ensure that clients are clearing any cached full hashes each time they send an update request, whether the update was successful or not. That way, we'll still be able to ensure quick removal of false positives via the full hash ping." In our implementation, we probably need to distinguish between an update caused by receiving a completion from the completion server (for which we should not clear the Completes or I'd be rather pointless), and an update from the regular server, where we do want to clear the Completes unconditionally.

Probably too late to uplift this to Firefox 17, but we'll have to remember to update the ESR with this.

This looks like something that can be tested, so I'll try to add a test for this in this bug.

https://tbpl.mozilla.org/?tree=Try&rev=cf9471af0c40 https://hg.mozilla.org/integration/mozilla-inbound/rev/31f1ffbffdb0 https://hg.mozilla.org/integration/mozilla-inbound/rev/d5adfb8bfc81

https://hg.mozilla.org/mozilla-central/rev/31f1ffbffdb0 https://hg.mozilla.org/mozilla-central/rev/d5adfb8bfc81

gcp: what's google's timeline on this? If you want it in Firefox esr-17 then do we also need it in Firefox 18? What about ESR-10? After next week's release we have one more planned release (to correspond with the release of Firefox 18) so that branch will live on another 12 weeks until EOL.

Google will enable the new behavior somewhere in Q1 2013 (AFAIK), but this doesn't mean they will turn off support for the old behavior - when they do that also depends on how long we need it. Basically we should look where getting this patch is reasonable for us, and agree with them based on that. The patch can't really be applied to Firefox 10-based ESR, because it has a completely different backend. We'd have to redo it completely for that version, and with ESR-17 around the corner, I think that's a bad idea. My plan was to let this ride the trains to Firefox 19, and if that turns out fine, uplift it to the next ESR at that point. I think this means we'll update the (by-then-17-based?) ESR at the release of Firefox 20 end of March. Google will change their behavior based on the client version we report, instead of protocol version we report, for reasons of their own (they want to put some more stuff in the next protocol version). I think this potentially creates complications in disambiguating Firefox 17 clients (ESR with patch, or "outdated" without patch). On the other hand, we do say clearly that old Firefox versions are security-vulnerable at the moment we release the next one, so I think we don't have to care much for people that stay on 17-non-ESR-release.

[Approval Request Comment] Bug caused by (feature/regressing bug #): bug 820283 (original cause is bug 673470) User impact if declined: Malware/phishing warnings may stay forever even if sites are clean. Testing completed (on m-c, etc.): Has been riding the train and is on Aurora Risk to taking this patch (and alternatives if risky): Given the testing it had, mostly any potential unforeseen interactions due to small differences between Aurora and Beta code. There is a smaller patch in bug 820283, but it has not been riding the train like this one. This one solves 820283 as a side-effect.

[Approval Request Comment] Same as patch 1. We have tests. Tests are cool, I wish we had more of them. Especially SafeBrowsing.

Comment on attachment 694103 [details] [diff] [review] Patch 1b. Rebased to mozilla-beta Took the patch in bug 820283 instead.

Marking FF18 as fixed per bug 820283, which has landed on mozilla-beta.

This isn't fixed in Firefox 18. We took the workaround from bug 820283 instead as you already indicated. That will fix 820283 but not this bug.

We took bug 820283 in the ESR released alongside FF18, so no need to track this.

As already stated, *this* bug isn't fixed in the ESR, and we want it, as indicated in basically all previous comments. We need to track this for ESR.

(In reply to Gian-Carlo Pascutto (:gcp) from comment #14) > As already stated, *this* bug isn't fixed in the ESR, and we want it, as > indicated in basically all previous comments. We need to track this for ESR. I didn't wontfix because the ESR is unaffected by this bug, I wontfix'd because it's not clear that this bug actually represents a user critical issue in any way (those were resolved by bug 820283). I don't think we've gotten word from Google that they plan to break ESR17 SafeBrowsing, but feel free to correct me.

The statement was: "We don't have any timetable currently for deprecating version 2.2. We'll let you know if we decide to do so, and give a few months notice -- how long do you need for the ESR releases?" I replied that, from my side, I would be confident in taking this as soon as Firefox 19 is out in the wild. If your position is that we wait until they deprecate the protocol, then I believe we have here what is called a "deadly embrace". :-)

Leave this for Alex to decide.

If the patch is ready and low risk, let's get an approval for ESR on this bug. Thanks gcp.

Comment on attachment 677816 [details] [diff] [review] Patch 1. Flush the completion cache on any update [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Protocol update for a partner service (Google SafeBrowsing) User impact if declined: Slightly higher bandwidth usage if the new protocol can't be used. Potentially loss of SafeBrowsing should Google not keep the old protocol alive (though quite unlikely they'd do this). Fix Landed on Version: Risk to taking this patch (and alternatives if risky): Low, it's been out in the field for a while, and it lowers the possibility of other safebrowsing bugs having a lasting effect. String or UUID changes made by this patch:

I presume that needinfo was directed a me.

Comment on attachment 678309 [details] [diff] [review] Patch 2. Modify and add tests for new behavior See above. We like tests.

https://hg.mozilla.org/releases/mozilla-esr17/rev/9e17def1f3b9 https://hg.mozilla.org/releases/mozilla-esr17/rev/c7f42dec3730