- Notifications
You must be signed in to change notification settings - Fork928
feat: implement key rotation system#14710
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes fromall commits
Commits
Show all changes
9 commits Select commitHold shift + click to select a range
08ae172 feat: add support key rotation
sreya0f28f6c feat: implement key rotation system
sreyacd3585f Update key rotation test to check re-rotation
sreyab59e423 cleanup API
sreyaa60586d Fix parameter order in keyrotate.Open function call
sreya7d73a40 address pr comments
sreyac1825a1 Merge branch 'main' into jon/keyrotate
sreya853c0f8 Add cleanup for clock trap in key rotation test
sreyae1bfe24 final comments
sreyaFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
6 changes: 5 additions & 1 deletioncoderd/database/dbgen/dbgen.go
This file contains hidden or 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
1 change: 1 addition & 0 deletionscoderd/database/lock.go
This file contains hidden or 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
298 changes: 298 additions & 0 deletionscoderd/keyrotate/rotate.go
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,298 @@ | ||
| package keyrotate | ||
| import ( | ||
| "context" | ||
| "crypto/rand" | ||
| "database/sql" | ||
| "encoding/hex" | ||
| "time" | ||
| "golang.org/x/xerrors" | ||
| "cdr.dev/slog" | ||
| "github.com/coder/coder/v2/coderd/database" | ||
| "github.com/coder/coder/v2/coderd/database/dbtime" | ||
| "github.com/coder/quartz" | ||
| ) | ||
| const ( | ||
| WorkspaceAppsTokenDuration = time.Minute | ||
| OIDCConvertTokenDuration = time.Minute * 5 | ||
| TailnetResumeTokenDuration = time.Hour * 24 | ||
| // defaultRotationInterval is the default interval at which keys are checked for rotation. | ||
| defaultRotationInterval = time.Minute * 10 | ||
| // DefaultKeyDuration is the default duration for which a key is valid. It applies to all features. | ||
| DefaultKeyDuration = time.Hour * 24 * 30 | ||
| ) | ||
| // rotator is responsible for rotating keys in the database. | ||
| type rotator struct { | ||
| db database.Store | ||
| logger slog.Logger | ||
| clock quartz.Clock | ||
| keyDuration time.Duration | ||
| features []database.CryptoKeyFeature | ||
| } | ||
| type Option func(*rotator) | ||
| func WithClock(clock quartz.Clock) Option { | ||
| return func(r *rotator) { | ||
| r.clock = clock | ||
| } | ||
| } | ||
| func WithKeyDuration(keyDuration time.Duration) Option { | ||
| return func(r *rotator) { | ||
| r.keyDuration = keyDuration | ||
| } | ||
| } | ||
| // StartRotator starts a background process that rotates keys in the database. | ||
| // It ensures there's at least one valid key per feature prior to returning. | ||
| // Canceling the provided context will stop the background process. | ||
| func StartRotator(ctx context.Context, logger slog.Logger, db database.Store, opts ...Option) error { | ||
| kr := &rotator{ | ||
| db: db, | ||
| logger: logger, | ||
| clock: quartz.NewReal(), | ||
| keyDuration: DefaultKeyDuration, | ||
| features: database.AllCryptoKeyFeatureValues(), | ||
| } | ||
| for _, opt := range opts { | ||
| opt(kr) | ||
| } | ||
| err := kr.rotateKeys(ctx) | ||
| if err != nil { | ||
| return xerrors.Errorf("rotate keys: %w", err) | ||
| } | ||
| go kr.start(ctx) | ||
| return nil | ||
| } | ||
| // start begins the process of rotating keys. | ||
| // Canceling the context will stop the rotation process. | ||
| func (k *rotator) start(ctx context.Context) { | ||
| k.clock.TickerFunc(ctx, defaultRotationInterval, func() error { | ||
| err := k.rotateKeys(ctx) | ||
| if err != nil { | ||
| k.logger.Error(ctx, "failed to rotate keys", slog.Error(err)) | ||
| } | ||
sreya marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| return nil | ||
| }) | ||
| k.logger.Debug(ctx, "ctx canceled, stopping key rotation") | ||
| } | ||
| // rotateKeys checks for any keys needing rotation or deletion and | ||
| // may insert a new key if it detects that a valid one does | ||
| // not exist for a feature. | ||
| func (k *rotator) rotateKeys(ctx context.Context) error { | ||
| return k.db.InTx( | ||
| func(tx database.Store) error { | ||
| err := tx.AcquireLock(ctx, database.LockIDCryptoKeyRotation) | ||
| if err != nil { | ||
| return xerrors.Errorf("acquire lock: %w", err) | ||
| } | ||
| cryptokeys, err := tx.GetCryptoKeys(ctx) | ||
| if err != nil { | ||
| return xerrors.Errorf("get keys: %w", err) | ||
| } | ||
| featureKeys, err := keysByFeature(cryptokeys, k.features) | ||
| if err != nil { | ||
| return xerrors.Errorf("keys by feature: %w", err) | ||
| } | ||
| now := dbtime.Time(k.clock.Now().UTC()) | ||
| for feature, keys := range featureKeys { | ||
| // We'll use a counter to determine if we should insert a new key. We should always have at least one key for a feature. | ||
| var validKeys int | ||
| for _, key := range keys { | ||
| switch { | ||
| case shouldDeleteKey(key, now): | ||
| _, err := tx.DeleteCryptoKey(ctx, database.DeleteCryptoKeyParams{ | ||
| Feature: key.Feature, | ||
| Sequence: key.Sequence, | ||
| }) | ||
| if err != nil { | ||
| return xerrors.Errorf("delete key: %w", err) | ||
| } | ||
| k.logger.Debug(ctx, "deleted key", | ||
| slog.F("key", key.Sequence), | ||
| slog.F("feature", key.Feature), | ||
| ) | ||
| case shouldRotateKey(key, k.keyDuration, now): | ||
| _, err := k.rotateKey(ctx, tx, key, now) | ||
| if err != nil { | ||
| return xerrors.Errorf("rotate key: %w", err) | ||
| } | ||
| k.logger.Debug(ctx, "rotated key", | ||
| slog.F("key", key.Sequence), | ||
| slog.F("feature", key.Feature), | ||
| ) | ||
| validKeys++ | ||
| default: | ||
| // We only consider keys without a populated deletes_at field as valid. | ||
| // This is because under normal circumstances the deletes_at field | ||
| // is set during rotation (meaning a new key was generated) | ||
| // but it's possible if the database was manually altered to | ||
| // delete the new key we may be in a situation where there | ||
| // isn't a key to replace the one scheduled for deletion. | ||
| if !key.DeletesAt.Valid { | ||
| validKeys++ | ||
| } | ||
| } | ||
| } | ||
| if validKeys == 0 { | ||
| k.logger.Info(ctx, "no valid keys detected, inserting new key", | ||
| slog.F("feature", feature), | ||
| ) | ||
| _, err := k.insertNewKey(ctx, tx, feature, now) | ||
| if err != nil { | ||
| return xerrors.Errorf("insert new key: %w", err) | ||
| } | ||
| } | ||
| } | ||
| return nil | ||
| }, &sql.TxOptions{ | ||
| Isolation: sql.LevelRepeatableRead, | ||
| }) | ||
| } | ||
| func (k *rotator) insertNewKey(ctx context.Context, tx database.Store, feature database.CryptoKeyFeature, startsAt time.Time) (database.CryptoKey, error) { | ||
| secret, err := generateNewSecret(feature) | ||
| if err != nil { | ||
| return database.CryptoKey{}, xerrors.Errorf("generate new secret: %w", err) | ||
| } | ||
| latestKey, err := tx.GetLatestCryptoKeyByFeature(ctx, feature) | ||
| if err != nil && !xerrors.Is(err, sql.ErrNoRows) { | ||
| return database.CryptoKey{}, xerrors.Errorf("get latest key: %w", err) | ||
| } | ||
| newKey, err := tx.InsertCryptoKey(ctx, database.InsertCryptoKeyParams{ | ||
| Feature: feature, | ||
| Sequence: latestKey.Sequence + 1, | ||
| Secret: sql.NullString{ | ||
| String: secret, | ||
| Valid: true, | ||
| }, | ||
| // Set by dbcrypt if it's required. | ||
| SecretKeyID: sql.NullString{}, | ||
| StartsAt: startsAt.UTC(), | ||
| }) | ||
| if err != nil { | ||
| return database.CryptoKey{}, xerrors.Errorf("inserting new key: %w", err) | ||
| } | ||
| k.logger.Info(ctx, "inserted new key for feature", slog.F("feature", feature)) | ||
| return newKey, nil | ||
| } | ||
| func (k *rotator) rotateKey(ctx context.Context, tx database.Store, key database.CryptoKey, now time.Time) ([]database.CryptoKey, error) { | ||
| startsAt := minStartsAt(key, now, k.keyDuration) | ||
| newKey, err := k.insertNewKey(ctx, tx, key.Feature, startsAt) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("insert new key: %w", err) | ||
| } | ||
| // Set old key's deletes_at to an hour + however long the token | ||
| // for this feature is expected to be valid for. This should | ||
| // allow for sufficient time for the new key to propagate to | ||
| // dependent services (i.e. Workspace Proxies). | ||
| deletesAt := startsAt.Add(time.Hour).Add(tokenDuration(key.Feature)) | ||
| updatedKey, err := tx.UpdateCryptoKeyDeletesAt(ctx, database.UpdateCryptoKeyDeletesAtParams{ | ||
| Feature: key.Feature, | ||
| Sequence: key.Sequence, | ||
| DeletesAt: sql.NullTime{ | ||
| Time: deletesAt.UTC(), | ||
| Valid: true, | ||
| }, | ||
| }) | ||
| if err != nil { | ||
| return nil, xerrors.Errorf("update old key's deletes_at: %w", err) | ||
| } | ||
| return []database.CryptoKey{updatedKey, newKey}, nil | ||
| } | ||
| func generateNewSecret(feature database.CryptoKeyFeature) (string, error) { | ||
| switch feature { | ||
| case database.CryptoKeyFeatureWorkspaceApps: | ||
| return generateKey(96) | ||
| case database.CryptoKeyFeatureOidcConvert: | ||
| return generateKey(32) | ||
| case database.CryptoKeyFeatureTailnetResume: | ||
| return generateKey(64) | ||
| } | ||
| return "", xerrors.Errorf("unknown feature: %s", feature) | ||
| } | ||
| func generateKey(length int) (string, error) { | ||
| b := make([]byte, length) | ||
| _, err := rand.Read(b) | ||
| if err != nil { | ||
| return "", xerrors.Errorf("rand read: %w", err) | ||
| } | ||
| return hex.EncodeToString(b), nil | ||
| } | ||
| func tokenDuration(feature database.CryptoKeyFeature) time.Duration { | ||
| switch feature { | ||
| case database.CryptoKeyFeatureWorkspaceApps: | ||
| return WorkspaceAppsTokenDuration | ||
| case database.CryptoKeyFeatureOidcConvert: | ||
| return OIDCConvertTokenDuration | ||
| case database.CryptoKeyFeatureTailnetResume: | ||
| return TailnetResumeTokenDuration | ||
| default: | ||
| return 0 | ||
| } | ||
| } | ||
| func shouldDeleteKey(key database.CryptoKey, now time.Time) bool { | ||
| return key.DeletesAt.Valid && !now.Before(key.DeletesAt.Time.UTC()) | ||
| } | ||
| func shouldRotateKey(key database.CryptoKey, keyDuration time.Duration, now time.Time) bool { | ||
| // If deletes_at is set, we've already inserted a key. | ||
| if key.DeletesAt.Valid { | ||
| return false | ||
| } | ||
| expirationTime := key.ExpiresAt(keyDuration) | ||
| return !now.Add(time.Hour).UTC().Before(expirationTime) | ||
| } | ||
| func keysByFeature(keys []database.CryptoKey, features []database.CryptoKeyFeature) (map[database.CryptoKeyFeature][]database.CryptoKey, error) { | ||
| m := map[database.CryptoKeyFeature][]database.CryptoKey{} | ||
| for _, feature := range features { | ||
| m[feature] = []database.CryptoKey{} | ||
| } | ||
| for _, key := range keys { | ||
| if _, ok := m[key.Feature]; !ok { | ||
| return nil, xerrors.Errorf("unknown feature: %s", key.Feature) | ||
| } | ||
| m[key.Feature] = append(m[key.Feature], key) | ||
sreya marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
| } | ||
| return m, nil | ||
| } | ||
| // minStartsAt ensures the minimum starts_at time we use for a new | ||
| // key is no less than 3*the default rotation interval. | ||
| func minStartsAt(key database.CryptoKey, now time.Time, keyDuration time.Duration) time.Time { | ||
| expiresAt := key.ExpiresAt(keyDuration) | ||
| minStartsAt := now.Add(3 * defaultRotationInterval) | ||
| if expiresAt.Before(minStartsAt) { | ||
| return minStartsAt | ||
| } | ||
| return expiresAt | ||
| } | ||
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
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.