Skip to content
This repository was archived by the owner on Sep 23, 2025. It is now read-only.

Commit b15417d

Browse files
wlynchwlynch
committed
Generate unique PolicyResult.ID for the same image.
This changes how IDs are created to use the signature digest rather than layer digest. Because of how layers are created, this was actually a digest of the signed content, which means that multiple signatures over the same content would yield the same ID. Instead, this uses a checksum of the signature, which should be unique enough for both content + public key. Signed-off-by: Billy Lynch <billy@chainguard.dev>
1 parent bb9d59b commit b15417d

3 files changed

Lines changed: 101 additions & 12 deletions

File tree

‎pkg/webhook/testdata/cert.pem‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIHwMIGXoAMCAQICAQEwCgYIKoZIzj0EAwIwADAiGA8wMDAxMDEwMTAwMDAwMFoY
3+
DzAwMDEwMTAxMDAwMDAwWjAAMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEMP4j
4+
1NMREAE3IpA1ihHN1xZFThO4oFdeRWbVTIUDhgOcCDVh5K/pmquzDE3uqG1D9TmZ
5+
wC5pPyURDgJ9dzTvjDAKBggqhkjOPQQDAgNIADBFAiEAtCfQlhemMkHHz+Brj9ls
6+
f1iHbBF+q2r9Ijud52yFeYoCIESU129jdhqmhm1yWb0bI95dCTEaiKLZaQ8mK+OV
7+
1yr2
8+
-----END CERTIFICATE-----

‎pkg/webhook/validator.go‎

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import (
1919
"bytes"
2020
"context"
2121
"crypto/ecdsa"
22+
"crypto/sha256"
2223
"crypto/x509"
24+
"encoding/hex"
2325
"encoding/json"
2426
"errors"
2527
"fmt"
@@ -642,9 +644,9 @@ func ociSignatureToPolicySignature(ctx context.Context, sigs []oci.Signature) []
642644
for _, ociSig := range sigs {
643645
logging.FromContext(ctx).Debugf("Converting signature %+v", ociSig)
644646

645-
id, err := ociSig.Digest()
647+
sigID, err := signatureID(ociSig)
646648
if err != nil {
647-
logging.FromContext(ctx).Debugf("Error fetching signature digest %+v", err)
649+
logging.FromContext(ctx).Debugf("Error fetching signature %+v", err)
648650
continue
649651
}
650652

@@ -653,7 +655,7 @@ func ociSignatureToPolicySignature(ctx context.Context, sigs []oci.Signature) []
653655
Cert: cert,
654656
}
655657
ret = append(ret, PolicySignature{
656-
ID: id.Hex,
658+
ID: sigID,
657659
Subject: csigs.CertSubject(cert),
658660
Issuer: ce.GetIssuer(),
659661
GithubExtensions: GithubExtensions{
@@ -666,14 +668,42 @@ func ociSignatureToPolicySignature(ctx context.Context, sigs []oci.Signature) []
666668
})
667669
} else {
668670
ret = append(ret, PolicySignature{
669-
ID: id.Hex,
671+
ID: sigID,
670672
// TODO(mattmoor): Is there anything we should encode for key-based?
671673
})
672674
}
673675
}
674676
return ret
675677
}
676678

