Fix some TODO(3.0) occurrences in ssl/t1_lib.c

One was related to probing for the combination of signature and hash
algorithm together. This is currently not easily possible. The TODO(3.0)
is converted to a normal comment and I've raised the problem as issue
number #14885 as something to resolve post 3.0.

The other TODO was a hard coded limit on the number of groups that could
be registered. This has been amended so that there is no limit.

Fixes #14333

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14886)
This commit is contained in:
Matt Caswell 2021-04-15 10:00:40 +01:00
parent b247113c05
commit 57e7401fc5
2 changed files with 82 additions and 17 deletions

View File

@ -691,13 +691,13 @@ err:
return 0;
}
/* TODO(3.0): An arbitrary amount for now. Take another look at this */
# define MAX_GROUPLIST 40
# define GROUPLIST_INCREMENT 40
# define GROUP_NAME_BUFFER_LENGTH 64
typedef struct {
SSL_CTX *ctx;
size_t gidcnt;
uint16_t gid_arr[MAX_GROUPLIST];
size_t gidmax;
uint16_t *gid_arr;
} gid_cb_st;
static int gid_cb(const char *elem, int len, void *arg)
@ -709,8 +709,14 @@ static int gid_cb(const char *elem, int len, void *arg)
if (elem == NULL)
return 0;
if (garg->gidcnt == MAX_GROUPLIST)
return 0;
if (garg->gidcnt == garg->gidmax) {
uint16_t *tmp =
OPENSSL_realloc(garg->gid_arr, garg->gidmax + GROUPLIST_INCREMENT);
if (tmp == NULL)
return 0;
garg->gidmax += GROUPLIST_INCREMENT;
garg->gid_arr = tmp;
}
if (len > (int)(sizeof(etmp) - 1))
return 0;
memcpy(etmp, elem, len);
@ -732,13 +738,20 @@ int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen,
{
gid_cb_st gcb;
uint16_t *tmparr;
int ret = 0;
gcb.gidcnt = 0;
gcb.gidmax = GROUPLIST_INCREMENT;
gcb.gid_arr = OPENSSL_malloc(gcb.gidmax * sizeof(*gcb.gid_arr));
if (gcb.gid_arr == NULL)
return 0;
gcb.ctx = ctx;
if (!CONF_parse_list(str, ':', 1, gid_cb, &gcb))
return 0;
if (pext == NULL)
return 1;
goto end;
if (pext == NULL) {
ret = 1;
goto end;
}
/*
* gid_cb ensurse there are no duplicates so we can just go ahead and set
@ -746,10 +759,13 @@ int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen,
*/
tmparr = OPENSSL_memdup(gcb.gid_arr, gcb.gidcnt * sizeof(*tmparr));
if (tmparr == NULL)
return 0;
goto end;
*pext = tmparr;
*pextlen = gcb.gidcnt;
return 1;
ret = 1;
end:
OPENSSL_free(gcb.gid_arr);
return ret;
}
/* Check a group id matches preferences */
@ -1142,7 +1158,7 @@ int ssl_setup_sig_algs(SSL_CTX *ctx)
/*
* Check hash is available.
* TODO(3.0): This test is not perfect. A provider could have support
* This test is not perfect. A provider could have support
* for a signature scheme, but not a particular hash. However the hash
* could be available from some other loaded provider. In that case it
* could be that the signature is available, and the hash is available

View File

@ -14,6 +14,7 @@
#include <openssl/params.h>
/* For TLS1_3_VERSION */
#include <openssl/ssl.h>
#include <internal/nelem.h>
static OSSL_FUNC_keymgmt_import_fn xor_import;
static OSSL_FUNC_keymgmt_import_types_fn xor_import_types;
@ -167,16 +168,52 @@ static const OSSL_PARAM xor_kemgroup_params[] = {
OSSL_PARAM_END
};
#define NUM_DUMMY_GROUPS 50
static char *dummy_group_names[NUM_DUMMY_GROUPS];
static int tls_prov_get_capabilities(void *provctx, const char *capability,
OSSL_CALLBACK *cb, void *arg)
{
if (strcmp(capability, "TLS-GROUP") == 0)
return cb(xor_group_params, arg)
&& cb(xor_kemgroup_params, arg);
int ret;
int i;
const char *dummy_base = "dummy";
const size_t dummy_name_max_size = strlen(dummy_base) + 3;
/* We don't support this capability */
return 0;
if (strcmp(capability, "TLS-GROUP") != 0) {
/* We don't support this capability */
return 0;
}
/* Register our 2 groups */
ret = cb(xor_group_params, arg);
ret &= cb(xor_kemgroup_params, arg);
/*
* Now register some dummy groups > GROUPLIST_INCREMENT (== 40) as defined
* in ssl/t1_lib.c, to make sure we exercise the code paths for registering
* large numbers of groups.
*/
for (i = 0; i < NUM_DUMMY_GROUPS; i++) {
OSSL_PARAM dummygroup[OSSL_NELEM(xor_group_params)];
memcpy(dummygroup, xor_group_params, sizeof(xor_group_params));
/* Give the dummy group a unique name */
if (dummy_group_names[i] == NULL) {
dummy_group_names[i] = OPENSSL_zalloc(dummy_name_max_size);
if (dummy_group_names[i] == NULL)
return 0;
BIO_snprintf(dummy_group_names[i],
dummy_name_max_size,
"%s%d", dummy_base, i);
}
dummygroup[0].data = dummy_group_names[i];
dummygroup[0].data_size = strlen(dummy_group_names[i]) + 1;
ret &= cb(dummygroup, arg);
}
return ret;
}
/*
@ -743,9 +780,21 @@ static const OSSL_ALGORITHM *tls_prov_query(void *provctx, int operation_id,
return NULL;
}
static void tls_prov_teardown(void *provctx)
{
int i;
OSSL_LIB_CTX_free(provctx);
for (i = 0; i < NUM_DUMMY_GROUPS; i++) {
OPENSSL_free(dummy_group_names[i]);
dummy_group_names[i] = NULL;
}
}
/* Functions we provide to the core */
static const OSSL_DISPATCH tls_prov_dispatch_table[] = {
{ OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))OSSL_LIB_CTX_free },
{ OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))tls_prov_teardown },
{ OSSL_FUNC_PROVIDER_QUERY_OPERATION, (void (*)(void))tls_prov_query },
{ OSSL_FUNC_PROVIDER_GET_CAPABILITIES, (void (*)(void))tls_prov_get_capabilities },
{ 0, NULL }