Skip to content

Commit b745c8e

Browse files
sreyasreya
committed
feat: enable key rotation
1 parent 512cbf1 commit b745c8e

File tree

4 files changed

+57
-17
lines changed

4 files changed

+57
-17
lines changed

‎coderd/coderd.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"github.com/coder/quartz"
4141
"github.com/coder/serpent"
4242

43+
"github.com/coder/coder/v2/coderd/cryptokeys"
4344
"github.com/coder/coder/v2/coderd/entitlements"
4445
"github.com/coder/coder/v2/coderd/idpsync"
4546
"github.com/coder/coder/v2/coderd/runtimeconfig"
@@ -609,6 +610,14 @@ func New(options *Options) *API {
609610
api.Logger.Fatal(api.ctx, "failed to initialize tailnet client service", slog.Error(err))
610611
}
611612

613+
// Start a background process that rotates keys.
614+
err = cryptokeys.StartRotator(api.ctx, api.Logger.Named("keyrotator"), api.Database)
615+
if err != nil {
616+
api.Logger.Fatal(api.ctx, "start key rotator", slog.Error(err))
617+
}
618+
619+
api.oauthConvertKeycache = cryptokeys.NewDBCache(api.Logger.Named("oauth_convert_keycache"), api.Database, database.CryptoKeyFeatureOidcConvert)
620+
612621
api.statsReporter = workspacestats.NewReporter(workspacestats.ReporterOptions{
613622
Database: options.Database,
614623
Logger: options.Logger.Named("workspacestats"),
@@ -1389,6 +1398,11 @@ type API struct {
13891398
// dbRolluper rolls up template usage stats from raw agent and app
13901399
// stats. This is used to provide insights in the WebUI.
13911400
dbRolluper *dbrollup.Rolluper
1401+
1402+
// resumeTokenKeycache is used to fetch and cache keys used for signing JWTs
1403+
// oauthConvertKeycache is used to fetch and cache keys used for signing JWTs
1404+
// during OAuth conversions. See userauth.go.convertUserToOauth.
1405+
oauthConvertKeycache cryptokeys.Keycache
13921406
}
13931407

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

‎coderd/cryptokeys/rotate.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"cdr.dev/slog"
1313
"github.com/coder/coder/v2/coderd/database"
14+
"github.com/coder/coder/v2/coderd/database/dbauthz"
1415
"github.com/coder/coder/v2/coderd/database/dbtime"
1516
"github.com/coder/quartz"
1617
)
@@ -54,6 +55,8 @@ func WithKeyDuration(keyDuration time.Duration) RotatorOption {
5455
// It ensures there's at least one valid key per feature prior to returning.
5556
// Canceling the provided context will stop the background process.
5657
func StartRotator(ctx context.Context, logger slog.Logger, db database.Store, opts ...RotatorOption) error {
58+
//nolint:gocritic // KeyRotator can only rotate crypto keys.
59+
ctx = dbauthz.AsSystemRestricted(ctx)
5760
kr := &rotator{
5861
db: db,
5962
logger: logger,

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,24 @@ var (
228228
Scope: rbac.ScopeAll,
229229
}.WithCachedASTValue()
230230

231+
// See cryptokeys package.
232+
subjectCryptoKey = rbac.Subject{
233+
FriendlyName: "Crypto Key Rotator",
234+
ID: uuid.Nil.String(),
235+
Roles: rbac.Roles([]rbac.Role{
236+
{
237+
Identifier: rbac.RoleIdentifier{Name: "keyrotator"},
238+
DisplayName: "Key Rotator",
239+
Site: rbac.Permissions(map[string][]policy.Action{
240+
rbac.ResourceCryptoKey.Type: {policy.WildcardSymbol, policy.ActionRead},
241+
}),
242+
Org: map[string][]rbac.Permission{},
243+
User: []rbac.Permission{},
244+
},
245+
}),
246+
Scope: rbac.ScopeAll,
247+
}.WithCachedASTValue()
248+
231249
subjectSystemRestricted = rbac.Subject{
232250
FriendlyName: "System",
233251
ID: uuid.Nil.String(),
@@ -281,6 +299,11 @@ func AsHangDetector(ctx context.Context) context.Context {
281299
return context.WithValue(ctx, authContextKey{}, subjectHangDetector)
282300
}
283301

302+
// AsKeyRotator returns a context with an actor that has permissions required for rotating crypto keys.
303+
func AsKeyRotator(ctx context.Context) context.Context {
304+
return context.WithValue(ctx, authContextKey{}, subjectCryptoKey)
305+
}
306+
284307
// AsSystemRestricted returns a context with an actor that has permissions
285308
// required for various system operations (login, logout, metrics cache).
286309
func AsSystemRestricted(ctx context.Context) context.Context {

‎coderd/userauth.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,18 @@ import (
1515
"time"
1616

1717
"github.com/coreos/go-oidc/v3/oidc"
18-
"github.com/golang-jwt/jwt/v4"
18+
"github.com/go-jose/go-jose/v4"
19+
"github.com/go-jose/go-jose/v4/jwt"
1920
"github.com/google/go-github/v43/github"
2021
"github.com/google/uuid"
2122
"github.com/moby/moby/pkg/namesgenerator"
2223
"golang.org/x/oauth2"
2324
"golang.org/x/xerrors"
2425

2526
"cdr.dev/slog"
27+
"github.com/coder/coder/v2/coderd/cryptokeys"
28+
"github.com/coder/coder/v2/coderd/idpsync"
29+
"github.com/coder/coder/v2/coderd/jwtutils"
2630

2731
"github.com/coder/coder/v2/coderd/apikey"
2832
"github.com/coder/coder/v2/coderd/audit"
@@ -49,7 +53,7 @@ const (
4953
)
5054

5155
type OAuthConvertStateClaims struct {
52-
jwt.RegisteredClaims
56+
jwt.Claims
5357

5458
UserID uuid.UUID `json:"user_id"`
5559
State string `json:"state"`
@@ -149,11 +153,11 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
149153
// Eg: Developers with more than 1 deployment.
150154
now := time.Now()
151155
claims := &OAuthConvertStateClaims{
152-
RegisteredClaims: jwt.RegisteredClaims{
156+
Claims: jwt.Claims{
153157
Issuer: api.DeploymentID,
154158
Subject: stateString,
155159
Audience: []string{user.ID.String()},
156-
ExpiresAt: jwt.NewNumericDate(now.Add(time.Minute * 5)),
160+
Expiry: jwt.NewNumericDate(now.Add(time.Minute * 5)),
157161
NotBefore: jwt.NewNumericDate(now.Add(time.Second * -1)),
158162
IssuedAt: jwt.NewNumericDate(now),
159163
ID: uuid.NewString(),
@@ -164,9 +168,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
164168
ToLoginType: req.ToType,
165169
}
166170

167-
token := jwt.NewWithClaims(jwt.SigningMethodHS512, claims)
168-
// Key must be a byte slice, not an array. So make sure to include the [:]
169-
tokenString, err := token.SignedString(api.OAuthSigningKey[:])
171+
token, err := jwtutils.Sign(dbauthz.AsKeyRotator(ctx), api.oauthConvertKeycache, claims)
170172
if err != nil {
171173
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
172174
Message: "Internal error signing state jwt.",
@@ -176,8 +178,8 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
176178
}
177179

178180
aReq.New = database.AuditOAuthConvertState{
179-
CreatedAt: claims.IssuedAt.Time,
180-
ExpiresAt: claims.ExpiresAt.Time,
181+
CreatedAt: claims.IssuedAt.Time(),
182+
ExpiresAt: claims.Expiry.Time(),
181183
FromLoginType: database.LoginType(claims.FromLoginType),
182184
ToLoginType: database.LoginType(claims.ToLoginType),
183185
UserID: claims.UserID,
@@ -186,8 +188,8 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
186188
http.SetCookie(rw, &http.Cookie{
187189
Name: OAuthConvertCookieValue,
188190
Path: "/",
189-
Value: tokenString,
190-
Expires: claims.ExpiresAt.Time,
191+
Value: token,
192+
Expires: claims.Expiry.Time(),
191193
Secure: api.SecureAuthCookie,
192194
HttpOnly: true,
193195
// Must be SameSite to work on the redirected auth flow from the
@@ -196,7 +198,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
196198
})
197199
httpapi.Write(ctx, rw, http.StatusCreated, codersdk.OAuthConversionResponse{
198200
StateString: stateString,
199-
ExpiresAt: claims.ExpiresAt.Time,
201+
ExpiresAt: claims.Expiry.Time(),
200202
ToType: claims.ToLoginType,
201203
UserID: claims.UserID,
202204
})
@@ -1675,10 +1677,8 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
16751677
}
16761678
}
16771679
var claims OAuthConvertStateClaims
1678-
token, err := jwt.ParseWithClaims(jwtCookie.Value, &claims, func(_ *jwt.Token) (interface{}, error) {
1679-
return api.OAuthSigningKey[:], nil
1680-
})
1681-
if xerrors.Is(err, jwt.ErrSignatureInvalid) || !token.Valid {
1680+
err = jwtutils.Verify(dbauthz.AsKeyRotator(ctx), api.oauthConvertKeycache, jwtCookie.Value, &claims)
1681+
if xerrors.Is(err, cryptokeys.ErrKeyNotFound) || xerrors.Is(err, cryptokeys.ErrKeyInvalid) || xerrors.Is(err, jose.ErrCryptoFailure) {
16821682
// These errors are probably because the user is mixing 2 coder deployments.
16831683
return database.User{}, idpsync.HTTPError{
16841684
Code: http.StatusBadRequest,
@@ -1707,7 +1707,7 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
17071707
oauthConvertAudit.UserID = claims.UserID
17081708
oauthConvertAudit.Old = user
17091709

1710-
if claims.RegisteredClaims.Issuer != api.DeploymentID {
1710+
if claims.Issuer != api.DeploymentID {
17111711
return database.User{}, idpsync.HTTPError{
17121712
Code: http.StatusForbidden,
17131713
Msg: "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.",

0 commit comments

Comments
 (0)