Skip to content

Commit 977322d

Browse files
fix(signing): idna-normalize hosts in etld+1 binding
tldextract is IDNA-agnostic: given a U-label it returns a U-label. Because host_from only case-folded, registrable_domain emitted 'straße.de' for the U-label spelling and 'xn--strae-oqa.de' for the A-label spelling of the same host, so same_registrable_domain compared two strings that can never be equal and returned False. That predicate is step 2a of the brand-authorization binding (brand_authz.py:255) and the redirect-containment gate (adagents.py:404). A brand whose brand.json and agent URL disagreed on spelling had its legitimate agent refused with reason="binding_failed"; the authorized_operators[] fallback compared the same mis-normalized values and missed too. Nothing was wrongly accepted, but a correct agent could not bind. Both branches of host_from now delegate to _idna_canonicalize.canonicalize_host — the same normalizer jwks, revocation_fetcher and key_origins already use. etld was the last signing module still on a bare .lower() after PR #777. That also fixes a second asymmetry the old docstring got wrong: the URL branch trimmed no trailing root dot while the bare-host branch trimmed all of them, so 'https://Example.COM./' and 'Example.COM.' normalized differently. registrable_domain maps idna.IDNAError onto None rather than letting it propagate or falling back to the raw string. Fail-closed is deliberate: - Propagating is not viable. IDNAError is a ValueError subclass, so it would be silently reclassified as brand_domain_invalid at one callsite and escape uncaught at five others mid-verification. - Failing open would loosen an existing check. adagents._idna_ascii_host is today the only gate rejecting an IDNA-invalid redirect target (_check_safe_host does not reject underscores). With a raw-string fallback, 'under_score.brand.com' would reduce to 'brand.com', match the origin, and the redirect would be accepted. - It widens a category the module already documents rather than inventing a contract: a string that is not an encodable hostname has no derivable eTLD+1, alongside IP literals and single-label hosts. The cost, stated plainly: hosts with an underscore label, a label over 63 bytes, or a leading/trailing-hyphen label now yield None instead of an eTLD+1. These are not valid hostnames per RFC 952/1123 and cannot appear in a fetchable https:// agent URL, but this is the one place an agent that binds today stops binding. Adopter-visible: registrable_domain and host_from are public exports and now return A-labels for IDN input; host_from additionally raises on IDNA-invalid input where it previously returned the string unchanged. Both sides of same_registrable_domain move together, so the predicate stays correct. adagents._idna_ascii_host is left in place — canonicalize_host is idempotent on the A-label output it produces, so it becomes a harmless double normalization. Removing that duplicate belongs in a follow-up that touches adagents. Fixes #988
1 parent 1ca7e07 commit 977322d

2 files changed

Lines changed: 169 additions & 18 deletions

File tree

