From 7beebc3d95dc4f6ea216ee60a9d893dab11c384a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20He=C3=9Felmann?= Date: Thu, 23 Jul 2026 15:23:06 +0200 Subject: [PATCH 1/4] ROX-34488: discover cosign signatures via OCI 1.1 Referrers API Extract signature payload fetching into cosign_payload_fetcher.go and add referrer-based discovery alongside the existing tag-based method. FetchSignatures now queries both paths and merges results. Referrer errors are swallowed so registries without OCI 1.1 support keep working. This enables discovery of signatures created by cosign v3+, which defaults to storing signatures as OCI referrers instead of legacy tags. Partially generated by AI. --- pkg/images/enricher/testing_test.go | 2 +- pkg/signatures/cosign_payload_fetcher.go | 160 +++++++++++ pkg/signatures/cosign_payload_fetcher_test.go | 101 +++++++ pkg/signatures/cosign_sig_fetcher.go | 250 ++++++++++-------- pkg/signatures/cosign_sig_fetcher_test.go | 152 +++++++++++ pkg/signatures/factory.go | 17 +- 6 files changed, 563 insertions(+), 119 deletions(-) create mode 100644 pkg/signatures/cosign_payload_fetcher.go create mode 100644 pkg/signatures/cosign_payload_fetcher_test.go diff --git a/pkg/images/enricher/testing_test.go b/pkg/images/enricher/testing_test.go index b3d59ce4d58cc..dd8e3a7d5dc3f 100644 --- a/pkg/images/enricher/testing_test.go +++ b/pkg/images/enricher/testing_test.go @@ -56,7 +56,7 @@ type fakeSigFetcher struct { } func (f *fakeSigFetcher) FetchSignatures(_ context.Context, _ *storage.Image, _ string, - _ types.Registry) ([]*storage.Signature, error) { + _ types.Registry, _ ...retry.OptionsModifier) ([]*storage.Signature, error) { if f.fail { err := errors.New("some error") if f.retryable { diff --git a/pkg/signatures/cosign_payload_fetcher.go b/pkg/signatures/cosign_payload_fetcher.go new file mode 100644 index 0000000000000..07008a08d4c42 --- /dev/null +++ b/pkg/signatures/cosign_payload_fetcher.go @@ -0,0 +1,160 @@ +package signatures + +import ( + "context" + "errors" + "fmt" + "net/http" + "strings" + + "github.com/google/go-containerregistry/pkg/name" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/sigstore/cosign/v3/pkg/cosign" + "github.com/sigstore/cosign/v3/pkg/oci" + ociremote "github.com/sigstore/cosign/v3/pkg/oci/remote" + "github.com/stackrox/rox/generated/storage" + imgUtils "github.com/stackrox/rox/pkg/images/utils" +) + +const ( + bundleSigArtifactTypePrefix = "application/vnd.dev.sigstore.bundle" + + // maxReferrerManifests limits the number of referrer signature manifests processed per image + // to bound latency and prevent pathological cases from consuming the caller's timeout budget. + maxReferrerManifests = 50 +) + +// signaturePayload is the internal representation of a fetched cosign signature before +// it is persisted into the CosignSignature proto. Two formats are supported: +// +// - SimpleSigning: the legacy cosign format. The signature, payload, certificate, and +// rekor bundle are stored as individual fields in cosign.SignedPayload. +// - Sigstore bundle: the OCI 1.1 bundle format. The raw bundle JSON is stored as-is +// in sigstoreBundle and verified directly via sigstore-go at verification time. +// +// The format is determined by sigstoreBundle: non-empty means sigstore bundle, +// empty means SimpleSigning. +type signaturePayload struct { + cosign.SignedPayload + sigstoreBundle []byte +} + +var _ oci.SignedEntity = (*tagSignedEntity)(nil) + +// tagSignedEntity adapts an image reference for tag-based signature discovery. +// Signatures() constructs the cosign tag reference (-.sig) and fetches +// the signature manifest from the registry. Digest() returns the image digest so +// cosign can build the tag. Only these two methods are called by cosign.FetchSignatures. +// Attestations and Attachment return safe defaults to prevent panics if cosign evolves. +type tagSignedEntity struct { + opts []ociremote.Option + imgRef name.Reference + imgSHA string +} + +func newTagSignedEntity(img *storage.Image, imgRef name.Reference, opts ...ociremote.Option) *tagSignedEntity { + return &tagSignedEntity{ + opts: opts, + imgRef: imgRef, + imgSHA: imgUtils.GetSHA(img), + } +} + +func (s *tagSignedEntity) Digest() (v1.Hash, error) { + return v1.NewHash(s.imgSHA) +} + +func (s *tagSignedEntity) Signatures() (oci.Signatures, error) { + h, err := s.Digest() + if err != nil { + return nil, err + } + // Cosign ref: https://github.com/sigstore/cosign/blob/main/pkg/oci/remote/remote.go + return ociremote.Signatures(s.imgRef.Context().Tag(fmt.Sprint(h.Algorithm, "-", h.Hex, ".sig")), s.opts...) +} + +func (s *tagSignedEntity) Attestations() (oci.Signatures, error) { return nil, nil } + +func (s *tagSignedEntity) Attachment(_ string) (oci.File, error) { + return nil, errors.New("attachments not supported on tag-based signature entity") +} + +// fetchSignaturesByTag discovers SimpleSigning signatures via the cosign tag-based method. +// Discovery: looks up the tag -.sig in the same repository as the image. +// Format: always SimpleSigning (signature and payload stored in OCI layer annotations). +func fetchSignaturesByTag(image *storage.Image, imgRef name.Reference, opts []ociremote.Option) ([]signaturePayload, error) { + se := newTagSignedEntity(image, imgRef, opts...) + payloads, err := cosign.FetchSignatures(se) + if err != nil && (isMissingSignatureError(err) || isUnknownMimeTypeError(err)) { + return nil, nil + } + wrapped := make([]signaturePayload, len(payloads)) + for i, p := range payloads { + wrapped[i] = signaturePayload{SignedPayload: p} + } + return wrapped, err +} + +// fetchSignaturesByReferrer discovers sigstore bundle signatures via the OCI 1.1 Referrers API. +// Discovery: queries the referrers index for the image digest and filters for sigstore +// bundle artifact types. Only the sigstore bundle format is supported for referrer-based +// discovery — cosign v3 exclusively produces bundles for this path. +func fetchSignaturesByReferrer(ctx context.Context, digestRef name.Digest, repo name.Repository, + opts []ociremote.Option, +) ([]signaturePayload, error) { + index, err := ociremote.Referrers(digestRef, "", opts...) + if err != nil { + if checkIfErrorContainsCode(err, http.StatusNotFound) { + log.Warnf("OCI referrers API not supported for %s (404)", digestRef.String()) + return nil, nil + } + return nil, err + } + if index == nil { + return nil, nil + } + + manifests := index.Manifests + if len(manifests) > maxReferrerManifests { + log.Warnf("Image %s has %d referrer manifests, processing only first %d", + digestRef.String(), len(manifests), maxReferrerManifests) + manifests = manifests[:maxReferrerManifests] + } + + var payloads []signaturePayload + for _, desc := range manifests { + if ctx.Err() != nil { + break + } + if !strings.HasPrefix(desc.ArtifactType, bundleSigArtifactTypePrefix) { + continue + } + p, err := fetchSigstoreBundle(repo.Digest(desc.Digest.String()), opts) + if err != nil { + log.Warnf("Failed to fetch sigstore bundle from referrer %s: %v", desc.Digest, err) + continue + } + payloads = append(payloads, p...) + } + return payloads, nil +} + +// fetchSigstoreBundle fetches a sigstore bundle referrer and stores the raw JSON. +// Format: sigstore bundle (DSSE envelope + verification material + tlog entries in one blob). +// Storage: the raw bundle JSON is stored in SigstoreBundle; no decomposition into individual +// fields. Verification is deferred to verifySigstoreBundle which uses sigstore-go directly. +func fetchSigstoreBundle(bundleRef name.Reference, opts []ociremote.Option) ([]signaturePayload, error) { + b, err := ociremote.Bundle(bundleRef, opts...) + if err != nil { + return nil, err + } + + bundleJSON, err := b.MarshalJSON() + if err != nil { + return nil, fmt.Errorf("marshalling sigstore bundle: %w", err) + } + + return []signaturePayload{{ + sigstoreBundle: bundleJSON, + }}, nil +} diff --git a/pkg/signatures/cosign_payload_fetcher_test.go b/pkg/signatures/cosign_payload_fetcher_test.go new file mode 100644 index 0000000000000..2b80c17a58063 --- /dev/null +++ b/pkg/signatures/cosign_payload_fetcher_test.go @@ -0,0 +1,101 @@ +package signatures + +import ( + "context" + "encoding/base64" + "fmt" + "testing" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/sigstore/cosign/v3/pkg/oci/mutate" + ociremote "github.com/sigstore/cosign/v3/pkg/oci/remote" + "github.com/sigstore/cosign/v3/pkg/oci/static" + "github.com/stackrox/rox/pkg/images/types" + imgUtils "github.com/stackrox/rox/pkg/images/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// uploadSignatureAsReferrer stores a cosign signature as an OCI 1.1 referrer for the given image. +func uploadSignatureAsReferrer(imgRef string, b64Sig string, sigPayload, certPEM, chainPEM []byte) error { + sigOpts := []static.Option{static.WithCertChain(certPEM, chainPEM)} + + sig, err := static.NewSignature(sigPayload, b64Sig, sigOpts...) + if err != nil { + return err + } + + ref, err := name.ParseReference(imgRef) + if err != nil { + return err + } + d, ok := ref.(name.Digest) + if !ok { + return fmt.Errorf("could not cast reference %q to name.Digest", ref.String()) + } + + se, err := ociremote.SignedEntity(ref) + if err != nil { + return err + } + + newSE, err := mutate.AttachSignatureToEntity(se, sig) + if err != nil { + return err + } + + return ociremote.WriteSignaturesExperimentalOCI(d, newSE) +} + +func TestFetchTagPayloads_WithSignatures(t *testing.T) { + registryServer, imgRef, err := registryServerWithImage("nginx") + require.NoError(t, err, "setting up registry") + defer registryServer.Close() + + sigPayload, err := base64.StdEncoding.DecodeString(payload1) + require.NoError(t, err) + + require.NoError(t, uploadSignatureForImage(imgRef, sig1, sigPayload, nil, nil, nil), + "uploading tag-based signature") + + ref, err := name.ParseReference(imgRef) + require.NoError(t, err) + img, err := imgUtils.GenerateImageFromString(imgRef) + require.NoError(t, err) + + payloads, err := fetchSignaturesByTag(types.ToImage(img), ref, nil) + require.NoError(t, err) + assert.Len(t, payloads, 1) + assert.Equal(t, sigPayload, payloads[0].Payload) + assert.Equal(t, sig1, payloads[0].Base64Signature) +} + +func TestFetchTagPayloads_NoSignatures(t *testing.T) { + registryServer, imgRef, err := registryServerWithImage("nginx") + require.NoError(t, err, "setting up registry") + defer registryServer.Close() + + ref, err := name.ParseReference(imgRef) + require.NoError(t, err) + img, err := imgUtils.GenerateImageFromString(imgRef) + require.NoError(t, err) + + payloads, err := fetchSignaturesByTag(types.ToImage(img), ref, nil) + require.NoError(t, err) + assert.Empty(t, payloads) +} + +func TestFetchReferrerPayloads_NoSignatures(t *testing.T) { + registryServer, imgRef, err := registryServerWithImage("nginx") + require.NoError(t, err, "setting up registry") + defer registryServer.Close() + + ref, err := name.ParseReference(imgRef) + require.NoError(t, err) + d, ok := ref.(name.Digest) + require.True(t, ok) + + payloads, err := fetchSignaturesByReferrer(context.Background(), d, ref.Context(), nil) + require.NoError(t, err) + assert.Empty(t, payloads) +} diff --git a/pkg/signatures/cosign_sig_fetcher.go b/pkg/signatures/cosign_sig_fetcher.go index bf72a1942b6e8..8d80d25ea7944 100644 --- a/pkg/signatures/cosign_sig_fetcher.go +++ b/pkg/signatures/cosign_sig_fetcher.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "encoding/base64" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -13,13 +14,10 @@ import ( "time" "github.com/google/go-containerregistry/pkg/name" - v1 "github.com/google/go-containerregistry/pkg/v1" gcrRemote "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" dockerRegistry "github.com/heroku/docker-registry-client/registry" - "github.com/pkg/errors" "github.com/sigstore/cosign/v3/pkg/cosign" - "github.com/sigstore/cosign/v3/pkg/oci" ociremote "github.com/sigstore/cosign/v3/pkg/oci/remote" "github.com/sigstore/sigstore/pkg/cryptoutils" "github.com/stackrox/rox/generated/storage" @@ -28,9 +26,12 @@ import ( "github.com/stackrox/rox/pkg/protoutils" registryTypes "github.com/stackrox/rox/pkg/registries/types" "github.com/stackrox/rox/pkg/retry" + "github.com/stackrox/rox/pkg/sync" "golang.org/x/time/rate" ) +const fetchTimeout = 10 * time.Second + type cosignSignatureFetcher struct { // registryRateLimiter is a rate limiter for parallel calls to the registry. This will avoid reaching out to the // registry too many times leading to 429 errors. @@ -53,11 +54,15 @@ func init() { } // FetchSignatures implements the SignatureFetcher interface. -// The signature associated with the image will be fetched from the given registry. -// It will return the storage.ImageSignature and an error that indicated whether the fetching should be retried or not. -// NOTE: No error will be returned when the image has no signature available. All occurring errors will be logged. +// Signatures are discovered via two methods concurrently: the legacy cosign tag-based +// path and the OCI 1.1 Referrers API. Results from both are merged and deduplicated. +// When one path fails but the other returns signatures, the error is logged and the +// successful results are returned. When both paths fail, the joined error is returned. +// Unauthorized errors are wrapped as errox.NotAuthorized regardless of which path +// produced them. +// NOTE: No error will be returned when the image has no signature available. func (c *cosignSignatureFetcher) FetchSignatures(ctx context.Context, image *storage.Image, - fullImageName string, registry registryTypes.Registry, + fullImageName string, registry registryTypes.Registry, retryOpts ...retry.OptionsModifier, ) ([]*storage.Signature, error) { // Short-circuit for images that do not have V2 metadata associated with them. These would be older images manifest // schemes that are not supported by cosign, like the docker v1 manifest. @@ -75,85 +80,153 @@ func (c *cosignSignatureFetcher) FetchSignatures(ctx context.Context, image *sto // Wait until the registry rate limiter allows entrance. err = c.registryRateLimiter.Wait(ctx) if err != nil { - return nil, errors.Wrapf(err, "waiting for rate limiter entrance for registry %q", registry.Name()) + return nil, fmt.Errorf("waiting for rate limiter entrance for registry %q: %w", registry.Name(), err) } - // Fetch the signatures by injecting the registry specific authentication options to the google/go-containerregistry - // client. - // Additionally, use a local signed entity to skip fetching the image manifest and only fetch the signature manifest. - se := newLocalSignedEntity(image, imgRef, ociremote.WithRemoteOptions(optionsFromRegistry(ctx, registry)...)) - signedPayloads, err := cosign.FetchSignatures(se) + // Inject the registry specific authentication options to the google/go-containerregistry client. + remoteOpts := optionsFromRegistry(ctx, registry) + ociOpts := []ociremote.Option{ociremote.WithRemoteOptions(remoteOpts...)} + + // Fetch from both discovery methods concurrently. Each path retries independently + // so a transient failure in one does not block the other. + var ( + tagPayloads []signaturePayload + tagErr error + referrerPayloads []signaturePayload + referrerErr error + wg sync.WaitGroup + ) + wg.Go(func() { + tagPayloads, tagErr = fetchWithRetry(ctx, ociOpts, retryOpts, func(opts []ociremote.Option) ([]signaturePayload, error) { + return fetchSignaturesByTag(image, imgRef, opts) + }) + }) + wg.Go(func() { + imgSHA := imgUtils.GetSHA(image) + if imgSHA == "" { + referrerErr = fmt.Errorf("image %q has no SHA digest for referrer lookup", fullImageName) + return + } + digestRef := imgRef.Context().Digest(imgSHA) + referrerPayloads, referrerErr = fetchWithRetry(ctx, ociOpts, retryOpts, func(opts []ociremote.Option) ([]signaturePayload, error) { + return fetchSignaturesByReferrer(ctx, digestRef, imgRef.Context(), opts) + }) + }) + wg.Wait() + + // Merge payloads from both discovery methods. + var allPayloads []signaturePayload + if tagErr == nil { + allPayloads = append(allPayloads, tagPayloads...) + } else { + log.Warnf("Tag-based discovery failed for %q: %v", fullImageName, tagErr) + } + if referrerErr == nil { + allPayloads = append(allPayloads, referrerPayloads...) + } else { + log.Warnf("Referrer-based discovery failed for %q: %v", fullImageName, referrerErr) + } - // Cosign will return an error in case no signature is associated, we don't want to return that error. Since no - // error types are exposed need to check for string comparison. - // Cosign ref: - // https://github.com/sigstore/cosign/blob/44f3814667ba6a398aef62814cabc82aee4896e5/pkg/cosign/fetch.go#L84-L86 - if err != nil && !isMissingSignatureError(err) && !isUnknownMimeTypeError(err) { - // Specifically mark an error as errox.NotAuthorized so we skip using the same credentials for fetching. - // We can safely skip the potential marking of retryable errors as unauthorized errors are not transient. - if isUnauthorizedError(err) { - return nil, errox.NotAuthorized.CausedBy(err) + fetchErr := errors.Join(tagErr, referrerErr) + if fetchErr != nil && len(allPayloads) == 0 { + if isUnauthorizedError(tagErr) || isUnauthorizedError(referrerErr) { + return nil, errox.NotAuthorized.CausedBy(fetchErr) } - // Ensure we mark transient errors as retryable. - return nil, makeTransientErrorRetryable(err) + return nil, fetchErr + } + + if len(allPayloads) == 0 { + return nil, nil } - // Short-circuit if no signatures are associated with the image. - if len(signedPayloads) == 0 { + cosignSignatures := convertPayloadsToSignatures(allPayloads, fullImageName) + if len(cosignSignatures) == 0 { return nil, nil } - cosignSignatures := make([]*storage.Signature, 0, len(signedPayloads)) + return protoutils.SliceUnique(cosignSignatures), nil +} - for _, signedPayload := range signedPayloads { - rawSig, err := base64.StdEncoding.DecodeString(signedPayload.Base64Signature) - // We skip the invalid base64 signature and log its occurrence. - if err != nil { - log.Errorf("Error during decoding of raw signature for image %q: %v", - fullImageName, err) - continue +// fetchWithRetry calls fn, optionally wrapping it in retry logic with a per-attempt timeout. +// The timeout context is injected into the OCI options so fn does not need to handle contexts. +// When retryOpts is empty, fn is called directly without retry or timeout wrapping. +func fetchWithRetry(ctx context.Context, ociOpts []ociremote.Option, retryOpts []retry.OptionsModifier, + fn func([]ociremote.Option) ([]signaturePayload, error), +) ([]signaturePayload, error) { + if len(retryOpts) == 0 { + return fn(ociOpts) + } + var payloads []signaturePayload + err := retry.WithRetry(func() error { + if ctx.Err() != nil { + return ctx.Err() } + fetchCtx, cancel := context.WithTimeout(ctx, fetchTimeout) + defer cancel() + opts := append(slices.Clone(ociOpts), + ociremote.WithMoreRemoteOptions(gcrRemote.WithContext(fetchCtx))) + var fetchErr error + payloads, fetchErr = fn(opts) + return makeTransientErrorRetryable(fetchErr) + }, retryOpts...) + return payloads, err +} - certPEM, err := certificateFromSignedPayload(signedPayload) - if err != nil { - log.Errorf("Error during unmarshalling certificate to PEM for image %q: %v", fullImageName, err) - } +// convertPayloadsToSignatures converts fetched payloads into CosignSignature protos. +// Storage: sigstore bundles are stored as raw JSON in SigstoreBundle. SimpleSigning +// signatures are decomposed into individual proto fields (RawSignature, SignaturePayload, +// CertPem, CertChainPem, RekorBundle). Old images without SigstoreBundle fall back to +// the decomposed fields at verification time. +func convertPayloadsToSignatures(payloads []signaturePayload, fullImageName string) []*storage.Signature { + signatures := make([]*storage.Signature, 0, len(payloads)) + + for _, signedPayload := range payloads { + cosignSig := &storage.CosignSignature{} + + // DSSE bundle takes priority if it exists. + if len(signedPayload.sigstoreBundle) > 0 { + cosignSig.SignatureFormat = storage.CosignSignature_DEAD_SIMPLE_SIGNING_ENVELOPE + cosignSig.SigstoreBundle = signedPayload.sigstoreBundle + } else { + cosignSig.SignatureFormat = storage.CosignSignature_SIMPLE_SIGNING + cosignSig.SignaturePayload = signedPayload.Payload + rawSig, err := base64.StdEncoding.DecodeString(signedPayload.Base64Signature) + if err != nil { + log.Errorf("Error during decoding of raw signature for image %q: %v", + fullImageName, err) + continue + } + cosignSig.RawSignature = rawSig - chainPEM, err := certificateChainFromSignedPayload(signedPayload) - if err != nil { - log.Errorf("Error during unmarshalling certificate chain to PEM for image %q: %v", - fullImageName, err) - } + certPEM, err := certificateFromSignedPayload(signedPayload.SignedPayload) + if err != nil { + log.Errorf("Error during unmarshalling certificate to PEM for image %q: %v", fullImageName, err) + } + cosignSig.CertPem = certPEM - var rekorBundle []byte - if signedPayload.Bundle != nil { - rekorBundle, err = json.Marshal(signedPayload.Bundle) + chainPEM, err := certificateChainFromSignedPayload(signedPayload.SignedPayload) if err != nil { - log.Errorf("Error during marshalling rekor bundle for image %q: %v", fullImageName, err) + log.Errorf("Error during unmarshalling certificate chain to PEM for image %q: %v", + fullImageName, err) + } + cosignSig.CertChainPem = chainPEM + + if signedPayload.Bundle != nil { + rekorBundle, err := json.Marshal(signedPayload.Bundle) + if err != nil { + log.Errorf("Error during marshalling rekor bundle for image %q: %v", fullImageName, err) + } else { + cosignSig.RekorBundle = rekorBundle + } } } - // Since we are only focusing on public keys and certificates, we are ignoring the rekor bundles associated with - // the signature. - cosignSignatures = append(cosignSignatures, &storage.Signature{ - Signature: &storage.Signature_Cosign{ - Cosign: &storage.CosignSignature{ - RawSignature: rawSig, - SignaturePayload: signedPayload.Payload, - CertPem: certPEM, - CertChainPem: chainPEM, - RekorBundle: rekorBundle, - }, - }, + signatures = append(signatures, &storage.Signature{ + Signature: &storage.Signature_Cosign{Cosign: cosignSig}, }) } - // Since we are skipping invalid base64 signatures, need to check the length of the result. - if len(cosignSignatures) == 0 { - return nil, nil - } - - return protoutils.SliceUnique(cosignSignatures), nil + return signatures } func certificateFromSignedPayload(sp cosign.SignedPayload) ([]byte, error) { @@ -261,6 +334,9 @@ func isUnknownMimeTypeError(err error) bool { // isUnauthorizedError is checking whether the returned error indicates that there was a http.StatusUnauthorized was // returned during fetching of signatures. func isUnauthorizedError(err error) bool { + if err == nil { + return false + } return checkIfErrorContainsCode(err, http.StatusUnauthorized, http.StatusForbidden) } @@ -271,55 +347,15 @@ func isUnauthorizedError(err error) bool { // If the error is not matching any of these types or the code is not contained in the given codes, false will be // returned. func checkIfErrorContainsCode(err error, codes ...int) bool { - var transportErr *transport.Error - var statusError *dockerRegistry.HttpStatusError - // Transport error is returned by go-containerregistry for any errors occurred post authentication. - if errors.As(err, &transportErr) { + if transportErr, ok := errors.AsType[*transport.Error](err); ok { return slices.Index(codes, transportErr.StatusCode) != -1 } // HttpStatusError is returned by heroku-client for any errors occurred during authentication. - if errors.As(err, &statusError) && statusError.Response != nil { + if statusError, ok := errors.AsType[*dockerRegistry.HttpStatusError](err); ok && statusError.Response != nil { return slices.Index(codes, statusError.Response.StatusCode) != -1 } return false } - -var _ oci.SignedEntity = (*localSignedEntity)(nil) - -// localSignedEntity is an implementation of oci.SignedEntity used for fetching signatures. -// This implementation skips fetching the manifest of the signed image, since within the image enriching, we already -// fetched the image manifest beforehand. -type localSignedEntity struct { - oci.SignedEntity - opts []ociremote.Option - imgRef name.Reference - imgSHA string -} - -func newLocalSignedEntity(img *storage.Image, imgRef name.Reference, opts ...ociremote.Option) *localSignedEntity { - imgSHA := imgUtils.GetSHA(img) - return &localSignedEntity{ - opts: opts, - imgRef: imgRef, - imgSHA: imgSHA, - } -} - -func (s *localSignedEntity) Digest() (v1.Hash, error) { - return v1.NewHash(s.imgSHA) -} - -func (s *localSignedEntity) Signatures() (oci.Signatures, error) { - h, err := s.Digest() - if err != nil { - return nil, err - } - // The name reference of the signature to fetch is going to be: - // /@.sig - // This is being kept in line with: - // https://github.com/sigstore/cosign/blob/65eb28af970d133adeefdc6c48d6e9304dd8cc3a/pkg/oci/remote/remote.go#L87 - return ociremote.Signatures(s.imgRef.Context().Tag(fmt.Sprint(h.Algorithm, "-", h.Hex, ".sig")), s.opts...) -} diff --git a/pkg/signatures/cosign_sig_fetcher_test.go b/pkg/signatures/cosign_sig_fetcher_test.go index 3309dc6390d38..8df87d350091e 100644 --- a/pkg/signatures/cosign_sig_fetcher_test.go +++ b/pkg/signatures/cosign_sig_fetcher_test.go @@ -19,6 +19,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote/transport" dockerRegistry "github.com/heroku/docker-registry-client/registry" "github.com/pkg/errors" + "github.com/sigstore/cosign/v3/pkg/cosign" "github.com/sigstore/cosign/v3/pkg/oci/mutate" ociremote "github.com/sigstore/cosign/v3/pkg/oci/remote" "github.com/sigstore/cosign/v3/pkg/oci/static" @@ -202,6 +203,7 @@ func TestCosignSignatureFetcher_FetchSignature_Success(t *testing.T) { Cosign: &storage.CosignSignature{ RawSignature: rawSig1, SignaturePayload: sigPayload1, + SignatureFormat: storage.CosignSignature_SIMPLE_SIGNING, }, }, }, @@ -210,6 +212,7 @@ func TestCosignSignatureFetcher_FetchSignature_Success(t *testing.T) { Cosign: &storage.CosignSignature{ RawSignature: rawSig2, SignaturePayload: sigPayload2, + SignatureFormat: storage.CosignSignature_SIMPLE_SIGNING, }, }, }, @@ -220,6 +223,7 @@ func TestCosignSignatureFetcher_FetchSignature_Success(t *testing.T) { SignaturePayload: sigPayload3, CertPem: certPEM, CertChainPem: chainPEM, + SignatureFormat: storage.CosignSignature_SIMPLE_SIGNING, }, }, }, @@ -231,6 +235,7 @@ func TestCosignSignatureFetcher_FetchSignature_Success(t *testing.T) { CertPem: certPEMWithTlog, CertChainPem: chainPEMWithTlog, RekorBundle: bundle, + SignatureFormat: storage.CosignSignature_SIMPLE_SIGNING, }, }, }, @@ -287,6 +292,32 @@ func TestCosignSignatureFetcher_FetchSignature_NoSignature(t *testing.T) { assert.Nil(t, result) } +func TestCosignSignatureFetcher_FetchSignature_SimpleSigningReferrerIgnored(t *testing.T) { + registryServer, imgRef, err := registryServerWithImage("nginx") + require.NoError(t, err, "setting up registry") + defer registryServer.Close() + + cimg, err := imgUtils.GenerateImageFromString(imgRef) + require.NoError(t, err, "creating test image") + img := types.ToImage(cimg) + img.Metadata = &storage.ImageMetadata{V2: &storage.V2Metadata{Digest: "something"}} + + sigPayload1, err := base64.StdEncoding.DecodeString(payload1) + require.NoError(t, err, "decoding payload") + + // Upload a SimpleSigning signature as OCI referrer. The referrer path only + // supports sigstore bundles, so this signature should be ignored. + require.NoError(t, uploadSignatureAsReferrer(imgRef, sig1, sigPayload1, nil, nil), + "uploading signature as referrer") + + f := newCosignSignatureFetcher() + reg := &mockRegistry{cfg: ®istryTypes.Config{}} + + res, err := f.FetchSignatures(context.Background(), img, img.GetName().GetFullName(), reg) + assert.NoError(t, err) + assert.Empty(t, res) +} + func TestIsMissingSignatureError(t *testing.T) { notFoundErr := dockerRegistry.HttpStatusError{ Response: &http.Response{ @@ -496,3 +527,124 @@ func TestIsUnknownMimeTypeError(t *testing.T) { }) } } + +func TestConvertPayloadsToSignatures_SigstoreBundle(t *testing.T) { + bundleJSON := []byte(`{"mediaType":"application/vnd.dev.sigstore.bundle.v0.3+json"}`) + payloads := []signaturePayload{{sigstoreBundle: bundleJSON}} + + sigs := convertPayloadsToSignatures(payloads, "test:latest") + require.Len(t, sigs, 1) + cosignSig := sigs[0].GetCosign() + assert.Equal(t, bundleJSON, cosignSig.GetSigstoreBundle()) + assert.Equal(t, storage.CosignSignature_DEAD_SIMPLE_SIGNING_ENVELOPE, cosignSig.GetSignatureFormat()) + assert.Empty(t, cosignSig.GetRawSignature()) + assert.Empty(t, cosignSig.GetSignaturePayload()) +} + +func TestConvertPayloadsToSignatures_SimpleSigning(t *testing.T) { + sigPayload, err := base64.StdEncoding.DecodeString(payload1) + require.NoError(t, err) + payloads := []signaturePayload{ + {SignedPayload: cosign.SignedPayload{Base64Signature: sig1, Payload: sigPayload}}, + } + + sigs := convertPayloadsToSignatures(payloads, "test:latest") + require.Len(t, sigs, 1) + cosignSig := sigs[0].GetCosign() + assert.Empty(t, cosignSig.GetSigstoreBundle()) + assert.Equal(t, storage.CosignSignature_SIMPLE_SIGNING, cosignSig.GetSignatureFormat()) + assert.Equal(t, sigPayload, cosignSig.GetSignaturePayload()) + assert.NotEmpty(t, cosignSig.GetRawSignature()) +} + +func TestConvertPayloadsToSignatures_MixedFormats(t *testing.T) { + sigPayload, err := base64.StdEncoding.DecodeString(payload1) + require.NoError(t, err) + bundleJSON := []byte(`{"mediaType":"application/vnd.dev.sigstore.bundle.v0.3+json"}`) + payloads := []signaturePayload{ + {SignedPayload: cosign.SignedPayload{Base64Signature: sig1, Payload: sigPayload}}, + {sigstoreBundle: bundleJSON}, + } + + sigs := convertPayloadsToSignatures(payloads, "test:latest") + require.Len(t, sigs, 2) + assert.Empty(t, sigs[0].GetCosign().GetSigstoreBundle()) + assert.Equal(t, bundleJSON, sigs[1].GetCosign().GetSigstoreBundle()) +} + +func TestCosignSignatureFetcher_FetchSignature_NoV2MetadataSkipsBoth(t *testing.T) { + cimg, err := imgUtils.GenerateImageFromString("test.io/test:latest") + require.NoError(t, err) + img := types.ToImage(cimg) + // No V2 metadata set — should short-circuit before any registry call. + + f := newCosignSignatureFetcher() + res, err := f.FetchSignatures(context.Background(), img, img.GetName().GetFullName(), nil) + assert.NoError(t, err) + assert.Nil(t, res) +} + +func TestCosignSignatureFetcher_FetchSignature_NoSignatureFromEither(t *testing.T) { + registryServer, imgRef, err := registryServerWithImage("nginx") + require.NoError(t, err, "setting up registry") + defer registryServer.Close() + + cimg, err := imgUtils.GenerateImageFromString(imgRef) + require.NoError(t, err, "creating test image") + img := types.ToImage(cimg) + img.Metadata = &storage.ImageMetadata{V2: &storage.V2Metadata{Digest: "something"}} + + f := newCosignSignatureFetcher() + reg := &mockRegistry{cfg: ®istryTypes.Config{}} + + result, err := f.FetchSignatures(context.Background(), img, img.GetName().GetFullName(), reg) + assert.NoError(t, err) + assert.Nil(t, result) +} + +func TestCosignSignatureFetcher_FetchSignature_WithCredentials(t *testing.T) { + registryServer, imgRef, err := registryServerWithImage("nginx") + require.NoError(t, err, "setting up registry") + defer registryServer.Close() + + cimg, err := imgUtils.GenerateImageFromString(imgRef) + require.NoError(t, err, "creating test image") + img := types.ToImage(cimg) + img.Metadata = &storage.ImageMetadata{V2: &storage.V2Metadata{Digest: "something"}} + + rawSig1, err := base64.StdEncoding.DecodeString(sig1) + require.NoError(t, err, "decoding signature") + sigPayload1, err := base64.StdEncoding.DecodeString(payload1) + require.NoError(t, err, "decoding payload") + + require.NoError(t, uploadSignatureForImage(imgRef, sig1, sigPayload1, nil, nil, nil), + "uploading tag-based signature") + + expectedSignatures := []*storage.Signature{ + { + Signature: &storage.Signature_Cosign{ + Cosign: &storage.CosignSignature{ + RawSignature: rawSig1, + SignaturePayload: sigPayload1, + SignatureFormat: storage.CosignSignature_SIMPLE_SIGNING, + }, + }, + }, + } + + ref, err := name.ParseReference(imgRef) + require.NoError(t, err) + + f := newCosignSignatureFetcher() + // Use a registry config with credentials to exercise the authenticated transport path. + // The URL is required for WrapTransport to perform the initial /v2/ probe. + reg := &mockRegistry{cfg: ®istryTypes.Config{ + Username: "testuser", + Password: "testpass", + URL: "http://" + ref.Context().RegistryStr(), + }} + + res, err := f.FetchSignatures(context.Background(), img, img.GetName().GetFullName(), reg) + assert.NoError(t, err) + protoassert.SlicesEqual(t, expectedSignatures, res) +} diff --git a/pkg/signatures/factory.go b/pkg/signatures/factory.go index f6cd1a00b88a4..3b11e23a5dbd1 100644 --- a/pkg/signatures/factory.go +++ b/pkg/signatures/factory.go @@ -27,7 +27,8 @@ type SignatureVerifier interface { // SignatureFetcher is responsible for fetching raw signatures supporting multiple specific signature formats. type SignatureFetcher interface { - FetchSignatures(ctx context.Context, image *storage.Image, fullImageName string, registry registryTypes.Registry) ([]*storage.Signature, error) + FetchSignatures(ctx context.Context, image *storage.Image, fullImageName string, + registry registryTypes.Registry, retryOpts ...retry.OptionsModifier) ([]*storage.Signature, error) } // NewSignatureVerifier creates a new signature verifier capable of verifying signatures against the provided config. @@ -121,20 +122,14 @@ func FetchImageSignaturesWithRetries(ctx context.Context, fetcher SignatureFetch return nil, nil } - var fetchedSignatures []*storage.Signature - var err error - err = retry.WithRetry(func() error { - sigFetchCtx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - fetchedSignatures, err = fetcher.FetchSignatures(sigFetchCtx, image, fullImageName, registry) - return err - }, + retryOpts := []retry.OptionsModifier{ retry.WithContext(ctx), retry.Tries(5), retry.OnlyRetryableErrors(), retry.BetweenAttempts(func(_ int) { time.Sleep(500 * time.Millisecond) - })) + }), + } - return fetchedSignatures, err + return fetcher.FetchSignatures(ctx, image, fullImageName, registry, retryOpts...) } From 72c9613da1bfddb76f96046729b56f266334cbc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20He=C3=9Felmann?= Date: Tue, 28 Jul 2026 18:58:25 +0200 Subject: [PATCH 2/4] ROX-34488: address review feedback on referrer discovery - Pass imgSHA string instead of *storage.Image to fetchSignaturesByTag and newTagSignedEntity since only the SHA is used. - Filter referrer manifests by artifact type before clamping to the 50-manifest budget so non-sigstore referrers don't consume the limit. - Return single signaturePayload from fetchSigstoreBundle instead of a slice since it always produces exactly one payload. - Add comment explaining that 404 from Referrers API means the registry does not support OCI 1.1. - Consolidate TestConvertPayloadsToSignatures into one table-driven test. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/signatures/cosign_payload_fetcher.go | 45 +++++----- pkg/signatures/cosign_payload_fetcher_test.go | 4 +- pkg/signatures/cosign_sig_fetcher.go | 5 +- pkg/signatures/cosign_sig_fetcher_test.go | 84 +++++++++++-------- 4 files changed, 75 insertions(+), 63 deletions(-) diff --git a/pkg/signatures/cosign_payload_fetcher.go b/pkg/signatures/cosign_payload_fetcher.go index 07008a08d4c42..54c275c95e87e 100644 --- a/pkg/signatures/cosign_payload_fetcher.go +++ b/pkg/signatures/cosign_payload_fetcher.go @@ -12,8 +12,6 @@ import ( "github.com/sigstore/cosign/v3/pkg/cosign" "github.com/sigstore/cosign/v3/pkg/oci" ociremote "github.com/sigstore/cosign/v3/pkg/oci/remote" - "github.com/stackrox/rox/generated/storage" - imgUtils "github.com/stackrox/rox/pkg/images/utils" ) const ( @@ -52,11 +50,11 @@ type tagSignedEntity struct { imgSHA string } -func newTagSignedEntity(img *storage.Image, imgRef name.Reference, opts ...ociremote.Option) *tagSignedEntity { +func newTagSignedEntity(imgSHA string, imgRef name.Reference, opts ...ociremote.Option) *tagSignedEntity { return &tagSignedEntity{ opts: opts, imgRef: imgRef, - imgSHA: imgUtils.GetSHA(img), + imgSHA: imgSHA, } } @@ -82,8 +80,8 @@ func (s *tagSignedEntity) Attachment(_ string) (oci.File, error) { // fetchSignaturesByTag discovers SimpleSigning signatures via the cosign tag-based method. // Discovery: looks up the tag -.sig in the same repository as the image. // Format: always SimpleSigning (signature and payload stored in OCI layer annotations). -func fetchSignaturesByTag(image *storage.Image, imgRef name.Reference, opts []ociremote.Option) ([]signaturePayload, error) { - se := newTagSignedEntity(image, imgRef, opts...) +func fetchSignaturesByTag(imgSHA string, imgRef name.Reference, opts []ociremote.Option) ([]signaturePayload, error) { + se := newTagSignedEntity(imgSHA, imgRef, opts...) payloads, err := cosign.FetchSignatures(se) if err != nil && (isMissingSignatureError(err) || isUnknownMimeTypeError(err)) { return nil, nil @@ -104,6 +102,7 @@ func fetchSignaturesByReferrer(ctx context.Context, digestRef name.Digest, repo ) ([]signaturePayload, error) { index, err := ociremote.Referrers(digestRef, "", opts...) if err != nil { + // Registries that do not implement the OCI 1.1 Referrers API return 404. if checkIfErrorContainsCode(err, http.StatusNotFound) { log.Warnf("OCI referrers API not supported for %s (404)", digestRef.String()) return nil, nil @@ -114,27 +113,31 @@ func fetchSignaturesByReferrer(ctx context.Context, digestRef name.Digest, repo return nil, nil } - manifests := index.Manifests - if len(manifests) > maxReferrerManifests { - log.Warnf("Image %s has %d referrer manifests, processing only first %d", - digestRef.String(), len(manifests), maxReferrerManifests) - manifests = manifests[:maxReferrerManifests] + // Filter for sigstore bundle artifact types first, then clamp so the budget is + // consumed only by manifests we are willing to process. + var bundleDescs []v1.Descriptor + for _, desc := range index.Manifests { + if strings.HasPrefix(desc.ArtifactType, bundleSigArtifactTypePrefix) { + bundleDescs = append(bundleDescs, desc) + } + } + if len(bundleDescs) > maxReferrerManifests { + log.Warnf("Image %s has %d sigstore bundle referrers, processing only first %d", + digestRef.String(), len(bundleDescs), maxReferrerManifests) + bundleDescs = bundleDescs[:maxReferrerManifests] } var payloads []signaturePayload - for _, desc := range manifests { + for _, desc := range bundleDescs { if ctx.Err() != nil { break } - if !strings.HasPrefix(desc.ArtifactType, bundleSigArtifactTypePrefix) { - continue - } p, err := fetchSigstoreBundle(repo.Digest(desc.Digest.String()), opts) if err != nil { log.Warnf("Failed to fetch sigstore bundle from referrer %s: %v", desc.Digest, err) continue } - payloads = append(payloads, p...) + payloads = append(payloads, p) } return payloads, nil } @@ -143,18 +146,16 @@ func fetchSignaturesByReferrer(ctx context.Context, digestRef name.Digest, repo // Format: sigstore bundle (DSSE envelope + verification material + tlog entries in one blob). // Storage: the raw bundle JSON is stored in SigstoreBundle; no decomposition into individual // fields. Verification is deferred to verifySigstoreBundle which uses sigstore-go directly. -func fetchSigstoreBundle(bundleRef name.Reference, opts []ociremote.Option) ([]signaturePayload, error) { +func fetchSigstoreBundle(bundleRef name.Reference, opts []ociremote.Option) (signaturePayload, error) { b, err := ociremote.Bundle(bundleRef, opts...) if err != nil { - return nil, err + return signaturePayload{}, err } bundleJSON, err := b.MarshalJSON() if err != nil { - return nil, fmt.Errorf("marshalling sigstore bundle: %w", err) + return signaturePayload{}, fmt.Errorf("marshalling sigstore bundle: %w", err) } - return []signaturePayload{{ - sigstoreBundle: bundleJSON, - }}, nil + return signaturePayload{sigstoreBundle: bundleJSON}, nil } diff --git a/pkg/signatures/cosign_payload_fetcher_test.go b/pkg/signatures/cosign_payload_fetcher_test.go index 2b80c17a58063..50a3b38deb039 100644 --- a/pkg/signatures/cosign_payload_fetcher_test.go +++ b/pkg/signatures/cosign_payload_fetcher_test.go @@ -63,7 +63,7 @@ func TestFetchTagPayloads_WithSignatures(t *testing.T) { img, err := imgUtils.GenerateImageFromString(imgRef) require.NoError(t, err) - payloads, err := fetchSignaturesByTag(types.ToImage(img), ref, nil) + payloads, err := fetchSignaturesByTag(imgUtils.GetSHA(types.ToImage(img)), ref, nil) require.NoError(t, err) assert.Len(t, payloads, 1) assert.Equal(t, sigPayload, payloads[0].Payload) @@ -80,7 +80,7 @@ func TestFetchTagPayloads_NoSignatures(t *testing.T) { img, err := imgUtils.GenerateImageFromString(imgRef) require.NoError(t, err) - payloads, err := fetchSignaturesByTag(types.ToImage(img), ref, nil) + payloads, err := fetchSignaturesByTag(imgUtils.GetSHA(types.ToImage(img)), ref, nil) require.NoError(t, err) assert.Empty(t, payloads) } diff --git a/pkg/signatures/cosign_sig_fetcher.go b/pkg/signatures/cosign_sig_fetcher.go index 8d80d25ea7944..5f867decf6557 100644 --- a/pkg/signatures/cosign_sig_fetcher.go +++ b/pkg/signatures/cosign_sig_fetcher.go @@ -87,6 +87,8 @@ func (c *cosignSignatureFetcher) FetchSignatures(ctx context.Context, image *sto remoteOpts := optionsFromRegistry(ctx, registry) ociOpts := []ociremote.Option{ociremote.WithRemoteOptions(remoteOpts...)} + imgSHA := imgUtils.GetSHA(image) + // Fetch from both discovery methods concurrently. Each path retries independently // so a transient failure in one does not block the other. var ( @@ -98,11 +100,10 @@ func (c *cosignSignatureFetcher) FetchSignatures(ctx context.Context, image *sto ) wg.Go(func() { tagPayloads, tagErr = fetchWithRetry(ctx, ociOpts, retryOpts, func(opts []ociremote.Option) ([]signaturePayload, error) { - return fetchSignaturesByTag(image, imgRef, opts) + return fetchSignaturesByTag(imgSHA, imgRef, opts) }) }) wg.Go(func() { - imgSHA := imgUtils.GetSHA(image) if imgSHA == "" { referrerErr = fmt.Errorf("image %q has no SHA digest for referrer lookup", fullImageName) return diff --git a/pkg/signatures/cosign_sig_fetcher_test.go b/pkg/signatures/cosign_sig_fetcher_test.go index 8df87d350091e..9f80b79ada534 100644 --- a/pkg/signatures/cosign_sig_fetcher_test.go +++ b/pkg/signatures/cosign_sig_fetcher_test.go @@ -528,48 +528,58 @@ func TestIsUnknownMimeTypeError(t *testing.T) { } } -func TestConvertPayloadsToSignatures_SigstoreBundle(t *testing.T) { - bundleJSON := []byte(`{"mediaType":"application/vnd.dev.sigstore.bundle.v0.3+json"}`) - payloads := []signaturePayload{{sigstoreBundle: bundleJSON}} - - sigs := convertPayloadsToSignatures(payloads, "test:latest") - require.Len(t, sigs, 1) - cosignSig := sigs[0].GetCosign() - assert.Equal(t, bundleJSON, cosignSig.GetSigstoreBundle()) - assert.Equal(t, storage.CosignSignature_DEAD_SIMPLE_SIGNING_ENVELOPE, cosignSig.GetSignatureFormat()) - assert.Empty(t, cosignSig.GetRawSignature()) - assert.Empty(t, cosignSig.GetSignaturePayload()) -} - -func TestConvertPayloadsToSignatures_SimpleSigning(t *testing.T) { - sigPayload, err := base64.StdEncoding.DecodeString(payload1) - require.NoError(t, err) - payloads := []signaturePayload{ - {SignedPayload: cosign.SignedPayload{Base64Signature: sig1, Payload: sigPayload}}, - } - - sigs := convertPayloadsToSignatures(payloads, "test:latest") - require.Len(t, sigs, 1) - cosignSig := sigs[0].GetCosign() - assert.Empty(t, cosignSig.GetSigstoreBundle()) - assert.Equal(t, storage.CosignSignature_SIMPLE_SIGNING, cosignSig.GetSignatureFormat()) - assert.Equal(t, sigPayload, cosignSig.GetSignaturePayload()) - assert.NotEmpty(t, cosignSig.GetRawSignature()) -} - -func TestConvertPayloadsToSignatures_MixedFormats(t *testing.T) { +func TestConvertPayloadsToSignatures(t *testing.T) { sigPayload, err := base64.StdEncoding.DecodeString(payload1) require.NoError(t, err) bundleJSON := []byte(`{"mediaType":"application/vnd.dev.sigstore.bundle.v0.3+json"}`) - payloads := []signaturePayload{ - {SignedPayload: cosign.SignedPayload{Base64Signature: sig1, Payload: sigPayload}}, - {sigstoreBundle: bundleJSON}, + + cases := map[string]struct { + payloads []signaturePayload + assert func(t *testing.T, sigs []*storage.Signature) + }{ + "sigstore bundle": { + payloads: []signaturePayload{{sigstoreBundle: bundleJSON}}, + assert: func(t *testing.T, sigs []*storage.Signature) { + require.Len(t, sigs, 1) + cosignSig := sigs[0].GetCosign() + assert.Equal(t, bundleJSON, cosignSig.GetSigstoreBundle()) + assert.Equal(t, storage.CosignSignature_DEAD_SIMPLE_SIGNING_ENVELOPE, cosignSig.GetSignatureFormat()) + assert.Empty(t, cosignSig.GetRawSignature()) + assert.Empty(t, cosignSig.GetSignaturePayload()) + }, + }, + "simple signing": { + payloads: []signaturePayload{ + {SignedPayload: cosign.SignedPayload{Base64Signature: sig1, Payload: sigPayload}}, + }, + assert: func(t *testing.T, sigs []*storage.Signature) { + require.Len(t, sigs, 1) + cosignSig := sigs[0].GetCosign() + assert.Empty(t, cosignSig.GetSigstoreBundle()) + assert.Equal(t, storage.CosignSignature_SIMPLE_SIGNING, cosignSig.GetSignatureFormat()) + assert.Equal(t, sigPayload, cosignSig.GetSignaturePayload()) + assert.NotEmpty(t, cosignSig.GetRawSignature()) + }, + }, + "mixed formats": { + payloads: []signaturePayload{ + {SignedPayload: cosign.SignedPayload{Base64Signature: sig1, Payload: sigPayload}}, + {sigstoreBundle: bundleJSON}, + }, + assert: func(t *testing.T, sigs []*storage.Signature) { + require.Len(t, sigs, 2) + assert.Empty(t, sigs[0].GetCosign().GetSigstoreBundle()) + assert.Equal(t, bundleJSON, sigs[1].GetCosign().GetSigstoreBundle()) + }, + }, } - sigs := convertPayloadsToSignatures(payloads, "test:latest") - require.Len(t, sigs, 2) - assert.Empty(t, sigs[0].GetCosign().GetSigstoreBundle()) - assert.Equal(t, bundleJSON, sigs[1].GetCosign().GetSigstoreBundle()) + for name, c := range cases { + t.Run(name, func(t *testing.T) { + sigs := convertPayloadsToSignatures(c.payloads, "test:latest") + c.assert(t, sigs) + }) + } } func TestCosignSignatureFetcher_FetchSignature_NoV2MetadataSkipsBoth(t *testing.T) { From 416063531d38433b1af6e10468be7c7e77ba629f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20He=C3=9Felmann?= Date: Tue, 28 Jul 2026 19:03:40 +0200 Subject: [PATCH 3/4] ROX-34488: return nil from Attachment() for consistency Both Attestations() and Attachment() on tagSignedEntity are unused safe defaults for the oci.SignedEntity interface. Return nil, nil from both for consistency. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/signatures/cosign_payload_fetcher.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/signatures/cosign_payload_fetcher.go b/pkg/signatures/cosign_payload_fetcher.go index 54c275c95e87e..1290328faa039 100644 --- a/pkg/signatures/cosign_payload_fetcher.go +++ b/pkg/signatures/cosign_payload_fetcher.go @@ -2,7 +2,6 @@ package signatures import ( "context" - "errors" "fmt" "net/http" "strings" @@ -73,9 +72,7 @@ func (s *tagSignedEntity) Signatures() (oci.Signatures, error) { func (s *tagSignedEntity) Attestations() (oci.Signatures, error) { return nil, nil } -func (s *tagSignedEntity) Attachment(_ string) (oci.File, error) { - return nil, errors.New("attachments not supported on tag-based signature entity") -} +func (s *tagSignedEntity) Attachment(_ string) (oci.File, error) { return nil, nil } // fetchSignaturesByTag discovers SimpleSigning signatures via the cosign tag-based method. // Discovery: looks up the tag -.sig in the same repository as the image. From be1fb7cd034e2385f9f69c199343493c175c4a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stephan=20He=C3=9Felmann?= Date: Thu, 30 Jul 2026 09:43:12 +0200 Subject: [PATCH 4/4] ROX-34488: propagate referrer errors and fix dual-path merge logic Propagate bundle-fetch and context-cancellation errors from the referrer loop so fetchWithRetry can retry incomplete scans. Fix FetchSignatures merge logic to determine path success from absence of error rather than signature count, so referrer errors are ignored when tag discovery succeeds. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/signatures/cosign_payload_fetcher.go | 5 ++--- pkg/signatures/cosign_sig_fetcher.go | 8 +++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/signatures/cosign_payload_fetcher.go b/pkg/signatures/cosign_payload_fetcher.go index 1290328faa039..2ef5e4dc4d5fd 100644 --- a/pkg/signatures/cosign_payload_fetcher.go +++ b/pkg/signatures/cosign_payload_fetcher.go @@ -127,12 +127,11 @@ func fetchSignaturesByReferrer(ctx context.Context, digestRef name.Digest, repo var payloads []signaturePayload for _, desc := range bundleDescs { if ctx.Err() != nil { - break + return payloads, ctx.Err() } p, err := fetchSigstoreBundle(repo.Digest(desc.Digest.String()), opts) if err != nil { - log.Warnf("Failed to fetch sigstore bundle from referrer %s: %v", desc.Digest, err) - continue + return payloads, fmt.Errorf("fetching sigstore bundle from referrer %s: %w", desc.Digest, err) } payloads = append(payloads, p) } diff --git a/pkg/signatures/cosign_sig_fetcher.go b/pkg/signatures/cosign_sig_fetcher.go index 5f867decf6557..ae3ac21bffd84 100644 --- a/pkg/signatures/cosign_sig_fetcher.go +++ b/pkg/signatures/cosign_sig_fetcher.go @@ -115,7 +115,8 @@ func (c *cosignSignatureFetcher) FetchSignatures(ctx context.Context, image *sto }) wg.Wait() - // Merge payloads from both discovery methods. + // Merge payloads from both discovery methods. A path is successful when it + // returns no error, regardless of whether it found signatures. var allPayloads []signaturePayload if tagErr == nil { allPayloads = append(allPayloads, tagPayloads...) @@ -128,8 +129,9 @@ func (c *cosignSignatureFetcher) FetchSignatures(ctx context.Context, image *sto log.Warnf("Referrer-based discovery failed for %q: %v", fullImageName, referrerErr) } - fetchErr := errors.Join(tagErr, referrerErr) - if fetchErr != nil && len(allPayloads) == 0 { + // Return an error only when both discovery paths failed. + if tagErr != nil && referrerErr != nil { + fetchErr := errors.Join(tagErr, referrerErr) if isUnauthorizedError(tagErr) || isUnauthorizedError(referrerErr) { return nil, errox.NotAuthorized.CausedBy(fetchErr) }