Begin the deprecation of auto-idna for x509.DNSName

CHANGELOG.rst

@@ -9,6 +9,12 @@ Changelog
 * **BACKWARDS INCOMPATIBLE:** ``Whirlpool``, ``RIPEMD160``, and
   ``UnsupportedExtension`` have been removed in accordance with our
   :doc:`/api-stability` policy.
+* Deprecated passing unicode to the :class:`~cryptography.x509.DNSName`
+  constructor. Instead, users should pass DNS names as ``bytes``, with ``idna``
+  encoding if necessary. In addition, the
+  :attr:`~cryptography.x509.DNSName.value` attribute was deprecated, users
+  should use :attr:`~cryptography.x509.DNSName.bytes_value` to access the
+  raw DNS name.
 
 2.0.2 - 2017-07-27
 ~~~~~~~~~~~~~~~~~~

docs/spelling_wordlist.txt

@@ -1,3 +1,4 @@
+accessor
 affine
 Authenticator
 backend
@@ -43,6 +44,7 @@ Google
 hazmat
 Homebrew
 hostname
+idna
 indistinguishability
 initialisms
 interoperable

docs/x509/reference.rst

@@ -599,7 +599,7 @@ X.509 Certificate Builder
         >>> builder = builder.public_key(public_key)
         >>> builder = builder.add_extension(
         ...     x509.SubjectAlternativeName(
-        ...         [x509.DNSName(u'cryptography.io')]
+        ...         [x509.DNSName(b'cryptography.io')]
         ...     ),
         ...     critical=False
         ... )
@@ -1242,8 +1242,14 @@ General Name Classes
 
     This corresponds to a domain name. For example, ``cryptography.io``.
 
+    .. attribute:: bytes_value
+
+        :type: bytes
+
     .. attribute:: value
 
+        Deprecated accessor for the idna-decoded value of :attr:`bytes_value`
+
         :type: :term:`text`
 
 .. class:: DirectoryName(value)

docs/x509/tutorial.rst

@@ -70,9 +70,9 @@ a few details:
     ... ])).add_extension(
     ...     x509.SubjectAlternativeName([
     ...         # Describe what sites we want this certificate for.
-    ...         x509.DNSName(u"mysite.com"),
-    ...         x509.DNSName(u"www.mysite.com"),
-    ...         x509.DNSName(u"subdomain.mysite.com"),
+    ...         x509.DNSName(b"mysite.com"),
+    ...         x509.DNSName(b"www.mysite.com"),
+    ...         x509.DNSName(b"subdomain.mysite.com"),
     ...     ]),
     ...     critical=False,
     ... # Sign the CSR with our private key.
@@ -142,7 +142,7 @@ Then we generate the certificate itself:
     ...     # Our certificate will be valid for 10 days
     ...     datetime.datetime.utcnow() + datetime.timedelta(days=10)
     ... ).add_extension(
-    ...     x509.SubjectAlternativeName([x509.DNSName(u"localhost")]),
+    ...     x509.SubjectAlternativeName([x509.DNSName(b"localhost")]),
     ...     critical=False,
     ... # Sign our certificate with our private key
     ... ).sign(key, hashes.SHA256(), default_backend())

src/cryptography/hazmat/backends/openssl/decode_asn1.py

@@ -88,23 +88,7 @@ def _decode_general_names(backend, gns):
 def _decode_general_name(backend, gn):
     if gn.type == backend._lib.GEN_DNS:
         data = _asn1_string_to_bytes(backend, gn.d.dNSName)
