1287267 - TLS 1.3: Flush handshake messages before sending alert

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Eric Rescorla (:ekr)]

In DTLS 1.3 it's possible to have situations where you

Comment 1

[Eric Rescorla (:ekr)]

Where you stage the ServerHello but don't flush it and then get an alert (e.g., CertificateVerify processing), but it's encrypted. We need to either detect the problem earlier or flush the ServerHello

Comment 2

[Martin Thomson [:mt:]]

http://codereview.appspot.com/305130043

ekr, you were right that this wasn't trivial.  What I discovered here was that we had a bunch of calls to FATAL_ERROR in tls13_SendXXX functions.  Those trigger a fatal assert because the SendXXX methods have the xmit lock open and SSL3_SendAlert insists that it gets the lock itself.

I just removed the FATAL_ALERT calls so that we could fall back to the FATAL_ALERT calls that are outside the locks that we take around tls13_SendServerHelloSequence.

tls13_SendClientSecondRound() has a simpler structure, it now just sends the alert in its "loser" cleanup block.  I fixed a double lock release in there at the same time.

I don't know if I got all cases where we were sending alerts while holding the xmit lock, but this is probably a step in the right direction.

Comment 3

[Eric Rescorla (:ekr)]

Not happy about removing all the fatal alerts because they also tell us on stderr what happened. See review

Comment 4

[Martin Thomson [:mt:]]

https://hg.mozilla.org/projects/nss/rev/17b272d7e4fd