-
Notifications
You must be signed in to change notification settings - Fork 22.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Observatory docs to MDN #33793
base: main
Are you sure you want to change the base?
Add Observatory docs to MDN #33793
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…lls/content into add-observatory-docs-to-mdn
files/en-us/web/security/practical_implementation/clickjacking/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation/clickjacking/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation/clickjacking/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation/cookies/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation/cookies/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation/referrer_policy/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation/referrer_policy/index.md
Outdated
Show resolved
Hide resolved
…/index.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…/index.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…x.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…/index.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…x.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…icy/index.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…icy/index.md Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preview pages for the last few files did not update, I am guessing because of an incomplete check (check-redirects).
I'm going by your updates/responses and the overall content is looking in good shape. Leaving my +1 here. Let me know if you need me to check anything again. Thanks @chrisdavidmills!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some links might need to be updated
files/en-us/web/security/practical_implementation_guides/robots_txt/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/robots_txt/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/cookies/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/cookies/index.md
Outdated
Show resolved
Hide resolved
- `Path` | ||
- : Cookies should be set to the most restrictive `Path` possible; for most applications, this will be set to the root directory. | ||
- `SameSite` | ||
- : Forbid sending the cookie via cross-origin requests (for example from {{htmlelement("img")}} element), as a strong [anti-CSRF](/en-US/docs/Web/Security/Practical_implementation_guides/CSRF_prevention) measure. `SameSite` is also useful in protecting against [Clickjacking](/en-US/docs/Glossary/Clickjacking) attacks, in cases that rely on the user being authenticated. You should use one of the following two values: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- : Forbid sending the cookie via cross-origin requests (for example from {{htmlelement("img")}} element), as a strong [anti-CSRF](/en-US/docs/Web/Security/Practical_implementation_guides/CSRF_prevention) measure. `SameSite` is also useful in protecting against [Clickjacking](/en-US/docs/Glossary/Clickjacking) attacks, in cases that rely on the user being authenticated. You should use one of the following two values: | |
- : Forbid sending the cookie via cross-origin requests (for example from an {{htmlelement("img")}} element), as a strong [anti-CSRF](/en-US/docs/Web/Security/Practical_implementation_guides/CSRF_prevention) measure by setting `SameSite` to `Strict`. `SameSite` is also useful in protecting against [Clickjacking](/en-US/docs/Glossary/Clickjacking) attacks, in cases that rely on the user being authenticated. You should use one of the following two values: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a couple of updates here, but I've not added "by setting SameSite
to Strict
" — the end of the paragraph and the following sub-bullets detail which values to use.
Are you saying you think it should always be set to Strict
? I thought Lax
was OK if Strict
caused problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My point is that setting SameSite
to strict
prevents
sending the cookie via cross-origin requests (for example from {{htmlelement("img")}} element), as a strong anti-CSRF measure.
Setting SameSite
to lax
which is the same as not setting SameSite
at all, allows sending the cookie via cross-origin requests, potentially enabling CSRF attacks.
Are you saying you think it should always be set to Strict? I thought Lax was OK if Strict caused problems?
No, you're right, that lax
is needed if using strict
causes problems, but I just want to make sure it's clear that only setting Samesite
to strict
will a strong anti-CSRF measure. Setting Samesite
to lax
does not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification. I've updated this section to the following:
SameSite
-
: Forbid sending cookies via cross-origin requests (for example from {{htmlelement("img")}} elements) using
SameSite
. You should use one of the following two values:SameSite=Strict
: Only send the cookie when your site is directly navigated to. This is a strong anti-CSRF measure, so use this value if possible.SameSite=Lax
: Additionally send the cookie when navigating to your site from another site. This is the default behavior used in modern browsers if noSameSite
directive is set, and should only be used ifStrict
is too restrictive.
Both of the above values are useful in protecting against Clickjacking attacks in cases that rely on the user being authenticated.
-
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting SameSite to lax which is the same as not setting SameSite at all, allows sending the cookie via cross-origin requests, potentially enabling CSRF attacks.
That's not true. Only Chrome sets SameSite=lax
by default, and funnily there are some caveats too. The explicit setting is more stricter than the implied, due to Chrome being to aggressive during roll-out and not feeding their changes back properly to the standards. https://www.ietf.org/archive/id/draft-ietf-httpbis-rfc6265bis-10.html#section-5.4.7.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Do I need to update the text I've used here then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mozfreddyb what should I say?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something along the lines of
... while in theory, strict
would be a good setting, we know it's breaking navigations (users clicking a link to arrive at the website appears not to be logged in, while the browser has in fact omitted the cookies on purpose).
The best middleground is to something like this
- use strict only on csrf/xsrf tokens
- use strict everywhere, but do a refresh/reload/cookie-check in javascript during every page load iff there's an indication that the user is logged in but not sending all cookies
files/en-us/web/security/practical_implementation_guides/robots_txt/index.md
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/sri/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/tls/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/tls/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/tls/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/tls/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/index.md
Outdated
Show resolved
Hide resolved
@gene1wood thanks for the review! I've fixed most things, but just had a few comments asking for clarification on a few points. |
files/en-us/web/security/practical_implementation_guides/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/corp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/corp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/corp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/corp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/robots_txt/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/robots_txt/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, @chrisdavidmills 🙌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Just a few comments.
|
||
- : Forbid sending cookies via cross-origin requests (for example from {{htmlelement("img")}} elements) using `SameSite`. You should use one of the following two values: | ||
|
||
- `SameSite=Strict`: Only send the cookie when your site is directly navigated to. This is a strong [anti-CSRF](/en-US/docs/Web/Security/Practical_implementation_guides/CSRF_prevention) measure, so use this value if possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't fully true. If you are navigating from a.example to b.example, the cookies will be omitted. They are only sent when the navigation is same-site.
Thus, SameSite=strict is breaking pages left and right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've updated this wording to
SameSite=Strict
: Only send the cookie on same-site navigations. Cookies are omitted on same-origin navigations (e.g.a.example.com
tob.example.com
). This is a very strict setting, but it does provide strong CSRF protection, so use this value if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cookies are omitted in cross-site requests (hotlinking) and also on cross-site navigation (e.g., when following a link from a different web page). Cookies will be only sent in same-site contexts (context here meaning navigation & other requests)
- : Forbid sending cookies via cross-origin requests (for example from {{htmlelement("img")}} elements) using `SameSite`. You should use one of the following two values: | ||
|
||
- `SameSite=Strict`: Only send the cookie when your site is directly navigated to. This is a strong [anti-CSRF](/en-US/docs/Web/Security/Practical_implementation_guides/CSRF_prevention) measure, so use this value if possible. | ||
- `SameSite=Lax`: Additionally send the cookie when navigating to your site from another site. This is the default behavior used in modern browsers if no `SameSite` directive is set, and should only be used if `Strict` is too restrictive. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"additionally" seems odd when you have to pick one of the settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I've updated it to
SameSite=Lax
: Send the cookie on same-site and same-origin navigations, and when navigating to your site from another site. This is the default behavior used in modern browsers if noSameSite
directive is set, and should be used ifStrict
is too restrictive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Remember that navigations are not the most interesting thing for CSRF / same-site cookies, but inclusion of cross-origin resources from attackers is.
-
same-site is a superset of same-origin-
How about "Only send the cookie in same-site requests but also when navigating to your website."
- Omit the "default behavior in modern browsers". Only Chrome ships this and Firefox currently has no intention of shipping it.
@@ -6,28 +6,51 @@ page-type: guide | |||
|
|||
{{QuickLinksWithSubpages("/en-US/docs/Web/Security")}} | |||
|
|||
Cross-Origin Resource Policy is set by the {{httpheader("Cross-Origin-Resource-Policy")}} header, which lets websites and applications opt in to protection against certain requests from other origins (such as those issued with elements like {{htmlelement("script")}} and {{htmlelement("img")}}). | |||
Cross-Origin Resource Policy (CORP) is set by the {{httpheader("Cross-Origin-Resource-Policy")}} header, which lets websites and applications opt in to protection against certain cross-origin requests (such as those made by the {{htmlelement("script")}} and {{htmlelement("img")}} elements). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unclear what kind of protection this is serving from the sentence above. The request is going to be sent anyway. This is a protection of the response, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that phrasing is a bit weird. I've updated it to:
Cross-Origin Resource Policy (CORP) is set by the {{httpheader("Cross-Origin-Resource-Policy")}} header, which lets websites and applications opt-in to protection against vulnerabilities related to certain cross-origin requests (such as those made by the {{htmlelement("script")}} and {{htmlelement("img")}} elements).
Is that any better? The solution section explains that it works by stripping out the response body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is :) thanks
The possible values are: | ||
|
||
- `same-origin` | ||
- : Limits resource access to requests coming from the same origin. This is recommended for requests for sensitive user information, or requests to private APIs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "....recommended for URLs that reply with sensitive user information or private APIs"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
like it; updated
|
||
Set the most restrictive value possible for your site. | ||
|
||
If your use case requires `cross-origin` access, opt into a better default by sending a {{httpheader("Cross-Origin-Embedder-Policy")}} header. This will prevent loading of cross-origin resources that don't also explicitly send a `Cross-Origin-Resource-Policy: cross-origin` header. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If, in turn, your page requires access to cross-origin
resources, ...."
Note that COEP is a header that needs to be set on the requesting page and CORP is a header that needs to be sent from the page that is returning the response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this paragraph to
If, in turn, your site requires access to cross-origin resources, opt into a better default by sending a {{httpheader("Cross-Origin-Embedder-Policy")}} header along with the associated requests. This will prevent loading of cross-origin resources that don't also explicitly send a
Cross-Origin-Resource-Policy: cross-origin
header.
I've also updated some of the earlier wording about CORP to say that it is recommended on responses. I've also stated that it is a response header in the intro.
@@ -20,7 +20,7 @@ Use SRI to lock an external JavaScript resource to its known contents at a speci | |||
|
|||
If the file is modified after this point, the hash won't match, and supporting web browsers will refuse to load it. | |||
|
|||
SRI should be used when loading all external JavaScript or stylesheet resources. The resources should be loaded over HTTPS and, ideally, from a similar origin. Fewer origins reduce the potential for tampering. | |||
SRI should be used when loading all external JavaScript or stylesheet resources. The resources should be loaded over HTTPS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand. What's the word "all" doing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasting space, I think. Removed ;-)
This pull request has merge conflicts that must be resolved before it can be merged. |
Description
Mozilla Observatory is being moved to MDN. This PR adds the accompanying cheat sheet docs to MDN, and restructures the existing security docs to make space for them.
Motivation
Additional details
See mdn/mdn#548 for the MDN project tracking item.
Related issues and pull requests