-        if not data:
-            decoded = u""
-        elif data.startswith(b"*."):
-            # This is a wildcard name. We need to remove the leading wildcard,
-            # IDNA decode, then re-add the wildcard. Wildcard characters should
-            # always be left-most (RFC 2595 section 2.4).
-            decoded = u"*." + idna.decode(data[2:])
-        else:
-            # Not a wildcard, decode away. If the string has a * in it anywhere
-            # invalid this will raise an InvalidCodePoint
-            decoded = idna.decode(data)
-            if data.startswith(b"."):
-                # idna strips leading periods. Name constraints can have that
-                # so we need to re-add it. Sigh.
-                decoded = u"." + decoded
-
-        return x509.DNSName(decoded)
+        return x509.DNSName(data)
     elif gn.type == backend._lib.GEN_URI:
         data = _asn1_string_to_ascii(backend, gn.d.uniformResourceIdentifier)
         parsed = urllib_parse.urlparse(data)

src/cryptography/hazmat/backends/openssl/encode_asn1.py

@@ -7,8 +7,6 @@
 import calendar
 import ipaddress
 
-import idna
-
 import six
 
 from cryptography import utils, x509
@@ -370,15 +368,6 @@ def _encode_subject_key_identifier(backend, ski):
     return _encode_asn1_str_gc(backend, ski.digest, len(ski.digest))
 
 
-def _idna_encode(value):
-    # Retain prefixes '*.' for common/alt names and '.' for name constraints
-    for prefix in ['*.', '.']:
-        if value.startswith(prefix):
-            value = value[len(prefix):]
-            return prefix.encode('ascii') + idna.encode(value)
-    return idna.encode(value)
-
-
 def _encode_general_name(backend, name):
     if isinstance(name, x509.DNSName):
         gn = backend._lib.GENERAL_NAME_new()
@@ -387,7 +376,7 @@ def _encode_general_name(backend, name):
 
         ia5 = backend._lib.ASN1_IA5STRING_new()
         backend.openssl_assert(ia5 != backend._ffi.NULL)
-        value = _idna_encode(name.value)
+        value = name.bytes_value
 
         res = backend._lib.ASN1_STRING_set(ia5, value, len(value))
         backend.openssl_assert(res == 1)

src/cryptography/utils.py

@@ -16,6 +16,7 @@
 # cycle ends.
 PersistentlyDeprecated = DeprecationWarning
 DeprecatedIn19 = DeprecationWarning
+DeprecatedIn21 = PendingDeprecationWarning
 
 
 def _check_bytes(name, value):

src/cryptography/x509/general_name.py

@@ -6,6 +6,7 @@
 
 import abc
 import ipaddress
+import warnings
 from email.utils import parseaddr
 
 import idna
@@ -89,24 +90,81 @@ def __hash__(self):
         return hash(self.value)
 
 
+def _idna_encode(value):
+    # Retain prefixes '*.' for common/alt names and '.' for name constraints
+    for prefix in ['*.', '.']:
+        if value.startswith(prefix):
+            value = value[len(prefix):]
+            return prefix.encode('ascii') + idna.encode(value)
+    return idna.encode(value)
+
+
 @utils.register_interface(GeneralName)
 class DNSName(object):
     def __init__(self, value):
-        if not isinstance(value, six.text_type):
-            raise TypeError("value must be a unicode string")
-
-        self._value = value
-
-    value = utils.read_only_property("_value")
+        if isinstance(value, six.text_type):
+            try:
+                value = value.encode("ascii")
+            except UnicodeEncodeError:
+                value = _idna_encode(value)
+                warnings.warn(
+                    "DNSName values should be passed as idna-encoded bytes, "
+                    "not strings. Support for passing unicode strings will be "
+                    "removed in a future version.",
+                    utils.DeprecatedIn21,
+                    stacklevel=2,
+                )
+            else:
+                warnings.warn(
+                    "DNSName values should be passed as bytes, not strings. "
+                    "Support for passing unicode strings will be removed in a "
+                    "future version.",
+                    utils.DeprecatedIn21,
+                    stacklevel=2,
+                )
+        elif not isinstance(value, bytes):
+            raise TypeError("value must be bytes")
+
+        self._bytes_value = value
+
+    bytes_value = utils.read_only_property("_bytes_value")
+
+    @property
+    def value(self):
+        warnings.warn(
+            "DNSName.bytes_value should be used instead of DNSName.value; it "
+            "contains the DNS name as raw bytes, instead of as an idna-decoded"
+            " unicode string. DNSName.value will be removed in a future "
+            "version.",
+            utils.DeprecatedIn21,
+            stacklevel=2
+        )
+        data = self._bytes_value
+        if not data:
+            decoded = u""
+        elif data.startswith(b"*."):
+            # This is a wildcard name. We need to remove the leading wildcard,
+            # IDNA decode, then re-add the wildcard. Wildcard characters should
+            # always be left-most (RFC 2595 section 2.4).
+            decoded = u"*." + idna.decode(data[2:])
+        else:
+            # Not a wildcard, decode away. If the string has a * in it anywhere
+            # invalid this will raise an InvalidCodePoint
+            decoded = idna.decode(data)
+            if data.startswith(b"."):
+                # idna strips leading periods. Name constraints can have that
+                # so we need to re-add it. Sigh.
+                decoded = u"." + decoded
+        return decoded
 
     def __repr__(self):
