jwtx: Add [pfx]KEYS_ACCEPTED configuration to enable seamless key rotations, add [oauth2] JWT_KEYS_ACCEPTED (#12307)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12307
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

Author: Gusted

.deadcode-out

@@ -131,6 +131,11 @@ forgejo.org/modules/json
 	StdJSON.NewDecoder
 	StdJSON.Indent
 
+forgejo.org/modules/jwtx
+	savePublicKey
+	NewVerifier
+	Verifier.Parse
+
 forgejo.org/modules/log
 	eventWriterBuffer.Close
 	eventWriterBuffer.Write

custom/conf/app.example.ini

@@ -557,6 +557,14 @@ ENABLED = true
 ;; Alternative location to specify OAuth2 authentication secret. You cannot specify both this and JWT_SECRET, and must pick one
 ;JWT_SECRET_URI = file:/etc/gitea/oauth2_jwt_secret
 ;;
+;; Additional keys accepted for validation. Useful for key rotation. Format:
+;; <alg>:[<base64>|<uri>] ...
+;; alg: see JWT_SIGNING_ALGORITHM
+;; uri: file://<path>
+;; example: JWT_SIGNING_ALGORITHM = RS256:file:module/pub/*.pem HS256:file:module/old.key
+;; there is no default
+;JWT_SIGNING_ALGORITHM =
+;;
 ;; Lifetime of an OAuth2 access token in seconds
 ;ACCESS_TOKEN_EXPIRATION_TIME = 3600
 ;;

modules/jwtx/signingkey.go

@@ -35,8 +35,8 @@ type SigningKeyCfg struct {
 }
 
 type KeyCfg struct {
-	Signing *SigningKeyCfg
-	// more later
+	Signing      *SigningKeyCfg
+	Verification []*VerificationKeyCfg
 }
 
 // ErrInvalidAlgorithmType represents an invalid algorithm error.
@@ -61,6 +61,7 @@ type SigningKey interface {
 	SignKey() any
 	VerifyKey() any
 	ToJWK() (map[string]string, error)
+	ID() string
 	PreProcessToken(*jwt.Token)
 	// convenience: jwt.NewWithClaims + PreProcessToken + SignedString
 	JWT(jwt.Claims, ...jwt.TokenOption) (string, error)
@@ -94,6 +95,10 @@ func (key hmacSigningKey) ToJWK() (map[string]string, error) {
 	}, nil
 }
 
+func (key hmacSigningKey) ID() string {
+	return ""
+}
+
 func (key hmacSigningKey) PreProcessToken(*jwt.Token) {}
 
 func (key hmacSigningKey) JWT(claims jwt.Claims, opts ...jwt.TokenOption) (string, error) {
@@ -147,6 +152,10 @@ func (key rsaSigningKey) ToJWK() (map[string]string, error) {
 	}, nil
 }
 
+func (key rsaSigningKey) ID() string {
+	return key.id
+}
+
 func (key rsaSigningKey) PreProcessToken(token *jwt.Token) {
 	token.Header["kid"] = key.id
 }
@@ -202,6 +211,10 @@ func (key eddsaSigningKey) ToJWK() (map[string]string, error) {
 	}, nil
 }
 
+func (key eddsaSigningKey) ID() string {
+	return key.id
+}
+
 func (key eddsaSigningKey) PreProcessToken(token *jwt.Token) {
 	token.Header["kid"] = key.id
 }
@@ -258,6 +271,10 @@ func (key ecdsaSigningKey) ToJWK() (map[string]string, error) {
 	}, nil
 }
 
+func (key ecdsaSigningKey) ID() string {
+	return key.id
+}
+
 func (key ecdsaSigningKey) PreProcessToken(token *jwt.Token) {
 	token.Header["kid"] = key.id
 }
@@ -381,7 +398,7 @@ func createAsymmetricKey(keyPath, algorithm string) error {
 	return pem.Encode(f, privateKeyPEM)
 }
 
-func loadAsymmetricKey(keyPath string) (any, error) {
+func loadPrivateKey(keyPath string) (any, error) {
 	bytes, err := os.ReadFile(keyPath)
 	if err != nil {
 		return nil, err
@@ -410,7 +427,36 @@ func loadOrCreateAsymmetricKey(keyPath, algorithm string) (any, error) {
 			return nil, fmt.Errorf("Error generating private key %s: %v", keyPath, err)
 		}
 	}
-	return loadAsymmetricKey(keyPath)
+	return loadPrivateKey(keyPath)
+}
+
+// save the public key for a private key
+func savePublicKey(keyPath string, signingKey SigningKey) error {
+	if signingKey.IsSymmetric() {
+		return fmt.Errorf("Saving symmatric key deliberately not supported (path: \"%s\")", keyPath)
+	}
+	bytes, err := x509.MarshalPKIXPublicKey(signingKey.VerifyKey())
+	if err != nil {
+		return err
+	}
+
+	publicKeyPEM := &pem.Block{Type: "PUBLIC KEY", Bytes: bytes}
+
+	if err := os.MkdirAll(filepath.Dir(keyPath), os.ModePerm); err != nil {
+		return err
+	}
+
+	f, err := os.OpenFile(keyPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if err = f.Close(); err != nil {
+			log.Error("Close: %v", err)
+		}
+	}()
+
+	return pem.Encode(f, publicKeyPEM)
 }
 
 // InitSigningKey creates a signing key from SigningKeyCfg
@@ -563,3 +609,33 @@ func ParseJWKToPublicKey(jwk map[string]any) (any, error) {
 		return nil, fmt.Errorf("unsupported key type in JWK: %s", kty)
 	}
 }
+
+// Init Signing Key and Verifier
+func InitWithParser(keyCfgP **KeyCfg, parser *jwt.Parser) (SigningKey, *Verifier, error) {
+	keyCfg := *keyCfgP
+	*keyCfgP = nil
+
+	signingKey, err := InitSigningKey(&keyCfg.Signing)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// we always add the signing key to the verifier (we accept tokens which
+	// we issue). No idea if there could be future cases where we do not
+	// want this, but for now antything else would be very un-POLA
+	verifier := NewVerifierWithParser(parser)
+	err = verifier.AddKey(signingKey)
+	if err != nil {
+		return nil, nil, err
+	}
+	err = verifier.AddVerificationKeyCfg(&keyCfg.Verification)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return signingKey, verifier, nil
+}
+
+func Init(keyCfgP **KeyCfg) (SigningKey, *Verifier, error) {
+	return InitWithParser(keyCfgP, jwt.NewParser())
+}

modules/jwtx/signingkey_test.go

@@ -13,12 +13,14 @@ import (
 	"path/filepath"
 	"testing"
 
+	"forgejo.org/modules/generate"
+
 	"github.com/golang-jwt/jwt/v5"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func testSignVerify(t *testing.T, signKey, verifyKey SigningKey) {
+func testSignVerify(t *testing.T, signKey SigningKey, verifyKey VerificationKey) {
 	t.Helper()
 	// test sign and verify
 	claimsIn := jwt.RegisteredClaims{
@@ -34,9 +36,15 @@ func testSignVerify(t *testing.T, signKey, verifyKey SigningKey) {
 		assert.NotNil(t, valToken.Method)
 		assert.Equal(t, signKey.SigningMethod().Alg(), valToken.Method.Alg())
 		assert.Equal(t, verifyKey.SigningMethod().Alg(), valToken.Method.Alg())
+
+		// asymmetric keys generate JWT with a kid, symmetric not
 		kid, ok := valToken.Header["kid"]
-		assert.True(t, ok)
-		assert.NotNil(t, kid)
+		if signKey.IsSymmetric() {
+			assert.False(t, ok)
+		} else {
+			assert.True(t, ok)
+			assert.NotNil(t, kid)
+		}
 
 		return verifyKey.VerifyKey(), nil
 	})
@@ -68,17 +76,53 @@ func TestLoadOrCreateAsymmetricKey(t *testing.T) {
 	useKey := func(t *testing.T, keyPath, algorithm string) {
 		t.Helper()
 		// duplicates loadKey() to some extent, but uses SigningKey
-		cfg := &SigningKeyCfg{
+		assert.NotEmpty(t, keyPath)
+		scfg := &SigningKeyCfg{
 			Algorithm:      algorithm,
 			PrivateKeyPath: &keyPath,
 		}
 
-		key, err := InitSigningKey(&cfg)
+		// load the signing key via the settings interface
+		key, err := InitSigningKey(&scfg)
 		require.NoError(t, err)
 		assert.NotNil(t, key)
-		assert.Nil(t, cfg)
+		assert.Nil(t, scfg)
+		assert.NotEmpty(t, key.ID())
+		assert.Nil(t, scfg)
 
 		testSignVerify(t, key, key)
+
+		// load the signing key file as a verification key
+		assert.NotEmpty(t, keyPath)
+		vcfg := &VerificationKeyCfg{
+			Algorithm:     algorithm,
+			PublicKeyPath: &keyPath,
+		}
+
+		// load the verification key via the settings interface
+		vkey, err := InitVerificationKey(&vcfg)
+		require.NoError(t, err)
+		assert.NotNil(t, key)
+		assert.Equal(t, key.ID(), vkey.ID())
+		assert.Nil(t, scfg)
+
+		testSignVerify(t, key, vkey)
+
+		// save the public key, then load it and test
+		pKeyPath := keyPath + ".pub"
+		err = savePublicKey(pKeyPath, key)
+		require.NoError(t, err)
+		vcfg = &VerificationKeyCfg{
+			Algorithm:     algorithm,
+			PublicKeyPath: &pKeyPath,
+		}
+		vkey, err = InitVerificationKey(&vcfg)
+		require.NoError(t, err)
+		assert.NotNil(t, vkey)
+		assert.Equal(t, key.ID(), vkey.ID())
+		assert.Nil(t, vcfg)
+
+		testSignVerify(t, key, vkey)
 	}
 	t.Run("RSA-2048", func(t *testing.T) {
 		keyPath := filepath.Join(t.TempDir(), "jwt-rsa-2048.priv")
@@ -184,3 +228,39 @@ func TestCannotCreatePrivateKey(t *testing.T) {
 	require.Error(t, err)
 	require.ErrorContains(t, err, "Error generating private key")
 }
+
+// test symmetic algorithms used via the SigningKey and VerificationKey
+// interfaces
+func TestSymmetricKey(t *testing.T) {
+	algorithms := []string{"HS256", "HS384", "HS512"}
+	for _, algorithm := range algorithms {
+		t.Run(algorithm, func(t *testing.T) {
+			secret, _ := generate.NewJwtSecret()
+			assert.NotEmpty(t, secret)
+
+			// init the signing key via the settings interface
+			scfg := &SigningKeyCfg{
+				Algorithm:   algorithm,
+				SecretBytes: &secret,
+			}
+			skey, err := InitSigningKey(&scfg)
+			require.NoError(t, err)
+			assert.NotNil(t, skey)
+			assert.Nil(t, scfg)
+
+			testSignVerify(t, skey, skey)
+
+			// init the same key as a VerificationKey
+			vcfg := &VerificationKeyCfg{
+				Algorithm:   algorithm,
+				SecretBytes: &secret,
+			}
+			vkey, err := InitVerificationKey(&vcfg)
+			require.NoError(t, err)
+			assert.NotNil(t, vkey)
+			assert.Nil(t, vcfg)
+
+			testSignVerify(t, skey, vkey)
+		})
+	}
+}