diff --git a/pkg/security/provenance/attach_rekor_test.go b/pkg/security/provenance/attach_rekor_test.go new file mode 100644 index 00000000..934fd753 --- /dev/null +++ b/pkg/security/provenance/attach_rekor_test.go @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) Simple Container + +package provenance + +import ( + "context" + "fmt" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + . "github.com/onsi/gomega" + + "github.com/simple-container-com/api/pkg/security/signing" +) + +// installFakeCosign puts a `cosign` stub first on PATH that fails the first +// failures invocations with a Rekor 409 entry conflict, then succeeds. +// Invocation count lives in a file so it survives across separate process +// executions. +func installFakeCosign(t *testing.T, failures int) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("fake-binary PATH harness is POSIX-shell only") + } + dir := t.TempDir() + counter := filepath.Join(dir, "calls") + + conflict := `Error: attaching provenance: cosign attest failed: [POST /api/v1/log/entries][409] ` + + `createLogEntryConflict {"code":409,"message":"an equivalent entry already exists in the ` + + `transparency log with UUID 108e9186e8c5677aed8837db0e5e2ae48507be504018ed6aeb9f8bba9d12f6a2"}` + + script := fmt.Sprintf("#!/bin/sh\n"+ + "n=$(cat %[1]s 2>/dev/null || echo 0)\n"+ + "n=$((n+1))\n"+ + "echo $n > %[1]s\n"+ + "if [ \"$n\" -le %[2]d ]; then\n"+ + " echo '%[3]s' >&2\n"+ + " exit 1\n"+ + "fi\n"+ + "echo 'tlog entry created with index: 987654'\n"+ + "exit 0\n", counter, failures, conflict) + + bin := filepath.Join(dir, "cosign") + Expect(os.WriteFile(bin, []byte(script), 0o755)).To(Succeed()) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + return counter +} + +func callCount(t *testing.T, counter string) string { + t.Helper() + data, err := os.ReadFile(counter) + if err != nil { + return "0" + } + return strings.TrimSpace(string(data)) +} + +func testStatement() *Statement { + predicate := []byte(`{"buildDefinition":{"buildType":"https://simple-container.com/build/v1"}}`) + return NewStatement(FormatSLSAV10, predicate, provTestImage, &Metadata{BuilderID: "sc"}) +} + +const provTestImage = "registry.example.com/team/app@sha256:e6ba56b60370949f74b515333ea56a827c1775002aa8b808e37797d1f4304309" + +func testProvAttacher() *Attacher { + return &Attacher{ + SigningConfig: &signing.Config{Enabled: true, Keyless: true, OIDCToken: "a.b.c"}, + Timeout: 30 * time.Second, + } +} + +// Provenance attest races the SBOM attest on the same digest, and was observed +// to hit the conflict first, so it needs the same tolerance. +func TestProvenanceAttach_RetriesRekorConflict(t *testing.T) { + RegisterTestingT(t) + + counter := installFakeCosign(t, 1) + + err := testProvAttacher().Attach(context.Background(), testStatement(), provTestImage) + + Expect(err).ToNot(HaveOccurred()) + Expect(callCount(t, counter)).To(Equal("2"), "conflict must trigger exactly one retry") +} + +func TestProvenanceAttach_PersistentConflictStillFails(t *testing.T) { + RegisterTestingT(t) + + counter := installFakeCosign(t, signing.MaxCosignAttempts+1) + + err := testProvAttacher().Attach(context.Background(), testStatement(), provTestImage) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cosign attest failed")) + Expect(callCount(t, counter)).To(Equal(fmt.Sprint(signing.MaxCosignAttempts))) +} + +func TestProvenanceAttach_NoRetryOnOtherErrors(t *testing.T) { + RegisterTestingT(t) + + dir := t.TempDir() + counter := filepath.Join(dir, "calls") + script := fmt.Sprintf("#!/bin/sh\n"+ + "n=$(cat %[1]s 2>/dev/null || echo 0)\n"+ + "echo $((n+1)) > %[1]s\n"+ + "echo 'Error: getting signer: retrieving cert: oidc: token expired' >&2\n"+ + "exit 1\n", counter) + bin := filepath.Join(dir, "cosign") + Expect(os.WriteFile(bin, []byte(script), 0o755)).To(Succeed()) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + err := testProvAttacher().Attach(context.Background(), testStatement(), provTestImage) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("token expired")) + Expect(callCount(t, counter)).To(Equal("1"), "a non-conflict error must fail fast") +} diff --git a/pkg/security/provenance/provenance.go b/pkg/security/provenance/provenance.go index 09d56cb1..8515982f 100644 --- a/pkg/security/provenance/provenance.go +++ b/pkg/security/provenance/provenance.go @@ -212,16 +212,21 @@ func (a *Attacher) Attach(ctx context.Context, statement *Statement, imageRef st args = append(args, a.buildSigningArgs()...) args = append(args, imageRef) - cmd := exec.CommandContext(timeoutCtx, "cosign", args...) - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - cmd.Env = append(os.Environ(), a.buildSigningEnv()...) - if err := cmd.Run(); err != nil { - return fmt.Errorf("cosign attest failed: %w (stderr: %s)", err, strings.TrimSpace(stderr.String())) - } - - return nil + // Retry a Rekor entry conflict on a fresh invocation; see + // signing.RetryOnRekorConflict. The command is rebuilt per attempt because an + // exec.Cmd cannot be run twice. + return signing.RetryOnRekorConflict("attest", func() (string, error) { + cmd := exec.CommandContext(timeoutCtx, "cosign", args...) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + cmd.Env = append(os.Environ(), a.buildSigningEnv()...) + if err := cmd.Run(); err != nil { + return stderr.String() + stdout.String(), + fmt.Errorf("cosign attest failed: %w (stderr: %s)", err, strings.TrimSpace(stderr.String())) + } + return "", nil + }) } // Verify verifies the provenance attestation and returns the decoded predicate. diff --git a/pkg/security/sbom/attacher.go b/pkg/security/sbom/attacher.go index e0e4bd33..9c2da9fe 100644 --- a/pkg/security/sbom/attacher.go +++ b/pkg/security/sbom/attacher.go @@ -60,21 +60,25 @@ func (a *Attacher) Attach(ctx context.Context, sbom *SBOM, image string) error { // Add image args = append(args, image) - cmd := exec.CommandContext(timeoutCtx, "cosign", args...) - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - // Set environment variables for signing - cmd.Env = append(os.Environ(), a.buildSigningEnv()...) - - // Execute cosign attest - if err := cmd.Run(); err != nil { - return fmt.Errorf("cosign attest failed: %w (stderr: %s)", err, stderr.String()) - } - - return nil + // Execute cosign attest, retrying a Rekor entry conflict on a fresh + // invocation. Building the command inside the closure keeps each attempt + // independent — an exec.Cmd cannot be run twice. + return signing.RetryOnRekorConflict("attest", func() (string, error) { + cmd := exec.CommandContext(timeoutCtx, "cosign", args...) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + // Set environment variables for signing + cmd.Env = append(os.Environ(), a.buildSigningEnv()...) + + if err := cmd.Run(); err != nil { + return stderr.String() + stdout.String(), + fmt.Errorf("cosign attest failed: %w (stderr: %s)", err, stderr.String()) + } + return "", nil + }) } // Verify verifies an SBOM attestation diff --git a/pkg/security/sbom/attacher_rekor_test.go b/pkg/security/sbom/attacher_rekor_test.go new file mode 100644 index 00000000..d5d871af --- /dev/null +++ b/pkg/security/sbom/attacher_rekor_test.go @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) Simple Container + +package sbom + +import ( + "context" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + . "github.com/onsi/gomega" + + "github.com/simple-container-com/api/pkg/security/signing" +) + +// installFakeCosign puts a `cosign` stub first on PATH that fails the first +// failures invocations with a Rekor 409 entry conflict, then succeeds. It counts +// invocations in a file so the count survives across separate process +// executions, and returns that path so the test can assert the attempt count. +func installFakeCosign(t *testing.T, failures int) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("fake-binary PATH harness is POSIX-shell only") + } + dir := t.TempDir() + counter := filepath.Join(dir, "calls") + + conflict := `Error: signing bundle: error signing bundle: [POST /api/v1/log/entries][409] ` + + `createLogEntryConflict {"code":409,"message":"an equivalent entry already exists in the ` + + `transparency log with UUID 108e9186e8c5677a2c45c17488e67ac4beb48541bb66419307a9718e2253460406"}` + + script := "#!/bin/sh\n" + + "n=$(cat " + counter + " 2>/dev/null || echo 0)\n" + + "n=$((n+1))\n" + + "echo $n > " + counter + "\n" + + "if [ \"$n\" -le " + itoa(failures) + " ]; then\n" + + " echo '" + conflict + "' >&2\n" + + " exit 1\n" + + "fi\n" + + "echo 'tlog entry created with index: 123456'\n" + + "exit 0\n" + + bin := filepath.Join(dir, "cosign") + Expect(os.WriteFile(bin, []byte(script), 0o755)).To(Succeed()) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + return counter +} + +func readCallCount(t *testing.T, counter string) string { + t.Helper() + data, err := os.ReadFile(counter) + if err != nil { + return "0" + } + return strings.TrimSpace(string(data)) +} + +func testAttacher() *Attacher { + return &Attacher{ + SigningConfig: &signing.Config{Enabled: true, Keyless: true, OIDCToken: "a.b.c"}, + Timeout: 30 * time.Second, + } +} + +const testImage = "registry.example.com/team/app@sha256:f7ed9277c480591d7ec36fe7da13e112b33d898b7687f9bcbcda5c214a242099" + +// A Rekor conflict on the first attest must not fail the deploy: a fresh keyless +// invocation mints a new ephemeral cert, so the replayed body differs. +func TestAttach_RetriesRekorConflict(t *testing.T) { + RegisterTestingT(t) + + counter := installFakeCosign(t, 1) + sbom := NewSBOM(FormatCycloneDXJSON, []byte(`{"bomFormat":"CycloneDX"}`), "sha256:f7ed9277", nil) + + err := testAttacher().Attach(context.Background(), sbom, testImage) + + Expect(err).ToNot(HaveOccurred()) + Expect(readCallCount(t, counter)).To(Equal("2"), "conflict must trigger exactly one retry") +} + +// Regression guard: before the fix a single conflict aborted `sc sbom attach`, +// which failed the Pulumi update even though the workload rollout had already +// completed. +func TestAttach_PersistentConflictStillFails(t *testing.T) { + RegisterTestingT(t) + + counter := installFakeCosign(t, signing.MaxCosignAttempts+1) + sbom := NewSBOM(FormatCycloneDXJSON, []byte(`{"bomFormat":"CycloneDX"}`), "sha256:f7ed9277", nil) + + err := testAttacher().Attach(context.Background(), sbom, testImage) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cosign attest failed")) + Expect(readCallCount(t, counter)).To(Equal(itoa(signing.MaxCosignAttempts))) +} + +func TestAttach_SucceedsWithoutConflict(t *testing.T) { + RegisterTestingT(t) + + counter := installFakeCosign(t, 0) + sbom := NewSBOM(FormatCycloneDXJSON, []byte(`{"bomFormat":"CycloneDX"}`), "sha256:f7ed9277", nil) + + err := testAttacher().Attach(context.Background(), sbom, testImage) + + Expect(err).ToNot(HaveOccurred()) + Expect(readCallCount(t, counter)).To(Equal("1"), "no conflict means no retry") +} diff --git a/pkg/security/signing/keybased_test.go b/pkg/security/signing/keybased_test.go index 17d12c55..ef002aa9 100644 --- a/pkg/security/signing/keybased_test.go +++ b/pkg/security/signing/keybased_test.go @@ -121,7 +121,7 @@ func TestKeyBasedSigner_Sign_GivesUpOnPersistentRekorConflict(t *testing.T) { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("createLogEntryConflict")) - Expect(calls).To(Equal(maxSignAttempts)) + Expect(calls).To(Equal(MaxCosignAttempts)) } func TestKeyBasedSigner_Sign_RetriesOnceOnTransientConflict(t *testing.T) { diff --git a/pkg/security/signing/keyless.go b/pkg/security/signing/keyless.go index 5a895360..6e37ebd1 100644 --- a/pkg/security/signing/keyless.go +++ b/pkg/security/signing/keyless.go @@ -17,35 +17,65 @@ import ( // execFn matches tools.ExecCommand; injectable for tests. type execFn func(ctx context.Context, name string, args []string, env []string, timeout time.Duration) (string, string, error) -// maxSignAttempts bounds the Rekor-conflict retry loop in runCosignSign. -const maxSignAttempts = 3 +// MaxCosignAttempts bounds every Rekor-conflict retry loop: `cosign sign` here +// and `cosign attest` in the sbom and provenance packages. +const MaxCosignAttempts = 3 -// isRekorConflict reports a Rekor createLogEntryConflict (HTTP 409) — an +// IsRekorConflict reports a Rekor createLogEntryConflict (HTTP 409) — an // identical entry already in the tlog, typically a cosign upload retry after // a client-side timeout whose first attempt succeeded server-side. -func isRekorConflict(output string) bool { +func IsRekorConflict(output string) bool { return strings.Contains(output, "createLogEntryConflict") || (strings.Contains(output, "409") && strings.Contains(output, "/api/v1/log/entries")) } -// runCosignSign retries the full `cosign sign` on Rekor entry conflicts (a -// fresh invocation can't conflict with itself). Deterministic keys reproduce -// the same signature and exhaust the loop — correct, since a tlog entry does -// not prove the signature reached the registry. Other errors fail fast. -func runCosignSign(ctx context.Context, exec execFn, args, env []string, timeout time.Duration) (string, error) { +// RetryOnRekorConflict runs attempt until it succeeds, fails for a reason other +// than a Rekor entry conflict, or exhausts MaxCosignAttempts. +// +// attempt must perform one complete cosign invocation and return the output to +// classify (stderr and stdout concatenated is fine) alongside its error. Each +// call has to be a fresh invocation: under keyless signing that mints a new +// ephemeral certificate, so the replayed Rekor body differs and the conflict +// clears. Deterministic keys reproduce the same signature and exhaust the loop, +// which is the correct outcome — a tlog entry does not prove the signature or +// attestation reached the registry, and cosign uploads to Rekor before it +// pushes to the registry. +// +// operation names the cosign subcommand for the retry warning, e.g. "attest". +func RetryOnRekorConflict(operation string, attempt func() (string, error)) error { var lastErr error - for attempt := 1; attempt <= maxSignAttempts; attempt++ { - stdout, stderr, err := exec(ctx, "cosign", args, env, timeout) + for i := 1; i <= MaxCosignAttempts; i++ { + output, err := attempt() if err == nil { - return stdout, nil + return nil + } + lastErr = err + if !IsRekorConflict(output) { + return lastErr } - lastErr = fmt.Errorf("cosign sign failed: %w\nStderr: %s\nStdout: %s", err, stderr, stdout) - if !isRekorConflict(stderr) && !isRekorConflict(stdout) { - return "", lastErr + fmt.Fprintf(os.Stderr, "Warning: Rekor transparency-log conflict on %s attempt %d/%d, retrying\n", + operation, i, MaxCosignAttempts) + } + return lastErr +} + +// runCosignSign retries the full `cosign sign` on Rekor entry conflicts. +// See RetryOnRekorConflict for why a retry — not a success — is the right +// response to a conflict. +func runCosignSign(ctx context.Context, exec execFn, args, env []string, timeout time.Duration) (string, error) { + var signed string + err := RetryOnRekorConflict("sign", func() (string, error) { + stdout, stderr, err := exec(ctx, "cosign", args, env, timeout) + if err == nil { + signed = stdout + return "", nil } - fmt.Fprintf(os.Stderr, "Warning: Rekor transparency-log conflict on sign attempt %d/%d, retrying\n", attempt, maxSignAttempts) + return stderr + stdout, fmt.Errorf("cosign sign failed: %w\nStderr: %s\nStdout: %s", err, stderr, stdout) + }) + if err != nil { + return "", err } - return "", lastErr + return signed, nil } // KeylessSigner implements keyless signing using OIDC tokens diff --git a/pkg/security/signing/keyless_test.go b/pkg/security/signing/keyless_test.go index 2c9ab843..846417c2 100644 --- a/pkg/security/signing/keyless_test.go +++ b/pkg/security/signing/keyless_test.go @@ -193,7 +193,7 @@ func TestIsRekorConflict(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { RegisterTestingT(t) - Expect(isRekorConflict(tt.output)).To(Equal(tt.want)) + Expect(IsRekorConflict(tt.output)).To(Equal(tt.want)) }) } } @@ -250,5 +250,5 @@ func TestKeylessSigner_Sign_GivesUpAfterMaxConflictAttempts(t *testing.T) { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("createLogEntryConflict")) - Expect(calls).To(Equal(maxSignAttempts)) + Expect(calls).To(Equal(MaxCosignAttempts)) } diff --git a/pkg/security/signing/rekor_retry_test.go b/pkg/security/signing/rekor_retry_test.go new file mode 100644 index 00000000..766bd02d --- /dev/null +++ b/pkg/security/signing/rekor_retry_test.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) Simple Container + +package signing + +import ( + "fmt" + "testing" + + . "github.com/onsi/gomega" +) + +// rekorConflictStderr reproduces the cosign stderr shape observed when several +// deploy jobs attest against the public-good Rekor instance at once and its +// upload retry replays a body that already landed. +const rekorConflictStderr = `Error: signing registry.example.com/team/worker@sha256:f7ed9277c480591d7ec36fe7da13e112b33d898b7687f9bcbcda5c214a242099: ` + + `signing bundle: error signing bundle: [POST /api/v1/log/entries][409] createLogEntryConflict ` + + `{"code":409,"message":"an equivalent entry already exists in the transparency log with UUID 108e9186e8c5677a2c45c17488e67ac4beb48541bb66419307a9718e225346040648ffdc7942792e"}` + +func TestRetryOnRekorConflict_SucceedsOnRetry(t *testing.T) { + RegisterTestingT(t) + + calls := 0 + err := RetryOnRekorConflict("attest", func() (string, error) { + calls++ + if calls == 1 { + return rekorConflictStderr, fmt.Errorf("exit status 1") + } + return "", nil + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(calls).To(Equal(2), "a conflict must trigger exactly one retry") +} + +func TestRetryOnRekorConflict_NoRetryOnOtherErrors(t *testing.T) { + RegisterTestingT(t) + + calls := 0 + err := RetryOnRekorConflict("attest", func() (string, error) { + calls++ + return "Error: GET https://registry.example.com/v2/: unexpected status 409", fmt.Errorf("exit status 1") + }) + + Expect(err).To(HaveOccurred()) + Expect(calls).To(Equal(1), "an unrelated 409 must not be retried") +} + +// A deterministic key reproduces the same signature, so every attempt replays an +// identical Rekor body. Exhausting the loop and surfacing the error is correct: +// cosign uploads to Rekor before it pushes to the registry, so a tlog conflict +// does not prove the attestation was attached. +func TestRetryOnRekorConflict_GivesUpAndReportsError(t *testing.T) { + RegisterTestingT(t) + + calls := 0 + err := RetryOnRekorConflict("attest", func() (string, error) { + calls++ + return rekorConflictStderr, fmt.Errorf("cosign attest failed: exit status 1") + }) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cosign attest failed")) + Expect(calls).To(Equal(MaxCosignAttempts)) +} + +func TestRetryOnRekorConflict_SucceedsFirstTry(t *testing.T) { + RegisterTestingT(t) + + calls := 0 + err := RetryOnRekorConflict("attest", func() (string, error) { + calls++ + return "", nil + }) + + Expect(err).ToNot(HaveOccurred()) + Expect(calls).To(Equal(1)) +}