Skip to content

Commit 0323f79

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 7fe88ea commit 0323f79

File tree

13 files changed

+50
-66
lines changed

13 files changed

+50
-66
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/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
})

‎coderd/workspaceapps/token_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@ import (
77
"testing"
88
"time"
99

10+
"github.com/go-jose/go-jose/v4/jwt"
11+
1012
"github.com/coder/coder/v2/codersdk"
1113
"github.com/coder/coder/v2/testutil"
12-
"github.com/go-jose/go-jose/v4/jwt"
1314

1415
"github.com/google/uuid"
1516
"github.com/stretchr/testify/require"

‎codersdk/workspacesdk/connector_internal_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ func TestTailnetAPIConnector_ResumeToken(t *testing.T) {
168168
require.NoError(t, err)
169169
mgr := jwtutils.StaticKeyManager{
170170
ID: uuid.New().String(),
171-
Key: resumeTokenSigningKey,
171+
Key: resumeTokenSigningKey[:],
172172
}
173173
resumeTokenProvider := tailnet.NewResumeTokenKeyProvider(mgr, clock, time.Hour)
174174
svc, err := tailnet.NewClientService(tailnet.ClientServiceOptions{
@@ -287,7 +287,7 @@ func TestTailnetAPIConnector_ResumeTokenFailure(t *testing.T) {
287287
require.NoError(t, err)
288288
mgr := jwtutils.StaticKeyManager{
289289
ID: uuid.New().String(),
290-
Key: resumeTokenSigningKey,
290+
Key: resumeTokenSigningKey[:],
291291
}
292292
resumeTokenProvider := tailnet.NewResumeTokenKeyProvider(mgr, clock, time.Hour)
293293
svc, err := tailnet.NewClientService(tailnet.ClientServiceOptions{

‎enterprise/wsproxy/wsproxy.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"github.com/coder/coder/v2/buildinfo"
3232
"github.com/coder/coder/v2/cli/cliutil"
3333
"github.com/coder/coder/v2/coderd"
34+
"github.com/coder/coder/v2/coderd/cryptokeys"
3435
"github.com/coder/coder/v2/coderd/httpapi"
3536
"github.com/coder/coder/v2/coderd/httpmw"
3637
"github.com/coder/coder/v2/coderd/tracing"
@@ -92,7 +93,9 @@ type Options struct {
9293
// from the dashboardURL. This should only be used in development.
9394
AllowAllCors bool
9495

95-
StatsCollectorOptions workspaceapps.StatsCollectorOptions
96+
StatsCollectorOptions workspaceapps.StatsCollectorOptions
97+
WorkspaceAppsEncryptionKeycache cryptokeys.EncryptionKeycache
98+
WorkspaceAppsSigningKeycache cryptokeys.SigningKeycache
9699
}
97100

98101
func (o *Options) Validate() error {
@@ -240,11 +243,6 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
240243
return nil, xerrors.Errorf("handle register: %w", err)
241244
}
242245

243-
secKey, err := workspaceapps.KeyFromString(regResp.AppSecurityKey)
244-
if err != nil {
245-
return nil, xerrors.Errorf("parse app security key: %w", err)
246-
}
247-
248246
agentProvider, err := coderd.NewServerTailnet(ctx,
249247
s.Logger,
250248
nil,
@@ -277,14 +275,14 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
277275
HostnameRegex: opts.AppHostnameRegex,
278276
RealIPConfig: opts.RealIPConfig,
279277
SignedTokenProvider: &TokenProvider{
280-
DashboardURL: opts.DashboardURL,
281-
AccessURL: opts.AccessURL,
282-
AppHostname: opts.AppHostname,
283-
Client: client,
284-
SecurityKey: secKey,
285-
Logger: s.Logger.Named("proxy_token_provider"),
278+
DashboardURL: opts.DashboardURL,
279+
AccessURL: opts.AccessURL,
280+
AppHostname: opts.AppHostname,
281+
Client: client,
282+
SigningKey: opts.WorkspaceAppsSigningKeycache,
283+
EncryptingKey: opts.WorkspaceAppsEncryptionKeycache,
284+
Logger: s.Logger.Named("proxy_token_provider"),
286285
},
287-
AppSecurityKey: secKey,
288286

289287
DisablePathApps: opts.DisablePathApps,
290288
SecureAuthCookie: opts.SecureAuthCookie,

‎tailnet/resume_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,6 @@ func TestResumeTokenKeyProvider(t *testing.T) {
188188
func newKeySigner(key tailnet.ResumeTokenSigningKey) jwtutils.SigningKeyManager {
189189
return jwtutils.StaticKeyManager{
190190
ID: uuid.New().String(),
191-
Key: key,
191+
Key: key[:],
192192
}
193193
}

0 commit comments

Comments
 (0)