[Security] Implement stateless headers/cookies-based CSRF protection #58095

nicolas-grekas · 2024-08-26T16:53:51Z

Q	A
Branch?	7.2
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	#13464
License	MIT

#54705 made me think about our CSRF protection and I wrote the attached CSRF token manager to implement stateless headers/cookies-based validation.

By defaults, the existing stateful manager is used.
In order to leverage this new stateless manager, one needs to list the token ids that should be managed this way:

framework:
    csrf_protection:
        stateless_token_ids: [my_stateless_token_id]

 * This CSRF token manager uses a combination of cookie and headers to validate non-persistent tokens.
 *
 * This manager is designed to be stateless and compatible with HTTP-caching.
 *
 * First, we validate the source of the request using the Origin/Referer headers. This relies
 * on the app being able to know its own target origin. Don't miss configuring your reverse proxy to
 * send the X-Forwarded-* / Forwarded headers if you're behind one.
 *
 * Then, we validate the request using a cookie and a CsrfToken. If the cookie is found, it should
 * contain the same value as the CsrfToken. A JavaScript snippet on the client side is responsible
 * for performing this double-submission. The token value should be regenerated on every request
 * using a cryptographically secure random generator.
 *
 * If either double-submit or Origin/Referer headers are missing, it typically indicates that
 * JavaScript is disabled on the client side, or that the JavaScript snippet was not properly
 * implemented, or that the Origin/Referer headers were filtered out.
 *
 * Requests lacking both double-submit and origin information are deemed insecure.
 *
 * When a session is found, a behavioral check is added to ensure that the validation method does not
 * downgrade from double-submit to origin checks. This prevents attackers from exploiting potentially
 * less secure validation methods once a more secure method has been confirmed as functional.
 *
 * On HTTPS connections, the cookie is prefixed with "__Host-" to prevent it from being forged on an
 * HTTP channel. On the JS side, the cookie should be set with samesite=strict to strengthen the CSRF
 * protection. The cookie is always cleared on the response to prevent any further use of the token.
 *
 * The $checkHeader argument allows the token to be checked in a header instead of or in addition to a
 * cookie. This makes it harder for an attacker to forge a request, though it may also pose challenges
 * when setting the header depending on the client-side framework in use.
 *
 * When a fallback CSRF token manager is provided, only tokens listed in the $tokenIds argument will be
 * managed by this manager. All other tokens will be delegated to the fallback manager.

Since it's stateless, end users won't loose their content if they take time to submit a form: even if the session is destroyed while they populate their form, remember-me will reconnect them and the form will be accepted.

Recipe update at symfony/recipes#1337

smnandre · 2024-08-26T23:22:17Z

(cc @tucksaun )

jderusse · 2024-08-27T13:39:35Z

AWASP discourage using this pattern and recommands using a hash/crypted value coupled to the session (or JWT lifetime) instead

https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#naive-double-submit-cookie-pattern-discouraged

nicolas-grekas · 2024-08-27T13:49:53Z

I've read that, and what I'm proposing here is significantly different from what they discourage to be considered seriously IMHO, esp with the new cookie attributes and with the style of double-submit I'm using here (which is not discussed on that page).

Aka I'd be interested in an open-minded threat model analysis of what this protects against (and what are the drawbacks)

nicolas-grekas · 2024-08-27T14:18:42Z

For additional background: the OWASP page links to https://owasp.org/www-chapter-london/assets/slides/David_Johansson-Double_Defeat_of_Double-Submit_Cookie.pdf to give more insights about vulnerabilities with the "naive double-submit".

I read that doc too, and they don't apply to this PR:

MITM are ruled out by HTTPS nowadays (and no CSRF techniques can resist to MITM anyway)
HTTPS is enforced for cookies by the __Host- prefix, which prevents overriding them from HTTP
overriding the cookie isn't enough since the double-submit happens via a custom header (which is listed as a valid CSRF-protection on that PDF and on the OWASP page)
the Origin header is checked also when available

Binding CSRF tokens to any server-side state is ruling out statelessness/cacheability of the strategy so I'm not looking for anything hashed/signed/etc.

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php

src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php

src/Symfony/Bundle/FrameworkBundle/Resources/config/form_csrf.php

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

wouterj · 2024-09-17T14:50:21Z

Fyi: I'm planning to look at this on Thursday. Need some focus time to properly digest all information in here :)

