Skip to content

Commit

Permalink
[tools] Surround mbedTLS's CTR_DRBG operations with spinlocks
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Dmitrii Kuvaiskii authored and mkow committed Feb 8, 2024
1 parent 915691a commit e740728
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion tools/sgx/common/pf_util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit e740728

Please sign in to comment.