fix(signing): idna-normalize hosts in etld+1 binding

src/adcp/signing/etld.py

@@ -20,18 +20,31 @@
 
 **Failure-closed convention.** Inputs whose eTLD+1 cannot be derived (raw
 IP addresses, single-label hosts like ``localhost``, hosts that are
-themselves public suffixes) yield ``None`` from :func:`registrable_domain`
-and ``False`` from :func:`same_registrable_domain`. Callers must treat
-None / False as a binding failure, not a soft skip.
+themselves public suffixes, hosts that are not IDNA-encodable — underscore
+labels, labels over 63 bytes, leading/trailing-hyphen labels) yield ``None``
+from :func:`registrable_domain` and ``False`` from
+:func:`same_registrable_domain`. Callers must treat None / False as a
+binding failure, not a soft skip.
+
+**Hosts are compared in canonical A-label form.** ``tldextract`` is
+IDNA-agnostic: hand it a U-label and it hands back a U-label. Comparing
+unencoded hosts made ``straße.de`` and ``xn--strae-oqa.de`` — one host,
+two spellings — never compare equal, refusing a legitimate agent whose
+brand.json and agent URL disagreed on spelling. :func:`host_from`
+therefore delegates to :func:`adcp.signing._idna_canonicalize.canonicalize_host`,
+the same normalizer the JWKS, revocation, and key-origin checks use.
 """
 
 from __future__ import annotations
 
 from functools import lru_cache
 from urllib.parse import urlsplit
 
+import idna
 import tldextract
 
+from ._idna_canonicalize import canonicalize_host
+
 
 @lru_cache(maxsize=1)
 def _extractor() -> tldextract.TLDExtract:
@@ -58,15 +71,27 @@ def _extractor() -> tldextract.TLDExtract:
 
 
 def host_from(value: str) -> str:
-    """Return the hostname portion of a URL, or pass a bare host through.
-
-    Normalizes case and trims a single trailing dot (the FQDN root
-    separator) so ``Example.COM.`` and ``example.com`` compare equal.
+    """Return the canonical hostname of a URL, or of a bare host.
+
+    Both branches delegate to
+    :func:`adcp.signing._idna_canonicalize.canonicalize_host`, which
+    strips a single trailing FQDN-root dot, ASCII-lowercases,
+    short-circuits IPv4/IPv6 literals (IDNA-2008 rejects purely-numeric
+    labels), and otherwise UTS-46 encodes with
+    ``idna.encode(uts46=True, transitional=False)``. So ``Example.COM.``
+    and ``example.com`` compare equal, and so do ``straße.de`` and
+    ``xn--strae-oqa.de``. Routing *both* branches through it is what
+    makes the URL form and the bare-host form of one host normalize
+    identically — previously only the bare-host branch trimmed the root
+    dot, and it trimmed every trailing dot rather than one.
 
     Raises :class:`ValueError` on input that is a URL with no parseable
-    host (``"http://"``) or empty after normalization. URL inputs MUST
-    use a scheme — a bare ``//example.com`` is treated as a bare host,
-    which is by design: bare-host inputs to this helper come from
+    host (``"http://"``), empty after normalization, or not encodable as
+    a hostname. The last case surfaces as ``idna.IDNAError``, which is a
+    :class:`ValueError` subclass (via ``UnicodeError``), so the raise
+    contract is unchanged for callers catching ``ValueError``. URL inputs
+    MUST use a scheme — a bare ``//example.com`` is treated as a bare
+    host, which is by design: bare-host inputs to this helper come from
     ``brand_url`` fields whose schema already constrains them, so a
     bare-host input is never an attacker-controlled URL.
     """
@@ -75,11 +100,13 @@ def host_from(value: str) -> str:
         host = parts.hostname
         if not host:
             raise ValueError(f"URL has no host: {value!r}")
-        return host.lower()
-    stripped = value.strip().rstrip(".").lower()
-    if not stripped:
-        raise ValueError("host is empty")
-    return stripped
+    else:
+        host = value.strip()
+        # Preserve the pre-existing message for ``""`` / ``"."`` / ``".."``;
+        # without this guard ``idna`` would raise "Empty domain" instead.
+        if not host.strip("."):
+            raise ValueError("host is empty")
+    return canonicalize_host(host)
 
 
 def registrable_domain(host_or_url: str) -> str | None:
@@ -92,14 +119,26 @@ def registrable_domain(host_or_url: str) -> str | None:
     * IP literals (v4 and v6) — IP addresses are not eTLD+1-bindable.
     * Single-label hosts (``localhost``, ``intranet``).
     * Hosts that are themselves a public suffix (``co.uk``).
