1288840 - Use the full social engineering list in official Mozilla builds

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[François Marier [:francois]]

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).

Comment 1

[Henry Chang [: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 :)

Comment 2

[Henry Chang [:hchang]]

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

Comment 3

[Henry Chang [:hchang]]

Attachment: Bug 1288840 - Use the private phishing site list in official build.

Review commit: https://reviewboard.mozilla.org/r/66742/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66742/

Comment 4

[Henry Chang [:hchang]]

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/

Comment 5

[François Marier [:francois]]

(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.

Comment 6

[François Marier [: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 7

[François Marier [:francois]]

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

Comment 8

[Henry Chang [:hchang]]

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

Comment 9

[Henry Chang [:hchang]]

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 10

[Henry Chang [:hchang]]

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/

Comment 11

[François Marier [:francois]]

Comment on attachment 8774196 [details]
Bug 1288840 - Use the private phishing site list in official build.

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aaf6318572b6
Use the private phishing site list in official build. r=francois

Comment 13

[François Marier [: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

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aaf6318572b6

Comment 15

[Matt Wobensmith [:mwobensmith][:matt:]]

Verified fixed on today's Nightly using criteria from comment 13, plus all of the other test links from the appspot page.