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 accepting multiple public keys #150

Conversation

rtyley
Copy link
Member

@rtyley rtyley commented Jul 30, 2024

This PR introduces a way for Panda keys to be manually rotated without disruption to users, and is a stepping stone towards the later goal of automatically rotating keys on a regular schedule. Consequently, some of the work here is temporary (like the CryptoConfForRotation script) and is intended for later replacement.

Updated requirements

Necessary changes to code using Panda

Code shouldn't directly reference private or public keys anymore (eg do not reference settings.signingKeyPair). Instead, use settings.signingAndVerification or publicSettings.verification. Note also that publicSettings.publicKey was previously optional, and publicSettings.verification is not.

Changes to .settings & .settings.public config files

Panda's settings for signing & verification keys have been extended, while keeping the config files backwards-compatible with old clients. The existing configuration keys remain unchanged, and can still only be used at most once in a config file:

  • privateKey
  • publicKey

...semantically, those settings now relate specifically to the active key pair (ie the one that is currently doing cookie-signing). In addition, there is now a pattern for settings that list additional public keys which will also be accepted:

  • alsoAccept.xxx.publicKey - where xxx can be any string that's valid in a property key identifier, allowing multiple entries like this to be in a config file

Generating a new key pair

The script used to generate a new cryptographic key pair has been replaced:

  • OLD: generateKeyPair.sh bash script, using openssl to generate the new key pair.
  • NEW : CryptoConfForRotation Scala code, executable from the sbt prompt, using java.security.KeyPairGenerator to generate a new key pair. The new script also generates the 3 phases of config required for a smooth key rotation.

Performing a smooth key rotation

To perform a smooth key rotation, without disruption to users, there needs to be 3 separate updates to each config file in S3, each with a delay between each step. The 4 phases can be seen in the example .settings files in src/test/resources/crypto-conf-rotation-example:

  1. legacy.settings - the settings before a rotation, with just a single active key-pair, defined by privateKey & publicKey
  2. rotation-upcoming.settings - the same settings, but now also with a alsoAccept.0.publicKey entry that specifies the public key of the upcoming active key-pair. Once this update has been applied to S3, there should be a delay of at least 1 minute before the next phase, to ensure that all Panda services are ready to accept a cookie signed with the upcoming active key-pair.
  3. rotation-in-progress.settings - the active key-pair (privateKey & publicKey) is now the new key-pair, and alsoAccept.0.publicKey is now set to the public key of the old active key-pair. Once this update has been applied, there should be a delay of at least 1 hour, to ensure all users have regenerated their cookies.
  4. rotation-complete.settings - finally, we remove alsoAccept.0.publicKey, and only the new active key-pair is accepted.

Instructions in the ReadMe.md have been updated to reflect this.

Testing

Supporting PRs

This PR sits on top of:

@rtyley rtyley force-pushed the support-accepting-multiple-public-keys branch 20 times, most recently from 89496e1 to 0e72f45 Compare August 7, 2024 16:12
@rtyley rtyley changed the base branch from main to update-settings-loading-and-parsing-code August 7, 2024 16:13
@rtyley rtyley force-pushed the update-settings-loading-and-parsing-code branch from b9844db to 65ca26c Compare August 7, 2024 16:20
@rtyley rtyley force-pushed the support-accepting-multiple-public-keys branch from 0e72f45 to 7c82eab Compare August 7, 2024 16:21
@rtyley rtyley force-pushed the support-accepting-multiple-public-keys branch 3 times, most recently from bff542b to 753274d Compare August 7, 2024 16:48
@rtyley rtyley changed the base branch from update-settings-loading-and-parsing-code to update-cookie-generation-and-parsing-code August 7, 2024 16:49
@rtyley rtyley force-pushed the support-accepting-multiple-public-keys branch 2 times, most recently from 086257c to 19d5a45 Compare August 7, 2024 16:53
rtyley added a commit to guardian/tagmanager that referenced this pull request Sep 18, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/grid that referenced this pull request Sep 18, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/grid that referenced this pull request Sep 18, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/grid that referenced this pull request Sep 18, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/grid that referenced this pull request Sep 18, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/frontend that referenced this pull request Sep 19, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as introduced with
guardian/pan-domain-authentication#150.

See also guardian/pan-domain-authentication#160.
rtyley added a commit to guardian/atom-workshop that referenced this pull request Sep 19, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.

As Atom Workshop is pretty standard user of Panda, the upgrade is pretty simple:

