[tls] Match server_name against SAN IPAddress #381
Conversation
|
Can you please quote the RFC or appropriate spec for this? We're also going to need an explicit unit test for this |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #381 +/- ##
===========================================
- Coverage 100.00% 99.93% -0.07%
===========================================
Files 22 22
Lines 4553 4555 +2
===========================================
- Hits 4553 4552 -1
- Misses 0 3 +3
☔ View full report in Codecov by Sentry. |
|
RFC 6125 explicitly considers matching by IP Address to be "out of scope", but I don't know if a subsequent RFC clarifies the expected behaviour: |
|
RFC 9001, Section 4.4 says "for example RFC 2818" which is the old HTTP-over-TLS, although the text does not seem very normative. The linked RFC 2818 simply says "the iPAddress subjectAltName must be present in the certificate and must exactly match the IP in the URI". There's a discrepancy here: HTTP specifies and uses URI, but QUIC, at the layer below HTTP/3, does not speak URI. So this text can only be taken as an "example", since it apparently cannot be applied as-is. So I'm not sure what's the IETF opinion on this. However this seems to be a common practice: CloudFlare, on 1.1.1.1, both TCP 443 and UDP 443, serves with their certificate containing SAN IP:1.1.1.1. I did not try any HTTP/3 clients, but as for HTTP 1 or 2 over TCP TLS, these clients accept that cert by default:
Just to be clear, I don't think either CloudFlare or these clients are authoritative on this topic; but they give me a feeling that support for this feature is more frequently implemented than not. Btw when digging those RFCs I noticed a missing piece: RFC 5280, Section 4.2.1.10 specifies that "Name Constraints" can be given in CIDR ranges, and (as I understand it) should be checked; I think this check is not implemented in this PR. I'm unsure how useful it is; this SO question implies Chrome does implement the check. Oh this is so complex I need some more time if we want this constraint implemented together... |
|
Ok fair enough I think I'm convinced the usecase is legitimate, thanks for the research! Implementation-wise, I'm currently considering switching subject validation to Also: how do you plan to pass an IP address to the TLS stack? Currently the TLS stack has no concept of the remote party's IP address, only the hostname to use for SNI. |
Yes, it covers matching of the IP. But it seems name constraints are not covered; I'm feeling that this may be a limitation in the API, because such constraints come from some issuer up the trust chain, but if we take only the end certificate as input we cannot tell if there's a constraint to apply. Either way, switching to that lib should be enough to supersede my PR.
I create |
|
Superseeded by #390 |
This simple change "works for me"; I think it could be useful to some, and worth adding if it does not introduce security holes, although I'm not 100% confident about that. Do you prefer this as-is, or some sort of opt-in switch for this part?