Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7557ed2

Browse files
committed
Refactor key management and enhance logging
- Improve clarity by naming loggers used in key cache creation.- Adjust key cache context to utilize KeyReader for consistent context handling.- Refactor API to use central key cache management approach.- Enhance error messages for crypto key fetching.- Update test to add safety against unexpected panics.
1 parentb770762 commit7557ed2

File tree

9 files changed

+75
-61
lines changed

9 files changed

+75
-61
lines changed

‎coderd/coderd.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ func New(options *Options) *API {
461461

462462
ifoptions.OIDCConvertKeyCache==nil {
463463
options.OIDCConvertKeyCache,err=cryptokeys.NewSigningCache(ctx,
464-
options.Logger,
464+
options.Logger.Named("oidc_convert_keycache"),
465465
fetcher,
466466
codersdk.CryptoKeyFeatureOIDCConvert,
467467
)
@@ -470,7 +470,7 @@ func New(options *Options) *API {
470470

471471
ifoptions.AppSigningKeyCache==nil {
472472
options.AppSigningKeyCache,err=cryptokeys.NewSigningCache(ctx,
473-
options.Logger,
473+
options.Logger.Named("app_signing_keycache"),
474474
fetcher,
475475
codersdk.CryptoKeyFeatureWorkspaceAppsToken,
476476
)
@@ -683,10 +683,9 @@ func New(options *Options) *API {
683683
AgentProvider:api.agentProvider,
684684
StatsCollector:workspaceapps.NewStatsCollector(options.WorkspaceAppsStatsCollectorOptions),
685685

686-
DisablePathApps:options.DeploymentValues.DisablePathApps.Value(),
687-
SecureAuthCookie:options.DeploymentValues.SecureAuthCookie.Value(),
688-
Signer:options.AppSigningKeyCache,
689-
EncryptingKeyManager:options.AppEncryptionKeyCache,
686+
DisablePathApps:options.DeploymentValues.DisablePathApps.Value(),
687+
SecureAuthCookie:options.DeploymentValues.SecureAuthCookie.Value(),
688+
APIKeyEncryptionKey:options.AppEncryptionKeyCache,
690689
}
691690

692691
apiKeyMiddleware:=httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{

‎coderd/cryptokeys/cache.go‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"cdr.dev/slog"
1515
"github.com/coder/coder/v2/coderd/database"
1616
"github.com/coder/coder/v2/coderd/database/db2sdk"
17+
"github.com/coder/coder/v2/coderd/database/dbauthz"
1718
"github.com/coder/coder/v2/codersdk"
1819
"github.com/coder/quartz"
1920
)
@@ -77,12 +78,12 @@ func (d *DBFetcher) Fetch(ctx context.Context, feature codersdk.CryptoKeyFeature
7778

7879
// cache implements the caching functionality for both signing and encryption keys.
7980
typecachestruct {
80-
clock quartz.Clock
81-
refreshCtxcontext.Context
82-
refreshCancel context.CancelFunc
83-
fetcherFetcher
84-
loggerslog.Logger
85-
featurecodersdk.CryptoKeyFeature
81+
ctxcontext.Context
82+
cancelcontext.CancelFunc
83+
clock quartz.Clock
84+
fetcherFetcher
85+
logger slog.Logger
86+
feature codersdk.CryptoKeyFeature
8687

8788
mu sync.Mutex
8889
keysmap[int32]codersdk.CryptoKey
@@ -136,12 +137,13 @@ func newCache(ctx context.Context, logger slog.Logger, fetcher Fetcher, feature
136137
}
137138

138139
cache.cond=sync.NewCond(&cache.mu)
139-
cache.refreshCtx,cache.refreshCancel=context.WithCancel(ctx)
140+
//nolint:gocritic // We need to be able to read the keys in order to cache them.
141+
cache.ctx,cache.cancel=context.WithCancel(dbauthz.AsKeyReader(ctx))
140142
cache.refresher=cache.clock.AfterFunc(refreshInterval,cache.refresh)
141143

142-
keys,err:=cache.cryptoKeys(ctx)
144+
keys,err:=cache.cryptoKeys(cache.ctx)
143145
iferr!=nil {
144-
cache.refreshCancel()
146+
cache.cancel()
145147
returnnil,xerrors.Errorf("initial fetch: %w",err)
146148
}
147149
cache.keys=keys
@@ -205,7 +207,7 @@ func isEncryptionKeyFeature(feature codersdk.CryptoKeyFeature) bool {
205207

206208
funcisSigningKeyFeature(feature codersdk.CryptoKeyFeature)bool {
207209
switchfeature {
208-
casecodersdk.CryptoKeyFeatureTailnetResume,codersdk.CryptoKeyFeatureOIDCConvert:
210+
casecodersdk.CryptoKeyFeatureTailnetResume,codersdk.CryptoKeyFeatureOIDCConvert,codersdk.CryptoKeyFeatureWorkspaceAppsToken:
209211
returntrue
210212
default:
211213
returnfalse
@@ -315,9 +317,9 @@ func (c *cache) refresh() {
315317
c.fetching=true
316318

317319
c.mu.Unlock()
318-
keys,err:=c.cryptoKeys(c.refreshCtx)
320+
keys,err:=c.cryptoKeys(c.ctx)
319321
iferr!=nil {
320-
c.logger.Error(c.refreshCtx,"fetch crypto keys",slog.Error(err))
322+
c.logger.Error(c.ctx,"fetch crypto keys",slog.Error(err))
321323
return
322324
}
323325

@@ -336,7 +338,7 @@ func (c *cache) refresh() {
336338
func (c*cache)cryptoKeys(ctx context.Context) (map[int32]codersdk.CryptoKey,error) {
337339
keys,err:=c.fetcher.Fetch(ctx,c.feature)
338340
iferr!=nil {
339-
returnnil,xerrors.Errorf("crypto keys: %w",err)
341+
returnnil,xerrors.Errorf("fetch: %w",err)
340342
}
341343
cache:=toKeyMap(keys,c.clock.Now())
342344
returncache,nil
@@ -363,7 +365,7 @@ func (c *cache) Close() error {
363365
}
364366

365367
c.closed=true
366-
c.refreshCancel()
368+
c.cancel()
367369
c.refresher.Stop()
368370
c.cond.Broadcast()
369371

‎coderd/userauth.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ 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.OIDCConvertKeyCache,claims)
170+
//nolint:gocritic // We need to read the system signing key
171+
// in order to sign the token.
172+
token,err:=jwtutils.Sign(dbauthz.AsKeyReader(ctx),api.OIDCConvertKeyCache,claims)
171173
iferr!=nil {
172174
httpapi.Write(ctx,rw,http.StatusInternalServerError, codersdk.Response{
173175
Message:"Internal error signing state jwt.",
@@ -1676,7 +1678,10 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
16761678
}
16771679
}
16781680
varclaimsOAuthConvertStateClaims
1679-
err=jwtutils.Verify(dbauthz.AsKeyRotator(ctx),api.OIDCConvertKeyCache,jwtCookie.Value,&claims)
1681+
1682+
//nolint:gocritic // We need to read the system signing key
1683+
// in order to verify the token.
1684+
err=jwtutils.Verify(dbauthz.AsKeyReader(ctx),api.OIDCConvertKeyCache,jwtCookie.Value,&claims)
16801685
ifxerrors.Is(err,cryptokeys.ErrKeyNotFound)||xerrors.Is(err,cryptokeys.ErrKeyInvalid)||xerrors.Is(err,jose.ErrCryptoFailure) {
16811686
// These errors are probably because the user is mixing 2 coder deployments.
16821687
return database.User{}, idpsync.HTTPError{

‎coderd/workspaceapps/proxy.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,8 @@ type Server struct {
100100
HostnameRegex*regexp.Regexp
101101
RealIPConfig*httpmw.RealIPConfig
102102

103-
SignedTokenProviderSignedTokenProvider
104-
Signer jwtutils.SigningKeyManager
105-
EncryptingKeyManager jwtutils.EncryptingKeyManager
103+
SignedTokenProviderSignedTokenProvider
104+
APIKeyEncryptionKey jwtutils.EncryptingKeyManager
106105

107106
// DisablePathApps disables path-based apps. This is a security feature as path
108107
// based apps share the same cookie as the dashboard, and are susceptible to XSS
@@ -181,7 +180,7 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request,
181180

182181
// Exchange the encoded API key for a real one.
183182
varpayloadEncryptedAPIKeyPayload
184-
err:=jwtutils.Decrypt(ctx,s.EncryptingKeyManager,encryptedAPIKey,&payload,jwtutils.WithDecryptExpected(jwt.Expected{
183+
err:=jwtutils.Decrypt(ctx,s.APIKeyEncryptionKey,encryptedAPIKey,&payload,jwtutils.WithDecryptExpected(jwt.Expected{
185184
Time:time.Now(),
186185
}))
187186
iferr!=nil {

‎coderd/workspaceapps/token.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,13 @@ import (
55
"strings"
66
"time"
77

8-
"github.com/go-jose/go-jose/v4"
98
"github.com/go-jose/go-jose/v4/jwt"
109
"github.com/google/uuid"
1110

1211
"github.com/coder/coder/v2/coderd/jwtutils"
1312
"github.com/coder/coder/v2/codersdk"
1413
)
1514

16-
const (
17-
tokenSigningAlgorithm=jose.HS512
18-
apiKeyEncryptionAlgorithm=jose.A256GCMKW
19-
)
20-
2115
// SignedToken is the struct data contained inside a workspace app JWE. It
2216
// contains the details of the workspace app that the token is valid for to
2317
// avoid database queries.

‎enterprise/wsproxy/tokenprovider.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,14 @@ type TokenProvider struct {
1919
AccessURL*url.URL
2020
AppHostnamestring
2121

22-
Client*wsproxysdk.Client
23-
SigningKey jwtutils.SigningKeyManager
24-
EncryptingKey jwtutils.EncryptingKeyManager
25-
Logger slog.Logger
22+
Client*wsproxysdk.Client
23+
TokenSigningKey jwtutils.SigningKeyManager
24+
APIKeyEncryptionKey jwtutils.EncryptingKeyManager
25+
Loggerslog.Logger
2626
}
2727

2828
func (p*TokenProvider)FromRequest(r*http.Request) (*workspaceapps.SignedToken,bool) {
29-
returnworkspaceapps.FromRequest(r,p.SigningKey)
29+
returnworkspaceapps.FromRequest(r,p.TokenSigningKey)
3030
}
3131

3232
func (p*TokenProvider)Issue(ctx context.Context,rw http.ResponseWriter,r*http.Request,issueReq workspaceapps.IssueTokenRequest) (*workspaceapps.SignedToken,string,bool) {
@@ -45,7 +45,7 @@ func (p *TokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *ht
4545

4646
// Check that it verifies properly and matches the string.
4747
vartoken workspaceapps.SignedToken
48-
err=jwtutils.Verify(ctx,p.SigningKey,resp.SignedTokenStr,&token)
48+
err=jwtutils.Verify(ctx,p.TokenSigningKey,resp.SignedTokenStr,&token)
4949
iferr!=nil {
5050
workspaceapps.WriteWorkspaceApp500(p.Logger,p.DashboardURL,rw,r,&appReq,err,"failed to verify newly generated signed token")
5151
returnnil,"",false

‎enterprise/wsproxy/wsproxy.go‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,12 @@ type Server struct {
131131
// the moon's token.
132132
SDKClient*wsproxysdk.Client
133133

134-
WorkspaceAppsEncryptionKeycache cryptokeys.EncryptionKeycache
135-
WorkspaceAppsSigningKeycache cryptokeys.SigningKeycache
134+
// apiKeyEncryptionKeycache manages the encryption keys for smuggling API
135+
// tokens to the alternate domain when using workspace apps.
136+
apiKeyEncryptionKeycache cryptokeys.EncryptionKeycache
137+
// appTokenSigningKeycache manages the signing keys for signing the app
138+
// tokens we use for workspace apps.
139+
appTokenSigningKeycache cryptokeys.SigningKeycache
136140

137141
// DERP
138142
derpMesh*derpmesh.Mesh
@@ -206,6 +210,7 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
206210
codersdk.CryptoKeyFeatureWorkspaceAppsAPIKey,
207211
)
208212
iferr!=nil {
213+
cancel()
209214
returnnil,xerrors.Errorf("create api key encryption cache: %w",err)
210215
}
211216
signingCache,err:=cryptokeys.NewSigningCache(ctx,
@@ -214,6 +219,7 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
214219
codersdk.CryptoKeyFeatureWorkspaceAppsToken,
215220
)
216221
iferr!=nil {
222+
cancel()
217223
returnnil,xerrors.Errorf("create api token signing cache: %w",err)
218224
}
219225

@@ -222,15 +228,17 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
222228
ctx:ctx,
223229
cancel:cancel,
224230

225-
Options:opts,
226-
Handler:r,
227-
DashboardURL:opts.DashboardURL,
228-
Logger:opts.Logger.Named("net.workspace-proxy"),
229-
TracerProvider:opts.Tracing,
230-
PrometheusRegistry:opts.PrometheusRegistry,
231-
SDKClient:client,
232-
derpMesh:derpmesh.New(opts.Logger.Named("net.derpmesh"),derpServer,meshTLSConfig),
233-
derpMeshTLSConfig:meshTLSConfig,
231+
Options:opts,
232+
Handler:r,
233+
DashboardURL:opts.DashboardURL,
234+
Logger:opts.Logger.Named("net.workspace-proxy"),
235+
TracerProvider:opts.Tracing,
236+
PrometheusRegistry:opts.PrometheusRegistry,
237+
SDKClient:client,
238+
derpMesh:derpmesh.New(opts.Logger.Named("net.derpmesh"),derpServer,meshTLSConfig),
239+
derpMeshTLSConfig:meshTLSConfig,
240+
apiKeyEncryptionKeycache:encryptionCache,
241+
appTokenSigningKeycache:signingCache,
234242
}
235243

236244
// Register the workspace proxy with the primary coderd instance and start a
@@ -295,20 +303,21 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
295303
HostnameRegex:opts.AppHostnameRegex,
296304
RealIPConfig:opts.RealIPConfig,
297305
SignedTokenProvider:&TokenProvider{
298-
DashboardURL:opts.DashboardURL,
299-
AccessURL:opts.AccessURL,
300-
AppHostname:opts.AppHostname,
301-
Client:client,
302-
SigningKey:signingCache,
303-
EncryptingKey:encryptionCache,
304-
Logger:s.Logger.Named("proxy_token_provider"),
306+
DashboardURL:opts.DashboardURL,
307+
AccessURL:opts.AccessURL,
308+
AppHostname:opts.AppHostname,
309+
Client:client,
310+
TokenSigningKey:signingCache,
311+
APIKeyEncryptionKey:encryptionCache,
312+
Logger:s.Logger.Named("proxy_token_provider"),
305313
},
306314

307315
DisablePathApps:opts.DisablePathApps,
308316
SecureAuthCookie:opts.SecureAuthCookie,
309317

310-
AgentProvider:agentProvider,
311-
StatsCollector:workspaceapps.NewStatsCollector(opts.StatsCollectorOptions),
318+
AgentProvider:agentProvider,
319+
StatsCollector:workspaceapps.NewStatsCollector(opts.StatsCollectorOptions),
320+
APIKeyEncryptionKey:encryptionCache,
312321
}
313322

314323
derpHandler:=derphttp.Handler(derpServer)
@@ -451,8 +460,8 @@ func (s *Server) Close() error {
451460
err=multierror.Append(err,agentProviderErr)
452461
}
453462
s.SDKClient.SDKClient.HTTPClient.CloseIdleConnections()
454-
_=s.WorkspaceAppsSigningKeycache.Close()
455-
_=s.WorkspaceAppsEncryptionKeycache.Close()
463+
_=s.appTokenSigningKeycache.Close()
464+
_=s.apiKeyEncryptionKeycache.Close()
456465
returnerr
457466
}
458467

‎enterprise/wsproxy/wsproxy_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http"
99
"net/http/httptest"
1010
"net/url"
11+
"runtime/debug"
1112
"testing"
1213
"time"
1314

@@ -321,6 +322,11 @@ resourceLoop:
321322

322323
funcTestDERPEndToEnd(t*testing.T) {
323324
t.Parallel()
325+
deferfunc() {
326+
ifr:=recover();r!=nil {
327+
debug.PrintStack()
328+
}
329+
}()
324330

325331
deploymentValues:=coderdtest.DeploymentValues(t)
326332
deploymentValues.Experiments= []string{

‎enterprise/wsproxy/wsproxysdk/wsproxysdk.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,7 @@ type CryptoKeysResponse struct {
581581

582582
func (c*Client)CryptoKeys(ctx context.Context,feature codersdk.CryptoKeyFeature) (CryptoKeysResponse,error) {
583583
res,err:=c.Request(ctx,http.MethodGet,
584-
"/api/v2/workspaceproxies/me/crypto-keys",
584+
"/api/v2/workspaceproxies/me/crypto-keys",nil,
585585
codersdk.WithQueryParam("feature",string(feature)),
586586
)
587587
iferr!=nil {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp