commit c032dfa27ec8a9f286e3ae7e545932aac13b87d6
author David Benjamin <davidben@google.com> Thu May 12 14:54:57 2016 -0400
committer David Benjamin <davidben@google.com> Fri May 13 20:07:16 2016 +0000
tree 6665271a788a289332981e4975b16fc14403b86e
parent 060cfb091194edf7341d91009764c64096a87157

Client auth is only legal in certificate-based ciphers.

OpenSSL used to only forbid it on the server in plain PSK and allow it on the
client. Enforce it properly on both sides. My read of the rule in RFC 5246 ("A
non-anonymous server can optionally request a certificate") and in RFC 4279
("The Certificate and CertificateRequest payloads are omitted from the
response.") is that client auth happens iff we're certificate-based.

The line in RFC 4279 is under the plain PSK section, but that doesn't make a
whole lot of sense and there is only one diagram. PSK already authenticates
both sides. I think the most plausible interpretation is that this is for
certificate-based ciphers.

Change-Id: If195232c83f21e011e25318178bb45186de707e6
Reviewed-on: https://boringssl-review.googlesource.com/7942
Reviewed-by: David Benjamin <davidben@google.com>

ssl/s3_clnt.c

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 3681590..8e79c81 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -256,7 +256,7 @@
 
       case SSL3_ST_CR_CERT_A:
       case SSL3_ST_CR_CERT_B:
-        if (ssl_cipher_has_server_public_key(ssl->s3->tmp.new_cipher)) {
+        if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
           ret = ssl3_get_server_certificate(ssl);
           if (ret <= 0) {
             goto end;
@@ -289,7 +289,11 @@
         if (ret <= 0) {
           goto end;
         }
-        ssl->state = SSL3_ST_CR_CERT_REQ_A;
+        if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
+          ssl->state = SSL3_ST_CR_CERT_REQ_A;
+        } else {
+          ssl->state = SSL3_ST_CR_SRVR_DONE_A;
+        }
         ssl->init_num = 0;
         break;
 
@@ -857,7 +861,8 @@
   /* If doing a full handshake with TLS 1.2, the server may request a client
    * certificate which requires hashing the handshake transcript under a
    * different hash. Otherwise, the handshake buffer may be released. */
-  if (ssl->hit || ssl3_protocol_version(ssl) < TLS1_2_VERSION) {
+  if (ssl->hit || ssl3_protocol_version(ssl) < TLS1_2_VERSION ||
+      !ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
     ssl3_free_handshake_buffer(ssl);
   }
 
@@ -1203,7 +1208,7 @@
            CBS_len(&server_key_exchange_orig) - CBS_len(&server_key_exchange));
 
   /* ServerKeyExchange should be signed by the server's public key. */
-  if (ssl_cipher_has_server_public_key(ssl->s3->tmp.new_cipher)) {
+  if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
     pkey = X509_get_pubkey(ssl->session->peer);
     if (pkey == NULL) {
       goto err;