Skip to content

sec(oidc): switch JWT signing from PKCS1v15 to ES256 #882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cristim wants to merge 1 commit into
base: feat/multicloud-web-frontend
Choose a base branch
from
Open

sec(oidc): switch JWT signing from PKCS1v15 to ES256 #882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions internal/api/handler_oidc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,10 @@ func TestHandleOIDCJWKS(t *testing.T) {
t.Fatalf("keys=%d want 1", len(jwks.Keys))
}
k := jwks.Keys[0]
if k.Kty != "RSA" || k.Alg != "RS256" || k.Kid == "" || k.N == "" {
t.Errorf("jwk malformed: %+v", k)
// ES256: EC key with P-256 curve. Positively assert the algorithm
// change -- regression to RSA/RS256 must not pass.
if k.Kty != "EC" || k.Alg != "ES256" || k.Crv != "P-256" || k.Kid == "" || k.X == "" || k.Y == "" {
t.Errorf("jwk malformed (want EC/ES256/P-256): %+v", k)
}
}

Expand Down
25 changes: 13 additions & 12 deletions internal/oidc/aws_signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ package oidc

import (
"context"
"crypto/rsa"
"crypto"
"crypto/ecdsa"
"crypto/x509"
"fmt"
"sync"
Expand All @@ -26,14 +27,15 @@ type AWSKMSSigner struct {
keyID string

once sync.Once
pubKey *rsa.PublicKey
pubKey *ecdsa.PublicKey
kid string
err error
}

// NewAWSKMSSigner constructs a signer bound to the given KMS key. The
// keyID may be the key ARN, alias ARN, or alias name — anything
// accepted by kms:Sign and kms:GetPublicKey.
// keyID may be the key ARN, alias ARN, or alias name -- anything
// accepted by kms:Sign and kms:GetPublicKey. The KMS key must be an
// ECC_NIST_P256 key configured for signing (ES256).
func NewAWSKMSSigner(ctx context.Context, keyID string) (*AWSKMSSigner, error) {
if keyID == "" {
return nil, fmt.Errorf("oidc: empty AWS KMS keyID")
Expand All @@ -52,14 +54,13 @@ func NewAWSKMSSignerFromClient(client AWSKMSClient, keyID string) *AWSKMSSigner
}

// Sign calls kms:Sign with the raw SHA-256 digest and the signing
// algorithm RSASSA_PKCS1_V1_5_SHA_256, which matches what RS256 JWS
// signatures expect.
// algorithm ECDSA_SHA_256, which matches what ES256 JWS signatures expect.
func (s *AWSKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) {
out, err := s.client.Sign(ctx, &kms.SignInput{
KeyId: &s.keyID,
Message: digest,
MessageType: types.MessageTypeDigest,
SigningAlgorithm: types.SigningAlgorithmSpecRsassaPkcs1V15Sha256,
SigningAlgorithm: types.SigningAlgorithmSpecEcdsaSha256,
})
if err != nil {
return nil, fmt.Errorf("oidc: kms:Sign: %w", err)
Expand All @@ -69,7 +70,7 @@ func (s *AWSKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error)

// PublicKey fetches the public half of the KMS key once and caches it.
// Subsequent calls return the cached value.
func (s *AWSKMSSigner) PublicKey(ctx context.Context) (*rsa.PublicKey, error) {
func (s *AWSKMSSigner) PublicKey(ctx context.Context) (crypto.PublicKey, error) {
s.resolveOnce(ctx)
return s.pubKey, s.err
}
Expand All @@ -92,17 +93,17 @@ func (s *AWSKMSSigner) resolveOnce(ctx context.Context) {
s.err = fmt.Errorf("oidc: parse kms public key: %w", err)
return
}
rsaPub, ok := pub.(*rsa.PublicKey)
ecPub, ok := pub.(*ecdsa.PublicKey)
if !ok {
s.err = fmt.Errorf("oidc: kms key is not RSA (got %T)", pub)
s.err = fmt.Errorf("oidc: kms key is not ECDSA (got %T); key must be ECC_NIST_P256", pub)
return
}
kid, err := ComputeKeyID(rsaPub)
kid, err := ComputeKeyID(ecPub)
if err != nil {
s.err = err
return
}
s.pubKey = rsaPub
s.pubKey = ecPub
s.kid = kid
})
}
37 changes: 21 additions & 16 deletions internal/oidc/aws_signer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package oidc

import (
"context"
"crypto"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
"crypto/rsa"
"crypto/sha256"
"crypto/x509"
"encoding/base64"
Expand All @@ -15,15 +15,15 @@ import (

var base64RawURL = base64.RawURLEncoding

// fakeKMSClient is a minimal AWSKMSClient backed by an in-process RSA
// key. It lets TestAWSKMSSigner exercise the Signer contract without
// fakeKMSClient is a minimal AWSKMSClient backed by an in-process P-256
// ECDSA key. It lets TestAWSKMSSigner exercise the Signer contract without
// touching real AWS.
type fakeKMSClient struct {
key *rsa.PrivateKey
key *ecdsa.PrivateKey
}

func (f *fakeKMSClient) Sign(_ context.Context, in *kms.SignInput, _ ...func(*kms.Options)) (*kms.SignOutput, error) {
sig, err := rsa.SignPKCS1v15(rand.Reader, f.key, crypto.SHA256, in.Message)
sig, err := ecdsa.SignASN1(rand.Reader, f.key, in.Message)
if err != nil {
return nil, err
}
Expand All @@ -40,19 +40,23 @@ func (f *fakeKMSClient) GetPublicKey(_ context.Context, _ *kms.GetPublicKeyInput

func TestAWSKMSSignerRoundTrip(t *testing.T) {
ctx := context.Background()
key, err := rsa.GenerateKey(rand.Reader, 2048)
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
t.Fatalf("gen key: %v", err)
}
signer := NewAWSKMSSignerFromClient(&fakeKMSClient{key: key}, "alias/test-key")

// Signer contract: PublicKey returns the RSA pub half.
pub, err := signer.PublicKey(ctx)
// Signer contract: PublicKey returns the ECDSA pub half.
rawPub, err := signer.PublicKey(ctx)
if err != nil {
t.Fatalf("public key: %v", err)
}
if pub.N.Cmp(key.PublicKey.N) != 0 {
t.Fatal("public key modulus mismatch")
ecPub, ok := rawPub.(*ecdsa.PublicKey)
if !ok {
t.Fatalf("public key is not *ecdsa.PublicKey, got %T", rawPub)
}
if ecPub.X.Cmp(key.PublicKey.X) != 0 || ecPub.Y.Cmp(key.PublicKey.Y) != 0 {
t.Fatal("public key point mismatch")
}

// KeyID stable across calls.
Expand All @@ -62,7 +66,7 @@ func TestAWSKMSSignerRoundTrip(t *testing.T) {
t.Errorf("kid unstable or empty: %s vs %s", k1, k2)
}

// Mint a JWT and verify the signature end-to-end.
// Mint a JWT and verify the ECDSA signature end-to-end.
jws, err := Mint(ctx, signer, map[string]any{
"iss": "https://cudly.example.com",
"sub": "cudly-controller",
Expand All @@ -71,16 +75,17 @@ func TestAWSKMSSignerRoundTrip(t *testing.T) {
if err != nil {
t.Fatalf("mint: %v", err)
}
// Verify using the underlying pub half.
// Verify header asserts ES256 algorithm.
parts := splitJWS(t, jws)
// Verify using the underlying EC pub half.
signingInput := parts[0] + "." + parts[1]
digest := sha256.Sum256([]byte(signingInput))
if err := rsa.VerifyPKCS1v15(pub, crypto.SHA256, digest[:], decodeB64(t, parts[2])); err != nil {
t.Errorf("signature verify: %v", err)
if !ecdsa.VerifyASN1(ecPub, digest[:], decodeB64(t, parts[2])) {
t.Errorf("ECDSA signature verify failed")
}
}

// helpers — unit tests only, kept private
// helpers -- unit tests only, kept private
func splitJWS(t *testing.T, jws string) [3]string {
t.Helper()
var out [3]string
Expand Down
35 changes: 19 additions & 16 deletions internal/oidc/azure_signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package oidc

import (
"context"
"crypto/rsa"
"crypto"
"crypto/ecdsa"
"crypto/elliptic"
"fmt"
"math/big"
"sync"
Expand All @@ -18,23 +20,23 @@ type AzureKeyVaultClient interface {
GetKey(ctx context.Context, name, version string, options *azkeys.GetKeyOptions) (azkeys.GetKeyResponse, error)
}

// AzureKeyVaultSigner signs JWTs via an Azure Key Vault RSA key. The
// AzureKeyVaultSigner signs JWTs via an Azure Key Vault EC key (P-256). The
// private half never leaves the vault.
type AzureKeyVaultSigner struct {
client AzureKeyVaultClient
keyName string
keyVersion string // may be empty = latest

once sync.Once
pubKey *rsa.PublicKey
pubKey *ecdsa.PublicKey
kid string
err error
}

// NewAzureKeyVaultSigner constructs a signer against a Key Vault using
// the standard azidentity default credential chain. vaultURL is the
// full vault URL (e.g. https://cudly-vault.vault.azure.net/); keyName
// is the name of the RSA key in that vault.
// is the name of the EC (P-256) key in that vault.
func NewAzureKeyVaultSigner(ctx context.Context, vaultURL, keyName string) (*AzureKeyVaultSigner, error) {
if vaultURL == "" || keyName == "" {
return nil, fmt.Errorf("oidc: azure key vault signer requires vaultURL + keyName")
Expand All @@ -57,10 +59,10 @@ func NewAzureKeyVaultSignerFromClient(client AzureKeyVaultClient, keyName, keyVe
return &AzureKeyVaultSigner{client: client, keyName: keyName, keyVersion: keyVersion}
}

// Sign calls Key Vault's Sign operation with RS256, passing the raw
// SHA-256 digest. Key Vault returns the raw RSA signature bytes.
// Sign calls Key Vault's Sign operation with ES256, passing the raw
// SHA-256 digest. Key Vault returns a DER-encoded ECDSA signature.
func (s *AzureKeyVaultSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) {
alg := azkeys.SignatureAlgorithmRS256
alg := azkeys.SignatureAlgorithmES256
resp, err := s.client.Sign(ctx, s.keyName, s.keyVersion, azkeys.SignParameters{
Algorithm: &alg,
Value: digest,
Expand All @@ -73,12 +75,12 @@ func (s *AzureKeyVaultSigner) Sign(ctx context.Context, digest []byte) ([]byte,

// PublicKey fetches the public half of the Key Vault key once and
// caches it.
func (s *AzureKeyVaultSigner) PublicKey(ctx context.Context) (*rsa.PublicKey, error) {
func (s *AzureKeyVaultSigner) PublicKey(ctx context.Context) (crypto.PublicKey, error) {
s.resolveOnce(ctx)
return s.pubKey, s.err
}

// KeyID returns a stable kid derived from the public key modulus.
// KeyID returns a stable kid derived from the public key point.
func (s *AzureKeyVaultSigner) KeyID(ctx context.Context) (string, error) {
s.resolveOnce(ctx)
return s.kid, s.err
Expand All @@ -91,20 +93,21 @@ func (s *AzureKeyVaultSigner) resolveOnce(ctx context.Context) {
s.err = fmt.Errorf("oidc: azure keyvault GetKey: %w", err)
return
}
if resp.Key == nil || resp.Key.N == nil || resp.Key.E == nil {
s.err = fmt.Errorf("oidc: azure keyvault returned incomplete key")
if resp.Key == nil || resp.Key.X == nil || resp.Key.Y == nil {
s.err = fmt.Errorf("oidc: azure keyvault returned incomplete EC key (missing X or Y)")
return
}
rsaPub := &rsa.PublicKey{
N: new(big.Int).SetBytes(resp.Key.N),
E: int(new(big.Int).SetBytes(resp.Key.E).Int64()),
ecPub := &ecdsa.PublicKey{
Curve: elliptic.P256(),
X: new(big.Int).SetBytes(resp.Key.X),
Y: new(big.Int).SetBytes(resp.Key.Y),
}
kid, err := ComputeKeyID(rsaPub)
kid, err := ComputeKeyID(ecPub)
if err != nil {
s.err = err
return
}
s.pubKey = rsaPub
s.pubKey = ecPub
s.kid = kid
})
}
22 changes: 12 additions & 10 deletions internal/oidc/gcp_signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ package oidc

import (
"context"
"crypto/rsa"
"crypto"
"crypto/ecdsa"
"crypto/x509"
"encoding/pem"
"fmt"
Expand Down Expand Up @@ -35,14 +36,14 @@ func (w gcpKMSWrapper) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKey
return w.real.GetPublicKey(ctx, req)
}

// GCPKMSSigner signs JWTs using a GCP Cloud KMS asymmetric key. The
// GCPKMSSigner signs JWTs using a GCP Cloud KMS asymmetric key (EC P-256). The
// private half never leaves the KMS.
type GCPKMSSigner struct {
client GCPKMSClient
keyResource string // full resource name, incl. /cryptoKeyVersions/N

once sync.Once
pubKey *rsa.PublicKey
pubKey *ecdsa.PublicKey
kid string
err error
}
Expand All @@ -51,6 +52,8 @@ type GCPKMSSigner struct {
// version resource. Example resource:
//
// projects/.../locations/global/keyRings/.../cryptoKeys/.../cryptoKeyVersions/1
//
// The key must be an EC_SIGN_P256_SHA256 key.
func NewGCPKMSSigner(ctx context.Context, keyResource string) (*GCPKMSSigner, error) {
if keyResource == "" {
return nil, fmt.Errorf("oidc: empty GCP KMS key resource")
Expand All @@ -69,8 +72,7 @@ func NewGCPKMSSignerFromClient(client GCPKMSClient, keyResource string) *GCPKMSS

// Sign calls AsymmetricSign with the SHA-256 digest. The caller must
// have already hashed the signing input; the digest is forwarded
// as-is with the expected algorithm
// RSA_SIGN_PKCS1_2048_SHA256 (implicit in the key config).
// as-is. The key must be configured as EC_SIGN_P256_SHA256 in GCP KMS.
func (s *GCPKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) {
crc := int64(crc32.Checksum(digest, crc32.MakeTable(crc32.Castagnoli)))
req := &kmspb.AsymmetricSignRequest{
Expand All @@ -88,7 +90,7 @@ func (s *GCPKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error)
}

// PublicKey fetches the public half of the KMS key once and caches it.
func (s *GCPKMSSigner) PublicKey(ctx context.Context) (*rsa.PublicKey, error) {
func (s *GCPKMSSigner) PublicKey(ctx context.Context) (crypto.PublicKey, error) {
s.resolveOnce(ctx)
return s.pubKey, s.err
}
Expand Down Expand Up @@ -116,17 +118,17 @@ func (s *GCPKMSSigner) resolveOnce(ctx context.Context) {
s.err = fmt.Errorf("oidc: parse gcp kms public key: %w", err)
return
}
rsaPub, ok := pub.(*rsa.PublicKey)
ecPub, ok := pub.(*ecdsa.PublicKey)
if !ok {
s.err = fmt.Errorf("oidc: gcp kms key is not RSA (got %T)", pub)
s.err = fmt.Errorf("oidc: gcp kms key is not ECDSA (got %T); key must be EC_SIGN_P256_SHA256", pub)
return
}
kid, err := ComputeKeyID(rsaPub)
kid, err := ComputeKeyID(ecPub)
if err != nil {
s.err = err
return
}
s.pubKey = rsaPub
s.pubKey = ecPub
s.kid = kid
})
}
Loading
Loading