From 7ad31eed5a48fa547caa48757a43cc8458cc65de Mon Sep 17 00:00:00 2001
From: Hayden Blauzvern <hblauzvern@google.com>
Date: Tue, 13 Sep 2022 22:02:42 +0000
Subject: [PATCH] Fix BYO-root with intermediate to fetch intermediates from
 annotation

This fixes a regression where intermediates, when not present in the
intermediate certificate pool, would be fetched from the OCI chain
annotation.

Ref #1554, which includes more details

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
---
 .../cosign/fulcio/fulcioroots/fulcioroots.go  |  6 ++-
 .../fulcio/fulcioroots/fulcioroots_test.go    | 43 +++++++++++++++++--
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots.go b/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots.go
index c02db558d7f..4403c529a90 100644
--- a/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots.go
+++ b/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots.go
@@ -59,7 +59,8 @@ func GetIntermediates() (*x509.CertPool, error) {
 
 func initRoots() (*x509.CertPool, *x509.CertPool, error) {
 	rootPool := x509.NewCertPool()
-	intermediatePool := x509.NewCertPool()
+	// intermediatePool should be nil if no intermediates are found
+	var intermediatePool *x509.CertPool
 
 	rootEnv := os.Getenv(altRoot)
 	if rootEnv != "" {
@@ -76,6 +77,9 @@ func initRoots() (*x509.CertPool, *x509.CertPool, error) {
 			if bytes.Equal(cert.RawSubject, cert.RawIssuer) {
 				rootPool.AddCert(cert)
 			} else {
+				if intermediatePool == nil {
+					intermediatePool = x509.NewCertPool()
+				}
 				intermediatePool.AddCert(cert)
 			}
 		}
diff --git a/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots_test.go b/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots_test.go
index 7fcacf59bdd..b21331eb444 100644
--- a/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots_test.go
+++ b/internal/pkg/cosign/fulcio/fulcioroots/fulcioroots_test.go
@@ -16,13 +16,19 @@ package fulcioroots
 
 import (
 	"os"
+	"sync"
 	"testing"
 
 	"github.com/sigstore/cosign/test"
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 )
 
+func resetState() {
+	rootsOnce = sync.Once{}
+}
+
 func TestGetFulcioRoots(t *testing.T) {
+	t.Cleanup(resetState)
 	rootCert, rootPriv, _ := test.GenerateRootCa()
 	rootPemCert, _ := cryptoutils.MarshalCertificateToPEM(rootCert)
 	subCert, _, _ := test.GenerateSubordinateCa(rootCert, rootPriv)
@@ -42,17 +48,46 @@ func TestGetFulcioRoots(t *testing.T) {
 	}
 	t.Setenv("SIGSTORE_ROOT_FILE", tmpCertFile.Name())
 
-	if rootCertPool, re := Get(); err != nil {
-		t.Fatalf("failed to get roots: %v", re)
+	if rootCertPool, err := Get(); err != nil {
+		t.Fatalf("failed to get roots: %v", err)
 	} 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")
 	}
 
-	if subCertPool, ie := GetIntermediates(); err != nil {
-		t.Fatalf("failed to get intermediates: %v", ie)
+	if subCertPool, err := GetIntermediates(); err != nil {
+		t.Fatalf("failed to get intermediates: %v", err)
 	} 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")
 	}
 }
+
+func TestGetFulcioRootsWithoutIntermediate(t *testing.T) {
+	t.Cleanup(resetState)
+	rootCert, _, _ := test.GenerateRootCa()
+	rootPemCert, _ := cryptoutils.MarshalCertificateToPEM(rootCert)
+
+	tmpCertFile, err := os.CreateTemp(t.TempDir(), "cosign_fulcio_root_*.cert")
+	if err != nil {
+		t.Fatalf("failed to create temp cert file: %v", err)
+	}
+	defer tmpCertFile.Close()
+	if _, err := tmpCertFile.Write(rootPemCert); err != nil {
+		t.Fatalf("failed to write cert file: %v", err)
+	}
+	t.Setenv("SIGSTORE_ROOT_FILE", tmpCertFile.Name())
+
+	if rootCertPool, err := Get(); err != nil {
+		t.Fatalf("failed to get roots: %v", err)
+	} 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")
+	}
+
+	if subCertPool, err := GetIntermediates(); err != nil {
+		t.Fatalf("failed to get intermediates: %v", err)
+	} else if subCertPool != nil {
+		t.Errorf("expected no intermediate cert pool")
+	}
+}