commit ea9a0d5313f4244f2765e02d762788c1cb9be72a
author David Benjamin <davidben@google.com> Fri Jul 08 15:52:59 2016 -0700
committer David Benjamin <davidben@google.com> Tue Jul 12 16:32:31 2016 +0000
tree d5100ecccf1f97805c7755ae95dcc057725fb2b9
parent d246b817515b52b77ccc4876f25ddf4f41e67477

Refine SHA-1 default in signature algorithm negotiation.

Rather than blindly select SHA-1 if we can't find a matching one, act as
if the peer advertised rsa_pkcs1_sha1 and ecdsa_sha1. This means that we
will fail the handshake if no common algorithm may be found.

This is done in preparation for removing the SHA-1 default in TLS 1.3.

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

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index f1551c8..c05bc4f 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2586,7 +2586,7 @@
   return 1;
 }
 
-uint16_t tls1_choose_signature_algorithm(SSL *ssl) {
+int tls1_choose_signature_algorithm(SSL *ssl, uint16_t *out) {
   CERT *cert = ssl->cert;
   int type = ssl_private_key_type(ssl);
   size_t i, j;
@@ -2595,9 +2595,11 @@
    * handshake. It is fixed at MD5-SHA1 for RSA and SHA1 for ECDSA. */
   if (ssl3_protocol_version(ssl) < TLS1_2_VERSION) {
     if (type == EVP_PKEY_RSA) {
-      return SSL_SIGN_RSA_PKCS1_MD5_SHA1;
+      *out = SSL_SIGN_RSA_PKCS1_MD5_SHA1;
+    } else {
+      *out = SSL_SIGN_ECDSA_SHA1;
     }
-    return SSL_SIGN_ECDSA_SHA1;
+    return 1;
   }
 
   const uint16_t *sigalgs = kDefaultSignatureAlgorithms;
@@ -2608,24 +2610,35 @@
     sigalgs_len = cert->sigalgs_len;
   }
 
+  const uint16_t *peer_sigalgs = cert->peer_sigalgs;
+  size_t peer_sigalgs_len = cert->peer_sigalgslen;
+  if (peer_sigalgs_len == 0) {
+    /* If the client didn't specify any signature_algorithms extension then
+     * we can assume that it supports SHA1. See
+     * http://tools.ietf.org/html/rfc5246#section-7.4.1.4.1 */
+    static const uint16_t kDefaultPeerAlgorithms[] = {SSL_SIGN_RSA_PKCS1_SHA1,
+                                                      SSL_SIGN_ECDSA_SHA1};
+    peer_sigalgs = kDefaultPeerAlgorithms;
+    peer_sigalgs_len =
+        sizeof(kDefaultPeerAlgorithms) / sizeof(kDefaultPeerAlgorithms);
+  }
+
   for (i = 0; i < sigalgs_len; i++) {
-    for (j = 0; j < cert->peer_sigalgslen; j++) {
-      uint16_t signature_algorithm = cert->peer_sigalgs[j];
+    for (j = 0; j < peer_sigalgs_len; j++) {
+      uint16_t signature_algorithm = peer_sigalgs[j];
       /* SSL_SIGN_RSA_PKCS1_MD5_SHA1 is an internal value and should never be
        * negotiated. */
       if (signature_algorithm != SSL_SIGN_RSA_PKCS1_MD5_SHA1 &&
           signature_algorithm == sigalgs[i] &&
           tls12_get_pkey_type(signature_algorithm) == type) {
-        return signature_algorithm;
+        *out = signature_algorithm;
+        return 1;
       }
     }
   }
 
-  /* If no suitable digest may be found, default to SHA-1. */
-  if (type == EVP_PKEY_RSA) {
-    return SSL_SIGN_RSA_PKCS1_SHA1;
-  }
-  return SSL_SIGN_ECDSA_SHA1;
+  OPENSSL_PUT_ERROR(SSL, SSL_R_NO_COMMON_SIGNATURE_ALGORITHMS);
+  return 0;
 }
 
 int tls1_channel_id_hash(SSL *ssl, uint8_t *out, size_t *out_len) {