Accept longer context for TLS 1.2 exporters
authorDaiki Ueno <dueno@redhat.com>
Mon, 23 Oct 2023 04:56:53 +0000 (13:56 +0900)
committerHugo Landau <hlandau@openssl.org>
Thu, 26 Oct 2023 14:47:15 +0000 (15:47 +0100)
While RFC 5705 implies that the maximum length of context for
exporters to be 65535 bytes as the length is embedded in uint16, the
current implementation enforces much smaller limit, which is less than
1024 bytes.  This removes the restriction by dynamically allocating
memory.

Signed-off-by: Daiki Ueno <dueno@redhat.com>
Reviewed-by: Todd Short <todd.short@me.com>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22465)

providers/implementations/kdfs/tls1_prf.c
ssl/t1_enc.c
test/sslapitest.c

index ff305579c324954d772c568839f79f112032b413..2792486924df8401fabb4ba6becc112100e01561 100644 (file)
@@ -69,6 +69,9 @@
 #include "prov/provider_util.h"
 #include "prov/securitycheck.h"
 #include "internal/e_os.h"
+#include "internal/safe_math.h"
+
+OSSL_SAFE_MATH_UNSIGNED(size_t, size_t)
 
 static OSSL_FUNC_kdf_newctx_fn kdf_tls1_prf_new;
 static OSSL_FUNC_kdf_dupctx_fn kdf_tls1_prf_dup;
@@ -85,7 +88,6 @@ static int tls1_prf_alg(EVP_MAC_CTX *mdctx, EVP_MAC_CTX *sha1ctx,
                         const unsigned char *seed, size_t seed_len,
                         unsigned char *out, size_t olen);
 
-#define TLS1_PRF_MAXBUF 1024
 #define TLS_MD_MASTER_SECRET_CONST        "\x6d\x61\x73\x74\x65\x72\x20\x73\x65\x63\x72\x65\x74"
 #define TLS_MD_MASTER_SECRET_CONST_SIZE   13
 
@@ -101,8 +103,8 @@ typedef struct {
     /* Secret value to use for PRF */
     unsigned char *sec;
     size_t seclen;
-    /* Buffer of concatenated seed data */
-    unsigned char seed[TLS1_PRF_MAXBUF];
+    /* Concatenated seed data */
+    unsigned char *seed;
     size_t seedlen;
 } TLS1_PRF;
 
@@ -136,7 +138,7 @@ static void kdf_tls1_prf_reset(void *vctx)
     EVP_MAC_CTX_free(ctx->P_hash);
     EVP_MAC_CTX_free(ctx->P_sha1);
     OPENSSL_clear_free(ctx->sec, ctx->seclen);
-    OPENSSL_cleanse(ctx->seed, ctx->seedlen);
+    OPENSSL_clear_free(ctx->seed, ctx->seedlen);
     memset(ctx, 0, sizeof(*ctx));
     ctx->provctx = provctx;
 }
@@ -156,8 +158,9 @@ static void *kdf_tls1_prf_dup(void *vctx)
             goto err;
         if (!ossl_prov_memdup(src->sec, src->seclen, &dest->sec, &dest->seclen))
             goto err;
-        memcpy(dest->seed, src->seed, src->seedlen);
-        dest->seedlen = src->seedlen;
+        if (!ossl_prov_memdup(src->seed, src->seedlen, &dest->seed,
+                              &dest->seedlen))
+            goto err;
     }
     return dest;
 
@@ -250,16 +253,29 @@ static int kdf_tls1_prf_set_ctx_params(void *vctx, const OSSL_PARAM params[])
     if ((p = OSSL_PARAM_locate_const(params, OSSL_KDF_PARAM_SEED)) != NULL) {
         for (; p != NULL; p = OSSL_PARAM_locate_const(p + 1,
                                                       OSSL_KDF_PARAM_SEED)) {
-            const void *q = ctx->seed + ctx->seedlen;
-            size_t sz = 0;
-
-            if (p->data_size != 0
-                && p->data != NULL
-                && !OSSL_PARAM_get_octet_string(p, (void **)&q,
-                                                TLS1_PRF_MAXBUF - ctx->seedlen,
-                                                &sz))
-                return 0;
-            ctx->seedlen += sz;
+            if (p->data_size != 0 && p->data != NULL) {
+                const void *val = NULL;
+                size_t sz = 0;
+                unsigned char *seed;
+                size_t seedlen;
+                int err = 0;
+
+                if (!OSSL_PARAM_get_octet_string_ptr(p, &val, &sz))
+                    return 0;
+
+                seedlen = safe_add_size_t(ctx->seedlen, sz, &err);
+                if (err)
+                    return 0;
+
+                seed = OPENSSL_clear_realloc(ctx->seed, ctx->seedlen, seedlen);
+                if (!seed)
+                    return 0;
+
+                ctx->seed = seed;
+                if (ossl_assert(sz != 0))
+                    memcpy(ctx->seed + ctx->seedlen, val, sz);
+                ctx->seedlen = seedlen;
+            }
         }
     }
     return 1;
