feat(fulcioroots): singleton error pattern (#1965)

Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com> Co-authored-by: Furkan Türkal <furkan.turkal@trendyol.com> Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com> Co-authored-by: Furkan Türkal <furkan.turkal@trendyol.com>
sigstore · Jun 7, 2022 · 75b2bd1 · 75b2bd1
1 parent 63fc4b5
commit 75b2bd1
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 35 deletions.
diff --git a/cmd/cosign/cli/fulcio/fulcio.go b/cmd/cosign/cli/fulcio/fulcio.go
@@ -186,11 +186,11 @@ func (f *Signer) PublicKey(opts ...signature.PublicKeyOption) (crypto.PublicKey,
 
 var _ signature.Signer = &Signer{}
 
-func GetRoots() *x509.CertPool {
+func GetRoots() (*x509.CertPool, error) {
 	return fulcioroots.Get()
 }
 
-func GetIntermediates() *x509.CertPool {
+func GetIntermediates() (*x509.CertPool, error) {
 	return fulcioroots.GetIntermediates()
 }
 

diff --git a/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go b/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go
@@ -29,9 +29,10 @@ import (
 )
 
 var (
-	rootsOnce     sync.Once
-	roots         *x509.CertPool
-	intermediates *x509.CertPool
+	rootsOnce        sync.Once
+	roots            *x509.CertPool
+	intermediates    *x509.CertPool
+	singletonRootErr error
 )
 
 // This is the root in the fulcio project.
@@ -61,26 +62,24 @@ const (
 	altRoot = "SIGSTORE_ROOT_FILE"
 )
 
-func Get() *x509.CertPool {
+func Get() (*x509.CertPool, error) {
 	rootsOnce.Do(func() {
-		var err error
-		roots, intermediates, err = initRoots()
-		if err != nil {
-			panic(err)
+		roots, intermediates, singletonRootErr = initRoots()
+		if singletonRootErr != nil {
+			return
 		}
 	})
-	return roots
+	return roots, singletonRootErr
 }
 
-func GetIntermediates() *x509.CertPool {
+func GetIntermediates() (*x509.CertPool, error) {
 	rootsOnce.Do(func() {
-		var err error
-		roots, intermediates, err = initRoots()
-		if err != nil {
-			panic(err)
+		roots, intermediates, singletonRootErr = initRoots()
+		if singletonRootErr != nil {
+			return
 		}
 	})
-	return intermediates
+	return intermediates, singletonRootErr
 }
 
 func initRoots() (*x509.CertPool, *x509.CertPool, error) {

diff --git a/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots_test.go b/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots_test.go
@@ -42,15 +42,17 @@ func TestGetFulcioRoots(t *testing.T) {
 	}
 	t.Setenv("SIGSTORE_ROOT_FILE", tmpCertFile.Name())
 
-	rootCertPool := Get()
-	// ignore deprecation error because certificates do not contain from SystemCertPool
-	if len(rootCertPool.Subjects()) != 1 { // nolint:staticcheck
+	if rootCertPool, re := Get(); err != nil {
+		t.Fatalf("failed to get roots: %v", re)
+	} else if len(rootCertPool.Subjects()) != 1 { // nolint:staticcheck
+		// ignore deprecation error because certificates do not contain from SystemCertPool
 		t.Errorf("expected 1 root certificate, got 0")
 	}
 
-	subCertPool := GetIntermediates()
-	// ignore deprecation error because certificates do not contain from SystemCertPool
-	if len(subCertPool.Subjects()) != 1 { // nolint:staticcheck
+	if subCertPool, ie := GetIntermediates(); err != nil {
+		t.Fatalf("failed to get intermediates: %v", ie)
+	} else if len(subCertPool.Subjects()) != 1 { // nolint:staticcheck
+		// ignore deprecation error because certificates do not contain from SystemCertPool
 		t.Errorf("expected 1 intermediate certificate, got 0")
 	}
 }
diff --git a/cmd/cosign/cli/verify/verify.go b/cmd/cosign/cli/verify/verify.go
@@ -110,8 +110,14 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) {
 			}
 			co.RekorClient = rekorClient
 		}
-		co.RootCerts = fulcio.GetRoots()
-		co.IntermediateCerts = fulcio.GetIntermediates()
+		co.RootCerts, err = fulcio.GetRoots()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio roots: %w", err)
+		}
+		co.IntermediateCerts, err = fulcio.GetIntermediates()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio intermediates: %w", err)
+		}
 	}
 	keyRef := c.KeyRef
 	certRef := c.CertRef

diff --git a/cmd/cosign/cli/verify/verify_attestation.go b/cmd/cosign/cli/verify/verify_attestation.go
@@ -91,8 +91,14 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e
 			}
 			co.RekorClient = rekorClient
 		}
-		co.RootCerts = fulcio.GetRoots()
-		co.IntermediateCerts = fulcio.GetIntermediates()
+		co.RootCerts, err = fulcio.GetRoots()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio roots: %w", err)
+		}
+		co.IntermediateCerts, err = fulcio.GetIntermediates()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio intermediates: %w", err)
+		}
 	}
 	keyRef := c.KeyRef
 

