From ac23078b78305ba7b60d1459cf0db5df96e89d84 Mon Sep 17 00:00:00 2001 From: Pauli Date: Tue, 14 Jan 2020 19:36:39 +1000 Subject: [PATCH] param_bld: add a padded BN call. To aviod leaking size information when passing private value using the OSSL_PARAM builder, a padded BN call is required. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/10840) --- crypto/param_build.c | 23 ++++++++++++++++------- doc/internal/man3/ossl_param_bld_init.pod | 17 ++++++++++++++--- include/internal/param_build.h | 2 ++ test/param_build_test.c | 19 +++++++++++++++++-- 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/crypto/param_build.c b/crypto/param_build.c index 01866b01d9..21bed31393 100644 --- a/crypto/param_build.c +++ b/crypto/param_build.c @@ -138,21 +138,30 @@ int ossl_param_bld_push_double(OSSL_PARAM_BLD *bld, const char *key, int ossl_param_bld_push_BN(OSSL_PARAM_BLD *bld, const char *key, const BIGNUM *bn) { - int sz = -1, secure = 0; + return ossl_param_bld_push_BN_pad(bld, key, bn, + bn == NULL ? 0 : BN_num_bytes(bn)); +} + +int ossl_param_bld_push_BN_pad(OSSL_PARAM_BLD *bld, const char *key, + const BIGNUM *bn, size_t sz) +{ + int n, secure = 0; OSSL_PARAM_BLD_DEF *pd; if (bn != NULL) { - sz = BN_num_bytes(bn); - if (sz < 0) { - CRYPTOerr(CRYPTO_F_OSSL_PARAM_BLD_PUSH_BN, - CRYPTO_R_ZERO_LENGTH_NUMBER); + n = BN_num_bytes(bn); + if (n < 0) { + CRYPTOerr(0, CRYPTO_R_ZERO_LENGTH_NUMBER); + return 0; + } + if (sz < (size_t)n) { + CRYPTOerr(0, CRYPTO_R_TOO_SMALL_BUFFER); return 0; } if (BN_get_flags(bn, BN_FLG_SECURE) == BN_FLG_SECURE) secure = 1; } - pd = param_push(bld, key, sz, sz >= 0 ? sz : 0, - OSSL_PARAM_UNSIGNED_INTEGER, secure); + pd = param_push(bld, key, sz, sz, OSSL_PARAM_UNSIGNED_INTEGER, secure); if (pd == NULL) return 0; pd->bn = bn; diff --git a/doc/internal/man3/ossl_param_bld_init.pod b/doc/internal/man3/ossl_param_bld_init.pod index 2fb7c4f359..545eaf1415 100644 --- a/doc/internal/man3/ossl_param_bld_init.pod +++ b/doc/internal/man3/ossl_param_bld_init.pod @@ -8,9 +8,9 @@ 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_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 @@ -34,6 +34,8 @@ ossl_param_bld_push_octet_ptr int ossl_param_bld_push_BN(OSSL_PARAM_BLD *bld, const char *key, const BIGNUM *bn); + int ossl_param_bld_push_BN_pad(OSSL_PARAM_BLD *bld, const char *key, + const BIGNUM *bn, size_t sz); int ossl_param_bld_push_utf8_string(OSSL_PARAM_BLD *bld, const char *key, const char *buf, size_t bsize); @@ -90,6 +92,15 @@ will also be securely allocated. The I argument is stored by reference and the underlying BIGNUM object must exist until after ossl_param_bld_to_param() has been called. +ossl_param_bld_push_BN_pad() is a function that will create an OSSL_PARAM object +that holds the specified BIGNUM I. +The object will be padded to occupy exactly I bytes, if insufficient space +is specified an error results. +If I is marked as being securely allocated, it's OSSL_PARAM representation +will also be securely allocated. +The I argument is stored by reference and the underlying BIGNUM object +must exist until after ossl_param_bld_to_param() has been called. + ossl_param_bld_push_utf8_string() is a function that will create an OSSL_PARAM object that references the UTF8 string specified by I. If the length of the string, I, is zero then it will be calculated. diff --git a/include/internal/param_build.h b/include/internal/param_build.h index a8116e35cd..ac1945f6f6 100644 --- a/include/internal/param_build.h +++ b/include/internal/param_build.h @@ -68,6 +68,8 @@ int ossl_param_bld_push_double(OSSL_PARAM_BLD *bld, const char *key, double val); int ossl_param_bld_push_BN(OSSL_PARAM_BLD *bld, const char *key, const BIGNUM *bn); +int ossl_param_bld_push_BN_pad(OSSL_PARAM_BLD *bld, const char *key, + const BIGNUM *bn, size_t sz); int ossl_param_bld_push_utf8_string(OSSL_PARAM_BLD *bld, const char *key, const char *buf, size_t bsize); int ossl_param_bld_push_utf8_ptr(OSSL_PARAM_BLD *bld, const char *key, diff --git a/test/param_build_test.c b/test/param_build_test.c index 55f6f0eab0..6d54946cb9 100644 --- a/test/param_build_test.c +++ b/test/param_build_test.c @@ -196,6 +196,8 @@ static int template_static_params_test(int n) OSSL_PARAM_BLD bld; OSSL_PARAM params[20], *p; BIGNUM *bn = NULL, *bn_r = NULL; + BIGNUM *bn0 = NULL, *bn0_r = NULL; + const size_t bn_bytes = 200; unsigned int i; char *utf = NULL; int res = 0; @@ -204,7 +206,11 @@ static int template_static_params_test(int n) if (!TEST_true(ossl_param_bld_push_uint(&bld, "i", 6)) || !TEST_ptr(bn = (n & 1) == 0 ? BN_new() : BN_secure_new()) || !TEST_true(BN_set_word(bn, 1337)) - || !TEST_true(ossl_param_bld_push_BN(&bld, "bn", bn)) + || !TEST_false(ossl_param_bld_push_BN_pad(&bld, "bn", bn, 0)) + || !TEST_false(ossl_param_bld_push_BN_pad(&bld, "bn", bn, 1)) + || !TEST_true(ossl_param_bld_push_BN_pad(&bld, "bn", bn, bn_bytes)) + || !TEST_ptr(bn0 = BN_new()) + || !TEST_true(ossl_param_bld_push_BN_pad(&bld, "bn0", bn0, 0)) || !TEST_true(ossl_param_bld_push_utf8_string(&bld, "utf8_s", "bar", 0)) || !TEST_ptr(ossl_param_bld_to_param_ex(&bld, params, @@ -223,8 +229,15 @@ static int template_static_params_test(int n) || !TEST_true(OSSL_PARAM_get_BN(p, &bn_r)) || !TEST_str_eq(p->key, "bn") || !TEST_uint_eq(p->data_type, OSSL_PARAM_UNSIGNED_INTEGER) - || !TEST_size_t_le(p->data_size, sizeof(BN_ULONG)) + || !TEST_size_t_eq(p->data_size, bn_bytes) || !TEST_uint_eq((unsigned int)BN_get_word(bn_r), 1337) + /* Check BIGNUM zero */ + || !TEST_ptr(p = OSSL_PARAM_locate(params, "bn0")) + || !TEST_true(OSSL_PARAM_get_BN(p, &bn0_r)) + || !TEST_str_eq(p->key, "bn0") + || !TEST_uint_eq(p->data_type, OSSL_PARAM_UNSIGNED_INTEGER) + || !TEST_size_t_eq(p->data_size, 0) + || !TEST_uint_eq((unsigned int)BN_get_word(bn0_r), 0) /* Check UTF8 string */ || !TEST_ptr(p = OSSL_PARAM_locate(params, "utf8_s")) || !TEST_str_eq(p->data, "bar") @@ -236,6 +249,8 @@ err: OPENSSL_free(utf); BN_free(bn); BN_free(bn_r); + BN_free(bn0); + BN_free(bn0_r); return res; } -- 2.25.1