From e740728548ef52615cffdb64f573a998abdfa61f Mon Sep 17 00:00:00 2001 From: Dmitrii Kuvaiskii Date: Fri, 2 Feb 2024 05:58:50 -0800 Subject: [PATCH] [tools] Surround mbedTLS's CTR_DRBG operations with spinlocks mbedTLS explicitly says that CTR_DRBG operations are not thread-safe. Previously, our Protected Files utility code (currently used only for the `gramine-sgx-pf-crypt` tool) was rendered thread-unsafe because it used CTR_DRBG without any synchronization (note that we do not build mbedTLS with `MBEDTLS_THREADING_C`). This was not a security issue in Gramine because the only user (`gramine-sgx-pf-crypt`) is single-threaded, but it is a potential vulnerability if this utility code is reused in some other, multi-threaded scenarios. To make CTR_DRBG thread-safe, this commit surrounds the relevant logic with spinlocks. The only identified mbedTLS function that needs this is `mbedtls_ctr_drbg_random()`. See also mbedTLS docs. Kudos to Petr Evstifeev (Villain88) who reported this issue. Signed-off-by: Dmitrii Kuvaiskii --- tools/sgx/common/pf_util.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/sgx/common/pf_util.c b/tools/sgx/common/pf_util.c index 2c6a36a257..ebcfca91c9 100644 --- a/tools/sgx/common/pf_util.c +++ b/tools/sgx/common/pf_util.c @@ -25,6 +25,7 @@ #include "api.h" #include "path_utils.h" #include "perm.h" +#include "spinlock.h" #include "util.h" /* High-level protected files helper functions. */ @@ -185,8 +186,14 @@ pf_status_t mbedtls_aes_gcm_decrypt(const pf_key_t* key, const pf_iv_t* iv, cons static mbedtls_entropy_context g_entropy; static mbedtls_ctr_drbg_context g_prng; +/* CTR_DRBG functions of mbedTLS are not thread-safe, must explicitly sync them */ +static spinlock_t g_mbedtls_ctr_drbg_lock = INIT_SPINLOCK_UNLOCKED; + static pf_status_t mbedtls_random(uint8_t* buffer, size_t size) { - if (mbedtls_ctr_drbg_random(&g_prng, buffer, size) != 0) { + spinlock_lock(&g_mbedtls_ctr_drbg_lock); + int ret = mbedtls_ctr_drbg_random(&g_prng, buffer, size); + spinlock_unlock(&g_mbedtls_ctr_drbg_lock); + if (ret != 0) { ERROR("Failed to get random bytes\n"); return PF_STATUS_CALLBACK_FAILED; } @@ -226,7 +233,9 @@ int pf_init(void) { int pf_generate_wrap_key(const char* wrap_key_path) { pf_key_t wrap_key; + spinlock_lock(&g_mbedtls_ctr_drbg_lock); int ret = mbedtls_ctr_drbg_random(&g_prng, (unsigned char*)&wrap_key, sizeof(wrap_key)); + spinlock_unlock(&g_mbedtls_ctr_drbg_lock); if (ret != 0) { ERROR("Failed to read random bytes: %d\n", ret); return ret;