Fix ASN1 string type encoding for several Name OIDs (#4035)

* Fix ASN1 string type encoding for several Name OIDs

When we changed over to the new type encoding system we didn't verify
that the new code exactly matched the ASN1 string types that OpenSSL was
previously choosing. This caused serialNumber, dnQualifier,
emailAddress, and domainComponent to change from their proper encodings
to UTF8String as of version 2.1.

Now we check to see if there's a sentinel value (indicating no custom
type has been passed) and then check if the OID has a different default
than UTF8. If it does, we set it.

This PR also adds tests for the ASN1 string type of ever supported
NameOID.

* review feedback

2 files changed, 150 insertions, 23 deletions

diff --git a/src/cryptography/x509/name.py b/src/cryptography/x509/name.py
index 2fbaee91..0daa8bbd 100644
--- a/src/cryptography/x509/name.py
+++ b/src/cryptography/x509/name.py
@@ -27,6 +27,14 @@ class _ASN1Type(Enum):
 
 _ASN1_TYPE_TO_ENUM = dict((i.value, i) for i in _ASN1Type)
 _SENTINEL = object()
+_NAMEOID_DEFAULT_TYPE = {
+    NameOID.COUNTRY_NAME: _ASN1Type.PrintableString,
+    NameOID.JURISDICTION_COUNTRY_NAME: _ASN1Type.PrintableString,
+    NameOID.SERIAL_NUMBER: _ASN1Type.PrintableString,
+    NameOID.DN_QUALIFIER: _ASN1Type.PrintableString,
+    NameOID.EMAIL_ADDRESS: _ASN1Type.IA5String,
+    NameOID.DOMAIN_COMPONENT: _ASN1Type.IA5String,
+}
 
 
 class NameAttribute(object):
@@ -50,17 +58,17 @@ class NameAttribute(object):
                     "Country name must be a 2 character country code"
                 )
 
-            if _type == _SENTINEL:
-                _type = _ASN1Type.PrintableString
-
         if len(value) == 0:
             raise ValueError("Value cannot be an empty string")
 
-        # Set the default string type for encoding ASN1 strings to UTF8. This
-        # is the default for newer OpenSSLs for several years (1.0.1h+) and is
-        # recommended in RFC 2459.
+        # The appropriate ASN1 string type varies by OID and is defined across
+        # multiple RFCs including 2459, 3280, and 5280. In general UTF8String
+        # is preferred (2459), but 3280 and 5280 specify several OIDs with
+        # alternate types. This means when we see the sentinel value we need
+        # to look up whether the OID has a non-UTF8 type. If it does, set it
+        # to that. Otherwise, UTF8!
         if _type == _SENTINEL:
-            _type = _ASN1Type.UTF8String
+            _type = _NAMEOID_DEFAULT_TYPE.get(oid, _ASN1Type.UTF8String)
 
         if not isinstance(_type, _ASN1Type):
             raise TypeError("_type must be from the _ASN1Type enum")
diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py
index 27e284a3..6c33043b 100644
--- a/tests/x509/test_x509.py
+++ b/tests/x509/test_x509.py
@@ -1650,6 +1650,59 @@ class TestCertificateBuilder(object):
 
         builder.sign(private_key, hashes.SHA256(), backend)
 
