Closed Bug 1388582 Opened 7 years ago Closed 7 years ago

The goog-harmful-proto list doesn't appear to be working

Categories

(Toolkit :: Safe Browsing, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: francois, Assigned: hchang)

References

Details

(Whiteboard: #sbv4-m10)

Attachments

(1 file)

I tried to verify that it would be downloaded properly by adding it to
urlclassifier.malwareTable but then I got a 404 from the server:

listmanager: 16:57:11 GMT-0700 (PDT): makeUpdateRequestForEntry_:
requestPayload
CgkKB0ZpcmVmb3gaJwgFEAIaGwoNCAUQBhgBIgMwMDEwARC83wIaAhgHY2pClCICIAIoARonCAEQAhobCg0IARAGGAEiAzAwMTABEJPiAhoCGAf4HNKaIgIgAigBGicIAxACGhsKDQgDEAYYASIDMDAxMAEQ1NACGgIYBycLau4iAiACKAEaJwgHEAIaGwoNCAcQBhgBIgMwMDEwARDAzwEaAhgH3dDFQyICIAIoARolCAkQAhoZCg0ICRAGGAEiAzAwMTABEAgaAhgHHPr_oCICIAIoARolCAgQAhoZCg0ICBAGGAEiAzAwMTABEGQaAhgH7pJM0yICIAIoARoKCAQQAiICIAIoAQ==
update:
https://safebrowsing.googleapis.com/v4/threatListUpdates:fetch?$ct=application/x-protobuf&key=[trimmed-google-api-key]&$httpMethod=POST
tablelist:
goog-phish-proto,goog-malware-proto,goog-unwanted-proto,goog-badbinurl-proto,goog-downloadwhite-proto,goog-harmful-proto

listmanager: 16:57:11 GMT-0700 (PDT): download error for
goog-phish-proto,goog-malware-proto,goog-unwanted-proto,goog-badbinurl-proto,goog-downloadwhite-proto,goog-passwordwhite-proto,goog-harmful-proto:
404

Maybe it's a problem because that list only exists on the Android platform? If that's the case, it should work as-is on Fennec.
Whiteboard: #sbv4-m9
Flags: needinfo?(francois)
(In reply to Henry Chang [:hchang] from comment #1)
> The curl command for 'goog-harmful-proto' only update request is:
> 
> curl -s -L  --header "X-HTTP-Method-Override: POST"
> "https://safebrowsing.googleapis.com/v4/threatListUpdates:
> fetch?%24req=CgkKB0ZpcmVmb3gaCggEEAQiAiACKAE=&%24ct=application/x-
> protobuf..."

Can you give me the JSON equivalent of that protobuf so that we can easily see what platform, etc. we are passing?
Group: mozilla-employee-confidential
Flags: needinfo?(francois) → needinfo?(hchang)
(In reply to François Marier [:francois] from comment #2)
> (In reply to Henry Chang [:hchang] from comment #1)
> > The curl command for 'goog-harmful-proto' only update request is:
> > 
> > curl -s -L  --header "X-HTTP-Method-Override: POST"
> > "https://safebrowsing.googleapis.com/v4/threatListUpdates:
> > fetch?%24req=CgkKB0ZpcmVmb3gaCggEEAQiAiACKAE=&%24ct=application/x-
> > protobuf..."
> 
> Can you give me the JSON equivalent of that protobuf so that we can easily
> see what platform, etc. we are passing?

I didn't make any change to generating goog-harmful-proto update request.
In other words, it would work no difference from other v4 lists.
But you reminded me the 'platform' might matter! Lemme give "android" platform
a try!
Flags: needinfo?(hchang)
It is working with ANDROID_PLATFORM :)

(I was just aware you have mentioned this in the bug description :p)

So, do you think what we should do for this special case? The thing is
we update all v4 tables with "one batch request" and google server would
respond error as long as the requests contains
(POTENTIALLY_HARMFUL_APPLICATION, non-android platform) tuple. 

http://searchfox.org/mozilla-central/rev/c329d562fb6c6218bdb79290faaf015467ef89e2/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp#108

Given what we know, we can bypass POTENTIALLY_HARMFUL_APPLICATION update
on non-android-platform to be very fool-proof (we might accidentally add goog-harmful-proto
to firefox.js). Do you have any other idea?
Flags: needinfo?(francois)
(In reply to Henry Chang [:hchang] from comment #4)
> It is working with ANDROID_PLATFORM :)
> 
> (I was just aware you have mentioned this in the bug description :p)
> 
> So, do you think what we should do for this special case? The thing is
> we update all v4 tables with "one batch request" and google server would
> respond error as long as the requests contains
> (POTENTIALLY_HARMFUL_APPLICATION, non-android platform) tuple. 
> 
> http://searchfox.org/mozilla-central/rev/
> c329d562fb6c6218bdb79290faaf015467ef89e2/toolkit/components/url-classifier/
> nsUrlClassifierUtils.cpp#108
> 
> Given what we know, we can bypass POTENTIALLY_HARMFUL_APPLICATION update
> on non-android-platform to be very fool-proof (we might accidentally add
> goog-harmful-proto
> to firefox.js). Do you have any other idea?

Two possible approaches:

1) Remove the mapping from non-android platform [1] so 'goog-harmful-proto'
   will be unknown/unsupported.
   Pros: clean, general and (POTENTIALLY_HARMFUL_APPLICATION, non-android platform)
         can be automatically bypass in [2]
   Cons: We can only test the conversion on android platform and it might
         not make much sense to make the conversion platorm-specific.

2) Add the (POTENTIALLY_HARMFUL_APPLICATION, non-android platform) check
   to [2].
   Pros: Explicitly point out we disallow this kind of threat type on android
         and easy to understand.
   Cons: A little intrusive and lack of generality.
   

[1] http://searchfox.org/mozilla-central/rev/c329d562fb6c6218bdb79290faaf015467ef89e2/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp#233
[2] http://searchfox.org/mozilla-central/rev/c329d562fb6c6218bdb79290faaf015467ef89e2/toolkit/components/url-classifier/nsUrlClassifierUtils.cpp#344
Group: mozilla-employee-confidential
Flags: needinfo?(francois)
(In reply to Henry Chang [:hchang] from comment #5)
> 2) Add the (POTENTIALLY_HARMFUL_APPLICATION, non-android platform) check
>    to [2].
>    Pros: Explicitly point out we disallow this kind of threat type on android
>          and easy to understand.
>    Cons: A little intrusive and lack of generality.

I don't have a strong preference but this seems like the safest.

We should make sure that if someone adds goog-harmful-proto to urlclassifier.malwareTable like I did, that list will be ignored on desktop (ideally with a warning) and the updates will continue to work.
Attachment #8897760 - Flags: review?(francois)
Whiteboard: #sbv4-m9 → #sbv4-m10
Comment on attachment 8897760 [details]
Bug 1388582 - Skip unsupported threat types on current platform while making v4 request.

https://reviewboard.mozilla.org/r/169066/#review174776

Very thorough, thanks Henry!

::: toolkit/components/url-classifier/nsUrlClassifierUtils.cpp:154
(Diff revision 3)
> +{
> +  PlatformType platform = GetPlatformType();
> +
> +  switch (aThreatType) {
> +  case POTENTIALLY_HARMFUL_APPLICATION:
> +    // Bug 1388582 - Google server would repond 404 error if the request

typo: "respond"
Attachment #8897760 - Flags: review?(francois) → review+
Pushed by hchang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dab947ebf3bb
Skip unsupported threat types on current platform while making v4 request. r=francois
https://hg.mozilla.org/mozilla-central/rev/dab947ebf3bb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: