Support intermediate certificates for OpenSSL client auth.

Support intermediate certificates for OpenSSL client auth.

Change the client auth callback to the 'cert_cb' callback which leaves the
callback in charge of installing the certificate, private key, etc. This lets
us attach a certificate chain.

BUG=393323
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289834

[davidben, 2014-08-13 23:20:05]

Turns out I was wrong and our Mac code actually does support intermediates, so
we do want this working for the OpenSSL cutover. Also turns out upstream added a
'cert_cb' callback that's probably more convenient than having the callback have
a complicated set of return codes and the like. Apparently I didn't notice it
when looking at this previously.

https://boringssl.googlesource.com/boringssl/+/master/ssl/s3_clnt.c#2672

[davidben, 2014-08-13 23:21:50]

Tested manually with certificates generated from
net/data/ssl/scripts/generate-client-certificates.sh. (Did client_[12].pk2 never
get checked in? I couldn't find C.pem in there.)

[Ryan Sleevi, 2014-08-14 01:27:18]

Gonna defer to Adam on this one. Looks right to me, but he's more familiar with
this.

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
File net/socket/ssl_client_socket_openssl.cc (right):

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.cc:60: void FreeX509Stack(STACK_OF(X509) *
ptr) {
STACK_OF(X509)* right?

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.cc:1492: return 1;
Does this guarantee that the sockets are cleared (e.g. in the event of
renegotiation)?

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
File net/socket/ssl_client_socket_openssl.h (right):

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.h:30: struct stack_st_X509;
Unnecessary?

[davidben, 2014-08-14 16:50:16]

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
File net/socket/ssl_client_socket_openssl.cc (right):

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.cc:60: void FreeX509Stack(STACK_OF(X509) *
ptr) {
On 2014/08/14 01:27:17, Ryan Sleevi wrote:
> STACK_OF(X509)* right?

Done.

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.cc:1492: return 1;
On 2014/08/14 01:27:18, Ryan Sleevi wrote:
> Does this guarantee that the sockets are cleared (e.g. in the event of
> renegotiation)?

No, although the old one didn't either. (The old one wouldn't even run as soon
as you'd previously set a certificate.) I've put an SSL_certs_clear at the top
to reset it, although I don't think it can matter much with this code because
ssl_config_.client_cert never changes. Maybe if the key no longer exists and we
want to fail earlier...

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
File net/socket/ssl_client_socket_openssl.h (right):

https://codereview.chromium.org/441823002/diff/20001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.h:30: struct stack_st_X509;
On 2014/08/14 01:27:18, Ryan Sleevi wrote:
> Unnecessary?

Removed. (Remnant of previous iteration.)

[agl, 2014-08-14 17:06:46]

lgtm

https://codereview.chromium.org/441823002/diff/40001/net/socket/ssl_client_so...
File net/socket/ssl_client_socket_openssl.cc (right):

https://codereview.chromium.org/441823002/diff/40001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.cc:179: CHECK(socket);
could probably be a DCHECK.

[davidben, 2014-08-14 17:50:07]

https://codereview.chromium.org/441823002/diff/40001/net/socket/ssl_client_so...
File net/socket/ssl_client_socket_openssl.cc (right):

https://codereview.chromium.org/441823002/diff/40001/net/socket/ssl_client_so...
net/socket/ssl_client_socket_openssl.cc:179: CHECK(socket);
On 2014/08/14 17:06:46, agl wrote:
> could probably be a DCHECK.

Done. (*shrug* I suppose it'd crash immediately afterwards anyway.)

[davidben, 2014-08-14 17:50:15]

The CQ bit was checked by davidben@chromium.org

[commit-bot: I haz the power, 2014-08-14 17:52:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/davidben@chromium.org/441823002/60001

[commit-bot: I haz the power, 2014-08-15 09:38:32]

Committed patchset #4 (60001) as 289834