tlswg / draft-ietf-tls-esni

TLS Encrypted Client Hello
https://tlswg.github.io/draft-ietf-tls-esni/#go.draft-ietf-tls-esni.html
Other
229 stars 58 forks source link

Define the Padding message #457

Closed cjpatton closed 10 months ago

cjpatton commented 3 years ago

Addresses #264. (Alternative to #313.)

sftcd commented 3 years ago

Hiya,

I'm not sure this is needed, nor whether this draft would be the right place for it, if it is needed.

Implementations already support padding certs and cert verify (well, I assume so, OpenSSL does anyway), so I'm unsure a new generic message is a sufficient improvement to justify the work.

Secondly, this isn't really (only) related to ECH so if it turns out to be a good idea, it might deserve it's own spec. We seem to be discovering new issues related to padding via ECH, so might discover yet more if e.g. padding and improved resistance to traffic analysis had it's own draft.

Cheers, S.

davidben commented 3 years ago

The reason to switch from the existing record-layer padding is covered in #264. There was also an email to the list in https://mailarchive.ietf.org/arch/msg/tls/tEbTOI7fz2KnHFEolSs7yAtl2AY/ and I believe it was discussed at some meeting. @chris-wood would remember the details better, but I believe we had consensus on some form of padding in the handshake? Just actually working it out was queued behind the various other issues to be solved.

As for something in ECH or more generic, it's true that padding may be relevant in other contexts, but the way the protocol works, we cannot introduce new mechanisms without some way to negotiate them. That would break compatibility. This can be smoothed over at some future TLS 1.4, where we can bake in the tweaks we want, but for now we need a way to signal it.

