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

Support for Application Secret rotation #12520

Open
rtyley opened this issue Apr 4, 2024 · 0 comments
Open

Support for Application Secret rotation #12520

rtyley opened this issue Apr 4, 2024 · 0 comments

Comments

@rtyley
Copy link
Contributor

rtyley commented Apr 4, 2024

Key Rotation is super-important

Key rotation is an important security practice which ideally should be performed frequently, and automated. The longer a key lives, the more likely it is to be exfiltrated, or be in the possession of employees who have since left your organisation - so if you rotate your keys once a year, how many people will have left the company in that time? If you rotate your keys more frequently, how frequently can you perform rotation before rejected session cookies or CRSF tokens become a noticeable user problem?

Key Rotation without downtime is possible!

Supporting secret rotation without downtime means that there are periods of time when tokens may acceptably be signed with either the old secret or the new secret, both of which should be accepted for verification during a reasonable overlap period (eg, 2 hours) before new tokens can have been issued to all users. Consequently, all code that consumes the secret to perform signing/verifying (eg SessionCookieBaker, CSRFTokenSigner) requires access to:

  • the current active secret (not the one that was active when the app started)
  • any other secrets which are also currently acceptable for the purpose of verifying tokens

play-secret-rotation does key rotation for Play...

guardian/play-secret-rotation is a Scala add-on for Play that supports rotating the Play Application Secret on an active cluster of servers without downtime or user-facing errors. Guardian servers rotate their Application Secret every 6 hours, without downtime.

Other Play users are interested in adopting secret-rotation too:

...but being an add-on is not easy

Overriding the necessary parts of Play as an add-on to ensure that everything correctly supports key rotation is tricky, because obviously Play's codebase is written with the assumption that the Application Secret is a single immutable value - fixing the problem is not as simple as swapping out CookieSigner, because in play.api.BuiltInComponents there's just one CookieSigner, with one secret, and the classes that use it would need to perform verification using multiple secrets.

Consequently, for play-secret-rotation and its RotatingSecretComponents mix-in trait, it's a case of trying to override everything in BuiltInComponents that might be using CookieSigner - and failing, as guardian/play-secret-rotation#445 recently revealed.

play-secret-rotation works pretty well - and I think with guardian/play-secret-rotation#446 it's now comprehensive - but it's extending Play code in a way that is difficult at this moment in time.

Could we add a basic secret-rotation API to Play?

This wouldn't have to be a full implementation of key rotation - for instance, Play shouldn't have to support any given data store: the Guardian uses AWS Parameter Store, but other users would obviously have their data store of choice.

What would be necessary:

Removing/changing the secret field in SecretConfiguration

case class SecretConfiguration(secret: String = "changeme", provider: Option[String] = None)

This field, instead of being a single immutable string, would probably now be a trait like this:

case class SecretConfiguration(secretProvider: SecretProvider, provider: Option[String] = None)

trait SecretProvider {
  def activeSecret(): String
  def alsoAccepted(): Iterable[String]
}

case class StaticSecret(activeSecret: String) extends SecretProvider {
  override val alsoAccepted: Iterable[String] = Nil
}

The StaticSecret would be the default implementation of SecretProvider, and it's what would be used if play.http.secret.key was defined - but if it's not defined, it should be possible to specify an alternative implementation depending on the individual users requirements.

This would obviously have a lot of knock-on effects in the code, and the classes that play-secret-rotation currently extends (CookieDataCodec & CSRFTokenSigner) would need to be updated to respect SecretProvider and in particular it's alsoAccepted field when performing verification. But it would also allow users to hook in whatever implementation of SecretProvider they want, and make plugging in a secret-rotation library much easier.

I'm very happy to take a shot at implementing this as a PR, if it's something that sounds like it might be accepted? Also very glad to hear whatever feedback you might have @mkurz !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant