Subject: Re: [TLS] implementing ESNI/ECH draft-09

From: David Benjamin <[email-masked]>
Date: Tue, 2 Mar 2021 16:49:08 -0500

On Sun, Feb 28, 2021 at 12:35 PM Stephen Farrell <[email-masked]>
wrote:

> - This is *much* harder to implement compared to ESNI as
>    it interacts with the rest of the TLS stack/library in
>    many more ways. It should be an explicit goal to reduce
>    that complexity IMO and not increase it further.  That
>    complexity leads to *many* new failure modes that not
>    everyone will get right, e.g. incorrect encoding of inner
>    extensions, extension handlers that have side-effects
>    being called twice etc.
>      - Possible fix: organise an interim session (or slot
>        therein) with the explicit and only goal of
>        simplifying ECH.
>      - Possible fix: publish test vectors and require a
>        test vector from anyone proposing any new construct,
>        before accepting their suggested new crypto-wrinkle.
>      - Possible fix: make the spec much more explicit about
>        what goes into the transcript at every possible
>        stage, esp when that changes.
>

I agree calling extensions handlers twice would be problematic, but it
seems to me that's not necessary. The spec conceptually splits ECH into a
client-facing server and a backend server which I think, even for "Shared
Mode" implementations, hints at a cleaner implementation strategy:

Early in the handshake, before even version negotiation (as section 7.1
says) and certainly before calling any extensions handlers, see if there is
an ECH extension in the ClientHello. If so, attempt to decrypt the inner
ClientHello. If you succeed, continue the handshake with the inner
ClientHello. If you fail, continue the handshake with the outer
ClientHello. Ignore the ClientHello you didn't use.

This way you only run the complex stateful handshake logic on one
ClientHello, since that's logically the "backend server". The
"client-facing server" is really just a ClientHello dispatcher that needn't
interact with most of the handshake.

- There's no simple, sensible way to decide which
>    extensions to "compress" in the inner that I can see. As
>    a library, one could come up with some vastly complex API
>    to allow applications pretend to handle this, but that'd
>    seem more dangerous than useful. While I made the code
>    generic, it's not at all clear that'll ever be useful.
>      - Possible fix: forget generic compression and define
>        a small set of extension types that can be inherited
>        from the outer with all others being present as
>        needed in inner and outer. For future extensions
>        (e.g.  some many-octet PQ key agreement), have the
>        extension definition include the option of "see
>        outer" within the extension encoding.
>

Yeah, the compression mechanism is a bit complicated. I don't think "see
outer" in the extension definition is the right path. It would likely
introduce ClientHello malleability problems (see
https://tools.ietf.org/html/draft-ietf-tls-esni-09#section-10.10.3), unless
the extension further defines how to "compile out" out the "see outer"
portion.

Rather than exposing the details of the compression to the API, I think
it's better to reason about what is public, what is private, and what are
the goals of the wire image. It seems to me we have roughly two kinds
of clients to consider:

First, there are clients that send GREASE and wish to align their ECH and
non-ECH ClientHellos as much as possible. Those clients will likely send
the bulk of their extensions in ClientHelloOuter as most do not depend on
the server name. E.g. a browser always sends ALPN={h2,http/1.1}. There, the
default is to reference the extension, so the encrypted payload only
contains the couple of extensions that vary (SNI, PSK). Keeping that small
is good for bandwidth not just in ECH connections, but also to avoid an
overly expensive GREASE payload in non-ECH connections. Keeping the two
ClientHellos aligned also simplifies the rest of the handshake: you only
have one set of preferences to evaluate the server response against.

Then there are clients that don't especially care about their wire image.
They may instead send a minimal ClientHelloOuter, just enough to
authenticate a replacement config. (Or maybe they don't care about the
recovery flow at all and don't even do that.) Those clients will likely
send the bulk of their extensions in just ClientHelloInner, and omit them
from ClientHelloOuter altogether. There are a few extensions (key_share,
signature_algorithms, etc.) that are necessary in a minimal
ClientHelloOuter, so they may compress those still. Though this does give
outer and inner paths wildly different preferences, so the client needs to
modify the handshake accordingly.

Probably the way to simplify, in both spec and implementation, is to focus
on those two kinds of clients.

> - Given inner and outer can have different sets of
>    extensions, I guess there'll never be a way to make ECH
>    handling constant-time. If so, then that ought be covered
>    as a security consideration. If not, how'd that work?
>      - Possible fix: document potential consequences of
>        ECH never being constant time?
>

Yeah, constant-time seems a bit hopeless here. The point of ECH is to
protect sensitive fields in the inner ClientHello, but those sensitive
fields result in a choice of different extensions, certificate, and private
key in the server. That's going to result in a wildly different instruction
and memory access trace.

> - I don't plan on implementing the "MUST check
>    outer.sni==public_name" check ever really. (Unless
>    forced to as part of upstreaming maybe.)
>      - Possible fix: remove that MUST.
>

Is there such a requirement? I didn't think there was. This came up in
https://github.com/tlswg/draft-ietf-tls-esni/issues/389, which was closed.

David

---

From: Stephen Farrell <[email-masked]>
Date: Wed, 3 Mar 2021 21:21:16 +0000

Hiya,

>>     extensions, extension handlers that have side-effects

Just to note that my main point relates to complexity, and
extension handlers are just one example. If we could agree
that reducing the complexity is an important goal now, that'd
make me happy, regardless of what if anything the spec has
to say about extension handlers. But no harm to discuss that
example too of course (see below).

And to be honest: I am concerned that people want more
complexity and not less, (hi there HRR!) and ISTM that's the
wrong way to go, in this case, and in pretty much all cases
really.

> ClientHello. Ignore the ClientHello you didn't use.

So that'd mean if there's any failure processing any inner
extension after decryption in shared mode, then you don't
process any of the outer extensions at all and the server
MUST abort the entire connection as if it had received
gibberish? I guess the spec might need to say what error
to send in that case as otherwise we'd maybe be leaking
info as to what bit of the inner was dodgy?

> interact with most of the handshake.

There's also another extension-related issue: with 20+
extension types, some of which are inter-related, and
with compression options, that's a crap load of testing
to do. We could reduce that a lot if we ditch the
compression and e.g. state that one should follow the
design above for shared mode.

> Yeah, the compression mechanism is a bit complicated. 

Complicated, and not much use I think. An early assertion
I think I recall being made about ESNI was that it's size
didn't matter so long as the CH fits into one packet. Now
we seem to have reversed ground on that, at significant
expense in terms of complexity and for probably <100 octets
out of about 600.

I'd argue to just ditch this near-compression until we
really have a CH that's a few packets and then try figure
it out.

> I don't think "see

pqkeyshare.see_outer could be a hash of the PQ share from
the outer. Point is: we can leave that 'till later, we don't
need it yet.

> on those two kinds of clients.

I'm not convinced TBH, but you might be right. Seems though
that dichotomy might omit library clients which likely do
differ.

> key in the server. That's going to result in a wildly different instruction

Right. I'm not sure what info that might leak though.

> Is there such a requirement? I didn't think there was.

Bullet point 7 of page 13 of -09.

> This came up in

That GH issue may be closed but this wasn't discussed on the
list, and its not clear Martin was raising this exact topic,
(that I did raise in that GH issue without getting any
substantive response at all), and the above text is still in
the editor's draft, and it's the wrong text, and I still have
no plan to implement it. So I don't care if the GH issue is
closed or not:-)

