Skip to content
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

Merged
merged 68 commits into from
Jun 24, 2024

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented May 28, 2024

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

@chrisdavidmills chrisdavidmills requested a review from a team as a code owner May 28, 2024 13:14
@chrisdavidmills chrisdavidmills requested review from Elchi3 and removed request for a team May 28, 2024 13:14
@chrisdavidmills chrisdavidmills marked this pull request as draft May 28, 2024 13:15
@github-actions github-actions bot added Content:Security Security docs size/m [PR only] 51-500 LoC changed labels May 28, 2024
Copy link
Contributor

github-actions bot commented May 28, 2024

Preview URLs (21 pages)
Flaws (6)

Note! 15 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/HTTP
Title: HTTP
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/observatory/

URL: /en-US/docs/Web/HTTP/Headers/X-Content-Type-Options
Title: X-Content-Type-Options
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/observatory/

URL: /en-US/docs/Web/Security
Title: Security on the web
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/observatory/

URL: /en-US/docs/Web/Security/Practical_implementation_guides
Title: Practical security implementation guides
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/observatory/

URL: /en-US/docs/Web/Security/Transport_Layer_Security
Title: Transport Layer Security
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/observatory/

URL: /en-US/docs/Learn/Server-side/Apache_Configuration_htaccess
Title: Apache Configuration: .htaccess
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/observatory/
External URLs (28)

URL: /en-US/docs/Web/Security
Title: Security on the web


URL: /en-US/docs/Web/Security/Practical_implementation_guides
Title: Practical security implementation guides


URL: /en-US/docs/Web/Security/Practical_implementation_guides/CORP
Title: Cross-Origin Resource Policy (CORP) implementation


URL: /en-US/docs/Web/Security/Practical_implementation_guides/Turning_off_form_autocompletion
Title: How to turn off form autocompletion


URL: /en-US/docs/Web/Security/Practical_implementation_guides/CSP
Title: Content Security Policy (CSP) implementation


URL: /en-US/docs/Web/Security/Practical_implementation_guides/SRI
Title: Subresource integrity (SRI) implementation


URL: /en-US/docs/Web/Security/Practical_implementation_guides/Clickjacking
Title: Clickjacking prevention


URL: /en-US/docs/Web/Security/Practical_implementation_guides/CORS
Title: Cross-Origin Resource Sharing (CORS) configuration


URL: /en-US/docs/Web/Security/Practical_implementation_guides/Cookies
Title: Secure cookie configuration


URL: /en-US/docs/Web/Security/Practical_implementation_guides/Robots_txt
Title: robots.txt configuration


URL: /en-US/docs/Web/Security/Practical_implementation_guides/TLS
Title: Transport Layer Security (TLS) configuration


URL: /en-US/docs/Web/Security/Practical_implementation_guides/CSRF_prevention
Title: Cross-site request forgery (CSRF) prevention


URL: /en-US/docs/Web/Security/Transport_Layer_Security
Title: Transport Layer Security

(comment last updated: 2024-06-23 13:24:27)

@github-actions github-actions bot added size/l [PR only] 501-1000 LoC changed and removed size/m [PR only] 51-500 LoC changed labels May 28, 2024
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
files/en-us/web/security/index.md Outdated Show resolved Hide resolved
files/en-us/web/security/index.md Outdated Show resolved Hide resolved
chrisdavidmills and others added 3 commits May 28, 2024 14:50
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…lls/content into add-observatory-docs-to-mdn
@github-actions github-actions bot added size/xl [PR only] >1000 LoC changed and removed size/l [PR only] 501-1000 LoC changed labels May 30, 2024
chrisdavidmills and others added 10 commits May 30, 2024 17:43
…/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>
- `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:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- : 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:

Copy link
Contributor Author

@chrisdavidmills chrisdavidmills Jun 19, 2024

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?

Copy link

@gene1wood gene1wood Jun 20, 2024

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.

Copy link
Contributor Author

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 no SameSite directive is set, and should only be used if Strict is too restrictive.

      Both of the above values are useful in protecting against Clickjacking attacks in cases that rely on the user being authenticated.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 👍

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.

Copy link
Contributor Author

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?

Copy link
Contributor Author

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?

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

Copy link
Contributor Author

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 use SameSite=Strict only on tokens where CSRF is a concern or use SameSite=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?

@chrisdavidmills
Copy link
Contributor Author

@gene1wood thanks for the review! I've fixed most things, but just had a few comments asking for clarification on a few points.

Copy link
Contributor

@dipikabh dipikabh left a 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 🙌

Copy link

@mozfreddyb mozfreddyb left a 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.

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.

Copy link
Contributor Author

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 to b.example.com). This is a very strict setting, but it does provide strong CSRF protection, so use this value if possible.

Copy link

@mozfreddyb mozfreddyb Jun 21, 2024

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)

Copy link
Contributor Author

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. navigating a.example.com to b.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.

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.

Copy link
Contributor Author

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 no SameSite directive is set, and should be used if Strict is too restrictive.

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.

Copy link
Contributor Author

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.

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.

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"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like it; updated

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Jun 21, 2024
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@github-actions github-actions bot removed the merge conflicts 🚧 [PR only] label Jun 21, 2024
@chrisdavidmills chrisdavidmills merged commit 75e254f into mdn:main Jun 24, 2024
9 checks passed
@chrisdavidmills chrisdavidmills deleted the add-observatory-docs-to-mdn branch June 24, 2024 13:06
@mozfreddyb
Copy link

Looks like I can't finalize my review as "approved" given that this has already merged, but just for posterity. I approve! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs Content:Learn:Django Learning area Django docs Content:Learn Learning area docs Content:Security Security docs size/xl [PR only] >1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants