Merge pull request #3277 from charhate/cert-revoke

CRL Reason during certificate revoke

docs/developer/plugins/index.rst

@@ -104,7 +104,7 @@ The `IssuerPlugin` exposes four functions functions::
 
     def create_certificate(self, csr, issuer_options):
         # requests.get('a third party')
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         # requests.put('a third party')
     def get_ordered_certificate(self, order_id):
         # requests.get('already existing certificate')

lemur/certificates/cli.py

@@ -5,7 +5,6 @@
     :license: Apache, see LICENSE for more details.
 .. moduleauthor:: Kevin Glisson <kglisson@netflix.com>
 """
-import multiprocessing
 import sys
 from flask import current_app
 from flask_principal import Identity, identity_changed
@@ -26,9 +25,10 @@ from lemur.certificates.service import (
     get_all_valid_certs,
     get,
     get_all_certs_attached_to_endpoint_without_autorotate,
+    revoke as revoke_certificate,
 )
 from lemur.certificates.verify import verify_string
-from lemur.constants import SUCCESS_METRIC_STATUS, FAILURE_METRIC_STATUS
+from lemur.constants import SUCCESS_METRIC_STATUS, FAILURE_METRIC_STATUS, CRLReason
 from lemur.deployment import service as deployment_service
 from lemur.domains.models import Domain
 from lemur.endpoints import service as endpoint_service
@@ -586,11 +586,10 @@ def worker(data, commit, reason):
     parts = [x for x in data.split(" ") if x]
     try:
         cert = get(int(parts[0].strip()))
-        plugin = plugins.get(cert.authority.plugin_name)
 
         print("[+] Revoking certificate. Id: {0} Name: {1}".format(cert.id, cert.name))
         if commit:
-            plugin.revoke_certificate(cert, reason)
+            revoke_certificate(cert, reason)
 
         metrics.send(
             "certificate_revoke",
@@ -620,10 +619,10 @@ def clear_pending():
     v.clear_pending_certificates()
 
 
-@manager.option(
+@manager.option("-p", "--path", dest="path", help="Absolute file path to a Lemur query csv.")
-    "-p", "--path", dest="path", help="Absolute file path to a Lemur query csv."
+@manager.option("-id", "--certid", dest="cert_id", help="ID of the certificate to be revoked")
-)
+@manager.option("-r", "--reason", dest="reason", default="unspecified", help="CRL Reason as per RFC 5280 section 5.3.1")
-@manager.option("-r", "--reason", dest="reason", help="Reason to revoke certificate.")
+@manager.option("-m", "--message", dest="message", help="Message explaining reason for revocation")
 @manager.option(
     "-c",
     "--commit",
@@ -632,20 +631,32 @@ def clear_pending():
     default=False,
     help="Persist changes.",
 )
-def revoke(path, reason, commit):
+def revoke(path, cert_id, reason, message, commit):
     """
     Revokes given certificate.
     """
+    if not path and not cert_id:
+        print("[!] No input certificates mentioned to revoke")
+        return
+    if path and cert_id:
+        print("[!] Please mention single certificate id (-id) or input file (-p)")
+        return
+
     if commit:
         print("[!] Running in COMMIT mode.")
 
     print("[+] Starting certificate revocation.")
 
-    with open(path, "r") as f:
+    if reason not in CRLReason.__members__:
-        args = [[x, commit, reason] for x in f.readlines()[2:]]
+        reason = CRLReason.unspecified.name
+    comments = {"comments": message, "crl_reason": reason}
 
-    with multiprocessing.Pool(processes=3) as pool:
+    if cert_id:
-        pool.starmap(worker, args)
+        worker(cert_id, commit, comments)
+    else:
+        with open(path, "r") as f:
+            for x in f.readlines()[2:]:
+                worker(x, commit, comments)
 
 
 @manager.command

lemur/certificates/schemas.py

@@ -16,7 +16,7 @@ from lemur.certificates import utils as cert_utils
 from lemur.common import missing, utils, validators
 from lemur.common.fields import ArrowDateTime, Hex
 from lemur.common.schema import LemurInputSchema, LemurOutputSchema
-from lemur.constants import CERTIFICATE_KEY_TYPES
+from lemur.constants import CERTIFICATE_KEY_TYPES, CRLReason
 from lemur.destinations.schemas import DestinationNestedOutputSchema
 from lemur.dns_providers.schemas import DnsProvidersNestedOutputSchema
 from lemur.domains.schemas import DomainNestedOutputSchema
@@ -455,6 +455,7 @@ class CertificateNotificationOutputSchema(LemurOutputSchema):
 
 class CertificateRevokeSchema(LemurInputSchema):
     comments = fields.String()
+    crl_reason = fields.String(validate=validate.OneOf(CRLReason.__members__), missing="unspecified")
 
 
 certificates_list_request_parser = RequestParser()

lemur/certificates/service.py

@@ -828,6 +828,14 @@ def remove_from_destination(certificate, destination):
         plugin.clean(certificate=certificate, options=destination.options)
 
 
+def revoke(certificate, reason):
+    plugin = plugins.get(certificate.authority.plugin_name)
+    plugin.revoke_certificate(certificate, reason)
+
+    # Perform cleanup after revoke
+    return cleanup_after_revoke(certificate)
+
+
 def cleanup_after_revoke(certificate):
     """
     Perform the needed cleanup for a revoked certificate. This includes -

