Skip to content

Commit a50b1f0

Browse files
sreyasreya
committed
Refactor key cache management for better clarity
- Consolidates key cache handling by replacing legacy key cache references with central key caches. - Enhances modularity and maintainability by using consistent key management methods. - Removes redundant `StaticKeyManager` implementation for streamlined code. - Adjusts cryptographic key generation and cache utilization across critical components.
1 parent 5567bc7 commit a50b1f0

File tree

14 files changed

+51
-67
lines changed

14 files changed

+51
-67
lines changed

‎coderd/coderd.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,11 @@ type Options struct {
250250

251251
// OneTimePasscodeValidityPeriod specifies how long a one time passcode should be valid for.
252252
OneTimePasscodeValidityPeriod time.Duration
253+
254+
// Keycaches
255+
AppSigningKeyCache cryptokeys.SigningKeycache
256+
AppEncryptionKeyCache cryptokeys.EncryptionKeycache
257+
OIDCConvertKeyCache cryptokeys.SigningKeycache
253258
}
254259

255260
// @title Coder API
@@ -622,12 +627,6 @@ func New(options *Options) *API {
622627
api.Logger.Fatal(api.ctx, "start key rotator", slog.Error(err))
623628
}
624629

625-
api.oauthConvertKeycache, err = cryptokeys.NewSigningCache(api.Logger.Named("oauth_convert_keycache"), api.Database, database.CryptoKeyFeatureOIDCConvert)
626-
if err != nil {
627-
api.Logger.Fatal(api.ctx, "failed to initialize oauth convert key cache", slog.Error(err))
628-
}
629-
api.workspaceAppsKeyCache = appEncryptingKeyCache
630-
631630
api.statsReporter = workspacestats.NewReporter(workspacestats.ReporterOptions{
632631
Database: options.Database,
633632
Logger: options.Logger.Named("workspacestats"),
@@ -1406,12 +1405,6 @@ type API struct {
14061405
// dbRolluper rolls up template usage stats from raw agent and app
14071406
// stats. This is used to provide insights in the WebUI.
14081407
dbRolluper *dbrollup.Rolluper
1409-
1410-
// resumeTokenKeycache is used to fetch and cache keys used for signing JWTs
1411-
// oauthConvertKeycache is used to fetch and cache keys used for signing JWTs
1412-
// during OAuth conversions. See userauth.go.convertUserToOauth.
1413-
oauthConvertKeycache cryptokeys.SigningKeycache
1414-
workspaceAppsKeyCache cryptokeys.EncryptionKeycache
14151408
}
14161409

14171410
// Close waits for all WebSocket connections to drain before returning.

‎coderd/coderdtest/coderdtest.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import (
5555
"github.com/coder/coder/v2/coderd/audit"
5656
"github.com/coder/coder/v2/coderd/autobuild"
5757
"github.com/coder/coder/v2/coderd/awsidentity"
58+
"github.com/coder/coder/v2/coderd/cryptokeys"
5859
"github.com/coder/coder/v2/coderd/database"
5960
"github.com/coder/coder/v2/coderd/database/db2sdk"
6061
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -157,8 +158,7 @@ type Options struct {
157158
DatabaseRolluper *dbrollup.Rolluper
158159
WorkspaceUsageTrackerFlush chan int
159160
WorkspaceUsageTrackerTick chan time.Time
160-
161-
NotificationsEnqueuer notifications.Enqueuer
161+
NotificationsEnqueuer notifications.Enqueuer
162162
}
163163

164164
// New constructs a codersdk client connected to an in-memory API instance.
@@ -326,6 +326,13 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
326326
}
327327
auditor.Store(&options.Auditor)
328328

329+
oidcConvertKeyCache, err := cryptokeys.NewSigningCache(options.Logger.Named("oidc_convert_keycache"), options.Database, database.CryptoKeyFeatureOIDCConvert)
330+
require.NoError(t, err)
331+
appSigningKeyCache, err := cryptokeys.NewSigningCache(options.Logger.Named("app_signing_keycache"), options.Database, database.CryptoKeyFeatureWorkspaceAppsToken)
332+
require.NoError(t, err)
333+
appEncryptionKeyCache, err := cryptokeys.NewEncryptionCache(options.Logger.Named("app_encryption_keycache"), options.Database, database.CryptoKeyFeatureWorkspaceAppsAPIKey)
334+
require.NoError(t, err)
335+
329336
ctx, cancelFunc := context.WithCancel(context.Background())
330337
lifecycleExecutor := autobuild.NewExecutor(
331338
ctx,
@@ -533,6 +540,9 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can
533540
WorkspaceUsageTracker: wuTracker,
534541
NotificationsEnqueuer: options.NotificationsEnqueuer,
535542
OneTimePasscodeValidityPeriod: options.OneTimePasscodeValidityPeriod,
543+
AppSigningKeyCache: appSigningKeyCache,
544+
AppEncryptionKeyCache: appEncryptionKeyCache,
545+
OIDCConvertKeyCache: oidcConvertKeyCache,
536546
}
537547
}
538548

‎coderd/cryptokeys/dbkeycache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ func isEncryptionKeyFeature(feature database.CryptoKeyFeature) bool {
269269

270270
func isSigningKeyFeature(feature database.CryptoKeyFeature) bool {
271271
switch feature {
272-
case database.CryptoKeyFeatureTailnetResume, database.CryptoKeyFeatureOIDCConvert:
272+
case database.CryptoKeyFeatureTailnetResume, database.CryptoKeyFeatureOIDCConvert, database.CryptoKeyFeatureWorkspaceAppsToken:
273273
return true
274274
default:
275275
return false

‎coderd/cryptokeys/rotate.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,8 @@ func generateNewSecret(feature database.CryptoKeyFeature) (string, error) {
231231
switch feature {
232232
case database.CryptoKeyFeatureWorkspaceAppsAPIKey:
233233
return generateKey(32)
234+
case database.CryptoKeyFeatureWorkspaceAppsToken:
235+
return generateKey(64)
234236
case database.CryptoKeyFeatureOIDCConvert:
235237
return generateKey(64)
236238
case database.CryptoKeyFeatureTailnetResume:
@@ -252,6 +254,8 @@ func tokenDuration(feature database.CryptoKeyFeature) time.Duration {
252254
switch feature {
253255
case database.CryptoKeyFeatureWorkspaceAppsAPIKey:
254256
return WorkspaceAppsTokenDuration
257+
case database.CryptoKeyFeatureWorkspaceAppsToken:
258+
return WorkspaceAppsTokenDuration
255259
case database.CryptoKeyFeatureOIDCConvert:
256260
return OIDCConvertTokenDuration
257261
case database.CryptoKeyFeatureTailnetResume:

‎coderd/jwtutils/jwe.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -130,30 +130,3 @@ func Decrypt(ctx context.Context, d DecryptKeyProvider, token string, claims Cla
130130

131131
return claims.Validate(options.RegisteredClaims)
132132
}
133-
134-
type StaticKeyManager struct {
135-
ID string
136-
Key interface{}
137-
}
138-
139-
func (s StaticKeyManager) SigningKey(_ context.Context) (string, interface{}, error) {
140-
return s.ID, s.Key, nil
141-
}
142-
143-
func (s StaticKeyManager) VerifyingKey(_ context.Context, id string) (interface{}, error) {
144-
if id != s.ID {
145-
return nil, xerrors.Errorf("invalid id %q", id)
146-
}
147-
return s.Key, nil
148-
}
149-
150-
func (s StaticKeyManager) EncryptingKey(_ context.Context) (string, interface{}, error) {
151-
return s.ID, s.Key, nil
152-
}
153-
154-
func (s StaticKeyManager) DecryptingKey(_ context.Context, id string) (interface{}, error) {
155-
if id != s.ID {
156-
return nil, xerrors.Errorf("invalid id %q", id)
157-
}
158-
return s.Key, nil
159-
}

‎coderd/userauth.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
167167
ToLoginType: req.ToType,
168168
}
169169

170-
token, err := jwtutils.Sign(dbauthz.AsKeyRotator(ctx), api.oauthConvertKeycache, claims)
170+
token, err := jwtutils.Sign(dbauthz.AsKeyRotator(ctx), api.OIDCConvertKeyCache, claims)
171171
if err != nil {
172172
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
173173
Message: "Internal error signing state jwt.",
@@ -1676,7 +1676,7 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
16761676
}
16771677
}
16781678
var claims OAuthConvertStateClaims
1679-
err = jwtutils.Verify(dbauthz.AsKeyRotator(ctx), api.oauthConvertKeycache, jwtCookie.Value, &claims)
1679+
err = jwtutils.Verify(dbauthz.AsKeyRotator(ctx), api.OIDCConvertKeyCache, jwtCookie.Value, &claims)
16801680
if xerrors.Is(err, cryptokeys.ErrKeyNotFound) || xerrors.Is(err, cryptokeys.ErrKeyInvalid) || xerrors.Is(err, jose.ErrCryptoFailure) {
16811681
// These errors are probably because the user is mixing 2 coder deployments.
16821682
return database.User{}, idpsync.HTTPError{

‎coderd/workspaceagents_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ func TestWorkspaceAgentClientCoordinate_ResumeToken(t *testing.T) {
560560
resumeTokenSigningKey, err := tailnet.GenerateResumeTokenSigningKey()
561561
mgr := jwtutils.StaticKeyManager{
562562
ID: uuid.New().String(),
563-
Key: resumeTokenSigningKey,
563+
Key: resumeTokenSigningKey[:],
564564
}
565565
require.NoError(t, err)
566566
resumeTokenProvider := newResumeTokenRecordingProvider(

‎coderd/workspaceapps.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
123123
return
124124
}
125125

126-
encryptedAPIKey, err := jwtutils.Encrypt(ctx, api.workspaceAppsKeyCache, workspaceapps.EncryptedAPIKeyPayload{
126+
encryptedAPIKey, err := jwtutils.Encrypt(ctx, api.AppEncryptionKeyCache, workspaceapps.EncryptedAPIKeyPayload{
127127
APIKey: cookie.Value,
128128
})
129129
if err != nil {

‎coderd/workspaceapps/db.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import (
1313
"golang.org/x/exp/slices"
1414
"golang.org/x/xerrors"
1515

16+
"github.com/go-jose/go-jose/v4/jwt"
17+
1618
"cdr.dev/slog"
1719
"github.com/coder/coder/v2/coderd/database"
1820
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -22,7 +24,6 @@ import (
2224
"github.com/coder/coder/v2/coderd/rbac"
2325
"github.com/coder/coder/v2/coderd/rbac/policy"
2426
"github.com/coder/coder/v2/codersdk"
25-
"github.com/go-jose/go-jose/v4/jwt"
2627
)
2728

2829
// DBTokenProvider provides authentication and authorization for workspace apps

‎coderd/workspaceapps/db_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/coder/coder/v2/agent/agenttest"
2222
"github.com/coder/coder/v2/coderd/coderdtest"
2323
"github.com/coder/coder/v2/coderd/httpmw"
24+
"github.com/coder/coder/v2/coderd/jwtutils"
2425
"github.com/coder/coder/v2/coderd/workspaceapps"
2526
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
2627
"github.com/coder/coder/v2/codersdk"
@@ -295,7 +296,8 @@ func Test_ResolveRequest(t *testing.T) {
295296
require.Equal(t, codersdk.SignedAppTokenCookie, cookie.Name)
296297
require.Equal(t, req.BasePath, cookie.Path)
297298

298-
parsedToken, err := api.workspaceAppsKeyCache.VerifySignedToken(cookie.Value)
299+
var parsedToken workspaceapps.SignedToken
300+
err := jwtutils.Verify(ctx, api.AppSigningKeyCache, cookie.Value, &parsedToken)
299301
require.NoError(t, err)
300302
// normalize expiry
301303
require.WithinDuration(t, token.Expiry.Time(), parsedToken.Expiry.Time(), 2*time.Second)
@@ -551,7 +553,8 @@ func Test_ResolveRequest(t *testing.T) {
551553
AgentID: agentID,
552554
AppURL: appURL,
553555
}
554-
badTokenStr, err := api.AppSecurityKey.SignToken(badToken)
556+
557+
badTokenStr, err := jwtutils.Sign(ctx, api.AppSigningKeyCache, badToken)
555558
require.NoError(t, err)
556559

557560
req := (workspaceapps.Request{
@@ -594,7 +597,8 @@ func Test_ResolveRequest(t *testing.T) {
594597
require.Len(t, cookies, 1)
595598
require.Equal(t, cookies[0].Name, codersdk.SignedAppTokenCookie)
596599
require.NotEqual(t, cookies[0].Value, badTokenStr)
597-
parsedToken, err := api.AppSecurityKey.VerifySignedToken(cookies[0].Value)
600+
var parsedToken workspaceapps.SignedToken
601+
err = jwtutils.Verify(ctx, api.AppSigningKeyCache, cookies[0].Value, &parsedToken)
598602
require.NoError(t, err)
599603
require.Equal(t, appNameOwner, parsedToken.AppSlugOrPort)
600604
})

0 commit comments

Comments
 (0)