I don't mind if this gets merged before it, there is enough time till feature freeze to tweak it in a follow up PR if necessary.

wouterj

I feel like it may be the case that all my comments make no sense whatsoever, I had quite the struggle following the logic. Please be patient with me 😉

One of the struggles is the name of the feature. If I'm correct, the stance of Symfony 7.2 would be: verify origin and do not use tokens for CSRF protection.
Then, for cases where origin headers might not be set (e.g. corporate environments), we fallback to CSRF token protection using a modified double-submit pattern.

If that is the case, I think we should not use "double-submit" as the main name of the new feature (it's just the fallback). Setting double_submit_token_ids to enable origin checks for some token IDs sounds wrong. Let's brainstorm a better name if you agree.

In addition, I'm not sure if we should default to all the complexity the double-submit brings if this is just a fallback. Do we want to push JS code to the users that they probably don't even need? Or should we somehow make the recipe optional and have some docs that says "run composer install csrf-double-submit-pack if you can't (always) use origin checks".

Finally, by generating the CSRF token and creating the cookie in the JS code moves a security-critical part of the double-submit flow to the application code. I'm always much more in favor of keeping security-critical parts in the vendor code (3500 contributors have more security knowledge than most dev teams using Symfony). We must make it very clear that a user should not change the cryptographically secure random generator algorithm, otherwise they open themselves to vulnerabilities (as attackers can see the algorithm, they might be able to predict the value).

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php

wouterj · 2024-09-22T11:47:03Z

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php

+ * Then, we validate the request using a cookie and a CsrfToken. If the cookie is found, it should
+ * contain the same value as the CsrfToken. A JavaScript snippet on the client side is responsible
+ * for performing this double-submission. The token value should be regenerated on every request
+ * using a cryptographically secure random generator.


If the cookie is found, it should contain the same value as the CsrfToken

If I'm reading the implementation correctly, this is not the case. The name of the cookie must be equal to the CsrfToken prefixed with the cookie name, the value can be anything you like.

I can't directly think of a reason why the implementation was chosen to be like this, but we need to update the comment if this is indeed the logic.

Putting the token in the name of the cookie prevents race conditions.
About the wording, it's approximate, but not incorrect. The cookie is both its name and its value. And either of them can contain the token.

is there any doc or howto (not talking about the recipe) for this JavaScript snippet on the client side?
if not bounded for example with turbo or whatever high level JS lib ?

The recipe is the doc on the topic. We can add a link after its merged.

@nicolas-grekas for projects that implement manual CSRF protection (e.g. with the csrf_token() twig function to manually generate a token, and some verification in the controllers), it would be great to update the recipe with an example of a Twig template showing how to construct the hidden crsf form field, so that the sample .js in the recipe can work as is (e.g. what initial values to use for the required data-attributes?) - I'm trying to get it to work on a simple login form, but I'm having trouble with the cookie value that seems to be ignored during the validation.

nicolas-grekas · 2024-09-25T16:17:16Z

@wouterj this double-submit is not only a fallback. It's also a defense in-depth. What I mean is that if Origin is broken for any reasons, users won't be immediately exposed.

About the naming, "double-submit" suits me, but I also wondered about naming this "same-origin": SameOriginCsrfManager, same_origin_token_ids, etc. I could change if that's the consensus.

nicolas-grekas · 2024-10-03T13:28:11Z

Any other feedback @symfony/mergers ? I'm going to merge as is without any :)

Tobion · 2024-10-03T14:32:54Z

I agree that same-origin is a better name to me than double-submit because it's the main idea and is the better known term that is part of the http spec.

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php

wouterj · 2024-10-07T14:55:59Z

I also prefer "same origin CSRF manager" over double-submit 👍

Other than that, this PR is fine with me. But we need to write detailed docs about the pros and cons of all CSRF protection mechanism during stabilization phase :)

stof · 2024-10-07T15:08:05Z

One argument for renaming: it will avoid getting more reports in the future from people telling us that OWASP discourages the double submit pattern, because they miss that they only discourage naive double-submit.

nicolas-grekas · 2024-10-08T07:45:31Z

Renamed to SameSiteCsrfTokenManager.

The option is named stateless_token_ids:

framework:
    csrf_protection:
        stateless_token_ids: [my_stateless_token_id]

stof · 2024-10-08T10:17:39Z