index 673a53ad36a4bb0d1d0b3f88cbe74c397a8bfd9a..15197ffd465aa3b84beac518c977b9f91d61653a 100644 (file)
@@ -463,6 +463,15 @@ int tls1_export_keying_material(SSL_CONNECTION *s, unsigned char *out,
     size_t vallen = 0, currentvalpos;
     int rv = 0;
 
+    /*
+     * RFC 5705 embeds context length as uint16; reject longer context
+     * before proceeding.
+     */
+    if (contextlen > 0xffff) {
+        ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
+        return 0;
+    }
+
     /*
      * construct PRF arguments we construct the PRF argument ourself rather
      * than passing separate values into the TLS PRF to ensure that the
index 88294af16a2d7f064dc0ff1b8b290286ba1118da..61981101bc390cfd162577c62752a0c3ec18e2cd 100644 (file)
@@ -6221,8 +6221,9 @@ static int test_export_key_mat(int tst)
     const char label[LONG_LABEL_LEN + 1] = "test label";
     const unsigned char context[] = "context";
     const unsigned char *emptycontext = NULL;
-    unsigned char ckeymat1[80], ckeymat2[80], ckeymat3[80];
-    unsigned char skeymat1[80], skeymat2[80], skeymat3[80];
+    unsigned char longcontext[1280];
+    unsigned char ckeymat1[80], ckeymat2[80], ckeymat3[80], ckeymat4[80];
+    unsigned char skeymat1[80], skeymat2[80], skeymat3[80], skeymat4[80];
     size_t labellen;
     const int protocols[] = {
         TLS1_VERSION,
@@ -6300,6 +6301,8 @@ static int test_export_key_mat(int tst)
         labellen = SMALL_LABEL_LEN;
     }
 
+    memset(longcontext, 1, sizeof(longcontext));
+
     if (!TEST_int_eq(SSL_export_keying_material(clientssl, ckeymat1,
                                                 sizeof(ckeymat1), label,
                                                 labellen, context,
@@ -6313,6 +6316,12 @@ static int test_export_key_mat(int tst)
                                                        sizeof(ckeymat3), label,
                                                        labellen,
                                                        NULL, 0, 0), 1)
+            || !TEST_int_eq(SSL_export_keying_material(clientssl, ckeymat4,
+                                                       sizeof(ckeymat4), label,
+                                                       labellen,
+                                                       longcontext,
+                                                       sizeof(longcontext), 1),
+                            1)
             || !TEST_int_eq(SSL_export_keying_material(serverssl, skeymat1,
                                                        sizeof(skeymat1), label,
                                                        labellen,
@@ -6328,6 +6337,12 @@ static int test_export_key_mat(int tst)
                                                        sizeof(skeymat3), label,
                                                        labellen,
                                                        NULL, 0, 0), 1)
+            || !TEST_int_eq(SSL_export_keying_material(serverssl, skeymat4,
+                                                       sizeof(skeymat4), label,
+                                                       labellen,
+                                                       longcontext,
+                                                       sizeof(longcontext), 1),
+                            1)
                /*
                 * Check that both sides created the same key material with the
                 * same context.
@@ -6346,6 +6361,12 @@ static int test_export_key_mat(int tst)
                 */
             || !TEST_mem_eq(ckeymat3, sizeof(ckeymat3), skeymat3,
                             sizeof(skeymat3))
+               /*
+                * Check that both sides created the same key material with a
+                * long context.
+                */
+            || !TEST_mem_eq(ckeymat4, sizeof(ckeymat4), skeymat4,
+                            sizeof(skeymat4))
                /* Different contexts should produce different results */
             || !TEST_mem_ne(ckeymat1, sizeof(ckeymat1), ckeymat2,
                             sizeof(ckeymat2)))