-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
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>
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
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 added a note to the bottom of the section to communicate these details:
Note: In theory,
SameSite=Strict
should be more useful than it is in practice. It often breaks navigations — for example, users clicking a link to a website on which they are already logged in (i.e. a valid session cokie is set) appear not to be logged in, because the browser has deliberately omitted the session cookie. The best middle ground is to useSameSite=Strict
only on tokens where CSRF is a concern or useSameSite=Strict
everywhere, but reload the page and do a cookie check in JavaScript if there's an indication that the user is logged in but required cookies are not being sent.
Does that make sense? I did wonder about the JavaScript check you recommend — since the recommendation is to set HttpOnly
, especially for session identifiers, surely this won't work?
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)
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, updated to
SameSite=Strict
: Only send the cookie in same-site contexts (navigations and other requests). Cookies are omitted in same-origin contexts (e.g. navigatinga.example.com
tob.example.com
), cross-site requests (e.g. hotlinking), and cross-site navigation (e.g. when following a link from a different web page). This is a very strict setting, but it does provide strong CSRF protection, so use this value if possible.
- : 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.
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.
Updated to:
- `SameSite=Lax`: Send the cookie in same-site requests and when navigating _to_ your website. This should be used if `Strict` is too restrictive.
files/en-us/web/security/practical_implementation_guides/corp/index.md
Outdated
Show resolved
Hide resolved
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
files/en-us/web/security/practical_implementation_guides/corp/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/security/practical_implementation_guides/sri/index.md
Outdated
Show resolved
Hide resolved
This pull request has merge conflicts that must be resolved before it can be merged. |
Looks like I can't finalize my review as "approved" given that this has already merged, but just for posterity. I approve! 🙂 |
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