Skip to content

Commit

Permalink
Always pass server_hostname to wrap_socket()
Browse files Browse the repository at this point in the history
Since Python 3.5, wrap_socket always allow a server_hostname to be
passed, even if OpenSSL does not have SNI. This is how our pyOpenSSL and
SecureTransport backends work too.
  • Loading branch information
pquentin committed Mar 10, 2021
1 parent e0d3b95 commit 11fecfd
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/urllib3/connectionpool.py
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ def _validate_conn(self, conn):
f"Unverified HTTPS request is being made to host '{conn.host}'. "
"Adding certificate verification is strongly advised. See: "
"https://urllib3.readthedocs.io/en/latest/advanced-usage.html"
"#ssl-warnings"
"#tls-warnings"
),
InsecureRequestWarning,
)
Expand Down
15 changes: 4 additions & 11 deletions src/urllib3/util/ssl_.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,12 @@ def ssl_wrap_socket(
"certificate, which can cause validation failures. You can upgrade to "
"a newer version of Python to solve this. For more information, see "
"https://urllib3.readthedocs.io/en/latest/advanced-usage.html"
"#ssl-warnings",
"#tls-warnings",
SNIMissingWarning,
)

if send_sni:
ssl_sock = _ssl_wrap_socket_impl(
sock, context, tls_in_tls, server_hostname=server_hostname
)
else:
ssl_sock = _ssl_wrap_socket_impl(sock, context, tls_in_tls)
server_hostname = server_hostname if send_sni else None
ssl_sock = _ssl_wrap_socket_impl(sock, context, tls_in_tls, server_hostname)
return ssl_sock


Expand Down Expand Up @@ -392,7 +388,4 @@ def _ssl_wrap_socket_impl(sock, ssl_context, tls_in_tls, server_hostname=None):
SSLTransport._validate_ssl_context_for_tls_in_tls(ssl_context)
return SSLTransport(sock, ssl_context, server_hostname)

if server_hostname:
return ssl_context.wrap_socket(sock, server_hostname=server_hostname)
else:
return ssl_context.wrap_socket(sock)
return ssl_context.wrap_socket(sock, server_hostname=server_hostname)
8 changes: 2 additions & 6 deletions test/test_ssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,8 @@ def test_context_sni_with_ip_address(

ssl_.ssl_wrap_socket(sock, server_hostname=server_hostname, ssl_context=context)

if uses_sni:
context.wrap_socket.assert_called_with(
sock, server_hostname=server_hostname
)
else:
context.wrap_socket.assert_called_with(sock)
expected_hostname = server_hostname if uses_sni else None
context.wrap_socket.assert_called_with(sock, server_hostname=expected_hostname)

@pytest.mark.parametrize(
["has_sni", "server_hostname", "should_warn"],
Expand Down
10 changes: 5 additions & 5 deletions test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -880,15 +880,15 @@ def test_ssl_wrap_socket_sni_ip_address_no_warn(self):
"""Test that a warning is not made if server_hostname is an IP address."""
sock = object()
context, warn = self._wrap_socket_and_mock_warn(sock, "8.8.8.8")
if util.IS_SECURETRANSPORT:
context.wrap_socket.assert_called_once_with(sock, server_hostname="8.8.8.8")
else:
context.wrap_socket.assert_called_once_with(sock)
expected_hostname = "8.8.8.8" if util.IS_SECURETRANSPORT else None
context.wrap_socket.assert_called_once_with(
sock, server_hostname=expected_hostname
)
warn.assert_not_called()

def test_ssl_wrap_socket_sni_none_no_warn(self):
"""Test that a warning is not made if server_hostname is not given."""
sock = object()
context, warn = self._wrap_socket_and_mock_warn(sock, None)
context.wrap_socket.assert_called_once_with(sock)
context.wrap_socket.assert_called_once_with(sock, server_hostname=None)
warn.assert_not_called()

0 comments on commit 11fecfd

Please sign in to comment.