From 234afe786c8d41aea04818dddd38ffa2e891f56f Mon Sep 17 00:00:00 2001 From: asraa Date: Fri, 1 Jul 2022 06:54:00 -0500 Subject: [PATCH] feat: add subject URIs to index for x509 certificates (#897) * feat: add subject URIs to index for x509 certificates Signed-off-by: Asra Ali * fix comments Signed-off-by: Asra Ali * fix lint Signed-off-by: Asra Ali * Address another bob comment Signed-off-by: Asra Ali --- pkg/pki/minisign/minisign.go | 5 ++++ pkg/pki/pgp/pgp.go | 5 ++++ pkg/pki/pgp/pgp_test.go | 30 +++++++++++------------ pkg/pki/pkcs7/pkcs7.go | 5 ++++ pkg/pki/pkcs7/pkcs7_test.go | 2 +- pkg/pki/pki.go | 3 +++ pkg/pki/ssh/ssh.go | 5 ++++ pkg/pki/tuf/tuf.go | 5 ++++ pkg/pki/x509/testutils/cert_test_utils.go | 6 ++++- pkg/pki/x509/x509.go | 27 +++++++++++++++++++- pkg/pki/x509/x509_test.go | 14 ++++++++--- pkg/types/alpine/v0.0.1/entry.go | 2 +- pkg/types/cose/v0.0.1/entry.go | 2 +- pkg/types/cose/v0.0.1/entry_test.go | 4 +++ pkg/types/hashedrekord/v0.0.1/entry.go | 2 +- pkg/types/helm/v0.0.1/entry.go | 2 +- pkg/types/intoto/v0.0.1/entry.go | 4 +-- pkg/types/rekord/v0.0.1/entry.go | 2 +- pkg/types/rpm/v0.0.1/entry.go | 2 +- 19 files changed, 98 insertions(+), 29 deletions(-) diff --git a/pkg/pki/minisign/minisign.go b/pkg/pki/minisign/minisign.go index 666bf16c6..945db41ca 100644 --- a/pkg/pki/minisign/minisign.go +++ b/pkg/pki/minisign/minisign.go @@ -177,3 +177,8 @@ func (k PublicKey) CanonicalValue() ([]byte, error) { func (k PublicKey) EmailAddresses() []string { return nil } + +// Subjects implements the pki.PublicKey interface +func (k PublicKey) Subjects() []string { + return nil +} diff --git a/pkg/pki/pgp/pgp.go b/pkg/pki/pgp/pgp.go index 8a56abedd..52f13cc02 100644 --- a/pkg/pki/pgp/pgp.go +++ b/pkg/pki/pgp/pgp.go @@ -296,3 +296,8 @@ func (k PublicKey) EmailAddresses() []string { } return names } + +// Subjects implements the pki.PublicKey interface +func (k PublicKey) Subjects() []string { + return k.EmailAddresses() +} diff --git a/pkg/pki/pgp/pgp_test.go b/pkg/pki/pgp/pgp_test.go index 924d00e1e..366fb181c 100644 --- a/pkg/pki/pgp/pgp_test.go +++ b/pkg/pki/pgp/pgp_test.go @@ -347,18 +347,18 @@ func TestEmailAddresses(t *testing.T) { type test struct { caseDesc string inputFile string - emails []string + subjects []string } var k PublicKey - if len(k.EmailAddresses()) != 0 { - t.Errorf("EmailAddresses for unitialized key should give empty slice") + if len(k.Subjects()) != 0 { + t.Errorf("Subjects for unitialized key should give empty slice") } tests := []test{ - {caseDesc: "Valid armored public key", inputFile: "testdata/valid_armored_public.pgp", emails: []string{}}, - {caseDesc: "Valid armored public key with multiple subentries", inputFile: "testdata/valid_armored_complex_public.pgp", emails: []string{"linux-packages-keymaster@google.com", "linux-packages-keymaster@google.com"}}, - {caseDesc: "Valid binary public key", inputFile: "testdata/valid_binary_public.pgp", emails: []string{}}, - {caseDesc: "Valid binary public key with multiple subentries", inputFile: "testdata/valid_binary_complex_public.pgp", emails: []string{"linux-packages-keymaster@google.com", "linux-packages-keymaster@google.com"}}, + {caseDesc: "Valid armored public key", inputFile: "testdata/valid_armored_public.pgp", subjects: []string{}}, + {caseDesc: "Valid armored public key with multiple subentries", inputFile: "testdata/valid_armored_complex_public.pgp", subjects: []string{"linux-packages-keymaster@google.com", "linux-packages-keymaster@google.com"}}, + {caseDesc: "Valid binary public key", inputFile: "testdata/valid_binary_public.pgp", subjects: []string{}}, + {caseDesc: "Valid binary public key with multiple subentries", inputFile: "testdata/valid_binary_complex_public.pgp", subjects: []string{"linux-packages-keymaster@google.com", "linux-packages-keymaster@google.com"}}, } for _, tc := range tests { @@ -374,18 +374,18 @@ func TestEmailAddresses(t *testing.T) { t.Errorf("%v: Error reading input for TestEmailAddresses: %v", tc.caseDesc, err) } - emails := inputKey.EmailAddresses() + subjects := inputKey.Subjects() - if len(emails) == len(tc.emails) { - if len(emails) > 0 { - sort.Strings(emails) - sort.Strings(tc.emails) - if !reflect.DeepEqual(emails, tc.emails) { - t.Errorf("%v: Error getting email addresses from keys, got %v, expected %v", tc.caseDesc, emails, tc.emails) + if len(subjects) == len(tc.subjects) { + if len(subjects) > 0 { + sort.Strings(subjects) + sort.Strings(tc.subjects) + if !reflect.DeepEqual(subjects, tc.subjects) { + t.Errorf("%v: Error getting subjects from keys, got %v, expected %v", tc.caseDesc, subjects, tc.subjects) } } } else { - t.Errorf("%v: Error getting email addresses from keys length, got %v, expected %v", tc.caseDesc, len(emails), len(tc.emails)) + t.Errorf("%v: Error getting subjects from keys length, got %v, expected %v", tc.caseDesc, len(subjects), len(tc.subjects)) } } diff --git a/pkg/pki/pkcs7/pkcs7.go b/pkg/pki/pkcs7/pkcs7.go index 10f29a21e..b2939f766 100644 --- a/pkg/pki/pkcs7/pkcs7.go +++ b/pkg/pki/pkcs7/pkcs7.go @@ -209,3 +209,8 @@ func (k PublicKey) EmailAddresses() []string { return names } + +// Subjects implements the pki.PublicKey interface +func (k PublicKey) Subjects() []string { + return k.EmailAddresses() +} diff --git a/pkg/pki/pkcs7/pkcs7_test.go b/pkg/pki/pkcs7/pkcs7_test.go index 3b77256b6..9373a0e95 100644 --- a/pkg/pki/pkcs7/pkcs7_test.go +++ b/pkg/pki/pkcs7/pkcs7_test.go @@ -292,7 +292,7 @@ func TestEmailAddresses(t *testing.T) { if err != nil { t.Fatal(err) } - emails := pub.EmailAddresses() + emails := pub.Subjects() if len(emails) == len(tt.emails) { if len(emails) > 0 { diff --git a/pkg/pki/pki.go b/pkg/pki/pki.go index d1618034d..d6a2d2135 100644 --- a/pkg/pki/pki.go +++ b/pkg/pki/pki.go @@ -24,7 +24,10 @@ import ( // PublicKey Generic object representing a public key (regardless of format & algorithm) type PublicKey interface { CanonicalValue() ([]byte, error) + // Deprecated: EmailAddresses() will be deprecated in favor of Subjects() which will + // also return Subject URIs present in public keys. EmailAddresses() []string + Subjects() []string } // Signature Generic object representing a signature (regardless of format & algorithm) diff --git a/pkg/pki/ssh/ssh.go b/pkg/pki/ssh/ssh.go index 8012905aa..79c10f651 100644 --- a/pkg/pki/ssh/ssh.go +++ b/pkg/pki/ssh/ssh.go @@ -102,3 +102,8 @@ func (k PublicKey) CanonicalValue() ([]byte, error) { func (k PublicKey) EmailAddresses() []string { return nil } + +// Subjects implements the pki.PublicKey interface +func (k PublicKey) Subjects() []string { + return nil +} diff --git a/pkg/pki/tuf/tuf.go b/pkg/pki/tuf/tuf.go index 1b63049b2..dfc20e7aa 100644 --- a/pkg/pki/tuf/tuf.go +++ b/pkg/pki/tuf/tuf.go @@ -169,3 +169,8 @@ func (k PublicKey) SpecVersion() (string, error) { func (k PublicKey) EmailAddresses() []string { return nil } + +// Subjects implements the pki.PublicKey interface +func (k PublicKey) Subjects() []string { + return nil +} diff --git a/pkg/pki/x509/testutils/cert_test_utils.go b/pkg/pki/x509/testutils/cert_test_utils.go index ec0167eaf..c6ce32bb5 100644 --- a/pkg/pki/x509/testutils/cert_test_utils.go +++ b/pkg/pki/x509/testutils/cert_test_utils.go @@ -23,6 +23,7 @@ import ( "crypto/x509/pkix" "encoding/asn1" "math/big" + "net/url" "time" ) @@ -115,7 +116,7 @@ func GenerateSubordinateCa(rootTemplate *x509.Certificate, rootPriv crypto.Signe return cert, priv, nil } -func GenerateLeafCert(subject string, oidcIssuer string, parentTemplate *x509.Certificate, parentPriv crypto.Signer) (*x509.Certificate, *ecdsa.PrivateKey, error) { +func GenerateLeafCert(subject, oidcIssuer string, uri *url.URL, parentTemplate *x509.Certificate, parentPriv crypto.Signer) (*x509.Certificate, *ecdsa.PrivateKey, error) { certTemplate := &x509.Certificate{ SerialNumber: big.NewInt(1), EmailAddresses: []string{subject}, @@ -131,6 +132,9 @@ func GenerateLeafCert(subject string, oidcIssuer string, parentTemplate *x509.Ce Value: []byte(oidcIssuer), }}, } + if uri != nil { + certTemplate.URIs = []*url.URL{uri} + } priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { diff --git a/pkg/pki/x509/x509.go b/pkg/pki/x509/x509.go index 49fa1c544..e82ec9f00 100644 --- a/pkg/pki/x509/x509.go +++ b/pkg/pki/x509/x509.go @@ -181,8 +181,8 @@ func (k PublicKey) EmailAddresses() []string { cert = k.certs[0] } if cert != nil { + validate := validator.New() for _, name := range cert.EmailAddresses { - validate := validator.New() errs := validate.Var(name, "required,email") if errs == nil { names = append(names, strings.ToLower(name)) @@ -192,6 +192,31 @@ func (k PublicKey) EmailAddresses() []string { return names } +// Subjects implements the pki.PublicKey interface +func (k PublicKey) Subjects() []string { + var names []string + var cert *x509.Certificate + if k.cert != nil { + cert = k.cert.c + } else if len(k.certs) > 0 { + cert = k.certs[0] + } + if cert != nil { + validate := validator.New() + for _, name := range cert.EmailAddresses { + if errs := validate.Var(name, "required,email"); errs == nil { + names = append(names, strings.ToLower(name)) + } + } + for _, name := range cert.URIs { + if errs := validate.Var(name.String(), "required,uri"); errs == nil { + names = append(names, strings.ToLower(name.String())) + } + } + } + return names +} + func verifyCertChain(certChain []*x509.Certificate) error { if len(certChain) == 0 { return errors.New("no certificate chain provided") diff --git a/pkg/pki/x509/x509_test.go b/pkg/pki/x509/x509_test.go index aa0805477..307564c59 100644 --- a/pkg/pki/x509/x509_test.go +++ b/pkg/pki/x509/x509_test.go @@ -20,6 +20,7 @@ import ( "crypto" "crypto/ecdsa" "crypto/x509" + "net/url" "reflect" "strings" "testing" @@ -200,7 +201,8 @@ func TestSignature_VerifyFail(t *testing.T) { func TestPublicKeyWithCertChain(t *testing.T) { rootCert, rootKey, _ := testutils.GenerateRootCa() subCert, subKey, _ := testutils.GenerateSubordinateCa(rootCert, rootKey) - leafCert, leafKey, _ := testutils.GenerateLeafCert("subject@example.com", "oidc-issuer", subCert, subKey) + url, _ := url.Parse("https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml@refs/tags/v1.1.1") + leafCert, leafKey, _ := testutils.GenerateLeafCert("subject@example.com", "oidc-issuer", url, subCert, subKey) pemCertChain, err := cryptoutils.MarshalCertificatesToPEM([]*x509.Certificate{leafCert, subCert, rootCert}) if err != nil { @@ -220,7 +222,13 @@ func TestPublicKeyWithCertChain(t *testing.T) { } if !reflect.DeepEqual(pub.EmailAddresses(), leafCert.EmailAddresses) { - t.Fatalf("expected matching email addresses, expected %v, got %v", leafCert.EmailAddresses, pub.EmailAddresses()) + t.Fatalf("expected matching subjects, expected %v, got %v", leafCert.EmailAddresses, pub.EmailAddresses()) + } + + expectedSubjects := leafCert.EmailAddresses + expectedSubjects = append(expectedSubjects, leafCert.URIs[0].String()) + if !reflect.DeepEqual(pub.Subjects(), expectedSubjects) { + t.Fatalf("expected matching subjects, expected %v, got %v", expectedSubjects, pub.Subjects()) } canonicalValue, err := pub.CanonicalValue() @@ -274,7 +282,7 @@ func TestPublicKeyWithCertChain(t *testing.T) { } // Verify works with chain without intermediate - leafCert, leafKey, _ = testutils.GenerateLeafCert("subject@example.com", "oidc-issuer", rootCert, rootKey) + leafCert, leafKey, _ = testutils.GenerateLeafCert("subject@example.com", "oidc-issuer", nil, rootCert, rootKey) pemCertChain, _ = cryptoutils.MarshalCertificatesToPEM([]*x509.Certificate{leafCert, rootCert}) pub, _ = NewPublicKey(bytes.NewReader(pemCertChain)) signer, _ = signature.LoadSigner(leafKey, crypto.SHA256) diff --git a/pkg/types/alpine/v0.0.1/entry.go b/pkg/types/alpine/v0.0.1/entry.go index 39c9b9f20..a5280682d 100644 --- a/pkg/types/alpine/v0.0.1/entry.go +++ b/pkg/types/alpine/v0.0.1/entry.go @@ -78,7 +78,7 @@ func (v V001Entry) IndexKeys() ([]string, error) { keyHash := sha256.Sum256(key) result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:]))) - result = append(result, keyObj.EmailAddresses()...) + result = append(result, keyObj.Subjects()...) if v.AlpineModel.Package.Hash != nil { hashKey := strings.ToLower(fmt.Sprintf("%s:%s", *v.AlpineModel.Package.Hash.Algorithm, *v.AlpineModel.Package.Hash.Value)) diff --git a/pkg/types/cose/v0.0.1/entry.go b/pkg/types/cose/v0.0.1/entry.go index 55dcf3008..7ddb5a508 100644 --- a/pkg/types/cose/v0.0.1/entry.go +++ b/pkg/types/cose/v0.0.1/entry.go @@ -90,7 +90,7 @@ func (v V001Entry) IndexKeys() ([]string, error) { keyHash := sha256.Sum256(key) result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:]))) } - result = append(result, keyObj.EmailAddresses()...) + result = append(result, keyObj.Subjects()...) // 2. Overall envelope result = append(result, formatKey(v.CoseObj.Message)) diff --git a/pkg/types/cose/v0.0.1/entry_test.go b/pkg/types/cose/v0.0.1/entry_test.go index 77a030d2a..584256cbf 100644 --- a/pkg/types/cose/v0.0.1/entry_test.go +++ b/pkg/types/cose/v0.0.1/entry_test.go @@ -84,6 +84,10 @@ func (t testPublicKey) EmailAddresses() []string { return nil } +func (t testPublicKey) Subjects() []string { + return nil +} + func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } diff --git a/pkg/types/hashedrekord/v0.0.1/entry.go b/pkg/types/hashedrekord/v0.0.1/entry.go index c44c568a1..4aca73f19 100644 --- a/pkg/types/hashedrekord/v0.0.1/entry.go +++ b/pkg/types/hashedrekord/v0.0.1/entry.go @@ -74,7 +74,7 @@ func (v V001Entry) IndexKeys() ([]string, error) { if err != nil { return nil, err } - result = append(result, pub.EmailAddresses()...) + result = append(result, pub.Subjects()...) if v.HashedRekordObj.Data.Hash != nil { hashKey := strings.ToLower(fmt.Sprintf("%s:%s", *v.HashedRekordObj.Data.Hash.Algorithm, *v.HashedRekordObj.Data.Hash.Value)) diff --git a/pkg/types/helm/v0.0.1/entry.go b/pkg/types/helm/v0.0.1/entry.go index eebd9400f..9695cf791 100644 --- a/pkg/types/helm/v0.0.1/entry.go +++ b/pkg/types/helm/v0.0.1/entry.go @@ -82,7 +82,7 @@ func (v V001Entry) IndexKeys() ([]string, error) { keyHash := sha256.Sum256(key) result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:]))) - result = append(result, keyObj.EmailAddresses()...) + result = append(result, keyObj.Subjects()...) algorithm, chartHash, err := provenance.GetChartAlgorithmHash() diff --git a/pkg/types/intoto/v0.0.1/entry.go b/pkg/types/intoto/v0.0.1/entry.go index 279cef71f..0e2560f5b 100644 --- a/pkg/types/intoto/v0.0.1/entry.go +++ b/pkg/types/intoto/v0.0.1/entry.go @@ -88,8 +88,8 @@ func (v V001Entry) IndexKeys() ([]string, error) { keyHash := sha256.Sum256(key) result = append(result, fmt.Sprintf("sha256:%s", strings.ToLower(hex.EncodeToString(keyHash[:])))) - // add digest over any email addresses within signing certificate - result = append(result, v.keyObj.EmailAddresses()...) + // add digest over any subjects within signing certificate + result = append(result, v.keyObj.Subjects()...) } else { log.Logger.Errorf("could not canonicalize public key to include in index keys: %w", err) } diff --git a/pkg/types/rekord/v0.0.1/entry.go b/pkg/types/rekord/v0.0.1/entry.go index 3fde155c1..970ac1de6 100644 --- a/pkg/types/rekord/v0.0.1/entry.go +++ b/pkg/types/rekord/v0.0.1/entry.go @@ -84,7 +84,7 @@ func (v V001Entry) IndexKeys() ([]string, error) { result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:]))) } - result = append(result, keyObj.EmailAddresses()...) + result = append(result, keyObj.Subjects()...) if v.RekordObj.Data.Hash != nil { hashKey := strings.ToLower(fmt.Sprintf("%s:%s", *v.RekordObj.Data.Hash.Algorithm, *v.RekordObj.Data.Hash.Value)) diff --git a/pkg/types/rpm/v0.0.1/entry.go b/pkg/types/rpm/v0.0.1/entry.go index ee58fc768..cd9d3dd97 100644 --- a/pkg/types/rpm/v0.0.1/entry.go +++ b/pkg/types/rpm/v0.0.1/entry.go @@ -80,7 +80,7 @@ func (v V001Entry) IndexKeys() ([]string, error) { keyHash := sha256.Sum256(key) result = append(result, strings.ToLower(hex.EncodeToString(keyHash[:]))) - result = append(result, keyObj.EmailAddresses()...) + result = append(result, keyObj.Subjects()...) if v.RPMModel.Package.Hash != nil { hashKey := strings.ToLower(fmt.Sprintf("%s:%s", *v.RPMModel.Package.Hash.Algorithm, *v.RPMModel.Package.Hash.Value))