Allow Root + Intermediate Key_Usage to be set (#30034)

* outline of key usage fix

* Changelog, and test-fix

* Simplify code setting key_usage

* make fmt

* Per internal dicussion to align closer to the CAB guidelines, only allow DigitalSignature.

* Breaking Change: error if invalid key_usage to generate root or sign-intermediate.

* Change error to warning in order to not break backwards compatibility.

builtin/logical/pki/backend_test.go

@@ -7420,6 +7420,53 @@ func TestIssuance_AlwaysEnforceErr(t *testing.T) {
 	})
 }
 
+// TestIssuance_SignIntermediateKeyUsages tests the field "key_usage" both when directly generating a root certificate,
+// and when signing a CA CSR.  In particular, this test verifies that:
+// - the key usage DigitalSignature is added if present
+// - non-existent or invalid key usages return a warning, but are ignored by certificate generation
+// - CertSign and CRLSign are ignored
+func TestIssuance_SignIntermediateKeyUsages(t *testing.T) {
+	t.Parallel()
+	b, s := CreateBackendWithStorage(t)
+
+	resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+		"common_name": "root myvault.com",
+		"key_type":    "ec",
+		"ttl":         "10h",
+		"issuer_name": "root-ca",
+		"key_name":    "root-key",
+		"key_usage":   "DigitalSignature,DogPetting",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "expected root generation to succeed")
+	require.Contains(t, resp.Warnings, "Invalid key usage will be ignored: unrecognized key usage DogPetting")
+	rootCertRaw := resp.Data["certificate"]
+	rootCert := parseCert(t, rootCertRaw.(string))
+	require.Equal(t, x509.KeyUsageDigitalSignature, rootCert.KeyUsage&x509.KeyUsageDigitalSignature, "keyUsage Digital Signature was not present")
+	require.Equal(t, x509.KeyUsageCertSign, rootCert.KeyUsage&x509.KeyUsageCertSign, "keyUsage CertSign was not present")
+	require.Equal(t, x509.KeyUsageCRLSign, rootCert.KeyUsage&x509.KeyUsageCRLSign, "keyUsage CRLSign was not present")
+	require.Equal(t, x509.KeyUsageDigitalSignature|x509.KeyUsageCertSign|x509.KeyUsageCRLSign, rootCert.KeyUsage, "unexpected KeyUsage present")
+	resp, err = CBWrite(b, s, "intermediate/generate/internal", map[string]interface{}{
+		"common_name": "myint.com",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "failed generating intermediary CSR")
+	requireFieldsSetInResp(t, resp, "csr")
+	csr := resp.Data["csr"]
+
+	resp, err = CBWrite(b, s, "issuer/root-ca/sign-intermediate", map[string]interface{}{
+		"csr":       csr,
+		"key_usage": "CRLSign,CertSign,DigitalSignature,KeyEncipherment,KeyAgreement",
+		"ttl":       "60h",
+	})
+	requireSuccessNonNilResponse(t, resp, err, "expected intermediate signing to succeed")
+	require.Contains(t, resp.Warnings, "Invalid key usage: key usage KeyEncipherment is only valid for non-Ca certs; key usage KeyAgreement is only valid for non-Ca certs")
+	intCertRaw := resp.Data["certificate"]
+	intCert := parseCert(t, intCertRaw.(string))
+	require.Equal(t, x509.KeyUsageDigitalSignature, intCert.KeyUsage&x509.KeyUsageDigitalSignature, "keyUsage Digital Signature was not present")
+	require.Equal(t, x509.KeyUsageCertSign, intCert.KeyUsage&x509.KeyUsageCertSign, "keyUsage CertSign was not present")
+	require.Equal(t, x509.KeyUsageCRLSign, intCert.KeyUsage&x509.KeyUsageCRLSign, "keyUsage CRLSign was not present")
+	require.Equal(t, x509.KeyUsageDigitalSignature|x509.KeyUsageCertSign|x509.KeyUsageCRLSign, intCert.KeyUsage, "unexpected KeyUsage present on intermediate certificate")
+}
+
 var (
 	initTest  sync.Once
 	rsaCAKey  string

builtin/logical/pki/path_root.go

@@ -197,6 +197,13 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request,
 		},
 	}
 
