-
Notifications
You must be signed in to change notification settings - Fork 979
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 8 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,186 @@ | ||
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" | ||
) | ||
|
||
// 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 | ||
fetched chan struct{} | ||
} | ||
|
||
type DBCacheOption func(*DBCache) | ||
|
||
func WithDBCacheClock(clock quartz.Clock) DBCacheOption { | ||
return func(d *DBCache) { | ||
d.clock = clock | ||
} | ||
} | ||
|
||
// NewDBCache creates a new DBCache. It starts a background | ||
// process that periodically refreshes the cache. The context should | ||
// be canceled to stop the background process. | ||
func NewDBCache(ctx context.Context, logger slog.Logger, db database.Store, feature database.CryptoKeyFeature, opts ...func(*DBCache)) *DBCache { | ||
d := &DBCache{ | ||
db: db, | ||
feature: feature, | ||
clock: quartz.NewReal(), | ||
logger: logger, | ||
fetched: make(chan struct{}), | ||
} | ||
|
||
for _, opt := range opts { | ||
opt(d) | ||
} | ||
|
||
go d.clear(ctx) | ||
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) { | ||
now := d.clock.Now() | ||
d.keysMu.RLock() | ||
key, ok := d.keys[sequence] | ||
d.keysMu.RUnlock() | ||
if ok { | ||
return checkKey(key, now) | ||
} | ||
|
||
d.keysMu.Lock() | ||
defer d.keysMu.Unlock() | ||
|
||
key, ok = d.keys[sequence] | ||
if ok { | ||
return checkKey(key, now) | ||
} | ||
|
||
cache, latest, err := d.fetch(ctx) | ||
if err != nil { | ||
return codersdk.CryptoKey{}, xerrors.Errorf("fetch: %w", err) | ||
} | ||
d.keys, d.latestKey = cache, latest | ||
|
||
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() | ||
latest := d.latestKey | ||
d.keysMu.RUnlock() | ||
|
||
now := d.clock.Now() | ||
if latest.CanSign(now) { | ||
return db2sdk.CryptoKey(latest), nil | ||
} | ||
|
||
d.keysMu.Lock() | ||
defer d.keysMu.Unlock() | ||
|
||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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. | ||
cache, latest, err := d.fetch(ctx) | ||
if err != nil { | ||
return codersdk.CryptoKey{}, xerrors.Errorf("fetch: %w", err) | ||
} | ||
d.keys, d.latestKey = cache, latest | ||
|
||
return db2sdk.CryptoKey(d.latestKey), nil | ||
} | ||
|
||
func (d *DBCache) clear(ctx context.Context) { | ||
for { | ||
fired := make(chan struct{}) | ||
timer := d.clock.AfterFunc(time.Minute*10, func() { | ||
defer close(fired) | ||
|
||
// There's a small window where the timer fires as we're fetching | ||
// keys that could result in us immediately invalidating the cache that we just populated. | ||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
d.keysMu.Lock() | ||
defer d.keysMu.Unlock() | ||
d.keys = nil | ||
d.latestKey = database.CryptoKey{} | ||
}) | ||
|
||
select { | ||
case <-ctx.Done(): | ||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return | ||
case <-d.fetched: | ||
timer.Stop() | ||
case <-fired: | ||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
} | ||
|
||
// fetch fetches all keys for the given feature and determines the latest key. | ||
func (d *DBCache) fetch(ctx context.Context) (map[int32]database.CryptoKey, database.CryptoKey, error) { | ||
now := d.clock.Now() | ||
keys, err := d.db.GetCryptoKeysByFeature(ctx, d.feature) | ||
if err != nil { | ||
return nil, database.CryptoKey{}, xerrors.Errorf("get crypto keys by feature: %w", err) | ||
} | ||
|
||
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 nil, database.CryptoKey{}, ErrKeyNotFound | ||
} | ||
|
||
if !latest.CanSign(now) { | ||
return nil, database.CryptoKey{}, ErrKeyInvalid | ||
} | ||
|
||
select { | ||
case <-ctx.Done(): | ||
return nil, database.CryptoKey{}, ctx.Err() | ||
case d.fetched <- struct{}{}: | ||
} | ||
sreya marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return cache, latest, 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 | ||
} |
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.