feat: add subject URIs to index for x509 certificates (#897)

* feat: add subject URIs to index for x509 certificates Signed-off-by: Asra Ali <asraa@google.com> * fix comments Signed-off-by: Asra Ali <asraa@google.com> * fix lint Signed-off-by: Asra Ali <asraa@google.com> * Address another bob comment Signed-off-by: Asra Ali <asraa@google.com>
sigstore · Jul 1, 2022 · 234afe7 · 234afe7
1 parent 66f5c06
commit 234afe7
Show file tree

Hide file tree

Showing 19 changed files with 98 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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))