bpo-37463: match_hostname requires quad-dotted IPv4

[tiran]

ssl.match_hostname() no longer accepts IPv4 addresses with additional text
after the address and only quad-dotted notation without trailing
whitespaces. Some inet_aton() implementations ignore whitespace and all data
after whitespace, e.g. '127.0.0.1 whatever'.

Short notations like '127.1' for '127.0.0.1' were already filtered out.

The bug was initially found by Dominik Czarnota and reported by Paul Kehrer.

Signed-off-by: Christian Heimes christian@python.org

https://bugs.python.org/issue37463

[zooba]

Any reason not to ipname.rstrip() here for the comparison? Does whitespace matter?

[zooba]

I just notice that you're explicitly testing trailing whitespace below.

[tiran]

rstrip() would not deal with invalid input like 127.0.0.1 whatever, 127.0.0.1\tinvalid, or other embedded white spaces.

[tiran]

I just notice that you're explicitly testing trailing whitespace below.

The rstrip call in _ipaddress_match is necessary because OpenSSL includes a trailing new line for IPv6 addresses. The code removes trailing white space in SAN IP Address items of decoded certs, not of the user supplied hostname argument.

[zooba]

FWIW, on Windows if you pass an invalid address you get OSError: illegal IP address string passed to inet_aton. Not sure if that's a Python error

[tiran]

Some implementations of inet_aton are strict and don't allow trailing data. glibc's inet_aton is more lean and ignores trailing data if the first character is some sort of white space.

[vstinner]

_ipaddress_match() is documented as "Exact matching of IP addresses." but is explicitly strips trailing spaces. Maybe just add a sentence to explain that in its docstring?

_inet_paton() disallow leading and trailing whitespaces: maybe explain that in its docstring?

_inet_paton() allows short form of IPv6 addresses, but not for IPv4 address. Is it a deliberate choice? Again, maybe document it in the docstring.

[tiran]

I don't think it is useful to add more doc strings to internal helper methods for a deprecated feature. ssl.match_hostname is deprecated and I plan to drop it in 3.9 or 3.10.

The strip in _ipaddress_match() is an implementation detail that works around a quirk in OpenSSL. The IP address may sometimes contain a newline. IIRC IPv6 addresses only.

Yes, that is deliberate. Short notation of IPv4 addresses is an uncommon and rarely used feature. However short notation of IPv6 is pretty much the default. The behavior also reflects the behavior of the old implementation with ipaddress module.

[vstinner]

nitpick: usually, Python error messages don't end with a dot. (Same comment for the 3 error messages.)

[tiran]

I'm following the style of the surrounding code. The exception messages are full sentences and end with a full stop.

[tiran]

I have added a few more comments to the code. I also changed one argument to make it more obvious what the function is doing.

@vstinner The other exceptions use the same style: full sentence with a full stop.

cpython/Lib/ssl.py

Lines 275 to 346 in 7cb9204

	def _dnsname_match(dn, hostname):
	"""Matching according to RFC 6125, section 6.4.3
	- Hostnames are compared lower case.
	- For IDNA, both dn and hostname must be encoded as IDN A-label (ACE).
	- Partial wildcards like 'www*.example.org', multiple wildcards, sole
	wildcard or wildcards in labels other then the left-most label are not
	supported and a CertificateError is raised.
	- A wildcard must match at least one character.
	"""
	if not dn:
	return False
	wildcards = dn.count('*')
	# speed up common case w/o wildcards
	if not wildcards:
	return dn.lower() == hostname.lower()
	if wildcards > 1:
	raise CertificateError(
	"too many wildcards in certificate DNS name: {!r}.".format(dn))
	dn_leftmost, sep, dn_remainder = dn.partition('.')
	if '*' in dn_remainder:
	# Only match wildcard in leftmost segment.
	raise CertificateError(
	"wildcard can only be present in the leftmost label: "
	"{!r}.".format(dn))
	if not sep:
	# no right side
	raise CertificateError(
	"sole wildcard without additional labels are not support: "
	"{!r}.".format(dn))
	if dn_leftmost != '*':
	# no partial wildcard matching
	raise CertificateError(
	"partial wildcards in leftmost label are not supported: "
	"{!r}.".format(dn))
	hostname_leftmost, sep, hostname_remainder = hostname.partition('.')
	if not hostname_leftmost or not sep:
	# wildcard must match at least one char
	return False
	return dn_remainder.lower() == hostname_remainder.lower()
	def _inet_paton(ipname):
	"""Try to convert an IP address to packed binary form
	Supports IPv4 addresses on all platforms and IPv6 on platforms with IPv6
	support.
	"""
	# inet_aton() also accepts strings like '1'
	if ipname.count('.') == 3:
	try:
	return _socket.inet_aton(ipname)
	except OSError:
	pass
	try:
	return _socket.inet_pton(_socket.AF_INET6, ipname)
	except OSError:
	raise ValueError("{!r} is neither an IPv4 nor an IP6 "
	"address.".format(ipname))
	except AttributeError:
	# AF_INET6 not available
	pass
	raise ValueError("{!r} is not an IPv4 address.".format(ipname))

[miss-islington]

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

[bedevere-bot]

GH-14559 is a backport of this pull request to the 3.8 branch.

[bedevere-bot]

GH-14560 is a backport of this pull request to the 3.7 branch.