Add BYOC-based revocation to PKI secrets engine (#16564)
* Refactor serial creation to common helper Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add BYOC revocation to PKI mount This allows operators to revoke certificates via a PEM blob passed to Vault. In particular, Vault verifies the signature on the certificate from an existing issuer within the mount, ensuring that one indeed issued this certificate. The certificate is then added to storage and its serial submitted for revocation. This allows certificates generated with no_store=true to be submitted for revocation afterwards, given a full copy of the certificate. As a consequence, all roles can now safely move to no_store=true (if desired for performance) and revocation can be done on a case-by-case basis. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add docs on BYOC revocation Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add PEM length check to BYOC import Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add changelog entry Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Add tests for BYOC Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> * Guard against legacy CA bundle usage This prevents usage of the BYOC cert on a hybrid 1.10/1.12 cluster with an non-upgraded CA issuer bundle. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com> Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel
GitHub
parent
6d45a421ff
commit
e388cfec64
|
|
@ -1908,7 +1908,7 @@ func TestBackend_PathFetchValidRaw(t *testing.T) {
|
|||
|
||||
issueCrtAsPem := resp.Data["certificate"].(string)
|
||||
issuedCrt := parseCert(t, issueCrtAsPem)
|
||||
expectedSerial := certutil.GetHexFormatted(issuedCrt.SerialNumber.Bytes(), ":")
|
||||
expectedSerial := serialFromCert(issuedCrt)
|
||||
expectedCert := []byte(issueCrtAsPem)
|
||||
|
||||
// get der cert
|
||||
|
|
|
|||
|
|
@ -303,6 +303,123 @@ func TestCrlRebuilder(t *testing.T) {
|
|||
"initial crl time: %#v not before next crl rebuild time: %#v", crl1.ThisUpdate, crl3.ThisUpdate)
|
||||
}
|
||||
|
||||
func TestBYOC(t *testing.T) {
|
||||
b, s := createBackendWithStorage(t)
|
||||
|
||||
// Create a root CA.
|
||||
resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
|
||||
"common_name": "root example.com",
|
||||
"issuer_name": "root",
|
||||
"key_type": "ec",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
require.NotEmpty(t, resp.Data["certificate"])
|
||||
oldRoot := resp.Data["certificate"].(string)
|
||||
|
||||
// Create a role for issuance.
|
||||
_, err = CBWrite(b, s, "roles/local-testing", map[string]interface{}{
|
||||
"allow_any_name": true,
|
||||
"enforce_hostnames": false,
|
||||
"key_type": "ec",
|
||||
"ttl": "75s",
|
||||
"no_store": "true",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Issue a leaf cert and ensure we can revoke it.
|
||||
resp, err = CBWrite(b, s, "issue/local-testing", map[string]interface{}{
|
||||
"common_name": "testing",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
require.NotEmpty(t, resp.Data["certificate"])
|
||||
|
||||
_, err = CBWrite(b, s, "revoke", map[string]interface{}{
|
||||
"certificate": resp.Data["certificate"],
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Issue a second leaf, but hold onto it for now.
|
||||
resp, err = CBWrite(b, s, "issue/local-testing", map[string]interface{}{
|
||||
"common_name": "testing2",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
require.NotEmpty(t, resp.Data["certificate"])
|
||||
notStoredCert := resp.Data["certificate"].(string)
|
||||
|
||||
// Update the role to make things stored and issue another cert.
|
||||
_, err = CBWrite(b, s, "roles/stored-testing", map[string]interface{}{
|
||||
"allow_any_name": true,
|
||||
"enforce_hostnames": false,
|
||||
"key_type": "ec",
|
||||
"ttl": "75s",
|
||||
"no_store": "false",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Issue a leaf cert and ensure we can revoke it.
|
||||
resp, err = CBWrite(b, s, "issue/stored-testing", map[string]interface{}{
|
||||
"common_name": "testing",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
require.NotEmpty(t, resp.Data["certificate"])
|
||||
storedCert := resp.Data["certificate"].(string)
|
||||
|
||||
// Delete the root and regenerate a new one.
|
||||
_, err = CBDelete(b, s, "issuer/default")
|
||||
require.NoError(t, err)
|
||||
|
||||
resp, err = CBList(b, s, "issuers")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, len(resp.Data), 0)
|
||||
|
||||
_, err = CBWrite(b, s, "root/generate/internal", map[string]interface{}{
|
||||
"common_name": "root2 example.com",
|
||||
"issuer_name": "root2",
|
||||
"key_type": "ec",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Issue a new leaf and revoke that one.
|
||||
resp, err = CBWrite(b, s, "issue/local-testing", map[string]interface{}{
|
||||
"common_name": "testing3",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
require.NotEmpty(t, resp.Data["certificate"])
|
||||
|
||||
_, err = CBWrite(b, s, "revoke", map[string]interface{}{
|
||||
"certificate": resp.Data["certificate"],
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Now attempt to revoke the earlier leaves. The first should fail since
|
||||
// we deleted its issuer, but the stored one should succeed.
|
||||
_, err = CBWrite(b, s, "revoke", map[string]interface{}{
|
||||
"certificate": notStoredCert,
|
||||
})
|
||||
require.Error(t, err)
|
||||
|
||||
_, err = CBWrite(b, s, "revoke", map[string]interface{}{
|
||||
"certificate": storedCert,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Import the old root again and revoke the no stored leaf should work.
|
||||
_, err = CBWrite(b, s, "issuers/import/bundle", map[string]interface{}{
|
||||
"pem_bundle": oldRoot,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = CBWrite(b, s, "revoke", map[string]interface{}{
|
||||
"certificate": notStoredCert,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
func requestCrlFromBackend(t *testing.T, s logical.Storage, b *backend) *logical.Response {
|
||||
crlReq := &logical.Request{
|
||||
Operation: logical.ReadOperation,
|
||||
|
|
|
|||
|
|
@ -15,7 +15,6 @@ import (
|
|||
|
||||
"github.com/hashicorp/vault/sdk/helper/consts"
|
||||
|
||||
"github.com/hashicorp/vault/sdk/helper/certutil"
|
||||
"github.com/hashicorp/vault/sdk/helper/errutil"
|
||||
"github.com/hashicorp/vault/sdk/logical"
|
||||
)
|
||||
|
|
@ -151,7 +150,7 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st
|
|||
}
|
||||
|
||||
colonSerial := strings.ReplaceAll(strings.ToLower(serial), "-", ":")
|
||||
if colonSerial == certutil.GetHexFormatted(parsedBundle.Certificate.SerialNumber.Bytes(), ":") {
|
||||
if colonSerial == serialFromCert(parsedBundle.Certificate) {
|
||||
return logical.ErrorResponse(fmt.Sprintf("adding issuer (id: %v) to its own CRL is not allowed", issuer)), nil
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,8 @@ package pki
|
|||
|
||||
import (
|
||||
"context"
|
||||
"crypto/x509"
|
||||
"encoding/pem"
|
||||
"fmt"
|
||||
"strings"
|
||||
|
||||
|
|
@ -20,6 +22,11 @@ func pathRevoke(b *backend) *framework.Path {
|
|||
Description: `Certificate serial number, in colon- or
|
||||
hyphen-separated octal`,
|
||||
},
|
||||
"certificate": {
|
||||
Type: framework.TypeString,
|
||||
Description: `Certificate to revoke in PEM format; must be
|
||||
signed by an issuer in this mount.`,
|
||||
},
|
||||
},
|
||||
|
||||
Operations: map[logical.Operation]framework.OperationHandler{
|
||||
|
|
@ -56,12 +63,165 @@ func pathRotateCRL(b *backend) *framework.Path {
|
|||
}
|
||||
}
|
||||
|
||||
func (b *backend) pathRevokeWriteHandleCertificate(ctx context.Context, req *logical.Request, data *framework.FieldData, certPem string) (string, []byte, error) {
|
||||
// This function handles just the verification of the certificate against
|
||||
// the global issuer set, checking whether or not it is importable.
|
||||
//
|
||||
// We return the parsed serial number, an optionally-nil byte array to
|
||||
// write out to disk, and an error if one occurred.
|
||||
if b.useLegacyBundleCaStorage() {
|
||||
// We require listing all issuers from the 1.11 method. If we're
|
||||
// still using the legacy CA bundle but with the newer certificate
|
||||
// attribute, we err and require the operator to upgrade and migrate
|
||||
// prior to servicing new requests.
|
||||
return "", nil, errutil.UserError{Err: "unable to process BYOC revocation until CA issuer migration has completed"}
|
||||
}
|
||||
|
||||
// First start by parsing the certificate.
|
||||
if len(certPem) < 75 {
|
||||
// See note in pathImportIssuers about this check.
|
||||
return "", nil, errutil.UserError{Err: "provided certificate data was too short; perhaps a path was passed to the API rather than the contents of a PEM file"}
|
||||
}
|
||||
|
||||
pemBlock, _ := pem.Decode([]byte(certPem))
|
||||
if pemBlock == nil {
|
||||
return "", nil, errutil.UserError{Err: "certificate contains no PEM data"}
|
||||
}
|
||||
|
||||
certReference, err := x509.ParseCertificate(pemBlock.Bytes)
|
||||
if err != nil {
|
||||
return "", nil, errutil.UserError{Err: fmt.Sprintf("certificate could not be parsed: %v", err)}
|
||||
}
|
||||
|
||||
// Ensure we have a well-formed serial number before continuing.
|
||||
serial := serialFromCert(certReference)
|
||||
if len(serial) == 0 {
|
||||
return "", nil, errutil.UserError{Err: fmt.Sprintf("invalid serial number on presented certificate")}
|
||||
}
|
||||
|
||||
// We have two approaches here: we could start verifying against issuers
|
||||
// (which involves fetching and parsing them), or we could see if, by
|
||||
// some chance we've already imported it (cheap). The latter tells us
|
||||
// if we happen to have a serial number collision (which shouldn't
|
||||
// happen in practice) versus an already-imported cert (which might
|
||||
// happen and its fine to handle safely).
|
||||
//
|
||||
// Start with the latter since its cheaper. Fetch the cert (by serial)
|
||||
// and if it exists, compare the contents.
|
||||
certEntry, err := fetchCertBySerial(ctx, b, req, req.Path, serial)
|
||||
if err != nil {
|
||||
return serial, nil, err
|
||||
}
|
||||
|
||||
if certEntry != nil {
|
||||
// As seen with importing issuers, it is best to parse the certificate
|
||||
// and compare parsed values, rather than attempting to infer equality
|
||||
// from the raw data.
|
||||
certReferenceStored, err := x509.ParseCertificate(certEntry.Value)
|
||||
if err != nil {
|
||||
return serial, nil, err
|
||||
}
|
||||
|
||||
if !areCertificatesEqual(certReference, certReferenceStored) {
|
||||
// Here we refuse the import with an error because the two certs
|
||||
// are unequal but we would've otherwise overwritten the existing
|
||||
// copy.
|
||||
return serial, nil, fmt.Errorf("certificate with same serial but unequal value already present in this cluster's storage; refusing to revoke")
|
||||
} else {
|
||||
// Otherwise, we can return without an error as we've already
|
||||
// imported this certificate, likely when we issued it. We don't
|
||||
// need to re-verify the signature as we assume it was already
|
||||
// verified when it was imported.
|
||||
return serial, nil, nil
|
||||
}
|
||||
}
|
||||
|
||||
// Otherwise, we must not have a stored copy. From here on out, the second
|
||||
// parameter (except in error cases) should be the copy to write out.
|
||||
//
|
||||
// Fetch and iterate through each issuer.
|
||||
sc := b.makeStorageContext(ctx, req.Storage)
|
||||
issuers, err := sc.listIssuers()
|
||||
if err != nil {
|
||||
return serial, nil, err
|
||||
}
|
||||
|
||||
foundMatchingIssuer := false
|
||||
for _, issuerId := range issuers {
|
||||
issuer, err := sc.fetchIssuerById(issuerId)
|
||||
if err != nil {
|
||||
return serial, nil, err
|
||||
}
|
||||
|
||||
issuerCert, err := issuer.GetCertificate()
|
||||
if err != nil {
|
||||
return serial, nil, err
|
||||
}
|
||||
|
||||
if err := certReference.CheckSignatureFrom(issuerCert); err == nil {
|
||||
// If the signature was valid, we found our match and can safely
|
||||
// exit.
|
||||
foundMatchingIssuer = true
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
if foundMatchingIssuer {
|
||||
return serial, certReference.Raw, nil
|
||||
}
|
||||
|
||||
return serial, nil, errutil.UserError{Err: fmt.Sprintf("unable to verify signature on presented cert from any present issuer in this mount; certificates from previous CAs will need to have their issuing CA and key re-imported if revocation is necessary")}
|
||||
}
|
||||
|
||||
func (b *backend) pathRevokeWrite(ctx context.Context, req *logical.Request, data *framework.FieldData, _ *roleEntry) (*logical.Response, error) {
|
||||
serial := data.Get("serial_number").(string)
|
||||
rawSerial, haveSerial := data.GetOk("serial_number")
|
||||
rawCertificate, haveCert := data.GetOk("certificate")
|
||||
|
||||
if !haveSerial && !haveCert {
|
||||
return logical.ErrorResponse("The serial number or certificate to revoke must be provided."), nil
|
||||
} else if haveSerial && haveCert {
|
||||
return logical.ErrorResponse("Must provide either the certificate or the serial to revoke; not both."), nil
|
||||
}
|
||||
|
||||
var serial string
|
||||
if haveSerial {
|
||||
// Easy case: this cert should be in storage already.
|
||||
serial = rawSerial.(string)
|
||||
} else {
|
||||
// Otherwise, we've gotta parse the certificate from the request and
|
||||
// then import it into cluster-local storage. Before writing the
|
||||
// certificate (and forwarding), we want to verify this certificate
|
||||
// was actually signed by one of our present issuers.
|
||||
var err error
|
||||
var certBytes []byte
|
||||
serial, certBytes, err = b.pathRevokeWriteHandleCertificate(ctx, req, data, rawCertificate.(string))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// At this point, a forward operation will occur if we're on a standby
|
||||
// node as we're now attempting to write the bytes of the cert out to
|
||||
// disk.
|
||||
if certBytes != nil {
|
||||
err = req.Storage.Put(ctx, &logical.StorageEntry{
|
||||
Key: "certs/" + serial,
|
||||
Value: certBytes,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
// Finally, we have a valid serial number to use for BYOC revocation!
|
||||
}
|
||||
|
||||
if len(serial) == 0 {
|
||||
return logical.ErrorResponse("The serial number must be provided"), nil
|
||||
}
|
||||
|
||||
// Assumption: this check is cheap. Call this twice, in the cert-import
|
||||
// case, to allow cert verification to get rejected on the standby node,
|
||||
// but we still need it to protect the serial number case.
|
||||
if b.System().ReplicationState().HasState(consts.ReplicationPerformanceStandby) {
|
||||
return nil, logical.ErrReadOnly
|
||||
}
|
||||
|
|
|
|||
|
|
@ -669,7 +669,7 @@ func (sc *storageContext) importIssuer(certValue string, issuerName string) (*is
|
|||
return nil, false, fmt.Errorf("bad issuer: potentially multiple PEM blobs in one certificate storage entry:\n%v", result.Certificate)
|
||||
}
|
||||
|
||||
result.SerialNumber = strings.TrimSpace(certutil.GetHexFormatted(issuerCert.SerialNumber.Bytes(), ":"))
|
||||
result.SerialNumber = serialFromCert(issuerCert)
|
||||
|
||||
// Before we return below, we need to iterate over _all_ keys and see if
|
||||
// one of them a public key matching this certificate, and if so, update our
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package pki
|
|||
|
||||
import (
|
||||
"crypto"
|
||||
"crypto/x509"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
|
@ -25,6 +26,10 @@ var (
|
|||
errKeyNameInUse = errutil.UserError{Err: "key name already in use"}
|
||||
)
|
||||
|
||||
func serialFromCert(cert *x509.Certificate) string {
|
||||
return strings.TrimSpace(certutil.GetHexFormatted(cert.SerialNumber.Bytes(), ":"))
|
||||
}
|
||||
|
||||
func normalizeSerial(serial string) string {
|
||||
return strings.ReplaceAll(strings.ToLower(serial), ":", "-")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,3 @@
|
|||
```release-note:improvement
|
||||
secrets/pki: Allow revocation of certificates with explicitly provided certificate (bring your own certificate / BYOC).
|
||||
```
|
||||
|
|
@ -834,9 +834,16 @@ successful revocation will rotate the CRL.
|
|||
|
||||
#### Parameters
|
||||
|
||||
- `serial_number` `(string: <required>)` - Specifies the serial number of the
|
||||
~> Note: either `serial_number` or `certificate` (but not both) must be
|
||||
specified on requests to this endpoint.
|
||||
|
||||
- `serial_number` `(string: <optional>)` - Specifies the serial number of the
|
||||
certificate to revoke, in hyphen-separated or colon-separated hexadecimal.
|
||||
|
||||
- `certificate` `(string: <optional>)` - Specifies the certificate to revoke,
|
||||
in PEM format. This certificate must have been signed by one of the issuers
|
||||
in this mount in order to be accepted for revocation.
|
||||
|
||||
#### Sample Payload
|
||||
|
||||
```json
|
||||
|
|
|
|||
Loading…
Reference in New Issue