lemur/certificates/views.py

@@ -20,7 +20,6 @@ from lemur.auth.permissions import AuthorityPermission, CertificatePermission
 from lemur.certificates import service
 from lemur.certificates.models import Certificate
 from lemur.extensions import sentry
-from lemur.plugins.base import plugins
 from lemur.certificates.schemas import (
     certificate_input_schema,
     certificate_output_schema,
@@ -29,6 +28,7 @@ from lemur.certificates.schemas import (
     certificate_export_input_schema,
     certificate_edit_input_schema,
     certificates_list_output_schema_factory,
+    certificate_revoke_schema,
 )
 
 from lemur.roles import service as role_service
@@ -1398,7 +1398,7 @@ class CertificateRevoke(AuthenticatedResource):
         self.reqparse = reqparse.RequestParser()
         super(CertificateRevoke, self).__init__()
 
-    @validate_schema(None, None)
+    @validate_schema(certificate_revoke_schema, None)
     def put(self, certificate_id, data=None):
         """
         .. http:put:: /certificates/1/revoke
@@ -1413,6 +1413,11 @@ class CertificateRevoke(AuthenticatedResource):
               Host: example.com
               Accept: application/json, text/javascript
 
+              {
+                "crlReason": "affiliationChanged",
+                "comments": "Additional details if any"
+              }
+
            **Example response**:
 
            .. sourcecode:: http
@@ -1422,12 +1427,13 @@ class CertificateRevoke(AuthenticatedResource):
               Content-Type: text/javascript
 
               {
-                'id': 1
+                "id": 1
               }
 
            :reqheader Authorization: OAuth token to authenticate
            :statuscode 200: no error
-           :statuscode 403: unauthenticated
+           :statuscode 403: unauthenticated or cert attached to LB
+           :statuscode 400: encountered error, more details in error message
 
         """
         cert = service.get(certificate_id)
@@ -1459,16 +1465,18 @@ class CertificateRevoke(AuthenticatedResource):
                         403,
                     )
 
-        plugin = plugins.get(cert.authority.plugin_name)
+        try:
-        plugin.revoke_certificate(cert, data)
+            error_message = service.revoke(cert, data)
-
             log_service.create(g.current_user, "revoke_cert", certificate=cert)
 
-        # Perform cleanup after revoke
-        error_message = service.cleanup_after_revoke(cert)
             if error_message:
                 return dict(message=f"Certificate (id:{cert.id}) is revoked - {error_message}"), 400
             return dict(id=cert.id)
+        except NotImplementedError as ne:
+            return dict(message="Revoke is not implemented for issuer of this certificate"), 400
+        except Exception as e:
+            sentry.captureException()
+            return dict(message=f"Failed to revoke: {str(e)}"), 400
 
 
 api.add_resource(

lemur/constants.py

@@ -3,6 +3,8 @@
     :copyright: (c) 2018 by Netflix Inc.
     :license: Apache, see LICENSE for more details.
 """
+from enum import IntEnum
+
 SAN_NAMING_TEMPLATE = "SAN-{subject}-{issuer}-{not_before}-{not_after}"
 DEFAULT_NAMING_TEMPLATE = "{subject}-{issuer}-{not_before}-{not_after}"
 NONSTANDARD_NAMING_TEMPLATE = "{issuer}-{not_before}-{not_after}"
@@ -32,3 +34,17 @@ CERTIFICATE_KEY_TYPES = [
     "ECCSECT409R1",
     "ECCSECT571R2",
 ]
+
+
+# As per RFC 5280 section 5.3.1 (https://tools.ietf.org/html/rfc5280#section-5.3.1)
+class CRLReason(IntEnum):
+    unspecified = 0,
+    keyCompromise = 1,
+    cACompromise = 2,
+    affiliationChanged = 3,
+    superseded = 4,
+    cessationOfOperation = 5,
+    certificateHold = 6,
+    removeFromCRL = 8,
+    privilegeWithdrawn = 9,
+    aACompromise = 10

lemur/plugins/bases/issuer.py

@@ -23,7 +23,7 @@ class IssuerPlugin(Plugin):
     def create_authority(self, options):
         raise NotImplementedError
 
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         raise NotImplementedError
 
     def get_ordered_certificate(self, certificate):

lemur/plugins/lemur_acme/acme_handlers.py

@@ -221,7 +221,7 @@ class AcmeHandler(object):
         current_app.logger.debug("Got these domains: {0}".format(domains))
         return domains
 
-    def revoke_certificate(self, certificate):
+    def revoke_certificate(self, certificate, crl_reason=0):
         if not self.reuse_account(certificate.authority):
             raise InvalidConfiguration("There is no ACME account saved, unable to revoke the certificate.")
         acme_client, _ = self.setup_acme_client(certificate.authority)
@@ -231,7 +231,7 @@ class AcmeHandler(object):
                 OpenSSL.crypto.FILETYPE_PEM, certificate.body))
 
         try:
-            acme_client.revoke(fullchain_com, 0)  # revocation reason = 0
+            acme_client.revoke(fullchain_com, crl_reason)  # revocation reason as int (per RFC 5280 section 5.3.1)
         except (errors.ConflictError, errors.ClientError, errors.Error) as e:
             # Certificate already revoked.
             current_app.logger.error("Certificate revocation failed with message: " + e.detail)

lemur/plugins/lemur_acme/plugin.py

@@ -17,6 +17,7 @@ from acme.messages import Error as AcmeError
 from botocore.exceptions import ClientError
 from flask import current_app
 from lemur.authorizations import service as authorization_service
+from lemur.constants import CRLReason
 from lemur.dns_providers import service as dns_provider_service
 from lemur.exceptions import InvalidConfiguration
 from lemur.extensions import metrics, sentry
@@ -267,9 +268,13 @@ class ACMEIssuerPlugin(IssuerPlugin):
         # Needed to override issuer function.
         pass
 
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         self.acme = AcmeDnsHandler()
-        return self.acme.revoke_certificate(certificate)
+        crl_reason = CRLReason.unspecified
+        if "crl_reason" in reason:
+            crl_reason = CRLReason[reason["crl_reason"]]
+
+        return self.acme.revoke_certificate(certificate, crl_reason.value)
 
 
 class ACMEHttpIssuerPlugin(IssuerPlugin):
@@ -368,6 +373,11 @@ class ACMEHttpIssuerPlugin(IssuerPlugin):
         # Needed to override issuer function.
         pass
 
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         self.acme = AcmeHandler()
-        return self.acme.revoke_certificate(certificate)
+
+        crl_reason = CRLReason.unspecified
+        if "crl_reason" in reason:
+            crl_reason = CRLReason[reason["crl_reason"]]
+
+        return self.acme.revoke_certificate(certificate, crl_reason.value)

lemur/plugins/lemur_adcs/plugin.py

@@ -59,8 +59,8 @@ class ADCSIssuerPlugin(IssuerPlugin):
         )
         return cert, chain, None
 
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
-        raise NotImplementedError("Not implemented\n", self, certificate, comments)
+        raise NotImplementedError("Not implemented\n", self, certificate, reason)
 
     def get_ordered_certificate(self, order_id):
         raise NotImplementedError("Not implemented\n", self, order_id)

lemur/plugins/lemur_cfssl/plugin.py

@@ -18,6 +18,7 @@ from flask import current_app
 
 from lemur.common.utils import parse_certificate
 from lemur.common.utils import get_authority_key
+from lemur.constants import CRLReason
 from lemur.plugins.bases import IssuerPlugin
 from lemur.plugins import lemur_cfssl as cfssl
 from lemur.extensions import metrics
@@ -102,16 +103,23 @@ class CfsslIssuerPlugin(IssuerPlugin):
         role = {"username": "", "password": "", "name": "cfssl"}
         return current_app.config.get("CFSSL_ROOT"), "", [role]
 
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         """Revoke a CFSSL certificate."""
         base_url = current_app.config.get("CFSSL_URL")
         create_url = "{0}/api/v1/cfssl/revoke".format(base_url)
+
+        crl_reason = CRLReason.unspecified
+        if "crl_reason" in reason:
+            crl_reason = CRLReason[reason["crl_reason"]]
+
         data = (
             '{"serial": "'
             + certificate.external_id
             + '","authority_key_id": "'
             + get_authority_key(certificate.body)
-            + '", "reason": "superseded"}'
+            + '", "reason": "'
+            + crl_reason
+            + '"}'
         )
         current_app.logger.debug("Revoking cert: {0}".format(data))
         response = self.session.post(

lemur/plugins/lemur_digicert/plugin.py

@@ -368,7 +368,7 @@ class DigiCertIssuerPlugin(IssuerPlugin):
             certificate_id,
         )
 
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         """Revoke a Digicert certificate."""
         base_url = current_app.config.get("DIGICERT_URL")
 
@@ -376,6 +376,11 @@ class DigiCertIssuerPlugin(IssuerPlugin):
         create_url = "{0}/services/v2/certificate/{1}/revoke".format(
             base_url, certificate.external_id
         )
+
+        comments = reason["comments"] if "comments" in reason else ''
+        if "crl_reason" in reason:
+            comments += '(' + reason["crl_reason"] + ')'
+
         metrics.send("digicert_revoke_certificate", "counter", 1)
         response = self.session.put(create_url, data=json.dumps({"comments": comments}))
         return handle_response(response)
@@ -575,7 +580,7 @@ class DigiCertCISIssuerPlugin(IssuerPlugin):
             data["id"],
         )
 
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         """Revoke a Digicert certificate."""
         base_url = current_app.config.get("DIGICERT_CIS_URL")
 
@@ -584,6 +589,10 @@ class DigiCertCISIssuerPlugin(IssuerPlugin):
             base_url, certificate.external_id
         )
         metrics.send("digicert_revoke_certificate_success", "counter", 1)
+
+        comments = reason["comments"] if "comments" in reason else ''
+        if "crl_reason" in reason:
+            comments += '(' + reason["crl_reason"] + ')'
         response = self.session.put(revoke_url, data=json.dumps({"comments": comments}))
 
         if response.status_code != 204:

lemur/plugins/lemur_entrust/plugin.py

@@ -5,6 +5,7 @@ import sys
 from flask import current_app
 from retrying import retry
 
+from lemur.constants import CRLReason
 from lemur.plugins import lemur_entrust as entrust
 from lemur.plugins.bases import IssuerPlugin, SourcePlugin
 from lemur.extensions import metrics
@@ -256,16 +257,20 @@ class EntrustIssuerPlugin(IssuerPlugin):
         return cert, chain, external_id
 
     @retry(stop_max_attempt_number=3, wait_fixed=1000)
-    def revoke_certificate(self, certificate, comments):
+    def revoke_certificate(self, certificate, reason):
         """Revoke an Entrust certificate."""
         base_url = current_app.config.get("ENTRUST_URL")
 
         # make certificate revoke request
         revoke_url = f"{base_url}/certificates/{certificate.external_id}/revocations"
-        if not comments or comments == '':
+        if "comments" not in reason or reason["comments"] == '':
             comments = "revoked via API"
+        crl_reason = CRLReason.unspecified
+        if "crl_reason" in reason:
+            crl_reason = CRLReason[reason["crl_reason"]]
+
         data = {
-            "crlReason": "superseded",  # enum (keyCompromise, affiliationChanged, superseded, cessationOfOperation)
+            "crlReason": crl_reason,  # per RFC 5280 section 5.3.1
             "revocationComment": comments
         }
         response = self.session.post(revoke_url, json=data)

lemur/static/app/angular/certificates/certificate/certificate.js

@@ -419,8 +419,8 @@ angular.module('lemur')
     $uibModalInstance.dismiss('cancel');
   };
 
-  $scope.revoke = function (certificate) {
+  $scope.revoke = function (certificate, crlReason) {
-   CertificateService.revoke(certificate).then(
+   CertificateService.revoke(certificate, crlReason).then(
       function () {
         toaster.pop({
           type: 'success',

lemur/static/app/angular/certificates/certificate/revoke.tpl.html

@@ -4,13 +4,13 @@
   <h3 class="modal-title">Revoke <span class="text-muted"><small>{{ certificate.name }}</small></span></h3>
 </div>
 <div class="modal-body">
-  <form name="revokeForm" ng-if="!certificate.endpoints.length" novalidate>
+  <form name="revokeForm" novalidate>
     <p><strong>Certificate revocation is final. Once revoked the certificate is no longer valid.</strong></p>
     <div class="form-horizontal">
       <div class="form-group"
            ng-class="{'has-error': revokeForm.confirm.$invalid, 'has-success': !revokeForm.$invalid&&revokeForm.confirm.$dirty}">
         <label class="control-label col-sm-2">
-          Confirm Revocation
+          Confirm Certificate Name
         </label>
         <div class="col-sm-10">
           <input name="confirm" ng-model="confirm" placeholder='{{ certificate.name }}'
@@ -23,6 +23,27 @@
             You must confirm certificate revocation.</p>
         </div>
       </div>
+      <div class="form-group">
+        <label class="control-label col-sm-2">
+          Reason
+        </label>
+        <div class="col-sm-10">
+          <select class="form-control" ng-model="crlReason"
+                  ng-options="option.value as option.name for option in [
+                      {'name': 'Unspecified', 'value': 'unspecified'},
+                      {'name': 'Key Compromise', 'value': 'keyCompromise'},
+                      {'name': 'CA Compromise', 'value': 'cACompromise'},
+                      {'name': 'Affiliation Changed', 'value': 'affiliationChanged'},
+                      {'name': 'Superseded', 'value': 'superseded'},
+                      {'name': 'Cessation of Operation', 'value': 'cessationOfOperation'},
+                      {'name': 'Certificate Hold', 'value': 'certificateHold'},
+                      {'name': 'Remove from CRL', 'value': 'removeFromCRL'},
+                      {'name': 'Privilege Withdrawn', 'value': 'privilegeWithdrawn'},
+                      {'name': 'Attribute Authority Compromise', 'value': 'aACompromise'}]"
+
+                  ng-init="crlReason = 'unspecified'"></select>
+        </div>
+      </div>
     </div>
   </form>
   <div ng-if="certificate.endpoints.length">
@@ -40,7 +61,7 @@
   </div>
 </div>
 <div class="modal-footer">
-  <button type="submit" ng-click="revoke(certificate)" ng-disabled="revokeForm.confirm.$invalid"
+  <button type="submit" ng-click="revoke(certificate, crlReason)" ng-disabled="revokeForm.confirm.$invalid"
           class="btn btn-danger">Revoke
   </button>
   <button ng-click="cancel()" class="btn">Cancel</button>

lemur/static/app/angular/certificates/services.js

@@ -313,8 +313,8 @@ angular.module('lemur')
       return certificate.customPOST(certificate.exportOptions, 'export');
     };
 
-    CertificateService.revoke = function (certificate) {
+    CertificateService.revoke = function (certificate, crlReason) {
-      return certificate.customPUT({}, 'revoke');
+      return certificate.customPUT({'crlReason':crlReason}, 'revoke');
     };
 
     return CertificateService;

lemur/tests/test_certificates.py

@@ -103,6 +103,30 @@ def test_delete_cert(session):
     assert not cert_exists
 
 
+def test_cleanup_after_revoke(session, issuer_plugin, crypto_authority):
+    from lemur.certificates.service import cleanup_after_revoke, get
+    from lemur.tests.factories import CertificateFactory
+
+    revoke_this = CertificateFactory(name="REVOKEME")
+    session.commit()
+
+    to_be_revoked = get(revoke_this.id)
+    assert to_be_revoked
+    to_be_revoked.notify = True
+    to_be_revoked.rotation = True
+
+    # Assuming the cert is revoked by corresponding issuer, update the records in lemur
+    cleanup_after_revoke(to_be_revoked)
+    revoked_cert = get(to_be_revoked.id)
+
+    # then not exist after delete
+    assert revoked_cert
+    assert revoked_cert.status == "revoked"
+    assert not revoked_cert.notify
+    assert not revoked_cert.rotation
+    assert not revoked_cert.destinations
+
+
 def test_get_by_attributes(session, certificate):
     from lemur.certificates.service import get_by_attributes
 
@@ -658,6 +682,23 @@ def test_certificate_upload_schema_wrong_chain_2nd(client):
     }
 
 
+def test_certificate_revoke_schema():
+    from lemur.certificates.schemas import CertificateRevokeSchema
+
+    input = {
+        "comments": "testing certificate revoke schema",
+        "crl_reason": "cessationOfOperation"
+    }
+    data, errors = CertificateRevokeSchema().load(input)
+    assert not errors
+
+    input["crl_reason"] = "fakeCrlReason"
+    data, errors = CertificateRevokeSchema().load(input)
+    assert errors == {
+        "crl_reason": ['Not a valid choice.']
+    }
+
+
 def test_create_basic_csr(client):
     csr_config = dict(
         common_name="example.com",