Subject: Re: [TLS] Don't Split HelloRetryRequest

From: Stephen Farrell <[email-masked]>
Date: Thu, 1 Apr 2021 12:46:17 +0100

I'm not sure I agree with all of Martin's remarks below,
but I absolutely agree both that we seem to be almost
happily adding complexity and that that's a bad plan.

S.

On 01/04/2021 02: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
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls
>

---

From: Christopher Patton <[email-masked]>
Date: Thu, 1 Apr 2021 10:29:55 -0700

Hi Martin, would you mind working out a PR? I think being able to compare
407 to a concrete alternative would be helpful. Just so that we're on the
same page, here's a quick summary of the issues that 407 is designed to
solve. (These may or may not be problems in your view, and I don't claim
this list is exhaustive.)
- 233: No acceptance signal until after HRR, so the procedure for computing
CH2 is underspecified. This can be avoided by advertising the same
preferences in CHI/CHO, but the spec doesn't require this.
- 373: To fix 3233, can we put an acceptance signal in HRR.random? Probably
not, since HRR.random has a value specified in RFC8446.
- 358: RFC8446 allows the value of an extension in CH2 to differ from CH1
only if the extension appears in HRR.
- 333: "Split mode" is broken, since the client doesn't know who the cookie
is for.

Best,
Chris P.

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 1 Apr 2021 19:07:08 +0100

Given the range of Martin's comments, I'd be surprised
if that could be reduced to a PR;-)

But let me ask a question meanwhile - how often does HRR
actually happen and could we not just let ECH fail in a
bunch of situations that would otherwise require all this
new complexity?

Thanks,
S.

---

From: Christopher Patton <[email-masked]>
Date: Thu, 1 Apr 2021 11:13:56 -0700

> bunch of situations that would otherwise require all this

One way HRR is used is in case the client's and server's cipher suite
preferences don't intersect. This feature is an essential part of TLS, as
there's no a priori reason why the client and server will initially
advertise overlapping preferences. (They usually do, hence the claim that
HRR is rare.) I don't think aborting the handshake instead of HRR is an
acceptable solution, as this would mean there are deployments with which
TLS couldn't be used.

Chris P.

---

From: Christopher Patton <[email-masked]>
Date: Thu, 1 Apr 2021 11:20:15 -0700

> acceptable solution, as this would mean there are deployments with which

Slight refinement: David B. pointed out to me that "cipher suite
preference" isn't quite the right term here. The client provides key shares
in its CH that it guesses the server can use; if it's wrong, then the
server replies with HRR. A more accurate statement would be that "HRR is
essential for ensuring the client sends a key share the server supports."

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 1 Apr 2021 19:24:18 +0100

Hiya,

On 01/04/2021 19:13, Christopher Patton wrote:
> HRR is rare.) 

I've seen that claim and kinda suspect it's valid but my
question was whether anyone had measured it?

> I don't think aborting the handshake instead of HRR is an

No. At worst it might mean that there are places that need
to do some config before TLS+ECH will work, but that's true
already - they'll need to publish an ECHConfig anyway. I
really wonder if we could not achieve as good an outcome
much more easily with some guidance on checking your front-
end's choice of curves and failing when some of the HRR
cases get out of whack. (There'd still be some work to do
for ECH as we can't as you say get rid of HRR, but there
might be a lot less work/complexity.)

Cheers,
S.

---

From: Carrick Bartle <[email-masked]>
Date: Thu, 1 Apr 2021 11:28:41 -0700

> I absolutely agree both that we seem to be almost
> happily adding complexity 

I don't think you have to assume bad intent (or negligence). Several people brought up valid concerns and we're trying to address them.

> On Apr 1, 2021, at 4:46 AM, Stephen Farrell <[email-masked]> wrote:
> 
> 
> <OpenPGP_0x5AB2FAF17B172BEA.asc>_______________________________________________
> TLS mailing list
> [email-masked]

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 1 Apr 2021 19:41:59 +0100

Hiya,

On 01/04/2021 19:28, Carrick Bartle wrote:
>> I absolutely agree both that we seem to be almost happily adding
>> complexity
> 
> I don't think you have to assume bad intent (or negligence). Several
> people brought up valid concerns and we're trying to address them.

Hmm. I got an offlist mail on that too. I'm sorry
if I gave the impression that people's intentions
were dodgy, that really wasn't my intent. I think
it's entirely possible to have the best intentions
and yet to be too happy to add complexity. I've
done it myself loads of times in the past, and there
are even RFCs that show that;-) That is what I
think may be happening wrt ECH - we seem to be
forgetting that this stuff is inherently complex,
even in it's simplest case, and making it more
complex could kill it.

Cheers,
S.

