backwards incompatible change to RFC822Name (#3953)
* backwards incompatible change to RFC822Name
During this release cycle we decided to officially deprecate passing
U-labels to our GeneralName constructors. At first we tried changing
this in a purely backwards compatible way but get_values_for_type made
that untenable. This PR modifies RFC822Name to accept two types:
U-label strings (which raises a deprecation warning) and A-label strings
(the new preferred type). There is also a constructor for RFC822Name
that bypasses validation so we can parse garbage out of certificates
(and round trip it if necessary)
* whoops
diff --git a/src/cryptography/hazmat/backends/openssl/decode_asn1.py b/src/cryptography/hazmat/backends/openssl/decode_asn1.py
index aefb242..86f8f8d 100644
--- a/src/cryptography/hazmat/backends/openssl/decode_asn1.py
+++ b/src/cryptography/hazmat/backends/openssl/decode_asn1.py
@@ -134,8 +134,14 @@
_decode_x509_name(backend, gn.d.directoryName)
)
elif gn.type == backend._lib.GEN_EMAIL:
- data = _asn1_string_to_bytes(backend, gn.d.rfc822Name)
- return x509.RFC822Name(data)
+ # Convert to bytes and then decode to utf8. We don't use
+ # asn1_string_to_utf8 here because it doesn't properly convert
+ # utf8 from ia5strings.
+ data = _asn1_string_to_bytes(backend, gn.d.rfc822Name).decode("utf8")
+ # We don't use the constructor for RFC822Name so we can bypass
+ # validation. This allows us to create RFC822Name objects that have
+ # unicode chars when a certificate (against the RFC) contains them.
+ return x509.RFC822Name._init_without_validation(data)
elif gn.type == backend._lib.GEN_OTHERNAME:
type_id = _obj2txt(backend, gn.d.otherName.type_id)
value = _asn1_to_der(backend, gn.d.otherName.value)
diff --git a/src/cryptography/hazmat/backends/openssl/encode_asn1.py b/src/cryptography/hazmat/backends/openssl/encode_asn1.py
index 3177cf9..3f0a4c8 100644
--- a/src/cryptography/hazmat/backends/openssl/encode_asn1.py
+++ b/src/cryptography/hazmat/backends/openssl/encode_asn1.py
@@ -434,9 +434,10 @@
elif isinstance(name, x509.RFC822Name):
gn = backend._lib.GENERAL_NAME_new()
backend.openssl_assert(gn != backend._ffi.NULL)
- asn1_str = _encode_asn1_str(
- backend, name.bytes_value, len(name.bytes_value)
- )
+ # ia5strings are supposed to be ITU T.50 but to allow round-tripping
+ # of broken certs that encode utf8 we'll encode utf8 here too.
+ data = name.value.encode("utf8")
+ asn1_str = _encode_asn1_str(backend, data, len(data))
gn.type = backend._lib.GEN_EMAIL
gn.d.rfc822Name = asn1_str
elif isinstance(name, x509.UniformResourceIdentifier):
diff --git a/src/cryptography/x509/general_name.py b/src/cryptography/x509/general_name.py
index d4d92c8..6f7fa7a 100644
--- a/src/cryptography/x509/general_name.py
+++ b/src/cryptography/x509/general_name.py
@@ -53,77 +53,55 @@
def __init__(self, value):
if isinstance(value, six.text_type):
try:
- value = value.encode("ascii")
+ value.encode("ascii")
except UnicodeEncodeError:
value = self._idna_encode(value)
warnings.warn(
- "RFC822Name values should be passed as bytes, not strings."
- " Support for passing unicode strings will be removed in a"
- " future version.",
+ "RFC822Name values should be passed as an A-label string. "
+ "This means unicode characters should be encoded via "
+ "idna. Support for passing unicode strings (aka U-label) "
+ " will be removed in a future version.",
utils.DeprecatedIn21,
stacklevel=2,
)
- else:
- warnings.warn(
- "RFC822Name 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")
+ else:
+ raise TypeError("value must be string")
- name, address = parseaddr(value.decode("ascii"))
+ name, address = parseaddr(value)
if name or not address:
# parseaddr has found a name (e.g. Name <email>) or the entire
# value is an empty string.
raise ValueError("Invalid rfc822name value")
- self._bytes_value = value
+ self._value = value
- bytes_value = utils.read_only_property("_bytes_value")
+ value = utils.read_only_property("_value")
+
+ @classmethod
+ def _init_without_validation(cls, value):
+ instance = cls.__new__(cls)
+ instance._value = value
+ return instance
def _idna_encode(self, value):
_, address = parseaddr(value)
parts = address.split(u"@")
- return parts[0].encode("ascii") + b"@" + idna.encode(parts[1])
-
- @property
- def value(self):
- warnings.warn(
- "RFC822Name.bytes_value should be used instead of RFC822Name.value"
- "; it contains the name as raw bytes, instead of as an idna-"
- "decoded unicode string. RFC822Name.value will be removed in a "
- "future version.",
- utils.DeprecatedIn21,
- stacklevel=2
- )
- _, address = parseaddr(self.bytes_value.decode("ascii"))
- parts = address.split(u"@")
- if len(parts) == 1:
- # Single label email name. This is valid for local delivery.
- # No IDNA decoding needed since there is no domain component.
- return address
- else:
- # A normal email of the form user@domain.com. Let's attempt to
- # encode the domain component and reconstruct the address.
- return parts[0] + u"@" + idna.decode(parts[1])
+ return parts[0] + "@" + idna.encode(parts[1]).decode("ascii")
def __repr__(self):
- return "<RFC822Name(bytes_value={0!r})>".format(self.bytes_value)
+ return "<RFC822Name(value={0!r})>".format(self.value)
def __eq__(self, other):
if not isinstance(other, RFC822Name):
return NotImplemented
- return self.bytes_value == other.bytes_value
+ return self.value == other.value
def __ne__(self, other):
return not self == other
def __hash__(self):
- return hash(self.bytes_value)
+ return hash(self.value)
def _idna_encode(value):
diff --git a/tests/x509/test_x509.py b/tests/x509/test_x509.py
index afe1c0e..d6aafcd 100644
--- a/tests/x509/test_x509.py
+++ b/tests/x509/test_x509.py
@@ -2243,7 +2243,14 @@
"add_ext",
[
x509.SubjectAlternativeName(
- [x509.DNSName._init_without_validation(u'a\xedt\xe1s.test')]
+ [
+ # These examples exist to verify compatibility with
+ # certificates that have utf8 encoded data in the ia5string
+ x509.DNSName._init_without_validation(u'a\xedt\xe1s.test'),
+ x509.RFC822Name._init_without_validation(
+ u'test@a\xedt\xe1s.test'
+ ),
+ ]
),
x509.CertificatePolicies([
x509.PolicyInformation(
diff --git a/tests/x509/test_x509_ext.py b/tests/x509/test_x509_ext.py
index 6e376bb..5ad2835 100644
--- a/tests/x509/test_x509_ext.py
+++ b/tests/x509/test_x509_ext.py
@@ -248,7 +248,7 @@
x509.DNSName(u"cryptography.io"),
x509.DNSName(u"crypto.local"),
x509.DNSName(u"another.local"),
- x509.RFC822Name(b"email@another.local"),
+ x509.RFC822Name(u"email@another.local"),
x509.UniformResourceIdentifier(b"http://another.local"),
])
assert ci[-1] == ci[4]
@@ -1755,53 +1755,51 @@
class TestRFC822Name(object):
def test_repr(self):
- gn = x509.RFC822Name(b"string")
+ gn = x509.RFC822Name(u"string")
if six.PY3:
- assert repr(gn) == "<RFC822Name(bytes_value=b'string')>"
+ assert repr(gn) == "<RFC822Name(value='string')>"
else:
- assert repr(gn) == "<RFC822Name(bytes_value='string')>"
+ assert repr(gn) == "<RFC822Name(value=u'string')>"
def test_equality(self):
- gn = x509.RFC822Name(b"string")
- gn2 = x509.RFC822Name(b"string2")
- gn3 = x509.RFC822Name(b"string")
+ gn = x509.RFC822Name(u"string")
+ gn2 = x509.RFC822Name(u"string2")
+ gn3 = x509.RFC822Name(u"string")
assert gn != gn2
assert gn != object()
assert gn == gn3
- def test_not_text_or_bytes(self):
+ def test_not_text(self):
with pytest.raises(TypeError):
x509.RFC822Name(1.3)
+ with pytest.raises(TypeError):
+ x509.RFC822Name(b"bytes")
+
def test_invalid_email(self):
with pytest.raises(ValueError):
x509.RFC822Name(u"Name <email>")
- with pytest.raises(ValueError):
- x509.RFC822Name(b"Name <email>")
with pytest.raises(ValueError):
- x509.RFC822Name(b"")
+ x509.RFC822Name(u"")
def test_single_label(self):
- gn = x509.RFC822Name(b"administrator")
- with pytest.warns(utils.DeprecatedIn21):
- assert gn.value == u"administrator"
-
- assert gn.bytes_value == b"administrator"
+ gn = x509.RFC822Name(u"administrator")
+ assert gn.value == u"administrator"
def test_idna(self):
with pytest.warns(utils.DeprecatedIn21):
gn = x509.RFC822Name(u"email@em\xe5\xefl.com")
- with pytest.warns(utils.DeprecatedIn21):
- assert gn.value == u"email@em\xe5\xefl.com"
+ assert gn.value == u"email@xn--eml-vla4c.com"
- assert gn.bytes_value == b"email@xn--eml-vla4c.com"
+ gn2 = x509.RFC822Name(u"email@xn--eml-vla4c.com")
+ assert gn2.value == u"email@xn--eml-vla4c.com"
def test_hash(self):
- g1 = x509.RFC822Name(b"email@host.com")
- g2 = x509.RFC822Name(b"email@host.com")
- g3 = x509.RFC822Name(b"admin@host.com")
+ g1 = x509.RFC822Name(u"email@host.com")
+ g2 = x509.RFC822Name(u"email@host.com")
+ g3 = x509.RFC822Name(u"admin@host.com")
assert hash(g1) == hash(g2)
assert hash(g1) != hash(g3)
@@ -2046,7 +2044,7 @@
x509.DNSName(u"cryptography.io"),
x509.DNSName(u"crypto.local"),
x509.DNSName(u"another.local"),
- x509.RFC822Name(b"email@another.local"),
+ x509.RFC822Name(u"email@another.local"),
x509.UniformResourceIdentifier(b"http://another.local"),
])
assert gn[-1] == gn[4]
@@ -2087,7 +2085,7 @@
[x509.DNSName(u"cryptography.io")]
)
gns2 = x509.GeneralNames(
- [x509.RFC822Name(b"admin@cryptography.io")]
+ [x509.RFC822Name(u"admin@cryptography.io")]
)
assert gns != gns2
assert gns != object()
@@ -2095,7 +2093,7 @@
def test_hash(self):
gns = x509.GeneralNames([x509.DNSName(u"cryptography.io")])
gns2 = x509.GeneralNames([x509.DNSName(u"cryptography.io")])
- gns3 = x509.GeneralNames([x509.RFC822Name(b"admin@cryptography.io")])
+ gns3 = x509.GeneralNames([x509.RFC822Name(u"admin@cryptography.io")])
assert hash(gns) == hash(gns2)
assert hash(gns) != hash(gns3)
@@ -2124,7 +2122,7 @@
x509.DNSName(u"cryptography.io"),
x509.DNSName(u"crypto.local"),
x509.DNSName(u"another.local"),
- x509.RFC822Name(b"email@another.local"),
+ x509.RFC822Name(u"email@another.local"),
x509.UniformResourceIdentifier(b"http://another.local"),
])
assert ian[-1] == ian[4]
@@ -2167,7 +2165,7 @@
[x509.DNSName(u"cryptography.io")]
)
san2 = x509.IssuerAlternativeName(
- [x509.RFC822Name(b"admin@cryptography.io")]
+ [x509.RFC822Name(u"admin@cryptography.io")]
)
assert san != san2
assert san != object()
@@ -2176,7 +2174,7 @@
ian = x509.IssuerAlternativeName([x509.DNSName(u"cryptography.io")])
ian2 = x509.IssuerAlternativeName([x509.DNSName(u"cryptography.io")])
ian3 = x509.IssuerAlternativeName(
- [x509.RFC822Name(b"admin@cryptography.io")]
+ [x509.RFC822Name(u"admin@cryptography.io")]
)
assert hash(ian) == hash(ian2)
assert hash(ian) != hash(ian3)
@@ -2249,7 +2247,7 @@
x509.DNSName(u"cryptography.io"),
x509.DNSName(u"crypto.local"),
x509.DNSName(u"another.local"),
- x509.RFC822Name(b"email@another.local"),
+ x509.RFC822Name(u"email@another.local"),
x509.UniformResourceIdentifier(b"http://another.local"),
])
assert san[-1] == san[4]
@@ -2292,7 +2290,7 @@
[x509.DNSName(u"cryptography.io")]
)
san2 = x509.SubjectAlternativeName(
- [x509.RFC822Name(b"admin@cryptography.io")]
+ [x509.RFC822Name(u"admin@cryptography.io")]
)
assert san != san2
assert san != object()
@@ -2301,7 +2299,7 @@
san = x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")])
san2 = x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")])
san3 = x509.SubjectAlternativeName(
- [x509.RFC822Name(b"admin@cryptography.io")]
+ [x509.RFC822Name(u"admin@cryptography.io")]
)
assert hash(san) == hash(san2)
assert hash(san) != hash(san3)
@@ -2487,7 +2485,7 @@
san = ext.value
rfc822name = san.get_values_for_type(x509.RFC822Name)
- assert [u"email@em\xe5\xefl.com"] == rfc822name
+ assert [u"email@xn--eml-vla4c.com"] == rfc822name
def test_idna2003_invalid(self, backend):
cert = _load_cert(
@@ -2520,7 +2518,7 @@
rfc822_name = ext.value.get_values_for_type(x509.RFC822Name)
dns_name = ext.value.get_values_for_type(x509.DNSName)
uri = ext.value.get_values_for_type(x509.UniformResourceIdentifier)
- assert rfc822_name == [u"email@\u043f\u044b\u043a\u0430.cryptography"]
+ assert rfc822_name == [u"email@xn--80ato2c.cryptography"]
assert dns_name == [u"xn--80ato2c.cryptography"]
assert uri == [u"https://www.\u043f\u044b\u043a\u0430.cryptography"]
@@ -2569,10 +2567,14 @@
x509.load_pem_x509_certificate,
backend
)
- with pytest.raises(ValueError) as exc:
- cert.extensions
-
- assert 'Invalid rfc822name value' in str(exc.value)
+ san = cert.extensions.get_extension_for_class(
+ x509.SubjectAlternativeName
+ ).value
+ values = san.get_values_for_type(x509.RFC822Name)
+ assert values == [
+ u'email', u'email <email>', u'email <email@email>',
+ u'email <email@xn--eml-vla4c.com>', u'myemail:'
+ ]
def test_other_name(self, backend):
cert = _load_cert(