Fix CSRFTokenSigner - it needs to cope with rotated secrets too
#446
Conversation
rtyley
changed the title
CSRFTokenSignerCSRFTokenSigner - it needs to cope with rotated secrets too
rtyley
force-pushed
the
fix-forgotten-CSRFTokenSigner
branch
6 times, most recently
from
516d15c
to
f4d0c11
Compare
| override lazy val requestFactory: RequestFactory = | ||
| RotatingSecretComponents.requestFactoryFor(secretStateSupplier, httpConfiguration) | ||
|
|
||
| override lazy val csrfTokenSigner: CSRFTokenSigner = | ||
| new RotatingSecretComponents.RotatingKeyCSRFTokenSigner(secretStateSupplier, systemUTC()) |
There was a problem hiding this comment.
This the essential bit of this fix - although RotatingSecretComponents has always overridden the RequestFactory to handle rotating the Play Application Secret, we forgot to override the CSRFTokenSigner to teach it about rotating secrets.
RequestFactory- includes components that handle signing of Session & Flash cookiesCSRFTokenSigner- signs the CSRF token used for CSRF checking
...as far as I can tell, there aren't any other uses of the Play Application Secret for signing/verification that I've forgotten about.
rtyley
force-pushed
the
fix-forgotten-CSRFTokenSigner
branch
from
f4d0c11
to
d57673c
Compare
This fixes #445.
rtyley
force-pushed
the
fix-forgotten-CSRFTokenSigner
branch
from
d57673c
to
6be2eca
Compare
|
@rtyley has published a preview version of this PR with release workflow run #32, based on commit 6be2eca: 8.2.0-PREVIEW.fix-forgotten-CSRFTokenSigner.2024-04-04T1033.6be2ecaa Want to make another preview release?Click 'Run workflow' in the GitHub UI, specifying the fix-forgotten-CSRFTokenSigner branch, or use the GitHub CLI command: gh workflow run release.yml --ref fix-forgotten-CSRFTokenSigner Want to make a full release after this PR is merged?Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command: gh workflow run release.yml |
| @@ -59,4 +66,39 @@ object RotatingSecretComponents { | |||
| val snapshotProvider: SnapshotProvider) extends FlashCookieBaker with RotatingSecretCookieCodec { | |||
| override val jwtConfiguration: JWTConfiguration = config.jwt | |||
| } | |||
|
|
|||
| class RotatingKeyCSRFTokenSigner( | |||
There was a problem hiding this comment.
This class is the alternative implementation to Play's DefaultCSRFTokenSigner - our alternative implementation can cope with rotating keys.
Our implementation borrows some code as copy'n'paste from DefaultCSRFTokenSigner where it's unavoidable - specifically compareSignedTokens() - but delegates to underlying instances of DefaultCSRFTokenSigner as much as possible. If you look at the implementation of DefaultCSRFTokenSigner you can see it's doing some relatively obscure crypto things that we'd prefer delegate, rather than chase & re-implement ourselves.
This fixes #445 - although
play-secret-rotationhas always overridden Play'sRequestFactoryto handle rotating the Play Application Secret, we forgot to also override theCSRFTokenSignerto teach it about rotating secrets - this PR fixes that.Essential to coping with rotating secret keys is the ability to tolerate tokens signed with old secret keys for a transition period (for us the overlap period is 2 hours). So our
RotatingKeyCSRFTokenSignerneeds to always sign with the 'current' secret, but when verifying tokens, needs to try verifying the token against all applicable secret keys, to see if any of them validate it.See also: