From 20c98cd45399423f760dbd75d8912769c6b7b10e Mon Sep 17 00:00:00 2001 From: Pauli Date: Fri, 27 Mar 2020 10:33:46 +1000 Subject: [PATCH] Param builder: Remove the static size limit. Prior to this, the param builder had a statically sized array internally. This changes it so that it uses a stack instead. Reviewed-by: Nicola Tuveri (Merged from https://github.com/openssl/openssl/pull/11390) --- crypto/param_build.c | 59 +++++++++++++------ ...L_PARAM_BLD_new.pod => OSSL_PARAM_BLD.pod} | 21 +++---- test/param_build_test.c | 44 ++++++++++++++ 3 files changed, 95 insertions(+), 29 deletions(-) rename doc/man3/{OSSL_PARAM_BLD_new.pod => OSSL_PARAM_BLD.pod} (91%) diff --git a/crypto/param_build.c b/crypto/param_build.c index 11986d999b..4f999678cb 100644 --- a/crypto/param_build.c +++ b/crypto/param_build.c @@ -13,14 +13,10 @@ #include #include #include +#include #include "internal/cryptlib.h" #include "openssl/param_build.h" -/* - * The number of OSSL_PARAM elements a builder will allow. - */ -#define OSSL_PARAM_BLD_MAX 25 - /* * Special internal param type to indicate the end of an allocate OSSL_PARAM * array. @@ -46,11 +42,12 @@ typedef struct { } num; } OSSL_PARAM_BLD_DEF; +DEFINE_STACK_OF(OSSL_PARAM_BLD_DEF) + struct ossl_param_bld_st { - size_t curr; size_t total_blocks; size_t secure_blocks; - OSSL_PARAM_BLD_DEF params[OSSL_PARAM_BLD_MAX]; + STACK_OF(OSSL_PARAM_BLD_DEF) *params; }; typedef union { @@ -68,14 +65,12 @@ static OSSL_PARAM_BLD_DEF *param_push(OSSL_PARAM_BLD *bld, const char *key, int size, size_t alloc, int type, int secure) { - OSSL_PARAM_BLD_DEF *pd; + OSSL_PARAM_BLD_DEF *pd = OPENSSL_zalloc(sizeof(*pd)); - if (bld->curr >= OSSL_PARAM_BLD_MAX) { - CRYPTOerr(CRYPTO_F_PARAM_PUSH, CRYPTO_R_TOO_MANY_RECORDS); + if (pd == NULL) { + CRYPTOerr(CRYPTO_F_PARAM_PUSH, ERR_R_MALLOC_FAILURE); return NULL; } - pd = bld->params + bld->curr++; - memset(pd, 0, sizeof(*pd)); pd->key = key; pd->type = type; pd->size = size; @@ -84,6 +79,10 @@ static OSSL_PARAM_BLD_DEF *param_push(OSSL_PARAM_BLD *bld, const char *key, bld->secure_blocks += pd->alloc_blocks; else bld->total_blocks += pd->alloc_blocks; + if (sk_OSSL_PARAM_BLD_DEF_push(bld->params, pd) <= 0) { + OPENSSL_free(pd); + pd = NULL; + } return pd; } @@ -104,11 +103,30 @@ static int param_push_num(OSSL_PARAM_BLD *bld, const char *key, OSSL_PARAM_BLD *OSSL_PARAM_BLD_new(void) { - return OPENSSL_zalloc(sizeof(OSSL_PARAM_BLD)); + OSSL_PARAM_BLD *r = OPENSSL_zalloc(sizeof(OSSL_PARAM_BLD)); + + if (r != NULL) { + r->params = sk_OSSL_PARAM_BLD_DEF_new_null(); + if (r->params == NULL) { + OPENSSL_free(r); + r = NULL; + } + } + return r; +} + +static void free_all_params(OSSL_PARAM_BLD *bld) +{ + int i, n = sk_OSSL_PARAM_BLD_DEF_num(bld->params); + + for (i = 0; i < n; i++) + OPENSSL_free(sk_OSSL_PARAM_BLD_DEF_pop(bld->params)); } void OSSL_PARAM_BLD_free(OSSL_PARAM_BLD *bld) { + free_all_params(bld); + sk_OSSL_PARAM_BLD_DEF_free(bld->params); OPENSSL_free(bld); } @@ -285,12 +303,12 @@ static OSSL_PARAM *param_bld_convert(OSSL_PARAM_BLD *bld, OSSL_PARAM *param, OSSL_PARAM_BLD_BLOCK *blk, OSSL_PARAM_BLD_BLOCK *secure) { - size_t i; + int i, num = sk_OSSL_PARAM_BLD_DEF_num(bld->params); OSSL_PARAM_BLD_DEF *pd; void *p; - for (i = 0; i < bld->curr; i++) { - pd = bld->params + i; + for (i = 0; i < num; i++) { + pd = sk_OSSL_PARAM_BLD_DEF_value(bld->params, i); param[i].key = pd->key; param[i].data_type = pd->type; param[i].data_size = pd->size; @@ -333,7 +351,8 @@ OSSL_PARAM *OSSL_PARAM_BLD_to_param(OSSL_PARAM_BLD *bld) { OSSL_PARAM_BLD_BLOCK *blk, *s = NULL; OSSL_PARAM *params, *last; - const size_t p_blks = bytes_to_blocks((1 + bld->curr) * sizeof(*params)); + const int num = sk_OSSL_PARAM_BLD_DEF_num(bld->params); + const size_t p_blks = bytes_to_blocks((1 + num) * sizeof(*params)); const size_t total = ALIGN_SIZE * (p_blks + bld->total_blocks); const size_t ss = ALIGN_SIZE * bld->secure_blocks; @@ -359,8 +378,10 @@ OSSL_PARAM *OSSL_PARAM_BLD_to_param(OSSL_PARAM_BLD *bld) last->data = s; last->data_type = OSSL_PARAM_ALLOCATED_END; - /* Reset for reuse */ - memset(bld, 0, sizeof(*bld)); + /* Reset builder for reuse */ + bld->total_blocks = 0; + bld->secure_blocks = 0; + free_all_params(bld); return params; } diff --git a/doc/man3/OSSL_PARAM_BLD_new.pod b/doc/man3/OSSL_PARAM_BLD.pod similarity index 91% rename from doc/man3/OSSL_PARAM_BLD_new.pod rename to doc/man3/OSSL_PARAM_BLD.pod index 72b2ff87a3..ed82e32073 100644 --- a/doc/man3/OSSL_PARAM_BLD_new.pod +++ b/doc/man3/OSSL_PARAM_BLD.pod @@ -2,15 +2,16 @@ =head1 NAME -OSSL_PARAM_BLD_new, OSSL_PARAM_BLD_to_param, OSSL_PARAM_BLD_free_params, -OSSL_PARAM_BLD_free, OSSL_PARAM_BLD_push_int, OSSL_PARAM_BLD_push_uint, -OSSL_PARAM_BLD_push_long, OSSL_PARAM_BLD_push_ulong, -OSSL_PARAM_BLD_push_int32, OSSL_PARAM_BLD_push_uint32, -OSSL_PARAM_BLD_push_int64, OSSL_PARAM_BLD_push_uint64, -OSSL_PARAM_BLD_push_size_t, OSSL_PARAM_BLD_push_double, -OSSL_PARAM_BLD_push_BN, OSSL_PARAM_BLD_push_BN_pad, -OSSL_PARAM_BLD_push_utf8_string, OSSL_PARAM_BLD_push_utf8_ptr, -OSSL_PARAM_BLD_push_octet_string, OSSL_PARAM_BLD_push_octet_ptr +OSSL_PARAM_BLD, OSSL_PARAM_BLD_new, OSSL_PARAM_BLD_to_param, +OSSL_PARAM_BLD_free_params, OSSL_PARAM_BLD_free, OSSL_PARAM_BLD_push_int, +OSSL_PARAM_BLD_push_uint, OSSL_PARAM_BLD_push_long, +OSSL_PARAM_BLD_push_ulong, OSSL_PARAM_BLD_push_int32, +OSSL_PARAM_BLD_push_uint32, OSSL_PARAM_BLD_push_int64, +OSSL_PARAM_BLD_push_uint64, OSSL_PARAM_BLD_push_size_t, +OSSL_PARAM_BLD_push_double, OSSL_PARAM_BLD_push_BN, +OSSL_PARAM_BLD_push_BN_pad, OSSL_PARAM_BLD_push_utf8_string, +OSSL_PARAM_BLD_push_utf8_ptr, OSSL_PARAM_BLD_push_octet_string, +OSSL_PARAM_BLD_push_octet_ptr - functions to assist in the creation of OSSL_PARAM arrays =head1 SYNOPSIS @@ -184,7 +185,7 @@ The functions described here were all added in OpenSSL 3.0. =head1 COPYRIGHT -Copyright 2019 The OpenSSL Project Authors. All Rights Reserved. +Copyright 2019-2020 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/test/param_build_test.c b/test/param_build_test.c index 9b20e33360..c5fcc70a66 100644 --- a/test/param_build_test.c +++ b/test/param_build_test.c @@ -192,9 +192,53 @@ err: return res; } +static int builder_limit_test(void) +{ + const int n = 100; + char names[100][3]; + OSSL_PARAM_BLD *bld = OSSL_PARAM_BLD_new(); + OSSL_PARAM *params = NULL; + int i, res = 0; + + if (!TEST_ptr(bld)) + goto err; + + for (i = 0; i < n; i++) { + names[i][0] = 'A' + (i / 26) - 1; + names[i][0] = 'a' + (i % 26) - 1; + names[i][2] = '\0'; + if (!TEST_true(OSSL_PARAM_BLD_push_int(bld, names[i], 3 * i + 1))) + goto err; + } + if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))) + goto err; + /* Count the elements in the params arrary, expecting n */ + for (i = 0; params[i].key != NULL; i++); + if (!TEST_int_eq(i, n)) + goto err; + + /* Verify that the build, cleared the builder structure */ + OSSL_PARAM_BLD_free_params(params); + params = NULL; + + if (!TEST_true(OSSL_PARAM_BLD_push_int(bld, "g", 2)) + || !TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld))) + goto err; + /* Count the elements in the params arrary, expecting 1 */ + for (i = 0; params[i].key != NULL; i++); + if (!TEST_int_eq(i, 1)) + goto err; + res = 1; +err: + OSSL_PARAM_BLD_free_params(params); + OSSL_PARAM_BLD_free(bld); + return res; +} + int setup_tests(void) { ADD_TEST(template_public_test); ADD_TEST(template_private_test); + ADD_TEST(builder_limit_test); return 1; } -- 2.39.5