Add param builder free function.
authorPauli <paul.dale@oracle.com>
Thu, 18 Jul 2019 15:14:07 +0000 (01:14 +1000)
committerPauli <paul.dale@oracle.com>
Thu, 18 Jul 2019 15:14:07 +0000 (01:14 +1000)
This means include deallocation information in the return from
the ossl_param_bld_to_param function.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9404)

crypto/param_build.c
doc/internal/man3/ossl_param_bld_init.pod
include/internal/param_build.h
test/param_build_test.c

index 851b735..4d28c87 100644 (file)
@@ -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,
index a65aa8b..2385ffc 100644 (file)
@@ -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<bld> into an allocated OSSL_PARAM array.
-The pointer referenced by the B<secure> 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<secure>.
+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<val> 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<bn>.
-If B<bn> is marked as being securely allocated, the secure flag is
-set in the OSSL_PARAM_BLD structure.
+If B<bn> is marked as being securely allocated, it's OSSL_PARAM representation
+will also be securely allocated.
 The B<bn> 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
 
index 762d7b1..e1235ee 100644 (file)
@@ -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,
index 278553d..55f6f0e 100644 (file)
@@ -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;