S

---

From: Rob Sayre <[email-masked]>
Date: Tue, 9 Mar 2021 15:25:26 -0800

On Sun, Feb 28, 2021 at 9:35 AM Stephen Farrell <[email-masked]>
wrote:

>    that complexity IMO and not increase it further.
>

I agree that ECH is much more difficult to implement*, but I think the
changes required are all improvements.

Some of the complexity stems from accommodating clear-text ClientHello as
well as ECH.

thanks,
Rob

* Except in the sense that all of the crypto is delegated correctly
now--happy to let lower-level libraries do HPKE etc.

---

From: Stephen Farrell <[email-masked]>
Date: Wed, 10 Mar 2021 22:33:21 +0000

Hiya,

Since I was logged into the github web site (as happens
occasionally but not often) and as requested at the TLS
session, I translated the text below into github issues
in the hope that they might be included in discussion.
Links to each below.

On 28/02/2021 17:34, Stephen Farrell wrote:
> 
> Hiya,
> 
> I've just got my OpenSSL code "working" for draft-09.
> The s_client and s_server talk to one another and do
> ECH; NSS's tstclnt talks to my s_server and does ECH
> and my s_client talks to cloudflare's test server and
> does ECH. So this can be made work, which is the good
> news. (Thanks to Martin Thomson and Chris Patton for
> help in getting that done btw.)
> 
> Note that the scare quotes on "working" above are
> because the code I have now is horrible and needs a
> lot of re-factoring but I'm not keen on doing that
> yet if we're considering further breaking changes
> that might add more horror;-)
> 
> I have the following specific comments having done
> this:
> 

https://github.com/tlswg/draft-ietf-tls-esni/issues/401

> 
> - Including the client-sender's ephemeral public key in
>    the AAD means clients cannot use a single-shot HPKE API.
>    That then means that the client key pair has to be
>    separately generated and held onto until HPKE encryption.
>    It'd be a good bit better and simpler were that not the
>    case and a single shot HPKE encrypt API could be used
>    witout exposing that key pair to the stack.
>      - Possible fix: having a value linked to the inner
>        plaintext could suffice maybe?

https://github.com/tlswg/draft-ietf-tls-esni/issues/397

> 
> - The SH.random magic confirmation value interacts with
>    ticket handling on the server in some ways I don't yet
>    understand fully. Messing with the lower 8 octets there
>    also breaks internal OpenSSL APIs for handling packets
>    but so do other parts of ECH, so that might be fixable.
>      - Possible fix: re-consider the use of SH.random as a
>        signal - ECH sticks out in the outer CH anyway, so
>        maybe return an ECH in the SH with a value that does
>        the same trick so the observer can't tell from the
>        SH if it was a successful use of ECH, grease or a
>        failure in ECH.

https://github.com/tlswg/draft-ietf-tls-esni/issues/399

>        outer" within the extension encoding.

https://github.com/tlswg/draft-ietf-tls-esni/issues/398

>        ECH never being constant time?

https://github.com/tlswg/draft-ietf-tls-esni/issues/400

>      - Possible fix: remove that MUST.

https://github.com/tlswg/draft-ietf-tls-esni/issues/396

Cheers,
S.

> 
> - I've not implemented padding, nor attempted to make
>    greasing as complex as called for, so don't yet have
>    comments on those features as they are in -09.
> 
> There are probably other times when coding this up where
> I swore at the spec text but I've forgotten so maybe those
> weren't so bad in the end:-)
> 
> Cheers,
> S.
> 
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls
>