From 04c781ad9be626d836682551033cd6beb748a5af Mon Sep 17 00:00:00 2001 From: Daniel Pouzzner Date: Mon, 16 Sep 2024 17:33:25 -0500 Subject: [PATCH] wolfcrypt/src/dh.c: in wc_DhAgree_ct(), implement failsafe constant-time key size fixup, to work around sp-math constant-time key clamping. also fix a -Wunused in src/ssl_load.c:DataToDerBuffer() teased out by configuration permutations. --- src/ssl_load.c | 1 + wolfcrypt/src/dh.c | 63 ++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/ssl_load.c b/src/ssl_load.c index d27295c65b..f20de2c34d 100644 --- a/src/ssl_load.c +++ b/src/ssl_load.c @@ -137,6 +137,7 @@ static int DataToDerBuffer(const unsigned char* buff, word32 len, int format, FreeDer(der); } #else + (void)algId; ret = NOT_COMPILED_IN; #endif } diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c index c8743cbbae..96000d4be8 100644 --- a/wolfcrypt/src/dh.c +++ b/wolfcrypt/src/dh.c @@ -2138,6 +2138,13 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz, #endif #if !defined(WOLFSSL_SP_MATH) + if (ct) { + /* for the constant-time variant, we will probably use more bits in x for + * the modexp than we read from the private key, and those extra bits need + * to be zeroed. + */ + XMEMSET(x, 0, sizeof *x); + } if (mp_init_multi(x, y, z, 0, 0, 0) != MP_OKAY) { #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC) XFREE(z, key->heap, DYNAMIC_TYPE_DH); @@ -2181,13 +2188,11 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz, if (ret == 0) { if (ct) { - if (mp_to_unsigned_bin_len_ct(z, agree, (int)*agreeSz) != MP_OKAY) - ret = MP_TO_E; + ret = mp_to_unsigned_bin_len_ct(z, agree, (int)*agreeSz); } else { - if (mp_to_unsigned_bin(z, agree) != MP_OKAY) - ret = MP_TO_E; - if (ret == 0) + ret = mp_to_unsigned_bin(z, agree); + if (ret == MP_OKAY) *agreeSz = (word32)mp_unsigned_bin_size(z); } } @@ -2296,13 +2301,57 @@ int wc_DhAgree(DhKey* key, byte* agree, word32* agreeSz, const byte* priv, int wc_DhAgree_ct(DhKey* key, byte* agree, word32 *agreeSz, const byte* priv, word32 privSz, const byte* otherPub, word32 pubSz) { + int ret; + word32 requested_agreeSz; +#ifndef WOLFSSL_NO_MALLOC + byte *agree_buffer = NULL; +#else + byte agree_buffer[DH_MAX_SIZE / 8]; +#endif + if (key == NULL || agree == NULL || agreeSz == NULL || priv == NULL || otherPub == NULL) { return BAD_FUNC_ARG; } - return wc_DhAgree_Sync(key, agree, agreeSz, priv, privSz, otherPub, pubSz, - 1); + requested_agreeSz = *agreeSz; + +#ifndef WOLFSSL_NO_MALLOC + agree_buffer = (byte *)XMALLOC(requested_agreeSz, key->heap, + DYNAMIC_TYPE_DH); + if (agree_buffer == NULL) + return MEMORY_E; +#endif + + XMEMSET(agree, 0, requested_agreeSz); + XMEMSET(agree_buffer, 0, requested_agreeSz); + + ret = wc_DhAgree_Sync(key, agree_buffer, agreeSz, priv, privSz, otherPub, + pubSz, 1); + + if (ret == 0) { + /* Arrange for correct fixed-length, right-justified key, even if the + * crypto back end doesn't support it. This assures that the key is + * unconditionally agreed correctly. With some crypto back ends, + * e.g. heapmath, there are no provisions for actual constant time, but + * with others the key computation and clamping is constant time, and + * the unclamping here is also constant time. + */ + byte *agree_src = agree_buffer + *agreeSz - 1, + *agree_dst = agree + requested_agreeSz - 1; + while (agree_dst >= agree) { + word32 mask = (agree_src >= agree_buffer) - 1U;; + agree_src += (mask & requested_agreeSz); + *agree_dst-- = *agree_src--; + } + *agreeSz = requested_agreeSz; + } + +#ifndef WOLFSSL_NO_MALLOC + XFREE(agree_buffer, key->heap, DYNAMIC_TYPE_DH); +#endif + + return ret; } #ifdef WOLFSSL_DH_EXTRA