Add remove_roots_from_chain to sign and issue pki apis (#16935)

* Add remove_roots_from_chain flag to sign and issue pki apis

 - Add a new flag to allow end-users to control if we return the
   root/self-signed CA certificate within the list of certificates in
   ca_chain field on issue and sign api calls.

* Add cl

* PR feedback

builtin/logical/pki/backend_test.go

@@ -4046,7 +4046,7 @@ func runFullCAChainTest(t *testing.T, keyType string) {
 
 	resp, err = CBWrite(b_root, s_root, "root/sign-intermediate", map[string]interface{}{
 		"csr":    intermediateData["csr"],
-		"format": "pem_bundle",
+		"format": "pem",
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -4154,6 +4154,21 @@ func runFullCAChainTest(t *testing.T, keyType string) {
 
 	// Verify that the certificates are signed by the intermediary CA key...
 	requireSignedBy(t, issuedCrt, intermediaryCaCert.PublicKey)
+
+	// Test that we can request that the root ca certificate not appear in the ca_chain field
+	resp, err = CBWrite(b_ext, s_ext, "issue/example", map[string]interface{}{
+		"common_name":             "test.example.com",
+		"ttl":                     "5m",
+		"remove_roots_from_chain": "true",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "error issuing certificate when removing self signed")
+	fullChain = strings.Join(resp.Data["ca_chain"].([]string), "\n")
+	if strings.Count(fullChain, intermediateCert) != 1 {
+		t.Fatalf("expected full chain to contain intermediate certificate; got %v occurrences", strings.Count(fullChain, intermediateCert))
+	}
+	if strings.Count(fullChain, rootCert) != 0 {
+		t.Fatalf("expected full chain to NOT contain root certificate; got %v occurrences", strings.Count(fullChain, rootCert))
+	}
 }
 
 func requireCertInCaChainArray(t *testing.T, chain []string, cert string, msgAndArgs ...interface{}) {

builtin/logical/pki/fields.go

@@ -145,6 +145,13 @@ be larger than the role max TTL.`,
 The value format should be given in UTC format YYYY-MM-ddTHH:MM:SSZ`,
 	}
 
+	fields["remove_roots_from_chain"] = &framework.FieldSchema{
+		Type:    framework.TypeBool,
+		Default: false,
+		Description: `Whether or not to remove self-signed CA certificates in the output
+of the ca_chain field.`,
+	}
+
 	fields = addIssuerRefField(fields)
 
 	return fields

builtin/logical/pki/path_issue_sign.go

@@ -1,9 +1,11 @@
 package pki
 
 import (
+	"bytes"
 	"context"
 	"crypto/rand"
 	"encoding/base64"
+	"encoding/pem"
 	"fmt"
 	"strings"
 	"time"
@@ -333,6 +335,8 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
 		return nil, fmt.Errorf("error converting raw cert bundle to cert bundle: %w", err)
 	}
 
+	caChainGen := newCaChainOutput(parsedBundle, data)
+
 	respData := map[string]interface{}{
 		"expiration":    int64(parsedBundle.Certificate.NotAfter.Unix()),
 		"serial_number": cb.SerialNumber,
@@ -342,8 +346,8 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
 	case "pem":
 		respData["issuing_ca"] = signingCB.Certificate
 		respData["certificate"] = cb.Certificate
-		if cb.CAChain != nil && len(cb.CAChain) > 0 {
+		if caChainGen.containsChain() {
-			respData["ca_chain"] = cb.CAChain
+			respData["ca_chain"] = caChainGen.pemEncodedChain()
 		}
 		if !useCSR {
 			respData["private_key"] = cb.PrivateKey
@@ -353,8 +357,8 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
 	case "pem_bundle":
 		respData["issuing_ca"] = signingCB.Certificate
 		respData["certificate"] = cb.ToPEMBundle()
-		if cb.CAChain != nil && len(cb.CAChain) > 0 {
+		if caChainGen.containsChain() {
-			respData["ca_chain"] = cb.CAChain
+			respData["ca_chain"] = caChainGen.pemEncodedChain()
 		}
 		if !useCSR {
 			respData["private_key"] = cb.PrivateKey
@@ -365,12 +369,8 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
 		respData["certificate"] = base64.StdEncoding.EncodeToString(parsedBundle.CertificateBytes)
 		respData["issuing_ca"] = base64.StdEncoding.EncodeToString(signingBundle.CertificateBytes)
 
-		var caChain []string
+		if caChainGen.containsChain() {
-		for _, caCert := range parsedBundle.CAChain {
+			respData["ca_chain"] = caChainGen.derEncodedChain()
-			caChain = append(caChain, base64.StdEncoding.EncodeToString(caCert.Bytes))
-		}
-		if caChain != nil && len(caChain) > 0 {
-			respData["ca_chain"] = caChain
 		}
 
 		if !useCSR {
@@ -429,6 +429,50 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d
 	return resp, nil
 }
 
+type caChainOutput struct {
+	chain []*certutil.CertBlock
+}
+
+func newCaChainOutput(parsedBundle *certutil.ParsedCertBundle, data *framework.FieldData) caChainOutput {
+	if filterCaChain := data.Get("remove_roots_from_chain").(bool); filterCaChain {
+		var myChain []*certutil.CertBlock
+		for _, certBlock := range parsedBundle.CAChain {
+			cert := certBlock.Certificate
+
+			if (len(cert.AuthorityKeyId) > 0 && !bytes.Equal(cert.AuthorityKeyId, cert.SubjectKeyId)) ||
+				(len(cert.AuthorityKeyId) == 0 && (!bytes.Equal(cert.RawIssuer, cert.RawSubject) || cert.CheckSignatureFrom(cert) != nil)) {
+				// We aren't self-signed so add it to the list.
+				myChain = append(myChain, certBlock)
+			}
+		}
+		return caChainOutput{chain: myChain}
+	}
+
+	return caChainOutput{chain: parsedBundle.CAChain}
+}
+
+func (cac *caChainOutput) containsChain() bool {
+	return len(cac.chain) > 0
+}
+
+func (cac *caChainOutput) pemEncodedChain() []string {
+	var chain []string
+	for _, cert := range cac.chain {
+		block := pem.Block{Type: "CERTIFICATE", Bytes: cert.Bytes}
+		certificate := strings.TrimSpace(string(pem.EncodeToMemory(&block)))
+		chain = append(chain, certificate)
+	}
+	return chain
+}
+
+func (cac *caChainOutput) derEncodedChain() []string {
+	var derCaChain []string
+	for _, caCert := range cac.chain {
+		derCaChain = append(derCaChain, base64.StdEncoding.EncodeToString(caCert.Bytes))
+	}
+	return derCaChain
+}
+
 const pathIssueHelpSyn = `
 Request a certificate using a certain role with the provided details.
 `

changelog/16935.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Add a new flag to issue/sign APIs which can filter out root CAs from the returned ca_chain field
+```

website/content/api-docs/secret/pki.mdx

@@ -402,6 +402,10 @@ It is suggested to limit access to the path-overridden sign endpoint (on
   `YYYY-MM-ddTHH:MM:SSZ`. Supports the Y10K end date for IEEE 802.1AR-2018
   standard devices, `9999-12-31T23:59:59Z`.
 
+- `remove_roots_from_chain` `(bool: false)` - If true, the returned `ca_chain`
+  field will not include any self-signed CA certificates. Useful if end-users
+  already have the root CA in their trust store.
+
 #### Sample Payload
 
 ```json
@@ -783,6 +787,10 @@ have access.**
   over PKCS#1v1.5 signatures when a RSA-type issuer is used. Ignored for
   ECDSA/Ed25519 issuers.
 
+- `remove_roots_from_chain` `(bool: false)` - If true, the returned `ca_chain`
+  field will not include any self-signed CA certificates. Useful if end-users
+  already have the root CA in their trust store.
+
 #### Sample Payload
 
 ```json