Change HKDF to alloc the info buffer.

Fixes #19909

I have enforced a maximum bound still but it is much higher.
Note also that TLS13 still uses the 2048 buffer size.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19923)

diff --git a/providers/implementations/kdfs/hkdf.c b/providers/implementations/kdfs/hkdf.c
index 1293d6fc8a491ed81934269bfff6650f892eeb63..867d27c79ebc18bd5d0e47dd0b4e746e9ed1678d 100644 (file)
--- a/providers/implementations/kdfs/hkdf.c
+++ b/providers/implementations/kdfs/hkdf.c
@@ -32,6 +32,7 @@
 #include "internal/e_os.h"
 
 #define HKDF_MAXBUF 2048
+#define HKDF_MAXINFO (32*1024)
 
 static OSSL_FUNC_kdf_newctx_fn kdf_hkdf_new;
 static OSSL_FUNC_kdf_dupctx_fn kdf_hkdf_dup;
@@ -83,7 +84,7 @@ typedef struct {
     size_t label_len;
     unsigned char *data;
     size_t data_len;
-    unsigned char info[HKDF_MAXBUF];
+    unsigned char *info;
     size_t info_len;
 } KDF_HKDF;
 
@@ -120,7 +121,7 @@ static void kdf_hkdf_reset(void *vctx)
     OPENSSL_free(ctx->label);
     OPENSSL_clear_free(ctx->data, ctx->data_len);
     OPENSSL_clear_free(ctx->key, ctx->key_len);
-    OPENSSL_cleanse(ctx->info, ctx->info_len);
+    OPENSSL_clear_free(ctx->info, ctx->info_len);
     memset(ctx, 0, sizeof(*ctx));
     ctx->provctx = provctx;
 }
@@ -142,10 +143,10 @@ static void *kdf_hkdf_dup(void *vctx)
                                      &dest->label, &dest->label_len)
                 || !ossl_prov_memdup(src->data, src->data_len,
                                      &dest->data, &dest->data_len)
+                || !ossl_prov_memdup(src->info, src->info_len,
+                                     &dest->info, &dest->info_len)
                 || !ossl_prov_digest_copy(&dest->digest, &src->digest))
             goto err;
-        memcpy(dest->info, src->info, sizeof(dest->info));
-        dest->info_len = src->info_len;
         dest->mode = src->mode;
     }
     return dest;
@@ -273,6 +274,41 @@ static int hkdf_common_set_ctx_params(KDF_HKDF *ctx, const OSSL_PARAM params[])
     return 1;
 }
 
+/*
+ * Use WPACKET to concat one or more OSSL_KDF_PARAM_INFO fields into a fixed
+ * out buffer of size *outlen.
+ * If out is NULL then outlen is used to return the required buffer size.
+ */
+static int setinfo_fromparams(const OSSL_PARAM *p, unsigned char *out, size_t *outlen)
+{
+    int ret = 0;
+    WPACKET pkt;
+
+    if (out == NULL) {
+        if (!WPACKET_init_null(&pkt, 0))
+            return 0;
+    } else {
+        if (!WPACKET_init_static_len(&pkt, out, *outlen, 0))
+            return 0;
+    }
+
+    for (; p != NULL; p = OSSL_PARAM_locate_const(p + 1, OSSL_KDF_PARAM_INFO)) {
+        if (p->data_type != OSSL_PARAM_OCTET_STRING)
+            goto err;
+        if (p->data != NULL
+                && p->data_size != 0
+                && !WPACKET_memcpy(&pkt, p->data, p->data_size))
+            goto err;
+    }
+    if (!WPACKET_get_total_written(&pkt, outlen)
+            || !WPACKET_finish(&pkt))
+        goto err;
+    ret = 1;
+err:
+    WPACKET_cleanup(&pkt);
+    return ret;
+}
+
 static int kdf_hkdf_set_ctx_params(void *vctx, const OSSL_PARAM params[])
 {
     const OSSL_PARAM *p;
@@ -286,20 +322,26 @@ static int kdf_hkdf_set_ctx_params(void *vctx, const OSSL_PARAM params[])
 
     /* The info fields concatenate, so process them all */
     if ((p = OSSL_PARAM_locate_const(params, OSSL_KDF_PARAM_INFO)) != NULL) {
-        ctx->info_len = 0;
-        for (; p != NULL; p = OSSL_PARAM_locate_const(p + 1,
-                                                      OSSL_KDF_PARAM_INFO)) {
-            const void *q = ctx->info + ctx->info_len;
-            size_t sz = 0;
-
-            if (p->data_size != 0
-                && p->data != NULL
-                && !OSSL_PARAM_get_octet_string(p, (void **)&q,
-                                                HKDF_MAXBUF - ctx->info_len,
-                                                &sz))
-                return 0;
-            ctx->info_len += sz;
-        }
+        size_t sz = 0;
+
+        /* calculate the total size */
+        if (!setinfo_fromparams(p, NULL, &sz))
+            return 0;
+        if (sz > HKDF_MAXINFO)
+            return 0;
+
+        OPENSSL_clear_free(ctx->info, ctx->info_len);
+        ctx->info = NULL;
+        if (sz == 0)
+            return 1;
+        /* Alloc the buffer */
+        ctx->info = OPENSSL_malloc(sz);
+        if (ctx->info == NULL)
+            return 0;
+        ctx->info_len = sz;
+        /* Concat one or more OSSL_KDF_PARAM_INFO fields */
+        if (!setinfo_fromparams(p, ctx->info, &sz))
+            return 0;
     }
     return 1;
 }

diff --git a/test/recipes/30-test_evp_data/evpkdf_hkdf.txt b/test/recipes/30-test_evp_data/evpkdf_hkdf.txt
index d8adb7f732579f8b68c0788ac0dd40fc97f78948..60f92c4db4fba4672be719a3a6c801bf2b64eb94 100644 (file)
--- a/test/recipes/30-test_evp_data/evpkdf_hkdf.txt
+++ b/test/recipes/30-test_evp_data/evpkdf_hkdf.txt
@@ -202,3 +202,14 @@ Ctrl.IKM = hexkey:0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b
 Ctrl.salt = salt:
 Output = da8c8a73
 Result = KDF_DERIVE_ERROR
+
+# Test concat of multiple info (Uses existing test data, and just splits the info into separate fields)
+KDF = HKDF
+Ctrl.mode = mode:EXPAND_ONLY
+Ctrl.digest = digest:SHA1
+Ctrl.IKM = hexkey:8adae09a2a307059478d309b26c4115a224cfaf6
+Ctrl.info = hexinfo:b0b1b2b3b4b5b6b7b8b9babbbcbdbebfc0
+Ctrl.info = hexinfo:c1c2c3
+Ctrl.info = hexinfo:c4c5c6c7c8c9cacbcccdcecfd0d1d2d3d4d5d6d7d8d9
+Ctrl.info = hexinfo:dadbdcdddedfe0e1e2e3e4e5e6e7e8e9eaebecedeeeff0f1f2f3f4f5f6f7f8f9fafbfcfdfeff
+Output = 0bd770a74d1160f7c9f12cd5912a06ebff6adcae899d92191fe4305673ba2ffe8fa3f1a4e5ad79f3f334b3b202b2173c486ea37ce3d397ed034c7f9dfeb15c5e927336d0441f4c4300e2cff0d0900b52d3b4