Skip to content

Commit

Permalink
Add remove_roots_from_chain to sign and issue pki apis (#16935)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
stevendpclark authored Aug 31, 2022
1 parent a36f4a0 commit 0636467
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 11 deletions.
17 changes: 16 additions & 1 deletion builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{}) {
Expand Down
7 changes: 7 additions & 0 deletions builtin/logical/pki/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
64 changes: 54 additions & 10 deletions builtin/logical/pki/path_issue_sign.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package pki

import (
"bytes"
"context"
"crypto/rand"
"encoding/base64"
"encoding/pem"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
`
Expand Down
3 changes: 3 additions & 0 deletions changelog/16935.txt
Original file line number Diff line number Diff line change
@@ -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
```
8 changes: 8 additions & 0 deletions website/content/api-docs/secret/pki.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0636467

Please sign in to comment.