887044 - Nail down API key, User-Agent for requests to Google Application Reputation API

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

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

Moheeb Rajab from Google Safebrowing team asked me over email to use User-Agent: CsdTesting/Mozilla for now. Also he has promised us an API key which we will need to use before turning on this feature at scale.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Will it require a separate API key than geolocation (bug 882485)? If so, we'll need to add support for multiple Google keys. Hopefully that can be avoided?

Comment 2

[Doug Turner (:dougt)]

It should not.  The key was generated via the google console, so we can just flip whatever flag we need.

Monica, loop me and Moheeb in an email?

Comment 3

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

Yes, and sorry for the delay!

Comment 4

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

Attachment: Remove CsdTesting/Mozilla from User-Agent, set safebrowsing URLs to use the GOOGLE_API_KEY (

Comment 5

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

Comment on attachment 811270 [details] [diff] [review]
Remove CsdTesting/Mozilla from User-Agent, set safebrowsing URLs to use the GOOGLE_API_KEY (

Hi Doug,

I think it's time to turn on the safebrowsing service for our API key. The application reputation stuff is waiting on its final integration with the download manager (bug 895476), at which point we'll need the API key for this to work in nightly. The tracking bug is bug 662819 -- unclosed bugs are things that increase coverage, not that are necessary for this feature to work.

The user-agent change is because Moheeb from the Safebrowsing team asked us to use that while we were in testing, which I interpreted to mean not in production. In any case, if we are using the key they can use that for throttling and abuse prevention.

Thanks,
Monica

Comment 6

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

I forgot to mention the client=%NAME% thing is because the new safebrowsing checks require client=Firefox, not SAFEBROWSING_ID.

Comment 7

[Doug Turner (:dougt)]

I have enabled "Safe Browsing API" on the GOOGLE_API_KEY.  There is a 10k limit on the number of requests per day you can do.  Please ensure that we have a contract in place that allows more than that or you're going to have a bad day.

Comment 8

[Doug Turner (:dougt)]

Comment on attachment 811270 [details] [diff] [review]
Remove CsdTesting/Mozilla from User-Agent, set safebrowsing URLs to use the GOOGLE_API_KEY (

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

::: browser/app/profile/firefox.js
@@ +770,5 @@
>  pref("browser.safebrowsing.enabled", true);
>  pref("browser.safebrowsing.malware.enabled", true);
>  pref("browser.safebrowsing.debug", false);
>  
> +pref("browser.safebrowsing.updateURL", "http://safebrowsing.clients.google.com/safebrowsing/downloads?client=%NAME%&appver=%VERSION%&pver=2.2&apikey=%GOOGLE_API_KEY%")

ooc, do this have an https endpoint that we could use instead?

@@ +772,5 @@
>  pref("browser.safebrowsing.debug", false);
>  
> +pref("browser.safebrowsing.updateURL", "http://safebrowsing.clients.google.com/safebrowsing/downloads?client=%NAME%&appver=%VERSION%&pver=2.2&apikey=%GOOGLE_API_KEY%")
> +pref("browser.safebrowsing.keyURL", "https://sb-ssl.google.com/safebrowsing/newkey?client=%NAME%&appver=%VERSION%&pver=2.2");
> +pref("browser.safebrowsing.gethashURL", "http://safebrowsing.clients.google.com/safebrowsing/gethash?client=%NAME%&appver=%VERSION%&pver=2.2");

same.  this end point doesn't take the key, right?

(Honestly, lgtm, but I don't really know about these endpoints.)

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to Doug Turner (:dougt) from comment #7)
> I have enabled "Safe Browsing API" on the GOOGLE_API_KEY.  There is a 10k
> limit on the number of requests per day you can do.  Please ensure that we
> have a contract in place that allows more than that or you're going to have
> a bad day.

Sounds like this patch can't land on Nightly, then?

Comment 10

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

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #9)
> (In reply to Doug Turner (:dougt) from comment #7)
> > I have enabled "Safe Browsing API" on the GOOGLE_API_KEY.  There is a 10k
> > limit on the number of requests per day you can do.  Please ensure that we
> > have a contract in place that allows more than that or you're going to have
> > a bad day.
> 
> Sounds like this patch can't land on Nightly, then?

Ooph, that's not what I expected. Doug, I'm going to resurrect an ancient email thread with Moheeb and you.

The bug to convert all endpoints to SSL is bug 783047 and is a little more complicated than just switching things over to https. We can try switching existing URLs to https but I'd like to leave that for another patch.

Comment 11

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

Google basically said we were ok to turn on in Nightly. I am going to revert the client= part of this patch because I see now that SAFEBROWSING_ID gets replaced in code with the pref browser.safebrowsing.id (navclient-auto-ffox in firefox.js) which seems to already work.

I am going to sync up with my contact there to see whether API requests to the application reputation service can be throttled separately from existing requests to download goog-phish-shavar, etc. Hopefully the answer is yes.

In any case, bug 895476 will not land without making sure that failed calls to the API don't break the download flow, and this bug will land concurrently with that.

Comment 12

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

Attachment: Remove CsdTesting/Mozilla from User-Agent, set safebrowsing URLs to use the GOOGLE_API_KEY (

Comment 13

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

I'm going to go ahead and land, since bug 895476 depends on this change. We can start including the API key whether or not we are using application reputation.

Comment 14

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

Attachment: Remove CsdTesting/Mozilla from User-Agent, set safebrowsing URLs to use the GOOGLE_API_KEY (

Comment 15

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

Comment on attachment 826148 [details] [diff] [review]
Remove CsdTesting/Mozilla from User-Agent, set safebrowsing URLs to use the GOOGLE_API_KEY (

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

Carry over doug's review from comment 8. This change does not enable any new services, it just means we start sending the API key for some existing safebrowsing requests in preparation for starting to send application reputation list requests.

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0f1dc225c521