-
-    The returned domain is lowercased.
+    * Hosts that are not IDNA-encodable (``under_score.brand.com``, a
+      label over 63 bytes, ``-lead.brand.com``). Such a string is not a
+      hostname, so it has no eTLD+1 to derive. Failing open here would
+      let ``under_score.brand.com`` reduce to ``brand.com`` and satisfy
+      the binding on a name the encoder rejects.
+
+    The returned domain is lowercased and in canonical A-label form:
+    ``straße.de`` returns ``"xn--strae-oqa.de"``. This is a change in
+    public return values for IDN inputs; both sides of
+    :func:`same_registrable_domain` move together, so the predicate
+    stays correct.
 
     Callers performing a binding check should treat ``None`` as a
     failure (the agent's host has no registrable domain to bind
     against), NOT as "no opinion".
     """
-    host = host_from(host_or_url)
+    try:
+        host = host_from(host_or_url)
+    except (idna.IDNAError, UnicodeError):
+        return None
     result = _extractor()(host)
     if not result.domain or not result.suffix:
         return None

tests/conformance/signing/test_etld.py

@@ -0,0 +1,112 @@
+"""IDNA conformance tests for :mod:`adcp.signing.etld`.
+
+Complements the behavioral suite in ``tests/test_etld.py`` (eTLD+1
+derivation, PSL private section, failure-closed IP/single-label cases)
+with the host-canonicalization contract the brand-authorization binding
+depends on.
+
+Behavior under test:
+
+* A host spelled as a U-label (``straße.de``) and the same host spelled
+  as an A-label (``xn--strae-oqa.de``) are ONE host and must bind.
+* :func:`registrable_domain` returns canonical A-label form.
+* The single-trailing-dot rule is symmetric across the URL branch and
+  the bare-host branch of :func:`host_from`.
+* Failure-closed convention extends to hosts that are not IDNA-encodable
+  (underscore labels, labels over 63 bytes, leading-hyphen labels).
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from adcp.signing.etld import host_from, registrable_domain, same_registrable_domain
+
+# ----- host_from -----
+
+
+def test_host_from_trailing_dot_symmetric_across_url_and_bare_forms() -> None:
+    # The documented single-trailing-dot rule applies to BOTH branches.
+    # The URL branch previously trimmed nothing and the bare-host branch
+    # trimmed every trailing dot, so the two spellings of one host
+    # normalized differently.
+    assert host_from("https://Example.COM./") == "example.com"
+    assert host_from("Example.COM.") == "example.com"
+    assert host_from("https://x.example.com..") == host_from("x.example.com..")
+
+
+def test_host_from_returns_a_label_for_idn() -> None:
+    # host_from is the single normalization point for the binding, so it
+    # owns UTS-46 / IDNA-2008 encoding, not just case folding.
+    assert host_from("https://shop.straße.de/") == "shop.xn--strae-oqa.de"
+    assert host_from("STRASSE.straße.DE") == "strasse.xn--strae-oqa.de"
+    assert host_from("xn--strae-oqa.de") == "xn--strae-oqa.de"
+
+
+def test_host_from_ip_literals_pass_through() -> None:
+    # IDNA-2008 rejects purely-numeric labels; IP literals must survive
+    # normalization unchanged so the downstream eTLD+1 lookup can fail
+    # them closed for the right reason.
+    assert host_from("192.0.2.1") == "192.0.2.1"
+    assert host_from("https://[2001:0db8::0001]/") == "2001:db8::1"
+
+
+def test_host_from_idna_invalid_host_raises_value_error() -> None:
+    # ``idna.IDNAError`` subclasses ``UnicodeError`` subclasses
+    # ``ValueError``, so the documented "raises ValueError" contract is
+    # unchanged and brand_authz's ``except ValueError`` around host_from
+    # still catches it.
+    with pytest.raises(ValueError):
+        host_from("under_score.brand.com")
+
+
+def test_host_from_empty_and_dot_only_still_raise() -> None:
+    # The pre-existing emptiness contract survives the delegation.
+    for value in ("", ".", "..", "   "):
+        with pytest.raises(ValueError):
+            host_from(value)
+
+
+# ----- registrable_domain -----
+
+
+def test_registrable_domain_returns_a_label_for_idn() -> None:
+    assert registrable_domain("straße.de") == "xn--strae-oqa.de"
+    assert registrable_domain("ADS.Straße.DE") == "xn--strae-oqa.de"
+    assert registrable_domain("xn--strae-oqa.de") == "xn--strae-oqa.de"
+
+
+def test_registrable_domain_idna_invalid_host_fails_closed() -> None:
+    # Not encodable as a hostname -> no derivable eTLD+1 -> None, the
+    # same failure-closed category as IP literals and single-label hosts.
+    # Failing open here would let ``under_score.brand.com`` reduce to
+    # ``brand.com`` and satisfy the binding on a string IDNA rejects.
+    assert registrable_domain("under_score.brand.com") is None
+    assert registrable_domain("a" * 64 + ".brand.com") is None
+    assert registrable_domain("-lead.brand.com") is None
+    assert same_registrable_domain("under_score.brand.com", "brand.com") is False
+
+
+def test_registrable_domain_ip_literals_still_none() -> None:
+    # Guard for the IP short-circuit inside the canonicalizer: routing
+    # host_from through IDNA must not turn these into raises.
+    assert registrable_domain("192.0.2.1") is None
+    assert registrable_domain("https://[2001:db8::1]/") is None
+
+
+# ----- same_registrable_domain -----
+
+
+def test_same_registrable_domain_idna_u_label_binds_to_a_label() -> None:
+    # A brand publishing brand.json under the U-label host and listing
+    # its agent under the A-label form (or the reverse) is one host and
+    # MUST bind. Uses a real delegated TLD (.de) — ``.example`` is RFC
+    # 2606 reserved, not in the PSL, and returns None for both spellings
+    # regardless, which would make this assertion vacuous.
+    assert same_registrable_domain("https://shop.straße.de/", "xn--strae-oqa.de") is True
+    assert same_registrable_domain("xn--strae-oqa.de", "https://shop.straße.de/") is True
+
+
+def test_same_registrable_domain_idn_cross_domain_still_false() -> None:
+    # Canonicalizing both sides must not collapse distinct IDN domains.
+    assert same_registrable_domain("straße.de", "xn--bcher-kva.de") is False