Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backwards incompatible change to DNSName #3951

Merged
merged 8 commits into from
Oct 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/x509/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ X.509 Certificate Builder
>>> builder = builder.public_key(public_key)
>>> builder = builder.add_extension(
... x509.SubjectAlternativeName(
... [x509.DNSName(b'cryptography.io')]
... [x509.DNSName(u'cryptography.io')]
... ),
... critical=False
... )
Expand Down
10 changes: 8 additions & 2 deletions src/cryptography/hazmat/backends/openssl/decode_asn1.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,14 @@ 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)
return x509.DNSName(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.dNSName).decode("utf8")
# We don't use the constructor for DNSName so we can bypass validation
# This allows us to create DNSName objects that have unicode chars
# when a certificate (against the RFC) contains them.
return x509.DNSName._init_without_validation(data)
elif gn.type == backend._lib.GEN_URI:
data = _asn1_string_to_bytes(backend, gn.d.uniformResourceIdentifier)
return x509.UniformResourceIdentifier(data)
Expand Down
4 changes: 3 additions & 1 deletion src/cryptography/hazmat/backends/openssl/encode_asn1.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,9 @@ def _encode_general_name(backend, name):

ia5 = backend._lib.ASN1_IA5STRING_new()
backend.openssl_assert(ia5 != backend._ffi.NULL)
value = 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.
value = name.value.encode("utf8")

res = backend._lib.ASN1_STRING_set(ia5, value, len(value))
backend.openssl_assert(res == 1)
Expand Down
67 changes: 19 additions & 48 deletions src/cryptography/x509/general_name.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,82 +131,53 @@ def _idna_encode(value):
for prefix in ['*.', '.']:
if value.startswith(prefix):
value = value[len(prefix):]
return prefix.encode('ascii') + idna.encode(value)
return idna.encode(value)
return prefix + idna.encode(value).decode("ascii")
return idna.encode(value).decode("ascii")


@utils.register_interface(GeneralName)
class DNSName(object):
def __init__(self, value):
if isinstance(value, six.text_type):
try:
value = value.encode("ascii")
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.",
"DNSName 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,
)
elif not isinstance(value, bytes):
raise TypeError("value must be bytes")
else:
raise TypeError("value must be string")

self._bytes_value = value
self._value = value

bytes_value = utils.read_only_property("_bytes_value")
value = utils.read_only_property("_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
@classmethod
def _init_without_validation(cls, value):
instance = cls.__new__(cls)
instance._value = value
return instance

def __repr__(self):
return "<DNSName(bytes_value={0!r})>".format(self.bytes_value)
return "<DNSName(value={0!r})>".format(self.value)

def __eq__(self, other):
if not isinstance(other, DNSName):
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)


@utils.register_interface(GeneralName)
Expand Down
55 changes: 38 additions & 17 deletions tests/x509/test_x509.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ def test_extensions(self, backend):
assert aia.value == x509.AuthorityInformationAccess([
x509.AccessDescription(
AuthorityInformationAccessOID.CA_ISSUERS,
x509.DNSName(b"cryptography.io")
x509.DNSName(u"cryptography.io")
)
])
assert ian.value == x509.IssuerAlternativeName([
Expand Down Expand Up @@ -777,6 +777,24 @@ def test_unicode_name(self, backend):
)
]

def test_non_ascii_dns_name(self, backend):
cert = _load_cert(
os.path.join("x509", "utf8-dnsname.pem"),
x509.load_pem_x509_certificate,
backend
)
san = cert.extensions.get_extension_for_class(
x509.SubjectAlternativeName
).value

names = san.get_values_for_type(x509.DNSName)

assert names == [
u'partner.biztositas.hu', u'biztositas.hu', u'*.biztositas.hu',
u'biztos\xedt\xe1s.hu', u'*.biztos\xedt\xe1s.hu',
u'xn--biztosts-fza2j.hu', u'*.xn--biztosts-fza2j.hu'
]

def test_all_subject_name_types(self, backend):
cert = _load_cert(
os.path.join(
Expand Down Expand Up @@ -1243,8 +1261,8 @@ def test_subject_alt_name(self, backend):
ExtensionOID.SUBJECT_ALTERNATIVE_NAME
)
assert list(ext.value) == [
x509.DNSName(b"cryptography.io"),
x509.DNSName(b"sub.cryptography.io"),
x509.DNSName(u"cryptography.io"),
x509.DNSName(u"sub.cryptography.io"),
]

def test_public_bytes_pem(self, backend):
Expand Down Expand Up @@ -1472,7 +1490,7 @@ def test_build_cert(self, backend):
).add_extension(
x509.BasicConstraints(ca=False, path_length=None), True,
).add_extension(
x509.SubjectAlternativeName([x509.DNSName(b"cryptography.io")]),
x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
critical=False,
).not_valid_before(
not_valid_before
Expand All @@ -1494,7 +1512,7 @@ def test_build_cert(self, backend):
ExtensionOID.SUBJECT_ALTERNATIVE_NAME
)
assert list(subject_alternative_name.value) == [
x509.DNSName(b"cryptography.io"),
x509.DNSName(u"cryptography.io"),
]

