Closed Bug 1288840 Opened 5 years ago Closed 5 years ago

Use the full social engineering list in official Mozilla builds

Categories

(Toolkit :: Safe Browsing, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla50
Tracking Status
firefox50 --- verified

People

(Reporter: francois, Assigned: hchang)

References

Details

Attachments

(1 file)

We are allowed to use the "internal" social engineering list in official Mozilla builds, so we should change from SOCIAL_ENGINEERING_PUBLIC (2) here:

https://hg.mozilla.org/mozilla-central/rev/29ead859749a#l2.124

to SOCIAL_ENGINEERING (5) as defined here:

https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/toolkit/components/url-classifier/chromium/safebrowsing.proto#266

based on the presence of this compile-time variable:

https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/modules/libpref/init/all.js#5146

In addition, we should change this in the V2 code too.

So what I would recommend is:

1. Add two entries to the conversion table for V4 (https://hg.mozilla.org/mozilla-central/rev/29ead859749a#l2.122):

  googpub-phish-proto -> SOCIAL_ENGINEERING_PUBLIC (2)
  goog-phish-proto -> SOCIAL_ENGINEERING (5)

2. Add both the public (googpub-phish-) and the private (goog-phish-) lists to these prefs:

  browser.safebrowsing.provider.google.lists
  browser.safebrowsing.provider.google4.lists

3. Use "#ifdef MOZILLA_OFFICIAL" around the urlclassifier.phishTable pref in https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/modules/libpref/init/all.js#5062 to choose between goog-phish-shavar and googpub-phish-shavar (V2).
Assignee: nobody → hchang
(In reply to François Marier [:francois] from comment #0)
> We are allowed to use the "internal" social engineering list in official
> Mozilla builds, so we should change from SOCIAL_ENGINEERING_PUBLIC (2) here:
> 
> https://hg.mozilla.org/mozilla-central/rev/29ead859749a#l2.124
> 
> to SOCIAL_ENGINEERING (5) as defined here:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/toolkit/components/url-classifier/
> chromium/safebrowsing.proto#266
> 
> based on the presence of this compile-time variable:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> 4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/modules/libpref/init/all.js#5146
> 
> In addition, we should change this in the V2 code too.
> 
> So what I would recommend is:
> 
> 1. Add two entries to the conversion table for V4
> (https://hg.mozilla.org/mozilla-central/rev/29ead859749a#l2.122):
> 
>   googpub-phish-proto -> SOCIAL_ENGINEERING_PUBLIC (2)
>   goog-phish-proto -> SOCIAL_ENGINEERING (5)
> 
> 2. Add both the public (googpub-phish-) and the private (goog-phish-) lists
> to these prefs:
> 
>   browser.safebrowsing.provider.google.lists
>   browser.safebrowsing.provider.google4.lists
> 
> 3. Use "#ifdef MOZILLA_OFFICIAL" around the urlclassifier.phishTable pref in
> https://dxr.mozilla.org/mozilla-central/rev/
> 4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/modules/libpref/init/all.js#5062 to
> choose between goog-phish-shavar and googpub-phish-shavar (V2).

Hi Francois,

Just one question: what does "public" mean here :p Does the private database has the blacklist of websites which  has higher chance to be false alarm?

Thanks :)
Flags: needinfo?(francois)
Just curious. I haven't seen chromium using the non-public "SOCIAL_ENGINEERING" threat type. Is the private one still not ready for use?

[1] https://cs.chromium.org/search/?q=SOCIAL_ENGINEERING&sq=package:chromium&type=cs
Status: NEW → ASSIGNED
Comment on attachment 8774196 [details]
Bug 1288840 - Use the private phishing site list in official build.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66742/diff/1-2/
(In reply to Henry Chang [:henry][:hchang] from comment #1)
> Just one question: what does "public" mean here :p Does the private database
> has the blacklist of websites which  has higher chance to be false alarm?

Public is the phishing list that Google is free to let everyone use. Private is the same as the public list but it also includes extra data providers which Google isn't allowed to let everyone use. The license they have allows them to share it with us.

The level of false positives should be very low in both cases.
Flags: needinfo?(francois)
(In reply to Henry Chang [:henry][:hchang] from comment #2)
> Just curious. I haven't seen chromium using the non-public
> "SOCIAL_ENGINEERING" threat type. Is the private one still not ready for use?

According to Google, it should be ready to use. If we run into problems we'll report them to Google.
Comment on attachment 8774196 [details]
Bug 1288840 - Use the private phishing site list in official build.

https://reviewboard.mozilla.org/r/66742/#review64916

I was going to give you an r+, but we can't land with the typo :)

::: modules/libpref/init/all.js:5063
(Diff revision 2)
> +#ifdef MOZILLA_OFFICIAL
> +// In the official build, we are allowed to use google's private
> +// phishing list "goog-phish-shavar". See Bug 1288840.
>  pref("urlclassifier.phishTable", "goog-phish-shavar,test-phish-simple");
> +#else
> +// Otherwise, we should use the public phishing list "googpub-phish-shavar".

nit: I would personally leave this comment out. You already link to the bug number in the main clause of that ifdef and explain it well, so it's not really necessary in the else IMHO.

::: modules/libpref/init/all.js:5119
(Diff revision 2)
>  pref("browser.safebrowsing.downloads.remote.block_uncommon",             true);
>  pref("browser.safebrowsing.debug", false);
>  
>  // The protocol version we communicate with google server.
>  pref("browser.safebrowsing.provider.google.pver", "2.2");
> -pref("browser.safebrowsing.provider.google.lists", "goog-badbinurl-shavar,goog-downloadwhite-digest256,goog-phish-shavar,goog-malware-shavar,goog-unwanted-shavar");
> +pref("browser.safebrowsing.provider.google.lists", "goog-badbinurl-shavar,goog-downloadwhite-digest256,goog-phish-shavar,googpub-phish-sharvar,goog-malware-shavar,goog-unwanted-shavar");

typo: sharvar
Attachment #8774196 - Flags: review?(francois) → review-
Thanks for the review :)

I remember you mentioned in the mail that it'd be better to land this patch in firefox 50. Do you still think so or has changed your mind?

(In reply to François Marier [:francois] from comment #7)
> Comment on attachment 8774196 [details]
> Bug 1288840 - Use the private phishing site list in official build.
> 
> https://reviewboard.mozilla.org/r/66742/#review64916
> 
> I was going to give you an r+, but we can't land with the typo :)
> 
> ::: modules/libpref/init/all.js:5063
> (Diff revision 2)
> > +#ifdef MOZILLA_OFFICIAL
> > +// In the official build, we are allowed to use google's private
> > +// phishing list "goog-phish-shavar". See Bug 1288840.
> >  pref("urlclassifier.phishTable", "goog-phish-shavar,test-phish-simple");
> > +#else
> > +// Otherwise, we should use the public phishing list "googpub-phish-shavar".
> 
> nit: I would personally leave this comment out. You already link to the bug
> number in the main clause of that ifdef and explain it well, so it's not
> really necessary in the else IMHO.
> 
> ::: modules/libpref/init/all.js:5119
> (Diff revision 2)
> >  pref("browser.safebrowsing.downloads.remote.block_uncommon",             true);
> >  pref("browser.safebrowsing.debug", false);
> >  
> >  // The protocol version we communicate with google server.
> >  pref("browser.safebrowsing.provider.google.pver", "2.2");
> > -pref("browser.safebrowsing.provider.google.lists", "goog-badbinurl-shavar,goog-downloadwhite-digest256,goog-phish-shavar,goog-malware-shavar,goog-unwanted-shavar");
> > +pref("browser.safebrowsing.provider.google.lists", "goog-badbinurl-shavar,goog-downloadwhite-digest256,goog-phish-shavar,googpub-phish-sharvar,goog-malware-shavar,goog-unwanted-shavar");
> 
> typo: sharvar
Flags: needinfo?(francois)
I've verified the new prefs do not break the table update :)

henry@local:/home/hchang/dev/mi-dev$ ll obj-x86_64-pc-linux-gnu/tmp/scratch_user/safebrowsing/
total 3100
drwxr-xr-x  2 henry henry   4096 Jul 29 11:07 ./
drwxrwxr-x 14 henry henry   4096 Jul 29 11:10 ../
-rw-rw-r--  1 henry henry     12 Jul 29 11:07 goog-badbinurl-shavar.cache
-rw-rw-r--  1 henry henry 134572 Jul 29 11:07 goog-badbinurl-shavar.pset
-rw-rw-r--  1 henry henry  89090 Jul 29 11:07 goog-badbinurl-shavar.sbstore
-rw-rw-r--  1 henry henry     12 Jul 29 11:07 goog-malware-shavar.cache
-rw-rw-r--  1 henry henry 356138 Jul 29 11:07 goog-malware-shavar.pset
-rw-rw-r--  1 henry henry 698465 Jul 29 11:07 goog-malware-shavar.sbstore
-rw-rw-r--  1 henry henry     12 Jul 29 11:07 googpub-phish-shavar.cache
-rw-rw-r--  1 henry henry 325294 Jul 29 11:07 googpub-phish-shavar.pset
-rw-rw-r--  1 henry henry 324207 Jul 29 11:07 googpub-phish-shavar.sbstore
-rw-rw-r--  1 henry henry     12 Jul 29 11:07 goog-unwanted-shavar.cache
-rw-rw-r--  1 henry henry 177650 Jul 29 11:07 goog-unwanted-shavar.pset
-rw-rw-r--  1 henry henry 163819 Jul 29 11:07 goog-unwanted-shavar.sbstore
-rw-rw-r--  1 henry henry  54988 Jul 29 11:07 mozstd-track-digest256.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 mozstd-track-digest256.pset
-rw-rw-r--  1 henry henry  62044 Jul 29 11:07 mozstd-track-digest256.sbstore
-rw-rw-r--  1 henry henry 309708 Jul 29 11:07 mozstd-trackwhite-digest256.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 mozstd-trackwhite-digest256.pset
-rw-rw-r--  1 henry henry 348604 Jul 29 11:07 mozstd-trackwhite-digest256.sbstore
-rw-rw-r--  1 henry henry     44 Jul 29 11:07 test-block-simple.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 test-block-simple.pset
-rw-rw-r--  1 henry henry    232 Jul 29 11:07 test-block-simple.sbstore
-rw-rw-r--  1 henry henry     44 Jul 29 11:07 test-malware-simple.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 test-malware-simple.pset
-rw-rw-r--  1 henry henry    232 Jul 29 11:07 test-malware-simple.sbstore
-rw-rw-r--  1 henry henry     44 Jul 29 11:07 test-phish-simple.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 test-phish-simple.pset
-rw-rw-r--  1 henry henry    232 Jul 29 11:07 test-phish-simple.sbstore
-rw-rw-r--  1 henry henry     76 Jul 29 11:07 test-track-simple.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 test-track-simple.pset
-rw-rw-r--  1 henry henry    272 Jul 29 11:07 test-track-simple.sbstore
-rw-rw-r--  1 henry henry     44 Jul 29 11:07 test-trackwhite-simple.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 test-trackwhite-simple.pset
-rw-rw-r--  1 henry henry    232 Jul 29 11:07 test-trackwhite-simple.sbstore
-rw-rw-r--  1 henry henry     44 Jul 29 11:07 test-unwanted-simple.cache
-rw-rw-r--  1 henry henry     16 Jul 29 11:07 test-unwanted-simple.pset
-rw-rw-r--  1 henry henry    232 Jul 29 11:07 test-unwanted-simple.sbstore
Comment on attachment 8774196 [details]
Bug 1288840 - Use the private phishing site list in official build.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/66742/diff/2-3/
Attachment #8774196 - Flags: review- → review?(francois)
Comment on attachment 8774196 [details]
Bug 1288840 - Use the private phishing site list in official build.

https://reviewboard.mozilla.org/r/66742/#review65144
Attachment #8774196 - Flags: review?(francois) → review+
Pushed by fmarier@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aaf6318572b6
Use the private phishing site list in official build. r=francois
(In reply to Henry Chang [:henry][:hchang] from comment #8)
> I remember you mentioned in the mail that it'd be better to land this patch
> in firefox 50. Do you still think so or has changed your mind?

Yes, I think that's a safe change so I've requested an auto-land.

I will however ask Matt to verify that we haven't broken the phishing list in Nightly, including:

- newly-created profiles download the phishing list (goog-phish-shavar.*)
- item 1 of "Webpage Warnings" on https://testsafebrowsing.appspot.com/ is correctly blocked
- http://itisatrap.org/firefox/its-a-trap.html is correctly blocked
Flags: needinfo?(francois)
QA Contact: mwobensmith
https://hg.mozilla.org/mozilla-central/rev/aaf6318572b6
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Verified fixed on today's Nightly using criteria from comment 13, plus all of the other test links from the appspot page.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 557752
You need to log in before you can comment on or make changes to this bug.