From 7a1f13dcb55ded3f0dc4c36d4b9665e590502a20 Mon Sep 17 00:00:00 2001 From: sayali Date: Mon, 30 Nov 2020 20:06:37 -0800 Subject: [PATCH 1/6] CRL Reason for certificate revoke --- docs/developer/plugins/index.rst | 2 +- lemur/certificates/cli.py | 8 ++-- lemur/certificates/schemas.py | 3 +- lemur/certificates/service.py | 8 ++++ lemur/certificates/views.py | 10 ++--- lemur/constants.py | 15 +++++++ lemur/plugins/bases/issuer.py | 2 +- lemur/plugins/lemur_acme/acme_handlers.py | 4 +- lemur/plugins/lemur_acme/plugin.py | 18 ++++++-- lemur/plugins/lemur_adcs/plugin.py | 4 +- lemur/plugins/lemur_cfssl/plugin.py | 12 +++++- lemur/plugins/lemur_digicert/plugin.py | 13 +++++- lemur/plugins/lemur_entrust/plugin.py | 11 +++-- .../certificates/certificate/certificate.js | 4 +- .../certificates/certificate/revoke.tpl.html | 27 ++++++++++-- .../app/angular/certificates/services.js | 4 +- lemur/tests/test_certificates.py | 41 +++++++++++++++++++ 17 files changed, 151 insertions(+), 35 deletions(-) diff --git a/docs/developer/plugins/index.rst b/docs/developer/plugins/index.rst index c2a8c48a..fd207fbd 100644 --- a/docs/developer/plugins/index.rst +++ b/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') diff --git a/lemur/certificates/cli.py b/lemur/certificates/cli.py index f23948be..1a603a68 100644 --- a/lemur/certificates/cli.py +++ b/lemur/certificates/cli.py @@ -623,7 +623,8 @@ def clear_pending(): @manager.option( "-p", "--path", dest="path", help="Absolute file path to a Lemur query csv." ) -@manager.option("-r", "--reason", dest="reason", help="Reason to revoke certificate.") +@manager.option("-r", "--reason", dest="reason", default="unspecified", help="CRL Reason as per RFC 5280 section 5.3.1") +@manager.option("-m", "--message", dest="message", help="Message explaining reason for revocation") @manager.option( "-c", "--commit", @@ -632,7 +633,7 @@ def clear_pending(): default=False, help="Persist changes.", ) -def revoke(path, reason, commit): +def revoke(path, reason, message, commit): """ Revokes given certificate. """ @@ -640,9 +641,10 @@ def revoke(path, reason, commit): print("[!] Running in COMMIT mode.") print("[+] Starting certificate revocation.") + comments = {"comments": message, "crl_reason": reason} with open(path, "r") as f: - args = [[x, commit, reason] for x in f.readlines()[2:]] + args = [[x, commit, comments] for x in f.readlines()[2:]] with multiprocessing.Pool(processes=3) as pool: pool.starmap(worker, args) diff --git a/lemur/certificates/schemas.py b/lemur/certificates/schemas.py index 3dc864e7..d3ed1776 100644 --- a/lemur/certificates/schemas.py +++ b/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() diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index 1aabec48..3d3e2ca0 100644 --- a/lemur/certificates/service.py +++ b/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 - diff --git a/lemur/certificates/views.py b/lemur/certificates/views.py index 52937fbd..da6c426b 100644 --- a/lemur/certificates/views.py +++ b/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 @@ -1459,13 +1459,9 @@ class CertificateRevoke(AuthenticatedResource): 403, ) - plugin = plugins.get(cert.authority.plugin_name) - 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) diff --git a/lemur/constants.py b/lemur/constants.py index cc1653cb..092b0e2d 100644 --- a/lemur/constants.py +++ b/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,16 @@ CERTIFICATE_KEY_TYPES = [ "ECCSECT409R1", "ECCSECT571R2", ] + + +class CRLReason(IntEnum): + unspecified = 0, + keyCompromise = 1, + cACompromise = 2, + affiliationChanged = 3, + superseded = 4, + cessationOfOperation = 5, + certificateHold = 6, + removeFromCRL = 8, + privilegeWithdrawn = 9, + aACompromise = 10 diff --git a/lemur/plugins/bases/issuer.py b/lemur/plugins/bases/issuer.py index f1e6aa0e..51b31590 100644 --- a/lemur/plugins/bases/issuer.py +++ b/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): diff --git a/lemur/plugins/lemur_acme/acme_handlers.py b/lemur/plugins/lemur_acme/acme_handlers.py index 55e4a076..83dfb1ba 100644 --- a/lemur/plugins/lemur_acme/acme_handlers.py +++ b/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) diff --git a/lemur/plugins/lemur_acme/plugin.py b/lemur/plugins/lemur_acme/plugin.py index 4763a2fa..d0dc3ae6 100644 --- a/lemur/plugins/lemur_acme/plugin.py +++ b/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) diff --git a/lemur/plugins/lemur_adcs/plugin.py b/lemur/plugins/lemur_adcs/plugin.py index 4b4eb20c..d2efe83f 100644 --- a/lemur/plugins/lemur_adcs/plugin.py +++ b/lemur/plugins/lemur_adcs/plugin.py @@ -59,8 +59,8 @@ class ADCSIssuerPlugin(IssuerPlugin): ) return cert, chain, None - def revoke_certificate(self, certificate, comments): - raise NotImplementedError("Not implemented\n", self, certificate, comments) + def revoke_certificate(self, certificate, reason): + raise NotImplementedError("Not implemented\n", self, certificate, reason) def get_ordered_certificate(self, order_id): raise NotImplementedError("Not implemented\n", self, order_id) diff --git a/lemur/plugins/lemur_cfssl/plugin.py b/lemur/plugins/lemur_cfssl/plugin.py index 02f3159d..e7dfbf5f 100644 --- a/lemur/plugins/lemur_cfssl/plugin.py +++ b/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( diff --git a/lemur/plugins/lemur_digicert/plugin.py b/lemur/plugins/lemur_digicert/plugin.py index d8e88fa3..e8f0c897 100644 --- a/lemur/plugins/lemur_digicert/plugin.py +++ b/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: diff --git a/lemur/plugins/lemur_entrust/plugin.py b/lemur/plugins/lemur_entrust/plugin.py index 924345eb..4e7ad00e 100644 --- a/lemur/plugins/lemur_entrust/plugin.py +++ b/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) diff --git a/lemur/static/app/angular/certificates/certificate/certificate.js b/lemur/static/app/angular/certificates/certificate/certificate.js index 41e04d55..3f05e148 100644 --- a/lemur/static/app/angular/certificates/certificate/certificate.js +++ b/lemur/static/app/angular/certificates/certificate/certificate.js @@ -419,8 +419,8 @@ angular.module('lemur') $uibModalInstance.dismiss('cancel'); }; - $scope.revoke = function (certificate) { - CertificateService.revoke(certificate).then( + $scope.revoke = function (certificate, crlReason) { + CertificateService.revoke(certificate, crlReason).then( function () { toaster.pop({ type: 'success', diff --git a/lemur/static/app/angular/certificates/certificate/revoke.tpl.html b/lemur/static/app/angular/certificates/certificate/revoke.tpl.html index 779d2ffd..21c6318e 100644 --- a/lemur/static/app/angular/certificates/certificate/revoke.tpl.html +++ b/lemur/static/app/angular/certificates/certificate/revoke.tpl.html @@ -4,13 +4,13 @@