Re: [TLS] Don't Split HelloRetryRequest

[Martin Thomson <[e-mail]>]

I commented on the pull requests, but to close the loop here:

I can live with the acceptance signal proposed in #423.  However, I might still prefer the approach I proposed.  I say "might" because I might not understand the objections that have been raised properly.

I *think* that the main concern is that it imposes a cost on all future extensions (limited to those that might change in response to HRR) in that they need to consider how they might be affected by ECH.  I think that's entirely reasonable, but others seem to think that this is somehow unmanageable.

On Fri, Apr 16, 2021, at 10:08, Christopher Patton wrote:
> HI Martin, all, I added another alternative, so let me summarize for 
> everyone the possible paths forward, with links to the corresponding 
> PRs.
> 
> 1. https://github.com/tlswg/draft-ietf-tls-esni/pull/407: HRRInner and 
> HRROuter (original proposal, deemed too complicated).
> 2. https://github.com/tlswg/draft-ietf-tls-esni/pull/417: Strengthen 
> language around HRR-sensitive parameters (might be underspecified).
> 3. https://github.com/tlswg/draft-ietf-tls-esni/pull/423  Signal ECH 
> acceptance after HRR (a simpler alternative to #407).
> 
> Best,
> Chris P.
> 
> On Mon, Apr 5, 2021 at 7:29 PM Martin Thomson <[e-mail]> wrote:
> > I've created a few pull requests that make the changes I propose.  I think that the whole suite of related issues are closed as a result.
> > 
> > The main one is here: https://github.com/tlswg/draft-ietf-tls-esni/pull/417
> > There's a bit of rewriting here, but the change is not that large.  I would expect most implementations to be compliant already (it's much more work not to be).
> > 
> > The whole set: https://github.com/tlswg/draft-ietf-tls-esni/pulls/martinthomson
> > 
> > On Thu, Apr 1, 2021, at 12:57, Martin Thomson wrote:
> > > 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.
> > 
> > _______________________________________________
> > TLS mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/tls