Subject: [TLS] Don't Split HelloRetryRequest

From: "Martin Thomson" <[email-masked]>
Date: Thu, 01 Apr 2021 12:57:40 +1100

I just reviewed the proposal to split HelloRetryRequest into outer and (encrypted) inner.

https://github.com/tlswg/draft-ietf-tls-esni/pull/407/files

I'm strongly opposed to taking the change.  It complicates the design greatly and unnecessarily.

The existing design has some flaws, but they can be fixed more elegantly than this.

(Apologies if this seems a little long.  I'm writing down every possible argument I can think of, because I can't guarantee that I will be coherent at the meeting.)

# HelloRetryRequest Has a Narrow Purpose

Let's first address the key question of what HelloRetryRequest exists to do.  It exists to ensure that the client and server are able to jointly agree on keys to use for the remainder of the handshake.  This is a very narrow scope.

Furthermore, the particulars of key agreement are public.  This is important as we can also say that all hidden servers need to use the same configuration as it relates to cipher suites, key exchange, and related parameters, as the results of negotiation are sent in the clear in the ServerHello.

My claim here is that there is no value in protecting any parameter that might change in response to HelloRetryRequest.

# Don't Seek Complexity

It is entirely possible to imagine scenarios where an inner ClientHello has different preferences from an outer ClientHello.  While in theory we can construct a design that would support that (the pull request does this well enough), to do so only serves to increase complexity.  It does not address any real use case or problem that I'm aware of.

If we allow for the client to provide different values in inner and outer ClientHello messages, then the current design means that the client is faced with some ambiguity about which of the two messages a HelloRetryRequest applies to.  If we try to create an indicator, then that leaks.

We could solve the problem by making the protocol more complex.  Or we could avoid it.

This problem is entirely avoidable.

# Matching Inner and Outer Values

When we get right down to it, there are a very small number of things that truly change in response to HelloRetryRequest.  And all of these changes are to values that do not need confidentiality protection.

The draft lists three fields that change: ciphersuites, key_share, and version.  From my perspective, changing cipher suites, supported groups, or versions would be a big mistake.  So what changes is even more limited.  Just the shares in key_share.

On this basis, a client that offers cipher suites, groups, versions, and key shares that are identical in both inner and outer ClientHello messages will always receive a HelloRetryRequest that applies equally to both messages.

The only adjustment that is acceptable with respect to these fields being identical is the addition of TLS 1.2-only options to the outer ClientHello (or the removal of the same from the inner ClientHello if you prefer it that way around).  This is a fine optimization on the basis that accepting ECH represents a commitment to support TLS 1.3 (or higher).  But it is really just an optimization (the draft makes this mandatory, which is silly).  The client can therefore remove options from the inner ClientHello only if it is impossible to select them with TLS 1.3 or higher.

For new extensions, if they define a means of adjustment or correction via HelloRetryRequest (there is currently just one: password_salt, which I haven't examined), then they too need to follow this restriction.   It's not an onerous one.

Follow this simple constraint and HelloRetryRequest will always apply equally to both inner and outer ClientHello and everything works.  Conveniently, this is more or less exactly what the current draft says.  Almost.

The draft currently allows inner and outer ClientHello to have different types of key share.  The way it handles this is terrible: it recommends breaking the transcript.  To me, that seems like it would only serve to open the protocol up to downgrade attack.

Incidentally, I don't see a problem with having a different key share *value* in inner and outer ClientHello.  There's no point in doing that unless you believe that these values leak information (they really shouldn't), but it wouldn't break this model if a client decided to do that.

https://github.com/tlswg/draft-ietf-tls-esni/issues/333 appears to be concerned about the cookie only applying to one or other ClientHello.  I don't see how is the case, so I'm just going to say that this is fixed by having HelloRetryRequest apply to both inner and outer ClientHello messages.  If the client receives HelloRetryRequest that applies to just one of the two, then the problem is that the client is faulty.  That would be treated as a programming error as normal (crash, open a bug report, send an internal_error alert, etc...).

Then there are the things that more or less have to change in response to HelloRetryRequest, but really only because the ClientHello changes: padding, pre_shared_key, and ECH itself.  For those, we need to address a minor inconsistency problem at the level of the core protocol itself.

# Addressing Minor HelloRetryRequest Problems

We do need to fix RFC 8446 rules regarding HelloRetryRequest.  David already suggested some minute adjustments for that problem in https://github.com/tlswg/draft-ietf-tls-esni/issues/358 .  The short version is that extensions can define their own rules for how they change after HelloRetryRequest.  This is a good amendment, especially as it relates to extensions that are not known to the server.

That tweak does have deployment issues, because the original rules have been interpreted too literally in some cases, but that should not affect ECH specifically.  Servers that have this bug won't be able to deploy ECH without fixing the bug and that's OK.  Other servers will only see grease.

The draft currently mandates that greasing values not change after HelloRetryRequest, which will avoid this compatibility bug, but also reveal the fraud.  I can tolerate that small amount of leakage.

# Avoiding HelloRetryRequest

I think that Nick's suggestion for helping avoid HelloRetryRequest by placing hints about key shares in DNS SVCB/HTTPS records is a fine one.

I see the arguments about this being about the configuration needing to speak for backend servers when the record relates to frontend servers.  But my perspective here is that you already need to ensure that backend servers have a consistent cryptographic support profile; adding a small number of frontend servers to the set that need to be made consistent isn't that difficult.  If this consistency is not possible in some deployments, that's understandable, but then it is an optional enhancement that won't be available to those deployments, that's all.

Of course, this is an extension that we can pursue separately.

# Conclusion

I'm firmly opposed to splitting HelloRetryRequest.  I would like to deploy ECH and this doesn't really help with that.

I don't agree that there is a problem that needs to be fixed with the current draft.

On the other hand, I can guarantee that this change will delay Firefox deployment significantly (that is, for an indefinite period).  It would require rearchitecting a piece of code that is rarely used already (despite being a source of significant complexity) and replacing it with code that is even more complex and would include paths that are even more lightly used.