tree 636a226e768551dcf18948fe00cdb20c8cecd136
parent 21a879a78a60c8667468a9eba994c8365eaf92ea
author David Benjamin <davidben@google.com> 1581284971 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> 1581622240 +0000

Require handshake flights end at record boundaries.

The TLS handshake runs over a sequence of messages, serialized onto a
stream, which is then packetized into records, with no correlation to
message boundaries.

TLS messages may span records, so a TLS implementation will buffer up
excess data in a record for the next message. If not checked, that next
message may a round-trip or even a key change later. Carrying data
across a key change has security consequences, so we reject any excess
data across key changes (see ChangeCipherSpec synchronization tests and
(d)tls_set_read_state). However, we do not currently check it across
network round trips that do not change keys.

For instance, a TLS 1.2 client may pack part of ClientKeyExchange (the
first byte, at least, is deterministic) into the ClientHello record,
before even receiving ServerHello. Most TLS implementations will accept
this.

However, the handback logic does *not* serialize excess data in hs_buf.
There shouldn't be any, but if the peer is doing strange things as
above, that data will get silently dropped. The way TLS 1.3 0-RTT
handback logic works (the key isn't installed until after handback),
this data is even silently dropped though there is a key change.

To keep all our behavior consistent, check for unprocessed handshake
data at the end of each flight and reject it. Add a bunch of tests.

Update-Note: If the peer packs data across handshake flights, or packs
HelloRequest into the same record as Finished, this will now be an
error. (The former is pathologically odd behavior. The latter is also
rejected by Schannel and also odd.)

Change-Id: I9412bbdea09ee7fdcfeb78d3456329505a190641
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39987
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>