Closes #648, also fixes several issues #666. (#678)

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):
+            value = extension.value
-                return_extensions['basic_constraints'] = extension
+            if isinstance(value, x509.BasicConstraints):
-            elif isinstance(extension, x509.SubjectAlternativeName):
+                return_extensions['basic_constraints'] = extension.value
-                return_extensions['sub_alt_names'] = extension
+
-            elif isinstance(extension, x509.ExtendedKeyUsage):
+            elif isinstance(value, x509.SubjectAlternativeName):
-                return_extensions['extended_key_usage'] = extension
+                return_extensions['sub_alt_names'] = {'names': extension.value}
-            elif isinstance(extension, x509.KeyUsage):
+
-                return_extensions['key_usage'] = extension
+            elif isinstance(value, x509.ExtendedKeyUsage):
-            elif isinstance(extension, x509.SubjectKeyIdentifier):
+                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
+            # TODO: Don't support CRLDistributionPoints yet https://github.com/Netflix/lemur/issues/662
-                pass
+            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
+                current_app.logger.warning('Custom OIDs not yet supported for clone operation.')
-                pass
 
         return return_extensions
 

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,11 +347,16 @@ 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:
+        if v:
-            current_app.logger.debug("Add CExt: {0} {1}".format(k, 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 and v:
+
-            current_app.logger.debug("Add Ext: {0} {1}".format(k, v))
+            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', {})

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
+                usage_list['useOCSPSigning'] = True
-            if usage.dotted_string == "1.3.6.1.5.5.7.3.14":
+            if usage.dotted_string == '1.3.6.1.5.5.7.3.14':
-                usage_list["useEapOverLAN"] = True
+                usage_list['useEapOverLAN'] = True
-            if usage.dotted_string == "1.3.6.1.5.5.7.3.13":
+            if usage.dotted_string == '1.3.6.1.5.5.7.3.13':
-                usage_list["useEapOverPPP"] = True
+                usage_list['useEapOverPPP'] = True
-            if usage.dotted_string == "1.3.6.1.4.1.311.20.2.2":
+            if usage.dotted_string == '1.3.6.1.4.1.311.20.2.2':
-                usage_list["useSmartCardLogon"] = True
+                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

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 = {
+    data = dict(certificate={
-        "certificate":
-            {
         "common_name": options['common_name'],
         "csr": csr,
         "signature_hash":
             signature_hash(options.get('signing_algorithm')),
-            },
+    }, organization={
-        "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')

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.',

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'):
+    data['subject_alt_names'] = ",".join(get_additional_names(options))
-        if options['extensions'].get('sub_alt_names'):
-            data['subject_alt_names'] = ",".join(x['value'] for x in options['extensions']['sub_alt_names']['names'])
 
     if options.get('validity_end'):
         period = get_default_issuance(options)

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

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

@@ -26,40 +26,6 @@
              class="help-block">Enter a valid certificate signing request.</p>
         </div>
       </div>
-      <div class="form-group">
-        <label class="control-label col-sm-2">
-          Subject Alternate Names
-        </label>
-        <div class="col-sm-3">
-          <select class="form-control" ng-model="certificate.subAltType"
-                  ng-options="item for item in ['DNSName', 'IPAddress', 'IPNetwork', 'uniformResourceIdentifier', 'directoryName','rfc822Name', 'registeredID']"></select>
-        </div>
-        <div class="col-sm-6">
-          <div class="input-group">
-            <input tooltip-trigger="focus" tooltip-placement="top"
-                   uib-tooltip="String or Base64-encoded DER ASN.1 structure for the value" class="form-control"
-                   name="value" ng-model="certificate.subAltValue" placeholder="Value" class="form-control" required/>
-            <span class="input-group-btn">
-          <button ng-click="certificate.attachSubAltName()" class="btn btn-info">Add</button>
-        </span>
-          </div>
-        </div>
-      </div>
-      <div class="form-group">
-        <div class="col-sm-10 col-sm-offset-2">
-          <table class="table">
-            <tr ng-repeat="alt in certificate.extensions.subAltNames.names track by $index">
-              <td>{{ alt.nameType }}</td>
-              <td>{{ alt.value }}</td>
-              <td>
-                <button type="button" ng-click="certificate.removeSubAltName($index)"
-                        class="btn btn-danger btn-sm pull-right">Remove
-                </button>
-              </td>
-            </tr>
-          </table>
-        </div>
-      </div>
       <div class="form-group">
         <label class="control-label col-sm-2">
           Key Type

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

@@ -40,6 +40,40 @@
           enter a common name and it must be less than 64 characters</p>
       </div>
     </div>
+    <div class="form-group">
+      <label class="control-label col-sm-2">
+        Subject Alternate Names
+      </label>
+      <div class="col-sm-4">
+        <select class="form-control" ng-model="certificate.subAltType"
+                ng-options="item for item in ['DNSName', 'IPAddress', 'IPNetwork', 'uniformResourceIdentifier', 'directoryName','rfc822Name', 'registeredID']"></select>
+      </div>
+      <div class="col-sm-6">
+        <div class="input-group">
+          <input tooltip-trigger="focus" tooltip-placement="top"
+                 uib-tooltip="String or Base64-encoded DER ASN.1 structure for the value" class="form-control"
+                 name="value" ng-model="certificate.subAltValue" placeholder="Value" class="form-control" required/>
+          <span class="input-group-btn">
+          <button ng-click="certificate.attachSubAltName()" class="btn btn-info">Add</button>
+        </span>
+        </div>
+      </div>
+    </div>
+    <div class="form-group">
+      <div class="col-sm-10 col-sm-offset-2">
+        <table class="table">
+          <tr ng-repeat="alt in certificate.extensions.subAltNames.names track by $index">
+            <td>{{ alt.nameType }}</td>
+            <td>{{ alt.value }}</td>
+            <td>
+              <button type="button" ng-click="certificate.removeSubAltName($index)"
+                      class="btn btn-danger btn-sm pull-right">Remove
+              </button>
+            </td>
+          </tr>
+        </table>
+      </div>
+    </div>
     <div class="form-group"
          ng-class="{'has-error': trackingForm.description.$invalid, 'has-success': !trackingForm.$invalid&&trackingForm.description.$dirty}">
       <label class="control-label col-sm-2">

lemur/tests/test_certificates.py

@@ -310,7 +310,7 @@ def test_create_csr():
     assert csr
     assert private_key
 
-    extensions = {'sub_alt_names': x509.SubjectAlternativeName([x509.DNSName('AnotherCommonName')])}
+    extensions = {'sub_alt_names': {'names': x509.SubjectAlternativeName([x509.DNSName('AnotherCommonName')])}}
     csr, private_key = create_csr(owner='joe@example.com', common_name='ACommonName', organization='test', organizational_unit='Meters', country='US',
                                   state='CA', location='Here', extensions=extensions, key_type='RSA2048')
     assert csr