Skip to content

Commit

Permalink
Fix BYO-root with intermediate to fetch intermediates from annotation
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
haydentherapper committed Sep 13, 2022
1 parent 01492c6 commit de2ae2a
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 5 deletions.
6 changes: 5 additions & 1 deletion internal/pkg/cosign/fulcio/fulcioroots/fulcioroots.go
Expand Up @@ -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 != "" {
Expand All @@ -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)
}
}
Expand Down
43 changes: 39 additions & 4 deletions internal/pkg/cosign/fulcio/fulcioroots/fulcioroots_test.go
Expand Up @@ -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)
Expand All @@ -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")
}
}

0 comments on commit de2ae2a

Please sign in to comment.