IdentityServer4: combo of response_mode=form_post and state protection not working

[wjkawecki]

I am using latest 3.14.0 release and have trouble setting up following provider:

Providers.IdentityServer4({
      id: 'identity-server4',
      name: 'IdentityServer4',
      scope: 'openid profile email offline_access',
      domain: XXX,
      clientId: XXX,
      clientSecret: XXX,
      authorizationParams: {
        prompt: 'login',
        response_mode: 'form_post'
      }
    })

When setting response_mode to 'form_post' (which enforces the IdP to send the data back via POST instead of GET https://identityserver4.readthedocs.io/en/latest/endpoints/authorize.html) I run into several issues. GET method works fine, but pen-test team insists to use POST.

First is the fact, that state-handler accepts only state passed in query https://github.com/nextauthjs/next-auth/blob/main/src/server/lib/oauth/state-handler.js#L19

Whenever I change it to:

const state = req.query.state || req.body.state;

I am able to get a step further, to next issue. It appears that the existing and expected state calculation is somehow wrong:

[next-auth][debug][oauth_callback_protection] Comparing received and expected state {
  state: 'c064b8b9ff3df38f8b3a8ce260cc5e9fc1c7dfcfe855c19da38863e207b63a55',
  expectedState: '226418f7beb8a5a755f6fb0b42dfad47c75b84130b6bbeec0d3474af6c03f5f0'
}

Does anyone have a similar setup and experience similar issues?

[balazsorban44]

Thanks for the information, I'll have a look at this! At a first glance, it seems to be an easy fix.

[balazsorban44]

It wasn't an easy fix after all... I think I know what causes the problem, but I'll have to try to find a way to solve it.

Basically when response_mode=form_post is set, the response method will also be POST. this seems to ignore the cookies, and thus the csrfToken cookie will be undefined and the mismatch of states will happen.

[wjkawecki]

Damn, if cookies are ignored, then protection: 'pkce' and ['pkce', 'state'] are probably affected as well. Thanks for looking into it.

[balazsorban44]

Good news @wjkawecki! After some thorough digging, I managed to get this working locally! (I used Auth0, but I am confident that it will apply to IDS4.)

The required changes were the following:

1. Your suggestion const state = req.query.state || req.body.state will indeed be necessary, I am going to address this in a PR
2. In addition, some advanced cookie configuration will be needed:

export default NextAuth({
  ...
  cookies: {
    csrfToken: {
      name: 'next-auth.csrf-token',
      options: {
        httpOnly: true,
        sameSite: 'none',
        path: '/',
        secure: true
      }
    },
    pkceCodeVerifier: {
      name: 'next-auth.pkce.code_verifier',
      options: {
        httpOnly: true,
        sameSite: 'none',
        path: '/',
        secure: true
      }
    }
  },
...
})

Refer to the documentation: https://next-auth.js.org/configuration/options#cookies

So there is some interesting stuff going on, so let me try to explain.

We override two cookies here, csrf-token and code_verifier respectively. They are used for when the protection: ["state", "pkce"] option is set on a provider. csrf-token corresponds to state and code_verifier to PKCE.

My research shows that state has been the way of protecting against CSRF attacks for a long time (ref: https://danielfett.de/2020/05/16/pkce-vs-nonce-equivalent-or-not/)

But OAuth and OIDC have introduced two new ways, namely PKCE and Nonce. Currently, we only support PKCE, although once we switch to openid-client (#1048), nonce implementation would also be possible/much easier.

So even though (I THINK(!)) PKCE is more than enough protection in most cases, you can certainly use both "state" and "pkce" protection together for now.

The cookie overrides are needed because when you set response_mode=form_post, the IdP will answer with a POST request, which won't include the cookies if sameSite is set to anything else than none (ref: https://auth0.com/docs/sessions/cookies/samesite-cookie-attribute-changes#browser-cookie-changes, https://docs.feide.no/general/faq/samesite_cookie.html#changes-that-need-to-be-made-on-the-service-provider-side)

For increased security, I also set secure: true, which will require the requesting party to use HTTPS. (I cannot find to find my resource again, - I'll link if I find it again -, but I think in some cases it is straight REQUIRED when sameSite: "none" is set.)

With the above changes/settings, I have been able to successfully log in even when using response_mode=form_post.

⚠ NOTE:
Although I may have to verify with @iaincollins that the cookie overrides are proper recommendations in this situation. (But I don't see any alternatives at the moment 🤷)

Hope this helps! (I certainly learned something.)

[wjkawecki]

Thank you @balazsorban44 for investigating this topic! I can confirm that custom cookie setup with sameSite: 'none' did the trick, I was now able to successfully log in, also when using protection: ['state', 'pkce']! 🎉

Please let me know if I can help somehow with reviewing the state = query || body change.

[balazsorban44]

@wjkawecki I merged #1669. I would be glad if your penetration test team had some feedback and gave their opinion on setting the CSRF Token and PKCE code verifier to sameSite: "none" and secure: true and its implications! I wonder if we have to do other modifications before we can suggest its use.

This for example could be an issue:
https://www.adexchanger.com/privacy/chrome-is-killing-cookies-but-samesite-still-needs-to-be-updated

With that in mind, it’s inevitable that Google will eventually block any cookie with the “SameSite=none” setting

If SameSite=none goes away, my suggestion won't work anymore, so we might have to look for alternatives.

[wjkawecki]

Thank you @balazsorban44, I've bumped the package version to latest. Pentest team is planning a retest round later this week - I'll get back to you once I have some feedback from their end.

[wjkawecki]

@balazsorban44 another round of penetration tests is through and the feedback is positive 🥳 High five!

[balazsorban44]

That is good to hear! I'm going to close this issue then, but keep in mind that this will probably start behaving strangely when SameSite=none won't be available anymore. 🙄