1394056 - goog-phish-proto cannot be used on non-official builds

Status: VERIFIED FIXED

(Toolkit :: Safe Browsing, defect, P1)

Description

[François Marier [:francois]]

We can only use goog-phish-proto on official builds and need to use googpub-phish-proto otherwise like we do for Safe Browsing V2 lists: https://searchfox.org/mozilla-central/rev/5696c3e525fc8222674eed6a562f5fcbe804c4c7/modules/libpref/init/all.js#5343-5349

[Tracking Requested - why for this release]: Safe Browsing will be broken on most Linux distributions unless we fix this.

Comment 1

[Gerry Chang [:gchang]]

Track 56+ as regression in safe browsing.

Comment 2

[Thomas Nguyen (:tnguyen)]

Attachment: Bug 1394056 - Should not use SOCIAL_ENGINEERING threat type on non-official builds

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

Comment 3

[François Marier [:francois]]

Comment on attachment 8901650 [details]
Bug 1394056 - Should not use SOCIAL_ENGINEERING threat type on non-official builds

https://reviewboard.mozilla.org/r/173072/#review178644

Looks good.

Thomas, can you please test by building both an official build and a non-official build? That way we can confirm that the right list is selected at build time.

Comment 4

[Thomas Nguyen (:tnguyen)]

Thanks Francois, verified it works by adding/removing line
export MOZILLA_OFFICIAL=1
in mozconfig

Comment 5

[Thomas Nguyen (:tnguyen)]

https://treeherder.mozilla.org/#/jobs?repo=try&tochange=9309f280df23bf975c18a016a1fa8d9a035d8238&fromchange=ab26aa132ce72f927505b18f1fa455035a61b4d7&selectedJob=126605247

Comment 6

[Thomas Nguyen (:tnguyen)]

Attachment: For Beta release - Should not use SOCIAL_ENGINEERING threat type on non-official builds

MozReview-Commit-ID: L8N9vg9RGg

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97423f06e9c8
Should not use SOCIAL_ENGINEERING threat type on non-official builds r=francois

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/97423f06e9c8

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 10

[François Marier [:francois]]

Comment on attachment 8902176 [details] [diff] [review]
For Beta release - Should not use SOCIAL_ENGINEERING threat type on non-official builds

Approval Request Comment
[Feature/Bug causing the regression]: bug 1394056
[User impact if declined]: Safe Browsing is completely broken on non-official builds.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not yet, it just landed on central.

[Needs manual test from QE? If yes, steps to reproduce]:

Start with an official build of Firefox (e.g. Nightly).

1. Start the browser in a new profile
2. Wait 5 minutes
3. Go to https://testsafebrowsing.appspot.com/ and test the first 3 links under "Webpage Warnings".

Repeat the test with a non-official build.

[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's a simple pref flip.
[String changes made/needed]: None

Comment 11

[Gerry Chang [:gchang]]

Hi Florin, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build? Thanks!

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8902176 [details] [diff] [review]
For Beta release - Should not use SOCIAL_ENGINEERING threat type on non-official builds

I think we should go ahead and uplift this for beta 8 and verify it there. 

Will this need a different way to test/verify because of the experiment with v4 safebrowsing? How do we know that we're testing the correct version?

Comment 13

[Liz Henry (:lizzard) (relman/hg->git project)]

Question for you in comment 12, francois. Thanks!

Comment 14

[François Marier [:francois]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #12)
> Comment on attachment 8902176 [details] [diff] [review]
> For Beta release - Should not use SOCIAL_ENGINEERING threat type on
> non-official builds
> 
> I think we should go ahead and uplift this for beta 8 and verify it there. 

Agreed. It's a trivial patch, so very low-risk.

> Will this need a different way to test/verify because of the experiment with
> v4 safebrowsing? How do we know that we're testing the correct version?

No, the experiment has not yet started. Beta and Nightly are all 100% V4 at the moment.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9bcdcd27ed40

Comment 16

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Removing ni? as this will be covered as part of bug verification in Beta 56.

Comment 17

[Cynthia Tang [:cynthiatang]]

Verified! It works. :)

Step:
 1. Download source code from Beta branch 
 2. Run ./mach build
 3. Run ./mach run
 4. Enter about:config in Url bar
 5. Check the preference: url classifier.phishTable

Expected Result:
 - url classifier.phishTable is "googpub-phish-proto,test-phish-simple"

Actual Result:
 - url classifier.phishTable is "googpub-phish-proto,test-phish-simple"


OS: Windows 10 Pro 
Build info: 56.0b11
Hg log: changeset: 421677:6bd183fc6921

Comment 18

[Andrei Vaida [:avaida]]

Updating status flags per Comment 17, thank you Cynthia!