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 <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10840)

diff --git a/crypto/param_build.c b/crypto/param_build.c
index 01866b01d970fe9966624d3ade571258e48e1711..21bed31393fd8be89acc27e2a6acf74aadb69882 100644 (file)
--- 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 2fb7c4f359afa4ae5d1ce0bef9088ade4aaeb473..545eaf141549f4787e8575944619cba7903ede65 100644 (file)
--- 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<bn> 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<bn>.
+The object will be padded to occupy exactly I<sz> bytes, if insufficient space
+is specified an error results.
+If I<bn> is marked as being securely allocated, it's OSSL_PARAM representation
+will also be securely allocated.
+The I<bn> 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<buf>.
 If the length of the string, I<bsize>, is zero then it will be calculated.

diff --git a/include/internal/param_build.h b/include/internal/param_build.h
index a8116e35cdbd5821ed912e4dda43f6ccbe373ed5..ac1945f6f6bef8c509bdbdbe15782b9f38abce6a 100644 (file)
--- 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 55f6f0eab02f00683cd36e160a8f90eeaf4fcdfc..6d54946cb9f12370ce997c35612d26d99cc579de 100644 (file)
--- 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;
 }