diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index e8ec98c221..d1497b115b 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -4287,15 +4287,15 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL_CONNECTION *s, STACK_OF(SSL_CIPHER) *cl } for (i = 0; i < sk_SSL_CIPHER_num(prio); i++) { + int minversion, maxversion; + c = sk_SSL_CIPHER_value(prio, i); + minversion = SSL_CONNECTION_IS_DTLS(s) ? c->min_dtls : c->min_tls; + maxversion = SSL_CONNECTION_IS_DTLS(s) ? c->max_dtls : c->max_tls; /* Skip ciphers not supported by the protocol version */ - if (!SSL_CONNECTION_IS_DTLS(s) && - ((s->version < c->min_tls) || (s->version > c->max_tls))) - continue; - if (SSL_CONNECTION_IS_DTLS(s) && - (DTLS_VERSION_LT(s->version, c->min_dtls) || - DTLS_VERSION_GT(s->version, c->max_dtls))) + if (ssl_version_cmp(s, s->version, minversion) < 0 + || ssl_version_cmp(s, s->version, maxversion) > 0) continue; /* diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index 164ec57b2e..4e99ada22c 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2629,6 +2629,7 @@ __owur int ssl3_handshake_write(SSL_CONNECTION *s); __owur int ssl_allow_compression(SSL_CONNECTION *s); +__owur int ssl_version_cmp(const SSL_CONNECTION *s, int versiona, int versionb); __owur int ssl_version_supported(const SSL_CONNECTION *s, int version, const SSL_METHOD **meth); diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index caf9d7d11c..922f8a1119 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -4119,15 +4119,12 @@ int ssl_cipher_list_to_bytes(SSL_CONNECTION *s, STACK_OF(SSL_CIPHER) *sk, /* Sanity check that the maximum version we offer has ciphers enabled */ if (!maxverok) { - if (SSL_CONNECTION_IS_DTLS(s)) { - if (DTLS_VERSION_GE(c->max_dtls, s->s3.tmp.max_ver) - && DTLS_VERSION_LE(c->min_dtls, s->s3.tmp.max_ver)) - maxverok = 1; - } else { - if (c->max_tls >= s->s3.tmp.max_ver - && c->min_tls <= s->s3.tmp.max_ver) - maxverok = 1; - } + int minproto = SSL_CONNECTION_IS_DTLS(s) ? c->min_dtls : c->min_tls; + int maxproto = SSL_CONNECTION_IS_DTLS(s) ? c->max_dtls : c->max_tls; + + if (ssl_version_cmp(s, maxproto, s->s3.tmp.max_ver) >= 0 + && ssl_version_cmp(s, minproto, s->s3.tmp.max_ver) <= 0) + maxverok = 1; } totlen += len; diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 5693a1269d..16b5f590a0 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -156,17 +156,12 @@ int tls_setup_handshake(SSL_CONNECTION *s) /* Sanity check that we have MD5-SHA1 if we need it */ if (sctx->ssl_digest_methods[SSL_MD_MD5_SHA1_IDX] == NULL) { - int md5sha1_needed = 0; + int negotiated_minversion; + int md5sha1_needed_maxversion = SSL_CONNECTION_IS_DTLS(s) + ? DTLS1_VERSION : TLS1_1_VERSION; /* We don't have MD5-SHA1 - do we need it? */ - if (SSL_CONNECTION_IS_DTLS(s)) { - if (DTLS_VERSION_LE(ver_max, DTLS1_VERSION)) - md5sha1_needed = 1; - } else { - if (ver_max <= TLS1_1_VERSION) - md5sha1_needed = 1; - } - if (md5sha1_needed) { + if (ssl_version_cmp(s, ver_max, md5sha1_needed_maxversion) <= 0) { SSLfatal_data(s, SSL_AD_HANDSHAKE_FAILURE, SSL_R_NO_SUITABLE_DIGEST_ALGORITHM, "The max supported SSL/TLS version needs the" @@ -177,14 +172,12 @@ int tls_setup_handshake(SSL_CONNECTION *s) } ok = 1; + /* Don't allow TLSv1.1 or below to be negotiated */ - if (SSL_CONNECTION_IS_DTLS(s)) { - if (DTLS_VERSION_LT(ver_min, DTLS1_2_VERSION)) - ok = SSL_set_min_proto_version(ssl, DTLS1_2_VERSION); - } else { - if (ver_min < TLS1_2_VERSION) - ok = SSL_set_min_proto_version(ssl, TLS1_2_VERSION); - } + negotiated_minversion = SSL_CONNECTION_IS_DTLS(s) ? + DTLS1_2_VERSION : TLS1_2_VERSION; + if (ssl_version_cmp(s, ver_min, negotiated_minversion) < 0) + ok = SSL_set_min_proto_version(ssl, negotiated_minversion); if (!ok) { /* Shouldn't happen */ SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, ERR_R_INTERNAL_ERROR); @@ -204,16 +197,16 @@ int tls_setup_handshake(SSL_CONNECTION *s) */ for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) { const SSL_CIPHER *c = sk_SSL_CIPHER_value(ciphers, i); + int cipher_minprotover = SSL_CONNECTION_IS_DTLS(s) + ? c->min_dtls : c->min_tls; + int cipher_maxprotover = SSL_CONNECTION_IS_DTLS(s) + ? c->max_dtls : c->max_tls; - if (SSL_CONNECTION_IS_DTLS(s)) { - if (DTLS_VERSION_GE(ver_max, c->min_dtls) && - DTLS_VERSION_LE(ver_max, c->max_dtls)) - ok = 1; - } else if (ver_max >= c->min_tls && ver_max <= c->max_tls) { + if (ssl_version_cmp(s, ver_max, cipher_minprotover) >= 0 + && ssl_version_cmp(s, ver_max, cipher_maxprotover) <= 0) { ok = 1; - } - if (ok) break; + } } if (!ok) { SSLfatal_data(s, SSL_AD_HANDSHAKE_FAILURE, @@ -1788,15 +1781,23 @@ int ssl_allow_compression(SSL_CONNECTION *s) return ssl_security(s, SSL_SECOP_COMPRESSION, 0, 0, NULL); } -static int version_cmp(const SSL_CONNECTION *s, int a, int b) +/* + * SSL/TLS/DTLS version comparison + * + * Returns + * 0 if versiona is equal to versionb + * 1 if versiona is greater than versionb + * -1 if versiona is less than versionb + */ +int ssl_version_cmp(const SSL_CONNECTION *s, int versiona, int versionb) { int dtls = SSL_CONNECTION_IS_DTLS(s); - if (a == b) + if (versiona == versionb) return 0; if (!dtls) - return a < b ? -1 : 1; - return DTLS_VERSION_LT(a, b) ? -1 : 1; + return versiona < versionb ? -1 : 1; + return DTLS_VERSION_LT(versiona, versionb) ? -1 : 1; } typedef struct { @@ -1873,12 +1874,12 @@ static int ssl_method_error(const SSL_CONNECTION *s, const SSL_METHOD *method) int version = method->version; if ((s->min_proto_version != 0 && - version_cmp(s, version, s->min_proto_version) < 0) || + ssl_version_cmp(s, version, s->min_proto_version) < 0) || ssl_security(s, SSL_SECOP_VERSION, 0, version, NULL) == 0) return SSL_R_VERSION_TOO_LOW; if (s->max_proto_version != 0 && - version_cmp(s, version, s->max_proto_version) > 0) + ssl_version_cmp(s, version, s->max_proto_version) > 0) return SSL_R_VERSION_TOO_HIGH; if ((s->options & method->mask) != 0) @@ -1966,7 +1967,7 @@ int ssl_version_supported(const SSL_CONNECTION *s, int version, switch (SSL_CONNECTION_GET_SSL(s)->method->version) { default: /* Version should match method version for non-ANY method */ - return version_cmp(s, version, s->version) == 0; + return ssl_version_cmp(s, version, s->version) == 0; case TLS_ANY_VERSION: table = tls_version_table; break; @@ -1976,10 +1977,10 @@ int ssl_version_supported(const SSL_CONNECTION *s, int version, } for (vent = table; - vent->version != 0 && version_cmp(s, version, vent->version) <= 0; + vent->version != 0 && ssl_version_cmp(s, version, vent->version) <= 0; ++vent) { if (vent->cmeth != NULL - && version_cmp(s, version, vent->version) == 0 + && ssl_version_cmp(s, version, vent->version) == 0 && ssl_method_error(s, vent->cmeth()) == 0 && (!s->server || version != TLS1_3_VERSION @@ -2153,7 +2154,7 @@ int ssl_choose_server_version(SSL_CONNECTION *s, CLIENTHELLO_MSG *hello, switch (server_version) { default: if (!SSL_CONNECTION_IS_TLS13(s)) { - if (version_cmp(s, client_version, s->version) < 0) + if (ssl_version_cmp(s, client_version, s->version) < 0) return SSL_R_WRONG_SSL_VERSION; *dgrd = DOWNGRADE_NONE; /* @@ -2210,7 +2211,7 @@ int ssl_choose_server_version(SSL_CONNECTION *s, CLIENTHELLO_MSG *hello, return SSL_R_BAD_LEGACY_VERSION; while (PACKET_get_net_2(&versionslist, &candidate_vers)) { - if (version_cmp(s, candidate_vers, best_vers) <= 0) + if (ssl_version_cmp(s, candidate_vers, best_vers) <= 0) continue; if (ssl_version_supported(s, candidate_vers, &best_method)) best_vers = candidate_vers; @@ -2245,7 +2246,7 @@ int ssl_choose_server_version(SSL_CONNECTION *s, CLIENTHELLO_MSG *hello, * If the supported versions extension isn't present, then the highest * version we can negotiate is TLSv1.2 */ - if (version_cmp(s, client_version, TLS1_3_VERSION) >= 0) + if (ssl_version_cmp(s, client_version, TLS1_3_VERSION) >= 0) client_version = TLS1_2_VERSION; /* @@ -2256,7 +2257,7 @@ int ssl_choose_server_version(SSL_CONNECTION *s, CLIENTHELLO_MSG *hello, const SSL_METHOD *method; if (vent->smeth == NULL || - version_cmp(s, client_version, vent->version) < 0) + ssl_version_cmp(s, client_version, vent->version) < 0) continue; method = vent->smeth(); if (ssl_method_error(s, method) == 0) { @@ -2344,13 +2345,8 @@ int ssl_choose_client_version(SSL_CONNECTION *s, int version, SSLfatal(s, SSL_AD_PROTOCOL_VERSION, ret); return 0; } - if (SSL_CONNECTION_IS_DTLS(s) ? DTLS_VERSION_LT(s->version, ver_min) - : s->version < ver_min) { - s->version = origv; - SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_R_UNSUPPORTED_PROTOCOL); - return 0; - } else if (SSL_CONNECTION_IS_DTLS(s) ? DTLS_VERSION_GT(s->version, ver_max) - : s->version > ver_max) { + if (ssl_version_cmp(s, s->version, ver_min) < 0 + || ssl_version_cmp(s, s->version, ver_max) > 0) { s->version = origv; SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_R_UNSUPPORTED_PROTOCOL); return 0; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 236c1b49a7..de82d2b33a 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -849,6 +849,7 @@ int tls_valid_group(SSL_CONNECTION *s, uint16_t group_id, const TLS_GROUP_INFO *ginfo = tls1_group_id_lookup(SSL_CONNECTION_GET_CTX(s), group_id); int ret; + int group_minversion, group_maxversion; if (okfortls13 != NULL) *okfortls13 = 0; @@ -856,27 +857,22 @@ int tls_valid_group(SSL_CONNECTION *s, uint16_t group_id, if (ginfo == NULL) return 0; - if (SSL_CONNECTION_IS_DTLS(s)) { - if (ginfo->mindtls < 0 || ginfo->maxdtls < 0) - return 0; - if (ginfo->maxdtls == 0) - ret = 1; - else - ret = DTLS_VERSION_LE(minversion, ginfo->maxdtls); - if (ginfo->mindtls > 0) - ret &= DTLS_VERSION_GE(maxversion, ginfo->mindtls); - } else { - if (ginfo->mintls < 0 || ginfo->maxtls < 0) - return 0; - if (ginfo->maxtls == 0) - ret = 1; - else - ret = (minversion <= ginfo->maxtls); - if (ginfo->mintls > 0) - ret &= (maxversion >= ginfo->mintls); + group_minversion = SSL_CONNECTION_IS_DTLS(s) ? ginfo->mindtls : ginfo->mintls; + group_maxversion = SSL_CONNECTION_IS_DTLS(s) ? ginfo->maxdtls : ginfo->maxtls; + + if (group_minversion < 0 || group_maxversion < 0) + return 0; + if (group_maxversion == 0) + ret = 1; + else + ret = (ssl_version_cmp(s, minversion, group_maxversion) <= 0); + if (group_minversion > 0) + ret &= (ssl_version_cmp(s, maxversion, group_minversion) >= 0); + + if (!SSL_CONNECTION_IS_DTLS(s)) { if (ret && okfortls13 != NULL && maxversion == TLS1_3_VERSION) - *okfortls13 = (ginfo->maxtls == 0) - || (ginfo->maxtls >= TLS1_3_VERSION); + *okfortls13 = (group_maxversion == 0) + || (group_maxversion >= TLS1_3_VERSION); } ret &= !isec || strcmp(ginfo->algorithm, "EC") == 0 @@ -962,6 +958,7 @@ uint16_t tls1_shared_group(SSL_CONNECTION *s, int nmatch) for (k = 0, i = 0; i < num_pref; i++) { uint16_t id = pref[i]; const TLS_GROUP_INFO *inf; + int minversion, maxversion; if (!tls1_in_list(id, supp, num_supp) || !tls_group_allowed(s, id, SSL_SECOP_CURVE_SHARED)) @@ -969,20 +966,17 @@ uint16_t tls1_shared_group(SSL_CONNECTION *s, int nmatch) inf = tls1_group_id_lookup(ctx, id); if (!ossl_assert(inf != NULL)) return 0; - if (SSL_CONNECTION_IS_DTLS(s)) { - if (inf->maxdtls == -1) - continue; - if ((inf->mindtls != 0 && DTLS_VERSION_LT(s->version, inf->mindtls)) - || (inf->maxdtls != 0 - && DTLS_VERSION_GT(s->version, inf->maxdtls))) - continue; - } else { - if (inf->maxtls == -1) - continue; - if ((inf->mintls != 0 && s->version < inf->mintls) - || (inf->maxtls != 0 && s->version > inf->maxtls)) - continue; - } + + minversion = SSL_CONNECTION_IS_DTLS(s) + ? inf->mindtls : inf->mintls; + maxversion = SSL_CONNECTION_IS_DTLS(s) + ? inf->maxdtls : inf->maxtls; + if (maxversion == -1) + continue; + if ((minversion != 0 && ssl_version_cmp(s, s->version, minversion) < 0) + || (maxversion != 0 + && ssl_version_cmp(s, s->version, maxversion) > 0)) + continue; if (nmatch == k) return id; @@ -2060,6 +2054,9 @@ int ssl_set_client_disabled(SSL_CONNECTION *s) int ssl_cipher_disabled(const SSL_CONNECTION *s, const SSL_CIPHER *c, int op, int ecdhe) { + int minversion = SSL_CONNECTION_IS_DTLS(s) ? c->min_dtls : c->min_tls; + int maxversion = SSL_CONNECTION_IS_DTLS(s) ? c->max_dtls : c->max_tls; + if (c->algorithm_mkey & s->s3.tmp.mask_k || c->algorithm_auth & s->s3.tmp.mask_a) return 1; @@ -2077,23 +2074,17 @@ int ssl_cipher_disabled(const SSL_CONNECTION *s, const SSL_CIPHER *c, return 1; } - if (!SSL_CONNECTION_IS_DTLS(s)) { - int min_tls = c->min_tls; + /* + * For historical reasons we will allow ECHDE to be selected by a server + * in SSLv3 if we are a client + */ + if (minversion == TLS1_VERSION + && ecdhe + && (c->algorithm_mkey & (SSL_kECDHE | SSL_kECDHEPSK)) != 0) + minversion = SSL3_VERSION; - /* - * For historical reasons we will allow ECHDE to be selected by a server - * in SSLv3 if we are a client - */ - if (min_tls == TLS1_VERSION && ecdhe - && (c->algorithm_mkey & (SSL_kECDHE | SSL_kECDHEPSK)) != 0) - min_tls = SSL3_VERSION; - - if ((min_tls > s->s3.tmp.max_ver) || (c->max_tls < s->s3.tmp.min_ver)) - return 1; - } - if (SSL_CONNECTION_IS_DTLS(s) - && (DTLS_VERSION_GT(c->min_dtls, s->s3.tmp.max_ver) - || DTLS_VERSION_LT(c->max_dtls, s->s3.tmp.min_ver))) + if (ssl_version_cmp(s, minversion, s->s3.tmp.max_ver) > 0 + || ssl_version_cmp(s, maxversion, s->s3.tmp.min_ver) < 0) return 1; return !ssl_security(s, op, c->strength_bits, 0, (void *)c);