* Panda v6:
  * guardian/pan-domain-authentication#155 requires
    `panDomainSettings` is a `val`, not a `def`
rtyley added a commit to guardian/login.gutools that referenced this pull request Sep 19, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.

As login.gutools.co.uk is pretty special user of Panda the upgrade is slightly
more involved than other upgrades (eg guardian/atom-workshop#361):

* Panda v6:
  * guardian/pan-domain-authentication#152
    `CookieUtils.generateCookieData()` now communicates errors with
    `CookieResult` values containing `CookieIntegrityFailure`, rather than
    exceptions.
* Panda v7:
  * guardian/pan-domain-authentication#150 means
    that code shouldn't directly reference private or public keys anymore
    (eg do not reference `settings.signingKeyPair`). Instead, use
    `settings.signingAndVerification` or `publicSettings.verification`.
    Note also that `publicSettings.publicKey` was previously optional, and
    `publicSettings.verification` is not.
rtyley added a commit to guardian/tagmanager that referenced this pull request Sep 19, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/frontend that referenced this pull request Sep 19, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as introduced with
guardian/pan-domain-authentication#150.

See also guardian/pan-domain-authentication#160.
rtyley added a commit to guardian/frontend that referenced this pull request Sep 19, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as introduced with
guardian/pan-domain-authentication#150.

See also guardian/pan-domain-authentication#160.
rtyley added a commit to guardian/grid that referenced this pull request Sep 20, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/grid that referenced this pull request Sep 24, 2024
This upgrades Panda from v5 to v7, allowing us to use key rotation as
introduced with guardian/pan-domain-authentication#150.
rtyley added a commit to guardian/giant that referenced this pull request Nov 8, 2024
This upgrades Panda from v3 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.

### Necessary code changes

* Panda v5
  * guardian/pan-domain-authentication#147 removed the old `PublicKey` & `PrivateKey` classes in our `com.gu.pandomainauth` package, in favour of using the existing `java.security` classes. To create instances of those classes, we can use the `SettingsReader.{privateKeyFor, publicKeyFor}` methods.
* Panda v6:
  * guardian/pan-domain-authentication#152 means the `CookieUtils.generateCookieData()` method now communicates errors with `CookieResult` values containing `CookieIntegrityFailure`, rather than exceptions.
* Panda v7:
  * guardian/pan-domain-authentication#150 means that code shouldn't directly reference private or public keys anymore (eg do not reference `settings.signingKeyPair`). Instead, use `settings.signingAndVerification` or `publicSettings.verification`. Note also that `publicSettings.publicKey` was previously optional, and `publicSettings.verification` is not.
rtyley added a commit to guardian/giant that referenced this pull request Nov 8, 2024
This upgrades Panda from v3 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.

### Necessary code changes

* Panda v5
  * guardian/pan-domain-authentication#147 removed the old `PublicKey` & `PrivateKey` classes in our `com.gu.pandomainauth` package, in favour of using the existing `java.security` classes. To create instances of those classes, we can use the `SettingsReader.{privateKeyFor, publicKeyFor}` methods.
* Panda v6:
  * guardian/pan-domain-authentication#152 means the `CookieUtils.generateCookieData()` method now communicates errors with `CookieResult` values containing `CookieIntegrityFailure`, rather than exceptions.
* Panda v7:
  * guardian/pan-domain-authentication#150 means that code shouldn't directly reference private or public keys anymore (eg do not reference `settings.signingKeyPair`). Instead, use `settings.signingAndVerification` or `publicSettings.verification`. Note also that `publicSettings.publicKey` was previously optional, and `publicSettings.verification` is not.
davidfurey added a commit to guardian/editorial-viewer that referenced this pull request Jan 8, 2025
This upgrades Panda from v4 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.
davidfurey added a commit to guardian/facia-tool that referenced this pull request Jan 8, 2025
This upgrades Panda from v4 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.
davidfurey added a commit to guardian/story-packages that referenced this pull request Jan 9, 2025
This upgrades Panda from v4 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.
davidfurey added a commit to guardian/facia-tool that referenced this pull request Jan 17, 2025
This upgrades Panda from v4 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.
davidfurey added a commit to guardian/facia-tool that referenced this pull request Jan 21, 2025
This upgrades Panda from v4 to v7, allowing us to use key rotation as introduced with guardian/pan-domain-authentication#150.
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

Successfully merging this pull request may close these issues.

2 participants