mozilla / concepts

Static site tool for generating product concepts
4 stars 6 forks source link

Security Checklist #27

Closed jvehent closed 5 years ago

jvehent commented 5 years ago

Risk Management

Development

Web Applications

Security Features

Common issues

clouserw commented 5 years ago

Checking some free boxes above:

jvehent commented 5 years ago

@g-k please help @clouserw and team figure out a CSP that @bqbn can set by default on the hosting side. From the RRA, the CSP can pretty must only allow google analytics, so we can prevent loading untrusted scripts down the road.

g-k commented 5 years ago

@bqbn how is this built and hosted for prod? Is it behind nginx (in which case we can capture the CSP reports there)?

I copied over the copy-me concept and can run the dev server, but still get a 404 page. Local dev probably has features we can lock down w/ CSP for prod (code reloading, react dev tools support, etc.).

bqbn commented 5 years ago

how is this built and hosted for prod? Is it behind nginx

@g-k no nginx is involved. The build is driven by CircleCI only. See https://github.com/mozilla/concepts/blob/master/.circleci/config.yml

There are only 3 components, S3, Cloudfront and Edge lambda. The architecture looks like this: https://github.com/mozilla-services/terraform-cloudops-static-website#architecture

The security headers are set for each S3 object as its metadata. Their current values are here: https://github.com/mozilla/concepts/blob/master/.utils/deploy.sh#L32-L45

I copied over the copy-me concept and can run the dev server, but still get a 404 page.

I believe the site has no contents yet as of this comment. 404 is expected.

g-k commented 5 years ago

Thanks @bqbn! The CSP in https://github.com/mozilla/concepts/blob/master/.utils/deploy.sh#L32-L45 looks good I would just:

fzzzy commented 5 years ago

We don't set any cookies, so I'm checking the box related to that.

fzzzy commented 5 years ago

rel="noopener noreferrer" is now present on the one target="_blank" link.

fzzzy commented 5 years ago

I made a pr to use audit-filter but we should ensure it is run in ci. https://github.com/mozilla/concepts/pull/57

fzzzy commented 5 years ago

The csp being defined in the aws metadata, using one csp for the whole site, doesn't seem to be compatible with gatsby, which inlines all the things. We've enabled gatsby-plugin-csp, which creates a meta http-equiv="Content-Security-Policy" tag in the header of each page and adds the hashes of all the inline resources.

Here's an example of the meta tag it added for the version of the page currently at https://firstlook.stage.mozaws.net/demo

base-uri 'self'; default-src 'self'; script-src 'self' 'sha256-LQr/qWtsc0SPbJwEyHdeZ1I/TUWNuy4r3984PAFyjqw=' 'sha256-uk7oUIAmdTl64uzSqEcJRD+cLnKvLUG06SQ9Kkydq5s='; style-src 'self' 'sha256-1FISDnGkJZuH9HKDyfp7tl+Ei6rGyEuwOdWWUcG5WGg=' 'sha256-Rvcv+/u0Wt4cEnCmaHHZIB0mmt9BP8QaLWGSA5x2LiE='; object-src 'none'; form-action 'self'; font-src 'self' data:; connect-src 'self'; img-src 'self' data:;

@g-k does this look ok? Is embedding the csp in a meta tag even secure?

The two things that jump out at me as wrong are font-src data: and img-src data:. I believe we will need to figure out how to get rid of those data urls.

I think we can also set form-action and connect-src to none.

fzzzy commented 5 years ago

Oh, and for the meta http-equiv="Content-Security-Policy" tag to be respected, we will have to remove the csp header from the aws metadata. I tried to do it but wasn't successful.

clouserw commented 5 years ago

I think @bqbn will need to flush all the current objects to clear those. @bqbn please help ^ :)

g-k commented 5 years ago

@fzzzy

@g-k does this look ok? Is embedding the csp in a meta tag even secure?

Looks OK. CSP in meta tags is fine too.

Notes / caveats:

To workaround we can either:

Dropping the directives that aren't allowed is an option too but I'd rather not do that since frame-ancestors is good clickjacking protection.

The two things that jump out at me as wrong are font-src data: and img-src data:. I believe we will need to figure out how to get rid of those data urls.

That would be nice, but I also recognize that it makes sense to inline them for perf.

I think we can also set form-action and connect-src to none.

+1 if the page doesn't have forms or need to make XHR-ish requests that'd be great.

bqbn commented 5 years ago

I think @bqbn will need to flush all the current objects to clear those.

So I read the above, and if I understand correctly, what we want is to not set the "Content-Security-Policy" meta tag for individual S3 object (so it is not returned as an HTTP header by S3), instead, we embed "Content-Security-Policy" in a meta tag of the HTML file.

If that is the case, then all we need to do is to remove all the CSP reference in https://github.com/mozilla/concepts/blob/master/.utils/deploy.sh (i.e. [1], [2], and [3]), because that's when and where the objects are tagged when we upload them to S3.

If you do that, next -stage release should overwrite the existing objects without the "Content-Security-Policy" meta tag, and invalidate all objects in Cloudfront too.

fzzzy commented 5 years ago

@bqbn Yes, I think your analysis is correct. However, I already tried just removing the csp references from deploy.sh and it wasn't enough, because the old metadata csp was still there. So I thought they should be cleared somehow.

But I see from @g-k 's second comment that it is possible both to set the CSP in a header, and to also set it in a meta tag, providing additional restrictions. I had tried this but it appeared to not work because it's only possible to make the csp stricter in the meta tag, not looser. So, this is exactly what I need, and I can take care of all of it.

Finally, I suppose the data: urls will be safe because the rest of the csp is so locked down, so there should be no opportunity for an attacker to inject a hostile data url.

