feat(ssl): separate chain validation from hostname verification - #173
Merged
Conversation
A consumer pinned to a panel's CA gets one boolean for two different questions -- "is this the panel" and "does its certificate name this address" -- so a panel that moved to a new DHCP lease is indistinguishable from something impersonating one, and both are reported as a verification failure. `build_panel_ssl_context` gains `check_hostname`, which leaves the chain, signature and expiry checks intact and only stops asserting the binding between the certificate and the name used to dial it. An attacker without a key the pinned CA signed still cannot complete the handshake. `leaf_names_host` supplies the other half, hand-written against `getpeercert()` because `ssl.match_hostname` was removed in 3.12. It is stricter than that function was: no wildcards, no `commonName` fallback, DNS and IP entries that never substitute for one another, addresses compared parsed and names casefolded. Both live here rather than on the consumer side for the reason the module docstring already gives about the fingerprint: a security primitive with two implementations is a defect waiting for a firmware upgrade to find it.
CodeFactor flagged the matcher as a complex method, and it was: SAN extraction, host normalisation, address parsing and two matching rules in one body, with the branch for which rule applies interleaved through the loop. Split into `_san_entries`, `_names_address` and `_names_dns`, leaving `leaf_names_host` as the dispatcher it should have been -- normalise, then ask exactly one of the two questions. Behaviour is unchanged and the rules now read one per function, which for security code is the point.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A consumer pinned to a panel's CA gets one boolean for two different questions — is this the panel and does its certificate name this address — so a panel that moved to a new DHCP lease is indistinguishable from something impersonating one, and both surface as a verification failure.
The consumer that needs the distinction is the Home Assistant integration's reconfigure flow, where the two have opposite remedies: one is repaired by asking the panel to regenerate its certificate, and the other must be refused.
What
build_panel_ssl_contexttakescheck_hostname. The chain, the signature and the expiry are still verified against the pinned anchor; only the binding between the certificate and the name used to dial it is left unasserted. An attacker without a key that anchor signed cannot complete the handshake either way.leaf_names_hostsupplies the other half. Hand-written againstgetpeercert()becausessl.match_hostnamewas removed in Python 3.12, and stricter than that function was:commonNamefallbackEverything it cannot read answers
False.Why here rather than in the consumer
_ssl.py's module docstring already makes the argument about the fingerprint: a security primitive with two implementations is a defect waiting for a firmware upgrade to find it. That applies more to a hand-written hostname matcher, which is security-relevant and has no standard-library implementation left to defer to.Testing
1172 pass. 18 new tests cover the relaxed context end-to-end against a real handshake (a leaf naming
localhost, reached at127.0.0.1), that relaxing the name does not relax trust, and the matcher's rules individually._ssl.pyis at 100% coverage.Compatibility
Additive.
check_hostnamedefaults toTrue, so every existing caller keeps hostname verification without asking for it.