Merge pull request #67 from evalops/codex/asb-verifier-security-tests

[codex] expand ASB verifier security tests

Author: haasonsaas

internal/authn/oidc/verifier.go

@@ -2,6 +2,9 @@ package oidc
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/ed25519"
+	"crypto/rsa"
 	"fmt"
 	"strconv"
 	"strings"
@@ -51,7 +54,14 @@ func (v *Verifier) Verify(ctx context.Context, in *core.Attestation) (*core.Work
 
 	claims := jwt.MapClaims{}
 	parsed, err := jwt.ParseWithClaims(in.Token, claims, func(token *jwt.Token) (any, error) {
-		return v.keyfunc(ctx, token)
+		key, err := v.keyfunc(ctx, token)
+		if err != nil {
+			return nil, err
+		}
+		if err := ensureSupportedSigningMethod(token, key); err != nil {
+			return nil, err
+		}
+		return key, nil
 	}, jwt.WithIssuer(v.issuer), jwt.WithAudience(v.audience))
 	if err != nil {
 		return nil, fmt.Errorf("%w: %v", core.ErrUnauthorized, err)
@@ -141,3 +151,22 @@ func copyNumericClaim(attributes map[string]string, claims jwt.MapClaims, key st
 		}
 	}
 }
+
+func ensureSupportedSigningMethod(token *jwt.Token, key any) error {
+	switch key.(type) {
+	case ed25519.PublicKey:
+		if token.Method == jwt.SigningMethodEdDSA {
+			return nil
+		}
+	case *rsa.PublicKey:
+		switch token.Method.(type) {
+		case *jwt.SigningMethodRSA, *jwt.SigningMethodRSAPSS:
+			return nil
+		}
+	case *ecdsa.PublicKey:
+		if _, ok := token.Method.(*jwt.SigningMethodECDSA); ok {
+			return nil
+		}
+	}
+	return fmt.Errorf("%w: unexpected signing method %q", core.ErrUnauthorized, token.Method.Alg())
+}

internal/authn/oidc/verifier_test.go

@@ -11,6 +11,23 @@ import (
 	"github.com/golang-jwt/jwt/v5"
 )
 
