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>
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index a80ae8c..4333ca0 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -1792,7 +1792,10 @@
goto err;
}
- uint16_t signature_algorithm = tls1_choose_signature_algorithm(ssl);
+ uint16_t signature_algorithm;
+ if (!tls1_choose_signature_algorithm(ssl, &signature_algorithm)) {
+ goto err;
+ }
if (ssl3_protocol_version(ssl) >= TLS1_2_VERSION) {
/* Write out the digest type in TLS 1.2. */
if (!CBB_add_u16(&body, signature_algorithm)) {
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 0b9b871..ca253b4 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -1045,7 +1045,10 @@
}
/* Determine the signature algorithm. */
- uint16_t signature_algorithm = tls1_choose_signature_algorithm(ssl);
+ uint16_t signature_algorithm;
+ if (!tls1_choose_signature_algorithm(ssl, &signature_algorithm)) {
+ goto err;
+ }
if (ssl3_protocol_version(ssl) >= TLS1_2_VERSION) {
if (!CBB_add_u16(&body, signature_algorithm)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
diff --git a/ssl/internal.h b/ssl/internal.h
index 03a1320..e451d15 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1215,9 +1215,10 @@
uint32_t ssl_get_algorithm_prf(const SSL *ssl);
int tls1_parse_peer_sigalgs(SSL *ssl, const CBS *sigalgs);
-/* tls1_choose_signature_algorithm returns a signature algorithm for use with
- * |ssl|'s private key based on the peer's preferences the digests supported. */
-uint16_t tls1_choose_signature_algorithm(SSL *ssl);
+/* tls1_choose_signature_algorithm sets |*out| to a signature algorithm for use
+ * with |ssl|'s private key based on the peer's preferences and the digests
+ * supported. It returns one on success and zero on error. */
+int tls1_choose_signature_algorithm(SSL *ssl, uint16_t *out);
size_t tls12_get_psigalgs(SSL *ssl, const uint16_t **psigs);
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) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 60127da..5d5facf 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4876,7 +4876,7 @@
//
// TODO(davidben): Add TLS 1.3 versions of these.
testCases = append(testCases, testCase{
- name: "Agree-Digest-Fallback",
+ name: "NoCommonAlgorithms",
config: Config{
MaxVersion: VersionTLS12,
ClientAuth: RequireAnyClientCert,
@@ -4889,8 +4889,9 @@
"-cert-file", path.Join(*resourceDir, rsaCertificateFile),
"-key-file", path.Join(*resourceDir, rsaKeyFile),
},
- digestPrefs: "SHA256",
- expectedPeerSignatureAlgorithm: signatureRSAPKCS1WithSHA1,
+ digestPrefs: "SHA256",
+ shouldFail: true,
+ expectedError: ":NO_COMMON_SIGNATURE_ALGORITHMS:",
})
testCases = append(testCases, testCase{
name: "Agree-Digest-SHA256",
