From 5fb330b3560432bba58f6d011173d5537c7f34c8 Mon Sep 17 00:00:00 2001
From: kitography <khaines@mit.edu>
Date: Fri, 10 Jan 2025 08:59:18 -0500
Subject: [PATCH] Move all pki-verification calls from sdk-Verify() to
 pki-specific VerifyCertifcate(...); update sdk-Verify to allow multiple
 chains, but validate that at least one of those chains is valid.

---
 builtin/logical/pki/path_acme_order.go |  2 +-
 builtin/logical/pki/path_root.go       |  4 +--
 sdk/helper/certutil/types.go           | 49 +++++++++++++++++++-------
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/builtin/logical/pki/path_acme_order.go b/builtin/logical/pki/path_acme_order.go
index ab80ee38eb28..10b47c84dd57 100644
--- a/builtin/logical/pki/path_acme_order.go
+++ b/builtin/logical/pki/path_acme_order.go
@@ -580,7 +580,7 @@ func issueCertFromCsr(ac *acmeContext, csr *x509.CertificateRequest) (*certutil.
 		return nil, "", fmt.Errorf("%w: refusing to sign CSR: %s", ErrBadCSR, err.Error())
 	}
 
-	if err = parsedBundle.Verify(); err != nil {
+	if err = issuing.VerifyCertificate(ac.Context, ac.sc.Storage, issuerId, parsedBundle); err != nil {
 		return nil, "", fmt.Errorf("verification of parsed bundle failed: %w", err)
 	}
 
diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go
index a4ce2d25108b..d5f9b9053314 100644
--- a/builtin/logical/pki/path_root.go
+++ b/builtin/logical/pki/path_root.go
@@ -366,7 +366,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R
 
 	var caErr error
 	sc := b.makeStorageContext(ctx, req.Storage)
-	signingBundle, caErr := sc.fetchCAInfo(issuerName, issuing.IssuanceUsage)
+	signingBundle, issuerId, caErr := sc.fetchCAInfoWithIssuer(issuerName, issuing.IssuanceUsage)
 	if caErr != nil {
 		switch caErr.(type) {
 		case errutil.UserError:
@@ -413,7 +413,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R
 		}
 	}
 
-	if err := parsedBundle.Verify(); err != nil {
+	if err := issuing.VerifyCertificate(sc.GetContext(), sc.GetStorage(), issuerId, parsedBundle); err != nil {
 		return nil, fmt.Errorf("verification of parsed bundle failed: %w", err)
 	}
 
diff --git a/sdk/helper/certutil/types.go b/sdk/helper/certutil/types.go
index 398a58e210df..76d42e14acb8 100644
--- a/sdk/helper/certutil/types.go
+++ b/sdk/helper/certutil/types.go
@@ -370,7 +370,9 @@ func (p *ParsedCertBundle) ToCertBundle() (*CertBundle, error) {
 
 // Verify checks if the parsed bundle is valid.  It validates the public
 // key of the certificate to the private key and checks the certificate trust
-// chain for path issues.
+// chain for path issues.  Inside the PKI secrets engine, the more configurable
+// builtin/logical/pki/issuing/cert_verify VerifyCertificate should be used
+// instead.
 func (p *ParsedCertBundle) Verify() error {
 	// If private key exists, check if it matches the public key of cert
 	if p.PrivateKey != nil && p.Certificate != nil {
@@ -383,19 +385,40 @@ func (p *ParsedCertBundle) Verify() error {
 		}
 	}
 
-	certPath := p.GetCertificatePath()
-	if len(certPath) > 1 {
-		for i, caCert := range certPath[1:] {
-			if !caCert.Certificate.IsCA {
-				return fmt.Errorf("certificate %d of certificate chain is not a certificate authority", i+1)
-			}
-			if !bytes.Equal(certPath[i].Certificate.AuthorityKeyId, caCert.Certificate.SubjectKeyId) {
-				return fmt.Errorf("certificate %d of certificate chain ca trust path is incorrect (%q/%q) (%X/%X)",
-					i+1,
-					certPath[i].Certificate.Subject.CommonName, caCert.Certificate.Subject.CommonName,
-					certPath[i].Certificate.AuthorityKeyId, caCert.Certificate.SubjectKeyId)
-			}
+	rootCertPool := x509.NewCertPool()
+	intermediateCertPool := x509.NewCertPool()
+
+	for index, certBytes := range p.CAChain {
+		parsedCert, err := x509.ParseCertificate(certBytes.Bytes)
+		if err != nil {
+			return fmt.Errorf("could not parse certificate number %v in chain: %w", index, err)
 		}
+		if bytes.Equal(parsedCert.RawIssuer, parsedCert.RawSubject) {
+			rootCertPool.AddCert(parsedCert)
+		} else {
+			intermediateCertPool.AddCert(parsedCert)
+		}
+		if index > 0 && !parsedCert.IsCA {
+			return fmt.Errorf("certificate %v is not a CA certificate", index)
+		}
+	}
+	emptyCertPool := x509.NewCertPool()
+
+	if rootCertPool.Equal(emptyCertPool) {
+		// Alright, this is weird, since we don't have the root CA, we'll treat the intermediate as
+		// the root, otherwise we'll get a "x509: certificate signed by unknown authority" error.
+		rootCertPool, intermediateCertPool = intermediateCertPool, rootCertPool
+	}
+
+	options := x509.VerifyOptions{
+		Roots:         rootCertPool,
+		Intermediates: intermediateCertPool,
+		CurrentTime:   time.Now(),
+	}
+
+	_, err := p.Certificate.Verify(options)
+	if err != nil {
+		return err
 	}
 
 	return nil