From 542e9539199d4c3a51c77ee9911e28cff7afcf90 Mon Sep 17 00:00:00 2001 From: Marti Raudsepp Date: Wed, 20 Jun 2018 18:42:34 +0300 Subject: [PATCH 1/9] Check that stored private keys match certificates This is done in two places: * Certificate import validator -- throws validation errors. * Certificate model constructor -- to ensure integrity of Lemur's data even when issuer plugins or other code paths have bugs. --- lemur/certificates/models.py | 14 ++++- lemur/certificates/schemas.py | 26 ++++++++-- lemur/common/utils.py | 15 ++++++ lemur/common/validators.py | 33 ++++++------ lemur/tests/conftest.py | 9 +++- lemur/tests/factories.py | 5 ++ lemur/tests/test_certificates.py | 88 ++++++++++++++++++++++++++++++-- lemur/tests/test_validators.py | 22 ++++++-- 8 files changed, 181 insertions(+), 31 deletions(-) diff --git a/lemur/certificates/models.py b/lemur/certificates/models.py index e2ac2cba..3eaba746 100644 --- a/lemur/certificates/models.py +++ b/lemur/certificates/models.py @@ -19,7 +19,7 @@ from sqlalchemy.sql.expression import case, extract from sqlalchemy_utils.types.arrow import ArrowType from werkzeug.utils import cached_property -from lemur.common import defaults, utils +from lemur.common import defaults, utils, validators from lemur.constants import SUCCESS_METRIC_STATUS, FAILURE_METRIC_STATUS from lemur.database import db from lemur.domains.models import Domain @@ -186,6 +186,18 @@ class Certificate(db.Model): for domain in defaults.domains(cert): self.domains.append(Domain(name=domain)) + # Check integrity before saving anything into the database. + # For user-facing API calls, validation should also be done in schema validators. + self.check_integrity() + + def check_integrity(self): + """ + Integrity checks: Does the cert have a matching private key? + """ + if self.private_key: + validators.verify_private_key_match(utils.parse_private_key(self.private_key), self.parsed_cert, + error_class=AssertionError) + @cached_property def parsed_cert(self): assert self.body, "Certificate body not set" diff --git a/lemur/certificates/schemas.py b/lemur/certificates/schemas.py index bf18eac9..6b457086 100644 --- a/lemur/certificates/schemas.py +++ b/lemur/certificates/schemas.py @@ -10,7 +10,7 @@ from marshmallow import fields, validate, validates_schema, post_load, pre_load from marshmallow.exceptions import ValidationError from lemur.authorities.schemas import AuthorityNestedOutputSchema -from lemur.common import validators, missing +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 @@ -242,8 +242,8 @@ class CertificateUploadInputSchema(CertificateCreationSchema): authority = fields.Nested(AssociatedAuthoritySchema, required=False) notify = fields.Boolean(missing=True) external_id = fields.String(missing=None, allow_none=True) - private_key = fields.String(validate=validators.private_key) - body = fields.String(required=True, validate=validators.public_certificate) + private_key = fields.String() + body = fields.String(required=True) chain = fields.String(validate=validators.public_certificate, missing=None, allow_none=True) # TODO this could be multiple certificates @@ -258,6 +258,26 @@ class CertificateUploadInputSchema(CertificateCreationSchema): if not data.get('private_key'): raise ValidationError('Destinations require private key.') + @validates_schema + def validate_cert_private_key(self, data): + cert = None + key = None + if data.get('body'): + try: + cert = utils.parse_certificate(data['body']) + except ValueError: + raise ValidationError("Public certificate presented is not valid.", field_names=['body']) + + if data.get('private_key'): + try: + key = utils.parse_private_key(data['private_key']) + except ValueError: + raise ValidationError("Private key presented is not valid.", field_names=['private_key']) + + if cert and key: + # Throws ValidationError + validators.verify_private_key_match(key, cert) + class CertificateExportInputSchema(LemurInputSchema): plugin = fields.Nested(PluginInputSchema) diff --git a/lemur/common/utils.py b/lemur/common/utils.py index 7ea9d7f2..62e59d69 100644 --- a/lemur/common/utils.py +++ b/lemur/common/utils.py @@ -13,6 +13,7 @@ import sqlalchemy from cryptography import x509 from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives.asymmetric import rsa, ec +from cryptography.hazmat.primitives.serialization import load_pem_private_key from flask_restful.reqparse import RequestParser from sqlalchemy import and_, func @@ -52,6 +53,20 @@ def parse_certificate(body): return x509.load_pem_x509_certificate(body, default_backend()) +def parse_private_key(private_key): + """ + Parses a PEM-format private key (RSA, DSA, ECDSA or any other supported algorithm). + + Raises ValueError for an invalid string. + + :param private_key: String containing PEM private key + """ + if isinstance(private_key, str): + private_key = private_key.encode('utf8') + + return load_pem_private_key(private_key, password=None, backend=default_backend()) + + def parse_csr(csr): """ Helper function that parses a CSR. diff --git a/lemur/common/validators.py b/lemur/common/validators.py index 47a94a30..90169553 100644 --- a/lemur/common/validators.py +++ b/lemur/common/validators.py @@ -2,14 +2,12 @@ import re from cryptography import x509 from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives import serialization from cryptography.x509 import NameOID from flask import current_app from marshmallow.exceptions import ValidationError from lemur.auth.permissions import SensitiveDomainPermission from lemur.common.utils import parse_certificate, is_weekend -from lemur.domains import service as domain_service def public_certificate(body): @@ -26,22 +24,6 @@ def public_certificate(body): raise ValidationError('Public certificate presented is not valid.') -def private_key(key): - """ - User to validate that a given string is a RSA private key - - :param key: - :return: :raise ValueError: - """ - try: - if isinstance(key, bytes): - serialization.load_pem_private_key(key, None, backend=default_backend()) - else: - serialization.load_pem_private_key(key.encode('utf-8'), None, backend=default_backend()) - except Exception: - raise ValidationError('Private key presented is not valid.') - - def common_name(value): """If the common name could be a domain name, apply domain validation rules.""" # Common name could be a domain name, or a human-readable name of the subject (often used in CA names or client @@ -66,6 +48,9 @@ def sensitive_domain(domain): raise ValidationError('Domain {0} does not match whitelisted domain patterns. ' 'Contact an administrator to issue the certificate.'.format(domain)) + # Avoid circular import. + from lemur.domains import service as domain_service + if any(d.sensitive for d in domain_service.get_by_name(domain)): raise ValidationError('Domain {0} has been marked as sensitive. ' 'Contact an administrator to issue the certificate.'.format(domain)) @@ -141,3 +126,15 @@ def dates(data): raise ValidationError('Validity end must not be after {0}'.format(data['authority'].authority_certificate.not_after)) return data + + +def verify_private_key_match(key, cert, error_class=ValidationError): + """ + Checks that the supplied private key matches the certificate. + + :param cert: Parsed certificate + :param key: Parsed private key + :param error_class: Exception class to raise on error + """ + if key.public_key().public_numbers() != cert.public_key().public_numbers(): + raise error_class("Private key does not match certificate.") diff --git a/lemur/tests/conftest.py b/lemur/tests/conftest.py index d292e6d6..9a48eb94 100644 --- a/lemur/tests/conftest.py +++ b/lemur/tests/conftest.py @@ -15,7 +15,7 @@ from lemur.tests.vectors import SAN_CERT_KEY, INTERMEDIATE_KEY from .factories import ApiKeyFactory, AuthorityFactory, NotificationFactory, DestinationFactory, \ CertificateFactory, UserFactory, RoleFactory, SourceFactory, EndpointFactory, \ - RotationPolicyFactory, PendingCertificateFactory, AsyncAuthorityFactory + RotationPolicyFactory, PendingCertificateFactory, AsyncAuthorityFactory, CryptoAuthorityFactory def pytest_runtest_setup(item): @@ -91,6 +91,13 @@ def authority(session): return a +@pytest.fixture +def crypto_authority(session): + a = CryptoAuthorityFactory() + session.commit() + return a + + @pytest.fixture def async_authority(session): a = AsyncAuthorityFactory() diff --git a/lemur/tests/factories.py b/lemur/tests/factories.py index cae2c354..3717c64d 100644 --- a/lemur/tests/factories.py +++ b/lemur/tests/factories.py @@ -168,6 +168,11 @@ class AsyncAuthorityFactory(AuthorityFactory): authority_certificate = SubFactory(CertificateFactory) +class CryptoAuthorityFactory(AuthorityFactory): + """Authority factory based on 'cryptography' plugin.""" + plugin = {'slug': 'cryptography-issuer'} + + class DestinationFactory(BaseFactory): """Destination factory.""" plugin_name = 'test-destination' diff --git a/lemur/tests/test_certificates.py b/lemur/tests/test_certificates.py index 87416a7a..a1df1c0d 100644 --- a/lemur/tests/test_certificates.py +++ b/lemur/tests/test_certificates.py @@ -18,7 +18,7 @@ from lemur.domains.models import Domain from lemur.tests.vectors import VALID_ADMIN_API_TOKEN, VALID_ADMIN_HEADER_TOKEN, VALID_USER_HEADER_TOKEN, CSR_STR, \ - INTERMEDIATE_CERT_STR, SAN_CERT_STR, SAN_CERT_KEY + INTERMEDIATE_CERT_STR, SAN_CERT_STR, SAN_CERT_KEY, ROOTCA_KEY, ROOTCA_CERT_STR def test_get_or_increase_name(session, certificate): @@ -365,6 +365,85 @@ def test_certificate_sensitive_name(client, authority, session, logged_in_user): assert errors['common_name'][0].startswith("Domain sensitive.example.com has been marked as sensitive") +def test_certificate_upload_schema_ok(client): + from lemur.certificates.schemas import CertificateUploadInputSchema + data = { + 'name': 'Jane', + 'owner': 'pwner@example.com', + 'body': SAN_CERT_STR, + 'privateKey': SAN_CERT_KEY, + 'chain': INTERMEDIATE_CERT_STR, + 'external_id': '1234', + } + data, errors = CertificateUploadInputSchema().load(data) + assert not errors + + +def test_certificate_upload_schema_minimal(client): + from lemur.certificates.schemas import CertificateUploadInputSchema + data = { + 'owner': 'pwner@example.com', + 'body': SAN_CERT_STR, + } + data, errors = CertificateUploadInputSchema().load(data) + assert not errors + + +def test_certificate_upload_schema_long_chain(client): + from lemur.certificates.schemas import CertificateUploadInputSchema + data = { + 'owner': 'pwner@example.com', + 'body': SAN_CERT_STR, + 'chain': INTERMEDIATE_CERT_STR + '\n' + ROOTCA_CERT_STR + } + data, errors = CertificateUploadInputSchema().load(data) + assert not errors + + +def test_certificate_upload_schema_invalid_body(client): + from lemur.certificates.schemas import CertificateUploadInputSchema + data = { + 'owner': 'pwner@example.com', + 'body': 'Hereby I certify that this is a valid body', + } + data, errors = CertificateUploadInputSchema().load(data) + assert errors == {'body': ['Public certificate presented is not valid.']} + + +def test_certificate_upload_schema_invalid_pkey(client): + from lemur.certificates.schemas import CertificateUploadInputSchema + data = { + 'owner': 'pwner@example.com', + 'body': SAN_CERT_STR, + 'privateKey': 'Look at me Im a private key!!111', + } + data, errors = CertificateUploadInputSchema().load(data) + assert errors == {'private_key': ['Private key presented is not valid.']} + + +def test_certificate_upload_schema_invalid_chain(client): + from lemur.certificates.schemas import CertificateUploadInputSchema + data = { + 'body': SAN_CERT_STR, + 'chain': 'CHAINSAW', + 'owner': 'pwner@example.com', + } + data, errors = CertificateUploadInputSchema().load(data) + assert errors == {'chain': ['Public certificate presented is not valid.']} + + +def test_certificate_upload_schema_wrong_pkey(client): + from lemur.certificates.schemas import CertificateUploadInputSchema + data = { + 'body': SAN_CERT_STR, + 'privateKey': ROOTCA_KEY, + 'chain': INTERMEDIATE_CERT_STR, + 'owner': 'pwner@example.com', + } + data, errors = CertificateUploadInputSchema().load(data) + assert errors == {'_schema': ['Private key does not match certificate.']} + + def test_create_basic_csr(client): csr_config = dict( common_name='example.com', @@ -462,8 +541,11 @@ def test_create_certificate(issuer_plugin, authority, user): assert cert.name == 'ACustomName1' -def test_reissue_certificate(issuer_plugin, authority, certificate): +def test_reissue_certificate(issuer_plugin, crypto_authority, certificate, logged_in_user): from lemur.certificates.service import reissue_certificate + + # test-authority would return a mismatching private key, so use 'cryptography-issuer' plugin instead. + certificate.authority = crypto_authority new_cert = reissue_certificate(certificate) assert new_cert @@ -487,7 +569,7 @@ def test_import(user): assert str(cert.not_after) == '2047-12-31T22:00:00+00:00' assert str(cert.not_before) == '2017-12-31T22:00:00+00:00' assert cert.issuer == 'LemurTrustUnittestsClass1CA2018' - assert cert.name == 'SAN-san.example.org-LemurTrustUnittestsClass1CA2018-20171231-20471231-AFF2DB4F8D2D4D8E80FA382AE27C2333-2' + assert cert.name.startswith('SAN-san.example.org-LemurTrustUnittestsClass1CA2018-20171231-20471231') cert = import_certificate(body=SAN_CERT_STR, chain=INTERMEDIATE_CERT_STR, private_key=SAN_CERT_KEY, owner='joe@example.com', name='ACustomName2', creator=user['user']) assert cert.name == 'ACustomName2' diff --git a/lemur/tests/test_validators.py b/lemur/tests/test_validators.py index 815b7c9d..c3d5357d 100644 --- a/lemur/tests/test_validators.py +++ b/lemur/tests/test_validators.py @@ -1,16 +1,28 @@ -import pytest from datetime import datetime -from .vectors import SAN_CERT_KEY + +import pytest from marshmallow.exceptions import ValidationError +from lemur.common.utils import parse_private_key +from lemur.common.validators import verify_private_key_match +from lemur.tests.vectors import INTERMEDIATE_CERT, SAN_CERT, SAN_CERT_KEY + def test_private_key(session): - from lemur.common.validators import private_key + parse_private_key(SAN_CERT_KEY) - private_key(SAN_CERT_KEY) + with pytest.raises(ValueError): + parse_private_key('invalid_private_key') + + +def test_validate_private_key(session): + key = parse_private_key(SAN_CERT_KEY) + + verify_private_key_match(key, SAN_CERT) with pytest.raises(ValidationError): - private_key('invalid_private_key') + # Wrong key for certificate + verify_private_key_match(key, INTERMEDIATE_CERT) def test_sub_alt_type(session): From 3ee12cc50be99bb9e6b9b074f606468d9e2aa742 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Thu, 10 Jan 2019 09:26:15 -0800 Subject: [PATCH 2/9] Update requirements --- requirements-dev.txt | 7 +++---- requirements-docs.txt | 6 +++--- requirements-tests.txt | 8 ++++---- requirements.txt | 6 +++--- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index e9e47ed5..21156588 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -5,8 +5,7 @@ # pip-compile --no-index --output-file requirements-dev.txt requirements-dev.in # aspy.yaml==1.1.1 # via pre-commit -bleach==3.0.2 # via readme-renderer -cached-property==1.5.1 # via pre-commit +bleach==3.1.0 # via readme-renderer certifi==2018.11.29 # via requests cfgv==1.4.0 # via pre-commit chardet==3.0.4 # via requests @@ -19,8 +18,8 @@ importlib-resources==1.0.2 # via pre-commit invoke==1.2.0 mccabe==0.6.1 # via flake8 nodeenv==1.3.3 -pkginfo==1.5.0 # via twine -pre-commit==1.13.0 +pkginfo==1.5.0.1 # via twine +pre-commit==1.14.0 pycodestyle==2.3.1 # via flake8 pyflakes==1.6.0 # via flake8 pygments==2.3.1 # via readme-renderer diff --git a/requirements-docs.txt b/requirements-docs.txt index 19ebb0ea..f3182456 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -9,7 +9,7 @@ alabaster==0.7.12 # via sphinx alembic-autogenerate-enums==0.0.2 alembic==1.0.5 amqp==2.3.2 -aniso8601==4.0.1 +aniso8601==4.1.0 arrow==0.13.0 asn1crypto==0.24.0 asyncpool==1.0 @@ -17,8 +17,8 @@ babel==2.6.0 # via sphinx bcrypt==3.1.5 billiard==3.5.0.5 blinker==1.4 -boto3==1.9.75 -botocore==1.12.75 +boto3==1.9.76 +botocore==1.12.76 celery[redis]==4.2.1 certifi==2018.11.29 cffi==1.11.5 diff --git a/requirements-tests.txt b/requirements-tests.txt index a11de6ec..490d74d1 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -8,9 +8,9 @@ asn1crypto==0.24.0 # via cryptography atomicwrites==1.2.1 # via pytest attrs==18.2.0 # via pytest aws-xray-sdk==0.95 # via moto -boto3==1.9.75 # via moto +boto3==1.9.76 # via moto boto==2.49.0 # via moto -botocore==1.12.75 # via boto3, moto, s3transfer +botocore==1.12.76 # via boto3, moto, s3transfer certifi==2018.11.29 # via requests cffi==1.11.5 # via cryptography chardet==3.0.4 # via requests @@ -38,7 +38,7 @@ more-itertools==5.0.0 # via pytest moto==1.3.7 nose==1.3.7 pbr==5.1.1 # via mock -pluggy==0.8.0 # via pytest +pluggy==0.8.1 # via pytest py==1.7.0 # via pytest pyaml==18.11.0 # via moto pycparser==2.19 # via cffi @@ -60,5 +60,5 @@ text-unidecode==1.2 # via faker urllib3==1.24.1 # via botocore, requests websocket-client==0.54.0 # via docker werkzeug==0.14.1 # via flask, moto, pytest-flask -wrapt==1.10.11 # via aws-xray-sdk +wrapt==1.11.0 # via aws-xray-sdk xmltodict==0.11.0 # via moto diff --git a/requirements.txt b/requirements.txt index 59871284..bc72db0a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,15 +8,15 @@ acme==0.30.0 alembic-autogenerate-enums==0.0.2 alembic==1.0.5 # via flask-migrate amqp==2.3.2 # via kombu -aniso8601==4.0.1 # via flask-restful +aniso8601==4.1.0 # via flask-restful arrow==0.13.0 asn1crypto==0.24.0 # via cryptography asyncpool==1.0 bcrypt==3.1.5 # via flask-bcrypt, paramiko billiard==3.5.0.5 # via celery blinker==1.4 # via flask-mail, flask-principal, raven -boto3==1.9.75 -botocore==1.12.75 +boto3==1.9.76 +botocore==1.12.76 celery[redis]==4.2.1 certifi==2018.11.29 cffi==1.11.5 # via bcrypt, cryptography, pynacl From 0e02e6da799af16120b9ddb54c7542a68aa4365f Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Fri, 11 Jan 2019 11:13:43 -0800 Subject: [PATCH 3/9] Be more forgiving to throttling --- lemur/plugins/lemur_aws/elb.py | 18 +++++++++--------- lemur/plugins/lemur_aws/iam.py | 8 ++++---- lemur/plugins/lemur_aws/sts.py | 16 +++++++++++++--- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/lemur/plugins/lemur_aws/elb.py b/lemur/plugins/lemur_aws/elb.py index 4c4ce97f..b4391dd8 100644 --- a/lemur/plugins/lemur_aws/elb.py +++ b/lemur/plugins/lemur_aws/elb.py @@ -95,7 +95,7 @@ def get_all_elbs_v2(**kwargs): @sts_client('elbv2') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def get_listener_arn_from_endpoint(endpoint_name, endpoint_port, **kwargs): """ Get a listener ARN from an endpoint. @@ -113,7 +113,7 @@ def get_listener_arn_from_endpoint(endpoint_name, endpoint_port, **kwargs): @sts_client('elb') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def get_elbs(**kwargs): """ Fetches one page elb objects for a given account and region. @@ -123,7 +123,7 @@ def get_elbs(**kwargs): @sts_client('elbv2') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def get_elbs_v2(**kwargs): """ Fetches one page of elb objects for a given account and region. @@ -136,7 +136,7 @@ def get_elbs_v2(**kwargs): @sts_client('elbv2') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def describe_listeners_v2(**kwargs): """ Fetches one page of listener objects for a given elb arn. @@ -149,7 +149,7 @@ def describe_listeners_v2(**kwargs): @sts_client('elb') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def describe_load_balancer_policies(load_balancer_name, policy_names, **kwargs): """ Fetching all policies currently associated with an ELB. @@ -161,7 +161,7 @@ def describe_load_balancer_policies(load_balancer_name, policy_names, **kwargs): @sts_client('elbv2') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def describe_ssl_policies_v2(policy_names, **kwargs): """ Fetching all policies currently associated with an ELB. @@ -173,7 +173,7 @@ def describe_ssl_policies_v2(policy_names, **kwargs): @sts_client('elb') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def describe_load_balancer_types(policies, **kwargs): """ Describe the policies with policy details. @@ -185,7 +185,7 @@ def describe_load_balancer_types(policies, **kwargs): @sts_client('elb') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def attach_certificate(name, port, certificate_id, **kwargs): """ Attaches a certificate to a listener, throws exception @@ -205,7 +205,7 @@ def attach_certificate(name, port, certificate_id, **kwargs): @sts_client('elbv2') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=1000) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def attach_certificate_v2(listener_arn, port, certificates, **kwargs): """ Attaches a certificate to a listener, throws exception diff --git a/lemur/plugins/lemur_aws/iam.py b/lemur/plugins/lemur_aws/iam.py index b2a07798..7010c909 100644 --- a/lemur/plugins/lemur_aws/iam.py +++ b/lemur/plugins/lemur_aws/iam.py @@ -52,7 +52,7 @@ def create_arn_from_cert(account_number, region, certificate_name): @sts_client('iam') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def upload_cert(name, body, private_key, path, cert_chain=None, **kwargs): """ Upload a certificate to AWS @@ -95,7 +95,7 @@ def upload_cert(name, body, private_key, path, cert_chain=None, **kwargs): @sts_client('iam') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def delete_cert(cert_name, **kwargs): """ Delete a certificate from AWS @@ -112,7 +112,7 @@ def delete_cert(cert_name, **kwargs): @sts_client('iam') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def get_certificate(name, **kwargs): """ Retrieves an SSL certificate. @@ -126,7 +126,7 @@ def get_certificate(name, **kwargs): @sts_client('iam') -@retry(retry_on_exception=retry_throttled, stop_max_attempt_number=7, wait_exponential_multiplier=100) +@retry(retry_on_exception=retry_throttled, wait_fixed=2000) def get_certificates(**kwargs): """ Fetches one page of certificate objects for a given account. diff --git a/lemur/plugins/lemur_aws/sts.py b/lemur/plugins/lemur_aws/sts.py index 001ea2c8..6253ad7a 100644 --- a/lemur/plugins/lemur_aws/sts.py +++ b/lemur/plugins/lemur_aws/sts.py @@ -9,14 +9,22 @@ from functools import wraps import boto3 +from botocore.config import Config from flask import current_app +config = Config( + retries=dict( + max_attempts=20 + ) +) + + def sts_client(service, service_type='client'): def decorator(f): @wraps(f) def decorated_function(*args, **kwargs): - sts = boto3.client('sts') + sts = boto3.client('sts', config=config) arn = 'arn:aws:iam::{0}:role/{1}'.format( kwargs.pop('account_number'), current_app.config.get('LEMUR_INSTANCE_PROFILE', 'Lemur') @@ -31,7 +39,8 @@ def sts_client(service, service_type='client'): region_name=kwargs.pop('region', 'us-east-1'), aws_access_key_id=role['Credentials']['AccessKeyId'], aws_secret_access_key=role['Credentials']['SecretAccessKey'], - aws_session_token=role['Credentials']['SessionToken'] + aws_session_token=role['Credentials']['SessionToken'], + config=config ) kwargs['client'] = client elif service_type == 'resource': @@ -40,7 +49,8 @@ def sts_client(service, service_type='client'): region_name=kwargs.pop('region', 'us-east-1'), aws_access_key_id=role['Credentials']['AccessKeyId'], aws_secret_access_key=role['Credentials']['SecretAccessKey'], - aws_session_token=role['Credentials']['SessionToken'] + aws_session_token=role['Credentials']['SessionToken'], + config=config ) kwargs['resource'] = resource return f(*args, **kwargs) From c4e6e7c59bae61855ea1e0ea514fc8da5566b962 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Mon, 14 Jan 2019 08:02:27 -0800 Subject: [PATCH 4/9] Optimize DB cert filtering --- lemur/certificates/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index c9a2fa24..e4503324 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -307,7 +307,7 @@ def render(args): if filt: terms = filt.split(';') - term = '%{0}%'.format(terms[1]) + term = '{0}%'.format(terms[1]) # Exact matches for quotes. Only applies to name, issuer, and cn if terms[1].startswith('"') and terms[1].endswith('"'): term = terms[1][1:-1] From 31a86687e72e02883a9d80abe345b5b5b64d2667 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Mon, 14 Jan 2019 09:20:02 -0800 Subject: [PATCH 5/9] Reduce the expense of joins --- lemur/certificates/service.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index e4503324..1b203260 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -20,7 +20,6 @@ from lemur.common.utils import generate_private_key, truthiness from lemur.destinations.models import Destination from lemur.domains.models import Domain from lemur.extensions import metrics, sentry, signals -from lemur.models import certificate_associations from lemur.notifications.models import Notification from lemur.pending_certificates.models import PendingCertificate from lemur.plugins.base import plugins @@ -341,13 +340,13 @@ def render(args): elif 'id' in terms: query = query.filter(Certificate.id == cast(terms[1], Integer)) elif 'name' in terms: - query = query.outerjoin(certificate_associations).outerjoin(Domain).filter( + query = query.filter( or_( Certificate.name.ilike(term), - Domain.name.ilike(term), + Certificate.domains.any(Domain.name.ilike(term)), Certificate.cn.ilike(term), ) - ).group_by(Certificate.id) + ) else: query = database.filter(query, Certificate, terms) From 3567a768d5a0e281d41d7c3f8bdb73bf3c6a7728 Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Mon, 14 Jan 2019 13:35:55 -0800 Subject: [PATCH 6/9] Compare certificate hashes to determine if Lemur already has a synced certificate --- lemur/common/utils.py | 11 +++++++++++ lemur/sources/service.py | 5 +++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lemur/common/utils.py b/lemur/common/utils.py index 62e59d69..0504c958 100644 --- a/lemur/common/utils.py +++ b/lemur/common/utils.py @@ -12,6 +12,7 @@ import string import sqlalchemy from cryptography import x509 from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes from cryptography.hazmat.primitives.asymmetric import rsa, ec from cryptography.hazmat.primitives.serialization import load_pem_private_key from flask_restful.reqparse import RequestParser @@ -226,3 +227,13 @@ def truthiness(s): """If input string resembles something truthy then return True, else False.""" return s.lower() in ('true', 'yes', 'on', 't', '1') + + +def find_matching_certificates_by_hash(cert, matching_certs): + """Given a Cryptography-formatted certificate cert, and Lemur-formatted certificates (matching_certs), + determine if any of the certificate hashes match and return the matches.""" + matching = [] + for c in matching_certs: + if parse_certificate(c.body).fingerprint(hashes.SHA256()) == cert.fingerprint(hashes.SHA256()): + matching.append(c) + return matching diff --git a/lemur/sources/service.py b/lemur/sources/service.py index 227f1bce..55d2ee62 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -17,7 +17,7 @@ from lemur.endpoints import service as endpoint_service from lemur.destinations import service as destination_service from lemur.certificates.schemas import CertificateUploadInputSchema -from lemur.common.utils import parse_certificate +from lemur.common.utils import find_matching_certificates_by_hash, parse_certificate from lemur.common.defaults import serial from lemur.plugins.base import plugins @@ -126,7 +126,8 @@ def sync_certificates(source, user): if not exists: cert = parse_certificate(certificate['body']) - exists = certificate_service.get_by_serial(serial(cert)) + matching_serials = certificate_service.get_by_serial(serial(cert)) + exists = find_matching_certificates_by_hash(cert, matching_serials) if not certificate.get('owner'): certificate['owner'] = user.email From d3284a4006a87940ca485adea33957c116176c02 Mon Sep 17 00:00:00 2001 From: Hossein Shafagh Date: Mon, 14 Jan 2019 17:52:06 -0800 Subject: [PATCH 7/9] adjusting the query to filter authorities based on matching CN --- lemur/authorities/service.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lemur/authorities/service.py b/lemur/authorities/service.py index 024cb42a..41c381e3 100644 --- a/lemur/authorities/service.py +++ b/lemur/authorities/service.py @@ -15,6 +15,7 @@ from lemur import database from lemur.common.utils import truthiness from lemur.extensions import metrics from lemur.authorities.models import Authority +from lemur.certificates.models import Certificate from lemur.roles import service as role_service from lemur.certificates.service import upload @@ -179,7 +180,12 @@ def render(args): if 'active' in filt: query = query.filter(Authority.active == truthiness(terms[1])) elif 'cn' in filt: - query = query.join(Authority.active == truthiness(terms[1])) + term = '%{0}%'.format(terms[1]) + sub_query = database.session_query(Certificate.root_authority_id) \ + .filter(Certificate.cn.ilike(term)) \ + .subquery() + + query = query.filter(Authority.id.in_(sub_query)) else: query = database.filter(query, Authority, terms) From 7f88c24e8374f669ba1e12c3d5ff06892de3b04c Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Thu, 17 Jan 2019 14:56:04 -0800 Subject: [PATCH 8/9] Fix LetsEncrypt Dyn flow for duplicate CN/SAN --- lemur/common/utils.py | 11 +++++++++++ lemur/plugins/lemur_acme/dyn.py | 8 ++++++-- lemur/sources/service.py | 5 +++-- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lemur/common/utils.py b/lemur/common/utils.py index 62e59d69..f26f07df 100644 --- a/lemur/common/utils.py +++ b/lemur/common/utils.py @@ -12,6 +12,7 @@ import string import sqlalchemy from cryptography import x509 from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes from cryptography.hazmat.primitives.asymmetric import rsa, ec from cryptography.hazmat.primitives.serialization import load_pem_private_key from flask_restful.reqparse import RequestParser @@ -226,3 +227,13 @@ def truthiness(s): """If input string resembles something truthy then return True, else False.""" return s.lower() in ('true', 'yes', 'on', 't', '1') + + +def find_matching_certificates_by_hash(cert, matching_certs): + """Given a Cryptography-formatted certificate cert, and Lemur-formatted certificates (matching_certs), + determine if any of the certificate hashes match and return the matches.""" + matching = [] + for c in matching_certs: + if parse_certificate(c).fingerprint(hashes.SHA256()) == cert.body.fingerprint(hashes.SHA256()): + matching.append(c) + return matching diff --git a/lemur/plugins/lemur_acme/dyn.py b/lemur/plugins/lemur_acme/dyn.py index 9bab3a65..5d419f7f 100644 --- a/lemur/plugins/lemur_acme/dyn.py +++ b/lemur/plugins/lemur_acme/dyn.py @@ -5,7 +5,7 @@ import dns.exception import dns.name import dns.query import dns.resolver -from dyn.tm.errors import DynectCreateError +from dyn.tm.errors import DynectCreateError, DynectGetError from dyn.tm.session import DynectSession from dyn.tm.zones import Node, Zone, get_all_zones from flask import current_app @@ -119,7 +119,11 @@ def delete_txt_record(change_id, account_number, domain, token): zone = Zone(zone_name) node = Node(zone_name, fqdn) - all_txt_records = node.get_all_records_by_type('TXT') + try: + all_txt_records = node.get_all_records_by_type('TXT') + except DynectGetError: + # No Text Records remain or host is not in the zone anymore because all records have been deleted. + return for txt_record in all_txt_records: if txt_record.txtdata == ("{}".format(token)): current_app.logger.debug("Deleting TXT record name: {0}".format(fqdn)) diff --git a/lemur/sources/service.py b/lemur/sources/service.py index 227f1bce..55d2ee62 100644 --- a/lemur/sources/service.py +++ b/lemur/sources/service.py @@ -17,7 +17,7 @@ from lemur.endpoints import service as endpoint_service from lemur.destinations import service as destination_service from lemur.certificates.schemas import CertificateUploadInputSchema -from lemur.common.utils import parse_certificate +from lemur.common.utils import find_matching_certificates_by_hash, parse_certificate from lemur.common.defaults import serial from lemur.plugins.base import plugins @@ -126,7 +126,8 @@ def sync_certificates(source, user): if not exists: cert = parse_certificate(certificate['body']) - exists = certificate_service.get_by_serial(serial(cert)) + matching_serials = certificate_service.get_by_serial(serial(cert)) + exists = find_matching_certificates_by_hash(cert, matching_serials) if not certificate.get('owner'): certificate['owner'] = user.email From d689f5cda3aad42ff7b6363c40332160ca3a395a Mon Sep 17 00:00:00 2001 From: Curtis Castrapel Date: Thu, 17 Jan 2019 14:59:57 -0800 Subject: [PATCH 9/9] Fix LetsEncrypt for duplicate CN/SAN --- requirements-dev.txt | 5 ++--- requirements-docs.txt | 18 +++++++++--------- requirements-tests.txt | 8 ++++---- requirements.txt | 16 ++++++++-------- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 21156588..c1f55581 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -14,12 +14,11 @@ flake8==3.5.0 identify==1.1.8 # via pre-commit idna==2.8 # via requests importlib-metadata==0.8 # via pre-commit -importlib-resources==1.0.2 # via pre-commit invoke==1.2.0 mccabe==0.6.1 # via flake8 nodeenv==1.3.3 pkginfo==1.5.0.1 # via twine -pre-commit==1.14.0 +pre-commit==1.14.2 pycodestyle==2.3.1 # via flake8 pyflakes==1.6.0 # via flake8 pygments==2.3.1 # via readme-renderer @@ -29,7 +28,7 @@ requests-toolbelt==0.8.0 # via twine requests==2.21.0 # via requests-toolbelt, twine six==1.12.0 # via bleach, cfgv, pre-commit, readme-renderer toml==0.10.0 # via pre-commit -tqdm==4.29.0 # via twine +tqdm==4.29.1 # via twine twine==1.12.1 urllib3==1.24.1 # via requests virtualenv==16.2.0 # via pre-commit diff --git a/requirements-docs.txt b/requirements-docs.txt index f3182456..a7df5395 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -7,18 +7,18 @@ acme==0.30.0 alabaster==0.7.12 # via sphinx alembic-autogenerate-enums==0.0.2 -alembic==1.0.5 -amqp==2.3.2 +alembic==1.0.6 +amqp==2.4.0 aniso8601==4.1.0 arrow==0.13.0 asn1crypto==0.24.0 asyncpool==1.0 babel==2.6.0 # via sphinx -bcrypt==3.1.5 +bcrypt==3.1.6 billiard==3.5.0.5 blinker==1.4 -boto3==1.9.76 -botocore==1.12.76 +boto3==1.9.80 +botocore==1.12.80 celery[redis]==4.2.1 certifi==2018.11.29 cffi==1.11.5 @@ -54,7 +54,7 @@ lockfile==0.12.2 mako==1.0.7 markupsafe==1.1.0 marshmallow-sqlalchemy==0.15.0 -marshmallow==2.17.0 +marshmallow==2.18.0 mock==2.0.0 ndg-httpsclient==0.5.1 packaging==18.0 # via sphinx @@ -69,7 +69,7 @@ pygments==2.3.1 # via sphinx pyjwt==1.7.1 pynacl==1.3.0 pyopenssl==18.0.0 -pyparsing==2.3.0 # via packaging +pyparsing==2.3.1 # via packaging pyrfc3339==1.1 python-dateutil==2.7.5 python-editor==1.0.3 @@ -87,8 +87,8 @@ sphinx-rtd-theme==0.4.2 sphinx==1.8.3 sphinxcontrib-httpdomain==1.7.0 sphinxcontrib-websupport==1.1.0 # via sphinx -sqlalchemy-utils==0.33.10 -sqlalchemy==1.2.15 +sqlalchemy-utils==0.33.11 +sqlalchemy==1.2.16 tabulate==0.8.2 urllib3==1.24.1 vine==1.2.0 diff --git a/requirements-tests.txt b/requirements-tests.txt index 490d74d1..2d54dce6 100644 --- a/requirements-tests.txt +++ b/requirements-tests.txt @@ -8,9 +8,9 @@ asn1crypto==0.24.0 # via cryptography atomicwrites==1.2.1 # via pytest attrs==18.2.0 # via pytest aws-xray-sdk==0.95 # via moto -boto3==1.9.76 # via moto +boto3==1.9.80 # via moto boto==2.49.0 # via moto -botocore==1.12.76 # via boto3, moto, s3transfer +botocore==1.12.80 # via boto3, moto, s3transfer certifi==2018.11.29 # via requests cffi==1.11.5 # via cryptography chardet==3.0.4 # via requests @@ -18,7 +18,7 @@ click==7.0 # via flask coverage==4.5.2 cryptography==2.4.2 # via moto docker-pycreds==0.4.0 # via docker -docker==3.6.0 # via moto +docker==3.7.0 # via moto docutils==0.14 # via botocore ecdsa==0.13 # via python-jose factory-boy==2.11.1 @@ -46,7 +46,7 @@ pycryptodome==3.7.2 # via python-jose pyflakes==2.0.0 pytest-flask==0.14.0 pytest-mock==1.10.0 -pytest==4.1.0 +pytest==4.1.1 python-dateutil==2.7.5 # via botocore, faker, freezegun, moto python-jose==2.0.2 # via moto pytz==2018.9 # via moto diff --git a/requirements.txt b/requirements.txt index bc72db0a..79268c8a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,17 +6,17 @@ # acme==0.30.0 alembic-autogenerate-enums==0.0.2 -alembic==1.0.5 # via flask-migrate -amqp==2.3.2 # via kombu +alembic==1.0.6 # via flask-migrate +amqp==2.4.0 # via kombu aniso8601==4.1.0 # via flask-restful arrow==0.13.0 asn1crypto==0.24.0 # via cryptography asyncpool==1.0 -bcrypt==3.1.5 # via flask-bcrypt, paramiko +bcrypt==3.1.6 # via flask-bcrypt, paramiko billiard==3.5.0.5 # via celery blinker==1.4 # via flask-mail, flask-principal, raven -boto3==1.9.76 -botocore==1.12.76 +boto3==1.9.80 +botocore==1.12.80 celery[redis]==4.2.1 certifi==2018.11.29 cffi==1.11.5 # via bcrypt, cryptography, pynacl @@ -51,7 +51,7 @@ lockfile==0.12.2 mako==1.0.7 # via alembic markupsafe==1.1.0 # via jinja2, mako marshmallow-sqlalchemy==0.15.0 -marshmallow==2.17.0 +marshmallow==2.18.0 mock==2.0.0 # via acme ndg-httpsclient==0.5.1 paramiko==2.4.2 @@ -77,8 +77,8 @@ requests[security]==2.21.0 retrying==1.3.3 s3transfer==0.1.13 # via boto3 six==1.12.0 -sqlalchemy-utils==0.33.10 -sqlalchemy==1.2.15 # via alembic, flask-sqlalchemy, marshmallow-sqlalchemy, sqlalchemy-utils +sqlalchemy-utils==0.33.11 +sqlalchemy==1.2.16 # via alembic, flask-sqlalchemy, marshmallow-sqlalchemy, sqlalchemy-utils tabulate==0.8.2 urllib3==1.24.1 # via botocore, requests vine==1.2.0 # via amqp