fzzzy commented 5 years ago

Ok, the CSP seems to be configured mostly properly for https://firstlook.stage.mozaws.net/demo now. This is the csp defined in a header:

content-security-policy: connect-src 'self'; base-uri 'self'; form-action 'none'; frame-ancestors 'self'; frame-src 'self'; object-src 'none'

And this is the CSP defined in the meta tag (I replaced ' with ' to make it easier to read):

script-src 'self' https://tagmanager.google.com/ https://www.googletagmanager.com/ https://www.google-analytics.com/ 'sha256-LQr/qWtsc0SPbJwEyHdeZ1I/TUWNuy4r3984PAFyjqw=' 'sha256-HtzrlW/hAqfqb9H/t4kUobn223YonGlv6uBsK+ru2dg='; style-src 'self' https://tagmanager.google.com/ https://fonts.googleapis.com/ 'sha256-GRMHs5TYPhtTgf5y7qbDkWG7ZR+2jVCP8jHEDA4yHWc='; img-src 'self' data: https://ssl.gstatic.com; font-src 'self' data:; default-src 'self';

@g-k Does this look ok?

clouserw commented 5 years ago

We've got an A+ on the observatory @ https://observatory.mozilla.org/analyze/firstlook.stage.mozaws.net

@psiinon -- please confirm we pass the Security Baseline scanner. You can use https://firstlook.stage.mozaws.net/demo as there isn't really a front page (or stable URL) for this project

psiinon commented 5 years ago

@clouserw its currently failing the baseline on:

The Cross-Domain Misconfiguration could be a false positive if you really do want any site to be able to access that endpoint. If so we can whitelist it, assuming we can come up with a suitably specific regex for the URL

Thats based on crawling from https://firstlook.stage.mozaws.net - is https://firstlook.stage.mozaws.net/demo a better option going forwards or is that just a temporary page that is expected to change?

fzzzy commented 5 years ago

Thank you @psiinon -- can you also try crawling from https://firstlook.stage.mozaws.net/demo

I believe the demo url is now and always will be the correct "root" of the site to test things with. The rest of the pages will be at somewhat private urls, and / is just a 404. The pages won't be enumerable; if someone has a url to a page, they can see it, but if they don't, they can't. All the pages will have the same js and etc. as the demo page, but they will have different text and image content.

@bqbn Do you know how "No HSTS on 404s" could be fixed?

I believe I fixed the cross-domain misconfiguration for json files. https://github.com/mozilla/concepts/pull/68

g-k commented 5 years ago

@fzzzy https://github.com/mozilla/concepts/issues/27#issuecomment-467155500 looks good! nit: it'd be nice to set default-src in the HTTP header in case we forget it on a page, but if that needs to be page-specific or it breaks how multiple CSPs combine it's OK as is.

Also per discussion in https://github.com/mozilla-services/terraform-cloudops-static-website/issues/8 we're dropping the report-uri requirement from the checklist (I still need to update the wiki).

bqbn commented 5 years ago

@fzzzy, does this site have a custom 404 page? If so, we can make Cloudfront always return that page when someone tries to access a non-existing page/object.

I just need to know the location of the 404 page relative to / in order to configure the Cloudfront.

fzzzy commented 5 years ago

@bqbn It doesn't right now, but I will add /error.html, which I think is the default thing s3 returns on 404, right?

fzzzy commented 5 years ago

@g-k Thank you very much. Yes, default-src cannot be in the header because if it is there, img-src falls back to default-src, and then cannot be overridden by the meta tag.

About the report-uri, is it possible to have the report uri point to another site? Since we are a static site we couldn't have a report-uri on the same site, but if there was a generic service set up somewhere that could handle the csp violation reports, we could point to that.

g-k commented 5 years ago

Yes, default-src cannot be in the header because if it is there, img-src falls back to default-src, and then cannot be overridden by the meta tag.

That makes sense. TIL

About the report-uri, is it possible to have the report uri point to another site?

Definitely! Not sure on the details, but I think we'd need to add it to connect-src to send the reports. We discussed some options (standing up a shared nginx instance, using report-uri.com, modifying sentry to accept CSP reports from Fx) in https://github.com/mozilla-services/terraform-cloudops-static-website/issues/8 and depending on what kind of bandwidth Ops has that'd be nice to have, but we don't consider it a security blocker.

bqbn commented 5 years ago

@fzzzy, I don't think we can configure S3 to return a default object if it receives a request for a non-existing object.

However, we can configure Cloudfront to retrieve a particular object from S3 if S3 tells Cloudfront something dosn't exit in the bucket. That's why we can put a custom 404 page in the S3 bucket, and then configure Cloudfront to retrieve that in case someone tries to access a non-existing page on this web site.

That said, now I see the site has a 404.html page at the root, i.e. https://firstlook.stage.mozaws.net/404.html. Can I configure Cloudfront to use that? or do you still want it to be /error.html?

fzzzy commented 5 years ago

@bqbn 404.html is fine. Thanks!

johngruen commented 5 years ago

@bqbn bumping this...is this something we could do today?

bqbn commented 5 years ago

@johngruen @fzzzy the test link above, i.e. https://firstlook.stage.mozaws.net/sitemap.xml has a HSTS header now, so should other random non-existing pages.

fzzzy commented 5 years ago

Thank you very much, @bqbn!

psiinon commented 5 years ago

Staging is now passing the baseline :) Good work guys!

fzzzy commented 5 years ago

Thanks @psiinon!!

clouserw commented 5 years ago

I made the repository public, as discussed, and enabled Renovate which should make a PR shortly. I also talked to @jvehent and we agreed this is in good shape and can be closed. Thanks everyone