-        return "<DNSName(value={0})>".format(self.value)
+        return "<DNSName(bytes_value={0!r})>".format(self.bytes_value)
 
     def __eq__(self, other):
         if not isinstance(other, DNSName):
             return NotImplemented
 
-        return self.value == other.value
+        return self.bytes_value == other.bytes_value
 
     def __ne__(self, other):
         return not self == other

tests/test_x509.py

@@ -225,7 +225,7 @@ def test_extensions(self, backend):
         assert aia.value == x509.AuthorityInformationAccess([
             x509.AccessDescription(
                 AuthorityInformationAccessOID.CA_ISSUERS,
-                x509.DNSName(u"cryptography.io")
+                x509.DNSName(b"cryptography.io")
             )
         ])
         assert ian.value == x509.IssuerAlternativeName([
@@ -1176,8 +1176,8 @@ def test_subject_alt_name(self, backend):
             ExtensionOID.SUBJECT_ALTERNATIVE_NAME
         )
         assert list(ext.value) == [
-            x509.DNSName(u"cryptography.io"),
-            x509.DNSName(u"sub.cryptography.io"),
+            x509.DNSName(b"cryptography.io"),
+            x509.DNSName(b"sub.cryptography.io"),
         ]
 
     def test_public_bytes_pem(self, backend):
@@ -1405,7 +1405,7 @@ def test_build_cert(self, backend):
         ).add_extension(
             x509.BasicConstraints(ca=False, path_length=None), True,
         ).add_extension(
-            x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
+            x509.SubjectAlternativeName([x509.DNSName(b"cryptography.io")]),
             critical=False,
         ).not_valid_before(
             not_valid_before
@@ -1427,7 +1427,7 @@ def test_build_cert(self, backend):
             ExtensionOID.SUBJECT_ALTERNATIVE_NAME
         )
         assert list(subject_alternative_name.value) == [
-            x509.DNSName(u"cryptography.io"),
+            x509.DNSName(b"cryptography.io"),
         ]
 
     def test_build_cert_printable_string_country_name(self, backend):
@@ -2166,7 +2166,7 @@ def test_build_cert_with_dsa_private_key(self, backend):
         ).add_extension(
             x509.BasicConstraints(ca=False, path_length=None), True,
         ).add_extension(
-            x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
+            x509.SubjectAlternativeName([x509.DNSName(b"cryptography.io")]),
             critical=False,
         ).not_valid_before(
             not_valid_before
@@ -2188,7 +2188,7 @@ def test_build_cert_with_dsa_private_key(self, backend):
             ExtensionOID.SUBJECT_ALTERNATIVE_NAME
         )
         assert list(subject_alternative_name.value) == [
-            x509.DNSName(u"cryptography.io"),
+            x509.DNSName(b"cryptography.io"),
         ]
 
     @pytest.mark.requires_backend_interface(interface=EllipticCurveBackend)
