1116881 - DTLS doesn't properly handle loss of last flight

Status: NEW

(NSS :: Libraries, defect, P3)

Description

[Eric Rescorla (:ekr)]

DTLS retransmission of the last flight is triggered by retransmission of
the next-to-last flight (from the other side). However, if the last flight
is lost, the sender will have incremented their receiving epoch and will thus
reject the message, leading to deadlock (see http://dxr.mozilla.org/mozilla-central/source/security/nss/lib/ssl/ssl3con.c#11350 and following).

This is a detailed part of the DTLS stack and I want a second opinion before marking it non-security.

Comment 1

[Eric Rescorla (:ekr)]

Attachment: Test that demonstrates the bug

To demonstrate the bug, run only DtlsConnectTest.ConnectDTLSDropServerCCS.

There is some memory issue in the changes to the test harness that creates problems running multiple tests. I will be fixing that in a bit.

Comment 2

[Eric Rescorla (:ekr)]

Martin, can you take a look at this issue and let me know if you think there is a security issue here, or just a functionality issue.

Comment 3

[Eric Rescorla (:ekr)]

One question to ask is if we should just fix the first handshake case. We're generally discouraging rehandshake anyway and I suspect that case is harder to fix.

Comment 4

[Martin Thomson [:mt:]]

This is a straight up bug; nothing here that I can see could be exploited.  Well, other than to produce the effect of the bug, that is.  And that's fairly easy to do.

As for fixing it, I guess you could hook into the existing epoch handling (do we keep keys for the last epoch?).  Given the possibility of renegotiation, you can't just assume cleartext handshake messages, I guess, though you could limit the fix to the initial epoch, I'm not sure that a partial fix is cool.

Comment 5

[Eric Rescorla (:ekr)]

(In reply to Martin Thomson [:mt] from comment #4)
> This is a straight up bug; nothing here that I can see could be exploited. 
> Well, other than to produce the effect of the bug, that is.  And that's
> fairly easy to do.
> 
> As for fixing it, I guess you could hook into the existing epoch handling
> (do we keep keys for the last epoch?).  Given the possibility of
> renegotiation, you can't just assume cleartext handshake messages, I guess,
> though you could limit the fix to the initial epoch, I'm not sure that a
> partial fix is cool.

That seems to be the big question. If we limit ourselves to the initial
epoch, then we don't need two sets of keys around. I'll take a crack
at a patch, but I'd also like to get WTC's opinion about what kind
of fix is needed here. Note that we currently disable renegotiation
for DTLS-SRTP in Firefox.

Comment 6

[Wan-Teh Chang]

Eric, Martin: I will take a look at this bug tomorrow.

Comment 7

[Wan-Teh Chang]

Eric, Martin:

1. It seems that this bug can only cause a handshake or renegotiation
to stall or cause the peer to continue sending application data using
the old key, which will be discarded by us.

2. To fix this bug, I believe we just need to keep the last flight of
messages around until we have successfully decrypted using the new key.
Whenever we receive a record from the old epoch, we retransmit the
last flight of messages without decrypting the received record. So we
don't need to keep the old key around.

Do you think this fix will work?

Comment 8

[Eric Rescorla (:ekr)]

Thanks for your response.

(In reply to Wan-Teh Chang from comment #7)
> Eric, Martin:
> 
> 1. It seems that this bug can only cause a handshake or renegotiation
> to stall or cause the peer to continue sending application data using
> the old key, which will be discarded by us.
> 
> 2. To fix this bug, I believe we just need to keep the last flight of
> messages around until we have successfully decrypted using the new key.
> Whenever we receive a record from the old epoch, we retransmit the
> last flight of messages without decrypting the received record. So we
> don't need to keep the old key around.
> 
> Do you think this fix will work?

That's a really interesting idea. I think it might well work. We could
also potentially improve this by only retransmitting upon receiving
a message of content type handshake, since the other side will eventually
send one if it has not received our Finished and that way we are less
sensitive to out of order packets.

I'l give it some more thought and if it seems good put together a patch
along these lines.

Comment 9

[Martin Thomson [:mt:]]

I'd definitely want to limit it to a retransmit on receipt of handshake messages.  Without something more stateful, receiving application data in large volumes could cause a fairly absurd number of retransmits.

I can't see any real down side here.  We can dispense with the keying material from the last epoch and still avoid the flaw.

Comment 10

[Martin Thomson [:mt:]]

ekr, I looked into this when doing DTLS 1.3, and concluded that it wasn't a problem: the client receives a Finished that is in the same epoch.  I would have to rewrite the test entirely to check for certain.  Do you think that it is still a problem?

Comment 11

[Eric Rescorla (:ekr)]

MT, I think it's not a problem with DTLS 1.3, but it is a problem in DTLS 1.2 AFAIK.