mozilla / blurts-server

Mozilla Monitor arms you with tools to keep your personal information safe. Find out what hackers already know about you and learn how to stay a step ahead of them.
https://monitor.mozilla.org
Mozilla Public License 2.0
731 stars 205 forks source link

'set-cookie' header to set 'awselb' doesn't have the 'secure' directive #224

Closed pdehaan closed 6 years ago

pdehaan commented 6 years ago

Via my sonarwhal scanner at https://github.com/pdehaan/blurts-sonar

Not sure if these are an OPs config or something else or maybe not even a bug, just seemed suspicious and worth filing.

Output:

$ npm test

> blurts-sonar@1.0.0 test /Users/pdehaan/dev/tmp/del/blurts-sonar/blurts-sonar
> npm run scan

> blurts-sonar@1.0.0 scan /Users/pdehaan/dev/tmp/del/blurts-sonar/blurts-sonar
> npm-run-all scan:*

> blurts-sonar@1.0.0 scan:breach /Users/pdehaan/dev/tmp/del/blurts-sonar/blurts-sonar
> sonarwhal $npm_package_homepage/?breach=Adobe

✖ Finishing...

https://monitor.firefox.com/?breach=Adobe
                Error    'set-cookie' header to set 'awselb' doesn't have the 'secure' directive.    validate-set-cookie-header
                Error    'set-cookie' header to set 'awselb' doesn't have the 'httponly' directive.  validate-set-cookie-header
line 10  col 5  Warning  The “type” attribute is unnecessary for JavaScript resources.               html-checker

✖ Found 2 errors and 1 warning

✖ Found a total of 2 errors and 1 warning
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! blurts-sonar@1.0.0 scan:breach: `sonarwhal $npm_package_homepage/?breach=Adobe`
npm ERR! Exit status 1
lesleyjanenorton commented 6 years ago

... I only wish I could convey the wave of confusion/cognitive faculty shutdown that immediately followed seeing / trying to make sense of 'blurts-sonar"... Anyway, this is definitely strange and suspicious. We have cookies set to "secureProxy" and "httpOnly" so why the httpOnly bit would be getting lost is confusing. BobM mentioned some CSP violation errors in the logs earlier today so I'll be in that neck of the repo this weekend in any case.

pdehaan commented 6 years ago

@Micheletto Are you setting some mysterious awselb cookie somewhere? Or should we just ignore these "errors"?

pdehaan commented 6 years ago

@lesleyjanenorton Yeah, sorry. I created my own little repo for the sonar linter config and dependencies.

I also have a few other boring blurts-* repos:

Although for those 3 goofy repos, I think I was using an older version of the breaches.json file from the add-on repo, so I should probably update them with the latest version from mozilla/blurts-addon /src/breaches.json since I think new breaches have been added.

lesleyjanenorton commented 6 years ago

nice! I ran a scan from the sonarwhal website that turned up all kinds of new things to obsess over.

Micheletto commented 6 years ago

@Micheletto Are you setting some mysterious awselb cookie somewhere? Or should we just ignore these "errors"?

The ELBs set the AWSELB cookie when you enable sticky sessions. We enabled sticky sessions so the rate limiting at the Nginx layer would be useful. I believe we can ignore this error based on the documentation.

groovecoder commented 6 years ago

@Micheletto - do we plan to use sticky sessions to rate-limit at the nginx layer in "production"?

Basically, can I close this issue, or should we leave it open as the task to change our rate-limiting layer and remove the sticky sessions?

Micheletto commented 6 years ago

@Micheletto - do we plan to use sticky sessions to rate-limit at the nginx layer in "production"?

For now we do. I think that it should eventually migrate to using the iprepd service, but that's not yet ready for prime time.

groovecoder commented 6 years ago

Okay, and it sounds like we don't need to set the Secure flag on the awselb cookie, so we can close this issue?

Micheletto commented 6 years ago

Okay, and it sounds like we don't need to set the Secure flag on the awselb cookie, so we can close this issue?

@psiinon can you advise us on whether this is safe?

psiinon commented 6 years ago

@Micheletto yeah this is fine - as per https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/elb-sticky-sessions.html :

You can't set the secure flag or HttpOnly flag on your duration-based session stickiness cookies. 
However, these cookies contain no sensitive data. Note that if you set the secure flag or HttpOnly flag on an application-controlled session stickiness cookie, it is also set on the AWSELB cookie.