@nicolas-grekas is it same site or same origin ? Those have different meaning.

wouterj · 2024-10-08T10:33:43Z

Good one, I believe it's indeed same-origin (exact match of scheme+hostname, including subdomain).

nicolas-grekas · 2024-10-08T12:56:51Z

Indeed, good catch, fixed!

GromNaN · 2024-12-24T09:50:13Z

src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php

@@ -73,6 +76,7 @@ public function finishView(FormView $view, FormInterface $form, array $options):
            $csrfForm = $factory->createNamed($options['csrf_field_name'], HiddenType::class, $data, [
                'block_prefix' => 'csrf_token',
                'mapped' => false,
+                'attr' => $this->fieldAttr + ['autocomplete' => 'off'],


The autocomplete attribute with value off on an hidden field seems invalid: #59294

… default CSRF token id apply only to the app; not to bundles (nicolas-grekas) This PR was merged into the 7.2 branch. Discussion ---------- [Form][FrameworkBundle] Use auto-configuration to make the default CSRF token id apply only to the app; not to bundles | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT After EasyCorp/EasyAdminBundle#6724, I realized I made a mistake in #58095: The `framework.form.csrf_protection.token_id` config option should not configure the default CSRF token id for *all* forms. Instead, we want this option to apply only to forms managed by the app. Bundles shouldn't be affected. This is what this PR does: it switches from global config to auto-configured form types only (which means app's form types). Commits ------- bf1e312 [Form][FrameworkBundle] Use auto-configuration to make the default CSRF token id apply only to the app; not to bundles

…double-submit CSRF protection (nicolas-grekas) This PR was merged into the 1.x-dev branch. Discussion ---------- [make:security:form-login] Tweak login forms to enable double-submit CSRF protection Will be leveraged by symfony/symfony#58095 Does no harm being enabled regardless of the Symfony version in use. --- Waiting on: - [x] symfony/symfony#58095 Commits ------- b13f0fb Generate data-controller="csrf-protection" on CSRF fields

nicolas-grekas requested review from chalasr, xabbuh and yceruto as code owners August 26, 2024 16:53

carsonbot added Status: Needs Review Feature Security labels Aug 26, 2024

carsonbot added this to the 7.2 milestone Aug 26, 2024

nicolas-grekas force-pushed the csrf-double-submit branch 5 times, most recently from 5b81f2f to 601b9ba Compare August 27, 2024 12:38

This was referenced Aug 27, 2024

CSRF Protection without starting session #13464

Closed

[Security][CSRF] Added CSRF CookieStorageInterface #33171

Closed

nicolas-grekas force-pushed the csrf-double-submit branch 2 times, most recently from 0e8e0da to 42233a3 Compare August 27, 2024 12:55

jderusse reviewed Aug 27, 2024

View reviewed changes

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php Outdated Show resolved Hide resolved

smnandre reviewed Aug 28, 2024

View reviewed changes

src/Symfony/Component/Form/Extension/Csrf/Type/FormTypeCsrfExtension.php Outdated Show resolved Hide resolved

smnandre reviewed Aug 28, 2024

View reviewed changes

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php Outdated Show resolved Hide resolved

smnandre reviewed Aug 28, 2024

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Resources/config/form_csrf.php Outdated Show resolved Hide resolved

nicolas-grekas force-pushed the csrf-double-submit branch 3 times, most recently from 09d6d78 to 66ae045 Compare August 28, 2024 09:45

nicolas-grekas commented Aug 28, 2024

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php Show resolved Hide resolved

nicolas-grekas commented Aug 28, 2024

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php Outdated Show resolved Hide resolved

nicolas-grekas force-pushed the csrf-double-submit branch from 7c212b9 to f459912 Compare September 17, 2024 14:29

wouterj reviewed Sep 22, 2024

View reviewed changes

javiereguiluz added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 3, 2024

94noni reviewed Oct 7, 2024

View reviewed changes

src/Symfony/Component/Security/Csrf/DoubleSubmitCsrfTokenManager.php Outdated Show resolved Hide resolved

nicolas-grekas force-pushed the csrf-double-submit branch from f459912 to ecc326f Compare October 7, 2024 13:52

nicolas-grekas changed the title ~~[Security] Implement double-submit CSRF protection~~ [Security] Implement stateless headers/cookies-based CSRF protection Oct 8, 2024

wouterj approved these changes Oct 8, 2024

View reviewed changes

chalasr approved these changes Oct 8, 2024

View reviewed changes

nicolas-grekas force-pushed the csrf-double-submit branch from ecc326f to 867c03b Compare October 8, 2024 12:53

[Security] Implement stateless headers/cookies-based CSRF protection

27d8a31

nicolas-grekas force-pushed the csrf-double-submit branch from 867c03b to 27d8a31 Compare October 8, 2024 13:08

nicolas-grekas merged commit f57a6de into symfony:7.2 Oct 8, 2024
7 of 10 checks passed

nicolas-grekas mentioned this pull request Oct 8, 2024

[Security] Implement stateless headers/cookies-based CSRF protection symfony/symfony-docs#20306

Open

nicolas-grekas deleted the csrf-double-submit branch October 8, 2024 13:14

jdreesen mentioned this pull request Oct 10, 2024

[Improvement]: access the admin API using a JWT instead of the session pimcore/pimcore#13271

Closed

fabpot mentioned this pull request Oct 27, 2024

Release v7.2.0-BETA1 #58686

Merged

GromNaN reviewed Dec 24, 2024

View reviewed changes

nicolas-grekas mentioned this pull request Feb 7, 2025

[Form][FrameworkBundle] Use auto-configuration to make the default CSRF token id apply only to the app; not to bundles #59728

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security] Implement stateless headers/cookies-based CSRF protection #58095

[Security] Implement stateless headers/cookies-based CSRF protection #58095

nicolas-grekas commented Aug 26, 2024 •

edited

Loading

smnandre commented Aug 26, 2024

jderusse commented Aug 27, 2024

nicolas-grekas commented Aug 27, 2024 •

edited

Loading

nicolas-grekas commented Aug 27, 2024 •

edited

Loading

wouterj commented Sep 17, 2024

wouterj left a comment

wouterj Sep 22, 2024

nicolas-grekas Sep 25, 2024

94noni Oct 7, 2024

nicolas-grekas Oct 8, 2024

coatpont Feb 26, 2025

nicolas-grekas commented Sep 25, 2024

nicolas-grekas commented Oct 3, 2024

Tobion commented Oct 3, 2024

wouterj commented Oct 7, 2024

stof commented Oct 7, 2024

nicolas-grekas commented Oct 8, 2024 •

edited

Loading

stof commented Oct 8, 2024

wouterj commented Oct 8, 2024

nicolas-grekas commented Oct 8, 2024

GromNaN Dec 24, 2024 •

edited

Loading

xabbuh Dec 24, 2024

[Security] Implement stateless headers/cookies-based CSRF protection #58095

[Security] Implement stateless headers/cookies-based CSRF protection #58095

Conversation

nicolas-grekas commented Aug 26, 2024 • edited Loading

smnandre commented Aug 26, 2024

jderusse commented Aug 27, 2024

nicolas-grekas commented Aug 27, 2024 • edited Loading

nicolas-grekas commented Aug 27, 2024 • edited Loading

wouterj commented Sep 17, 2024

wouterj left a comment

Choose a reason for hiding this comment

wouterj Sep 22, 2024

Choose a reason for hiding this comment

nicolas-grekas Sep 25, 2024

Choose a reason for hiding this comment

94noni Oct 7, 2024

Choose a reason for hiding this comment

nicolas-grekas Oct 8, 2024

Choose a reason for hiding this comment

coatpont Feb 26, 2025

Choose a reason for hiding this comment

nicolas-grekas commented Sep 25, 2024

nicolas-grekas commented Oct 3, 2024

Tobion commented Oct 3, 2024

wouterj commented Oct 7, 2024

stof commented Oct 7, 2024

nicolas-grekas commented Oct 8, 2024 • edited Loading

stof commented Oct 8, 2024

wouterj commented Oct 8, 2024

nicolas-grekas commented Oct 8, 2024

GromNaN Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

xabbuh Dec 24, 2024

Choose a reason for hiding this comment

nicolas-grekas commented Aug 26, 2024 •

edited

Loading

nicolas-grekas commented Aug 27, 2024 •

edited

Loading

nicolas-grekas commented Aug 27, 2024 •

edited

Loading

nicolas-grekas commented Oct 8, 2024 •

edited

Loading

GromNaN Dec 24, 2024 •

edited

Loading