mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
127 stars 41 forks source link

addons.mozilla.org has failed the web security baseline #13897

Closed moz-hwine closed 3 years ago

moz-hwine commented 3 years ago

Site https://addons.mozilla.org has failed the web security baseline scan.

The failing tests are:

Strict-Transport-Security Header Not Set [10035] x 8

This issue was automatically raised.

This issue is managed automatically by the baseline scan:

Full details, including how to test for these issues locally, can be found on this Security Baseline Service dashboard. If you have any questions or concerns please get in contact with secops+web-baseline@mozilla.com

bobsilverberg commented 3 years ago

This is an odd failure. It looks like "too many requests". I followed the link to the dashboard and refreshed, and then the tests passed, so maybe this is some temporary hiccup? I guess we can see if it passes next time and self-closes?

willdurand commented 3 years ago

@g-k do you maintain this scanner? if so, could you look into the 429 failures? If not, could you help me find who is the owner of this scanner? thanks!

g-k commented 3 years ago

@willdurand yep! I maintain the scanner now. I'll look into this and you can assign it to me. Sorry for the noise!

g-k commented 3 years ago

OK I was able to reproduce the 429s without STS (and other security headers). The responses are coming from nginx (so either the app reverse proxy or iprepd-nginx):

for i in $(seq 0 500); do curl -I https://addons.mozilla.org/zh-CN/android/search/; done
HTTP/1.1 200 OK       
amo-request-id: f8f7eab0-4492-41a2-8876-faf88facd4af
...

HTTP/1.1 429                                                                                                                                                             
Content-Length: 0                                                                                                                                                        
Date: Thu, 28 Jan 2021 15:51:33 GMT                                                                                                                                      
Server: nginx                                                                                                                                                            
Connection: keep-alive                                                                                                                                                   
                                                                                                                                                                         HTTP/1.1 429                                                                                                                                                             Content-Length: 0                                                                                                                                                        Date: Thu, 28 Jan 2021 15:51:33 GMT                                                                                                                                      
Server: nginx                                                                       
Connection: keep-alive                                                                                                                                                   

HTTP/1.1 429                                                                                                                                                             
Content-Length: 0                                                                   
Date: Thu, 28 Jan 2021 15:51:34 GMT
Server: nginx                  
Connection: keep-alive

...

@bqbn could we conditionally set security headers in nginx when the application doesn't return them like we do for other services?

bqbn commented 3 years ago

@g-k we have a rate limit for the /search/ endpoint. Please see [1] for the limit, and [2] for the reason behind that.

If the scanner is getting 429, I suppose it either scans a lot URLs in a short period of time, or scans a single URL repeatedly in a short period of time. In either case, could you make the scanner go easy on us? As you can see in [2], it does put pressure on our system and affect its performance.

g-k commented 3 years ago

Sure, I halved the scanner's spider step concurrency in prod, and I've opened https://github.com/mozilla-services/cloudops-deployment/pull/4184 to fix this issue.

moz-hwine commented 3 years ago

The web security baseline scan for site https://addons.mozilla.org now passes - well done team!

diox commented 3 years ago

Was the change to halve the spider step concurrency deployed ? We just got a new issue opened for the same problem: https://github.com/mozilla/addons/issues/13944

How fast is the spider going through our pages, and what pages does it go through exactly ?

g-k commented 3 years ago

Was the change to halve the spider step concurrency deployed ?

Yes that was https://github.com/mozilla-services/foxsec/pull/1728

https://github.com/mozilla-services/cloudops-deployment/pull/4184 is still open to fix the underlying issue.

How fast is the spider going through our pages, and what pages does it go through exactly ?

I believe it's following everything links from the page. I couldn't find a rate limit option, so I'll have to do more digging and get back to you.

bqbn commented 3 years ago

mozilla-services/cloudops-deployment#4184 is still open to fix the underlying issue.

@g-k what do you think it's the underlying issue?

If I read the first comment correctly, the bot filed this issue because it received 429 code, not because the URLs didn't return the Strict-Transport-Security header. So why https://github.com/mozilla-services/cloudops-deployment/pull/4184 fixes the underlying issue when the issue is about bot receiving 429 code?

Take one of the bot-reported URLs for example, I see that it does return the Strict-Transport-Security header,

$ curl -I "https://addons.mozilla.org/en-GB/firefox/search/?sort=users&type=statictheme"
HTTP/1.1 200 OK
amo-request-id: 3ec36dc8-4bb4-49cb-839b-a7d90ca4be14
Cache-Control: no-store
Content-Length: 318654
Content-Security-Policy: base-uri 'self';child-src 'none';connect-src https://www.google-analytics.com https://addons.mozilla.org https://sentry.prod.mozaws.net;font-src https://addons-amo.cdn.mozilla.net;form-action 'self';frame-src 'none';img-src 'self' data: https://addons.cdn.mozilla.net https://addons-amo.cdn.mozilla.net;manifest-src 'none';media-src 'none';object-src 'none';default-src https://addons-amo.cdn.mozilla.net;prefetch-src https://addons-amo.cdn.mozilla.net;script-src https://addons-amo.cdn.mozilla.net https://www.google-analytics.com/analytics.js;style-src https://addons-amo.cdn.mozilla.net;worker-src 'none';report-uri /__cspreport__
Content-Type: text/html; charset=utf-8
Date: Mon, 01 Feb 2021 21:13:48 GMT
ETag: W/"4dcbe-MBA1okpvXPLTrcVEfTWPwSuPAng"
Public-Key-Pins: max-age=5184000; includeSubDomains; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; pin-sha256="r/mIkG3eEpVdm+u/ko/cwxzOMo1bk4TyHIlByibiA5E="
Strict-Transport-Security: max-age=31536000
Vary: Accept-Encoding
Vary: DNT
Vary: User-Agent
X-Content-Type-Options: nosniff
X-Frame-Options: DENY
X-XSS-Protection: 0
Connection: keep-alive

As long as the bot slows down (rate below the value noted in [1]), I think it won't receive 429s any more.

bqbn commented 3 years ago

Also the rate limit was put in place in 2017, and why the bot started to report issues just recently? Has anything changed on the bot side?

g-k commented 3 years ago

If I read the first comment correctly, the bot filed this issue because it received 429 code, not because the URLs didn't return the Strict-Transport-Security header. So why mozilla-services/cloudops-deployment#4184 fixes the underlying issue when the issue is about bot receiving 429 code?

The bot is filing the issue for missing HSTS headers https://www.zaproxy.org/docs/alerts/10035/ for the responses with the 429 error code i.e.

  1. bot runs
  2. spiders the multiple search links
  3. hits rate limit
  4. gets 429 response without STS and other headers
g-k commented 3 years ago

Also the rate limit was put in place in 2017, and why the bot started to report issues just recently? Has anything changed on the bot side?

I'm not sure what's changed since 2017, since Simon ran it. But ZAP has seen several major releases since then, so it might be spidering more aggressively or following different links now.

bqbn commented 3 years ago

The bot is filing the issue for missing HSTS headers

OK, because the bot filed this issue saying

The failing tests are:

Strict-Transport-Security Header Not Set [10035] x 8

g-k commented 3 years ago

Oh yeah, the output is confusing. If you've got any ideas to improve it, I'm all ears.

I added links to the ZAP error page for the alerts and you can see it in https://github.com/mozilla/addons/issues/13944

Longer term, I'd like to make it easier to configure and run the scans.