>> I'm not sure I agree with all of Martin's remarks below, but I
>> absolutely agree both that we seem to be almost happily adding
>> complexity and that that's a bad plan.
>> 
>> S.
>> 
>>> 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 [email-masked] https://www.ietf.org/mailman/listinfo/tls
TLS mailing list
>> [email-masked] https://www.ietf.org/mailman/listinfo/tls
>

---

From: David Benjamin <[email-masked]>
Date: Thu, 1 Apr 2021 14:56:23 -0400

Hi Martin,

Thanks for the comments. As the author of #374, I obviously didn't think it
so clearly unnecessary, but glad to hear your thoughts. Hopefully we can
get on the same page as to what the issues are and/or a better solution.
(Always happy to replace something with a simpler option, provided it also
works!) To that end, I'd second Chris's suggestion of working through an
alternate PR so we have something concrete.

Going through some of these in detail (inline)

On Wed, Mar 31, 2021 at 9:58 PM Martin Thomson <[email-masked]> wrote:

> 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.
>

This was a previous attempt at this, but it seems to have been confusing.

I do agree that having ClientHelloInner vs ClientHelloOuter differences
that result in different HelloRetryRequest acceptance criteria is a bad
plan: in general, I think a client implementation should be judicious about
which difference it exposes because each difference needs special logic to
make sure it's using the right set as you evaluate the handshake.

At the same time, I worry that *mandating* it in the extension will make
this decision process tricky to navigate and get us in a rough corner. PR
#316 was trying to capture exactly this, but it seems we didn't get on the
same page. (Maybe we can now?) See this discussion:
https://github.com/tlswg/draft-ietf-tls-esni/pull/316#discussion_r509880007

And so the text ended up with something self-contradictory. It says:

"[...] clients offering ECH MUST ensure that all extensions or parameters
that might change in response to receiving a HelloRetryRequest have the
same values in ClientHelloInner and ClientHelloOuter."
https://tlswg.org/draft-ietf-tls-esni/draft-ietf-tls-esni.html#name-handling-helloretryrequest

And then it goes to list things that differ from this criteria! Indeed the
list is right and the criteria is wrong. The criteria is neither sufficient
(we do need cipher suites to align), nor sufficient (as you note, the key
share *values* don't strictly need to match). I concluded from this
exercise that trying to formulate the rules is a bit of a mess. As you
allude to with password_salt, we need to formulate something that can
capture future HRR extensions. (I'd also gotten that sense from one of the
past WG meetings: land the HRR-matching rule as a bandaid, but we didn't
have consensus for it as an actual solution.)

I actually had not heard of password_salt, so thanks for the pointer. I've
only skimmed it but, at a glance, this seems exactly like something where
you'd want ClientHelloInner vs ClientHelloOuter to differ. It seems to be
about some password-based TLS authentication, but any client authentication
should only apply to the true ClientHelloInner handshake and not the
fallback ClientHelloOuter handshake.

> 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.
>

I'm not sure I follow this point. Do you mean the draft as-is, or the PR? I
don't believe the draft as-is does, and I don't think the PR does anything
to the transcript that draft as-is doesn't already do. The transcript
behaviors are largely forced by a combination of backwards compatibility
for ClientHelloOuter and split mode for ClientHelloInner.

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...).
>

I think you may have misunderstood #333. That or I'm misunderstanding your
response. Imagine a client-facing server that wants to support HRR
statelessly. When that client-facing server accepts ECH and gets an HRR
from the backend server to forward, it has no opportunity to inject its own
cookie.

Maybe our answer is client-facing servers always do HRR on ECH accept. I'm
perfectly fine with that, but if that's our answer, we should explicitly
make that decision.

> 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.
>
> 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.
>

Independent of what we do in ECH, I like that adjustment to HRR rules. If
we want to do that as part of resolving the ECH issue, we should make that
decision in rfc8446bis. I filed
https://github.com/tlswg/tls13-spec/issues/1223, but it's not clear that
has consensus yet.

In terms of sticking out, Ben Schwartz also pointed out another issue with
the current situation. It's not just that, where HelloRetryRequest occurs
naturally, this quirk reveals GREASE. A forged HelloRetryRequest also
reveals GREASE. This PR resolves this by authenticating the ECH acceptance
signal in 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.
>

I think trying to avoid HRR, provided we actually make it optional, can be
pursued independent of this question. If we say it's optional, we still
need to make ECH + HRR work. If we make it mandatory, I don't see how we
can make it deployable on the server. In particular:

- Servers can't guarantee client behavior a priori, so it'd need a binding
"hint".
- We'd need a story for mismatch between DNS configs and the server, to
keep ECH deployable at all.
- We'd need to be comfortable that every HRR reason, past and present, can
be hinted away by DNS. I suspect cookie and password_hint already break
this.

> # Conclusion
>
> I'm firmly opposed to splitting HelloRetryRequest.  I would like to deploy
> ECH and this doesn't really help with that.
>
> 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.
>

