Skip to content

Commit

Permalink
Support intermediate certificates in bundle
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
cmurphy committed Sep 12, 2024
1 parent fbc922a commit 410da84
Show file tree
Hide file tree
Showing 12 changed files with 193 additions and 82 deletions.
29 changes: 12 additions & 17 deletions pkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
23 changes: 13 additions & 10 deletions pkg/bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -649,7 +650,8 @@ func TestVerificationContent(t *testing.T) {
},
},
},
wantCertificate: true,
wantCertificateChain: true,
wantCertificate: true,
},
{
name: "certificate chain with invalid cert",
Expand Down Expand Up @@ -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
}
})
}
Expand Down
41 changes: 30 additions & 11 deletions pkg/bundle/verification_content.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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, tm root.TrustedMaterial) bool {

Check failure on line 38 in pkg/bundle/verification_content.go

View workflow job for this annotation

GitHub Actions / lint

unused-parameter: parameter 'tm' seems to be unused, consider removing or renaming it as _ (revive)
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, tm root.TrustedMaterial) bool {

Check failure on line 49 in pkg/bundle/verification_content.go

View workflow job for this annotation

GitHub Actions / lint

unused-parameter: parameter 'tm' seems to be unused, consider removing or renaming it as _ (revive)
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 {
Expand All @@ -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
}
6 changes: 3 additions & 3 deletions pkg/fulcio/certificate/summarize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
30 changes: 29 additions & 1 deletion pkg/testing/ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion pkg/verify/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)).
Expand All @@ -51,6 +55,8 @@ func VerifyLeafCertificate(observerTimestamp time.Time, leafCert *x509.Certifica
},
}

leafCert := verificationContent.GetLeafCertificate()

_, err := leafCert.Verify(opts)
if err == nil {
return nil
Expand Down
63 changes: 51 additions & 12 deletions pkg/verify/certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
}
})
Expand Down
3 changes: 2 additions & 1 deletion pkg/verify/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Loading

0 comments on commit 410da84

Please sign in to comment.