From 7312ef3fc4a7d391272f3ba8075eabf81a229ad2 Mon Sep 17 00:00:00 2001 From: Pauli Date: Fri, 19 Jul 2019 01:14:07 +1000 Subject: [PATCH] Add param builder free function. This means include deallocation information in the return from the ossl_param_bld_to_param function. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/9404) --- crypto/param_build.c | 48 ++++++++++++++--------- doc/internal/man3/ossl_param_bld_init.pod | 45 ++++++++++----------- include/internal/param_build.h | 3 +- test/param_build_test.c | 29 ++++++++------ 4 files changed, 71 insertions(+), 54 deletions(-) diff --git a/crypto/param_build.c b/crypto/param_build.c index 851b735896..4d28c8737e 100644 --- a/crypto/param_build.c +++ b/crypto/param_build.c @@ -15,6 +15,8 @@ #include "internal/cryptlib.h" #include "internal/param_build.h" +#define OSSL_PARAM_ALLOCATED_END 127 + typedef union { OSSL_UNION_ALIGN; } OSSL_PARAM_BLD_BLOCK; @@ -274,40 +276,50 @@ static OSSL_PARAM *param_bld_convert(OSSL_PARAM_BLD *bld, OSSL_PARAM *param, } } param[i] = OSSL_PARAM_construct_end(); - return param; + return param + i; } -OSSL_PARAM *ossl_param_bld_to_param(OSSL_PARAM_BLD *bld, void **secure) +OSSL_PARAM *ossl_param_bld_to_param(OSSL_PARAM_BLD *bld) { OSSL_PARAM_BLD_BLOCK *blk, *s = NULL; - OSSL_PARAM *param; - const size_t p_blks = bytes_to_blocks((bld->curr + 1) * sizeof(*param)); + OSSL_PARAM *params, *last; + const size_t p_blks = bytes_to_blocks((1 + bld->curr) * sizeof(*params)); const size_t total = ALIGN_SIZE * (p_blks + bld->total_blocks); + const size_t ss = ALIGN_SIZE * bld->secure_blocks; - if (bld->secure_blocks > 0) { - if (secure == NULL) { - CRYPTOerr(CRYPTO_F_OSSL_PARAM_BLD_TO_PARAM, - CRYPTO_R_INVALID_NULL_ARGUMENT); - return NULL; - } - s = OPENSSL_secure_malloc(bld->secure_blocks * ALIGN_SIZE); + if (ss > 0) { + s = OPENSSL_secure_malloc(ss); if (s == NULL) { CRYPTOerr(CRYPTO_F_OSSL_PARAM_BLD_TO_PARAM, CRYPTO_R_SECURE_MALLOC_FAILURE); return NULL; } } - param = OPENSSL_malloc(total); - if (param == NULL) { + params = OPENSSL_malloc(total); + if (params == NULL) { CRYPTOerr(CRYPTO_F_OSSL_PARAM_BLD_TO_PARAM, ERR_R_MALLOC_FAILURE); OPENSSL_secure_free(s); return NULL; } - if (secure != NULL) - *secure = s; - blk = p_blks + (OSSL_PARAM_BLD_BLOCK *)(param); - param_bld_convert(bld, param, blk, s); - return param; + blk = p_blks + (OSSL_PARAM_BLD_BLOCK *)(params); + last = param_bld_convert(bld, params, blk, s); + last->data_size = ss; + last->data = s; + last->data_type = OSSL_PARAM_ALLOCATED_END; + return params; +} + +void ossl_param_bld_free(OSSL_PARAM *params) +{ + if (params != NULL) { + OSSL_PARAM *p; + + for (p = params; p->key != NULL; p++) + ; + if (p->data_type == OSSL_PARAM_ALLOCATED_END) + OPENSSL_secure_clear_free(p->data, p->data_size); + OPENSSL_free(params); + } } OSSL_PARAM *ossl_param_bld_to_param_ex(OSSL_PARAM_BLD *bld, OSSL_PARAM *params, diff --git a/doc/internal/man3/ossl_param_bld_init.pod b/doc/internal/man3/ossl_param_bld_init.pod index a65aa8bd83..2385ffcab6 100644 --- a/doc/internal/man3/ossl_param_bld_init.pod +++ b/doc/internal/man3/ossl_param_bld_init.pod @@ -2,32 +2,33 @@ =head1 NAME -ossl_param_bld_init, -ossl_param_bld_to_param, 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_utf8_string, ossl_param_bld_push_utf8_ptr, -ossl_param_bld_push_octet_string, ossl_param_bld_push_octet_ptr +ossl_param_bld_init, ossl_param_bld_to_param, ossl_param_bld_to_param_ex, +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_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 =for comment generic - #include "internal/params_template.h" + #include "internal/params_build.h" #define OSSL_PARAM_BLD_MAX 10 typedef struct { ... } OSSL_PARAM_BLD; void ossl_param_bld_init(OSSL_PARAM_BLD *bld); - OSSL_PARAM *ossl_param_bld_to_param(OSSL_PARAM_BLD *bld, void **secure); + OSSL_PARAM *ossl_param_bld_to_param(OSSL_PARAM_BLD *bld); OSSL_PARAM *ossl_param_bld_to_param_ex(OSSL_PARAM_BLD *bld, OSSL_PARAM *params, size_t param_n, void *data, size_t data_n, void *secure, size_t secure_n); + void ossl_param_bld_free(OSSL_PARAM *params); int ossl_param_bld_push_TYPE(OSSL_PARAM_BLD *bld, const char *key, TYPE val); @@ -55,11 +56,11 @@ Any existing values are cleared. ossl_param_bld_to_param() converts a built up OSSL_PARAM_BLD structure B into an allocated OSSL_PARAM array. -The pointer referenced by the B argument is set to point to an -allocated block of secure memory if required and to NULL it not. -The OSSL_PARAM array and all associated storage can be freed by calling -OPENSSL_free() with the functions return value and OPENSSL_secure_free() -with the pointer referenced by B. +The OSSL_PARAM array and all associated storage must be freed by calling +ossl_param_bld_free() with the functions return value. + +ossl_param_bld_free() deallocates the memory allocated by +ossl_param_bld_to_param(). ossl_param_bld_to_param_ex() behaves like ossl_param_bld_to_param(), except that no additional memory is allocated. @@ -76,8 +77,8 @@ B is stored by value and an expression or auto variable can be used. ossl_param_bld_push_BN() is a function that will create an OSSL_PARAM object that holds the specified BIGNUM B. -If B is marked as being securely allocated, the secure flag is -set in the OSSL_PARAM_BLD structure. +If B is marked as being securely allocated, it's OSSL_PARAM representation +will also be securely allocated. The B argument is stored by reference and the underlying BIGNUM object must exist until after ossl_param_bld_to_param() has been called. @@ -137,7 +138,6 @@ private key. OSSL_PARAM_BLD bld; OSSL_PARAM *params; - void *secure; ossl_param_bld_init(&bld, &secure); if (!ossl_param_bld_push_BN(&bld, "p", p) @@ -149,8 +149,7 @@ private key. goto err; /* Use params */ ... - OPENSSL_free(params); - OPENSSL_secure_free(secure); + ossl_param_bld_free(params); =head2 Example 2 @@ -159,7 +158,6 @@ public key. OSSL_PARAM_BLD bld; OSSL_PARAM *params; - void *secure; ossl_param_bld_init(&bld, &secure); if (!ossl_param_bld_push_BN(&bld, "n", n) @@ -168,8 +166,7 @@ public key. goto err; /* Use params */ ... - OPENSSL_free(params); - OPENSSL_secure_free(secure); + ossl_param_bld_free(params); =head1 SEE ALSO diff --git a/include/internal/param_build.h b/include/internal/param_build.h index 762d7b11c9..e1235eea29 100644 --- a/include/internal/param_build.h +++ b/include/internal/param_build.h @@ -40,7 +40,8 @@ typedef struct { } OSSL_PARAM_BLD; void ossl_param_bld_init(OSSL_PARAM_BLD *bld); -OSSL_PARAM *ossl_param_bld_to_param(OSSL_PARAM_BLD *bld, void **secure); +OSSL_PARAM *ossl_param_bld_to_param(OSSL_PARAM_BLD *bld); +void ossl_param_bld_free(OSSL_PARAM *params); OSSL_PARAM *ossl_param_bld_to_param_ex(OSSL_PARAM_BLD *bld, OSSL_PARAM *params, size_t param_n, void *data, size_t data_n, diff --git a/test/param_build_test.c b/test/param_build_test.c index 278553d3a3..55f6f0eab0 100644 --- a/test/param_build_test.c +++ b/test/param_build_test.c @@ -18,7 +18,7 @@ static int template_public_test(void) { OSSL_PARAM_BLD bld; OSSL_PARAM *params = NULL, *p; - void *secure = (void *)"abc"; + BIGNUM *bn = NULL, *bn_res = NULL; int i; long int l; int32_t i32; @@ -34,12 +34,14 @@ static int template_public_test(void) || !TEST_true(ossl_param_bld_push_int32(&bld, "i32", 1532)) || !TEST_true(ossl_param_bld_push_int64(&bld, "i64", -9999999)) || !TEST_true(ossl_param_bld_push_double(&bld, "d", 1.61803398875)) + || !TEST_ptr(bn = BN_new()) + || !TEST_true(BN_set_word(bn, 1729)) + || !TEST_true(ossl_param_bld_push_BN(&bld, "bignumber", bn)) || !TEST_true(ossl_param_bld_push_utf8_string(&bld, "utf8_s", "foo", sizeof("foo"))) || !TEST_true(ossl_param_bld_push_utf8_ptr(&bld, "utf8_p", "bar-boom", 0)) - || !TEST_ptr(params = ossl_param_bld_to_param(&bld, &secure)) - || !TEST_ptr_null(secure) + || !TEST_ptr(params = ossl_param_bld_to_param(&bld)) /* Check int */ || !TEST_ptr(p = OSSL_PARAM_locate(params, "i")) || !TEST_true(OSSL_PARAM_get_int(p, &i)) @@ -83,13 +85,20 @@ static int template_public_test(void) /* Check UTF8 pointer */ || !TEST_ptr(p = OSSL_PARAM_locate(params, "utf8_p")) || !TEST_true(OSSL_PARAM_get_utf8_ptr(p, &cutf)) - || !TEST_str_eq(cutf, "bar-boom")) + || !TEST_str_eq(cutf, "bar-boom") + /* Check BN */ + || !TEST_ptr(p = OSSL_PARAM_locate(params, "bignumber")) + || !TEST_str_eq(p->key, "bignumber") + || !TEST_uint_eq(p->data_type, OSSL_PARAM_UNSIGNED_INTEGER) + || !TEST_true(OSSL_PARAM_get_BN(p, &bn_res)) + || !TEST_int_eq(BN_cmp(bn_res, bn), 0)) goto err; res = 1; err: - OPENSSL_free(params); - OPENSSL_secure_free(secure); + ossl_param_bld_free(params); OPENSSL_free(utf); + BN_free(bn); + BN_free(bn_res); return res; } @@ -99,7 +108,6 @@ static int template_private_test(void) static unsigned char data2[] = { 2, 4, 6, 8, 10 }; OSSL_PARAM_BLD bld; OSSL_PARAM *params = NULL, *p; - void *secure = (void *)"abc"; unsigned int i; unsigned long int l; uint32_t i32; @@ -114,14 +122,14 @@ static int template_private_test(void) || !TEST_true(ossl_param_bld_push_uint32(&bld, "i32", 1532)) || !TEST_true(ossl_param_bld_push_uint64(&bld, "i64", 9999999)) || !TEST_true(ossl_param_bld_push_size_t(&bld, "st", 65537)) - || !TEST_ptr(bn = BN_new()) + || !TEST_ptr(bn = BN_secure_new()) || !TEST_true(BN_set_word(bn, 1729)) || !TEST_true(ossl_param_bld_push_BN(&bld, "bignumber", bn)) || !TEST_true(ossl_param_bld_push_octet_string(&bld, "oct_s", data1, sizeof(data1))) || !TEST_true(ossl_param_bld_push_octet_ptr(&bld, "oct_p", data2, sizeof(data2))) - || !TEST_ptr(params = ossl_param_bld_to_param(&bld, &secure)) + || !TEST_ptr(params = ossl_param_bld_to_param(&bld)) /* Check unsigned int */ || !TEST_ptr(p = OSSL_PARAM_locate(params, "i")) || !TEST_true(OSSL_PARAM_get_uint(p, &i)) @@ -176,8 +184,7 @@ static int template_private_test(void) goto err; res = 1; err: - OPENSSL_secure_free(secure); - OPENSSL_free(params); + ossl_param_bld_free(params); BN_free(bn); BN_free(bn_res); return res; -- 2.39.2