+	if keyUsages, ok := data.GetOk("key_usage"); ok {
+		err = validateCaKeyUsages(keyUsages.([]string))
+		if err != nil {
+			resp.AddWarning(fmt.Sprintf("Invalid key usage will be ignored: %v", err.Error()))
+		}
+	}
+
 	if len(parsedBundle.Certificate.RawSubject) <= 2 {
 		// Strictly a subject is a SEQUENCE of SETs of SEQUENCES.
 		//
@@ -432,6 +439,13 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R
 		resp.AddWarning(intCaTruncatationWarning)
 	}
 
+	if keyUsages, ok := data.GetOk("key_usage"); ok {
+		err = validateCaKeyUsages(keyUsages.([]string))
+		if err != nil {
+			resp.AddWarning(fmt.Sprintf("Invalid key usage: %v", err.Error()))
+		}
+	}
+
 	return resp, nil
 }
 
@@ -631,6 +645,24 @@ func publicKeyType(pub crypto.PublicKey) (pubType x509.PublicKeyAlgorithm, sigAl
 	return
 }
 
+func validateCaKeyUsages(keyUsages []string) error {
+	invalidKeyUsages := []string{}
+	for _, usage := range keyUsages {
+		cleanUsage := strings.ToLower(strings.TrimSpace(usage))
+		switch cleanUsage {
+		case "crlsign", "certsign", "digitalsignature":
+		case "contentcommitment", "keyencipherment", "dataencipherment", "keyagreement", "encipheronly", "decipheronly":
+			invalidKeyUsages = append(invalidKeyUsages, fmt.Sprintf("key usage %s is only valid for non-Ca certs", usage))
+		default:
+			invalidKeyUsages = append(invalidKeyUsages, fmt.Sprintf("unrecognized key usage %s", usage))
+		}
+	}
+	if invalidKeyUsages != nil {
+		return fmt.Errorf(strings.Join(invalidKeyUsages, "; "))
+	}
+	return nil
+}
+
 const pathGenerateRootHelpSyn = `
 Generate a new CA certificate and private key used for signing.
 `

changelog/30034.txt

@@ -0,0 +1,4 @@
+```release-note:bug
+secrets/pki: fix a bug where key_usage was ignored when generating root certificates, and signing certain
+intermediate certificates.
+```

sdk/helper/certutil/types.go

@@ -821,7 +821,14 @@ type CreationBundle struct {
 // information
 func AddKeyUsages(data *CreationBundle, certTemplate *x509.Certificate) {
 	if data.Params.IsCA {
-		certTemplate.KeyUsage = x509.KeyUsage(x509.KeyUsageCertSign | x509.KeyUsageCRLSign)
+		// https://cabforum.org/working-groups/server/baseline-requirements/documents/CA-Browser-Forum-TLS-BR-2.1.4.pdf
+		// Per Section 7.1.2.10.7, the only acceptable additional key usage is Digital Signature
+		if data.Params.KeyUsage&x509.KeyUsageDigitalSignature == x509.KeyUsageDigitalSignature {
+			certTemplate.KeyUsage = x509.KeyUsageDigitalSignature
+		} else {
+			certTemplate.KeyUsage = x509.KeyUsage(0)
+		}
+		certTemplate.KeyUsage |= x509.KeyUsageCertSign | x509.KeyUsageCRLSign
 		return
 	}
 

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

@@ -751,9 +751,9 @@ intermediary goes beyond the prescribed length.
 
 - `key_usage` `([]string: CRL,CertSign)` - This list of key usages will be added
   to the existing set of key usages, CRL,CertSign, on the generated certificate.
-  Values can be found at https://golang.org/pkg/crypto/x509/#KeyUsage part of the
-  name.  This is overwritten by use_csr_values if a key usage extension exists
-  within the csr.
+  Per the CAB Forum requirements, Vault ignores values other than `DigitalSignature`.
+  To get a certificate with additional `key_usage` values, add the desired values to
+  the CSR, then call this function with `use_csr_values` set to `true`.
 
 - `exclude_cn_from_sans` `(bool: false)` - If true, the given `common_name` will
   not be included in DNS or Email Subject Alternate Names (as appropriate).
@@ -2166,8 +2166,7 @@ use the values set via `config/urls`.
 
 - `key_usage` `([]string: CRL,CertSign)` - This list of key usages will be added
   to the existing set of key usages, CRL,CertSign, on the generated certificate.
-  Values can be found at https://golang.org/pkg/crypto/x509/#KeyUsage part of the
-  name.
+  Per the CAB Forum, Vault ignores additional values other than `DigitalSignature`.
 
 - `exclude_cn_from_sans` `(bool: false)` - If true, the given `common_name` will
   not be included in DNS or Email Subject Alternate Names (as appropriate).