1487597 - Early data that arrives before the handshake completes can be read afterwards

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Martin Thomson [:mt:]]

We agreed to not think too hard about early data (0-RTT) that arrives after the handshake completes in DTLS.  Rather than complicate the API we aim to maintain a simple division between "unsafe" 0-RTT and "safe" 1-RTT.  The intent is to have the handshake callback be the bright dividing line.  Reads before this are "at risk of reading replayable or replayed data", and reads after are "secure".

In practice, the code we have doesn't deliver on this promise.  

We currently stop accepting 0-RTT when we process EndOfEarlyData, but we fail to account for data that we accept before this point.  If the client writes records in 0-RTT and the server failed to read all of them before it completes the handshake (using SSL_ForceHandshake() perhaps, which doesn't read).  After the handshake completes, we will still have 0-RTT available for reading.

We push newly arrived records into a queue of pending 0-RTT reads at the server, but don't flush those when we process EndOfEarlyData.  That means that reads after handshake completion will pull from that queue.  The application will believe that these are good.  In some ways this data *is* good, because the handshake completed, but the application will be unaware of the risk that the same data could have been replayed and processed elsewhere and so it might make a bad decision.

I've a test that demonstrates the problem and the fix, which is dead simple.

Marking security-sensitive for the moment out of an abundance of caution.  I don't think that it's serious given the current usage of 0-RTT on the server side, but it doesn't hurt to have the time to check.

Comment 1

[Martin Thomson [:mt:]]

Attachment: Bug 1487597 - Improve 0-RTT data delivery

This just flushes the pending queue of early data when we read
EndOfEarlyData.  We could flush this later, perhaps just before we fire the
handshake callback, which *might* give a DTLS server a little more time to read,
but we expect EOED and Finished to be in the same datagram in most cases, so
we'd only be making things complicated by doing so.

Comment 2

[Martin Thomson [:mt:]]

https://hg.mozilla.org/projects/nss/rev/b56a298b42752de328f18a8514a1db80069b2b16

Note that this doesn't fix the described bug, because we allow early data to be read after handshake completion in DTLS. But TLS maintains that requirement.

Comment 3

[Martin Thomson [:mt:]]

I'm having a great week for this: https://hg.mozilla.org/projects/nss/rev/085cf1773a6c6122995c9b2b0eb0a4dae40a8285