mbedTLS: Fix concurrency issues with TLS

When we first integrated mbedTLS, we decided not to enable
MBEDTLS_THREADING_C (which adds mutex locking to calls modifying the
state), and instead to simply create separate contexts ("states") for
each connection.

This worked fine until recently.
Sadly, mbedTLS 3 added a global state for the new PSA crypto
functionalities (which are required to support TLSv1.3).
This results in TLSv1.3 connections to access and modify the global
state concurrently when running in threads.

This commit enables MBEDTLS_THREADING_C, and MBEDTLS_THREADING_C_ALT to
provide a generic Godot implementation using the engine Mutex class.

core/io/resource_uid.cpp

@@ -109,6 +109,13 @@ ResourceUID::ID ResourceUID::text_to_id(const String &p_text) const {
 }
 
 ResourceUID::ID ResourceUID::create_id() {
+	// mbedTLS may not be fully initialized when the ResourceUID is created, so we
+	// need to lazily instantiate the random number generator.
+	if (crypto == nullptr) {
+		crypto = memnew(CryptoCore::RandomGenerator);
+		((CryptoCore::RandomGenerator *)crypto)->init();
+	}
+
 	while (true) {
 		ID id = INVALID_ID;
 		MutexLock lock(mutex);
@@ -363,9 +370,9 @@ ResourceUID *ResourceUID::singleton = nullptr;
 ResourceUID::ResourceUID() {
 	ERR_FAIL_COND(singleton != nullptr);
 	singleton = this;
-	crypto = memnew(CryptoCore::RandomGenerator);
-	((CryptoCore::RandomGenerator *)crypto)->init();
 }
 ResourceUID::~ResourceUID() {
-	memdelete((CryptoCore::RandomGenerator *)crypto);
+	if (crypto != nullptr) {
+		memdelete((CryptoCore::RandomGenerator *)crypto);
+	}
 }

modules/mbedtls/register_types.cpp

@@ -45,15 +45,52 @@
 #include "tests/test_crypto_mbedtls.h"
 #endif
 
+#ifdef GODOT_MBEDTLS_THREADING_ALT
+extern "C" {
+void godot_mbedtls_mutex_init(mbedtls_threading_mutex_t *p_mutex) {
+	ERR_FAIL_NULL(p_mutex);
+	p_mutex->mutex = memnew(Mutex);
+}
+
+void godot_mbedtls_mutex_free(mbedtls_threading_mutex_t *p_mutex) {
+	ERR_FAIL_NULL(p_mutex);
+	ERR_FAIL_NULL(p_mutex->mutex);
+	memdelete((Mutex *)p_mutex->mutex);
+}
+
+int godot_mbedtls_mutex_lock(mbedtls_threading_mutex_t *p_mutex) {
+	ERR_FAIL_NULL_V(p_mutex, MBEDTLS_ERR_THREADING_BAD_INPUT_DATA);
+	ERR_FAIL_NULL_V(p_mutex->mutex, MBEDTLS_ERR_THREADING_BAD_INPUT_DATA);
+	((Mutex *)p_mutex->mutex)->lock();
+	return 0;
+}
+
+int godot_mbedtls_mutex_unlock(mbedtls_threading_mutex_t *p_mutex) {
+	ERR_FAIL_NULL_V(p_mutex, MBEDTLS_ERR_THREADING_BAD_INPUT_DATA);
+	ERR_FAIL_NULL_V(p_mutex->mutex, MBEDTLS_ERR_THREADING_BAD_INPUT_DATA);
+	((Mutex *)p_mutex->mutex)->unlock();
+	return 0;
+}
+};
+#endif
+
 static bool godot_mbedtls_initialized = false;
 
 void initialize_mbedtls_module(ModuleInitializationLevel p_level) {
-	if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
+	if (p_level != MODULE_INITIALIZATION_LEVEL_CORE) {
 		return;
 	}
 
 	GLOBAL_DEF("network/tls/enable_tls_v1.3", true);
 
+#ifdef GODOT_MBEDTLS_THREADING_ALT
+	mbedtls_threading_set_alt(
+			godot_mbedtls_mutex_init,
+			godot_mbedtls_mutex_free,
+			godot_mbedtls_mutex_lock,
+			godot_mbedtls_mutex_unlock);
+#endif
+
 #if MBEDTLS_VERSION_MAJOR >= 3
 	int status = psa_crypto_init();
 	ERR_FAIL_COND_MSG(status != PSA_SUCCESS, "Failed to initialize psa crypto. The mbedTLS modules will not work.");
@@ -74,7 +111,7 @@ void initialize_mbedtls_module(ModuleInitializationLevel p_level) {
 }
 
 void uninitialize_mbedtls_module(ModuleInitializationLevel p_level) {
-	if (p_level != MODULE_INITIALIZATION_LEVEL_SCENE) {
+	if (p_level != MODULE_INITIALIZATION_LEVEL_CORE) {
 		return;
 	}
 
@@ -90,4 +127,8 @@ void uninitialize_mbedtls_module(ModuleInitializationLevel p_level) {
 	PacketPeerMbedDTLS::finalize_dtls();
 	StreamPeerMbedTLS::finalize_tls();
 	CryptoMbedTLS::finalize_crypto();
+
+#ifdef GODOT_MBEDTLS_THREADING_ALT
+	mbedtls_threading_free_alt();
+#endif
 }

thirdparty/README.md

@@ -634,8 +634,8 @@ File extracted from upstream release tarball:
 - The `LICENSE` file (edited to keep only the Apache 2.0 variant)
 - Added 2 files `godot_core_mbedtls_platform.c` and `godot_core_mbedtls_config.h`
   providing configuration for light bundling with core
-- Added the file `godot_module_mbedtls_config.h` to customize the build
-  configuration when bundling the full library
+- Added 2 files `godot_module_mbedtls_config.h` and `threading_alt.h`
+  to customize the build configuration when bundling the full library
 
 Patches:
 

thirdparty/mbedtls/include/godot_module_mbedtls_config.h

@@ -49,6 +49,13 @@
 #undef MBEDTLS_DES_C
 #undef MBEDTLS_DHM_C
 
+#ifdef THREADS_ENABLED
+// In mbedTLS 3, the PSA subsystem has an implicit shared context, MBEDTLS_THREADING_C is required to make it thread safe.
+#define MBEDTLS_THREADING_C
+#define MBEDTLS_THREADING_ALT
+#define GODOT_MBEDTLS_THREADING_ALT
+#endif
+
 #if !(defined(__linux__) && defined(__aarch64__))
 // ARMv8 hardware AES operations. Detection only possible on linux.
 // May technically be supported on some ARM32 arches but doesn't seem

thirdparty/mbedtls/include/threading_alt.h

@@ -0,0 +1,3 @@
+typedef struct {
+	void *mutex;
+} mbedtls_threading_mutex_t;