com php-src: Revert "refactor php_win32_get_random_ bytes()": main/main.c win32/winutil.c win 32/winutil.h

[email protected] (Anatol Belski)
Newsgroups php.cvs
Message-ID <[email protected]>
Commit:    048aec47963ac86580a96ff64fa6d16de2f86045
Author:    Anatol Belski <[email protected]>         Sat, 11 Feb 2017 21:47:09 +0100
Parents:   23bd7bcde03c31e2678f23f12c72c96c24800c92
Branches:  master

Link:       http://git.php.net/?p=php-src.git;a=commitdiff;h=048aec47963ac86580a96ff64fa6d16de2f86045

Log:
Revert "refactor php_win32_get_random_bytes()"

This reverts commit 23bd7bcde03c31e2678f23f12c72c96c24800c92.

Looks like this change is unstable. If same CSP is use but multiple processers,
the initialization failures are possible. Thus, CryptAcquireContext in
every process, even if it won't be used at all, is not sensible. This
might actually motivate to look for better CSP APIs.

Changed paths:
  M  main/main.c
  M  win32/winutil.c
  M  win32/winutil.h


Diff:
diff --git a/main/main.c b/main/main.c
index 038981c..9e139ef 100644
--- a/main/main.c
+++ b/main/main.c
@@ -2095,10 +2095,7 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod
 #endif
 
 #ifdef PHP_WIN32
-	if (!php_win32_init_random_bytes()) {
-		php_printf("\ncrypto service provider failed\n");
-		return FAILURE;
-	}
+	php_win32_init_rng_lock();
 #endif
 
 	module_shutdown = 0;
@@ -2412,7 +2409,7 @@ void php_module_shutdown(void)
 #endif
 
 #ifdef PHP_WIN32
-	(void)php_win32_shutdown_random_bytes();
+	php_win32_free_rng_lock();
 #endif
 
 	sapi_flush();
diff --git a/win32/winutil.c b/win32/winutil.c
index f735597..b30ff03 100644
--- a/win32/winutil.c
+++ b/win32/winutil.c
@@ -51,50 +51,77 @@ int php_win32_check_trailing_space(const char * path, const int path_len) {
 	}
 }
 
-static HCRYPTPROV hCryptProv;
-static BOOL has_crypto_ctx = 0;
+HCRYPTPROV   hCryptProv;
+unsigned int has_crypto_ctx = 0;
 
-#ifdef PHP_EXPORTS
-BOOL php_win32_init_random_bytes(void)
+#ifdef ZTS
+MUTEX_T php_lock_win32_cryptoctx;
+void php_win32_init_rng_lock()
 {
-	int err;
-
-	/* CRYPT_VERIFYCONTEXT > only hashing&co-like use, no need to acces prv keys */
-	has_crypto_ctx = CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL, CRYPT_MACHINE_KEYSET|CRYPT_VERIFYCONTEXT);
-	err = GetLastError();
-	if (!has_crypto_ctx) {
-		/* Could mean that the key container does not exist, let try
-		   again by asking for a new one. If it fails here, it surely means that the user running
-		   this process does not have the permission(s) to use this container.
-		 */
-		if (NTE_BAD_KEYSET == err) {
-			has_crypto_ctx = CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL, CRYPT_NEWKEYSET | CRYPT_MACHINE_KEYSET | CRYPT_VERIFYCONTEXT);
-		}
-	}
-
-	return has_crypto_ctx;
+	php_lock_win32_cryptoctx = tsrm_mutex_alloc();
 }
 
-BOOL php_win32_shutdown_random_bytes(void)
+void php_win32_free_rng_lock()
 {
-	BOOL ret = TRUE;
-
-	if (has_crypto_ctx) {
-		ret = CryptReleaseContext(hCryptProv, 0);
+	tsrm_mutex_lock(php_lock_win32_cryptoctx);
+	if (has_crypto_ctx == 1) {
+		CryptReleaseContext(hCryptProv, 0);
+		has_crypto_ctx = 0;
 	}
+	tsrm_mutex_unlock(php_lock_win32_cryptoctx);
+	tsrm_mutex_free(php_lock_win32_cryptoctx);
 
-	return ret;
 }
+#else
+#define php_win32_init_rng_lock();
+#define php_win32_free_rng_lock();
 #endif
 
+
+
 PHP_WINUTIL_API int php_win32_get_random_bytes(unsigned char *buf, size_t size) {  /* {{{ */
 
 	BOOL ret;
 
+#ifdef ZTS
+	tsrm_mutex_lock(php_lock_win32_cryptoctx);
+#endif
+
+	if (has_crypto_ctx == 0) {
+		/* CRYPT_VERIFYCONTEXT > only hashing&co-like use, no need to acces prv keys */
+		if (!CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL, CRYPT_MACHINE_KEYSET|CRYPT_VERIFYCONTEXT )) {
+			/* Could mean that the key container does not exist, let try
+			   again by asking for a new one. If it fails here, it surely means that the user running
+               this process does not have the permission(s) to use this container.
+             */
+			if (GetLastError() == NTE_BAD_KEYSET) {
+				if (CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL, CRYPT_NEWKEYSET | CRYPT_MACHINE_KEYSET | CRYPT_VERIFYCONTEXT )) {
+					has_crypto_ctx = 1;
+				} else {
+					has_crypto_ctx = 0;
+				}
+			}
+		} else {
+			has_crypto_ctx = 1;
+		}
+	}
+
+#ifdef ZTS
+	tsrm_mutex_unlock(php_lock_win32_cryptoctx);
+#endif
+
+	if (has_crypto_ctx == 0) {
+		return FAILURE;
+	}
+
 	/* XXX should go in the loop if size exceeds UINT_MAX */
 	ret = CryptGenRandom(hCryptProv, (DWORD)size, buf);
 
-	return ret ? SUCCESS : FAILURE;
+	if (ret) {
+		return SUCCESS;
+	} else {
+		return FAILURE;
+	}
 }
 /* }}} */
 
diff --git a/win32/winutil.h b/win32/winutil.h
index ebd57f1..2898aad 100644
--- a/win32/winutil.h
+++ b/win32/winutil.h
@@ -27,9 +27,13 @@ PHP_WINUTIL_API char *php_win32_error_to_msg(HRESULT error);
 #define php_win_err()	php_win32_error_to_msg(GetLastError())
 int php_win32_check_trailing_space(const char * path, const int path_len);
 PHP_WINUTIL_API int php_win32_get_random_bytes(unsigned char *buf, size_t size);
-#ifdef PHP_EXPORTS
-BOOL php_win32_init_random_bytes(void);
-BOOL php_win32_shutdown_random_bytes(void);
+
+#ifdef ZTS
+void php_win32_init_rng_lock();
+void php_win32_free_rng_lock();
+#else
+#define php_win32_init_rng_lock();
+#define php_win32_free_rng_lock();
 #endif
 
 #if !defined(ECURDIR)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.