src/adcp/signing/etld.py

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,31 @@
2020
2121
**Failure-closed convention.** Inputs whose eTLD+1 cannot be derived (raw
2222
IP addresses, single-label hosts like ``localhost``, hosts that are
23-
themselves public suffixes) yield ``None`` from :func:`registrable_domain`
24-
and ``False`` from :func:`same_registrable_domain`. Callers must treat
25-
None / False as a binding failure, not a soft skip.
23+
themselves public suffixes, hosts that are not IDNA-encodable — underscore
24+
labels, labels over 63 bytes, leading/trailing-hyphen labels) yield ``None``
25+
from :func:`registrable_domain` and ``False`` from
26+
:func:`same_registrable_domain`. Callers must treat None / False as a
27+
binding failure, not a soft skip.
28+
29+
**Hosts are compared in canonical A-label form.** ``tldextract`` is
30+
IDNA-agnostic: hand it a U-label and it hands back a U-label. Comparing
31+
unencoded hosts made ``straße.de`` and ``xn--strae-oqa.de`` — one host,
32+
two spellings — never compare equal, refusing a legitimate agent whose
33+
brand.json and agent URL disagreed on spelling. :func:`host_from`
34+
therefore delegates to :func:`adcp.signing._idna_canonicalize.canonicalize_host`,
35+
the same normalizer the JWKS, revocation, and key-origin checks use.
2636
"""
2737

2838
from __future__ import annotations
2939

3040
from functools import lru_cache
3141
from urllib.parse import urlsplit
3242

43+
import idna
3344
import tldextract
3445

46+
from ._idna_canonicalize import canonicalize_host
47+
3548

3649
@lru_cache(maxsize=1)
3750
def _extractor() -> tldextract.TLDExtract:
@@ -58,15 +71,27 @@ def _extractor() -> tldextract.TLDExtract:
5871

5972

6073
def host_from(value: str) -> str:
61-
"""Return the hostname portion of a URL, or pass a bare host through.
62-
63-
Normalizes case and trims a single trailing dot (the FQDN root
64-
separator) so ``Example.COM.`` and ``example.com`` compare equal.
74+
"""Return the canonical hostname of a URL, or of a bare host.
75+
76+
Both branches delegate to
77+
:func:`adcp.signing._idna_canonicalize.canonicalize_host`, which
78+
strips a single trailing FQDN-root dot, ASCII-lowercases,
79+
short-circuits IPv4/IPv6 literals (IDNA-2008 rejects purely-numeric
80+
labels), and otherwise UTS-46 encodes with
81+
``idna.encode(uts46=True, transitional=False)``. So ``Example.COM.``
82+
and ``example.com`` compare equal, and so do ``straße.de`` and
83+
``xn--strae-oqa.de``. Routing *both* branches through it is what
84+
makes the URL form and the bare-host form of one host normalize
85+
identically — previously only the bare-host branch trimmed the root
86+
dot, and it trimmed every trailing dot rather than one.
6587
6688
Raises :class:`ValueError` on input that is a URL with no parseable
67-
host (``"http://"``) or empty after normalization. URL inputs MUST
68-
use a scheme — a bare ``//example.com`` is treated as a bare host,
69-
which is by design: bare-host inputs to this helper come from
89+
host (``"http://"``), empty after normalization, or not encodable as
90+
a hostname. The last case surfaces as ``idna.IDNAError``, which is a
91+
:class:`ValueError` subclass (via ``UnicodeError``), so the raise
92+
contract is unchanged for callers catching ``ValueError``. URL inputs
93+
MUST use a scheme — a bare ``//example.com`` is treated as a bare
94+
host, which is by design: bare-host inputs to this helper come from
7095
``brand_url`` fields whose schema already constrains them, so a
7196
bare-host input is never an attacker-controlled URL.
7297
"""
@@ -75,11 +100,13 @@ def host_from(value: str) -> str:
75100
host = parts.hostname
76101
if not host:
77102
raise ValueError(f"URL has no host: {value!r}")
78-
return host.lower()
79-
stripped = value.strip().rstrip(".").lower()
80-
if not stripped:
81-
raise ValueError("host is empty")
82-
return stripped
103+
else:
104+
host = value.strip()
105+
# Preserve the pre-existing message for ``""`` / ``"."`` / ``".."``;
106+
# without this guard ``idna`` would raise "Empty domain" instead.
107+
if not host.strip("."):
108+
raise ValueError("host is empty")
109+
return canonicalize_host(host)
83110

84111