We can always choose to signal it with some completely separate code point later (if ech || new_thing || tls14 { DoPaddingThing() }). But while the use case is ECH, I think it is simplest to signal it based on ECH itself. Otherwise, implementations will need to pick up more complexity (#401) to handle what happens if the peer negotiates ECH without padding.

sftcd commented 3 years ago

On 11/06/2021 23:00, David Benjamin wrote:

The reason to switch from the existing record-layer padding is covered in #264. There was also an email to the list in https://mailarchive.ietf.org/arch/msg/tls/tEbTOI7fz2KnHFEolSs7yAtl2AY/

I guess QUIC may be a valid justification.

and I believe it was discussed at some meeting. @chris-wood would remember the details better, but I believe we had consensus on some form of padding in the handshake? Just actually working it out was queued behind the various other issues to be solved.

As for something in ECH or more generic, it's true that padding may be relevant in other contexts, but the way the protocols, we cannot introduce new mechanisms without some way to negotiate them.

I don't think that's really an issue here. I've been padding the cert & cert-verify with the existing padding mechanism and I don't think that causes any problems.

Partly, I'm worried that adding a new thing like this might add more delay to what's already taking a very long time. And yet we seem to only be ending up with slightly better in terms of not sticking out and not leaking via lengths (e.g. the length of the ServerHello key share can leak a little).

Is that really worth the delay or would we be better to get this out the door sooner and gain from more real experience? (I guess you can easily guess my personal bias on that:-)

S

That would break compatibility. This can be smoothed over at some future TLS 1.4, where we can bake in the tweak we want, but for now we need a way to signal it.

We can always choose to signal it with some completely separate code point later (if ech || new_thing || tls14 { DoPaddingThing() }). But while the use case is ECH, I think it is simplest to signal it based on ECH itself. Otherwise, implementations will need to pick up more complexity (#401) to handle what happens if the peer negotiates ECH without this padding scheme.

cjpatton commented 3 years ago

@sftct, a big reason for filing this PR is that this is an alternative to #313 that we've been kicking around for a while, and I wanted to get it down on paper so that we can have something concrete to discuss. What's clear is that to solve #264 without rewriting a bunch of code, we need a way of adding padding via the handshake. Both this PR and #313 do this; this PR is intended to solve some problems with #313.

sftcd commented 3 years ago

On 11/06/2021 23:31, Christopher Patton wrote:

@sftct, a big reason for filing this PR is that this is an alternative to #313 that we've been kicking around for a while, and I wanted to get it down on paper so that we can have something concrete to discuss. What's clear is that to solve #264 without rewriting a bunch of code, we need a way of adding padding via the handshake. Both this PR and #313 do this; this PR is intended to solve some problems with #313.

I'll have to check those issues tomorrow sorry but just to note that using the existing padding with an OpenSSL server very simple - one callback that sets lengths was all it needed for s_server [1] and it'd be the same for any server.

I'd like to understand how this proposal is really better, as I'm fairly sure adding a new message will require mucking about with the state machine which is harder and much much more error prone and harder to test properly.

Ta, S.

[1] https://github.com/sftcd/openssl/blob/ECH_UPFRONT_DEC/apps/s_server.c#L100

chris-wood commented 3 years ago

@cjpatton can you please rebase? (Sorry for the conflicts!)

cjpatton commented 3 years ago

Rebased (still need to address the design comments).

cjpatton commented 3 years ago

Some reorganization is probably in order.

Yeah it's awkward :grimacing: I was trying to avoid refactoring too much with this PR. What do you think about moving all of the padding bits into a single section? Something like:

# Handshake Padding

## ClientHelloInner Padding

[@davidben's scheme in #443].

## Padding the Rest of the Handshake

[Defines the Padding message and how it's used.]

## Recommended Padding Schemes

### Client

[Recommend a way to compute the padding length for EncodedClientHelloInner and the client's Padding message]

### Backend Server

[Recommend a way to compute the padding length for the server's Padding message]
martinthomson commented 3 years ago

That would certainly be better. I would put that down the document, with forward references from the other sections.

cjpatton commented 3 years ago

Squashed. (@martinthomson, I'm gonna wait on editorial changes until #443 lands.)

cjpatton commented 3 years ago

Rebased.

ekr commented 3 years ago

While it seems to me that none of the options are ideal, I'm not that enthusiastic about a new message here.

I appreciate that putting the padding at the record layer adds some API complexity, but it seems modest. I note that that would also match the layering of having the ClientHelloInner padding existing outside the handshake message.

Note that if it turns out that this is impractical, we can always add a padding message later with a new extension.

ekr commented 3 years ago

Sorry about the close. Pushed the wrong button.

sftcd commented 3 years ago

On 02/07/2021 23:33, ekr wrote:

While it seems to me that none of the options are ideal, I'm not that enthusiastic about a new message here.

I appreciate that putting the padding at the record layer adds some API complexity, but it seems modest. I note that that would also match the layering of having the ClientHelloInner padding existing outside the handshake message.

Note that if it turns out that this is impractical, we can always add a padding message later with a new extension.

+1, I think I could live with the new message but still consider it more work than worthwhile at this point.

S.

cjpatton commented 3 years ago

@ekr, @sftcd, see #264 for why record layer pasdding is impractical. An alternative to the padding message is #313, which moves the padding to an extension in EncryptedExtensions.

ekr commented 3 years ago

I've read #264. I don't think it's persuasive for the reasons I mentioned above.

chris-wood commented 3 years ago

+1 to @ekr's suggestion to move this to a separate draft. We don't need to block on it since we have record-layer padding, and we can always add it later.

davidben commented 3 years ago

A separate draft means that QUIC implementations of ECH need to handle cases where the client does and doesn't support this padding mechanism. That means a QUIC ECH deployment cannot rely on the preferred padding mechanism.

The record-level padding is more than just an API issue. QUIC's record layer is not a reliable transport. In order to properly hide lengths, you would need to handle retransmits as if the message were longer. That means padding needs to be integrated with retransmission logic, yet it is not sequenced like CRYPTO frames are. This gets extra fun if TLS believes it wants padding greater than a packet. (Consider, especially, whether an attacker can probe this length by manipulating which packets you see. The receiver doesn't know to block on an all-padding packet.) In contrast, something like this PR is embedded into the byte stream and we get the correct behavior for free.

The API issue has also only gotten worse over time, now that QUIC has been standardized and implementations treat their APIs as more and more stable. Indeed all this started because I went to get the needed support into QUIC APIs and implementation ahead of time. The overwhelming feedback I got from the QUIC side was that this was unreasonable, and the TLS WG agreed (https://github.com/tlswg/draft-ietf-tls-esni/issues/264#issuecomment-686645194). As noted in #264, HTTP/3 does something similar, and introduces a reliable padding scheme, separate from the transport's unreliable one.

I note that that would also match the layering of having the ClientHelloInner padding existing outside the handshake message.

Not all places outside the handshake message are equal. The EncodedClientHelloInner padding doesn't have any of the above issues because its bytes are ordered. It also doesn't cross a protocol boundary. Indeed, this PR also meets this analogy by being outside the messages that we are trying to pad. That analogy mostly disqualifies sticking the padding into EncryptedExtensions, since that's deeply tied with the rest of message generation.

It is inside the transcript, but @martinthomson reported that'd be easier for NSS. An earlier incarnation of the Padding message idea was perhaps even closer to this analogy by mirroring ChangeCipherSpec: receiver ignores it and never passes it up to the handshake, but it's still sequenced with the other messages. (And, given how DTLS integrates retransmission and handshake state machine, integrating it into the handshake is probably indeed easier to reason about. Though I think sequence numbers are probably sufficient? DTLS, like QUIC, would need to integrate padding with retransmission.)

It also matches what we'd likely do in QUIC, had we decided earlier to outsource this kind of padding. To integrate it into the CRYPTO stream, we'd probably divide the CRYPTO stream into ordered data and padding chunks. Well, the stream is already divided into chunks (messages), so the natural answer is to use that layer. That's a Padding message.

cjpatton commented 10 months ago

Closing per decision at IETF 118 to close the issue.