Add MUST for clients to validate ECHConfig.contents.public_name.

[dmcardle]

[Edit: deleted "Fixes" line for #405]

[davidben]

From #405, we also talked about IP addresses and such. Do we still want to do that?

Whether the client validation is SHOULD or MUST (TBH I kinda prefer MUST, given the implications on private services... if it's a SHOULD, we should talk about why so implementations can make an informed decision), we do also need to say what values are valid for the server.

[dmcardle]

OK, switched to a MUST. Added IP addresses as well.

I'm calling out to RFC3986 for IPv4/IPv6 parsing. I'd like to avoid any ambiguities that might creep in if I were allowed to define the grammar, like allowing octal numbers in an IPv4 address, e.g. 192.168.0.042 != 192.168.0.42.

For hostname parsing, I'm borrowing from RFC5890 so we can parse IDNs.

[richsalz]

Added IP addresses as well.

I think that is a mistake.

[cbartle891]

Added IP addresses as well.

I think that is a mistake.

Why is that?

[richsalz]

I think IP addresses are a mistake because they can introduce parsing ambiguities (as pointed out above), and sometimes those cause problems (https://securityledger.com/2021/03/critical-flaws-found-in-widely-used-netmask-open-source-library/). Also, SNI is really limited to hostname (as @davidben has pointed out before, referencing AGL's post), and having the two mismatch in their capabilities bothers me.

[dmcardle]

Agreed that the capability mismatch between SNI and ECHConfig.public_name is odd. However, there is already a capability mismatch between ECHConfig.public_name and names allowable on server certificates. At least in theory, the server's certificate might not have a hostname.

That being said, I don't have a strong opinion either way. If we do decide to disallow IP addresses, I think we'd still want the client to look for them so it can reject those ECHConfigs that use IP addresses. This is about stopping wacky values from bubbling up to the application.

[richsalz]

I think we'd still want the client to look for them so it can reject those ECHConfigs that use IP addresses

The client sending the ECH or the server receiving it?

Current TLS stacks don't seem to check, on either side. (At least OpenSSL and its major forks)

[dmcardle]

I was referring to the client, but yeah, scratch that idea. If public_name can only be a hostname, the client should only validate that it looks like a hostname.

[davidben]

I don't think the capability mismatch between SNI and ECHConfig.public_name is a fatal. We can move ECHConfig.public_name up an abstraction level. I think we should do this whether we allow IPs or not.

• The public name isn't "put this in ClientHelloOuter.server_name" but "the name you are trying to connect to in the outer handshake". In particular, this encompasses both ClientHelloOuter.server_name and certificate verification.
• To fill in ClientHelloOuter.server_name, you use the rules in RFC6066: server_name is the name you are trying to connect to, provided it's a DNS name.

Under this model, it is perfectly coherent for "the name you are trying to connect to" to be an IP address, if we want to do that. If we wish to do that then, yeah, we'll need to spell out the syntax here. That syntax clearly does already exist (URLs, etc.), but not in TLS. I don't particular care whether we do it, though.

How about this:

1. Right now, IP addresses are implicitly not okay by way of CHOuter.server_name = ECHConfig.public_name. So let's not introduce them in this PR and merely iron out the DNS name validation. This PR adds to, but does not close #405.
2. We then rework the text to redefine public_name as above. This is a prerequisite to allowing IPs, but I think it's worth doing either way.
3. We decide the IP question (interim?) and resolve #405 with the result.

(Sorry, I think some of this confusion is my fault. I filed two related, but separate, discussion points in #405, which was confusing. My comment in #413 (comment) was most in reaction to "Fixes #405"; that we shouldn't close it out without resolving both points.)

[sftcd]

On 31/03/2021 20:50, David Benjamin wrote: 3. We decide the IP question (interim?) and resolve #405 with the result.

I think I'd mostly be against supporting IP addresses in ECHConfig.public_name. I suspect including an IP address in outer.sni would break some things. The comment below is from OpenSSL's ssl/statem/extensions_srvr.c: /* * Although the intent was for server_name to be extensible, RFC 4366 * was not clear about it; and so OpenSSL among other implementations, * always and only allows a 'host_name' name types. * RFC 6066 corrected the mistake but adding new name types * is nevertheless no longer feasible, so act as if no other * SNI types can exist, to simplify parsing. * * Also note that the RFC permits only one SNI value per type, * i.e., we can only have a single hostname. */ Separately, we could land ourselves with yet more confusion related to the address to which one ought send a ClientHello, (the address in the ECHConfig or in the svbb.ipv?hint or the A/AAAA of one of the names involved, and which to prefer if some things have DNSSEC etc etc.). Cheers, S.

[davidben]

@sftcd Right, I think it's unambiguous, whatever we do with IP addresses, they won't go in ClientHelloOuter.server_name. Any use of this, if we want it, would be for public name verification, not the server_name value. See my comment above with how to think about public_name.

[dmcardle]

SGTM, folks. Just dropped the parts about IP addresses.

[chris-wood]

This seems like it would prohibit _service.name. @huitema, @DavidSchinazi: would this be a problem for the DNS-SD case?

[dmcardle]

Looks like I was a little overzealous removing the IP address text.

We still need to check that public_name is not an IPv4 address because dotted-decimal IPv4 addresses would slip through the cracks of hostname validation. This is bad because a malicious/misconfigured DNS would cause the ECH client to break RFC6066's rule on the contents of "server_name":

Literal IPv4 and IPv6 addresses are not permitted in "HostName".

(Re-added this IPv4 check in the last commit.)

[richsalz]

I would rather make "validate the host name" be a SHOULD and remove all these requirements.

[davidben]

ECH implementations not validating that string risks surprising behavior for a lot of applications, and potentially more attack surface to private services than before. Have you looked at the discussion in issue #405, in particular:

Current TLS stacks don't seem to check, on either side. (At least OpenSSL and its major forks)

Sadly, the thing that makes that work (logic earlier in the application) doesn't apply here. See #405 (comment)

[richsalz]

ECH implementations not validating that string risks surprising behavior for a lot of applications, and potentially more attack surface to private services than before.

Can you explain your concerns in more detail?

I think the application should be responsible for validation.

[chris-wood]

I think the application should be responsible for validation.

The application should never see the value of ECHConfig.public_name, right?

[davidben]

Can you explain your concerns in more detail?

See #405, specifically #405 (comment) and #405 (comment)

I think the application should be responsible for validation.

The application should never see the value of ECHConfig.public_name, right?

Well, there are several things going on here:

First, different systems may differently divide the "TLS" part and the "application" part. The spec mostly captures behavior of the overall client system, because that's all we can usefully talk about. (For example, ECH specifies a retry flow that involves making a new connection. At the level of many TLS libraries, including OpenSSL + derivatives, the caller makes connections. So instead the TLS library can expose APIs and document the caller's responsibilities as part of enabling ECH. Other interfaces may be create their own connections and handle this internally.) So whether it's the application or TLS, at the implementation level, isn't a hard deciding factor.

Second, with that said, yes, I expect that the majority of TLS <-> application interfaces will treat the ECHConfigList as an opaque blob. That is a good division of responsibilities because the TLS half already needs to process individual ECHConfigs in the ECHConfigList and select one based on its internal capabilities. Having to thread through application-specific DNS validation seems like it'd be complex. (But this is just my view as an implementor. The spec merely describes the overall behavior of the client system. How you choose to layer it is an implementation decision.)

Finally, there's the question of whether TLS as a spec should have opinions here or if it should punt to some upper layer spec profile, as we did with 0-RTT. I don't think that's warranted here. RFC6066 already decided what can go in server_name, so TLS already is plenty opinionated on the shape of strings meant to be DNS names.

Thus, IMO, the simplest and most practical story is to have TLS handle this, rather than thread it up through all the layers.

[richsalz]

I am willing to accept that I am in the rough here.

[bemasc]

As noted in #405 (comment), I don't think we should be putting IP addresses in public_name.

[dmcardle]

In the interest of dealing with one problem at a time, this PR no longer attempts to spell out how to recognize IP addresses.

Roughly:

• The client MUST validate public_name as a LDH hostname.
• The client SHOULD exclude IP address literals, which may come in many strange forms, or else it may send a non-compliant "server_name" extension.

If we care to make excluding IP addresses a MUST, let's work out the details in another PR. Same goes for if we want to explicitly support IP address literals — we still need to validate them to protect server applications from receiving IP addresses in non-standard notations, e.g. octal.

[davidben]

Man, this IP address thing is a mess. The SHOULD makes me sad, but I don't have a clear alternate suggestion. I can see not wanting to tie to the slightly odd "first-match-wins" parse from URLs. (And then there's the mess that is URL parsing...) Although, amusingly, RFC6066 section 3 clearly assumes that we have somehow defined this in a TLS-compatible way, but it forgot to cite anything. :-(

I dunno, I'll defer to others on the exact wording here. @martinthomson, what do you think?

Either way, I'm on board with dealing with one problem at a time in this PR.

[martinthomson]

High-level, I think that we need to agree on what this value is for first.

Editorially, I think that this text needs more space to move. Trying to cram all this stuff into a single paragraph isn't doing us any favours. Make a new section for this. It's certainly important enough to do properly.

My sense is that the right answer to the underlying issues is that this is a reference identity rather than something we put in server_name in ClientHelloOuter.

In that case, the fallback is achieved by establishing a connection and validating that the entity providing the updated configuration is able to answer for this reference identity. You then construct ClientHelloOuter in such a way as to ensure that you are able to successfully connect to that reference identity if ClientHelloInner cannot be accepted.

If this is just a name, then this is over-specified. If the goal is to describe what goes in ClientHelloOuter server_name, none of the validation is necessary. If the server wants you to put junk in the field, then I don't see how that junk would be any more identifying than an identifying DNS name. You really only need special IP address rules if you intend for this to be a reference identity.

[davidben]

I think there are a couple things going on here:

1. ECHConfig.public_name, as currently written, is described as what goes in CHOuter.server_name. That wasn't right and we should have made it outer reference identity. I believe @chris-wood is going to do a separate PR after this goes through.

2. SNI or reference identity, we need to define what the type of the thing that goes in ECHConfig.public_name and what validation the client does. In particular, without validation, it gives the network more control over what goes into CHOuter.server_name than would otherwise be the case. Normally, in a typical HTTPS client, TLS can just assume the hostname has passed whatever checks earlier in the stack (URL parsing, DNS lookup, etc.) and so checks in TLS are not load-bearing. The public name breaks this because the TLS layer is fabricating a hostname on its own. So we should have TLS validate it before running too far with it.

This PR addresses (2).

[chris-wood]

Following up on @davidben's comment, I was indeed planning on clarifying that ECHConfig.public_name is a reference identity, and planned on renaming this to something other than public_name to match. Basically: ECHConfig.public_identity may be either a name or IP address, validated according to the rules in this PR, and clients will use that identity when constructing the ClientHello per the normal rules. (If it's a name, put it in "server_name," else don't put it in that name.)

I think we can probably land this PR as-is. Is everyone else OK with this plan?

[sftcd]

On 12/04/2021 21:56, Christopher Wood wrote: Following up on @davidben's comment, I was indeed planning on clarifying that ECHConfig.public_name is a reference identity, and planned on renaming this to something other than `public_name` to match. Basically: `ECHConfig.public_identity`

It's a nit, but I don't think I've ever seen a use of the term "identity" that didn't eventually cause confusion. I think public_name was better TBH.

may be either a name or IP address,

Sorry if I missed the discussion but: - Are one or more IPv4/IPv6 addresses allowed and if so with what syntax? - If only 1 is allowed, why? (Syntax still needs a reference.) - I don't know how an IP address here ought be handled if it differs from those in the SVCB address hints fields. (Not to mention the origin's A/AAAA records.) - Lastly, I forget why we even want an IP address there at all. Thanks, S.

…

validated according to the rules in this PR, and clients will use that identity when constructing the ClientHello per the normal rules. (If it's a name, put it in "server_name," else don't put it in that name.) I think we can probably land this PR as-is. Is everyone else OK with this plan?

[chris-wood]

I assumed there's at most one identity in this field, be it a name or address, but @dmcardle can clarify. Also, I think the identity here only pertains to how the client-facing server is authenticated. If a server uses an IP-address certificate for the rejection path then it might specify the single IP address of that certificate. (Clients don't use SVCB hints for this, so they're orthogonal.)

[sftcd]

On 12/04/2021 22:32, Christopher Wood wrote: I assumed there's at most one identity in this field, be it a name or address, but @dmcardle can clarify. Also, I think the identity here only pertains to how the client-facing server is authenticated. If a server uses an IP-address certificate for the rejection path then it might specify the single IP address of that certificate.

There can be many SANs even if those are IP addresses. So I think you mean that you want exactly one IP address here and if one is here that MUST match one SAN iPaddress from the cert? Personally I don't think that's useful enough to support but maybe I missed why it was really good.

(Clients don't use SVCB hints for this, so they're orthogonal.)

TBH, I doubt implementers will notice that orthogonality. And those deploying even less would be my guess. If one is allowed put a single IP address in an ECHConfig and others in SVCB hints then someone needs to say what to do when those aren't the same. S.

[chris-wood]

There can be many SANs even if those are IP addresses

True, but I'm just commenting on parity with what's there right now. (ECHConfig.public_name is just one name.) We may change that later if people want it? I don't see a reason to, though.

So I think you mean that you want exactly one IP address here and if one is here that MUST match one SAN iPaddress from the cert?

Yeah, that's what I'm thinking.

If one is allowed put a single IP address in an ECHConfig and others in SVCB hints then someone needs to say what to do when those aren't the same.

Hmm, yeah, we may be able to clarify this, but given that we don't discuss anything about the SVCB record (beyond the config) currently, it may not be needed.

[dmcardle]

Currently, this PR explicitly states that the client SHOULD ignore the ECHConfig if its public_name contains an IP address. Unfortunately, what constitutes a textual representation of an IPv4 address is pretty underspecified, so I didn't manage to come up with a foolproof validation algorithm, otherwise I would make it a MUST.

If we want to allow IP addresses, let's figure it out in another PR.

[martinthomson]

So I don't think that we need to worry about IPv6 here. That will never match LDH.

The IPv4 thing is unfortunate. I think that the risk is that the public name is passed to a certificate validation library that subsequently treats the name as an IP address (matching against an ipAddress SAN rather than dNSName SAN) when doing fallback process. If there is any potential for confusion about this string (or look-alikes if we consider the octal mess) then it might happen that there is variation in what certificates are considered OK.

That leads to a reliance on the certification authorities being careful about issuance for names that look like IP addresses. We don't want that.

Either way, I don't like "SHOULD" here. It retains the potential confusion. If one client component is sloppy and doesn't bother other components are then forced to contend with that choice.

I think that I've come around to @bemasc's viewpoint on this. It's reasonable to require a domain name for fallback in all cases, even where a server is identified by IP address alone.

I think that means prohibiting IPv4 addresses, just to avoid the potential confusion. That is, if the name contains exactly 4 labels that are only digits, then the record is invalid. IPv6 addresses will look after themselves as they aren't valid LDH A-labels.

Note: I don't know if there are rules that might allow 172.0.2.2045 to be registered, which is clearly not an IP address. I believe that the main protection we have against valid DNS names appearing to be IPv4 addresses is the gTLD registration rules (from https://newgtlds.icann.org/en/applicants/agb Section 2.2.1.3.1 clause 1.2.1 this is currently assured as only a-z is permitted from the ASCII range).

[chris-wood]

The IPv4 thing is unfortunate. I think that the risk is that the public name is passed to a certificate validation library that subsequently treats the name as an IP address (matching against an ipAddress SAN rather than dNSName SAN) when doing fallback process. If there is any potential for confusion about this string (or look-alikes if we consider the octal mess) then it might happen that there is variation in what certificates are considered OK.

@martinthomson can you drill into this? I must be misunderstanding you (and maybe the problem), but are you suggesting that strings which look like IP addresses might be parsed incorrectly by the thing validating certificates against such look-alike addresses?

That leads to a reliance on the certification authorities being careful about issuance for names that look like IP addresses. We don't want that.

They also have to be careful about names that aren't actually domain names ("DROP TABLES" or whatever). How is this a substantially new requirement?

Prohibiting IP addresses here seems overly restrictive. Given that it doesn't seem challenging to support, and does not, to my knowledge, introduce any new requirements for CAs or certificate validation APIs, I'd like to hear more reasons why we ought to definitely rule them out. And as @dmcardle says, we should hash this out in a separate issue and PR. In the interest of making forward progress, I'm going to merge this PR as-is, and we can discuss more in #424.

[martinthomson]

@martinthomson can you drill into this? I must be misunderstanding you (and maybe the problem), but are you suggesting that strings which look like IP addresses might be parsed incorrectly by the thing validating certificates against such look-alike addresses?

In most cases, I don't anticipate problems: there will be one certificate validator used during fallback and that will make a single determination. IP address lookalikes in that case probably won't be a problem. However, in cases where there are multiple components involved, there might be IP address lookalikes that pass one validator but not another. That might be used to attack clients.

[davidben]

Given how complicated IPv4 literals can get in some applications, I suspect the TLS library classifying and rejecting IPv4 literals in a way that ensures the calling application always interprets it as a DNS name is hopeless.

I'm now inclined to say we should:

1. Do #426 and say public_name is exclusively DNS. If anyone adds IP in the future, the expectation will be a separate extension and thus not require any parsing to pick semantics.
2. The exclusively-DNS public_name field is validated according to DNS rules and leave it at that. That at least avoids completely unbounded DNS control on the ClientHello.
3. Concede that we can't reliably exclude IPv4 literals at this layer. Call this out and say that clients MUST NOT validate the string as IP addresses. If your verifier has separate APIs for configuring DNS vs IP names for subjectAltName, feel free to pass the string as a DNS input and move on. If your verifier's API takes a more URL-like hostname, you should use whatever logic you used to distinguish DNS vs. IP and fail the verification if you think the string is an IP. (Effectively that verifier believes "DNS:1.2.3.4" is never valid because it has no way to spell it.)

This does mean the DNS can cause ClientHelloOuter to violate the RFC6066 prohibition on IPv4 literals, but since RFC6066 failed to specify what syntax it was prohibiting, the meaning of that rule is somewhat unclear.