Support intermediate certificates in bundle

Add support for processing and verifying a v0.3 bundle that contains a `X509CertificateChain` rather than a single X.509 certificate or public key. Signed-off-by: Colleen Murphy <[email protected]>
sigstore · Sep 12, 2024 · 53e9e5c · 53e9e5c
1 parent fbc922a
commit 53e9e5c
Show file tree

Hide file tree

Showing 12 changed files with 193 additions and 82 deletions.
diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go
@@ -108,15 +108,6 @@ func (b *Bundle) validate() error {
 		}
 	}
 
-	// if bundle version >= v0.3, require verification material to not be X.509 certificate chain (only single certificate is allowed)
-	if semver.Compare(bundleVersion, "v0.3") >= 0 {
-		certs := b.Bundle.VerificationMaterial.GetX509CertificateChain()
-
-		if certs != nil {
-			return errors.New("verification material cannot be X.509 certificate chain (for bundle v0.3)")
-		}
-	}
-
 	// if bundle version is >= v0.4, return error as this version is not supported
 	if semver.Compare(bundleVersion, "v0.4") >= 0 {
 		return fmt.Errorf("%w: bundle version %s is not yet supported", ErrUnsupportedMediaType, bundleVersion)
@@ -250,14 +241,18 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) {
 		if len(certs) == 0 || certs[0].RawBytes == nil {
 			return nil, ErrMissingVerificationMaterial
 		}
-		parsedCert, err := x509.ParseCertificate(certs[0].RawBytes)
-		if err != nil {
-			return nil, ErrValidationError(err)
+		parsedCerts := make([]*x509.Certificate, len(certs))
+		var err error
+		for i, c := range certs {
+			parsedCerts[i], err = x509.ParseCertificate(c.RawBytes)
+			if err != nil {
+				return nil, ErrValidationError(err)
+			}
 		}
-		cert := &Certificate{
-			Certificate: parsedCert,
+		certChain := &CertificateChain{
+			Certificates: parsedCerts,
 		}
-		return cert, nil
+		return certChain, nil
 	case *protobundle.VerificationMaterial_Certificate:
 		if content.Certificate == nil || content.Certificate.RawBytes == nil {
 			return nil, ErrMissingVerificationMaterial
@@ -266,8 +261,8 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) {
 		if err != nil {
 			return nil, ErrValidationError(err)
 		}
-		cert := &Certificate{
-			Certificate: parsedCert,
+		cert := &CertificateChain{
+			Certificates: []*x509.Certificate{parsedCert},
 		}
 		return cert, nil
 	case *protobundle.VerificationMaterial_PublicKey:

diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go
@@ -520,7 +520,7 @@ func Test_validate(t *testing.T) {
 					Content: &protobundle.Bundle_MessageSignature{},
 				},
 			},
-			wantErr: true,
+			wantErr: false,
 		},
 		{
 			name: "v0.3 without x.509 certificate chain",
@@ -584,11 +584,12 @@ func TestVerificationContent(t *testing.T) {
 	leafDer, err := x509.CreateCertificate(rand.Reader, leafCert, caCert, &leafKey.PublicKey, caKey)
 	require.NoError(t, err)
 	tests := []struct {
-		name            string
-		pb              Bundle
-		wantCertificate bool
-		wantPublicKey   bool
-		wantErr         bool
+		name                 string
+		pb                   Bundle
+		wantCertificateChain bool
+		wantCertificate      bool
+		wantPublicKey        bool
+		wantErr              bool
 	}{
 		{
 			name: "no verification material",
@@ -649,7 +650,8 @@ func TestVerificationContent(t *testing.T) {
 					},
 				},
 			},
-			wantCertificate: true,
+			wantCertificateChain: true,
+			wantCertificate:      true,
 		},
 		{
 			name: "certificate chain with invalid cert",
@@ -813,14 +815,15 @@ func TestVerificationContent(t *testing.T) {
 				return
 			}
 			require.NoError(t, gotErr)
+			if tt.wantCertificateChain {
+				require.NotNil(t, got.GetIntermediates())
+			}
 			if tt.wantCertificate {
-				require.NotNil(t, got.GetCertificate())
-				return
+				require.NotNil(t, got.GetLeafCertificate())
 			}
 			if tt.wantPublicKey {
 				_, hasPubKey := got.HasPublicKey()
 				require.True(t, hasPubKey)
-				return
 			}
 		})
 	}

diff --git a/pkg/bundle/verification_content.go b/pkg/bundle/verification_content.go
@@ -23,8 +23,8 @@ import (
 	"github.com/sigstore/sigstore-go/pkg/verify"
 )
 
-type Certificate struct {
-	*x509.Certificate
+type CertificateChain struct {
+	Certificates []*x509.Certificate
 }
 
 type PublicKey struct {
@@ -35,27 +35,42 @@ func (pk PublicKey) Hint() string {
 	return pk.hint
 }
 
-func (c *Certificate) CompareKey(key any, _ root.TrustedMaterial) bool {
+func (c *CertificateChain) CompareKey(key any, _ root.TrustedMaterial) bool {
+	if len(c.Certificates) < 1 {
+		return false
+	}
 	x509Key, ok := key.(*x509.Certificate)
 	if !ok {
 		return false
 	}
-
-	return c.Certificate.Equal(x509Key)
+	return c.Certificates[0].Equal(x509Key)
 }
 
-func (c *Certificate) ValidAtTime(t time.Time, _ root.TrustedMaterial) bool {
-	return !(c.Certificate.NotAfter.Before(t) || c.Certificate.NotBefore.After(t))
+func (c *CertificateChain) ValidAtTime(t time.Time, _ root.TrustedMaterial) bool {
+	if len(c.Certificates) < 1 {
+		return false
+	}
+	return !(c.Certificates[0].NotAfter.Before(t) || c.Certificates[0].NotBefore.After(t))
 }
 
-func (c *Certificate) GetCertificate() *x509.Certificate {
-	return c.Certificate
+func (c *CertificateChain) GetLeafCertificate() *x509.Certificate {
+	if len(c.Certificates) < 1 {
+		return nil
+	}
+	return c.Certificates[0]
 }
 
-func (c *Certificate) HasPublicKey() (verify.PublicKeyProvider, bool) {
+func (c *CertificateChain) HasPublicKey() (verify.PublicKeyProvider, bool) {
 	return PublicKey{}, false
 }
 
+func (c *CertificateChain) GetIntermediates() []*x509.Certificate {
+	if len(c.Certificates) < 2 {
+		return nil
+	}
+	return c.Certificates[1:]
+}
+
 func (pk *PublicKey) CompareKey(key any, tm root.TrustedMaterial) bool {
 	verifier, err := tm.PublicKeyVerifier(pk.hint)
 	if err != nil {
@@ -79,10 +94,14 @@ func (pk *PublicKey) ValidAtTime(t time.Time, tm root.TrustedMaterial) bool {
 	return verifier.ValidAtTime(t)
 }
 
-func (pk *PublicKey) GetCertificate() *x509.Certificate {
+func (pk *PublicKey) GetLeafCertificate() *x509.Certificate {
 	return nil
 }
 
 func (pk *PublicKey) HasPublicKey() (verify.PublicKeyProvider, bool) {
 	return *pk, true
 }
+
+func (pk *PublicKey) GetIntermediates() []*x509.Certificate {
+	return nil
+}
diff --git a/pkg/fulcio/certificate/summarize_test.go b/pkg/fulcio/certificate/summarize_test.go
@@ -30,7 +30,7 @@ func TestSummarizeCertificateWithActionsBundle(t *testing.T) {
 		t.Fatalf("failed to get verification content: %v", err)
 	}
 
-	leaf := vc.GetCertificate()
+	leaf := vc.GetLeafCertificate()
 
 	if leaf == nil {
 		t.Fatalf("expected verification content to be a certificate chain")
@@ -79,7 +79,7 @@ func TestSummarizeCertificateWithOauthBundle(t *testing.T) {
 		t.Fatalf("failed to get verification content: %v", err)
 	}
 
-	leaf := vc.GetCertificate()
+	leaf := vc.GetLeafCertificate()
 
 	if leaf == nil {
 		t.Fatalf("expected verification content to be a certificate chain")
@@ -108,7 +108,7 @@ func TestSummarizeCertificateWithOtherNameSAN(t *testing.T) {
 		t.Fatalf("failed to get verification content: %v", err)
 	}
 
-	leaf := vc.GetCertificate()
+	leaf := vc.GetLeafCertificate()
 
 	if leaf == nil {
 		t.Fatalf("expected verification content to be a certificate chain")

diff --git a/pkg/testing/ca/ca.go b/pkg/testing/ca/ca.go
@@ -332,6 +332,34 @@ func (ca *VirtualSigstore) PublicKeyVerifier(keyID string) (root.TimeConstrained
 	return v, nil
 }
 
+func (ca *VirtualSigstore) GenerateNewFulcioIntermediate(name string) (*x509.Certificate, *ecdsa.PrivateKey, error) {
+	subTemplate := &x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		Subject: pkix.Name{
+			CommonName:   name,
+			Organization: []string{"sigstore.dev"},
+		},
+		NotBefore:             time.Now().Add(-2 * time.Minute),
+		NotAfter:              time.Now().Add(2 * time.Hour),
+		KeyUsage:              x509.KeyUsageCertSign | x509.KeyUsageCRLSign,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageCodeSigning},
+		BasicConstraintsValid: true,
+		IsCA:                  true,
+	}
+
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	cert, err := createCertificate(subTemplate, ca.fulcioCA.Intermediates[0], &priv.PublicKey, ca.fulcioIntermediateKey)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return cert, priv, nil
+}
+
 func generateRekorEntry(kind, version string, artifact []byte, cert []byte, sig []byte) (string, error) {
 	// Generate the Rekor Entry
 	entryImpl, err := createEntry(context.Background(), kind, version, artifact, cert, sig)
@@ -481,7 +509,7 @@ type TestEntity struct {
 }
 
 func (e *TestEntity) VerificationContent() (verify.VerificationContent, error) {
-	return &bundle.Certificate{Certificate: e.certChain[0]}, nil
+	return &bundle.CertificateChain{[]*x509.Certificate{e.certChain[0]}}, nil
 }
 
 func (e *TestEntity) HasInclusionPromise() bool {

diff --git a/pkg/verify/certificate.go b/pkg/verify/certificate.go
@@ -22,7 +22,7 @@ import (
 	"github.com/sigstore/sigstore-go/pkg/root"
 )
 
-func VerifyLeafCertificate(observerTimestamp time.Time, leafCert *x509.Certificate, trustedMaterial root.TrustedMaterial) error { // nolint: revive
+func VerifyLeafCertificate(observerTimestamp time.Time, verificationContent VerificationContent, trustedMaterial root.TrustedMaterial) error { // nolint: revive
 	for _, ca := range trustedMaterial.FulcioCertificateAuthorities() {
 		if !ca.ValidityPeriodStart.IsZero() && observerTimestamp.Before(ca.ValidityPeriodStart) {
 			continue
@@ -38,6 +38,10 @@ func VerifyLeafCertificate(observerTimestamp time.Time, leafCert *x509.Certifica
 			intermediateCertPool.AddCert(cert)
 		}
 
+		for _, cert := range verificationContent.GetIntermediates() {
+			intermediateCertPool.AddCert(cert)
+		}
+
 		// From spec:
 		// > ## Certificate
 		// > For a signature with a given certificate to be considered valid, it must have a timestamp while every certificate in the chain up to the root is valid (the so-called “hybrid model” of certificate verification per Braun et al. (2013)).
@@ -51,6 +55,8 @@ func VerifyLeafCertificate(observerTimestamp time.Time, leafCert *x509.Certifica
 			},
 		}
 
+		leafCert := verificationContent.GetLeafCertificate()
+
 		_, err := leafCert.Verify(opts)
 		if err == nil {
 			return nil

diff --git a/pkg/verify/certificate_test.go b/pkg/verify/certificate_test.go
@@ -15,9 +15,14 @@
 package verify_test
 
 import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/x509"
 	"testing"
 	"time"
 
+	"github.com/sigstore/sigstore-go/pkg/bundle"
 	"github.com/sigstore/sigstore-go/pkg/testing/ca"
 	"github.com/sigstore/sigstore-go/pkg/verify"
 	"github.com/stretchr/testify/assert"
@@ -30,30 +35,64 @@ func TestVerifyValidityPeriod(t *testing.T) {
 	leaf, _, err := virtualSigstore.GenerateLeafCert("[email protected]", "issuer")
 	assert.NoError(t, err)
 
+	altIntermediate, intermediateKey, err := virtualSigstore.GenerateNewFulcioIntermediate("sigstore-subintermediate")
+	assert.NoError(t, err)
+
+	altPrivKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	assert.NoError(t, err)
+	altLeaf, err := ca.GenerateLeafCert("[email protected]", "issuer", time.Now().Add(time.Hour*24), altPrivKey, altIntermediate, intermediateKey)
+	assert.NoError(t, err)
+
 	tests := []struct {
-		name              string
-		observerTimestamp time.Time
-		wantErr           bool
+		name                string
+		observerTimestamp   time.Time
+		verificationContent verify.VerificationContent
+		wantErr             bool
 	}{
 		{
-			name:              "before validity period",
-			observerTimestamp: time.Now().Add(time.Hour * -24),
-			wantErr:           true,
+			name:                "before validity period",
+			observerTimestamp:   time.Now().Add(time.Hour * -24),
+			verificationContent: &bundle.CertificateChain{[]*x509.Certificate{leaf}},
+			wantErr:             true,
 		},
 		{
-			name:              "inside validity period",
+			name:                "inside validity period",
+			observerTimestamp:   time.Now(),
+			verificationContent: &bundle.CertificateChain{[]*x509.Certificate{leaf}},
+			wantErr:             false,
+		},
+		{
+			name:                "after validity period",
+			observerTimestamp:   time.Now().Add(time.Hour * 24),
+			verificationContent: &bundle.CertificateChain{[]*x509.Certificate{leaf}},
+			wantErr:             true,
+		},
+		{
+			name:              "with intermediates",
 			observerTimestamp: time.Now(),
-			wantErr:           false,
+			verificationContent: &bundle.CertificateChain{
+				[]*x509.Certificate{
+					altIntermediate,
+					altLeaf,
+				},
+			},
+			wantErr: false,
 		},
 		{
-			name:              "after validity period",
-			observerTimestamp: time.Now().Add(time.Hour * 24),
-			wantErr:           true,
+			name:              "with invalid intermediates",
+			observerTimestamp: time.Now(),
+			verificationContent: &bundle.CertificateChain{
+				[]*x509.Certificate{
+					altLeaf,
+					leaf,
+				},
+			},
+			wantErr: true,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if err := verify.VerifyLeafCertificate(tt.observerTimestamp, leaf, virtualSigstore); (err != nil) != tt.wantErr {
+			if err := verify.VerifyLeafCertificate(tt.observerTimestamp, tt.verificationContent, virtualSigstore); (err != nil) != tt.wantErr {
 				t.Errorf("VerifyLeafCertificate() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})

diff --git a/pkg/verify/interface.go b/pkg/verify/interface.go
@@ -64,7 +64,8 @@ type SignedEntity interface {
 type VerificationContent interface {
 	CompareKey(any, root.TrustedMaterial) bool
 	ValidAtTime(time.Time, root.TrustedMaterial) bool
-	GetCertificate() *x509.Certificate
+	GetLeafCertificate() *x509.Certificate
+	GetIntermediates() []*x509.Certificate
 	HasPublicKey() (PublicKeyProvider, bool)
 }