From c3e0597ef1f537462be1e6fd2c71bca0b7790039 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 16:53:15 -0800 Subject: [PATCH 1/5] introducing ACME_ADDITIONAL_ATTEMPTS --- lemur/common/celery.py | 3 ++- lemur/constants.py | 3 +++ lemur/pending_certificates/cli.py | 4 +++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lemur/common/celery.py b/lemur/common/celery.py index 578592dc..f852e459 100644 --- a/lemur/common/celery.py +++ b/lemur/common/celery.py @@ -20,6 +20,7 @@ from flask import current_app from lemur.authorities.service import get as get_authority from lemur.certificates import cli as cli_certificate from lemur.common.redis import RedisHandler +from lemur.constants import ACME_ADDITIONAL_ATTEMPTS from lemur.destinations import service as destinations_service from lemur.dns_providers import cli as cli_dns_providers from lemur.endpoints import cli as cli_endpoints @@ -301,7 +302,7 @@ def fetch_acme_cert(id): error_log["last_error"] = cert.get("last_error") error_log["cn"] = pending_cert.cn - if pending_cert.number_attempts > 4: + if pending_cert.number_attempts > ACME_ADDITIONAL_ATTEMPTS: error_log["message"] = "Deleting pending certificate" send_pending_failure_notification( pending_cert, notify_owner=pending_cert.notify diff --git a/lemur/constants.py b/lemur/constants.py index 64bee4c3..e89160f5 100644 --- a/lemur/constants.py +++ b/lemur/constants.py @@ -12,6 +12,9 @@ NONSTANDARD_NAMING_TEMPLATE = "{issuer}-{not_before}-{not_after}" SUCCESS_METRIC_STATUS = "success" FAILURE_METRIC_STATUS = "failure" +# when ACME attempts to resolve a certificate try in total 3 times +ACME_ADDITIONAL_ATTEMPTS = 2 + CERTIFICATE_KEY_TYPES = [ "RSA2048", "RSA4096", diff --git a/lemur/pending_certificates/cli.py b/lemur/pending_certificates/cli.py index 2ff29f10..73b0ce2b 100644 --- a/lemur/pending_certificates/cli.py +++ b/lemur/pending_certificates/cli.py @@ -12,10 +12,12 @@ from flask import current_app from flask_script import Manager from lemur.authorities.service import get as get_authority +from lemur.constants import ACME_ADDITIONAL_ATTEMPTS from lemur.notifications.messaging import send_pending_failure_notification from lemur.pending_certificates import service as pending_certificate_service from lemur.plugins.base import plugins + manager = Manager(usage="Handles pending certificate related tasks.") @@ -107,7 +109,7 @@ def fetch_all_acme(): error_log["last_error"] = cert.get("last_error") error_log["cn"] = pending_cert.cn - if pending_cert.number_attempts > 4: + if pending_cert.number_attempts > ACME_ADDITIONAL_ATTEMPTS: error_log["message"] = "Marking pending certificate as resolved" send_pending_failure_notification( pending_cert, notify_owner=pending_cert.notify From da8d3f42d279e6682139eac4177f1a951d542617 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 16:53:42 -0800 Subject: [PATCH 2/5] documenting number of attempts --- docs/administration.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/administration.rst b/docs/administration.rst index 9af08407..3d4bad7c 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -362,6 +362,9 @@ Whenever a pending ACME certificate fails to be issued, Lemur will send a notifi and security team (as specified by the ``LEMUR_SECURITY_TEAM_EMAIL`` configuration parameter). This email is not sent if the pending certificate had notifications disabled. +Lemur will attempt 3x times to resolve a pending certificate. +This can at times result into 3 duplicate certificates, if all certificate attempts get resolved. + **Certificate rotation** Whenever a cert is rotated, Lemur will send a notification via email to the certificate owner. This notification is From 9b0cc3bbef4b15a3be17c80cb5eb75391655cc9f Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Thu, 28 Jan 2021 16:54:06 -0800 Subject: [PATCH 3/5] consolidating the documentation for acme --- docs/administration.rst | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/docs/administration.rst b/docs/administration.rst index 3d4bad7c..f150296b 100644 --- a/docs/administration.rst +++ b/docs/administration.rst @@ -823,6 +823,20 @@ ACME Plugin Enables delegated DNS domain validation using CNAMES. When enabled, Lemur will attempt to follow CNAME records to authoritative DNS servers when creating DNS-01 challenges. +The following configration properties are optional for the ACME plugin to use. They allow reusing an existing ACME +account. See :ref:`Using a pre-existing ACME account ` for more details. + + +.. data:: ACME_PRIVATE_KEY + :noindex: + + This is the private key, the account was registered with (in JWK format) + +.. data:: ACME_REGR + :noindex: + + This is the registration for the ACME account, the most important part is the uri attribute (in JSON) + Active Directory Certificate Services Plugin ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -1339,23 +1353,6 @@ The following configuration properties are required to use the PowerDNS ACME Plu File/Dir path to CA Bundle: Verifies the TLS certificate was issued by a Certificate Authority in the provided CA bundle. -ACME Plugin -~~~~~~~~~~~~ - -The following configration properties are optional for the ACME plugin to use. They allow reusing an existing ACME -account. See :ref:`Using a pre-existing ACME account ` for more details. - - -.. data:: ACME_PRIVATE_KEY - :noindex: - - This is the private key, the account was registered with (in JWK format) - -.. data:: ACME_REGR - :noindex: - - This is the registration for the ACME account, the most important part is the uri attribute (in JSON) - .. _CommandLineInterface: Command Line Interface From 5fb98f747cfe45446a2b2139e528c40c296d5e4f Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 29 Jan 2021 15:45:51 -0800 Subject: [PATCH 4/5] comment --- lemur/certificates/service.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index e6414933..b9bc16f0 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -388,6 +388,7 @@ def create(**kwargs): cert = Certificate(**kwargs) kwargs["creator"].certificates.append(cert) else: + # ACME path cert = PendingCertificate(**kwargs) kwargs["creator"].pending_certificates.append(cert) From 31b20e0a30de160a8d17d49e5a551dd8a015b5b0 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Fri, 29 Jan 2021 15:46:22 -0800 Subject: [PATCH 5/5] ensuring a resolved job doesn't get resolved twice --- lemur/common/celery.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lemur/common/celery.py b/lemur/common/celery.py index f852e459..d5faf578 100644 --- a/lemur/common/celery.py +++ b/lemur/common/celery.py @@ -274,7 +274,8 @@ def fetch_acme_cert(id): real_cert = cert.get("cert") # It's necessary to reload the pending cert due to detached instance: http://sqlalche.me/e/bhk3 pending_cert = pending_certificate_service.get(cert.get("pending_cert").id) - if not pending_cert: + if not pending_cert or pending_cert.resolved: + # pending_cert is cleared or it was resolved by another process log_data[ "message" ] = "Pending certificate doesn't exist anymore. Was it resolved by another process?"