362788 - New Firefox versions result in a new pref entry for rc4 key

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Tony Chang (Google)]

Anti-phishing uses rc4 to encrypt urls in the "ask remote server" mode.  For this to work, the client needs to first download the rc4 key.  We only download the rc4 key once per day, but we use the key url (so each provider can have a separate key url) to determine if we've downloaded the key in the last 24 hours.  Since the client id is in the key and the client id includes the firefox version number, we end up grabbing a new key (and creating a new pref value) for each firefox version.  For example, I now have the following entries:
urlclassifier.keyupdatetime.https://sb-ssl.google.com/safebrowsing/getkey?client=Firefox2.0&
urlclassifier.keyupdatetime.https://sb-ssl.google.com/safebrowsing/getkey?client=Firefox2.0.0.1pre&

We should ignore query params to avoid this unnecessary duplication.

Comment 1

[Tony Chang (Google)]

Attachment: only use getkey URL up until ?

includes small test case

Comment 2

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 248335 [details] [diff] [review]
only use getkey URL up until ?

Looks good to me.

Comment 3

[Tony Chang (Google)]

on trunk.

Checking in testing/mochitest/tests/index.html;
/cvsroot/mozilla/testing/mochitest/tests/index.html,v  <--  index.html
new revision: 1.52; previous revision: 1.51
done
RCS file: /cvsroot/mozilla/testing/mochitest/tests/test_bug362788.xhtml,v
done
Checking in testing/mochitest/tests/test_bug362788.xhtml;
/cvsroot/mozilla/testing/mochitest/tests/test_bug362788.xhtml,v  <--
test_bug362788.xhtml
initial revision: 1.1
done
Checking in
toolkit/components/url-classifier/content/url-crypto-key-manager.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/url-crypto-key-manager.js,v
<--  url-crypto-key-manager.js
new revision: 1.10; previous revision: 1.9
done

Comment 4

[Tony Chang (Google)]

Attachment: patch for branch

exactly the same as the trunk patch without the unittests (no mochitest on branch)

Comment 5

[Jay Patel [:jay]]

Comment on attachment 249317 [details] [diff] [review]
patch for branch

Approved for 1.8 branch, a=jay for drivers.

Comment 6

[Tony Chang (Google)]

on branch

Checking in url-crypto-key-manager.js;
/cvsroot/mozilla/toolkit/components/url-classifier/content/url-crypto-key-manager.js,v  <--  url-crypto-key-manager.js
new revision: 1.1.2.8; previous revision: 1.1.2.7
done

Comment 7

[Jeff Walden [:Waldo]]

http://lxr.mozilla.org/mozilla/source/testing/mochitest/tests/test_bug362788.xhtml

Comment 8

[Jay Patel [:jay]]

Tony or Monica:  Is there an easy way to test this?  If your team can verify this with the latest 2.0.0.2 RC builds, that will be great.

For future reference, I was wondering if the existing keys in the client can be removed?  If so, I can try deleting th 5-6 entries I have currently and try running my profile with various builds/version to verify this myself.  Thanks in advance.

Comment 9

[Tony Chang (Google)]

Steps to test:
1) start ff and switch to remote lookup (key request is disabled for local list mode in bug 354199)
2) look at the pref value in urlclassifier.keyupdatetime.*  It shouldn't contain a version number.

It's safe to remove the other prefs.  The pref is only used to limit the request to once a day.  Deleting the pref just causes an extra request.

This works for me using this build:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla1.8/firefox-2.0.0.2pre.en-US.linux-i686.tar.gz

Comment 10

[Jay Patel [:jay]]

Thanks Tony!  FYI: Changing the bug status to VERIFIED should only be done for Trunk verifications.  If you verify on a branch, please replace the fixed1.8.x.x keyword with verified1.8.x.x.

Replacing fixed1.8.1.2 keyword with verified1.8.1.2.  Returning status to RESOLVED FIXED