Skip to content

Commit

Permalink
Fix: Create a static copy of signatures as part of verification.
Browse files Browse the repository at this point in the history
🐛 As part of verifying signatures and annotations create copies of what we are downloading to avoid later calls to `Payload()` redownloading the signature blob.

Examining trace data for policy-controller downstream, I observed that the same signature blob data was being fetched three times as part of verification.  I believe for attestations this may be even worse because we have an
additional call to `Payload()` where we parse out the contents of the attestation's predicate for policy evaluation.

This change eagerly fetches the signature metadata and stores it in a clone via `static.Copy`.  This clone is what we verify, and if it fails verification is it immediately discarded.  However, if it passes verification this clone is
returned as one of the "verified signatures" that the downstream logic can now access without refetching data from the registry.

/kind bug

Signed-off-by: Matt Moore <[email protected]>
  • Loading branch information
mattmoor committed Sep 27, 2022
1 parent d720f04 commit 17156ca
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pkg/cosign/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,11 @@ func verifySignatures(ctx context.Context, sigs oci.Signatures, h v1.Hash, co *C
validationErrs := []string{}

for _, sig := range sl {
sig, err := static.Copy(sig)
if err != nil {
validationErrs = append(validationErrs, err.Error())
continue
}
verified, err := VerifyImageSignature(ctx, sig, h, co)
bundleVerified = bundleVerified || verified
if err != nil {
Expand Down Expand Up @@ -702,6 +707,11 @@ func verifyImageAttestations(ctx context.Context, atts oci.Signatures, h v1.Hash

validationErrs := []string{}
for _, att := range sl {
att, err := static.Copy(att)
if err != nil {
validationErrs = append(validationErrs, err.Error())
continue
}
if err := func(att oci.Signature) error {
verifier := co.SigVerifier
if verifier == nil {
Expand Down
52 changes: 52 additions & 0 deletions pkg/oci/static/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,58 @@ func NewAttestation(payload []byte, opts ...Option) (oci.Signature, error) {
return NewSignature(payload, "", opts...)
}

// Copy constructs a new oci.Signature from the provided one.
func Copy(sig oci.Signature) (oci.Signature, error) {
payload, err := sig.Payload()
if err != nil {
return nil, err
}
b64sig, err := sig.Base64Signature()
if err != nil {
return nil, err
}
var opts []Option

mt, err := sig.MediaType()
if err != nil {
return nil, err
}
opts = append(opts, WithLayerMediaType(mt))

ann, err := sig.Annotations()
if err != nil {
return nil, err
}
opts = append(opts, WithAnnotations(ann))

bundle, err := sig.Bundle()
if err != nil {
return nil, err
}
opts = append(opts, WithBundle(bundle))

cert, err := sig.Cert()
if err != nil {
return nil, err
}
if cert != nil {
rawCert, err := cryptoutils.MarshalCertificateToPEM(cert)
if err != nil {
return nil, err
}
chain, err := sig.Chain()
if err != nil {
return nil, err
}
rawChain, err := cryptoutils.MarshalCertificatesToPEM(chain)
if err != nil {
return nil, err
}
opts = append(opts, WithCertChain(rawCert, rawChain))
}
return NewSignature(payload, b64sig, opts...)
}

type staticLayer struct {
b []byte
b64sig string
Expand Down

0 comments on commit 17156ca

Please sign in to comment.