feat: enable key rotation

coder · sreya · Oct 25, 2024 · Oct 5, 2024 · Oct 5, 2024 · Oct 5, 2024
commit b745c8eab95584fe00f7c49e57f79f2b07b6df9f
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -40,6 +40,7 @@ import (
 	"github.com/coder/quartz"
 	"github.com/coder/serpent"
 
+	"github.com/coder/coder/v2/coderd/cryptokeys"
 	"github.com/coder/coder/v2/coderd/entitlements"
 	"github.com/coder/coder/v2/coderd/idpsync"
 	"github.com/coder/coder/v2/coderd/runtimeconfig"
@@ -609,6 +610,14 @@ func New(options *Options) *API {
 		api.Logger.Fatal(api.ctx, "failed to initialize tailnet client service", slog.Error(err))
 	}
 
+	// Start a background process that rotates keys.
+	err = cryptokeys.StartRotator(api.ctx, api.Logger.Named("keyrotator"), api.Database)
+	if err != nil {
+		api.Logger.Fatal(api.ctx, "start key rotator", slog.Error(err))
+	}
+
+	api.oauthConvertKeycache = cryptokeys.NewDBCache(api.Logger.Named("oauth_convert_keycache"), api.Database, database.CryptoKeyFeatureOidcConvert)
+
 	api.statsReporter = workspacestats.NewReporter(workspacestats.ReporterOptions{
 		Database:              options.Database,
 		Logger:                options.Logger.Named("workspacestats"),
@@ -1389,6 +1398,11 @@ type API struct {
 	// dbRolluper rolls up template usage stats from raw agent and app
 	// stats. This is used to provide insights in the WebUI.
 	dbRolluper *dbrollup.Rolluper
+
+	// resumeTokenKeycache is used to fetch and cache keys used for signing JWTs
+	// oauthConvertKeycache is used to fetch and cache keys used for signing JWTs
+	// during OAuth conversions. See userauth.go.convertUserToOauth.
+	oauthConvertKeycache cryptokeys.Keycache
 }
 
 // Close waits for all WebSocket connections to drain before returning.

diff --git a/coderd/cryptokeys/rotate.go b/coderd/cryptokeys/rotate.go
@@ -11,6 +11,7 @@ import (
 
 	"cdr.dev/slog"
 	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbauthz"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/quartz"
 )
@@ -54,6 +55,8 @@ func WithKeyDuration(keyDuration time.Duration) RotatorOption {
 // It ensures there's at least one valid key per feature prior to returning.
 // Canceling the provided context will stop the background process.
 func StartRotator(ctx context.Context, logger slog.Logger, db database.Store, opts ...RotatorOption) error {
+	//nolint:gocritic // KeyRotator can only rotate crypto keys.
+	ctx = dbauthz.AsSystemRestricted(ctx)
 	kr := &rotator{
 		db:          db,
 		logger:      logger,

diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -228,6 +228,24 @@ var (
 		Scope: rbac.ScopeAll,
 	}.WithCachedASTValue()
 
+	// See cryptokeys package.
+	subjectCryptoKey = rbac.Subject{
+		FriendlyName: "Crypto Key Rotator",
+		ID:           uuid.Nil.String(),
+		Roles: rbac.Roles([]rbac.Role{
+			{
+				Identifier:  rbac.RoleIdentifier{Name: "keyrotator"},
+				DisplayName: "Key Rotator",
+				Site: rbac.Permissions(map[string][]policy.Action{
+					rbac.ResourceCryptoKey.Type: {policy.WildcardSymbol, policy.ActionRead},
+				}),
+				Org:  map[string][]rbac.Permission{},
+				User: []rbac.Permission{},
+			},
+		}),
+		Scope: rbac.ScopeAll,
+	}.WithCachedASTValue()
+
 	subjectSystemRestricted = rbac.Subject{
 		FriendlyName: "System",
 		ID:           uuid.Nil.String(),
@@ -281,6 +299,11 @@ func AsHangDetector(ctx context.Context) context.Context {
 	return context.WithValue(ctx, authContextKey{}, subjectHangDetector)
 }
 
+// AsKeyRotator returns a context with an actor that has permissions required for rotating crypto keys.
+func AsKeyRotator(ctx context.Context) context.Context {
+	return context.WithValue(ctx, authContextKey{}, subjectCryptoKey)
+}
+
 // AsSystemRestricted returns a context with an actor that has permissions
 // required for various system operations (login, logout, metrics cache).
 func AsSystemRestricted(ctx context.Context) context.Context {

diff --git a/coderd/userauth.go b/coderd/userauth.go
@@ -15,14 +15,18 @@ import (
 	"time"
 
 	"github.com/coreos/go-oidc/v3/oidc"
-	"github.com/golang-jwt/jwt/v4"
+	"github.com/go-jose/go-jose/v4"
+	"github.com/go-jose/go-jose/v4/jwt"
 	"github.com/google/go-github/v43/github"
 	"github.com/google/uuid"
 	"github.com/moby/moby/pkg/namesgenerator"
 	"golang.org/x/oauth2"
 	"golang.org/x/xerrors"
 
 	"cdr.dev/slog"
+	"github.com/coder/coder/v2/coderd/cryptokeys"
+	"github.com/coder/coder/v2/coderd/idpsync"
+	"github.com/coder/coder/v2/coderd/jwtutils"
 
 	"github.com/coder/coder/v2/coderd/apikey"
 	"github.com/coder/coder/v2/coderd/audit"
@@ -49,7 +53,7 @@ const (
 )
 
 type OAuthConvertStateClaims struct {
-	jwt.RegisteredClaims
+	jwt.Claims
 
 	UserID        uuid.UUID          `json:"user_id"`
 	State         string             `json:"state"`
@@ -149,11 +153,11 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
 	// Eg: Developers with more than 1 deployment.
 	now := time.Now()
 	claims := &OAuthConvertStateClaims{
-		RegisteredClaims: jwt.RegisteredClaims{
+		Claims: jwt.Claims{
 			Issuer:    api.DeploymentID,
 			Subject:   stateString,
 			Audience:  []string{user.ID.String()},
-			ExpiresAt: jwt.NewNumericDate(now.Add(time.Minute * 5)),
+			Expiry:    jwt.NewNumericDate(now.Add(time.Minute * 5)),
 			NotBefore: jwt.NewNumericDate(now.Add(time.Second * -1)),
 			IssuedAt:  jwt.NewNumericDate(now),
 			ID:        uuid.NewString(),
@@ -164,9 +168,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
 		ToLoginType:   req.ToType,
 	}
 
-	token := jwt.NewWithClaims(jwt.SigningMethodHS512, claims)
-	// Key must be a byte slice, not an array. So make sure to include the [:]
-	tokenString, err := token.SignedString(api.OAuthSigningKey[:])
+	token, err := jwtutils.Sign(dbauthz.AsKeyRotator(ctx), api.oauthConvertKeycache, claims)
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
 			Message: "Internal error signing state jwt.",
@@ -176,8 +178,8 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	aReq.New = database.AuditOAuthConvertState{
-		CreatedAt:     claims.IssuedAt.Time,
-		ExpiresAt:     claims.ExpiresAt.Time,
+		CreatedAt:     claims.IssuedAt.Time(),
+		ExpiresAt:     claims.Expiry.Time(),
 		FromLoginType: database.LoginType(claims.FromLoginType),
 		ToLoginType:   database.LoginType(claims.ToLoginType),
 		UserID:        claims.UserID,
@@ -186,8 +188,8 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
 	http.SetCookie(rw, &http.Cookie{
 		Name:     OAuthConvertCookieValue,
 		Path:     "/",
-		Value:    tokenString,
-		Expires:  claims.ExpiresAt.Time,
+		Value:    token,
+		Expires:  claims.Expiry.Time(),
 		Secure:   api.SecureAuthCookie,
 		HttpOnly: true,
 		// 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) {
 	})
 	httpapi.Write(ctx, rw, http.StatusCreated, codersdk.OAuthConversionResponse{
 		StateString: stateString,
-		ExpiresAt:   claims.ExpiresAt.Time,
+		ExpiresAt:   claims.Expiry.Time(),
 		ToType:      claims.ToLoginType,
 		UserID:      claims.UserID,
 	})
@@ -1675,10 +1677,8 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
 		}
 	}
 	var claims OAuthConvertStateClaims
-	token, err := jwt.ParseWithClaims(jwtCookie.Value, &claims, func(_ *jwt.Token) (interface{}, error) {
-		return api.OAuthSigningKey[:], nil
-	})
-	if xerrors.Is(err, jwt.ErrSignatureInvalid) || !token.Valid {
+	err = jwtutils.Verify(dbauthz.AsKeyRotator(ctx), api.oauthConvertKeycache, jwtCookie.Value, &claims)
+	if xerrors.Is(err, cryptokeys.ErrKeyNotFound) || xerrors.Is(err, cryptokeys.ErrKeyInvalid) || xerrors.Is(err, jose.ErrCryptoFailure) {
 		// These errors are probably because the user is mixing 2 coder deployments.
 		return database.User{}, idpsync.HTTPError{
 			Code: http.StatusBadRequest,
@@ -1707,7 +1707,7 @@ func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db data
 	oauthConvertAudit.UserID = claims.UserID
 	oauthConvertAudit.Old = user
 
-	if claims.RegisteredClaims.Issuer != api.DeploymentID {
+	if claims.Issuer != api.DeploymentID {
 		return database.User{}, idpsync.HTTPError{
 			Code: http.StatusForbidden,
 			Msg:  "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.",