commit ea7a88fa5c3841b25cfaa057e023157a54adea46
author David Benjamin <davidben@google.com> Wed Oct 23 23:48:53 2024 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Nov 01 20:55:28 2024 +0000
tree 1844d900086dbd2c99244d559ebfe38c00da44ca
parent 59dd77296d3aa24c8c543e02da0e320bc73a7701

Stage new DTLS 1.3 read epochs until the first record comes in

In DTLS 1.2, we install the new read epoch as soon as we change keys,
and immediately stop accepting data from the previous epoch. This is
fine because there is only one key change, and we don't need to receive
data from the previous epoch at that point. (Even if we used previous
flights to trigger retransmits---which we probably should but don't---I
believe all DTLS 1.2 key changes are such that the previous flight will
contain *some* message from the epoch we expect and we can key on that.)

In DTLS 1.3, things are different. The peer's KeyUpdates do not apply
until we send an ACK, but that ACK might be lost. So that means, at
least for KeyUpdate, we must stage the new epoch and only apply it
later.

During the handshake, this comes up in two cases:

- A server might send ServerHello..Finished but some of ServerHello (or
  the whole flight) is lost. The server is now expecting
  Certificate..Finished (epoch 2), but the client cannot transition to
  epoch 2 without the ServerHello. The client will then try to induce a
  retransmit by either sending an ACK or resending the ClientHello. The
  client must parse epoch 0 to be able to catch this. This is nice
  but not critical, because the server also has a retransmit timer.

- A client might send Certificate..Finished but it was all lost. The
  client is now done with the handshake (epoch 3), but the server
  cannot complete. The server will then retransmit its
  ServerHello..Finished flight or send an ACK. The ACK will actually
  come at epoch 3, so we don't need epoch 2 in that case. But the
  retransmit is purely in epochs 0 and 2, so the client must parse
  epoch 2 to catch this. This is also not critical because the
  client is expected to keep its retransmit timer running after the
  handshake (still TODO).

To support all this, new DTLS 1.3 read epochs are staged until we get a
record.

This means that record processing must now account for receiving records
at more epochs, including some invalid cases. Specifically:

- We might receive new handshake messages at the current epoch, even
  though the current epoch is being closed. This is an error.

- We might receive application data at epoch 2 even though we've
  finished the handshake and expect it at epoch 3. This is an error.

I've added checks for both of these, with tests. This also resolves a
few TODOs. The app data check will also help with 0-RTT, where epochs 1
and 2 will flow concurrently. (We'll still need an inverse check that
epoch 1 never carries handshake data.) It also once again changes the
broken SSL_get_read_sequence API from one kind of broken to another kind
of broken, so I've updated the comment to explain the current state.

As part of this, stop tracking read_level and write_level outside of
QUIC. It's only used in QUIC and the semantics are a little weird when
we defer activating new read epochs.

(I'm not sure whether this behavior is really necessary for epochs
before epoch 3. The only case it accomodates is the client using epoch 0
to induce a retransmit when ServerHello is lost. The impact it has on
SSL_get_read_sequence immediately after the handshake is kind of weird,
so maybe we want to special case it a bit. I can't think of any case
where we'd want to defer installing epoch 3.)

Bug: 42290594
Change-Id: Id222de2b77ff8e68e64f301f32d8c2b95ad3e7a7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72450
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>