+    @pytest.mark.requires_backend_interface(interface=RSABackend)
+    @pytest.mark.requires_backend_interface(interface=X509Backend)
+    def test_subject_dn_asn1_types(self, backend):
+        private_key = RSA_KEY_2048.private_key(backend)
+
+        name = x509.Name([
+            x509.NameAttribute(NameOID.COMMON_NAME, u"mysite.com"),
+            x509.NameAttribute(NameOID.COUNTRY_NAME, u"US"),
+            x509.NameAttribute(NameOID.LOCALITY_NAME, u"value"),
+            x509.NameAttribute(NameOID.STATE_OR_PROVINCE_NAME, u"value"),
+            x509.NameAttribute(NameOID.STREET_ADDRESS, u"value"),
+            x509.NameAttribute(NameOID.ORGANIZATION_NAME, u"value"),
+            x509.NameAttribute(NameOID.ORGANIZATIONAL_UNIT_NAME, u"value"),
+            x509.NameAttribute(NameOID.SERIAL_NUMBER, u"value"),
+            x509.NameAttribute(NameOID.SURNAME, u"value"),
+            x509.NameAttribute(NameOID.GIVEN_NAME, u"value"),
+            x509.NameAttribute(NameOID.TITLE, u"value"),
+            x509.NameAttribute(NameOID.GENERATION_QUALIFIER, u"value"),
+            x509.NameAttribute(NameOID.X500_UNIQUE_IDENTIFIER, u"value"),
+            x509.NameAttribute(NameOID.DN_QUALIFIER, u"value"),
+            x509.NameAttribute(NameOID.PSEUDONYM, u"value"),
+            x509.NameAttribute(NameOID.USER_ID, u"value"),
+            x509.NameAttribute(NameOID.DOMAIN_COMPONENT, u"value"),
+            x509.NameAttribute(NameOID.EMAIL_ADDRESS, u"value"),
+            x509.NameAttribute(NameOID.JURISDICTION_COUNTRY_NAME, u"US"),
+            x509.NameAttribute(NameOID.JURISDICTION_LOCALITY_NAME, u"value"),
+            x509.NameAttribute(
+                NameOID.JURISDICTION_STATE_OR_PROVINCE_NAME, u"value"
+            ),
+            x509.NameAttribute(NameOID.BUSINESS_CATEGORY, u"value"),
+            x509.NameAttribute(NameOID.POSTAL_ADDRESS, u"value"),
+            x509.NameAttribute(NameOID.POSTAL_CODE, u"value"),
+        ])
+        cert = x509.CertificateBuilder().subject_name(
+            name
+        ).issuer_name(
+            name
+        ).public_key(
+            private_key.public_key()
+        ).serial_number(
+            777
+        ).not_valid_before(
+            datetime.datetime(1999, 1, 1)
+        ).not_valid_after(
+            datetime.datetime(2020, 1, 1)
+        ).sign(private_key, hashes.SHA256(), backend)
+
+        for dn in (cert.subject, cert.issuer):
+            for oid, asn1_type in TestNameAttribute.EXPECTED_TYPES:
+                assert dn.get_attributes_for_oid(
+                    oid
+                )[0]._type == asn1_type
+
     @pytest.mark.skipif(sys.platform != "win32", reason="Requires windows")
     @pytest.mark.parametrize(
         ("not_valid_before", "not_valid_after"),
@@ -2748,6 +2801,47 @@ class TestCertificateSigningRequestBuilder(object):
         ]
 
     @pytest.mark.requires_backend_interface(interface=RSABackend)