I'd be very interested to understand the architectural constraints here.
This didn't seem likely to be particularly challenging to implement, but I
only know our own stack and certainly could have mispredicted.

David

---

From: Stephen Farrell <[email-masked]>
Date: Thu, 1 Apr 2021 19:57:31 +0100

Hiya,

On 01/04/2021 19:24, Stephen Farrell wrote:
> some guidance on checking your front-
> cases get out of whack

Actually it occurs to me that we could for example say
that back-ends are RECOMMENDED to support the first
curve listed in ECHConfig for both ECH and the TLS h/s
and then also RECOMMEND that clients include a key share
for that curve as well. With that, it might be acceptable
to not use HRR (but fail) if the inner CH has no key
shares that the back-end can handle. Things like that
might reduce the number of HRR cases we need to handle
via new protocol mechanisms.

S.

---

From: "Martin Thomson" <[email-masked]>
Date: Fri, 02 Apr 2021 07:17:40 +1100

I'm going to briefly respond, but as this is a public holiday and I have no real desire to work too much, please forgive any radio silence that follows.

On Fri, Apr 2, 2021, at 05:56, David Benjamin wrote:
> plan: in general, I think a client implementation should be judicious 
> about which difference it exposes because each difference needs special 
> logic to make sure it's using the right set as you evaluate the 
> handshake.
> 
> At the same time, I worry that *mandating* it in the extension will 
> make this decision process tricky to navigate and get us in a rough 
> corner. PR #316 was trying to capture exactly this, but it seems we 
> didn't get on the same page. (Maybe we can now?) See this discussion:

My claim (to be verified) is that mandating something is possible.  It might not require a simple rule (cannot be different); it might instead require that each extension be analyzed and have its own rules instead.  I'm confident that for the 4 or so extensions that can change in response to HRR, we can make that work.

> I'm not sure I follow this point. Do you mean the draft as-is, or the 
> PR? I don't believe the draft as-is does, and I don't think the PR does 
> anything to the transcript that draft as-is doesn't already do. The 
> transcript behaviors are largely forced by a combination of backwards 
> compatibility for ClientHelloOuter and split mode for ClientHelloInner. 

I refer to this, which I would propose be cut:

> Note: if ClientHelloOuter and ClientHelloInner use different groups for their key shares or differ in some other way, then the HelloRetryRequest may actually be invalid for one or the other ClientHello, in which case a fresh ClientHello MUST be generated, ignoring the instructions in HelloRetryRequest. 

-- https://tlswg.org/draft-ietf-tls-esni/draft-ietf-tls-esni.html#section-6.1.4-4

> I think you may have misunderstood #333. That or I'm misunderstanding 
> your response. Imagine a client-facing server that wants to support HRR 
> from the backend server to forward, it has no opportunity to inject its 
> own cookie.

Yeah, I missed that distinction.  Let's not support that.  If the client-facing server wants to forward AND use the cookie to route the second ClientHello without maintaining state, let it coordinate with the hidden server.
 
> In terms of sticking out, Ben Schwartz also pointed out another issue 
> with the current situation. It's not just that, where HelloRetryRequest 
> occurs naturally, this quirk reveals GREASE. A forged HelloRetryRequest 
> also reveals GREASE. This PR resolves this by authenticating the ECH 
> acceptance signal in HelloRetryRequest.

Yeah, that leaks that grease happened.  We can thank those who were overzealous in their interpretation of the rules in RFC 8446.  Either way, this style of attack doesn't really bother me that much as it is very obvious and requires active interception.
  
> I'd be very interested to understand the architectural constraints 
> here. This didn't seem likely to be particularly challenging to 
> implement, but I only know our own stack and certainly could have 
> mispredicted.

As I said in the meeting.  HRR only has limited battle testing in practice, this would be even less tested, but far more complex.  Anything we can do to limit complexity, even at the cost of more complex analysis, is worth doing in my opinion.  There is only one analysis we need to do for the specification, but (hopefully) multiple implementations.

---

From: "Martin Thomson" <[email-masked]>
Date: Tue, 06 Apr 2021 12:28:40 +1000

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:
> (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.)
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> 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.
> 
> HelloRetryRequest, which will avoid this compatibility bug, but also 
> reveal the fraud.  I can tolerate that small amount of leakage.
> 
> 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.
> 
> I'm firmly opposed to splitting HelloRetryRequest.  I would like to 
> deploy ECH and this doesn't really help with that.
> 
> 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.

---

From: Christopher Patton <[email-masked]>
Date: Thu, 15 Apr 2021 17:08:20 -0700

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 <[email-masked]> 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
>

---

From: "Martin Thomson" <[email-masked]>
Date: Mon, 19 Apr 2021 14:51:45 +1000

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.