Skip to content

Refresh public suffix list to 2026-10-20 (#333) - #370

Closed
alexanderdibenedetto wants to merge 3 commits into
v4-0-0from
adb/issue-333-invalid-domain-names
Closed

alexanderdibenedetto wants to merge 3 commits into
v4-0-0from
adb/issue-333-invalid-domain-names

Conversation

@alexanderdibenedetto

Copy link
Copy Markdown
Contributor

The 2018 table rejected 17 delegated TLDs, including .music, .kids, .web and .ss.

Alexander DiBenedetto added 3 commits October 22, 2026 01:35
The 2018 table rejected 17 delegated TLDs, including .music, .kids, .web and .ss.
Vendors a Python 3 port of the domain-registry-provider generator; the upstream copy is retired and Python 2 only.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Security-sensitive domain classification changes need human confirmation of generated-table validation and Apple-platform regression results.

No open issues 🎉

What changed in this PR

Adds tooling and regression tests to support refreshing TrustKit’s public suffix data, used to validate domains in pinning configurations.

Changes:

  • Adds a Python 3 table generator and an update script that validates generated tables before replacing them.
  • Adds configuration tests for recent TLDs and rejection of unknown suffixes, and documents the refresh command.
  • Temporarily removes watchOS simulator CI coverage pending a runner-image fix.
File Description
TrustKitTests/​TSKPinConfigurationTests.m Tests acceptance of recent TLDs and rejection of .invalid.
TrustKit/​Dependencies/​README.md Documents the suffix-list refresh command.
scripts/​update_public_suffix_list.sh Downloads, generates, validates, and installs registry tables.
scripts/​registry_tables_generator/​trie_node.py Adds trie structures for suffix generation.
scripts/​registry_tables_generator/​test_table_builder.py Generates rule-based verification cases.
scripts/​registry_tables_generator/​table_serializer.py Serializes C tables with offset bounds checks.
scripts/​registry_tables_generator/​string_table_builder.py Builds compressed hostname string tables.
scripts/​registry_tables_generator/​registry_tables_generator.py Coordinates Python 3 registry-table generation.
scripts/​registry_tables_generator/​node_table_builder.py Builds compact node and leaf tables.
.github/​workflows/​ci.yml Temporarily excludes the watchOS simulator job.

Review effort: Balanced

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟡 scripts/registry_tables_generator/registry_tables_generator.py — Apps pinning a parent domain with includeSubdomains may silently stop pinning hosts under a newly listed PRIVATE-section suffix, with no error after this refresh. _ReadRulesFromFile keeps every rule, including the PRIVATE DOMAINS section, which has grown a lot since 2018. isSubdomain in TrustKit/configuration_utils.m:30 returns 0 whenever the two registry lengths differ. Fix: decide whether pinning should follow PRIVATE entries and filter rules to the ICANN section if not; otherwise document the fail-open change and test a parent pin over a private suffix.

    Why this was flagged

    This depends on app configs that pin a domain with kTSKIncludeSubdomains where some name under it is now a PRIVATE-section suffix. I cannot list those configs from here, but hosting and CDN suffixes under amazonaws, cloudfront, appspot and similar grew from 134 to 581 matches in registry_tables.h. Example: "example.com" is pinned and "apps.example.com" is now on the list. For host "x.apps.example.com", GetRegistryLength returns len("apps.example.com") while the pinned domain gets len("com"). The check at configuration_utils.m:30-33 then returns 0. getPinningConfigurationKeyForDomain returns nil, and the connection runs without pin validation or reports. On the base table, the same host matched the parent pin. The same table change also makes parse_configuration.m:120 raise "includeSubdomains for a domain suffix" during initialization for any pinned domain that is now itself a suffix. Nothing warns the integrator about either effect.

    Verification: Trigger: an app pins a parent domain with kTSKIncludeSubdomains, and a name between that domain and the host was added to the PSL PRIVATE section after 2018. In TrustKit/configuration_utils.m:28-32, isSubdomain does if (subdomainRegistryLength != domainRegistryLength) return 0;, so the host is silently left unpinned. Using PRIVATE entries is existing design, not new.

  • 🟡 TrustKit/Dependencies/domain_registry/registry_tables_genfiles/registry_tables.h:11281 — Apps whose pin config still lists a host under a retired TLD will throw at TrustKit init after this update, where the base accepted them. The new table drops about 107 TLDs (kNumRootChildren 1555 to 1448), so GetRegistryLength returns 0 and parse_configuration.m:72 raises. Fix: make the break visible to integrators, for example a release note naming the dropped TLDs. Or skip unknown-TLD entries with a log, keeping the throw that testUnknownTldIsRejected expects for a config with no valid domains. Confirmed dropped: abarth, alfaromeo, chrysler, dodge, fiat, lancia, maserati, srt, avianca, afamilycompany.

    Why this was flagged

    This only hits apps whose kTSKPinnedDomains still holds a domain under a TLD removed from the root since the 2018 list. I cannot list those configs from here. The base header had root entries for abarth, afamilycompany, alfaromeo, avianca, chrysler, dodge, fiat, lancia, maserati and srt; the new header has none of them, and kNumRootChildren drops from 1555 to 1448 at registry_tables.h:11281. For such a domain, GetRegistryLength returns 0 because FindRegistryNode finds no root node. parseTrustKitConfiguration then raises NSException "TrustKit was initialized with an invalid domain %@" at parse_configuration.m:72-75. It is reached from -[TrustKit initWithConfiguration:] at TrustKit.m:137, usually during launch, so an uncaught exception crashes the app. On the base branch the same config parses and the app starts. No safeguard applies: the update script's verify step only tests rules in the new list, and the PR does not document the removals.

    Verification: Severity: nit. Trigger: an app's kTSKPinnedDomains still lists a host under a TLD that has been retired since the 2018 list. In the diff, static const size_t kNumRootChildren = 1555; becomes 1448 (registry_tables.h:11281). parse_configuration.m:72-75 raises on every pinned domain whose GetRegistryLength is 0. So a config the base accepted now raises at TrustKit initialization.

@alexanderdibenedetto

Copy link
Copy Markdown
Contributor Author

Closing in favor of removing the list entirely so that teams with internal/corporate domain names or IP Addresses can use them.

@alexanderdibenedetto

Copy link
Copy Markdown
Contributor Author

Declined in favor of #371 which loosens restrictions to allow IP Addresses and custom domains (such as those used internally for corporate apps).

@alexanderdibenedetto
alexanderdibenedetto deleted the adb/issue-333-invalid-domain-names branch October 22, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants