From 9de6cf4b7c8b39ee9d623f866717eb45644e38fa Mon Sep 17 00:00:00 2001 From: Hayden Blauzvern Date: Tue, 26 Apr 2022 17:06:45 +0000 Subject: [PATCH] Load in intermediate cert pool from TUF With the v3 TUF root, the intermediate CA certificate will be included, so that if the intermediate signing key was compromised, the intermediate certificate could be revoked by removing it from the TUF targets and replacing it with a trusted certificate. This change loads the intermediate certificate from TUF. However, we don't want to force all users to follow this structure - They may choose to use CRLs to detect revoked intermediates. Also, I don't want to enforce TUF usage in the Verify package. Therefore, for TUF, we lazily create a certificate pool only if an intermediate certificate is found, and if it's not found, then VerifyImageSignature will create a pool using the chain provided in the annotation. Signed-off-by: Hayden Blauzvern --- .../cli/fulcio/fulcioroots/fulcioroots.go | 19 ++++++++-- cmd/cosign/cli/verify/verify.go | 1 + cmd/cosign/cli/verify/verify_attestation.go | 1 + pkg/cosign/verify.go | 6 ++-- pkg/cosign/verify_test.go | 36 +++++++++++++++++++ pkg/sget/sget.go | 1 + 6 files changed, 60 insertions(+), 4 deletions(-) diff --git a/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go b/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go index db1fc460c7e..c0890bd77c2 100644 --- a/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go +++ b/cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go @@ -83,8 +83,8 @@ func GetIntermediates() *x509.CertPool { } func initRoots() (*x509.CertPool, *x509.CertPool, error) { - rootPool := x509.NewCertPool() - intermediatePool := x509.NewCertPool() + var rootPool *x509.CertPool + var intermediatePool *x509.CertPool rootEnv := os.Getenv(altRoot) if rootEnv != "" { @@ -99,8 +99,14 @@ func initRoots() (*x509.CertPool, *x509.CertPool, error) { for _, cert := range certs { // root certificates are self-signed if bytes.Equal(cert.RawSubject, cert.RawIssuer) { + if rootPool == nil { + rootPool = x509.NewCertPool() + } rootPool.AddCert(cert) } else { + if intermediatePool == nil { + intermediatePool = x509.NewCertPool() + } intermediatePool.AddCert(cert) } } @@ -127,12 +133,21 @@ func initRoots() (*x509.CertPool, *x509.CertPool, error) { for _, cert := range certs { // root certificates are self-signed if bytes.Equal(cert.RawSubject, cert.RawIssuer) { + if rootPool == nil { + rootPool = x509.NewCertPool() + } rootPool.AddCert(cert) } else { + if intermediatePool == nil { + intermediatePool = x509.NewCertPool() + } intermediatePool.AddCert(cert) } } } + if intermediatePool == nil { + intermediatePool = x509.NewCertPool() + } intermediatePool.AppendCertsFromPEM([]byte(fulcioIntermediateV1)) } return rootPool, intermediatePool, nil diff --git a/cmd/cosign/cli/verify/verify.go b/cmd/cosign/cli/verify/verify.go index e55381e0b1d..fb9595fd54a 100644 --- a/cmd/cosign/cli/verify/verify.go +++ b/cmd/cosign/cli/verify/verify.go @@ -111,6 +111,7 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) { co.RekorClient = rekorClient } co.RootCerts = fulcio.GetRoots() + co.IntermediateCerts = fulcio.GetIntermediates() } keyRef := c.KeyRef certRef := c.CertRef diff --git a/cmd/cosign/cli/verify/verify_attestation.go b/cmd/cosign/cli/verify/verify_attestation.go index 8ab95fd7dac..93867633976 100644 --- a/cmd/cosign/cli/verify/verify_attestation.go +++ b/cmd/cosign/cli/verify/verify_attestation.go @@ -92,6 +92,7 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e co.RekorClient = rekorClient } co.RootCerts = fulcio.GetRoots() + co.IntermediateCerts = fulcio.GetIntermediates() } keyRef := c.KeyRef diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go index f60c40ead31..fddc5092425 100644 --- a/pkg/cosign/verify.go +++ b/pkg/cosign/verify.go @@ -475,7 +475,8 @@ func VerifyImageSignature(ctx context.Context, sig oci.Signature, h v1.Hash, co // If the chain annotation is not present or there is only a root if chain == nil || len(chain) <= 1 { co.IntermediateCerts = nil - } else { + } else if co.IntermediateCerts == nil { + // If the intermediate certs have not been loaded in by TUF pool := x509.NewCertPool() for _, cert := range chain[:len(chain)-1] { pool.AddCert(cert) @@ -653,7 +654,8 @@ func verifyImageAttestations(ctx context.Context, atts oci.Signatures, h v1.Hash // If the chain annotation is not present or there is only a root if chain == nil || len(chain) <= 1 { co.IntermediateCerts = nil - } else { + } else if co.IntermediateCerts == nil { + // If the intermediate certs have not been loaded in by TUF pool := x509.NewCertPool() for _, cert := range chain[:len(chain)-1] { pool.AddCert(cert) diff --git a/pkg/cosign/verify_test.go b/pkg/cosign/verify_test.go index 6208cd02032..2c38ccd0cc4 100644 --- a/pkg/cosign/verify_test.go +++ b/pkg/cosign/verify_test.go @@ -309,6 +309,42 @@ func TestVerifyImageSignatureWithMissingSub(t *testing.T) { } } +func TestVerifyImageSignatureWithExistingSub(t *testing.T) { + rootCert, rootKey, _ := test.GenerateRootCa() + subCert, subKey, _ := test.GenerateSubordinateCa(rootCert, rootKey) + leafCert, privKey, _ := test.GenerateLeafCert("subject", "oidc-issuer", subCert, subKey) + pemRoot := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: rootCert.Raw}) + pemSub := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: subCert.Raw}) + pemLeaf := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: leafCert.Raw}) + + otherSubCert, _, _ := test.GenerateSubordinateCa(rootCert, rootKey) + + rootPool := x509.NewCertPool() + rootPool.AddCert(rootCert) + subPool := x509.NewCertPool() + // Load in different sub cert so the chain doesn't verify + rootPool.AddCert(otherSubCert) + + payload := []byte{1, 2, 3, 4} + h := sha256.Sum256(payload) + signature, _ := privKey.Sign(rand.Reader, h[:], crypto.SHA256) + + ociSig, _ := static.NewSignature(payload, + base64.StdEncoding.EncodeToString(signature), + static.WithCertChain(pemLeaf, appendSlices([][]byte{pemSub, pemRoot}))) + verified, err := VerifyImageSignature(context.TODO(), ociSig, v1.Hash{}, &CheckOpts{RootCerts: rootPool, IntermediateCerts: subPool}) + if err == nil { + t.Fatal("expected error while verifying signature") + } + if !strings.Contains(err.Error(), "certificate signed by unknown authority") { + t.Fatal("expected error while verifying signature") + } + // TODO: Create fake bundle and test verification + if verified == true { + t.Fatalf("expected verified=false, got verified=true") + } +} + func TestValidateAndUnpackCertSuccess(t *testing.T) { subject := "email@email" oidcIssuer := "https://accounts.google.com" diff --git a/pkg/sget/sget.go b/pkg/sget/sget.go index f61e23b8080..2eba346d96f 100644 --- a/pkg/sget/sget.go +++ b/pkg/sget/sget.go @@ -91,6 +91,7 @@ func (sg *SecureGet) Do(ctx context.Context) error { fulcioVerified := (co.SigVerifier == nil) co.RootCerts = fulcio.GetRoots() + co.IntermediateCerts = fulcio.GetIntermediates() sp, bundleVerified, err := cosign.VerifyImageSignatures(ctx, ref, co) if err != nil {