diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 3f5abaa48c..0311dadb7e 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -829,32 +829,43 @@ SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, SSL *user_ssl, goto err; s->session_ctx = ctx; - if (ctx->ext.ecpointformats) { + if (ctx->ext.ecpointformats != NULL) { s->ext.ecpointformats = OPENSSL_memdup(ctx->ext.ecpointformats, ctx->ext.ecpointformats_len); - if (!s->ext.ecpointformats) { + if (s->ext.ecpointformats == NULL) { s->ext.ecpointformats_len = 0; goto err; } s->ext.ecpointformats_len = ctx->ext.ecpointformats_len; } - if (ctx->ext.supportedgroups) { + if (ctx->ext.supportedgroups != NULL) { + size_t add = 0; + + if (ctx->ext.supportedgroups_len == 0) + /* Add 1 so allocation won't fail */ + add = 1; s->ext.supportedgroups = OPENSSL_memdup(ctx->ext.supportedgroups, - ctx->ext.supportedgroups_len - * sizeof(*ctx->ext.supportedgroups)); - if (!s->ext.supportedgroups) { + (ctx->ext.supportedgroups_len + add) + * sizeof(*ctx->ext.supportedgroups)); + if (s->ext.supportedgroups == NULL) { s->ext.supportedgroups_len = 0; goto err; } s->ext.supportedgroups_len = ctx->ext.supportedgroups_len; } if (ctx->ext.keyshares != NULL) { + size_t add = 0; + + if (ctx->ext.keyshares_len == 0) + /* Add 1 so allocation won't fail */ + add = 1; s->ext.keyshares = OPENSSL_memdup(ctx->ext.keyshares, - ctx->ext.keyshares_len * sizeof(*ctx->ext.keyshares)); + (ctx->ext.keyshares_len + add) + * sizeof(*ctx->ext.keyshares)); if (s->ext.keyshares == NULL) { s->ext.keyshares_len = 0; goto err; @@ -862,9 +873,15 @@ SSL *ossl_ssl_connection_new_int(SSL_CTX *ctx, SSL *user_ssl, s->ext.keyshares_len = ctx->ext.keyshares_len; } if (ctx->ext.tuples != NULL) { + size_t add = 0; + + if (ctx->ext.tuples_len == 0) + /* Add 1 so allocation won't fail */ + add = 1; s->ext.tuples = OPENSSL_memdup(ctx->ext.tuples, - ctx->ext.tuples_len * sizeof(*ctx->ext.tuples)); + (ctx->ext.tuples_len + add) + * sizeof(*ctx->ext.tuples)); if (s->ext.tuples == NULL) { s->ext.tuples_len = 0; goto err; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 1ec5596550..7c18dadbda 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -1565,13 +1566,13 @@ int tls1_set_groups_list(SSL_CTX *ctx, size_t **tplext, size_t *tplextlen, const char *str) { - size_t i, j; + size_t i = 0, j; int ret = 0, parse_ret = 0; gid_cb_st gcb; /* Sanity check */ if (ctx == NULL) { - ERR_raise(ERR_LIB_CONF, CONF_R_VARIABLE_HAS_NO_VALUE); + ERR_raise(ERR_LIB_CONF, ERR_R_INTERNAL_ERROR); return 0; } @@ -1595,6 +1596,11 @@ int tls1_set_groups_list(SSL_CTX *ctx, if (gcb.ksid_arr == NULL) goto end; + while (str[0] != '\0' && isspace((unsigned char)*str)) + str++; + if (str[0] == '\0') + goto empty_list; + /* * Start the (potentially recursive) tuple processing by calling CONF_parse_list * with the TUPLE_DELIMITER_CHARACTER (which will call tuple_cb after cleaning spaces) @@ -1624,12 +1630,6 @@ int tls1_set_groups_list(SSL_CTX *ctx, } gcb.tplcnt = i; - /* Some more checks (at least one remaining group, not more that nominally 4 key shares */ - if (gcb.gidcnt == 0) { - ERR_raise_data(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT, - "No valid groups in '%s'", str); - goto end; - } if (gcb.ksidcnt > OPENSSL_CLIENT_MAX_KEY_SHARES) { ERR_raise_data(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT, "To many keyshares requested in '%s' (max = %d)", @@ -1641,7 +1641,7 @@ int tls1_set_groups_list(SSL_CTX *ctx, * For backward compatibility we let the rest of the code know that a key share * for the first valid group should be added if no "*" prefix was used anywhere */ - if (gcb.ksidcnt == 0) { + if (gcb.gidcnt > 0 && gcb.ksidcnt == 0) { /* * No key share group prefix character was used, hence we indicate that a single * key share should be sent and flag that it should come from the supported_groups list @@ -1650,6 +1650,7 @@ int tls1_set_groups_list(SSL_CTX *ctx, gcb.ksid_arr[0] = 0; } + empty_list: /* * A call to tls1_set_groups_list with any of the args (other than ctx) set * to NULL only does a syntax check, hence we're done here and report success diff --git a/test/sslapitest.c b/test/sslapitest.c index 8c1b2073b8..f3395997c5 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -9846,7 +9846,7 @@ static int test_unknown_sigalgs_groups(void) 0)) goto end; - if (!TEST_int_le(SSL_CTX_set1_groups_list(ctx, + if (!TEST_int_gt(SSL_CTX_set1_groups_list(ctx, "?nonexistent1:?nonexistent2:?nonexistent3"), 0)) goto end; diff --git a/test/tls13groupselection_test.c b/test/tls13groupselection_test.c index fcebc1e43e..01d1eded5f 100644 --- a/test/tls13groupselection_test.c +++ b/test/tls13groupselection_test.c @@ -278,33 +278,27 @@ static const struct tls13groupselection_test_st tls13groupselection_tests[] = CLIENT_PREFERENCE, SYNTAX_FAILURE }, - /* test 33 remove all groups */ - { "X25519:secp256r1:X448:secp521r1:-X448:-secp256r1:-X25519:-secp521r1", + { "X25519:??secp256r1:X448", /* test 33 */ "", CLIENT_PREFERENCE, SYNTAX_FAILURE }, - { "X25519:??secp256r1:X448", /* test 34 */ + { "X25519:secp256r1:**X448", /* test 34 */ "", CLIENT_PREFERENCE, SYNTAX_FAILURE }, - { "X25519:secp256r1:**X448", /* test 35 */ + { "--X25519:secp256r1:X448", /* test 35 */ "", CLIENT_PREFERENCE, SYNTAX_FAILURE }, - { "--X25519:secp256r1:X448", /* test 36 */ + { "-DEFAULT", /* test 36 */ "", CLIENT_PREFERENCE, SYNTAX_FAILURE }, - { "-DEFAULT", /* test 37 */ - "", - CLIENT_PREFERENCE, - SYNTAX_FAILURE - }, - { "?DEFAULT", /* test 38 */ + { "?DEFAULT", /* test 37 */ "", CLIENT_PREFERENCE, SYNTAX_FAILURE @@ -313,6 +307,12 @@ static const struct tls13groupselection_test_st tls13groupselection_tests[] = * Negotiation Failures * No overlapping groups between client and server */ + /* test 38 remove all groups */ + { "X25519:secp256r1:X448:secp521r1:-X448:-secp256r1:-X25519:-secp521r1", + "", + CLIENT_PREFERENCE, + NEGOTIATION_FAILURE + }, { "secp384r1:secp521r1:X25519", /* test 39 */ "prime256v1:X448", CLIENT_PREFERENCE,