679+
// signatureID creates a unique hash for the Signature, using both the signature itself + the cert.
680+
func signatureID(sig oci.Signature) (string, error) {
681+
h := sha256.New()
682+
s, err := sig.Signature()
683+
if err != nil {
684+
return "", err
685+
}
686+
if _, err := h.Write(s); err != nil {
687+
return "", err
688+
}
689+
690+
cert, err := sig.Cert()
691+
if err != nil {
692+
return "", err
693+
}
694+
if cert != nil {
695+
c, err := cryptoutils.MarshalCertificateToPEM(cert)
696+
if err != nil {
697+
return "", err
698+
}
699+
if _, err := h.Write(c); err != nil {
700+
return "", err
701+
}
702+
}
703+
704+
return hex.EncodeToString(h.Sum(nil)), nil
705+
}
706+
677707
// attestation is used to accumulate the signature along with extracted and
678708
// validated metadata during validation to construct a list of
679709
// PolicyAttestations upon completion without needing to refetch any of the
@@ -690,9 +720,9 @@ func attestationToPolicyAttestations(ctx context.Context, atts []attestation) []
690720
for _, att := range atts {
691721
logging.FromContext(ctx).Debugf("Converting attestation %+v", att)
692722

693-
id, err := att.Digest()
723+
sigID, err := signatureID(att.Signature)
694724
if err != nil {
695-
logging.FromContext(ctx).Debugf("Error fetching attestation digest %+v", err)
725+
logging.FromContext(ctx).Debugf("Error fetching attestation signature %+v", err)
696726
continue
697727
}
698728

@@ -702,7 +732,7 @@ func attestationToPolicyAttestations(ctx context.Context, atts []attestation) []
702732
}
703733
ret = append(ret, PolicyAttestation{
704734
PolicySignature: PolicySignature{
705-
ID: id.Hex,
735+
ID: sigID,
706736
Subject: csigs.CertSubject(cert),
707737
Issuer: ce.GetIssuer(),
708738
GithubExtensions: GithubExtensions{
@@ -719,7 +749,7 @@ func attestationToPolicyAttestations(ctx context.Context, atts []attestation) []
719749
} else {
720750
ret = append(ret, PolicyAttestation{
721751
PolicySignature: PolicySignature{
722-
ID: id.Hex,
752+
ID: sigID,
723753
// TODO(mattmoor): Is there anything we should encode for key-based?
724754
},
725755
PredicateType: att.PredicateType,

‎pkg/webhook/validator_test.go‎

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828
"fmt"
2929
"net/http"
3030
"net/http/httptest"
31-
"reflect"
31+
"os"
3232
"strings"
3333
"testing"
3434
"time"
@@ -1764,7 +1764,7 @@ func TestValidatePolicy(t *testing.T) {
17641764
Attestations: map[string][]PolicyAttestation{
17651765
"test-att": {{
17661766
PolicySignature: PolicySignature{
1767-
ID: "01bd6aec99ad7c5d045d9aab649fd95b7af2b3b23887d34d7fce8b2e3c38ca0e",
1767+
ID: "00016978d0723c9bc73c599d296ab4052a392e37746509c1f5038494ca4bf34a",
17681768
Subject: "https://github.com/distroless/static/.github/workflows/release.yaml@refs/heads/main",
17691769
Issuer: "https://token.actions.githubusercontent.com",
17701770
GithubExtensions: GithubExtensions{
@@ -1817,8 +1817,8 @@ func TestValidatePolicy(t *testing.T) {
18171817
}
18181818
got, gotErrs := ValidatePolicy(testContext, system.Namespace(), digest, test.policy, kc)
18191819
validateErrors(t, test.wantErrs, gotErrs)
1820-
if !reflect.DeepEqual(test.want, got) {
1821-
t.Errorf("unexpected PolicyResult, want: %+v got: %+v", test.want, got)
1820+
if diff := cmp.Diff(test.want, got); diff != "" {
1821+
t.Errorf("unexpected PolicyResult, %s", diff)
18221822
}
18231823
})
18241824
}
@@ -3271,3 +3271,54 @@ func TestCheckOptsFromAuthority(t *testing.T) {
32713271
})
32723272
}
32733273
}
3274+
3275+
func TestSignatureID(t *testing.T) {
3276+
cert := mustRead(t, "testdata/cert.pem")
3277+
for _, tc := range []struct {
3278+
name string
3279+
sig oci.Signature
3280+
want string
3281+
}{
3282+
{
3283+
name: "no cert",
3284+
sig: newStaticSig(t, []byte("foo"), nil),
3285+
want: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
3286+
},
3287+
{
3288+
name: "with cert",
3289+
sig: newStaticSig(t, []byte("foo"), cert),
3290+
want: "0b413181a61e85e0426d9f49ccd58d2205314297ee0c3bb515ad9d0f89480995",
3291+
},
3292+
} {
3293+
got, err := signatureID(tc.sig)
3294+
if err != nil {
3295+
t.Fatal(err)
3296+
}
3297+
if tc.want != got {
3298+
t.Errorf("want %s, got %s", tc.want, got)
3299+
}
3300+
}
3301+
}
3302+
3303+
func mustRead(t *testing.T, path string) []byte {
3304+
t.Helper()
3305+
b, err := os.ReadFile(path)
3306+
if err != nil {
3307+
t.Fatal(err)
3308+
}
3309+
return b
3310+
}
3311+
3312+
func newStaticSig(t *testing.T, payload []byte, cert []byte) oci.Signature {
3313+
t.Helper()
3314+
3315+
var opts []static.Option
3316+
if cert != nil {
3317+
opts = append(opts, static.WithCertChain(cert, nil))
3318+
}
3319+
out, err := static.NewSignature(payload, "", opts...)
3320+
if err != nil {
3321+
t.Fatal(err)
3322+
}
3323+
return out
3324+
}

0 commit comments

Comments
 (0)