Skip to content

Commit

Permalink
Fix: Remove an extra registry request from verification path. (#2285)
Browse files Browse the repository at this point in the history
🐛 Remove an extra registry request when supplied with a digest.

The current logic calls `getSignedEntity` which must determine the media type of the referenced entity in order to produce an appropriately types `SignedEntity`.  However, we are only interested in the attached signatures (or
attestations) and not the underlying structure, so when we are passed a digest, this roundtrip is entirely superfluous.

This changes this logic to explicitly resolve the digest (since verification wants to know the `v1.Hash`), which is free for digests.  Then we explicitly invoke `ociremote.Signatures()` on one of
`ociremote.{Signature,Attestation}Tag`, which is also free (since we always pass the digest).

/kind bug

Signed-off-by: Matt Moore <mattmoor@chainguard.dev>
  • Loading branch information
mattmoor committed Sep 27, 2022
1 parent f449fa8 commit 32f80c0
Showing 1 changed file with 26 additions and 23 deletions.
49 changes: 26 additions & 23 deletions pkg/cosign/verify.go
Expand Up @@ -118,19 +118,6 @@ type CheckOpts struct {
Identities []Identity
}

func getSignedEntity(signedImgRef name.Reference, regClientOpts []ociremote.Option) (oci.SignedEntity, v1.Hash, error) {
se, err := ociremote.SignedEntity(signedImgRef, regClientOpts...)
if err != nil {
return nil, v1.Hash{}, err
}
// Both of the SignedEntity types implement Digest()
h, err := se.(interface{ Digest() (v1.Hash, error) }).Digest()
if err != nil {
return nil, v1.Hash{}, err
}
return se, h, nil
}

func verifyOCISignature(ctx context.Context, verifier signature.Verifier, sig oci.Signature) error {
b64sig, err := sig.Base64Signature()
if err != nil {
Expand Down Expand Up @@ -427,17 +414,25 @@ func VerifyImageSignatures(ctx context.Context, signedImgRef name.Reference, co
return nil, false, errors.New("one of verifier or root certs is required")
}

// TODO(mattmoor): We could implement recursive verification if we just wrapped
// most of the logic below here in a call to mutate.Map
se, h, err := getSignedEntity(signedImgRef, co.RegistryClientOpts)
// This is a carefully optimized sequence for fetching the signatures of the
// entity that minimizes registry requests when supplied with a digest input
digest, err := ociremote.ResolveDigest(signedImgRef, co.RegistryClientOpts...)
if err != nil {
return nil, false, err
}
h, err := v1.NewHash(digest.Identifier())
if err != nil {
return nil, false, err
}

var sigs oci.Signatures
sigRef := co.SignatureRef
if sigRef == "" {
sigs, err = se.Signatures()
st, err := ociremote.SignatureTag(digest, co.RegistryClientOpts...)
if err != nil {
return nil, false, err
}
sigs, err = ociremote.Signatures(st, co.RegistryClientOpts...)
if err != nil {
return nil, false, err
}
Expand Down Expand Up @@ -625,22 +620,30 @@ func loadSignatureFromFile(sigRef string, signedImgRef name.Reference, co *Check
}, nil
}

// VerifyAttestations does all the main cosign checks in a loop, returning the verified attestations.
// VerifyImageAttestations does all the main cosign checks in a loop, returning the verified attestations.
// If there were no valid attestations, we return an error.
func VerifyImageAttestations(ctx context.Context, signedImgRef name.Reference, co *CheckOpts) (checkedAttestations []oci.Signature, bundleVerified bool, err error) {
// Enforce this up front.
if co.RootCerts == nil && co.SigVerifier == nil {
return nil, false, errors.New("one of verifier or root certs is required")
}

// TODO(mattmoor): We could implement recursive verification if we just wrapped
// most of the logic below here in a call to mutate.Map

se, h, err := getSignedEntity(signedImgRef, co.RegistryClientOpts)
// This is a carefully optimized sequence for fetching the attestations of
// the entity that minimizes registry requests when supplied with a digest
// input.
digest, err := ociremote.ResolveDigest(signedImgRef, co.RegistryClientOpts...)
if err != nil {
return nil, false, err
}
atts, err := se.Attestations()
h, err := v1.NewHash(digest.Identifier())
if err != nil {
return nil, false, err
}
st, err := ociremote.AttestationTag(digest, co.RegistryClientOpts...)
if err != nil {
return nil, false, err
}
atts, err := ociremote.Signatures(st, co.RegistryClientOpts...)
if err != nil {
return nil, false, err
}
Expand Down

0 comments on commit 32f80c0

Please sign in to comment.