def test_build_cert_private_type_encoding(self, backend):
Expand Down Expand Up @@ -2122,7 +2140,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(b"cryptography.io")]),
x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
critical=False,
).not_valid_before(
not_valid_before
Expand All @@ -2144,7 +2162,7 @@ def test_build_cert_with_dsa_private_key(self, backend):
ExtensionOID.SUBJECT_ALTERNATIVE_NAME
)
assert list(subject_alternative_name.value) == [
x509.DNSName(b"cryptography.io"),
x509.DNSName(u"cryptography.io"),
]

@pytest.mark.requires_backend_interface(interface=EllipticCurveBackend)
Expand All @@ -2168,7 +2186,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(b"cryptography.io")]),
x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
critical=False,
).not_valid_before(
not_valid_before
Expand All @@ -2190,7 +2208,7 @@ def test_build_cert_with_ec_private_key(self, backend):
ExtensionOID.SUBJECT_ALTERNATIVE_NAME
)
assert list(subject_alternative_name.value) == [
x509.DNSName(b"cryptography.io"),
x509.DNSName(u"cryptography.io"),
]

@pytest.mark.requires_backend_interface(interface=RSABackend)
Expand Down Expand Up @@ -2224,6 +2242,9 @@ def test_build_cert_with_rsa_key_too_small(self, backend):
@pytest.mark.parametrize(
"add_ext",
[
x509.SubjectAlternativeName(
[x509.DNSName._init_without_validation(u'a\xedt\xe1s.test')]
),
x509.CertificatePolicies([
x509.PolicyInformation(
x509.ObjectIdentifier("2.16.840.1.12345.1.2.3.4.1"),
Expand Down Expand Up @@ -2279,7 +2300,7 @@ def test_build_cert_with_rsa_key_too_small(self, backend):
)
]),
x509.IssuerAlternativeName([
x509.DNSName(b"myissuer"),
x509.DNSName(u"myissuer"),
x509.RFC822Name(u"email@domain.com"),
]),
x509.ExtendedKeyUsage([
Expand Down Expand Up @@ -2308,7 +2329,7 @@ def test_build_cert_with_rsa_key_too_small(self, backend):
ipaddress.IPv6Network(u"FF:FF:0:0:0:0:0:0/128")
),
],
excluded_subtrees=[x509.DNSName(b"name.local")]
excluded_subtrees=[x509.DNSName(u"name.local")]
),
x509.NameConstraints(
permitted_subtrees=[
Expand All @@ -2318,7 +2339,7 @@ def test_build_cert_with_rsa_key_too_small(self, backend):
),
x509.NameConstraints(
permitted_subtrees=None,
excluded_subtrees=[x509.DNSName(b"name.local")]
excluded_subtrees=[x509.DNSName(u"name.local")]
),
x509.PolicyConstraints(
require_explicit_policy=None,
Expand Down Expand Up @@ -2847,7 +2868,7 @@ def test_add_unsupported_extension(self, backend):
x509.NameAttribute(NameOID.COUNTRY_NAME, u'US'),
])
).add_extension(
x509.SubjectAlternativeName([x509.DNSName(b"cryptography.io")]),
x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
critical=False,
).add_extension(
DummyExtension(), False
Expand Down Expand Up @@ -2933,7 +2954,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(b"cryptography.io")]),
x509.SubjectAlternativeName([x509.DNSName(u"cryptography.io")]),
critical=False,
).add_extension(
x509.BasicConstraints(ca=True, path_length=2), critical=True
Expand All @@ -2950,7 +2971,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(b"cryptography.io")]
assert list(ext.value) == [x509.DNSName(u"cryptography.io")]

def test_set_subject_twice(self):
builder = x509.CertificateSigningRequestBuilder()
Expand All @@ -2970,8 +2991,8 @@ def test_subject_alt_names(self, backend):
private_key = RSA_KEY_2048.private_key(backend)

san = x509.SubjectAlternativeName([
x509.DNSName(b"example.com"),
x509.DNSName(b"*.example.com"),
x509.DNSName(u"example.com"),
x509.DNSName(u"*.example.com"),
x509.RegisteredID(x509.ObjectIdentifier("1.2.3.4.5.6.7")),
x509.DirectoryName(x509.Name([
x509.NameAttribute(NameOID.COMMON_NAME, u'PyCA'),
Expand Down