Mac SSL fix: Go back to not enabling break-on-auth when we have a client cert to send.

Mac SSL fix: Go back to not enabling break-on-auth when we have a client cert to send.
BUG=38905
TEST=None (don't have a way for me to reproduce this yet)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=43131

[Jens Alfke, 2010-03-29 20:41:13]

It looks like the change to prevent early sending of the client cert, triggered
a SecureTransport bug that broke connection to some sites. This patch backs out
the new behavior (without literally backing out the entire previous patch, since
it contained other useful changes.)

[wtc, 2010-03-29 23:20:39]

LGTM.  The two old SetClientCert() calls should be removed
(they're redundant but most likely harmless).

Can we call EnableBreakOnAuth(true) and SetClientCert() at the
same time?  That might fix this bug.

http://codereview.chromium.org/1514004/diff/1/2
File net/socket/ssl_client_socket_mac.cc (left):

http://codereview.chromium.org/1514004/diff/1/2#oldcode784
net/socket/ssl_client_socket_mac.cc:784: status = EnableBreakOnAuth(true);
We should try calling EnableBreakOnAuth() and
SetClientCert().

http://codereview.chromium.org/1514004/diff/1/2
File net/socket/ssl_client_socket_mac.cc (right):

http://codereview.chromium.org/1514004/diff/1/2#newcode787
net/socket/ssl_client_socket_mac.cc:787: 
Nit: remove this blank line.

http://codereview.chromium.org/1514004/diff/1/2#newcode1039
net/socket/ssl_client_socket_mac.cc:1039: OSStatus status = SetClientCert();
You can remove this SetClientCert() call now.

http://codereview.chromium.org/1514004/diff/1/2#newcode1085
net/socket/ssl_client_socket_mac.cc:1085: status = SetClientCert();
You can remove this SetClientCert() call now.

[Jens Alfke, 2010-03-29 23:48:09]

http://codereview.chromium.org/1514004/diff/1/2
File net/socket/ssl_client_socket_mac.cc (left):

http://codereview.chromium.org/1514004/diff/1/2#oldcode784
net/socket/ssl_client_socket_mac.cc:784: status = EnableBreakOnAuth(true);
On 2010/03/29 23:20:40, wtc wrote:
> We should try calling EnableBreakOnAuth() and
> SetClientCert().

I just tried it right now — the result doesn't look any different from the
current trunk; that is, we still get the client-cert-requested message.
I don't know how to look at the actual SSL messages. Do you know how, Wan-Teh?

[Jens Alfke, 2010-03-30 18:03:22]

You were right, Wan-Teh: setting the cert up-front while leaving on
break-on-auth makes it work on 10.6 as well as 10.5. (I've just tested on my
personal MBP.)

[wtc, 2010-03-30 19:17:52]

LGTM.  I'm glad you found a simple solution.

http://codereview.chromium.org/1514004/diff/6001/7001
File net/socket/ssl_client_socket_mac.cc (right):

http://codereview.chromium.org/1514004/diff/6001/7001#newcode785
net/socket/ssl_client_socket_mac.cc:785: status = SetClientCert();
The other two SetClientCert() calls are now redundant and
should be removed.

http://codereview.chromium.org/1514004/diff/6001/7001#newcode792
net/socket/ssl_client_socket_mac.cc:792: // Only enable session resumption if
break-on-auth is available,
You changed the sense of this sentence (from "is not available"
to "is available", so the following words in subsequent
sentences need to be changed as well:

with break-on-auth => without break-on-auth
in this situation => when break-on-auth is not available

Alternatively, change back to the original comment.