85112
def registrable_domain(host_or_url: str) -> str | None:
@@ -92,14 +119,26 @@ def registrable_domain(host_or_url: str) -> str | None:
92119
* IP literals (v4 and v6) — IP addresses are not eTLD+1-bindable.
93120
* Single-label hosts (``localhost``, ``intranet``).
94121
* Hosts that are themselves a public suffix (``co.uk``).
95-
96-
The returned domain is lowercased.
122+
* Hosts that are not IDNA-encodable (``under_score.brand.com``, a
123+
label over 63 bytes, ``-lead.brand.com``). Such a string is not a
124+
hostname, so it has no eTLD+1 to derive. Failing open here would
125+
let ``under_score.brand.com`` reduce to ``brand.com`` and satisfy
126+
the binding on a name the encoder rejects.
127+
128+
The returned domain is lowercased and in canonical A-label form:
129+
``straße.de`` returns ``"xn--strae-oqa.de"``. This is a change in
130+
public return values for IDN inputs; both sides of
131+
:func:`same_registrable_domain` move together, so the predicate
132+
stays correct.
97133
98134
Callers performing a binding check should treat ``None`` as a
99135
failure (the agent's host has no registrable domain to bind
100136
against), NOT as "no opinion".
101137
"""
102-
host = host_from(host_or_url)
138+
try:
139+
host = host_from(host_or_url)
140+
except (idna.IDNAError, UnicodeError):
141+
return None
103142
result = _extractor()(host)
104143
if not result.domain or not result.suffix:
105144
return None
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
"""IDNA conformance tests for :mod:`adcp.signing.etld`.
2+
3+
Complements the behavioral suite in ``tests/test_etld.py`` (eTLD+1
4+
derivation, PSL private section, failure-closed IP/single-label cases)
5+
with the host-canonicalization contract the brand-authorization binding
6+
depends on.
7+
8+
Behavior under test:
9+
10+
* A host spelled as a U-label (``straße.de``) and the same host spelled
11+
as an A-label (``xn--strae-oqa.de``) are ONE host and must bind.
12+
* :func:`registrable_domain` returns canonical A-label form.
13+
* The single-trailing-dot rule is symmetric across the URL branch and
14+
the bare-host branch of :func:`host_from`.
15+
* Failure-closed convention extends to hosts that are not IDNA-encodable
16+
(underscore labels, labels over 63 bytes, leading-hyphen labels).
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import pytest
22+
23+
from adcp.signing.etld import host_from, registrable_domain, same_registrable_domain
24+
25+
# ----- host_from -----
26+
27+
28+
def test_host_from_trailing_dot_symmetric_across_url_and_bare_forms() -> None:
29+
# The documented single-trailing-dot rule applies to BOTH branches.
30+
# The URL branch previously trimmed nothing and the bare-host branch
31+
# trimmed every trailing dot, so the two spellings of one host
32+
# normalized differently.
33+
assert host_from("https://Example.COM./") == "example.com"
34+
assert host_from("Example.COM.") == "example.com"
35+
assert host_from("https://x.example.com..") == host_from("x.example.com..")
36+
37+
38+
def test_host_from_returns_a_label_for_idn() -> None:
39+
# host_from is the single normalization point for the binding, so it
40+
# owns UTS-46 / IDNA-2008 encoding, not just case folding.
41+
assert host_from("https://shop.straße.de/") == "shop.xn--strae-oqa.de"
42+
assert host_from("STRASSE.straße.DE") == "strasse.xn--strae-oqa.de"
43+
assert host_from("xn--strae-oqa.de") == "xn--strae-oqa.de"
44+
45+
46+
def test_host_from_ip_literals_pass_through() -> None:
47+
# IDNA-2008 rejects purely-numeric labels; IP literals must survive
48+
# normalization unchanged so the downstream eTLD+1 lookup can fail
49+
# them closed for the right reason.
50+
assert host_from("192.0.2.1") == "192.0.2.1"
51+
assert host_from("https://[2001:0db8::0001]/") == "2001:db8::1"
52+
53+
54+
def test_host_from_idna_invalid_host_raises_value_error() -> None:
55+
# ``idna.IDNAError`` subclasses ``UnicodeError`` subclasses
56+
# ``ValueError``, so the documented "raises ValueError" contract is
57+
# unchanged and brand_authz's ``except ValueError`` around host_from
58+
# still catches it.
59+
with pytest.raises(ValueError):
60+
host_from("under_score.brand.com")
61+
62+
63+
def test_host_from_empty_and_dot_only_still_raise() -> None:
64+
# The pre-existing emptiness contract survives the delegation.
65+
for value in ("", ".", "..", " "):
66+
with pytest.raises(ValueError):
67+
host_from(value)
68+
69+
70+
# ----- registrable_domain -----
71+
72+
73+
def test_registrable_domain_returns_a_label_for_idn() -> None:
74+
assert registrable_domain("straße.de") == "xn--strae-oqa.de"
75+
assert registrable_domain("ADS.Straße.DE") == "xn--strae-oqa.de"
76+
assert registrable_domain("xn--strae-oqa.de") == "xn--strae-oqa.de"
77+
78+
79+
def test_registrable_domain_idna_invalid_host_fails_closed() -> None:
80+
# Not encodable as a hostname -> no derivable eTLD+1 -> None, the
81+
# same failure-closed category as IP literals and single-label hosts.
82+
# Failing open here would let ``under_score.brand.com`` reduce to
83+
# ``brand.com`` and satisfy the binding on a string IDNA rejects.
84+
assert registrable_domain("under_score.brand.com") is None
85+
assert registrable_domain("a" * 64 + ".brand.com") is None
86+
assert registrable_domain("-lead.brand.com") is None
87+
assert same_registrable_domain("under_score.brand.com", "brand.com") is False
88+
89+
90+
def test_registrable_domain_ip_literals_still_none() -> None:
91+
# Guard for the IP short-circuit inside the canonicalizer: routing
92+
# host_from through IDNA must not turn these into raises.
93+
assert registrable_domain("192.0.2.1") is None
94+
assert registrable_domain("https://[2001:db8::1]/") is None
95+
96+
97+
# ----- same_registrable_domain -----
98+
99+
100+
def test_same_registrable_domain_idna_u_label_binds_to_a_label() -> None:
101+
# A brand publishing brand.json under the U-label host and listing
102+
# its agent under the A-label form (or the reverse) is one host and
103+
# MUST bind. Uses a real delegated TLD (.de) — ``.example`` is RFC
104+
# 2606 reserved, not in the PSL, and returns None for both spellings
105+
# regardless, which would make this assertion vacuous.
106+
assert same_registrable_domain("https://shop.straße.de/", "xn--strae-oqa.de") is True
107+
assert same_registrable_domain("xn--strae-oqa.de", "https://shop.straße.de/") is True
108+
109+
110+
def test_same_registrable_domain_idn_cross_domain_still_false() -> None:
111+
# Canonicalizing both sides must not collapse distinct IDN domains.
112+
assert same_registrable_domain("straße.de", "xn--bcher-kva.de") is False

0 commit comments

Comments
 (0)