Bug 1168425 - Reenable NSS_STRICT_SHUTDOWN and leak checking for gtests

Bug 1168425 - Reenable NSS_STRICT_SHUTDOWN and leak checking for gtests

[ekr-rietveld, 2016-04-25 20:14:42]

Can you explain more what the session cache change does?

https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3con.c
File lib/ssl/ssl3con.c (right):

https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3con.c#newcode8895
lib/ssl/ssl3con.c:8895: }
This comment is very confusing.

https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3ext.c
File lib/ssl/ssl3ext.c (right):

https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3ext.c#newcode2612
lib/ssl/ssl3ext.c:2612: PORT_Free(ss->ssl3.hs.clientSigAndHash);
This happens when you are rehandshaking?

https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3ext.c#newcode2613
lib/ssl/ssl3ext.c:2613: ss->ssl3.hs.clientSigAndHash = NULL;
This is a dead assignment because you assign in 2616

[ttaubert, 2016-04-25 20:26:17]

On 2016/04/25 20:14:42, ekr-webrtc wrote:
> Can you explain more what the session cache change does?

To understand the issue better it probably helps to see which test triggers the
leak, it's only this one:

TEST_P(TlsConnectStreamPre13, ConnectAndClientRenegotiate) {
  Connect();
  server_->PrepareForRenegotiate();
  client_->StartRenegotiate();
  Handshake();
  CheckConnected();
}

After connecting, we have a renegotiation initiated by the client which sends an
empty SID. From the initial handshake |ss->sec.ci.sid| is still set but will be
overwritten without freeing it first.

> https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3con.c
> File lib/ssl/ssl3con.c (right):
> 
> https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3con.c#newcode8895
> lib/ssl/ssl3con.c:8895: }
> This comment is very confusing.

Wanted to at least provide some explanation. Can try to improve it though.

> https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3ext.c
> File lib/ssl/ssl3ext.c (right):
> 
> https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3ext.c#newcode2612
> lib/ssl/ssl3ext.c:2612: PORT_Free(ss->ssl3.hs.clientSigAndHash);
> This happens when you are rehandshaking?

Yes (see test above).

> https://codereview.appspot.com/299800043/diff/1/lib/ssl/ssl3ext.c#newcode2613
> lib/ssl/ssl3ext.c:2613: ss->ssl3.hs.clientSigAndHash = NULL;
> This is a dead assignment because you assign in 2616

Thought about removing it and wasn't sure if it's cleaner to leave it. Will
remove.

[ekr-rietveld, 2016-04-25 20:41:43]

So to be clear, this is renegotiation without resumption?

[ttaubert, 2016-04-26 08:28:51]

On 2016/04/25 20:41:43, ekr-webrtc wrote:
> So to be clear, this is renegotiation without resumption?

Yes, the client renegotiates and sends an empty SID.

[ekr-rietveld, 2016-04-26 12:31:53]

Please fix the comment I called out. Otherwise LGTM