From 7e8ebb9a20b4c3c927a548b82d49ce765b09fc4e Mon Sep 17 00:00:00 2001 From: Shubham Bhardwaj Date: Mon, 6 Apr 2026 18:31:15 +0530 Subject: [PATCH] fix: enable Vault JWT auth without Spire for KMS signing When signers.kms.auth.oidc.path and signers.kms.auth.oidc.role are set in the Chains ConfigMap without Spire, the controller now reads the Kubernetes service account token from the pod filesystem and uses it for Vault JWT auth login. Previously, the OIDC token field was only populated by Spire, causing the code to fall through to VAULT_TOKEN lookup and fail with "read .vault-token file: no such file or directory". Signed-off-by: Shubham Bhardwaj --- docs/config.md | 1 + pkg/chains/signing/kms/kms.go | 19 +++ pkg/chains/signing/kms/kms_test.go | 213 +++++++++++++++++++++++++++++ pkg/config/config.go | 9 +- pkg/config/config_test.go | 21 +++ 5 files changed, 260 insertions(+), 3 deletions(-) diff --git a/docs/config.md b/docs/config.md index a6badf7209..5049471415 100644 --- a/docs/config.md +++ b/docs/config.md @@ -177,6 +177,7 @@ chains.tekton.dev/transparency-upload: "true" | `signers.kms.auth.token-path` | Path to store KMS server Auth token (e.g. `/etc/kms-secrets`) | | | `signers.kms.auth.oidc.path` | Path used for OIDC authentication (e.g. `jwt` for Vault) | | | `signers.kms.auth.oidc.role` | Role used for OIDC authentication | | +| `signers.kms.auth.oidc.token-path`| Path to a file containing the JWT token for OIDC authentication. If not set, defaults to the Kubernetes service account token at `/var/run/secrets/kubernetes.io/serviceaccount/token`. | | `/var/run/secrets/kubernetes.io/serviceaccount/token` | | `signers.kms.auth.spire.sock` | URI of the Spire socket used for KMS token (e.g. `unix:///tmp/spire-agent/public/api.sock`) | | | `signers.kms.auth.spire.audience` | Audience for requesting a SVID from Spire | | diff --git a/pkg/chains/signing/kms/kms.go b/pkg/chains/signing/kms/kms.go index fb5845e046..377cbb1816 100644 --- a/pkg/chains/signing/kms/kms.go +++ b/pkg/chains/signing/kms/kms.go @@ -38,6 +38,8 @@ import ( "github.com/tektoncd/chains/pkg/chains/signing" ) +const defaultOIDCTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token" //nolint:gosec // Not a credential, this is the path to read the K8s SA token file from + // Signer exposes methods to sign payloads using a KMS type Signer struct { signature.SignerVerifier @@ -125,6 +127,23 @@ func NewSigner(ctx context.Context, cfg config.KMSSigner) (*Signer, error) { } rpcAuth.OIDC.Token = token } + + if rpcAuth.OIDC.Token == "" && rpcAuth.Token == "" && (cfg.Auth.OIDC.Path != "" || cfg.Auth.OIDC.Role != "") { + tokenPath := cfg.Auth.OIDC.TokenPath + if tokenPath == "" { + tokenPath = defaultOIDCTokenPath + } + token, err := os.ReadFile(tokenPath) + if err != nil { + return nil, fmt.Errorf("reading OIDC token from %q: %w", tokenPath, err) + } + oidcToken := strings.TrimSpace(string(token)) + if oidcToken == "" { + return nil, fmt.Errorf("OIDC token file %q is empty", tokenPath) + } + rpcAuth.OIDC.Token = oidcToken + } + kmsOpts = append(kmsOpts, options.WithRPCAuthOpts(rpcAuth)) // get the signer/verifier from sigstore k, err := kms.Get(ctx, cfg.KMSRef, crypto.SHA256, kmsOpts...) diff --git a/pkg/chains/signing/kms/kms_test.go b/pkg/chains/signing/kms/kms_test.go index 92d35d8d83..2bf36c586b 100644 --- a/pkg/chains/signing/kms/kms_test.go +++ b/pkg/chains/signing/kms/kms_test.go @@ -17,11 +17,14 @@ package kms import ( "context" + "encoding/json" "net" "net/http" "net/http/httptest" "os" + "sync" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/tektoncd/chains/pkg/config" @@ -126,6 +129,216 @@ func TestValidVaultAddressConnection(t *testing.T) { }) } +// TestOIDCTokenEndToEnd proves the full flow: JWT file → rpcAuth.OIDC.Token +// → ApplyRPCAuthOpts → oidcLogin → Vault HTTP request. +// A mock Vault server captures the login request and verifies the JWT, role, +// and auth path arrive exactly as configured. +func TestOIDCTokenEndToEnd(t *testing.T) { + var mu sync.Mutex + var loginCalled bool + var receivedJWT, receivedRole, receivedPath string + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + defer mu.Unlock() + + switch { + case r.URL.Path == "/v1/auth/jwt/login" && r.Method == http.MethodPut: + loginCalled = true + receivedPath = "jwt" + + var body map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&body); err == nil { + if v, ok := body["jwt"].(string); ok { + receivedJWT = v + } + if v, ok := body["role"].(string); ok { + receivedRole = v + } + } + + w.Header().Set("Content-Type", "application/json") + resp := `{"auth":{"client_token":"hvs.mock-vault-token","policies":["default"],"lease_duration":3600,"renewable":true}}` + w.Write([]byte(resp)) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + defer server.Close() + + tokenFile, err := os.CreateTemp("", "jwt-token") + if err != nil { + t.Fatalf("creating temp file: %v", err) + } + defer os.Remove(tokenFile.Name()) + + err = os.WriteFile(tokenFile.Name(), []byte("eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.test-payload\n"), 0644) + if err != nil { + t.Fatalf("writing temp file: %v", err) + } + + cfg := config.KMSSigner{ + KMSRef: "hashivault://supply-chain", + Auth: config.KMSAuth{ + Address: server.URL, + OIDC: config.KMSAuthOIDC{ + Path: "jwt", + Role: "tekton-chains", + TokenPath: tokenFile.Name(), + }, + }, + } + + signer, err := NewSigner(context.Background(), cfg) + + // The signer should be created successfully — oidcLogin exchanges the + // JWT for a Vault token, newHashivaultClient stores it, no transit API + // call happens during construction. + if err != nil { + t.Fatalf("NewSigner should succeed when OIDC login returns a valid token, got: %v", err) + } + if signer == nil { + t.Fatal("signer must not be nil") + } + + mu.Lock() + defer mu.Unlock() + assert.True(t, loginCalled, "Vault auth/jwt/login endpoint must have been called") + assert.Equal(t, "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.test-payload", receivedJWT, + "JWT sent to Vault must match the trimmed file contents") + assert.Equal(t, "tekton-chains", receivedRole, + "role sent to Vault must match the configured OIDC role") + assert.Equal(t, "jwt", receivedPath, + "auth path must match the configured OIDC path") +} + +// TestOIDCTokenFallbackToDefaultPath proves that when oidc.path/role are set +// but no token-path is given, the code tries the default K8s SA token path. +func TestOIDCTokenFallbackToDefaultPath(t *testing.T) { + cfg := config.KMSSigner{ + Auth: config.KMSAuth{ + OIDC: config.KMSAuthOIDC{ + Path: "jwt", + Role: "tekton-chains", + }, + }, + } + + _, err := NewSigner(context.Background(), cfg) + + if err == nil { + t.Fatal("expected error when default SA token path does not exist") + } + assert.Contains(t, err.Error(), "reading OIDC token") + assert.Contains(t, err.Error(), defaultOIDCTokenPath, + "error must reference the default K8s SA token path") +} + +// TestOIDCTokenSkippedWhenNotConfigured proves the OIDC block is not entered +// when neither oidc.path nor oidc.role are set. +func TestOIDCTokenSkippedWhenNotConfigured(t *testing.T) { + cfg := config.KMSSigner{} + + _, err := NewSigner(context.Background(), cfg) + + if err == nil { + t.Fatal("expected error when no KMS config is set") + } + assert.NotContains(t, err.Error(), "reading OIDC token", + "OIDC reading must not be attempted when OIDC is not configured") + assert.NotContains(t, err.Error(), "OIDC token file", + "OIDC empty-file check must not be reached when OIDC is not configured") +} + +// TestOIDCTokenSkippedWhenStaticTokenSet proves that the file-based OIDC +// token reading is skipped when a static Vault token is already set, even if +// oidc.path/oidc.role are configured. This prevents breaking existing users +// who have both a static token and leftover OIDC config. +func TestOIDCTokenSkippedWhenStaticTokenSet(t *testing.T) { + cfg := config.KMSSigner{ + Auth: config.KMSAuth{ + Token: "my-static-vault-token", + OIDC: config.KMSAuthOIDC{ + Path: "jwt", + Role: "tekton-chains", + }, + }, + } + + _, err := NewSigner(context.Background(), cfg) + + if err == nil { + t.Fatal("expected error (no KMSRef), but should NOT be an OIDC reading error") + } + assert.NotContains(t, err.Error(), "reading OIDC token", + "OIDC file reading must be skipped when a static token is set") + assert.NotContains(t, err.Error(), "OIDC token file", + "OIDC empty check must be skipped when a static token is set") +} + +// TestOIDCTokenEmptyFileErrors proves that an empty token file produces a +// clear error rather than silently breaking OIDC. +func TestOIDCTokenEmptyFileErrors(t *testing.T) { + tokenFile, err := os.CreateTemp("", "empty-jwt") + if err != nil { + t.Fatalf("creating temp file: %v", err) + } + defer os.Remove(tokenFile.Name()) + + cfg := config.KMSSigner{ + Auth: config.KMSAuth{ + OIDC: config.KMSAuthOIDC{ + Path: "jwt", + Role: "tekton-chains", + TokenPath: tokenFile.Name(), + }, + }, + } + + _, err = NewSigner(context.Background(), cfg) + + if err == nil { + t.Fatal("expected error for empty OIDC token file") + } + assert.Contains(t, err.Error(), "OIDC token file") + assert.Contains(t, err.Error(), "is empty") +} + +// TestOIDCTokenNotReadWhenSpireConfigured proves that the file-based OIDC +// token reading is skipped when Spire is configured (Spire takes precedence). +// The Spire gRPC client retries indefinitely, so we use a short-lived context +// to make it fail quickly and verify the error is about Spire, not file-based +// OIDC. +func TestOIDCTokenNotReadWhenSpireConfigured(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + + cfg := config.KMSSigner{ + Auth: config.KMSAuth{ + OIDC: config.KMSAuthOIDC{ + Path: "jwt", + Role: "tekton-chains", + }, + Spire: config.KMSAuthSpire{ + Sock: "unix:///tmp/nonexistent-spire.sock", + Audience: "test", + }, + }, + } + + _, err := NewSigner(ctx, cfg) + + if err == nil { + t.Fatal("expected error when Spire socket does not exist") + } + // The error should be about Spire/context, NOT about reading an OIDC + // token file — proving the Spire block runs and the file block is skipped. + assert.NotContains(t, err.Error(), "reading OIDC token", + "file-based OIDC reading must be skipped when Spire is configured") + assert.NotContains(t, err.Error(), "OIDC token file", + "file-based OIDC empty check must be skipped when Spire is configured") +} + // Test for getKMSAuthToken with non-directory path func TestGetKMSAuthToken_NotADirectory(t *testing.T) { tempFile, err := os.CreateTemp("", "not-a-dir") diff --git a/pkg/config/config.go b/pkg/config/config.go index c045d6b25b..5ad4c7239c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -101,8 +101,9 @@ type KMSAuth struct { // KMSAuthOIDC configures settings to authenticate with OIDC type KMSAuthOIDC struct { - Path string - Role string + Path string + Role string + TokenPath string } // KMSAuthSpire configures settings to get an auth token from spire @@ -204,8 +205,9 @@ const ( kmsAuthAddress = "signers.kms.auth.address" kmsAuthToken = "signers.kms.auth.token" kmsAuthOIDCPath = "signers.kms.auth.oidc.path" - kmsAuthTokenPath = "signers.kms.auth.token-path" // #nosec G101 kmsAuthOIDCRole = "signers.kms.auth.oidc.role" + kmsAuthOIDCTokenPath = "signers.kms.auth.oidc.token-path" // #nosec G101 + kmsAuthTokenPath = "signers.kms.auth.token-path" // #nosec G101 kmsAuthSpireSock = "signers.kms.auth.spire.sock" kmsAuthSpireAudience = "signers.kms.auth.spire.audience" @@ -335,6 +337,7 @@ func NewConfigFromMap(data map[string]string) (*Config, error) { asString(kmsAuthTokenPath, &cfg.Signers.KMS.Auth.TokenPath), asString(kmsAuthOIDCPath, &cfg.Signers.KMS.Auth.OIDC.Path), asString(kmsAuthOIDCRole, &cfg.Signers.KMS.Auth.OIDC.Role), + asString(kmsAuthOIDCTokenPath, &cfg.Signers.KMS.Auth.OIDC.TokenPath), asString(kmsAuthSpireSock, &cfg.Signers.KMS.Auth.Spire.Sock), asString(kmsAuthSpireAudience, &cfg.Signers.KMS.Auth.Spire.Audience), diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index fa54ec2e41..3b07a3e23b 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -85,3 +85,24 @@ func TestArtifact_Enabled(t *testing.T) { }) } } + +func TestNewConfigFromMap_KMSAuthOIDC(t *testing.T) { + data := map[string]string{ + "signers.kms.auth.oidc.path": "jwt", + "signers.kms.auth.oidc.role": "tekton-chains", + "signers.kms.auth.oidc.token-path": "/var/run/secrets/tokens/vault-token", + } + cfg, err := NewConfigFromMap(data) + if err != nil { + t.Fatalf("NewConfigFromMap() error = %v", err) + } + if cfg.Signers.KMS.Auth.OIDC.Path != "jwt" { + t.Errorf("OIDC.Path = %q, want %q", cfg.Signers.KMS.Auth.OIDC.Path, "jwt") + } + if cfg.Signers.KMS.Auth.OIDC.Role != "tekton-chains" { + t.Errorf("OIDC.Role = %q, want %q", cfg.Signers.KMS.Auth.OIDC.Role, "tekton-chains") + } + if cfg.Signers.KMS.Auth.OIDC.TokenPath != "/var/run/secrets/tokens/vault-token" { + t.Errorf("OIDC.TokenPath = %q, want %q", cfg.Signers.KMS.Auth.OIDC.TokenPath, "/var/run/secrets/tokens/vault-token") + } +}