Skip to content

Update: Cross-Site Request Forgery Prevention #691

@rwinch

Description

@rwinch

What is missing or needs to be updated?

Summary

The Cross-Site Request Forgery Prevention Cheat Sheet leaves me with some questions around why SameSite cannot be used on its own for CSRF protection. It would be nice if the Chat

Details

Currently, the Cross-Site Request Forgery Prevention Cheat Sheet SameSite Cookie Attribute section states:

It is important to note that this attribute should be implemented as an additional layer defense in depth concept. This attribute protects the user through the browsers supporting it, and it contains as well 2 ways to bypass it as mentioned in the following section. This attribute should not replace having a CSRF Token. Instead, it should co-exist with that token in order to protect the user in a more robust way.

To me this is stating that we should not use SameSite Cookie Attribute as the primary mode of defense against CSRF for the reasons listed in the RFC. The RFC describes two ways to bypass Lax enforcement:

Lax enforcement provides reasonable defense in depth against CSRF
attacks that rely on unsafe HTTP methods (like "POST"), but does not
offer a robust defense against CSRF as a general category of attack:

  1. Attackers can still pop up new windows or trigger top-level
    navigations in order to create a "same-site" request (as
    described in section 2.1), which is only a speedbump along the
    road to exploitation.

  2. Features like "" [prerendering] can be
    exploited to create "same-site" requests without the risk of user
    detection.

The RFC says that these bypasses are the reason we should not use SameSite for the sole way of preventing CSRF. However, the bypasses are for Lax enforcement. What about Strict enforcement?

Immediately after providing the bypasses, the RFC states:

When possible, developers should use a session management mechanism
such as that described in Section 8.8.2 to mitigate the risk of CSRF
more completely.

Section 8.8.2 describes how two cookies, one for read access (SameSite=Lax) and one for write access (SameSite=Strict), can provide a robust defense in depth against CSRF attacks that avoids top level navigation confusions of a single SameSite=Strict cookie.

Setting the "SameSite" attribute in "strict" mode provides robust
defense in depth against CSRF attacks, but has the potential to
confuse users unless sites' developers carefully ensure that their
cookie-based session management systems deal reasonably well with
top-level navigations.
...
Developers can avoid this confusion by adopting a session management
system that relies on not one, but two cookies: one conceptually
granting "read" access, another granting "write" access. The latter
could be marked as "SameSite", and its absence would prompt a
reauthentication step before executing any non-idempotent action.
The former could drop the "SameSite" attribute entirely, or choose
the "Lax" version of enforcement, in order to allow users access to
data via top-level navigation.

How should this be resolved?

I think it would be nice of the Cheat Sheet was updated to answer:

  • There are many ways to use SameSite attribute. How is the Cheat Sheet assuming that SameSite is being used when it states:

    It is important to note that this attribute should be implemented as an additional layer defense in depth concept.

    • Is the Cheat Sheet suggesting to add SameSite to the session cookie?
    • Is the Cheat Sheet suggesting separate read/write cookies as described above?
    • Should Lax or Strict enforcement be specified?
  • Can SameSite (as described using separate read/write cookie above) be used to solve CSRF without Synchronizer Token Pattern?

    • If it is not adequate, "What is the reason that Synchronizer Token Pattern is still necessary (the explanation that is currently provided needs some clarity)?"

Perhaps one reason that Synchronizer Token Pattern is still necessary is to protect users of older browsers which do not support SameSite. If this is the case, it would be nice to update the document to reflect that reasoning and/or any other reasoning.

Metadata

Metadata

Assignees

No one assigned

    Labels

    ACK_WAITINGIssue waiting acknowledgement from core team before to start the work to fix it.HELP_WANTEDIssue for which help is wanted to do the job.UPDATE_CSIssue about the update/refactoring of a existing cheat sheet.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions