1392204 - Failure to update safe browsing v4 DB on Android device

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P3)

Description

[Cynthia Tang [:cynthiatang]]

Attachment: Screenshot_url-classifier.png

Steps to reproduce
 1. Update and launch Firefox Nightly on Android
 2. Go to about:config page
 3. Modify preferences:
    (1) urlclassifier.malwareTable: 
        goog-malware-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple,test-harmful-simple
    (2) urlclassifier.phishTable: 
        goog-phish-proto,test-phish-simpleurlclassifier.phishTable;goog-phish-proto,test-phish-simple
 3. New a tab and enter “about:url-classifier” in URL bar
 4. Press “Trigger Update” button for the provider "google4"



Expected result
 - The last update status should be "success"

Actual result
 - The last update status is "download error (400)

Device info.
 - Firefox: 57.0a1 (2017-08-20)
 - Android: 6.0.1
 - Sony Z3C

Comment 1

[Thomas Nguyen (:tnguyen)]

Thanks Cynthia for reporting.
I captured android logcat from Cynthia's device. At the first look, it's interesting that I always see there's update error 404 for google4

Comment 2

[Thomas Nguyen (:tnguyen)]

Attachment: logandroid.log.zip

08-21 15:45:11.253  2997  3025 I Gecko   : [Main Thread]: D/UrlClassifierStreamUpdater nsUrlClassifierStreamUpdater::OnStartRequest (status=NS_OK, uri=https://safebrowsing.googleapis.com/v4/threatListUpdates:fetch?$ct=application/x-protobuf&key=[trimmed-google-api-key]&$httpMethod=POST&$req=CgkKB0ZpcmVmb3gaCggHEAMiAiACKAEaCggJEAMiAiACKAE=, this=84285700)
08-21 15:45:11.253  2997  3025 I Gecko   : [Main Thread]: D/UrlClassifierStreamUpdater nsUrlClassifierStreamUpdater::OnStartRequest failed (404)
08-21 15:45:11.253  2997  3025 I Gecko   : [Main Thread]: D/UrlClassifierStreamUpdater nsUrlClassifierStreamUpdater::Download error [this=84285700]

Comment 3

[Thomas Nguyen (:tnguyen)]

(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #2)
> Created attachment 8899381 [details]
> logandroid.log.zip
> 
> 08-21 15:45:11.253  2997  3025 I Gecko   : [Main Thread]:
> D/UrlClassifierStreamUpdater nsUrlClassifierStreamUpdater::OnStartRequest
> (status=NS_OK,
> uri=https://safebrowsing.googleapis.com/v4/threatListUpdates:
> fetch?$ct=application/x-protobuf&key=[trimmed-google-api-
> key]&$httpMethod=POST&$req=CgkKB0ZpcmVmb3gaCggHEAMiAiACKAEaCggJEAMiAiACKAE=,
> this=84285700)
> 08-21 15:45:11.253  2997  3025 I Gecko   : [Main Thread]:
> D/UrlClassifierStreamUpdater nsUrlClassifierStreamUpdater::OnStartRequest
> failed (404)
> 08-21 15:45:11.253  2997  3025 I Gecko   : [Main Thread]:
> D/UrlClassifierStreamUpdater nsUrlClassifierStreamUpdater::Download error
> [this=84285700]

That is fresh installation from Google Store

Comment 4

[Henry Chang [:hchang]]

Attachment: Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

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

Comment 5

[Henry Chang [:hchang]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170982/diff/1-2/

Comment 6

[François Marier [:francois]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

https://reviewboard.mozilla.org/r/170982/#review176430

::: browser/app/profile/firefox.js:1704
(Diff revision 2)
>  // Enable safebrowsing v4 tables (suffixed by "-proto") update.
>  pref("urlclassifier.malwareTable", "goog-malware-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple,test-harmful-simple");
>  pref("urlclassifier.phishTable", "goog-phish-proto,test-phish-simple");
>  
> +// Tables for application reputation.
> +pref("urlclassifier.downloadAllowTable", "goog-downloadwhite-proto");

We should probably set these prefs to `goog-downloadwhite-shavar` and `goog-badbinurl-shavar` in the Fennec-spefic pref file.

Then we'll need to figure out what to do about it when we migrate from V2 to V4 there.

::: toolkit/components/url-classifier/tests/unit/test_platform_specific_threats.js:1
(Diff revision 2)
> +Cu.import("resource://gre/modules/AppConstants.jsm");

Missing the [Public Domain header](https://www.mozilla.org/en-US/MPL/headers/).

Comment 7

[Henry Chang [:hchang]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

https://reviewboard.mozilla.org/r/170982/#review176430

> We should probably set these prefs to `goog-downloadwhite-shavar` and `goog-badbinurl-shavar` in the Fennec-spefic pref file.
> 
> Then we'll need to figure out what to do about it when we migrate from V2 to V4 there.

We don't have 'goog-downloadwhite-shavar' in the pref now. Do you mean 'goog-downloadwhite-digest256'?

Comment 8

[François Marier [:francois]]

(In reply to Henry Chang [:hchang] from comment #7)
> We don't have 'goog-downloadwhite-shavar' in the pref now. Do you mean
> 'goog-downloadwhite-digest256'?

Yes, sorry for the typo.

Comment 9

[Henry Chang [:hchang]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170982/diff/2-3/

Comment 10

[François Marier [:francois]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

https://reviewboard.mozilla.org/r/170982/#review176594

Sorry, I confused you with my comments about which pref file should override which one. See below.

::: commit-message-128a7:1
(Diff revisions 2 - 3)
> -Bug 1392204 - Some threat types are unavailable on android.
> +Bug 1392204 - Remove 'goog-badbinurl-proto' and 'goog-downloadwhite-proto' from SBv4 update list on fennec.

nit: you could shorten that to "Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec."

::: commit-message-128a7:3
(Diff revisions 2 - 3)
> -Bug 1392204 - Some threat types are unavailable on android.
> +Bug 1392204 - Remove 'goog-badbinurl-proto' and 'goog-downloadwhite-proto' from SBv4 update list on fennec.
> +
> +The google SB v4 server would return 404 error if the request contains 'goog-badbinurl-proto'

I would say something like "Version 4 of the Google Safe Browsing server will return a 404 if any of the application reputation lists are requested on Android."

::: modules/libpref/init/all.js
(Diff revision 2)
>  pref("urlclassifier.phishTable", "goog-phish-shavar,test-phish-simple");
>  #else
>  pref("urlclassifier.phishTable", "googpub-phish-shavar,test-phish-simple");
>  #endif
>  
> -// Tables for application reputation.

Sorry, I think I misspoke.

We should match the rest of this file:

- `modules/libpref/init/all.js` uses the `-shavar` lists
- `browser/app/profile/firefox.js` overrides them to be `-proto`
- `mobile/android/app/mobile.js` doesn't override any of the lists from `all.js`

Comment 11

[Henry Chang [:hchang]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

https://reviewboard.mozilla.org/r/170982/#review176594

I am glad you have the same opinion with me actually :p We should have done something like this in Bug 1331139.

Comment 12

[Henry Chang [:hchang]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170982/diff/3-4/

Comment 13

[François Marier [:francois]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

https://reviewboard.mozilla.org/r/170982/#review176930

Comment 14

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 0445b72f8f66 -d e538796068a5: rebasing 415498:0445b72f8f66 "Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec. r=francois" (tip)
merging browser/app/profile/firefox.js
merging modules/libpref/init/all.js
merging toolkit/components/url-classifier/nsUrlClassifierUtils.cpp
warning: conflicts while merging modules/libpref/init/all.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 15

[Henry Chang [:hchang]]

Comment on attachment 8899677 [details]
Bug 1392204 - Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/170982/diff/4-5/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb5584b18e40
Disable goog-badbinurl-proto and goog-downloadwhite-proto on Fennec. r=francois

Comment 17

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

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

Comment 18

[François Marier [:francois]]

I have just tested the following on Fennec Nightly:

  urlclassifier.malwareTable = goog-harmful-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple,test-harmful-simple
  urlclassifier.phishTable = goog-phish-proto,test-phish-simple

using https://testsafebrowsing.appspot.com/ and all of the relevant test cases are blocked correctly:

- "Webpage Warnings" 1 to 4
- "IOS/OSX Warnings" 1 and 2

So it looks like we have working SBv4 on Fennec!

Cynthia, could you please confirm that it is working for you as well?

Comment 19

[Cynthia Tang [:cynthiatang]]

Hi Francois,
It works if I clear data and reinstall FirefoxNightly. Thanks!

It does not work if I don't clear data. 

How to clear data:
 1. Go to Settings -> Apps －> FirefoxNightly
 2. Tap on "FORCE STOP" -> Tap on CLEAR DATA
 3. Uninstall FirefoxNightly
 4. Reinstall FirefoxNightly from Play Store  

QA will create a test case to make sure that user can use safe browsing v4 after upgrading Firefox. 


Device info.
 - Firefox: 57.0a1 (2017-08-29)
 - Android: 6.0.1
 - Sony Z3C

Comment 20

[Thomas Nguyen (:tnguyen)]

(In reply to Cynthia Tang [:cynthiatang] from comment #19)
> Hi Francois,
> It works if I clear data and reinstall FirefoxNightly. Thanks!
> 
> It does not work if I don't clear data. 
> 
> How to clear data:
>  1. Go to Settings -> Apps －> FirefoxNightly
>  2. Tap on "FORCE STOP" -> Tap on CLEAR DATA
>  3. Uninstall FirefoxNightly
>  4. Reinstall FirefoxNightly from Play Store  

> QA will create a test case to make sure that user can use safe browsing v4
> after upgrading Firefox. 
> 

I guess we may have to think about preference migration when upgrading FF and change SB from V2 to V4

Comment 21

[Ethan Tseng [:ethan]]

(In reply to Cynthia Tang [:cynthiatang] from comment #19)
> Hi Francois,
> It works if I clear data and reinstall FirefoxNightly. Thanks!

Good job. Thanks!

Comment 22

[Ethan Tseng [:ethan]]

(In reply to Thomas Nguyen[:tnguyen] ni plz from comment #20)
> (In reply to Cynthia Tang [:cynthiatang] from comment #19)
> > It does not work if I don't clear data. 
> I guess we may have to think about preference migration when upgrading FF
> and change SB from V2 to V4

I think we should file a bug to deal with the preference migration issue.
What do you think, Francois?

Comment 23

[François Marier [:francois]]

Cynthia, I'm a little bit confused as to why you had to clear data. I didn't have to do that on my phone.

Could you please share the exact steps you took for the whole test?

Comment 24

[Cynthia Tang [:cynthiatang]]

1. Launch "Play Store"
2. Search "Firefox Nightly"
3. Tap on "UPDATE" 
4. Launch FirefoxNightly after updating
5. Go to about:config page and modify urlclassifier.phishTable and urlclassifier.malwareTable
   (replace words: shaver -> proto)
6. Go to about:url-classifier page
7. Tap on "Trigger Update" for google4

Actual result:
 - The last update status will be cannot update. (Thomas also helped me to check it)


Before update, the device info:
 - Firefox: 57.0a1 (2017-08-20)
 - Android: 6.0.1
 - Sony Z3C

After update, the device info:
 - Firefox: 57.0a1 (2017-08-29)
 - Android: 6.0.1
 - Sony Z3C

Comment 25

[Henry Chang [:hchang]]

(In reply to Cynthia Tang [:cynthiatang] from comment #24)
> 1. Launch "Play Store"
> 2. Search "Firefox Nightly"
> 3. Tap on "UPDATE" 
> 4. Launch FirefoxNightly after updating
> 5. Go to about:config page and modify urlclassifier.phishTable and
> urlclassifier.malwareTable
>    (replace words: shaver -> proto)
> 6. Go to about:url-classifier page
> 7. Tap on "Trigger Update" for google4
> 
> Actual result:
>  - The last update status will be cannot update. (Thomas also helped me to
> check it)
> 
> 
> Before update, the device info:
>  - Firefox: 57.0a1 (2017-08-20)
>  - Android: 6.0.1
>  - Sony Z3C
> 
> After update, the device info:
>  - Firefox: 57.0a1 (2017-08-29)
>  - Android: 6.0.1
>  - Sony Z3C

1. You might need to restart Fennec once you updated the tables. As far as I remember,
   we no longer keep all tables in sync across all references to cope with certain 
   deadlock issue. (But I am not sure if this is the case.)

2. Don't rely on about:url-classifier too much. You can try to check if the tables
   are actually downloaded or visit the test pages. On desktop, the downloaded tables
   are located in "[cache_dir]/safebrowsing". On mobile, they are under "app data" directory.
   However, the "app data" directory might differ on different version of Android.
   If you are familiar with android enough, you can "find" goog-phish-shavar.pset
   to know the actual path. (It seems to be in data/data/[fennec_package_name] but
   you can verify it on your own.)

Comment 26

[François Marier [:francois]]

There is some confusion here, possibly an upgrade problem.

We should confirm the status of this.

Comment 27

[Dimi Lee [:dimi]]

I "manually" modified a preference before updating, 'malwareTable' for example.
After udpating FirefoxNightly, the preference is still the one before updating.

I think this is why Cynthia has to to clear data, I'll double check this behavior with Fennec team.

Comment 28

[Dimi Lee [:dimi]]

(In reply to Dimi Lee[:dimi][:dlee] from comment #27)
> I "manually" modified a preference before updating, 'malwareTable' for
> example.
> After udpating FirefoxNightly, the preference is still the one before
> updating.
> 
> I think this is why Cynthia has to to clear data, I'll double check this
> behavior with Fennec team.

Confirmed this behavior with fennec team, if we have ever modified the preference manually, update firefox won't change the preference.
So my guess here is we changed the preference before so updated preference in latest build won't take effect.

Hi Cynthia, if you think this "might" be why you have to clear data, then we can close it for now and check this in QA's test case.

Comment 29

[Dimi Lee [:dimi]]

Discussed with Cynthia offline, we think Comment 28 might be the reason we need to clear cache.
Since its a expected behavior, I'll close this bug.

Comment 30

[François Marier [:francois]]

(In reply to Dimi Lee[:dimi][:dlee] from comment #27)
> I "manually" modified a preference before updating, 'malwareTable' for
> example.
> After udpating FirefoxNightly, the preference is still the one before
> updating.

Ah, that makes sense. It's also the case on Desktop. We never overwrite customized prefs, we just change the default value.

Thanks for confirming Dimi and thank you Cynthia for bringing it up.