Add --skip-att-sig-check flag #3267
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,6 +32,7 @@ import ( | |
| "github.com/santhosh-tekuri/jsonschema/v5" | ||
| "github.com/sigstore/cosign/v3/pkg/cosign" | ||
| cosignOCI "github.com/sigstore/cosign/v3/pkg/oci" | ||
| ociremote "github.com/sigstore/cosign/v3/pkg/oci/remote" | ||
| log "github.com/sirupsen/logrus" | ||
| "github.com/spf13/afero" | ||
|
|
||
|
|
@@ -201,9 +202,49 @@ func (a *ApplicationSnapshotImage) ValidateAttestationSignature(ctx context.Cont | |
| return a.parseAttestationsFromBundles(layers) | ||
| } | ||
|
|
||
| return a.parseAttestationsFromSignatures(layers) | ||
| } | ||
|
|
||
| // FetchAttestationsWithoutVerification fetches attestations from the registry | ||
| // without performing signature verification. This is used when --skip-att-sig-check | ||
| // is enabled but we still need the attestation data for policy evaluation. | ||
| func (a *ApplicationSnapshotImage) FetchAttestationsWithoutVerification(ctx context.Context) error { | ||
| if trace.IsEnabled() { | ||
| region := trace.StartRegion(ctx, "ec:fetch-attestations-without-verification") | ||
| defer region.End() | ||
| } | ||
|
|
||
| remoteOpts := oci.CreateRemoteOptions(ctx) | ||
| signedEntity, err := ociremote.SignedEntity(a.reference, ociremote.WithRemoteOptions(remoteOpts...)) | ||
|
Contributor
There was a problem hiding this comment. This calls |
||
| if err != nil { | ||
| return fmt.Errorf("failed to fetch signed entity: %w", err) | ||
| } | ||
|
|
||
| layers, err := signedEntity.Attestations() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to fetch attestations: %w", err) | ||
| } | ||
|
|
||
| // Check if using bundles | ||
| useBundles := a.hasBundles(ctx) | ||
| if useBundles { | ||
| sigs, err := layers.Get() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get attestation signatures: %w", err) | ||
| } | ||
| return a.parseAttestationsFromBundles(sigs) | ||
| } | ||
|
|
||
| sigs, err := layers.Get() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get attestation signatures: %w", err) | ||
| } | ||
|
|
||
|
Comment on lines
+231
to
+242
Contributor
There was a problem hiding this comment.
sigs, err := layers.Get()
if err != nil {
return fmt.Errorf("failed to get attestation signatures: %%w", err)
}
if useBundles {
return a.parseAttestationsFromBundles(sigs)
}
return a.parseAttestationsFromSignatures(sigs) |
||
| return a.parseAttestationsFromSignatures(sigs) | ||
| } | ||
|
Comment on lines
+211
to
+244
Contributor
There was a problem hiding this comment. This method has four error paths and a bundle/non-bundle branch, none tested at the unit level. The integration test does not mock this path either, so the fetch silently fails there. Routing through |
||
|
|
||
| func (a *ApplicationSnapshotImage) parseAttestationsFromSignatures(sigs []cosignOCI.Signature) error { | ||
|
Contributor
There was a problem hiding this comment.
|
||
| for _, sig := range sigs { | ||
| att, err := attestation.ProvenanceFromSignature(sig) | ||
| if err != nil { | ||
| return fmt.Errorf("unable to parse untyped provenance: %w", err) | ||
|
|
@@ -212,34 +253,27 @@ func (a *ApplicationSnapshotImage) ValidateAttestationSignature(ctx context.Cont | |
| log.Debugf("Found attestation with predicateType: %s", t) | ||
| switch t { | ||
| case attestation.PredicateSLSAProvenance: | ||
| // SLSAProvenanceFromSignature re-does the payload extraction from | ||
| // ProvenanceFromSignature. Could be deduplicated but not important. | ||
| sp, err := attestation.SLSAProvenanceFromSignature(sig) | ||
| if err != nil { | ||
| return fmt.Errorf("unable to parse as SLSA v0.2: %w", err) | ||
| } | ||
| a.attestations = append(a.attestations, sp) | ||
|
|
||
| case attestation.PredicateSLSAProvenanceV1: | ||
| sp, err := attestation.SLSAProvenanceFromSignatureV1(sig) | ||
| if err != nil { | ||
| return fmt.Errorf("unable to parse as SLSA v1: %w", err) | ||
| } | ||
| a.attestations = append(a.attestations, sp) | ||
|
|
||
| case attestation.PredicateSpdxDocument: | ||
| a.attestations = append(a.attestations, att) | ||
|
|
||
| // Todo: CycloneDX format SBOM | ||
|
|
||
| default: | ||
| a.attestations = append(a.attestations, att) | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attestation here is signed with the correct key, so the skip flag is redundant. The
--skip-image-sig-checkhas a companion scenario at line 214 with an invalid signature that proves the flag is needed. An equivalent scenario with a wrong attestation key would catch a regression that silently ignores this flag.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was being lazy here main to satisfy Codecov stats. 😅