-
Notifications
You must be signed in to change notification settings - Fork 937
feat: add cache abstraction for fetching signing keys #14777
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
Merged
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
c6fdfe6
feat: add keychain abstraction for fetching rotated keys
sreya f9bff68
Refactor DBKeyCache to DBCache with clock support
sreya 46503b6
Refactor CryptoKey handling to use codersdk package
sreya c63020a
Refactor CryptoKey conversion to use db2sdk package
sreya 3e723ce
make gen
sreya 201347f
Refactor key verification with db2sdk conversion
sreya 5f9744b
pr comments
sreya 5803092
Refactor error message for key fetching
sreya ea659b0
Refactor DBKeyCache with robust invalidation timing
sreya fe0b4e4
Add leak detection to dbkeycache tests
sreya 430e1c2
Add mutex lock to DBCache Close method
sreya 4dfdd4f
Add closed state to DBCache for safe shutdown
sreya 5078c2a
Optimize timer reset in fetch method
sreya 280d521
Refactor DBCache to remove unused timer function
sreya 05dec8a
finale
sreya File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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,210 @@ | ||
package cryptokeys | ||
|
||
import ( | ||
"context" | ||
"sync" | ||
"time" | ||
|
||
"golang.org/x/xerrors" | ||
|
||
"cdr.dev/slog" | ||
"github.com/coder/coder/v2/coderd/database" | ||
"github.com/coder/coder/v2/coderd/database/db2sdk" | ||
"github.com/coder/coder/v2/codersdk" | ||
"github.com/coder/quartz" | ||
) | ||
|
||
// never represents the maximum value for a time.Duration. | ||
const never = 1<<63 - 1 | ||
|
||
// DBCache implements Keycache for callers with access to the database. | ||
type DBCache struct { | ||
db database.Store | ||
feature database.CryptoKeyFeature | ||
logger slog.Logger | ||
clock quartz.Clock | ||
|
||
// The following are initialized by NewDBCache. | ||
keysMu sync.RWMutex | ||
keys map[int32]database.CryptoKey | ||
latestKey database.CryptoKey | ||
timer *quartz.Timer | ||
// invalidateAt is the time at which the keys cache should be invalidated. | ||
invalidateAt time.Time | ||
closed bool | ||
} | ||
|
||
type DBCacheOption func(*DBCache) | ||
|
||
func WithDBCacheClock(clock quartz.Clock) DBCacheOption { | ||
return func(d *DBCache) { | ||
d.clock = clock | ||
} | ||
} | ||
|
||
// NewDBCache creates a new DBCache. Close should be called to | ||
// release resources associated with its internal timer. | ||
func NewDBCache(logger slog.Logger, db database.Store, feature database.CryptoKeyFeature, opts ...func(*DBCache)) *DBCache { | ||
d := &DBCache{ | ||
db: db, | ||
feature: feature, | ||
clock: quartz.NewReal(), | ||
logger: logger, | ||
} | ||
|
||
for _, opt := range opts { | ||
opt(d) | ||
} | ||
|
||
d.timer = d.clock.AfterFunc(never, d.clear) | ||
|
||
return d | ||
} | ||
|
||
// Verifying returns the CryptoKey with the given sequence number, provided that | ||
// it is neither deleted nor has breached its deletion date. It should only be | ||
// used for verifying or decrypting payloads. To sign/encrypt call Signing. | ||
func (d *DBCache) Verifying(ctx context.Context, sequence int32) (codersdk.CryptoKey, error) { | ||
d.keysMu.RLock() | ||
if d.closed { | ||
d.keysMu.RUnlock() | ||
return codersdk.CryptoKey{}, ErrClosed | ||
} | ||
|
||
now := d.clock.Now() | ||
key, ok := d.keys[sequence] | ||
d.keysMu.RUnlock() | ||
if ok { | ||
return checkKey(key, now) | ||
} | ||
|
||
d.keysMu.Lock() | ||
defer d.keysMu.Unlock() | ||
|
||
if d.closed { | ||
return codersdk.CryptoKey{}, ErrClosed | ||
} | ||
|
||
key, ok = d.keys[sequence] | ||
if ok { | ||
return checkKey(key, now) | ||
} | ||
|
||
err := d.fetch(ctx) | ||
if err != nil { | ||
return codersdk.CryptoKey{}, xerrors.Errorf("fetch: %w", err) | ||
} | ||
|
||
key, ok = d.keys[sequence] | ||
if !ok { | ||
return codersdk.CryptoKey{}, ErrKeyNotFound | ||
} | ||
|
||
return checkKey(key, now) | ||
} | ||
|
||
// Signing returns the latest valid key for signing. A valid key is one that is | ||
// both past its start time and before its deletion time. | ||
func (d *DBCache) Signing(ctx context.Context) (codersdk.CryptoKey, error) { | ||
d.keysMu.RLock() | ||
|
||
if d.closed { | ||
d.keysMu.RUnlock() | ||
return codersdk.CryptoKey{}, ErrClosed | ||
} | ||
|
||
latest := d.latestKey | ||
d.keysMu.RUnlock() | ||
|
||
now := d.clock.Now() | ||
if latest.CanSign(now) { | ||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return db2sdk.CryptoKey(latest), nil | ||
} | ||
|
||
d.keysMu.Lock() | ||
defer d.keysMu.Unlock() | ||
|
||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if d.closed { | ||
return codersdk.CryptoKey{}, ErrClosed | ||
} | ||
|
||
if d.latestKey.CanSign(now) { | ||
return db2sdk.CryptoKey(d.latestKey), nil | ||
} | ||
|
||
// Refetch all keys for this feature so we can find the latest valid key. | ||
err := d.fetch(ctx) | ||
if err != nil { | ||
return codersdk.CryptoKey{}, xerrors.Errorf("fetch: %w", err) | ||
} | ||
|
||
return db2sdk.CryptoKey(d.latestKey), nil | ||
} | ||
|
||
// clear invalidates the cache. This forces the subsequent call to fetch fresh keys. | ||
func (d *DBCache) clear() { | ||
now := d.clock.Now("DBCache", "clear") | ||
d.keysMu.Lock() | ||
defer d.keysMu.Unlock() | ||
// Check if we raced with a fetch. It's possible that the timer fired and we | ||
// lost the race to the mutex. We want to avoid invalidating | ||
// a cache that was just refetched. | ||
if now.Before(d.invalidateAt) { | ||
return | ||
} | ||
d.keys = nil | ||
d.latestKey = database.CryptoKey{} | ||
} | ||
|
||
// fetch fetches all keys for the given feature and determines the latest key. | ||
// It must be called while holding the keysMu lock. | ||
func (d *DBCache) fetch(ctx context.Context) error { | ||
keys, err := d.db.GetCryptoKeysByFeature(ctx, d.feature) | ||
if err != nil { | ||
return xerrors.Errorf("get crypto keys by feature: %w", err) | ||
} | ||
|
||
now := d.clock.Now() | ||
_ = d.timer.Reset(time.Minute * 10) | ||
d.invalidateAt = now.Add(time.Minute * 10) | ||
|
||
cache := make(map[int32]database.CryptoKey) | ||
var latest database.CryptoKey | ||
for _, key := range keys { | ||
cache[key.Sequence] = key | ||
if key.CanSign(now) && key.Sequence > latest.Sequence { | ||
latest = key | ||
} | ||
} | ||
|
||
if len(cache) == 0 { | ||
return ErrKeyNotFound | ||
} | ||
|
||
if !latest.CanSign(now) { | ||
return ErrKeyInvalid | ||
} | ||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
d.keys, d.latestKey = cache, latest | ||
return nil | ||
} | ||
|
||
func checkKey(key database.CryptoKey, now time.Time) (codersdk.CryptoKey, error) { | ||
if !key.CanVerify(now) { | ||
return codersdk.CryptoKey{}, ErrKeyInvalid | ||
} | ||
|
||
return db2sdk.CryptoKey(key), nil | ||
} | ||
|
||
func (d *DBCache) Close() { | ||
d.keysMu.Lock() | ||
defer d.keysMu.Unlock() | ||
|
||
if d.closed { | ||
return | ||
} | ||
|
||
d.timer.Stop() | ||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
d.closed = true | ||
} |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.