return [][]*x509.Certificate

Signed-off-by: Bob Callaway <bcallaway@google.com>
sigstore · Jul 19, 2022 · a3fcc08 · a3fcc08
1 parent fb15fe9
commit a3fcc08
Show file tree

Hide file tree

Showing 9 changed files with 69 additions and 48 deletions.
diff --git a/cmd/app/http_test.go b/cmd/app/http_test.go
@@ -113,6 +113,6 @@ type TrivialCertificateAuthority struct {
 func (tca *TrivialCertificateAuthority) CreateCertificate(context.Context, identity.Principal, crypto.PublicKey) (*ca.CodeSigningCertificate, error) {
 	return nil, errors.New("CreateCertificate always fails for testing")
 }
-func (tca *TrivialCertificateAuthority) Root(ctx context.Context) ([]*x509.Certificate, error) {
-	return []*x509.Certificate{}, nil
+func (tca *TrivialCertificateAuthority) TrustBundle(ctx context.Context) ([][]*x509.Certificate, error) {
+	return [][]*x509.Certificate{}, nil
 }
diff --git a/pkg/ca/baseca/baseca.go b/pkg/ca/baseca/baseca.go
@@ -139,7 +139,7 @@ func (bca *BaseCA) CreateCertificate(ctx context.Context, principal identity.Pri
 	return ca.CreateCSCFromDER(finalCertBytes, certChain)
 }
 
-func (bca *BaseCA) Root(ctx context.Context) ([]*x509.Certificate, error) {
+func (bca *BaseCA) TrustBundle(ctx context.Context) ([][]*x509.Certificate, error) {
 	certs, _ := bca.GetSignerWithChain()
-	return certs, nil
+	return [][]*x509.Certificate{certs}, nil
 }
diff --git a/pkg/ca/baseca/baseca_test.go b/pkg/ca/baseca/baseca_test.go
@@ -46,12 +46,15 @@ func TestBaseCARoot(t *testing.T) {
 		SignerWithChain: &ca.SignerCerts{Certs: certChain, Signer: signer},
 	}
 
-	rootBytes, err := bca.Root(context.TODO())
+	rootChains, err := bca.TrustBundle(context.TODO())
 	if err != nil {
 		t.Fatalf("unexpected error reading root: %v", err)
 	}
+	if len(rootChains) != 1 {
+		t.Fatalf("unexpected number of chains: %d", len(rootChains))
+	}
 
-	if !reflect.DeepEqual(certChain, rootBytes) {
+	if !reflect.DeepEqual(certChain, rootChains[0]) {
 		t.Fatal("expected cert chains to be equivalent")
 	}
 }

diff --git a/pkg/ca/ca.go b/pkg/ca/ca.go
@@ -27,5 +27,5 @@ import (
 // fetching the CA trust bundle.
 type CertificateAuthority interface {
 	CreateCertificate(context.Context, identity.Principal, crypto.PublicKey) (*CodeSigningCertificate, error)
-	Root(ctx context.Context) ([]*x509.Certificate, error)
+	TrustBundle(ctx context.Context) ([][]*x509.Certificate, error)
 }
diff --git a/pkg/ca/googleca/v1/googleca.go b/pkg/ca/googleca/v1/googleca.go
@@ -136,32 +136,40 @@ func Req(parent string, pemBytes []byte, cert *x509.Certificate) (*privatecapb.C
 	}, nil
 }
 
-func (c *CertAuthorityService) Root(ctx context.Context) ([]*x509.Certificate, error) {
-	c.cachedRootsOnce.Do(func() {
-		var pems string
-		var err error
-		cas := c.client.ListCertificateAuthorities(ctx, &privatecapb.ListCertificateAuthoritiesRequest{
-			Parent: c.parent,
-		})
-		for {
-			c, done := cas.Next()
-			if done == iterator.Done {
-				break
-			}
-			if done != nil {
-				break
-			}
-			pems += strings.Join(c.PemCaCertificates, "")
-		}
-		c.cachedRoots, err = cryptoutils.LoadCertificatesFromPEM(strings.NewReader(pems))
-		if err != nil {
-			panic(fmt.Errorf("failed parsing PemCACertificates: %w", err))
-		}
+func (c *CertAuthorityService) TrustBundle(ctx context.Context) ([][]*x509.Certificate, error) {
+	// if we've already successfully fetched the CA info, just use the cached value
+	if c.cachedRoots != nil {
+		return [][]*x509.Certificate{c.cachedRoots}, nil
+	}
+
+	// fetch the latest values for the specified CA
+	var pems string
+	cas := c.client.ListCertificateAuthorities(ctx, &privatecapb.ListCertificateAuthoritiesRequest{
+		Parent: c.parent,
 	})
-	if len(c.cachedRoots) == 0 {
-		return c.cachedRoots, fmt.Errorf("error fetching root certificates")
+	for {
+		ca, done := cas.Next()
+		if done == iterator.Done {
+			break
+		} else if done != nil {
+			// if the iterator returns an issue for some reason, exit
+			return [][]*x509.Certificate{}, done
+		}
+		pems += strings.Join(ca.PemCaCertificates, "")
 	}
-	return c.cachedRoots, nil
+	// if we fail to parse the PEM content, return an error
+	roots, err := cryptoutils.LoadCertificatesFromPEM(strings.NewReader(pems))
+	if err != nil {
+		return [][]*x509.Certificate{}, fmt.Errorf("failed parsing PemCACertificates response: %w", err)
+	}
+	if len(roots) == 0 {
+		return [][]*x509.Certificate{}, fmt.Errorf("error fetching root certificates")
+	}
+	c.cachedRootsOnce.Do(func() {
+		c.cachedRoots = roots
+	})
+
+	return [][]*x509.Certificate{c.cachedRoots}, nil
 }
 
 func (c *CertAuthorityService) CreateCertificate(ctx context.Context, principal identity.Principal, publicKey crypto.PublicKey) (*ca.CodeSigningCertificate, error) {

diff --git a/pkg/ca/kmsca/kmsca_test.go b/pkg/ca/kmsca/kmsca_test.go
@@ -30,7 +30,6 @@ import (
 	"github.com/sigstore/fulcio/pkg/test"
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	"github.com/sigstore/sigstore/pkg/signature/kms/fake"
-	_ "github.com/sigstore/sigstore/pkg/signature/kms/fake"
 )
 
 func TestNewKMSCA(t *testing.T) {
@@ -56,11 +55,14 @@ func TestNewKMSCA(t *testing.T) {
 	}
 
 	// Expect certificate chain from Root matches provided certificate chain
-	rootBytes, err := ca.Root(context.TODO())
+	rootChains, err := ca.TrustBundle(context.TODO())
 	if err != nil {
 		t.Fatalf("error fetching root: %v", err)
 	}
-	if !reflect.DeepEqual(rootBytes, chain) {
+	if len(rootChains) != 1 {
+		t.Fatalf("unexpected number of chains: %d", len(rootChains))
+	}
+	if !reflect.DeepEqual(rootChains[0], chain) {
 		t.Fatal("cert chains do not match")
 	}
 

diff --git a/pkg/ca/tinkca/tinkca_test.go b/pkg/ca/tinkca/tinkca_test.go
@@ -80,11 +80,14 @@ func TestNewTinkCA(t *testing.T) {
 	}
 
 	// Expect certificate chain from Root matches provided certificate chain
-	rootBytes, err := ca.Root(context.TODO())
+	rootChains, err := ca.TrustBundle(context.TODO())
 	if err != nil {
 		t.Fatalf("error fetching root: %v", err)
 	}
-	if !reflect.DeepEqual(rootBytes, chain) {
+	if len(rootChains) != 1 {
+		t.Fatalf("unexpected number of chains: %d", len(rootChains))
+	}
+	if !reflect.DeepEqual(rootChains[0], chain) {
 		t.Fatal("cert chains do not match")
 	}
 

diff --git a/pkg/server/grpc_server.go b/pkg/server/grpc_server.go
@@ -234,23 +234,28 @@ func (g *grpcCAServer) CreateSigningCertificate(ctx context.Context, request *fu
 func (g *grpcCAServer) GetTrustBundle(ctx context.Context, _ *fulciogrpc.GetTrustBundleRequest) (*fulciogrpc.TrustBundle, error) {
 	logger := log.ContextLogger(ctx)
 
-	root, err := g.ca.Root(ctx)
+	trustBundle, err := g.ca.TrustBundle(ctx)
 	if err != nil {
-		logger.Error("Error retrieving root cert: ", err)
+		logger.Error("Error retrieving trust bundle: ", err)
 		return nil, handleFulcioGRPCError(ctx, codes.Internal, err, genericCAError)
 	}
 
-	tb := &fulciogrpc.TrustBundle{
-		Chains: []*fulciogrpc.CertificateChain{{}},
+	resp := &fulciogrpc.TrustBundle{
+		Chains: []*fulciogrpc.CertificateChain{},
 	}
-	for _, cert := range root {
-		certPEM, err := cryptoutils.MarshalCertificateToPEM(cert)
-		if err != nil {
-			return nil, handleFulcioGRPCError(ctx, codes.Internal, err, genericCAError)
+
+	for _, chain := range trustBundle {
+		certChain := &fulciogrpc.CertificateChain{}
+		for _, cert := range chain {
+			certPEM, err := cryptoutils.MarshalCertificateToPEM(cert)
+			if err != nil {
+				return nil, handleFulcioGRPCError(ctx, codes.Internal, err, genericCAError)
+			}
+			certChain.Certificates = append(certChain.Certificates, string(certPEM))
 		}
-		tb.Chains[0].Certificates = append(tb.Chains[0].Certificates, string(certPEM))
+		resp.Chains = append(resp.Chains, certChain)
 	}
-	return tb, nil
+	return resp, nil
 }
 
 func (g *grpcCAServer) GetConfiguration(ctx context.Context, _ *fulciogrpc.GetConfigurationRequest) (*fulciogrpc.Configuration, error) {

diff --git a/pkg/server/grpc_server_test.go b/pkg/server/grpc_server_test.go
@@ -1463,6 +1463,6 @@ type FailingCertificateAuthority struct {
 func (fca *FailingCertificateAuthority) CreateCertificate(context.Context, identity.Principal, crypto.PublicKey) (*ca.CodeSigningCertificate, error) {
 	return nil, errors.New("CreateCertificate always fails for testing")
 }
-func (fca *FailingCertificateAuthority) Root(ctx context.Context) ([]*x509.Certificate, error) {
-	return nil, errors.New("Root always fails for testing")
+func (fca *FailingCertificateAuthority) TrustBundle(ctx context.Context) ([][]*x509.Certificate, error) {
+	return nil, errors.New("TrustBundle always fails for testing")
 }