Closed Bug 1235366 Opened 5 years ago Closed 5 years ago

Assertion failure: ss->ssl3.hs.canFalseStart, at c:/Users/mozilla/debug-builds/m ozilla-central/security/nss/lib/ssl/sslsecur.c:1325

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: cbook, Assigned: mt)

References

()

Details

(Keywords: assertion)

Attachments

(1 file)

Assertion failure: ss->ssl3.hs.canFalseStart, at c:/Users/mozilla/debug-builds/m
ozilla-central/security/nss/lib/ssl/sslsecur.c:1325

Found via bughunter and reproduced on a win7 trunk debug build based on tip. 

Steps to reproduce:
-> Load http://www.wedj.com
(you might need to reload the page a few times)
---> Assertion failure 

filing as s-s bug, just in case
Attached file bughunter crash stack
Group: core-security → dom-core-security
Assignee: nobody → nobody
Group: dom-core-security → crypto-core-security
Component: Security → Libraries
Flags: needinfo?(dkeeler)
Product: Core → NSS
Version: 43 Branch → trunk
:mt would probably be able to get to the bottom of this faster than I.
Flags: needinfo?(dkeeler) → needinfo?(martin.thomson)
Reproduced after a few refreshes.

I don't know why, but this is passing the bit in the function where the handshake is checked.  That succeeds according to what I'm getting in the debugger.

This happens because the site is very slow to send a Finished after sending ChangeCipherSpec.  I'm clocking about 200ms, which is huge given that normally these have only milliseconds between them.  If Firefox decides to send a request in this interval we reach one of two states:

A. handshake not done, not in false start, handshake function returned successfully
B. handshake not done, not in false start, no more handshake functions to call

In other words, this block runs fine: https://dxr.mozilla.org/mozilla-central/rev/1ec3a3ff68f2d1a54e6ed33e926c28fee286bdf1/security/nss/lib/ssl/sslsecur.c#1296-1299

A doesn't seem right by my reading, because the only success return for ssl_GatherRecord1stHandshake/ssl3_GatherCompleteHandshake is if the handshake is complete.

But B doesn't seem right either because the only place we set the handshake functions to NULL is when we receive a Finished.  I'm a little concerned that we are reading ss->handshake under a different lock to where it is set in ssl3_HandleFinished, but this is all single-threaded in Firefox.

In the end, we are not in false start, but the handshake functions have reported success to the affected code.  It proceeds with the write, noticing that the handshake isn't marked as done, checks for false start, and explodes when it doesn't find it.

In a non-debug build, we will be sending to servers for which we haven't checked a Finished.  If we want to paper over this, we could buffer the write if we aren't done yet.  But I want to keep digging for why this is acting like it is.  I'll see if I can get a more reliable reproduction in a unit test.
Flags: needinfo?(martin.thomson)
This isn't a security issue.  The worst that happens is that for a false-start enabled server, we send data between CCS and Finished, despite having this assertion that doesn't expect that.

The process by which this state is reached is a little convoluted, but it is reproducible.

1. With false start fully enabled, ss->handshake is set to NULL once
the client processes ServerHelloDone.

https://dxr.mozilla.org/mozilla-central/rev/e0bcd16e1d4b99ba3e542149d0d41e0f60c54b5c/security/nss/lib/ssl/sslcon.c#1240

2. Normal packet processing will trigger ssl3_GatherAppDataRecord()
which will process the remaining CCS and Finished messages as they
arrive.

3. Processing CCS on its own, in this fashion will result in
ssl3_WaitingForStartOfServerSecondRound() returning false.

https://dxr.mozilla.org/mozilla-central/rev/1ec3a3ff68f2d1a54e6ed33e926c28fee286bdf1/security/nss/lib/ssl/ssl3con.c#7432

4. As a result, after processing CCS, ssl3_GatherCompleteHandshake()
sets ss->ssl3.hs.canFalseStart to false.

https://dxr.mozilla.org/mozilla-central/rev/1ec3a3ff68f2d1a54e6ed33e926c28fee286bdf1/security/nss/lib/ssl/ssl3gthr.c#419

5. If a new write is called before the Finished message arrives, we
skip the handshake processing here, because ss->handshake is NULL (not
because falseStart is true):

https://dxr.mozilla.org/mozilla-central/rev/1ec3a3ff68f2d1a54e6ed33e926c28fee286bdf1/security/nss/lib/ssl/sslsecur.c#1296-1299

6. Which leads to an assertion failure here because, although
everything is fine, we aren't done with the handshake, and
ss->ssl3.hs.canFalseStart is false.

https://dxr.mozilla.org/mozilla-central/rev/1ec3a3ff68f2d1a54e6ed33e926c28fee286bdf1/security/nss/lib/ssl/sslsecur.c#1325

On rietveld: https://codereview.appspot.com/286790043/
Flags: needinfo?(ekr)
Group: crypto-core-security
https://hg.mozilla.org/projects/nss/rev/3430648027ec
https://hg.mozilla.org/projects/nss/rev/88c7afc06b2e
https://hg.mozilla.org/projects/nss/rev/7d389601644f
Assignee: nobody → martin.thomson
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(ekr)
Resolution: --- → FIXED
Target Milestone: --- → 3.23
You need to log in before you can comment on or make changes to this bug.