Skip to content

Commit

Permalink
Strip trailing dot from FQDNs in Host and TLS context (#7601)
Browse files Browse the repository at this point in the history
Before this patch, the TLS verification fails with an exception if the
client uses a fully-qualified domain name with a trailing dot, like
https://github.com./ :
```console
aiohttp.client_exceptions.ClientConnectorCertificateError: Cannot connect to host github.com.:443 ssl:True
[SSLCertVerificationError: (1, "[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: Hostname mismatch,
certificate is not valid for 'github.com.'. (_ssl.c:1051)")]
```
The reason is that TLS certificates do not contain the trailing dot, as
per RFC 6066:

"HostName" contains the fully qualified DNS hostname of the server,
   as understood by the client.  The hostname is represented as a byte
   string using ASCII encoding without a trailing dot.

This change makes aiohttp strip the trailing dot for TLS context and
Host header, where trailing dots are not present.
For DNS resolution, we include the trailing dot as it signifies a
fully-qualified domain name (FQDN).
DNS lookups of FQDNs are faster as the resolver does not need to check
DNS search path, like for relative DNS names.

This effectively allows clients to connect to server if URL has dot at
the end of the hostname, e.g. `https://example.com./.

Fixes #3636
PR #7364

Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit d84fcf7)

<!-- Thank you for your contribution! -->

## What do these changes do?

Backport #7364 into 3.9

<!-- Please give a short brief about these changes. -->

## Are there changes in behavior for the user?

<!-- Outline any notable behaviour for the end users. -->

## Related issue number

<!-- Are there any issues opened that will be resolved by merging this
change? -->

## Checklist

- [ ] I think the code is well written
- [ ] Unit tests for the changes exist
- [ ] Documentation reflects the changes
- [ ] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [ ] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
  * ensure type is one of the following:
    * `.feature`: Signifying a new feature.
    * `.bugfix`: Signifying a bug fix.
    * `.doc`: Signifying a documentation improvement.
    * `.removal`: Signifying a deprecation or removal of public API.
* `.misc`: A ticket has been closed, but it is not of interest to users.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."
  • Loading branch information
martin-sucha and webknjaz authored Sep 30, 2023
1 parent dd8a06a commit c9e4d02
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGES/3636.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Implemented stripping the trailing dots from fully-qualified domain names in ``Host`` headers and TLS context when acting as an HTTP client.
This allows the client to connect to URLs with FQDN host name like ``https://example.com./``.
-- by :user:`martin-sucha`.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ Marko Kohtala
Martijn Pieters
Martin Melka
Martin Richard
Martin Sucha
Mathias Fröjdman
Mathieu Dugré
Matthias Marquardt
Expand Down
2 changes: 2 additions & 0 deletions aiohttp/client_reqrep.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,8 @@ def update_headers(self, headers: Optional[LooseHeaders]) -> None:
netloc = cast(str, self.url.raw_host)
if helpers.is_ipv6_address(netloc):
netloc = f"[{netloc}]"
# See https://github.com/aio-libs/aiohttp/issues/3636.
netloc = netloc.rstrip(".")
if self.url.port is not None and not self.url.is_default_port():
netloc += ":" + str(self.url.port)
self.headers[hdrs.HOST] = netloc
Expand Down
11 changes: 10 additions & 1 deletion aiohttp/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,11 @@ async def _create_direct_connection(

host = req.url.raw_host
assert host is not None
# Replace multiple trailing dots with a single one.
# A trailing dot is only present for fully-qualified domain names.
# See https://github.com/aio-libs/aiohttp/pull/7364.
if host.endswith(".."):
host = host.rstrip(".") + "."
port = req.port
assert port is not None
host_resolved = asyncio.ensure_future(
Expand Down Expand Up @@ -1183,8 +1188,12 @@ def drop_exception(fut: "asyncio.Future[List[Dict[str, Any]]]") -> None:
host = hinfo["host"]
port = hinfo["port"]

# Strip trailing dots, certificates contain FQDN without dots.
# See https://github.com/aio-libs/aiohttp/issues/3636
server_hostname = (
(req.server_hostname or hinfo["hostname"]) if sslcontext else None
(req.server_hostname or hinfo["hostname"]).rstrip(".")
if sslcontext
else None
)

try:
Expand Down
1 change: 1 addition & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def tls_certificate_authority():
def tls_certificate(tls_certificate_authority):
return tls_certificate_authority.issue_cert(
"localhost",
"xn--prklad-4va.localhost",
"127.0.0.1",
"::1",
)
Expand Down
39 changes: 38 additions & 1 deletion tests/test_client_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import urllib.parse
import zlib
from http.cookies import BaseCookie, Morsel, SimpleCookie
from typing import Any, Optional
from typing import Any, Dict, Optional
from unittest import mock

import pytest
Expand Down Expand Up @@ -283,6 +283,43 @@ def test_default_loop(loop) -> None:
loop.run_until_complete(req.close())


@pytest.mark.parametrize(
("url", "headers", "expected"),
(
pytest.param("http://localhost.", None, "localhost", id="dot only at the end"),
pytest.param("http://python.org.", None, "python.org", id="single dot"),
pytest.param(
"http://python.org.:99", None, "python.org:99", id="single dot with port"
),
pytest.param(
"http://python.org...:99",
None,
"python.org:99",
id="multiple dots with port",
),
pytest.param(
"http://python.org.:99",
{"host": "example.com.:99"},
"example.com.:99",
id="explicit host header",
),
pytest.param("https://python.org.", None, "python.org", id="https"),
pytest.param("https://...", None, "", id="only dots"),
pytest.param(
"http://príklad.example.org.:99",
None,
"xn--prklad-4va.example.org:99",
id="single dot with port idna",
),
),
)
def test_host_header_fqdn(
make_request: Any, url: str, headers: Dict[str, str], expected: str
) -> None:
req = make_request("get", url, headers=headers)
assert req.headers["HOST"] == expected


def test_default_headers_useragent(make_request) -> None:
req = make_request("get", "http://python.org/")

Expand Down
38 changes: 37 additions & 1 deletion tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -2045,10 +2045,23 @@ async def handler(request):
await session.close()


@pytest.mark.parametrize(
"host",
(
pytest.param("127.0.0.1", id="ip address"),
pytest.param("localhost", id="domain name"),
pytest.param("localhost.", id="fully-qualified domain name"),
pytest.param(
"localhost...", id="fully-qualified domain name with multiple trailing dots"
),
pytest.param("príklad.localhost.", id="idna fully-qualified domain name"),
),
)
async def test_tcp_connector_do_not_raise_connector_ssl_error(
aiohttp_server,
ssl_ctx,
client_ssl_ctx,
host,
) -> None:
async def handler(request):
return web.Response()
Expand All @@ -2060,10 +2073,33 @@ async def handler(request):
port = unused_port()
conn = aiohttp.TCPConnector(local_addr=("127.0.0.1", port))

# resolving something.localhost with the real DNS resolver does not work on macOS, so we have a stub.
async def _resolve_host(host, port, traces=None):
return [
{
"hostname": host,
"host": "127.0.0.1",
"port": port,
"family": socket.AF_INET,
"proto": 0,
"flags": socket.AI_NUMERICHOST,
},
{
"hostname": host,
"host": "::1",
"port": port,
"family": socket.AF_INET,
"proto": 0,
"flags": socket.AI_NUMERICHOST,
},
]

conn._resolve_host = _resolve_host

session = aiohttp.ClientSession(connector=conn)
url = srv.make_url("/")

r = await session.get(url, ssl=client_ssl_ctx)
r = await session.get(url.with_host(host), ssl=client_ssl_ctx)

r.release()
first_conn = next(iter(conn._conns.values()))[0][0]
Expand Down

0 comments on commit c9e4d02

Please sign in to comment.