Closed
Bug 1388582
Opened 8 years ago
Closed 7 years ago
The goog-harmful-proto list doesn't appear to be working
Categories
(Toolkit :: Safe Browsing, defect, P2)
Toolkit
Safe Browsing
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.
Reporter | ||
Updated•8 years ago
|
Whiteboard: #sbv4-m9
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(francois)
Reporter | ||
Comment 2•8 years ago
|
||
(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)
Assignee | ||
Comment 3•8 years ago
|
||
(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)
Assignee | ||
Comment 4•8 years ago
|
||
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)
Assignee | ||
Comment 5•8 years ago
|
||
(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
Reporter | ||
Updated•8 years ago
|
Group: mozilla-employee-confidential
Flags: needinfo?(francois)
Reporter | ||
Comment 6•8 years ago
|
||
(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.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8897760 -
Flags: review?(francois)
Reporter | ||
Updated•8 years ago
|
Whiteboard: #sbv4-m9 → #sbv4-m10
Reporter | ||
Comment 10•8 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
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
![]() |
||
Comment 13•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•