Closed Bug 1394056 Opened 7 years ago Closed 7 years ago

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

Categories

(Toolkit :: Safe Browsing, defect, P1)

56 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 + verified
firefox57 --- fixed

People

(Reporter: francois, Assigned: tnguyen)

References

Details

(Keywords: regression, Whiteboard: #sbv4-m9)

Attachments

(2 files)

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.
Assignee: nobody → tnguyen
Track 56+ as regression in safe browsing.
Attachment #8901650 - Flags: review?(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.
Attachment #8901650 - Flags: review?(francois) → review+
Status: NEW → ASSIGNED
Thanks Francois, verified it works by adding/removing line
export MOZILLA_OFFICIAL=1
in mozconfig
Keywords: checkin-needed
Attachment #8902176 - Attachment description: Should not use SOCIAL_ENGINEERING threat type on non-official builds → For Beta release - Should not use SOCIAL_ENGINEERING threat type on non-official builds
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/97423f06e9c8
Should not use SOCIAL_ENGINEERING threat type on non-official builds r=francois
Keywords: checkin-needed
Attachment #8902176 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/97423f06e9c8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Please nominate this for Beta approval when you get a chance.
Flags: needinfo?(tnguyen)
Version: unspecified → 56 Branch
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
Flags: needinfo?(tnguyen)
Attachment #8902176 - Flags: approval-mozilla-beta?
Hi Florin, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(florin.mezei)
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?
Attachment #8902176 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Question for you in comment 12, francois. Thanks!
Flags: needinfo?(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.
Flags: needinfo?(francois)
Removing ni? as this will be covered as part of bug verification in Beta 56.
Flags: needinfo?(florin.mezei)
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
Status: RESOLVED → VERIFIED
Updating status flags per Comment 17, thank you Cynthia!
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.