From 8b1db5d329740bd5363fd1763d4030d0e015b521 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 13 Jan 2021 17:27:10 +0000 Subject: Make supported_groups code independent of EC and DH The supported groups code was checking the OPENSSL_NO_EC and OPENSSL_NO_DH guards in order to work, and the list of default groups was based on those guards. However we now need it to work even in a no-ec and no-dh build, because new groups might be added from providers. Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/13916) --- crypto/err/openssl.txt | 2 +- include/openssl/sslerr.h | 1 + ssl/ssl_err.c | 1 + ssl/ssl_local.h | 2 +- ssl/sslerr.h | 2 +- ssl/statem/extensions_clnt.c | 30 +++++++++++++++++++----------- ssl/statem/extensions_srvr.c | 2 +- ssl/t1_lib.c | 27 +++++++++++++++++++++------ 8 files changed, 46 insertions(+), 21 deletions(-) diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index d64b356044..b8064ea678 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1273,7 +1273,6 @@ SSL_R_CERT_CB_ERROR:377:cert cb error SSL_R_CERT_LENGTH_MISMATCH:135:cert length mismatch SSL_R_CIPHERSUITE_DIGEST_HAS_CHANGED:218:ciphersuite digest has changed SSL_R_CIPHER_CODE_WRONG_LENGTH:137:cipher code wrong length -SSL_R_CIPHER_OR_HASH_UNAVAILABLE:138:cipher or hash unavailable SSL_R_CLIENTHELLO_TLSEXT:226:clienthello tlsext SSL_R_COMPRESSED_LENGTH_TOO_LONG:140:compressed length too long SSL_R_COMPRESSION_DISABLED:343:compression disabled @@ -1399,6 +1398,7 @@ SSL_R_NO_SHARED_GROUPS:410:no shared groups SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS:376:no shared signature algorithms SSL_R_NO_SRTP_PROFILES:359:no srtp profiles SSL_R_NO_SUITABLE_DIGEST_ALGORITHM:297:no suitable digest algorithm +SSL_R_NO_SUITABLE_GROUPS:295:no suitable groups SSL_R_NO_SUITABLE_KEY_SHARE:101:no suitable key share SSL_R_NO_SUITABLE_SIGNATURE_ALGORITHM:118:no suitable signature algorithm SSL_R_NO_VALID_SCTS:216:no valid scts diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index 27664afd58..d69055cf3d 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -195,6 +195,7 @@ # define SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS 376 # define SSL_R_NO_SRTP_PROFILES 359 # define SSL_R_NO_SUITABLE_DIGEST_ALGORITHM 297 +# define SSL_R_NO_SUITABLE_GROUPS 295 # define SSL_R_NO_SUITABLE_KEY_SHARE 101 # define SSL_R_NO_SUITABLE_SIGNATURE_ALGORITHM 118 # define SSL_R_NO_VALID_SCTS 216 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 879b276520..d6d0c671a2 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -299,6 +299,7 @@ static const ERR_STRING_DATA SSL_str_reasons[] = { {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SRTP_PROFILES), "no srtp profiles"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_DIGEST_ALGORITHM), "no suitable digest algorithm"}, + {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_GROUPS), "no suitable groups"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_KEY_SHARE), "no suitable key share"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_SIGNATURE_ALGORITHM), diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h index 4138f4eaa3..87a4f428f8 100644 --- a/ssl/ssl_local.h +++ b/ssl/ssl_local.h @@ -2661,7 +2661,7 @@ __owur int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen, const char *str); __owur EVP_PKEY *ssl_generate_pkey_group(SSL *s, uint16_t id); __owur int tls_valid_group(SSL *s, uint16_t group_id, int minversion, - int maxversion); + int maxversion, int isec, int *okfortls13); __owur EVP_PKEY *ssl_generate_param_group(SSL *s, uint16_t id); # ifndef OPENSSL_NO_EC void tls1_get_formatlist(SSL *s, const unsigned char **pformats, diff --git a/ssl/sslerr.h b/ssl/sslerr.h index ed70efc264..3ad54e4dcc 100644 --- a/ssl/sslerr.h +++ b/ssl/sslerr.h @@ -1,6 +1,6 @@ /* * Generated by util/mkerr.pl DO NOT EDIT - * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2020-2021 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the Apache License 2.0 (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 89e1422bbd..cc958aa1b0 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -108,7 +108,6 @@ EXT_RETURN tls_construct_ctos_srp(SSL *s, WPACKET *pkt, unsigned int context, } #endif -#ifndef OPENSSL_NO_EC static int use_ecc(SSL *s, int min_version, int max_version) { int i, end, ret = 0; @@ -144,7 +143,7 @@ static int use_ecc(SSL *s, int min_version, int max_version) for (j = 0; j < num_groups; j++) { uint16_t ctmp = pgroups[j]; - if (tls_valid_group(s, ctmp, min_version, max_version) + if (tls_valid_group(s, ctmp, min_version, max_version, 1, NULL) && tls_group_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) return 1; } @@ -152,6 +151,7 @@ static int use_ecc(SSL *s, int min_version, int max_version) return 0; } +#ifndef OPENSSL_NO_EC EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx) @@ -189,7 +189,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, size_t chainidx) { const uint16_t *pgroups = NULL; - size_t num_groups = 0, i; + size_t num_groups = 0, i, tls13added = 0, added = 0; int min_version, max_version, reason; reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL); @@ -198,13 +198,13 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, return EXT_RETURN_FAIL; } -#if defined(OPENSSL_NO_EC) - if (SSL_IS_DTLS(s) || max_version < TLS1_3_VERSION) - return EXT_RETURN_NOT_SENT; -#else - if (!use_ecc(s, min_version, max_version) && max_version < TLS1_3_VERSION) + /* + * We only support EC groups in TLSv1.2 or below, and in DTLS. Therefore + * if we don't have EC support then we don't send this extension. + */ + if (!use_ecc(s, min_version, max_version) + && (SSL_IS_DTLS(s) || max_version < TLS1_3_VERSION)) return EXT_RETURN_NOT_SENT; -#endif /* * Add TLS extension supported_groups to the ClientHello message @@ -222,17 +222,25 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, /* Copy group ID if supported */ for (i = 0; i < num_groups; i++) { uint16_t ctmp = pgroups[i]; + int okfortls13; - if (tls_valid_group(s, ctmp, min_version, max_version) + if (tls_valid_group(s, ctmp, min_version, max_version, 0, &okfortls13) && tls_group_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) { if (!WPACKET_put_bytes_u16(pkt, ctmp)) { SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } + if (okfortls13 && max_version == TLS1_3_VERSION) + tls13added++; + added++; } } if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); + if (added == 0 || (tls13added == 0 && max_version == TLS1_3_VERSION)) + SSLfatal_data(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_GROUPS, + "No groups enabled for max supported SSL/TLS version"); + else + SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 99cd515386..42fd6ee7d3 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -1355,7 +1355,7 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt, for (i = 0; i < numgroups; i++) { uint16_t group = groups[i]; - if (tls_valid_group(s, group, version, version) + if (tls_valid_group(s, group, version, version, 0, NULL) && tls_group_allowed(s, group, SSL_SECOP_CURVE_SUPPORTED)) { if (first) { /* diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 24bfa96382..6bc33215c1 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -391,15 +391,17 @@ static int discover_provider_groups(OSSL_PROVIDER *provider, void *vctx) int ssl_load_groups(SSL_CTX *ctx) { size_t i, j, num_deflt_grps = 0; - uint16_t tmp_supp_groups[sizeof(supported_groups_default)]; + uint16_t tmp_supp_groups[OSSL_NELEM(supported_groups_default)]; if (!OSSL_PROVIDER_do_all(ctx->libctx, discover_provider_groups, ctx)) return 0; - for (i = 0; i < sizeof(supported_groups_default); i++) { + for (i = 0; i < OSSL_NELEM(supported_groups_default); i++) { for (j = 0; j < ctx->group_list_len; j++) { - if (ctx->group_list[j].group_id == supported_groups_default[i]) + if (ctx->group_list[j].group_id == supported_groups_default[i]) { tmp_supp_groups[num_deflt_grps++] = ctx->group_list[j].group_id; + break; + } } } @@ -414,7 +416,9 @@ int ssl_load_groups(SSL_CTX *ctx) return 0; } - memcpy(ctx->ext.supported_groups_default, tmp_supp_groups, num_deflt_grps); + memcpy(ctx->ext.supported_groups_default, + tmp_supp_groups, + num_deflt_grps * sizeof(tmp_supp_groups[0])); ctx->ext.supported_groups_default_len = num_deflt_grps; return 1; @@ -534,11 +538,15 @@ void tls1_get_supported_groups(SSL *s, const uint16_t **pgroups, } } -int tls_valid_group(SSL *s, uint16_t group_id, int minversion, int maxversion) +int tls_valid_group(SSL *s, uint16_t group_id, int minversion, int maxversion, + int isec, int *okfortls13) { const TLS_GROUP_INFO *ginfo = tls1_group_id_lookup(s->ctx, group_id); int ret; + if (okfortls13 != NULL) + okfortls13 = 0; + if (ginfo == NULL) return 0; @@ -560,7 +568,14 @@ int tls_valid_group(SSL *s, uint16_t group_id, int minversion, int maxversion) ret = (minversion <= ginfo->maxtls); if (ginfo->mintls > 0) ret &= (maxversion >= ginfo->mintls); - } + if (ret && okfortls13 != NULL && maxversion == TLS1_3_VERSION) + *okfortls13 = (ginfo->maxtls == 0) + || (ginfo->maxtls >= TLS1_3_VERSION); + } + ret &= !isec + || strcmp(ginfo->algorithm, "EC") == 0 + || strcmp(ginfo->algorithm, "X25519") == 0 + || strcmp(ginfo->algorithm, "X448") == 0; return ret; } -- cgit v1.2.3