1013558 - ApplicationReputation should return shouldBlock=true if the remote verdict is DANGEROUS_HOST or UNKNOWN

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[[:mmc] Monica Chew (no longer reading bugmail)]

Currently we only return shouldBlock=true if the remote verdict is DANGEROUS, but the API has evolved since then.

DANGEROUS_HOST can use the same string as DANGEROUS and should also delete the downloaded target file.

UNKNOWN needs a slightly different one, e.g. "download.exe is not commonly downloaded and could be dangerous".

When we're ready we should double-check what Chrome is doing in the UNKNOWN case (removing the final target or not).

Comment 1

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: ApplicationReputation should block if verdict is DANGEROUS_HOST (

Comment 2

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: ApplicationReputation should block if verdict is DANGEROUS_HOST (

Comment 3

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8433616 [details] [diff] [review]
ApplicationReputation should block if verdict is DANGEROUS_HOST (

Review of attachment 8433616 [details] [diff] [review]:
-----------------------------------------------------------------

The generated files are generated by downloading http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/safe_browsing/csd.proto and running the protoc (protocol compiler) on it. I stripped protocol buffers that we were not using, since this file has grown since we used it last.

Comment 4

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8433616 [details] [diff] [review]
ApplicationReputation should block if verdict is DANGEROUS_HOST (

Review of attachment 8433616 [details] [diff] [review]:
-----------------------------------------------------------------

Maybe you should put a file like this, which describes how to generate the content from the upstream:
http://mxr.mozilla.org/mozilla-central/source/media/libvorbis/README_MOZILLA

::: toolkit/components/downloads/ApplicationReputation.cpp
@@ +975,5 @@
>      return NS_ERROR_CANNOT_CONVERT_DATA;
>    }
>  
> +  // There are several more verdicts, but we only respect DANGEROUS and
> +  // DANGEROUS_HOST for now and treat everything else as SAFE.

Any follow up bugs for the remainder?

Comment 5

[[:mmc] Monica Chew (no longer reading bugmail)]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #4)
> Comment on attachment 8433616 [details] [diff] [review]
> ApplicationReputation should block if verdict is DANGEROUS_HOST (
> 
> Review of attachment 8433616 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Maybe you should put a file like this, which describes how to generate the
> content from the upstream:
> http://mxr.mozilla.org/mozilla-central/source/media/libvorbis/README_MOZILLA
> 
> ::: toolkit/components/downloads/ApplicationReputation.cpp
> @@ +975,5 @@
> >      return NS_ERROR_CANNOT_CONVERT_DATA;
> >    }
> >  
> > +  // There are several more verdicts, but we only respect DANGEROUS and
> > +  // DANGEROUS_HOST for now and treat everything else as SAFE.
> 
> Any follow up bugs for the remainder?

https://bugzilla.mozilla.org/show_bug.cgi?id=1019933 for UNKNOWN_HOST. Presumably by the time we implement that there will be another one :P

Comment 6

[[:mmc] Monica Chew (no longer reading bugmail)]

Attachment: ApplicationReputation should block if verdict is DANGEROUS_HOST (

Comment 7

[[:mmc] Monica Chew (no longer reading bugmail)]

Comment on attachment 8446824 [details] [diff] [review]
ApplicationReputation should block if verdict is DANGEROUS_HOST (

I added a comment to the script that already generates this instead of adding another README.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/061a225551cb

Comment 9

[Ryan VanderMeulen [:RyanVM]]

*sigh* Thought I saw a Try push on this. Apparently not. Backed out for bustage. In the future, please provide one when requesting checkin as per the the policy announced on dev-platform awhile ago now.
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ef3fa0ebedf

https://tbpl.mozilla.org/php/getParsedLog.php?id=42625385&tree=Mozilla-Inbound

Comment 10

[[:mmc] Monica Chew (no longer reading bugmail)]

Argh, sorry about that -- -Werror_switch is enabled on some platforms but not others :(

Green try: https://tbpl.mozilla.org/?tree=Try&rev=706d56280f59

https://hg.mozilla.org/integration/mozilla-inbound/rev/6810b2fa1afe

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6810b2fa1afe