+    def test_subject_dn_asn1_types(self, backend):
+        private_key = RSA_KEY_2048.private_key(backend)
+
+        request = x509.CertificateSigningRequestBuilder().subject_name(
+            x509.Name([
+                x509.NameAttribute(NameOID.COMMON_NAME, u"mysite.com"),
+                x509.NameAttribute(NameOID.COUNTRY_NAME, u"US"),
+                x509.NameAttribute(NameOID.LOCALITY_NAME, u"value"),
+                x509.NameAttribute(NameOID.STATE_OR_PROVINCE_NAME, u"value"),
+                x509.NameAttribute(NameOID.STREET_ADDRESS, u"value"),
+                x509.NameAttribute(NameOID.ORGANIZATION_NAME, u"value"),
+                x509.NameAttribute(NameOID.ORGANIZATIONAL_UNIT_NAME, u"value"),
+                x509.NameAttribute(NameOID.SERIAL_NUMBER, u"value"),
+                x509.NameAttribute(NameOID.SURNAME, u"value"),
+                x509.NameAttribute(NameOID.GIVEN_NAME, u"value"),
+                x509.NameAttribute(NameOID.TITLE, u"value"),
+                x509.NameAttribute(NameOID.GENERATION_QUALIFIER, u"value"),
+                x509.NameAttribute(NameOID.X500_UNIQUE_IDENTIFIER, u"value"),
+                x509.NameAttribute(NameOID.DN_QUALIFIER, u"value"),
+                x509.NameAttribute(NameOID.PSEUDONYM, u"value"),
+                x509.NameAttribute(NameOID.USER_ID, u"value"),
+                x509.NameAttribute(NameOID.DOMAIN_COMPONENT, u"value"),
+                x509.NameAttribute(NameOID.EMAIL_ADDRESS, u"value"),
+                x509.NameAttribute(NameOID.JURISDICTION_COUNTRY_NAME, u"US"),
+                x509.NameAttribute(
+                    NameOID.JURISDICTION_LOCALITY_NAME, u"value"
+                ),
+                x509.NameAttribute(
+                    NameOID.JURISDICTION_STATE_OR_PROVINCE_NAME, u"value"
+                ),
+                x509.NameAttribute(NameOID.BUSINESS_CATEGORY, u"value"),
+                x509.NameAttribute(NameOID.POSTAL_ADDRESS, u"value"),
+                x509.NameAttribute(NameOID.POSTAL_CODE, u"value"),
+            ])
+        ).sign(private_key, hashes.SHA256(), backend)
+        for oid, asn1_type in TestNameAttribute.EXPECTED_TYPES:
+            assert request.subject.get_attributes_for_oid(
+                oid
+            )[0]._type == asn1_type
+
+    @pytest.mark.requires_backend_interface(interface=RSABackend)
     def test_build_ca_request_with_multivalue_rdns(self, backend):
         private_key = RSA_KEY_2048.private_key(backend)
         subject = x509.Name([
@@ -3668,6 +3762,47 @@ class TestOtherCertificate(object):
 
 
 class TestNameAttribute(object):
+    EXPECTED_TYPES = [
+        (NameOID.COMMON_NAME, _ASN1Type.UTF8String),
+        (NameOID.COUNTRY_NAME, _ASN1Type.PrintableString),
+        (NameOID.LOCALITY_NAME, _ASN1Type.UTF8String),
+        (NameOID.STATE_OR_PROVINCE_NAME, _ASN1Type.UTF8String),
+        (NameOID.STREET_ADDRESS, _ASN1Type.UTF8String),
+        (NameOID.ORGANIZATION_NAME, _ASN1Type.UTF8String),
+        (NameOID.ORGANIZATIONAL_UNIT_NAME, _ASN1Type.UTF8String),
+        (NameOID.SERIAL_NUMBER, _ASN1Type.PrintableString),
+        (NameOID.SURNAME, _ASN1Type.UTF8String),
+        (NameOID.GIVEN_NAME, _ASN1Type.UTF8String),
+        (NameOID.TITLE, _ASN1Type.UTF8String),
+        (NameOID.GENERATION_QUALIFIER, _ASN1Type.UTF8String),
+        (NameOID.X500_UNIQUE_IDENTIFIER, _ASN1Type.UTF8String),
+        (NameOID.DN_QUALIFIER, _ASN1Type.PrintableString),
+        (NameOID.PSEUDONYM, _ASN1Type.UTF8String),
+        (NameOID.USER_ID, _ASN1Type.UTF8String),
+        (NameOID.DOMAIN_COMPONENT, _ASN1Type.IA5String),
+        (NameOID.EMAIL_ADDRESS, _ASN1Type.IA5String),
+        (NameOID.JURISDICTION_COUNTRY_NAME, _ASN1Type.PrintableString),
+        (NameOID.JURISDICTION_LOCALITY_NAME, _ASN1Type.UTF8String),
+        (
+            NameOID.JURISDICTION_STATE_OR_PROVINCE_NAME,
+            _ASN1Type.UTF8String
+        ),
+        (NameOID.BUSINESS_CATEGORY, _ASN1Type.UTF8String),
+        (NameOID.POSTAL_ADDRESS, _ASN1Type.UTF8String),
+        (NameOID.POSTAL_CODE, _ASN1Type.UTF8String),
+    ]
+
+    def test_default_types(self):
+        for oid, asn1_type in TestNameAttribute.EXPECTED_TYPES:
+            na = x509.NameAttribute(oid, u"US")
+            assert na._type == asn1_type
+
+    def test_alternate_type(self):
+        na2 = x509.NameAttribute(
+            NameOID.COMMON_NAME, u"common", _ASN1Type.IA5String
+        )
+        assert na2._type == _ASN1Type.IA5String
+
     def test_init_bad_oid(self):
         with pytest.raises(TypeError):
             x509.NameAttribute(None, u'value')
@@ -3697,22 +3832,6 @@ class TestNameAttribute(object):
         with pytest.raises(ValueError):
             x509.NameAttribute(NameOID.ORGANIZATION_NAME, u'')
 
-    def test_country_name_type(self):
-        na = x509.NameAttribute(NameOID.COUNTRY_NAME, u"US")
-        assert na._type == _ASN1Type.PrintableString
-        na2 = x509.NameAttribute(
-            NameOID.COUNTRY_NAME, u"US", _ASN1Type.IA5String
-        )
-        assert na2._type == _ASN1Type.IA5String
-
-    def test_types(self):
-        na = x509.NameAttribute(NameOID.COMMON_NAME, u"common")
-        assert na._type == _ASN1Type.UTF8String
-        na2 = x509.NameAttribute(
-            NameOID.COMMON_NAME, u"common", _ASN1Type.IA5String
-        )
-        assert na2._type == _ASN1Type.IA5String
-
     def test_invalid_type(self):
         with pytest.raises(TypeError):
             x509.NameAttribute(NameOID.COMMON_NAME, u"common", "notanenum")