@@ -2212,7 +2212,7 @@ def test_build_cert_with_ec_private_key(self, backend):
         ).add_extension(
             x509.BasicConstraints(ca=False, path_length=None), True,
         ).add_extension(
-            x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
+            x509.SubjectAlternativeName([x509.DNSName(b"cryptography.io")]),
             critical=False,
         ).not_valid_before(
             not_valid_before
@@ -2234,7 +2234,7 @@ def test_build_cert_with_ec_private_key(self, backend):
             ExtensionOID.SUBJECT_ALTERNATIVE_NAME
         )
         assert list(subject_alternative_name.value) == [
-            x509.DNSName(u"cryptography.io"),
+            x509.DNSName(b"cryptography.io"),
         ]
 
     @pytest.mark.requires_backend_interface(interface=RSABackend)
@@ -2375,7 +2375,7 @@ def test_issuer_alt_name(self, backend):
             123
         ).add_extension(
             x509.IssuerAlternativeName([
-                x509.DNSName(u"myissuer"),
+                x509.DNSName(b"myissuer"),
                 x509.RFC822Name(u"[email protected]"),
             ]), critical=False
         ).sign(issuer_private_key, hashes.SHA256(), backend)
@@ -2385,7 +2385,7 @@ def test_issuer_alt_name(self, backend):
         )
         assert ext.critical is False
         assert ext.value == x509.IssuerAlternativeName([
-            x509.DNSName(u"myissuer"),
+            x509.DNSName(b"myissuer"),
             x509.RFC822Name(u"[email protected]"),
         ])
 
@@ -2525,7 +2525,7 @@ def test_policy_constraints(self, backend, pc):
                         ipaddress.IPv6Network(u"FF:FF:0:0:0:0:0:0/128")
                     ),
                 ],
-                excluded_subtrees=[x509.DNSName(u"name.local")]
+                excluded_subtrees=[x509.DNSName(b"name.local")]
             ),
             x509.NameConstraints(
                 permitted_subtrees=[
@@ -2535,7 +2535,7 @@ def test_policy_constraints(self, backend, pc):
             ),
             x509.NameConstraints(
                 permitted_subtrees=None,
-                excluded_subtrees=[x509.DNSName(u"name.local")]
+                excluded_subtrees=[x509.DNSName(b"name.local")]
             ),
         ]
     )
@@ -2909,7 +2909,7 @@ def test_add_unsupported_extension(self, backend):
                 x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'),
             ])
         ).add_extension(
-            x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
+            x509.SubjectAlternativeName([x509.DNSName(b"cryptography.io")]),
             critical=False,
         ).add_extension(
             DummyExtension(), False
@@ -2995,7 +2995,7 @@ def test_add_two_extensions(self, backend):
         request = builder.subject_name(
             x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME, u'US')])
         ).add_extension(
-            x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
+            x509.SubjectAlternativeName([x509.DNSName(b"cryptography.io")]),
             critical=False,
         ).add_extension(
             x509.BasicConstraints(ca=True, path_length=2), critical=True
@@ -3012,7 +3012,7 @@ def test_add_two_extensions(self, backend):
         ext = request.extensions.get_extension_for_oid(
             ExtensionOID.SUBJECT_ALTERNATIVE_NAME
         )
-        assert list(ext.value) == [x509.DNSName(u"cryptography.io")]
+        assert list(ext.value) == [x509.DNSName(b"cryptography.io")]
 
     def test_set_subject_twice(self):
         builder = x509.CertificateSigningRequestBuilder()
@@ -3032,8 +3032,8 @@ def test_subject_alt_names(self, backend):
         private_key = RSA_KEY_2048.private_key(backend)
 
         san = x509.SubjectAlternativeName([
-            x509.DNSName(u"example.com"),
-            x509.DNSName(u"*.example.com"),
+            x509.DNSName(b"example.com"),
+            x509.DNSName(b"*.example.com"),
             x509.RegisteredID(x509.ObjectIdentifier("1.2.3.4.5.6.7")),
             x509.DirectoryName(x509.Name([
                 x509.NameAttribute(NameOID.COMMON_NAME, u'PyCA'),