From bc9435385059486e7f33ede274cf4f4cbf71d0c8 Mon Sep 17 00:00:00 2001 From: kevgliss Date: Fri, 27 Jan 2017 21:05:25 -0800 Subject: [PATCH] Closes #648, also fixes several issues #666. (#678) --- lemur/certificates/models.py | 48 ++++++++++------ lemur/certificates/service.py | 19 ++++--- lemur/common/fields.py | 55 +++++++++---------- lemur/plugins/lemur_digicert/plugin.py | 29 +++++----- .../lemur_digicert/tests/test_digicert.py | 6 +- lemur/plugins/lemur_verisign/plugin.py | 21 ++++++- lemur/schemas.py | 6 +- .../certificates/certificate/options.tpl.html | 34 ------------ .../certificate/tracking.tpl.html | 34 ++++++++++++ lemur/tests/test_certificates.py | 2 +- 10 files changed, 142 insertions(+), 112 deletions(-) diff --git a/lemur/certificates/models.py b/lemur/certificates/models.py index e802f343..4855d37a 100644 --- a/lemur/certificates/models.py +++ b/lemur/certificates/models.py @@ -233,34 +233,46 @@ class Certificate(db.Model): return_extensions = {} cert = lemur.common.utils.parse_certificate(self.body) for extension in cert.extensions: - if isinstance(extension, x509.BasicConstraints): - return_extensions['basic_constraints'] = extension - elif isinstance(extension, x509.SubjectAlternativeName): - return_extensions['sub_alt_names'] = extension - elif isinstance(extension, x509.ExtendedKeyUsage): - return_extensions['extended_key_usage'] = extension - elif isinstance(extension, x509.KeyUsage): - return_extensions['key_usage'] = extension - elif isinstance(extension, x509.SubjectKeyIdentifier): + value = extension.value + if isinstance(value, x509.BasicConstraints): + return_extensions['basic_constraints'] = extension.value + + elif isinstance(value, x509.SubjectAlternativeName): + return_extensions['sub_alt_names'] = {'names': extension.value} + + elif isinstance(value, x509.ExtendedKeyUsage): + return_extensions['extended_key_usage'] = extension.value + + elif isinstance(value, x509.KeyUsage): + return_extensions['key_usage'] = extension.value + + elif isinstance(value, x509.SubjectKeyIdentifier): return_extensions['subject_key_identifier'] = {'include_ski': True} - elif isinstance(extension, x509.AuthorityInformationAccess): + + elif isinstance(value, x509.AuthorityInformationAccess): return_extensions['certificate_info_access'] = {'include_aia': True} - elif isinstance(extension, x509.AuthorityKeyIdentifier): + + elif isinstance(value, x509.AuthorityKeyIdentifier): aki = { 'use_key_identifier': False, 'use_authority_cert': False } - if extension.key_identifier: + + if value.key_identifier: aki['use_key_identifier'] = True - if extension.authority_cert_issuer: + + if value.authority_cert_issuer: aki['use_authority_cert'] = True + return_extensions['authority_key_identifier'] = aki - elif isinstance(extension, x509.CRLDistributionPoints): - # FIXME: Don't support CRLDistributionPoints yet https://github.com/Netflix/lemur/issues/662 - pass + + # TODO: Don't support CRLDistributionPoints yet https://github.com/Netflix/lemur/issues/662 + elif isinstance(value, x509.CRLDistributionPoints): + current_app.logger.warning('CRLDistributionPoints not yet supported for clone operation.') + + # TODO: Not supporting custom OIDs yet. https://github.com/Netflix/lemur/issues/665 else: - # FIXME: Not supporting custom OIDs yet. https://github.com/Netflix/lemur/issues/665 - pass + current_app.logger.warning('Custom OIDs not yet supported for clone operation.') return return_extensions diff --git a/lemur/certificates/service.py b/lemur/certificates/service.py index e03d67a0..c1be4b43 100644 --- a/lemur/certificates/service.py +++ b/lemur/certificates/service.py @@ -330,10 +330,8 @@ def create_csr(**csr_config): :param csr_config: """ - private_key = generate_private_key(csr_config.get('key_type')) - # TODO When we figure out a better way to validate these options they should be parsed as str builder = x509.CertificateSigningRequestBuilder() builder = builder.subject_name(x509.Name([ x509.NameAttribute(x509.OID_COMMON_NAME, csr_config['common_name']), @@ -349,12 +347,17 @@ def create_csr(**csr_config): critical_extensions = ['basic_constraints', 'sub_alt_names', 'key_usage'] noncritical_extensions = ['extended_key_usage'] for k, v in extensions.items(): - if k in critical_extensions and v: - current_app.logger.debug("Add CExt: {0} {1}".format(k, v)) - builder = builder.add_extension(v, critical=True) - if k in noncritical_extensions and v: - current_app.logger.debug("Add Ext: {0} {1}".format(k, v)) - builder = builder.add_extension(v, critical=False) + if v: + if k in critical_extensions: + current_app.logger.debug('Adding Critical Extension: {0} {1}'.format(k, v)) + if k == 'sub_alt_names': + builder = builder.add_extension(v['names'], critical=True) + else: + builder = builder.add_extension(v, critical=True) + + if k in noncritical_extensions: + current_app.logger.debug('Adding Extension: {0} {1}'.format(k, v)) + builder = builder.add_extension(v, critical=False) ski = extensions.get('subject_key_identifier', {}) if ski.get('include_ski', False): diff --git a/lemur/common/fields.py b/lemur/common/fields.py index c15c160c..b50a2088 100644 --- a/lemur/common/fields.py +++ b/lemur/common/fields.py @@ -131,23 +131,24 @@ class KeyUsageExtension(Field): 'encipher_only': False, 'decipher_only': False } + for k, v in value.items(): if k == 'useDigitalSignature': keyusages['digital_signature'] = v - if k == 'useNonRepudiation': + elif k == 'useNonRepudiation': keyusages['content_commitment'] = v - if k == 'useKeyEncipherment': + elif k == 'useKeyEncipherment': keyusages['key_encipherment'] = v - if k == 'useDataEncipherment': + elif k == 'useDataEncipherment': keyusages['data_encipherment'] = v - if k == 'useKeyCertSign': + elif k == 'useKeyCertSign': keyusages['key_cert_sign'] = v - if k == 'useCrlSign': + elif k == 'useCrlSign': keyusages['crl_sign'] = v - if k == 'useEncipherOnly' and v: + elif k == 'useEncipherOnly' and v: keyusages['encipher_only'] = True keyusages['key_agreement'] = True - if k == 'useDecipherOnly' and v: + elif k == 'useDecipherOnly' and v: keyusages['decipher_only'] = True keyusages['key_agreement'] = True @@ -182,23 +183,23 @@ class ExtendedKeyUsageExtension(Field): usage_list = {} for usage in usages: if usage.dotted_string == x509.oid.ExtendedKeyUsageOID.CLIENT_AUTH: - usage_list["useClientAuthentication"] = True + usage_list['useClientAuthentication'] = True if usage.dotted_string == x509.oid.ExtendedKeyUsageOID.SERVER_AUTH: - usage_list["useServerAuthentication"] = True + usage_list['useServerAuthentication'] = True if usage.dotted_string == x509.oid.ExtendedKeyUsageOID.CODE_SIGNING: - usage_list["useCodeSigning"] = True + usage_list['useCodeSigning'] = True if usage.dotted_string == x509.oid.ExtendedKeyUsageOID.EMAIL_PROTECTION: - usage_list["useEmailProtection"] = True + usage_list['useEmailProtection'] = True if usage.dotted_string == x509.oid.ExtendedKeyUsageOID.TIME_STAMPING: - usage_list["useTimestamping"] = True + usage_list['useTimestamping'] = True if usage.dotted_string == x509.oid.ExtendedKeyUsageOID.OCSP_SIGNING: - usage_list["useOCSPSigning"] = True - if usage.dotted_string == "1.3.6.1.5.5.7.3.14": - usage_list["useEapOverLAN"] = True - if usage.dotted_string == "1.3.6.1.5.5.7.3.13": - usage_list["useEapOverPPP"] = True - if usage.dotted_string == "1.3.6.1.4.1.311.20.2.2": - usage_list["useSmartCardLogon"] = True + usage_list['useOCSPSigning'] = True + if usage.dotted_string == '1.3.6.1.5.5.7.3.14': + usage_list['useEapOverLAN'] = True + if usage.dotted_string == '1.3.6.1.5.5.7.3.13': + usage_list['useEapOverPPP'] = True + if usage.dotted_string == '1.3.6.1.4.1.311.20.2.2': + usage_list['useSmartCardLogon'] = True return usage_list @@ -238,7 +239,7 @@ class BasicConstraintsExtension(Field): """ def _serialize(self, value, attr, obj): - return {'ca': value.ca(), 'path_length': value.path_length()} + return {'ca': value.ca, 'path_length': value.path_length} def _deserialize(self, value, attr, data): ca = value.get('ca', False) @@ -261,16 +262,15 @@ class SubjectAlternativeNameExtension(Field): :param kwargs: The same keyword arguments that :class:`Field` receives. """ - def _serialize(self, value, attr, obj): general_names = [] + name_type = None for name in value._general_names: - value = name.value() + value = name.value if isinstance(name, x509.DNSName): name_type = 'DNSName' if isinstance(name, x509.IPAddress): name_type = 'IPAddress' - value = str(value) if isinstance(name, x509.UniformResourceIdentifier): name_type = 'uniformResourceIdentifier' if isinstance(name, x509.DirectoryName): @@ -286,7 +286,7 @@ class SubjectAlternativeNameExtension(Field): def _deserialize(self, value, attr, data): general_names = [] - for name in value.get('names', []): + for name in value: if name['nameType'] == 'DNSName': general_names.append(x509.DNSName(name['value'])) if name['nameType'] == 'IPAddress': @@ -296,7 +296,7 @@ class SubjectAlternativeNameExtension(Field): if name['nameType'] == 'uniformResourceIdentifier': general_names.append(x509.UniformResourceIdentifier(name['value'])) if name['nameType'] == 'directoryName': - # FIXME: Need to parse a string in name['value'] like: + # TODO: Need to parse a string in name['value'] like: # 'CN=Common Name, O=Org Name, OU=OrgUnit Name, C=US, ST=ST, L=City/emailAddress=person@example.com' # or # 'CN=Common Name/O=Org Name/OU=OrgUnit Name/C=US/ST=NH/L=City/emailAddress=person@example.com' @@ -327,7 +327,4 @@ class SubjectAlternativeNameExtension(Field): # The Python Cryptography library doesn't support EDIPartyName types (yet?) pass - if general_names: - return x509.SubjectAlternativeName(general_names) - else: - return None + return x509.SubjectAlternativeName(general_names) diff --git a/lemur/plugins/lemur_digicert/plugin.py b/lemur/plugins/lemur_digicert/plugin.py index 225186b9..032ba6e6 100644 --- a/lemur/plugins/lemur_digicert/plugin.py +++ b/lemur/plugins/lemur_digicert/plugin.py @@ -22,13 +22,14 @@ from retrying import retry from flask import current_app +from cryptography import x509 + from lemur.extensions import metrics +from lemur.common.utils import validate_conf from lemur.plugins.bases import IssuerPlugin, SourcePlugin from lemur.plugins import lemur_digicert as digicert -from lemur.common.utils import validate_conf - def log_status_code(r, *args, **kwargs): """ @@ -106,7 +107,8 @@ def get_additional_names(options): # add SANs if present if options.get('extensions'): for san in options['extensions']['sub_alt_names']['names']: - names.append(san['value']) + if isinstance(san, x509.DNSName): + names.append(san.value) return names @@ -119,19 +121,14 @@ def map_fields(options, csr): """ options = get_issuance(options) - data = { - "certificate": - { - "common_name": options['common_name'], - "csr": csr, - "signature_hash": - signature_hash(options.get('signing_algorithm')), - }, - "organization": - { - "id": current_app.config.get("DIGICERT_ORG_ID") - }, - } + data = dict(certificate={ + "common_name": options['common_name'], + "csr": csr, + "signature_hash": + signature_hash(options.get('signing_algorithm')), + }, organization={ + "id": current_app.config.get("DIGICERT_ORG_ID") + }) data['certificate']['dns_names'] = get_additional_names(options) data['custom_expiration_date'] = options['validity_end'].format('YYYY-MM-DD') diff --git a/lemur/plugins/lemur_digicert/tests/test_digicert.py b/lemur/plugins/lemur_digicert/tests/test_digicert.py index a4e5f7a4..915c768e 100644 --- a/lemur/plugins/lemur_digicert/tests/test_digicert.py +++ b/lemur/plugins/lemur_digicert/tests/test_digicert.py @@ -4,6 +4,8 @@ from freezegun import freeze_time from lemur.tests.vectors import CSR_STR +from cryptography import x509 + def test_map_fields(app): from lemur.plugins.lemur_digicert.plugin import map_fields @@ -16,7 +18,7 @@ def test_map_fields(app): 'description': 'test certificate', 'extensions': { 'sub_alt_names': { - 'names': [{'name_type': 'DNSName', 'value': x} for x in names] + 'names': [x509.DNSName(x) for x in names] } }, 'validity_end': arrow.get(2017, 5, 7), @@ -48,7 +50,7 @@ def test_map_cis_fields(app): 'description': 'test certificate', 'extensions': { 'sub_alt_names': { - 'names': [{'name_type': 'DNSName', 'value': x} for x in names] + 'names': [x509.DNSName(x) for x in names] } }, 'organization': 'Example, Inc.', diff --git a/lemur/plugins/lemur_verisign/plugin.py b/lemur/plugins/lemur_verisign/plugin.py index 2ff5fe4c..88de9d29 100644 --- a/lemur/plugins/lemur_verisign/plugin.py +++ b/lemur/plugins/lemur_verisign/plugin.py @@ -13,6 +13,7 @@ import xmltodict from flask import current_app +from cryptography import x509 from lemur.extensions import metrics from lemur.plugins import lemur_verisign as verisign @@ -76,6 +77,22 @@ def log_status_code(r, *args, **kwargs): metrics.send('symantec_status_code_{}'.format(r.status_code), 'counter', 1) +def get_additional_names(options): + """ + Return a list of strings to be added to a SAN certificates. + + :param options: + :return: + """ + names = [] + # add SANs if present + if options.get('extensions'): + for san in options['extensions']['sub_alt_names']: + if isinstance(san, x509.DNSName): + names.append(san.value) + return names + + def process_options(options): """ Processes and maps the incoming issuer options to fields/options that @@ -94,9 +111,7 @@ def process_options(options): 'email': current_app.config.get("VERISIGN_EMAIL") } - if options.get('extensions'): - if options['extensions'].get('sub_alt_names'): - data['subject_alt_names'] = ",".join(x['value'] for x in options['extensions']['sub_alt_names']['names']) + data['subject_alt_names'] = ",".join(get_additional_names(options)) if options.get('validity_end'): period = get_default_issuance(options) diff --git a/lemur/schemas.py b/lemur/schemas.py index 9eff45e9..5ca674b6 100644 --- a/lemur/schemas.py +++ b/lemur/schemas.py @@ -195,12 +195,16 @@ class CustomOIDSchema(BaseExtensionSchema): is_critical = fields.Boolean() +class NamesSchema(BaseExtensionSchema): + names = SubjectAlternativeNameExtension() + + class ExtensionSchema(BaseExtensionSchema): basic_constraints = BasicConstraintsExtension() key_usage = KeyUsageExtension() extended_key_usage = ExtendedKeyUsageExtension() subject_key_identifier = fields.Nested(SubjectKeyIdentifierSchema) - sub_alt_names = SubjectAlternativeNameExtension() + sub_alt_names = fields.Nested(NamesSchema) authority_key_identifier = fields.Nested(AuthorityKeyIdentifierSchema) certificate_info_access = fields.Nested(CertificateInfoAccessSchema) # FIXME: Convert custom OIDs to a custom field in fields.py like other Extensions diff --git a/lemur/static/app/angular/certificates/certificate/options.tpl.html b/lemur/static/app/angular/certificates/certificate/options.tpl.html index f8917357..b4cfb8d7 100644 --- a/lemur/static/app/angular/certificates/certificate/options.tpl.html +++ b/lemur/static/app/angular/certificates/certificate/options.tpl.html @@ -26,40 +26,6 @@ class="help-block">Enter a valid certificate signing request.

-
- -
- -
-
-
- - - - -
-
-
-
-
- - - - - - -
{{ alt.nameType }}{{ alt.value }} - -
-
-
+
+ +
+ +
+
+
+ + + + +
+
+
+
+
+ + + + + + +
{{ alt.nameType }}{{ alt.value }} + +
+
+