chore(revoke): Support certificate revocation when certificate is not…

… found in backend
Keyfactor · Apr 19, 2024 · f1f9804 · f1f9804
1 parent b2af4c7
commit f1f9804
Show file tree

Hide file tree

Showing 5 changed files with 273 additions and 187 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# v1.3.0
+## Fixes
+* Certificate revocation with the `/revoke*` paths now support revocation of certificates not in local Engine storage if a certificate is provided. Revoked certificates are stored in the revoked storage regardless of the initial role configuration used to issue the certificate.
+
 # v1.2.0
 ## Features
 * Create `revoke-with-key` path to revoke certificate only if user proves they have the private key

diff --git a/certs_util.go b/certs_util.go
@@ -44,117 +44,175 @@ var (
  hostnameRegex = regexp.MustCompile(`^(\*\.)?(` + labelRegex + `\.)*` + labelRegex + `\.?$`)
 )
 
-func revokeCert(sc *storageContext, serialNumber string) (*logical.Response, error) {
- logger := sc.Backend.Logger().Named("revokeCert")
- logger.Info("revoking certificate with serial number " + serialNumber)
+// =================== Revoke Response Builder ===================
 
- client, err := sc.getClient()
- if err != nil {
- return nil, err
- }
+type revokeBuilder struct {
+ storageContext *storageContext
+ parsedCertBundle *certutil.ParsedCertBundle
 
- // Get the certificate
- parsedBundle, err := sc.Cert().fetchCertBundleBySerial(serialNumber)
- if err != nil {
- return nil, err
- }
+ issuerDn string
+ normalizedHexSerialNumber string
+ storageContextSerialNumber string
+
+ errorResponse *logical.Response
+}
 
- renormSerialNumber := strings.ReplaceAll(denormalizeSerial(serialNumber), ":", "")
+func (r *revokeBuilder) Config(sc *storageContext, path string, data *framework.FieldData) *revokeBuilder {
+ r.storageContext = sc
 
- logger.Debug("Calling EJBCA to revoke certificate with serial number " + renormSerialNumber)
- execute, _, err := client.V1CertificateApi.RevokeCertificate(sc.Context, parsedBundle.Certificate.Issuer.String(), renormSerialNumber).Reason("CESSATION_OF_OPERATION").Execute()
- if err != nil {
- return nil, client.createErrorFromEjbcaErr(sc.Backend, "failed to revoke certificate with serial number "+serialNumber, err)
- }
+ logger := r.storageContext.Backend.Logger().Named("revokeBuilder.Config")
 
- logger.Debug("Certificate with serial number " + renormSerialNumber + " revoked successfully")
+ privateKeyRequired := strings.HasPrefix(path, "revoke-with-key")
+ logger.Debug("Checking if revoke path requires private key", "privateKeyRequired", privateKeyRequired)
 
- //remove the certificate from vault.
- err = sc.Cert().deleteCert(serialNumber)
- if err != nil {
- return nil, err
- }
+ serialNumberInterface, serialPresent := data.GetOk("serial_number")
+ certificate, certPresent := data.GetOk("certificate")
+ privateKey, keyPresent := data.GetOk("private_key")
 
- bundle, err := parsedBundle.ToCertBundle()
- if err != nil {
- return nil, err
+ if serialPresent && certPresent {
+ logger.Error("Must provide either the certificate or the serial to revoke; not both.")
+ r.errorResponse = logical.ErrorResponse("Must provide either the certificate or the serial to revoke; not both.")
+ return r
  }
 
- logger.Trace("Creating revoked certificate entry")
- revokedEntry := &revokedCertEntry{
- Certificate: bundle.Certificate,
- SerialNumber: bundle.SerialNumber,
- RevocationTime: execute.RevocationDate.Unix(),
- RevocationTimeUTC: execute.RevocationDate.UTC(),
+ if !serialPresent && !certPresent {
+ logger.Error("The serial number or certificate to revoke must be provided.")
+ r.errorResponse = logical.ErrorResponse("The serial number or certificate to revoke must be provided.")
+ return r
  }
 
- err = sc.Cert().putRevokedCertEntry(revokedEntry)
- if err != nil {
- return nil, err
+ if !keyPresent && privateKeyRequired {
+ logger.Debug("Private key is required with the /revoke-with-key path")
+ r.errorResponse = logical.ErrorResponse("Private key must be provided to revoke a certificate with the /revoke-with-key-path")
+ return r
  }
 
- return &logical.Response{
- Data: map[string]interface{}{
- "revocation_time": execute.RevocationDate.Unix(),
- "revocation_time_rfc3339": execute.RevocationDate.UTC().Format(time.RFC3339Nano),
- "state": "revoked",
- },
- }, nil
-}
+ // Serialize the certificate - it was either passed in by the user or we can retrieve it from the backend
 
-func revokeCertWithPrivateKey(sc *storageContext, serialNumber string, privateKey crypto.PrivateKey) (*logical.Response, error) {
- logger := sc.Backend.Logger().Named("revokeCert")
+ var err error
+ var parsedCertBundle *certutil.ParsedCertBundle
+ if serialPresent {
+ parsedCertBundle, err = sc.Cert().fetchCertBundleBySerial(serialNumberInterface.(string))
+ if err != nil {
+ message := fmt.Sprintf("failed to fetch certificate with serial number %s from ejbcaBackend", serialNumberInterface.(string))
+ logger.Error(message)
+ r.errorResponse = logical.ErrorResponse(message)
+ return r
+ }
 
- client, err := sc.getClient()
- if err != nil {
- return nil, err
+ logger.Debug(fmt.Sprintf("Successfully fetched certificate with serial number %s from backend", serialNumberInterface.(string)))
  }
 
- // Get the certificate
- parsedBundle, err := sc.Cert().fetchCertBundleBySerial(serialNumber)
- if err != nil {
- return nil, err
+ if certPresent {
+ logger.Trace("Certificate present with request, serializing as PEM")
+ cert, err := serializePemCert(certificate.(string))
+ if err != nil {
+ r.errorResponse = logical.ErrorResponse(fmt.Sprintf("Error serializing certificate: %s", err))
+ return r
+ }
+
+ parsedCertBundle = &certutil.ParsedCertBundle{
+ CertificateBytes: cert.Raw,
+ Certificate: cert,
+ }
  }
 
- logger.Debug("Validating that private key matches certificate with serial number " + serialNumber)
- if !privateKeyMatchesCertificate(parsedBundle.Certificate, privateKey) {
- return nil, errors.New("private key does not match certificate with serial number " + serialNumber)
+ // EJBCA revocation requires the certificate to be a hex string, but the cert is stored in the storage storageContext
+ // with colons between the bytes. Prepare these now so we don't have to later.
+
+ certBundle, err := parsedCertBundle.ToCertBundle()
+ if err != nil {
+ logger.Error("Failed to convert parsed cert bundle to cert bundle: ", err)
+ r.errorResponse = logical.ErrorResponse("Failed to convert parsed cert bundle to cert bundle: ", err)
+ return r
  }
+ r.storageContextSerialNumber = certBundle.SerialNumber
+ r.normalizedHexSerialNumber = strings.ReplaceAll(r.storageContextSerialNumber, ":", "")
+ r.issuerDn = parsedCertBundle.Certificate.Issuer.String()
+ r.parsedCertBundle = parsedCertBundle
 
- logger.Info("Private Key matches, revoking certificate with serial number " + serialNumber)
+ if privateKeyRequired {
+ key, err := serializePemPrivateKey(privateKey.(string))
+ if err != nil {
+ logger.Error("Error serializing private key: ", err)
+ r.errorResponse = logical.ErrorResponse("Error serializing private key: ", err)
+ return r
+ }
 
- renormSerialNumber := strings.ReplaceAll(denormalizeSerial(serialNumber), ":", "")
+  logger.Debug("Validating that private key matches certificate with serial number " + r.storageContextSerialNumber)
 
- logger.Debug("Calling EJBCA to revoke certificate with serial number " + renormSerialNumber)
- execute, _, err := client.V1CertificateApi.RevokeCertificate(sc.Context, parsedBundle.Certificate.Issuer.String(), renormSerialNumber).Reason("CESSATION_OF_OPERATION").Execute()
- if err != nil {
- return nil, client.createErrorFromEjbcaErr(sc.Backend, "failed to revoke certificate with serial number "+serialNumber, err)
+ // We know that the certificate is present by this point
+ if !privateKeyMatchesCertificate(r.parsedCertBundle.Certificate, key) {
+ message := fmt.Sprintf("private key does not match certificate with serial number %s", r.storageContextSerialNumber)
+ logger.Error(message)
+ r.errorResponse = logical.ErrorResponse(message)
+ return r
+ }
+
+ logger.Info("Private Key matches")
+ }
+
+ return r
+}
+
+func (r *revokeBuilder) RevokeCertificate() (*logical.Response, error) {
+ if r.errorResponse != nil {
+ return r.errorResponse, nil
  }
 
- logger.Debug("Certificate with serial number " + renormSerialNumber + " revoked successfully")
+ logger := r.storageContext.Backend.Logger().Named("revokeBuilder.RevokeCertificate")
+ logger.Info(fmt.Sprintf("revoking certificate with serial number %s [%s]", r.storageContextSerialNumber, r.normalizedHexSerialNumber))
 
- //remove the certificate from vault.
- err = sc.Cert().deleteCert(serialNumber)
+ client, err := r.storageContext.getClient()
  if err != nil {
- return nil, err
+ message := "Failed to get EJBCA Client from backend: " + err.Error()
+ logger.Error(message)
+ return logical.ErrorResponse(message), nil
  }
 
- bundle, err := parsedBundle.ToCertBundle()
+ logger.Debug(fmt.Sprintf("Calling EJBCA to revoke certificate with serial number %s [%s]", r.storageContextSerialNumber, r.normalizedHexSerialNumber))
+ execute, _, err := client.V1CertificateApi.RevokeCertificate(r.storageContext.Context, r.issuerDn, r.normalizedHexSerialNumber).Reason("CESSATION_OF_OPERATION").Execute()
  if err != nil {
- return nil, err
+ ejbcaErr := client.createErrorFromEjbcaErr(r.storageContext.Backend, fmt.Sprintf("failed to revoke certificate with serial number %s [%s]", r.storageContextSerialNumber, r.normalizedHexSerialNumber), err)
+ logger.Error(ejbcaErr.Error())
+ return logical.ErrorResponse(ejbcaErr.Error()), nil
+ }
+
+ logger.Debug(fmt.Sprintf("Certificate with serial number %s [%s] revoked successfully", r.storageContextSerialNumber, r.normalizedHexSerialNumber))
+
+ // We only want to remove the certificate from the backend if it is present - the user could have enrolled
+ // the certificate by other measures.
+ _, err = r.storageContext.Cert().fetchCertBundleBySerial(r.storageContextSerialNumber)
+ if err == nil {
+ logger.Debug("Deleting certificate entry from backend")
+ err = r.storageContext.Cert().deleteCert(r.storageContextSerialNumber)
+ if err != nil {
+ message := fmt.Sprintf("Failed delete certificate entry from backend: %s", err)
+ logger.Error(message)
+ return logical.ErrorResponse(message), nil
+ }
+ }
+
+ bundle, err := r.parsedCertBundle.ToCertBundle()
+ if err != nil {
+ message := fmt.Sprintf("Failed to convert parsed cert bundle to cert bundle: %s", err)
+ logger.Error(message)
+ return logical.ErrorResponse(message), nil
  }
 
- logger.Debug("Creating revoked certificate entry")
+ logger.Trace("Creating revoked certificate entry")
  revokedEntry := &revokedCertEntry{
  Certificate: bundle.Certificate,
  SerialNumber: bundle.SerialNumber,
  RevocationTime: execute.RevocationDate.Unix(),
  RevocationTimeUTC: execute.RevocationDate.UTC(),
  }
 
- err = sc.Cert().putRevokedCertEntry(revokedEntry)
+ err = r.storageContext.Cert().putRevokedCertEntry(revokedEntry)
  if err != nil {
- return nil, err
+ message := fmt.Sprintf("Failed to add revoked certificate entry to backend: %s", err)
+ logger.Error(message)
+ return logical.ErrorResponse(message), nil
  }
 
  return &logical.Response{
@@ -1088,7 +1146,7 @@ func (i *issueSignHelper) validateNames(csr *x509.CertificateRequest) error {
  names = append(names, csr.Subject.CommonName)
 
  for j, name := range names {
- logger.Debug(fmt.Sprintf("Validating %s [%d/%d]", name, j+1, len(names)))
+ logger.Debug(fmt.Sprintf("Validating %s [%d/%d]", name, j+1, len(names)))
 
  reducedName := name
  emailDomain := reducedName
@@ -1307,11 +1365,11 @@ func serializePemPrivateKey(privateKey string) (crypto.PrivateKey, error) {
  // If we failed to parse the private key as PKCS#8, try to parse it as PKCS#1
  key, err = x509.ParsePKCS1PrivateKey(block.Bytes)
  if err != nil {
- // If we failed to parse the key as PKCS#1, try to parse it as ECC
- key, err = x509.ParseECPrivateKey(block.Bytes)
- if err != nil {
- return nil, fmt.Errorf("failed to parse private key as PKCS#8, PKCS#1, or ECC: %v", err)
- }
+ // If we failed to parse the key as PKCS#1, try to parse it as ECC
+ key, err = x509.ParseECPrivateKey(block.Bytes)
+ if err != nil {
+ return nil, fmt.Errorf("failed to parse private key as PKCS#8, PKCS#1, or ECC: %v", err)
+ }
  }
  }
 

diff --git a/path_revoke.go b/path_revoke.go
@@ -14,7 +14,6 @@ package ejbca_vault_pki_engine
 
 import (
  "context"
- "fmt"
  "net/http"
 
  "github.com/hashicorp/vault/sdk/framework"
@@ -109,7 +108,7 @@ func pathRevokeWithKey(b *ejbcaBackend) []*framework.Path {
 
  Operations: map[logical.Operation]framework.OperationHandler{
  logical.UpdateOperation: &framework.PathOperation{
- Callback: b.revokeCertificateWithPrivateKey,
+ Callback: b.revokeCertificate,
  Responses: map[int][]framework.Response{
  http.StatusOK: {{
  Description: "OK",
@@ -142,66 +141,9 @@ func pathRevokeWithKey(b *ejbcaBackend) []*framework.Path {
 }
 
 func (b *ejbcaBackend) revokeCertificate(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
- logger := b.Logger().Named("ejbcaBackend.revokeCertificate")
- sc := b.makeStorageContext(ctx, req.Storage)
-
- serial, serialPresent := data.GetOk("serial_number")
- certificate, certPresent := data.GetOk("certificate")
- if !serialPresent && !certPresent {
- return logical.ErrorResponse("The serial number or certificate to revoke must be provided."), nil
- } else if serialPresent && certPresent {
- return logical.ErrorResponse("Must provide either the certificate or the serial to revoke; not both."), nil
- }
-
- if certPresent {
- logger.Trace("Certificate present with request, serializing as PEM")
- cert, err := serializePemCert(certificate.(string))
- if err != nil {
- return nil, err
- }
-
- serial = cert.SerialNumber.String()
- }
-
- logger.Debug("Revoking certificate", "serial", serial, "certPresent", certPresent)
- return revokeCert(sc, serial.(string))
-}
-
-func (b *ejbcaBackend) revokeCertificateWithPrivateKey(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
- logger := b.Logger().Named("ejbcaBackend.revokeCertificateWithPrivateKey")
- sc := b.makeStorageContext(ctx, req.Storage)
-
- serial, serialPresent := data.GetOk("serial_number")
- certificate, certPresent := data.GetOk("certificate")
- privateKey, keyPresent := data.GetOk("private_key")
-
- if !serialPresent && !certPresent {
- return logical.ErrorResponse("The serial number or certificate to revoke must be provided."), nil
- } else if serialPresent && certPresent {
- return logical.ErrorResponse("Must provide either the certificate or the serial to revoke; not both."), nil
- }
-
- if !keyPresent {
- return logical.ErrorResponse("The private key must be provided to revoke a certificate."), nil
- }
-
- if certPresent {
- logger.Trace("Certificate present with request, serializing as PEM")
- cert, err := serializePemCert(certificate.(string))
- if err != nil {
- return nil, fmt.Errorf("Error serializing certificate: %s", err)
- }
-
- serial = cert.SerialNumber.String()
- }
-
- key, err := serializePemPrivateKey(privateKey.(string))
- if err != nil {
- return nil, fmt.Errorf("Error serializing private key: %s", err)
- }
-
- logger.Debug("Revoking certificate", "serial", serial, "certPresent", certPresent)
- return revokeCertWithPrivateKey(sc, serial.(string), key)
+ b.Logger().Named("ejbcaBackend.revokeCertificate").Debug("Path Revoke called")
+ builder := &revokeBuilder{}
+ return builder.Config(b.makeStorageContext(ctx, req.Storage), req.Path, data).RevokeCertificate()
 }
 
 const pathRevokeHelpSyn = `