refactor php_win32_get_random_bytes(), take 2

As in previous variant, locking is removed and the initialization
is done only once at process start. The CNG API turns out to be
faster, also the initialization is less resources hungry. The
initialization part could need to be improved, if too much startup
failures are sighted in the real world usage. Though that would mean
having locking back.

The usage of CNG was already pointed out and requested in several
reports, with the further refactoring it appears to make sense and
simplify things a backward compatible way.

3 files changed, 45 insertions, 61 deletions

diff --git a/win32/build/confutils.js b/win32/build/confutils.js
index 8c6d3f6f05..2fb4886acd 100644
--- a/win32/build/confutils.js
+++ b/win32/build/confutils.js
@@ -3120,7 +3120,7 @@ function toolset_setup_common_ldlags()
 function toolset_setup_common_libs()
 {
 	// urlmon.lib ole32.lib oleaut32.lib uuid.lib gdi32.lib winspool.lib comdlg32.lib
-	DEFINE("LIBS", "kernel32.lib ole32.lib user32.lib advapi32.lib shell32.lib ws2_32.lib Dnsapi.lib psapi.lib");
+	DEFINE("LIBS", "kernel32.lib ole32.lib user32.lib advapi32.lib shell32.lib ws2_32.lib Dnsapi.lib psapi.lib bcrypt.lib");
 }
 
 function toolset_setup_build_mode()
diff --git a/win32/winutil.c b/win32/winutil.c
index b30ff0394d..8e47d48760 100644
--- a/win32/winutil.c
+++ b/win32/winutil.c
@@ -21,7 +21,7 @@
 
 #include "php.h"
 #include "winutil.h"
-#include <wincrypt.h>
+#include <bcrypt.h>
 #include <lmcons.h>
 
 PHP_WINUTIL_API char *php_win32_error_to_msg(HRESULT error)
@@ -51,77 +51,65 @@ int php_win32_check_trailing_space(const char * path, const int path_len) {
 	}
 }
 
-HCRYPTPROV   hCryptProv;
-unsigned int has_crypto_ctx = 0;
+static BCRYPT_ALG_HANDLE bcrypt_algo;
+static BOOL has_crypto_ctx = 0;
 
-#ifdef ZTS
-MUTEX_T php_lock_win32_cryptoctx;
-void php_win32_init_rng_lock()
-{
-	php_lock_win32_cryptoctx = tsrm_mutex_alloc();
-}
+#define NT_SUCCESS(Status) (((NTSTATUS)(Status)) >= 0)
 
-void php_win32_free_rng_lock()
+#ifdef PHP_EXPORTS
+BOOL php_win32_shutdown_random_bytes(void)
 {
-	tsrm_mutex_lock(php_lock_win32_cryptoctx);
-	if (has_crypto_ctx == 1) {
-		CryptReleaseContext(hCryptProv, 0);
+	BOOL ret = TRUE;
+
+	if (has_crypto_ctx) {
+		ret = NT_SUCCESS(BCryptCloseAlgorithmProvider(bcrypt_algo, 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 php_win32_init_random_bytes(void)
+{
+	if (has_crypto_ctx) {
+		return TRUE;
+	}
 
-	BOOL ret;
+	has_crypto_ctx = NT_SUCCESS(BCryptOpenAlgorithmProvider(&bcrypt_algo, BCRYPT_RNG_ALGORITHM, NULL, 0));
 
-#ifdef ZTS
-	tsrm_mutex_lock(php_lock_win32_cryptoctx);
+	return has_crypto_ctx;
+}
 #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;
-		}
-	}
+PHP_WINUTIL_API int php_win32_get_random_bytes(unsigned char *buf, size_t size) {  /* {{{ */
 
-#ifdef ZTS
-	tsrm_mutex_unlock(php_lock_win32_cryptoctx);
-#endif
+	BOOL ret;
+	size_t got = 0;
 
+#if 0
+	/* Currently we fail on startup, with CNG API it shows no regressions so far and is secure.
+		Should switch on and try to reinit, if it fails too often on startup. This means also
+		bringing locks back. */
 	if (has_crypto_ctx == 0) {
 		return FAILURE;
 	}
+#endif
 
-	/* XXX should go in the loop if size exceeds UINT_MAX */
-	ret = CryptGenRandom(hCryptProv, (DWORD)size, buf);
+#if ZEND_ENABLE_ZVAL_LONG64
+	do {
+		ULONG to_read = (ULONG)(size - got);
+		ret = ret && NT_SUCCESS(BCryptGenRandom(bcrypt_algo, buf, to_read, 0));
+		if (ret) {
+			got += to_read;
+			buf += to_read;
+		}
+	} while (ret && got < size);
+#else
+	ret = NT_SUCCESS(BCryptGenRandom(bcrypt_algo, buf, size, 0));
+#endif
+	assert(got == size);
 
-	if (ret) {
-		return SUCCESS;
-	} else {
-		return FAILURE;
-	}
+	return ret ? SUCCESS : FAILURE;
 }
 /* }}} */
 
diff --git a/win32/winutil.h b/win32/winutil.h
index 2898aad8b8..ebd57f10a6 100644
--- a/win32/winutil.h
+++ b/win32/winutil.h
@@ -27,13 +27,9 @@ 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 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();
+#ifdef PHP_EXPORTS
+BOOL php_win32_init_random_bytes(void);
+BOOL php_win32_shutdown_random_bytes(void);
 #endif
 
 #if !defined(ECURDIR)