From c7817d8ce2f3c90b2de36cd3c654b80ab078abac Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 5 Nov 2015 18:28:33 -0500 Subject: [PATCH] Add SSL_CIPHER_get_min_version and tidy up SSL_TLSV1_2 logic. Later when TLS 1.3 comes around, we'll need SSL_CIPHER_get_max_version too. In the meantime, hide the SSL_TLSV1_2 messiness behind a reasonable API. Change-Id: Ibcc17cccf48dd99e364d6defdfa5a87d031ecf0a Reviewed-on: https://boringssl-review.googlesource.com/6452 Reviewed-by: Adam Langley --- include/openssl/ssl.h | 4 ++++ ssl/internal.h | 21 ++------------------- ssl/s3_clnt.c | 23 +++++++++-------------- ssl/s3_lib.c | 4 ++-- ssl/t1_lib.c | 10 +--------- 5 files changed, 18 insertions(+), 44 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 5b49fdc3..bf53d5e4 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -1067,6 +1067,10 @@ OPENSSL_EXPORT int SSL_CIPHER_is_block_cipher(const SSL_CIPHER *cipher); /* SSL_CIPHER_is_ECDSA returns one if |cipher| uses ECDSA. */ OPENSSL_EXPORT int SSL_CIPHER_is_ECDSA(const SSL_CIPHER *cipher); +/* SSL_CIPHER_get_min_version returns the minimum protocol version required + * for |cipher|. */ +OPENSSL_EXPORT uint16_t SSL_CIPHER_get_min_version(const SSL_CIPHER *cipher); + /* SSL_CIPHER_get_name returns the OpenSSL name of |cipher|. */ OPENSSL_EXPORT const char *SSL_CIPHER_get_name(const SSL_CIPHER *cipher); diff --git a/ssl/internal.h b/ssl/internal.h index cdf85927..aa8c4ef6 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -695,15 +695,6 @@ void ssl_write_buffer_clear(SSL *ssl); /* See if we use signature algorithms extension and signature algorithm before * signatures. */ #define SSL_USE_SIGALGS(s) (s->enc_method->enc_flags & SSL_ENC_FLAG_SIGALGS) -/* Allow TLS 1.2 ciphersuites: applies to DTLS 1.2 as well as TLS 1.2: may - * apply to others in future. */ -#define SSL_USE_TLS1_2_CIPHERS(s) \ - (s->enc_method->enc_flags & SSL_ENC_FLAG_TLS1_2_CIPHERS) -/* Determine if a client can use TLS 1.2 ciphersuites: can't rely on method - * flags because it may not be set to correct version yet. */ -#define SSL_CLIENT_USE_TLS1_2_CIPHERS(s) \ - ((SSL_IS_DTLS(s) && s->client_version <= DTLS1_2_VERSION) || \ - (!SSL_IS_DTLS(s) && s->client_version >= TLS1_2_VERSION)) /* SSL_kRSA <- RSA_ENC | (RSA_TMP & RSA_SIGN) | * <- (EXPORT & (RSA_ENC | RSA_TMP) & RSA_SIGN) @@ -739,17 +730,12 @@ typedef struct cert_st { const SSL_PRIVATE_KEY_METHOD *key_method; /* For clients the following masks are of *disabled* key and auth algorithms - * based on the current session. + * based on the current configuration. * * TODO(davidben): Remove these. They get checked twice: when sending the - * ClientHello and when processing the ServerHello. However, mask_ssl is a - * different value both times. mask_k and mask_a are not, but is a - * round-about way of checking the server's cipher was one of the advertised - * ones. (Currently it checks the masks and then the list of ciphers prior to - * applying the masks in ClientHello.) */ + * ClientHello and when processing the ServerHello. */ uint32_t mask_k; uint32_t mask_a; - uint32_t mask_ssl; DH *dh_tmp; DH *(*dh_tmp_cb)(SSL *ssl, int is_export, int keysize); @@ -857,9 +843,6 @@ struct ssl3_enc_method { #define SSL_ENC_FLAG_SIGALGS 0x2 /* Uses SHA256 default PRF */ #define SSL_ENC_FLAG_SHA256_PRF 0x4 -/* Allow TLS 1.2 ciphersuites: applies to DTLS 1.2 as well as TLS 1.2: - * may apply to others in future. */ -#define SSL_ENC_FLAG_TLS1_2_CIPHERS 0x8 /* lengths of messages */ #define DTLS1_COOKIE_LENGTH 256 diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 5ccce754..04c06ddb 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -604,9 +604,12 @@ static int ssl3_write_client_cipher_list(SSL *ssl, CBB *out) { for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { const SSL_CIPHER *cipher = sk_SSL_CIPHER_value(ciphers, i); /* Skip disabled ciphers */ - if (cipher->algorithm_ssl & ssl->cert->mask_ssl || - cipher->algorithm_mkey & ssl->cert->mask_k || - cipher->algorithm_auth & ssl->cert->mask_a) { + if ((cipher->algorithm_mkey & ssl->cert->mask_k) || + (cipher->algorithm_auth & ssl->cert->mask_a)) { + continue; + } + if (SSL_CIPHER_get_min_version(cipher) > + ssl3_version_from_wire(ssl, ssl->client_version)) { continue; } any_enabled = 1; @@ -741,7 +744,6 @@ int ssl3_get_server_hello(SSL *s) { CBS server_hello, server_random, session_id; uint16_t server_version, cipher_suite; uint8_t compression_method; - uint32_t mask_ssl; n = s->method->ssl_get_message(s, SSL3_ST_CR_SRVR_HELLO_A, SSL3_ST_CR_SRVR_HELLO_B, SSL3_MT_SERVER_HELLO, @@ -834,18 +836,11 @@ int ssl3_get_server_hello(SSL *s) { OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CIPHER_RETURNED); goto f_err; } - /* ct->mask_ssl was computed from client capabilities. Now - * that the final version is known, compute a new mask_ssl. */ - if (!SSL_USE_TLS1_2_CIPHERS(s)) { - mask_ssl = SSL_TLSV1_2; - } else { - mask_ssl = 0; - } /* If the cipher is disabled then we didn't sent it in the ClientHello, so if * the server selected it, it's an error. */ - if ((c->algorithm_ssl & mask_ssl) || - (c->algorithm_mkey & ct->mask_k) || - (c->algorithm_auth & ct->mask_a)) { + if ((c->algorithm_mkey & ct->mask_k) || + (c->algorithm_auth & ct->mask_a) || + SSL_CIPHER_get_min_version(c) > ssl3_version_from_wire(s, s->version)) { al = SSL_AD_ILLEGAL_PARAMETER; OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED); goto f_err; diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 5209802c..7bf223d5 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -503,8 +503,8 @@ const SSL_CIPHER *ssl3_choose_cipher( ok = 1; - /* Skip TLS v1.2 only ciphersuites if not supported */ - if ((c->algorithm_ssl & SSL_TLSV1_2) && !SSL_USE_TLS1_2_CIPHERS(s)) { + /* Check the TLS version. */ + if (SSL_CIPHER_get_min_version(c) > ssl3_version_from_wire(s, s->version)) { ok = 0; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index a6c48f1f..881a15a7 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -169,8 +169,7 @@ const SSL3_ENC_METHOD TLSv1_2_enc_data = { TLS_MD_SERVER_FINISH_CONST,TLS_MD_SERVER_FINISH_CONST_SIZE, tls1_alert_code, tls1_export_keying_material, - SSL_ENC_FLAG_EXPLICIT_IV|SSL_ENC_FLAG_SIGALGS|SSL_ENC_FLAG_SHA256_PRF - |SSL_ENC_FLAG_TLS1_2_CIPHERS, + SSL_ENC_FLAG_EXPLICIT_IV|SSL_ENC_FLAG_SIGALGS|SSL_ENC_FLAG_SHA256_PRF, }; static int compare_uint16_t(const void *p1, const void *p2) { @@ -727,13 +726,6 @@ void ssl_set_client_disabled(SSL *s) { c->mask_a = 0; c->mask_k = 0; - /* Don't allow TLS 1.2 only ciphers if we don't suppport them */ - if (!SSL_CLIENT_USE_TLS1_2_CIPHERS(s)) { - c->mask_ssl = SSL_TLSV1_2; - } else { - c->mask_ssl = 0; - } - /* Now go through all signature algorithms seeing if we support any for RSA, * DSA, ECDSA. Do this for all versions not just TLS 1.2. */ sigalgslen = tls12_get_psigalgs(s, &sigalgs);