From b21e06b91702b609a2bfbffff3ffefd8bac65d24 Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Wed, 31 Aug 2022 09:51:26 -0400 Subject: [PATCH] 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 | 17 ++++++- builtin/logical/pki/fields.go | 7 +++ builtin/logical/pki/path_issue_sign.go | 64 +++++++++++++++++++++---- changelog/16935.txt | 3 ++ website/content/api-docs/secret/pki.mdx | 8 ++++ 5 files changed, 88 insertions(+), 11 deletions(-) create mode 100644 changelog/16935.txt diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 27256a008..9f5b5b91d 100644 --- a/builtin/logical/pki/backend_test.go +++ b/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{}) { diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index 1444c721e..d977735e9 100644 --- a/builtin/logical/pki/fields.go +++ b/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 diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 79b47bdf2..c50898598 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/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 { - respData["ca_chain"] = cb.CAChain + if caChainGen.containsChain() { + 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 { - respData["ca_chain"] = cb.CAChain + if caChainGen.containsChain() { + 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 - for _, caCert := range parsedBundle.CAChain { - caChain = append(caChain, base64.StdEncoding.EncodeToString(caCert.Bytes)) - } - if caChain != nil && len(caChain) > 0 { - respData["ca_chain"] = caChain + if caChainGen.containsChain() { + respData["ca_chain"] = caChainGen.derEncodedChain() } 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. ` diff --git a/changelog/16935.txt b/changelog/16935.txt new file mode 100644 index 000000000..0b0b46fd1 --- /dev/null +++ b/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 +``` diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index c43a9ea89..112cf53ef 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/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