+func newTestVerifier(t *testing.T, publicKey ed25519.PublicKey, prefixes []string) *oidc.Verifier {
+	t.Helper()
+
+	verifier, err := oidc.NewVerifier(oidc.Config{
+		Issuer:                 "https://token.actions.githubusercontent.com",
+		Audience:               "asb-control-plane",
+		AllowedSubjectPrefixes: prefixes,
+		Keyfunc: func(context.Context, *jwt.Token) (any, error) {
+			return publicKey, nil
+		},
+	})
+	if err != nil {
+		t.Fatalf("NewVerifier() error = %v", err)
+	}
+	return verifier
+}
+
 func TestVerifier_VerifyGitHubActionsToken(t *testing.T) {
 	t.Parallel()
 
@@ -41,17 +58,7 @@ func TestVerifier_VerifyGitHubActionsToken(t *testing.T) {
 		t.Fatalf("SignedString() error = %v", err)
 	}
 
-	verifier, err := oidc.NewVerifier(oidc.Config{
-		Issuer:                 "https://token.actions.githubusercontent.com",
-		Audience:               "asb-control-plane",
-		AllowedSubjectPrefixes: []string{"repo:evalops/"},
-		Keyfunc: func(context.Context, *jwt.Token) (any, error) {
-			return publicKey, nil
-		},
-	})
-	if err != nil {
-		t.Fatalf("NewVerifier() error = %v", err)
-	}
+	verifier := newTestVerifier(t, publicKey, []string{"repo:evalops/"})
 
 	identity, err := verifier.Verify(context.Background(), &core.Attestation{
 		Kind:  core.AttestationKindOIDCJWT,
@@ -93,22 +100,128 @@ func TestVerifier_RejectsUnexpectedSubjectPrefix(t *testing.T) {
 		t.Fatalf("SignedString() error = %v", err)
 	}
 
-	verifier, err := oidc.NewVerifier(oidc.Config{
-		Issuer:                 "https://token.actions.githubusercontent.com",
-		Audience:               "asb-control-plane",
-		AllowedSubjectPrefixes: []string{"repo:evalops/"},
-		Keyfunc: func(context.Context, *jwt.Token) (any, error) {
-			return publicKey, nil
-		},
+	verifier := newTestVerifier(t, publicKey, []string{"repo:evalops/"})
+
+	if _, err := verifier.Verify(context.Background(), &core.Attestation{
+		Kind:  core.AttestationKindOIDCJWT,
+		Token: raw,
+	}); err == nil {
+		t.Fatal("Verify() error = nil, want non-nil")
+	}
+}
+
+func TestVerifier_AllowsAnySubjectWhenPrefixesUnset(t *testing.T) {
+	t.Parallel()
+
+	publicKey, privateKey, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatalf("GenerateKey() error = %v", err)
+	}
+
+	token := jwt.NewWithClaims(jwt.SigningMethodEdDSA, jwt.MapClaims{
+		"iss": "https://token.actions.githubusercontent.com",
+		"sub": "repo:other-org/repo:ref:refs/heads/main",
+		"aud": "asb-control-plane",
+		"exp": time.Now().Add(5 * time.Minute).Unix(),
+		"iat": time.Now().Add(-time.Minute).Unix(),
 	})
+	raw, err := token.SignedString(privateKey)
 	if err != nil {
-		t.Fatalf("NewVerifier() error = %v", err)
+		t.Fatalf("SignedString() error = %v", err)
+	}
+
+	verifier := newTestVerifier(t, publicKey, nil)
+	if _, err := verifier.Verify(context.Background(), &core.Attestation{
+		Kind:  core.AttestationKindOIDCJWT,
+		Token: raw,
+	}); err != nil {
+		t.Fatalf("Verify() error = %v", err)
+	}
+}
+
+func TestVerifier_RejectsExpiredToken(t *testing.T) {
+	t.Parallel()
+
+	publicKey, privateKey, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatalf("GenerateKey() error = %v", err)
+	}
+
+	token := jwt.NewWithClaims(jwt.SigningMethodEdDSA, jwt.MapClaims{
+		"iss": "https://token.actions.githubusercontent.com",
+		"sub": "repo:evalops/asb:ref:refs/heads/main",
+		"aud": "asb-control-plane",
+		"exp": time.Now().Add(-time.Minute).Unix(),
+		"iat": time.Now().Add(-2 * time.Minute).Unix(),
+	})
+	raw, err := token.SignedString(privateKey)
+	if err != nil {
+		t.Fatalf("SignedString() error = %v", err)
 	}
 
+	verifier := newTestVerifier(t, publicKey, []string{"repo:evalops/"})
 	if _, err := verifier.Verify(context.Background(), &core.Attestation{
 		Kind:  core.AttestationKindOIDCJWT,
 		Token: raw,
 	}); err == nil {
-		t.Fatal("Verify() error = nil, want non-nil")
+		t.Fatal("Verify() error = nil, want expired token failure")
+	}
+}
+
+func TestVerifier_RejectsTokenBeforeNotBefore(t *testing.T) {
+	t.Parallel()
+
+	publicKey, privateKey, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatalf("GenerateKey() error = %v", err)
+	}
+
+	token := jwt.NewWithClaims(jwt.SigningMethodEdDSA, jwt.MapClaims{
+		"iss": "https://token.actions.githubusercontent.com",
+		"sub": "repo:evalops/asb:ref:refs/heads/main",
+		"aud": "asb-control-plane",
+		"exp": time.Now().Add(5 * time.Minute).Unix(),
+		"iat": time.Now().Add(-time.Minute).Unix(),
+		"nbf": time.Now().Add(time.Minute).Unix(),
+	})
+	raw, err := token.SignedString(privateKey)
+	if err != nil {
+		t.Fatalf("SignedString() error = %v", err)
+	}
+
+	verifier := newTestVerifier(t, publicKey, []string{"repo:evalops/"})
+	if _, err := verifier.Verify(context.Background(), &core.Attestation{
+		Kind:  core.AttestationKindOIDCJWT,
+		Token: raw,
+	}); err == nil {
+		t.Fatal("Verify() error = nil, want not-before validation failure")
+	}
+}
+
+func TestVerifier_RejectsUnexpectedSigningMethod(t *testing.T) {
+	t.Parallel()
+
+	publicKey, _, err := ed25519.GenerateKey(nil)
+	if err != nil {
+		t.Fatalf("GenerateKey() error = %v", err)
+	}
+
+	token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
+		"iss": "https://token.actions.githubusercontent.com",
+		"sub": "repo:evalops/asb:ref:refs/heads/main",
+		"aud": "asb-control-plane",
+		"exp": time.Now().Add(5 * time.Minute).Unix(),
+	})
+	raw, err := token.SignedString([]byte("shared-secret"))
+	if err != nil {
+		t.Fatalf("SignedString() error = %v", err)
+	}
+
+	verifier := newTestVerifier(t, publicKey, []string{"repo:evalops/"})
+	if _, err := verifier.Verify(context.Background(), &core.Attestation{
+		Kind:  core.AttestationKindOIDCJWT,
+		Token: raw,
+	}); err == nil {
+		t.Fatal("Verify() error = nil, want signing-method failure")
 	}
 }