Repository navigation
Deprecate auto-idna in x509 layer #3357
Description
Trying to process IDNA within our certificate code was a mistake and has resulted in significant increases (rather than the desired decrease) in complexity for some of our downstream dependencies (see: https://github.com/kennethreitz/requests/blob/362da46e9a46da6e86e1907f03014384ab210151/requests/packages/urllib3/contrib/pyopenssl.py#L141). I'd like to see it removed, but let's talk a bit about whether we can actually achieve that goal.
Our current API requires that users do something like x509.DNSName(u"mydömain.com"). We also do this when parsing a certificate (we obtain the bytes, do some processing, idna.decode the parts we think we should, then pass the resulting string to the appropriate GeneralName class).
One potential path to removal would be to allow DNSName (and similar classes) to take bytes as well as unicode strings:
class DNSName(object):
def __init__(self, value):
if isinstance(value, six.binary_type):
self._value = decode_for_compat(value)
self._bytes_value = value
elif isinstance(value, six.text_type):
self._value = value
self._bytes_value = encode_to_bytes(value)
else:
raise TypeError("value must be a unicode string or bytes")
value = utils.read_only_property("_value")
bytes_value = utils.read_only_property("_bytes_value")This approach preserves compatibility, but results in a confusing API that allows users on Python 2 to set values like "dömain" as bytes when they really wanted unicode. (Although we could potentially mitigate this by calling .encode("ascii") on str in Python 2.)
Once that's done we could deprecate unicode construction and at the same time start raising a UserWarning that value will be changing to match bytes_value in the future. Then once the deprecation time is complete we'd drop support for unicode and make value an alias to bytes_value and potentially deprecate bytes_value at that time. This is a major breaking change so I would be in favor of a year long deprecation cycle if we choose to pursue it.
Activity
Begin the deprecation of auto-idna for x509.DNSName
alex commentedon Jul 29, 2017
DNSNameRFC822NameUniformResourceIdentifiertiran commentedon Jul 30, 2017
I have been struggling with similar issues in Python's ssl module. The current implementation cannot verify and match IDN host names. It's also a bit of a mess, SNI callback and server_hostname are IDN U-labels, cert attributes are IDN A-labels. I have played with several approaches to fix the issue. So far only one approach is both simple enough to be memorable and not a potential source of security issues. It's also fully backwards compatible with ASCII-only host names.
User supplied input (hostname for TCP connection, server hostname) can be specified as either IDN U-label (str), IDN A-label (aka ACE, str) or ACE bytes. Internally the socket module and SSL module use ACE bytes only. Text (str) are converted to ACE bytes using IDNA. Since ACE str are just ASCII, IDNA encoding of ACE str is equivalent to encoding with ASCII encoding.
All output (SAN dNSName, SAN URI, CN, SNI callback, server_hostname attribute) are decoded as ACE strings. Since IDN is not a bijective mapping and also depends on the IDNA standard (2003, 2008, UTS46), this avoids some potential security issues. X.509 hostname verification and matching is defined on ACE, not IDN U-labels. I would rather keep them as bytes, but it wouldn't be backwards compatible. Also the aligns the SSL module with the socket module.
socket.getnameinfo()decodes with ASCII, not with IDNA.The new approach will make the SSL module compatible with the external idna package and IDNA 2008. Users just have to pass in ACE bytes as server_hostname.
alex commentedon Jul 30, 2017
That sounds like basically the same approach as we're doing here: bytes everywhere internally, if people want to do things with IDNA-encoded-unicode, they need to handle it themselves.
Begin the deprecation of auto-idna for x509.DNSName (#3830)
tiran commentedon Jul 30, 2017
I'm so glad you came to the same conclusion. I was unsure if my solution is sensible. Host names as ACE bytes avoid some pitfalls as well as multiple decoding and encoding steps for a low-level library such as cryptography.
Did anybody talk to @hynek and @Lukasa how they are going to handle encoding in PyOpenSSL, urllib3, service identity, and TLS PEP?
alex commentedon Aug 2, 2017
welp. this is done.
hynek commentedon Aug 2, 2017
FTR, service_identity does’t use cryptography’s x509 (yet?—Alex opened a ticket re this exact thing) and since it’s a user-facing thingie, it takes unicode. However it only does idna.encode, if the string contains characters >127.
socketpair commentedon May 28, 2018
PLEASE write in which version of Cryptography this was changed.
alex commentedon May 28, 2018
This is documented in our changelog: https://cryptography.io/en/latest/changelog/#v2-1