-
Notifications
You must be signed in to change notification settings - Fork 937
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
OIDC: Cluster-wide key and salt for cookie encryption #15011
Draft
markylaing
wants to merge
14
commits into
canonical:main
Choose a base branch
from
markylaing:rp-cookie-rotation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6539d00
to
33330a8
Compare
Signed-off-by: Mark Laing <[email protected]>
Signed-off-by: Mark Laing <[email protected]>
Signed-off-by: Mark Laing <[email protected]>
Adds utils for managing a cluster wide secret and salt. Signed-off-by: Mark Laing <[email protected]>
Signed-off-by: Mark Laing <[email protected]>
Signed-off-by: Mark Laing <[email protected]>
This function, in combination with `clusterSecretInternal`, can be used to get a cluster-wide shared secret on demand. Signed-off-by: Mark Laing <[email protected]>
We need to unset any previously configured value so that the joining daemon will fetch the shared secret on the next call to `(*Daemon).getClusterSecret`. Signed-off-by: Mark Laing <[email protected]>
To do this, we need to delete the database entry first, then reset the `(*Daemon).clusterSecretInternal` so that new ones are generated when required. We only delete the database entries once, from the member that received the request. Signed-off-by: Mark Laing <[email protected]>
Signed-off-by: Mark Laing <[email protected]>
This change allows the verifier to perform OIDC discovery when it is created (on config change). Users will get faster feedback if their OIDC configuration is incorrect. Signed-off-by: Mark Laing <[email protected]>
Signed-off-by: Mark Laing <[email protected]>
This PR changes the `relyingParty` field to a slice of relying parties associated with the time at which they became outdated. In /oidc/callback, we use any available relying party that can decrypt the state cookie (and therefore complete the flow). Relying parties are only kept around for 5 minutes. Signed-off-by: Mark Laing <[email protected]>
…covery. Signed-off-by: Mark Laing <[email protected]>
33330a8
to
5d72a2b
Compare
Draft
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
API
Changes to the REST API
Bug
Confirmed to be a bug
Documentation
Documentation needs updating
Improvement
Improve to current situation
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When performing the authorization code flow two cookies are encrypted and stored on the client. After authentication, when the client is redirected to LXDs
/oidc/callback
endpoint, the receiving LXD cluster member must be able to decrypt these cookies to complete the PKCE part of the authentication flow.It is possible that, if LXD is deployed behind a load-balancer or DNS SRV records are used, the redirection to
/oidc/callback
will reach a different cluster member to the one that started the login flow. Currently, the encryption keys used for these cookies are cluster member specific. If the call to/oidc/login
starts on one cluster member, but the call to/oidc/callback
reaches another cluster member, then decryption of these cookies fails and the authentication flow must be restarted.This is documented in #13644
This PR adds the concept of a cluster wide secret key and salt. Both the key and the salt are stored in the
config
table but are hidden from the server configuration.The key and salt have the following formats:
(crypto/rand).Reader
as the source of entropy (rather than the default monotonic reader).(crypto/rand).Reader
. In the database, this is base64 encoded and prepended with the expiry time in unix milliseconds (UTC) (delimited by a.
, which base64 cannot contain).Note that the key is not a private TLS key. It's only use is as a source for HKDF derivation of hash and block keys for cookie encryption.
Whenever the salt or key are used, the caller should check if they are still valid. If they are not valid, the caller should check if another cluster member has created a valid key/salt and stored it in the database. If the key/salt in the database is not valid, the caller should set a new one. The
lxd/db/secret
package has been added to handle this.Two new configuration keys are added to manage the lifetime of the secret key (
core.secret_key_lifetime
default 30 (days)) and salt (core.salt_lifetime
default 60 (minutes)). When the key or salt lifetime is updated, the existing key and salt are invalidated. Additionally, when a daemon joins a cluster, any secrets it is holding in memory are invalidated.To preserve the authorization code flow in case of salt or key rotation occurring while a user is logging in, the OIDC verifier keeps old relying party configuration available in memory for 5 minutes. The verifier determines which RP to use to complete the flow by checking that it is able to decode the
state
cookie (which is set as part of the flow).This PR removes dependence on the cluster private key for derivation of unique cookie encryption keys for OIDC ID and access tokens. Instead we use the cluster-wide secret key, plus the session ID, to derive unique keys per user session.
Lastly, options for instantiating a new OIDC verifier have been expanded to pass in a "host" and a context. This means that the verifier will attempt to perform discovery when the configuration is set.
Closes #13644.
Marking as draft because this is really a collection of ideas and there is no spec. If we like the approach and decide to merge, beware that this will log out all OIDC users (though if they're still logged in at the IdP, the be re-logged into LXD after a brief redirect).