diff --git a/cmd/cosign/cli/verify/verify_blob.go b/cmd/cosign/cli/verify/verify_blob.go
@@ -204,12 +204,20 @@ func verifySigByUUID(ctx context.Context, ko options.KeyOpts, rClient *client.Re
 		}
 
 		co := &cosign.CheckOpts{
-			RootCerts:         fulcio.GetRoots(),
-			IntermediateCerts: fulcio.GetIntermediates(),
-			CertEmail:         certEmail,
-			CertOidcIssuer:    certOidcIssuer,
-			EnforceSCT:        enforceSCT,
+			CertEmail:      certEmail,
+			CertOidcIssuer: certOidcIssuer,
+			EnforceSCT:     enforceSCT,
 		}
+
+		co.RootCerts, err = fulcio.GetRoots()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio roots: %w", err)
+		}
+		co.IntermediateCerts, err = fulcio.GetIntermediates()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio intermediates: %w", err)
+		}
+
 		cert := certs[0]
 		verifier, err := cosign.ValidateAndUnpackCert(cert, co)
 		if err != nil {

diff --git a/pkg/cosign/kubernetes/webhook/validation.go b/pkg/cosign/kubernetes/webhook/validation.go
@@ -39,7 +39,11 @@ import (
 func valid(ctx context.Context, ref name.Reference, rekorClient *client.Rekor, keys []crypto.PublicKey, opts ...ociremote.Option) ([]oci.Signature, error) {
 	if len(keys) == 0 {
 		// If there are no keys, then verify against the fulcio root.
-		sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroots.Get(), nil /* rekor */, nil /* no identities */, opts...)
+		fulcioRoots, err := fulcioroots.Get()
+		if err != nil {
+			return nil, err
+		}
+		sps, err := validSignaturesWithFulcio(ctx, ref, fulcioRoots, nil /* rekor */, nil /* no identities */, opts...)
 		if err != nil {
 			return nil, err
 		}

diff --git a/pkg/sget/sget.go b/pkg/sget/sget.go
@@ -18,6 +18,7 @@ package sget
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io"
 
 	"github.com/google/go-containerregistry/pkg/authn"
@@ -90,8 +91,14 @@ func (sg *SecureGet) Do(ctx context.Context) error {
 		// was performed so we don't need to use this fragile logic here.
 		fulcioVerified := (co.SigVerifier == nil)
 
-		co.RootCerts = fulcio.GetRoots()
-		co.IntermediateCerts = fulcio.GetIntermediates()
+		co.RootCerts, err = fulcio.GetRoots()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio roots: %w", err)
+		}
+		co.IntermediateCerts, err = fulcio.GetIntermediates()
+		if err != nil {
+			return fmt.Errorf("getting Fulcio intermediates: %w", err)
+		}
 
 		sp, bundleVerified, err := cosign.VerifyImageSignatures(ctx, ref, co)
 		if err != nil {