Merge pull request #106167 from Faless/mbedtls/fix_enable_threading

mbedTLS: Fix concurrency issues with TLS

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

@@ -636,8 +636,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;