com php-src: refactor php_win32_get_random_bytes( ), take 2: main/main.c win32/build/confut ils.js win32/winutil.c win32/winutil.h

Commit:    d53d0a5dc43869a29f8908687e4790d7f2847830
Author:    Anatol Belski <[email protected]>         Sun, 12 Feb 2017 17:15:50 +0100
Parents:   963981df5898ceb2626d454e2825270c4c977718
Branches:  master

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

Log:
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.

Changed paths:
  M  main/main.c
  M  win32/build/confutils.js
  M  win32/winutil.c
  M  win32/winutil.h


Diff:
diff --git a/main/main.c b/main/main.c
index 9e139ef..07144af 100644
--- a/main/main.c
+++ b/main/main.c
@@ -2095,7 +2095,10 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod
 #endif
 
 #ifdef PHP_WIN32
-	php_win32_init_rng_lock();
+	if (!php_win32_init_random_bytes()) {
+		fprintf(stderr, "\ncrypt algorithm provider initialization failed\n");
+		return FAILURE;
+	}
 #endif
 
 	module_shutdown = 0;
@@ -2409,7 +2412,7 @@ void php_module_shutdown(void)
 #endif
 
 #ifdef PHP_WIN32
-	php_win32_free_rng_lock();
+	(void)php_win32_shutdown_random_bytes();
 #endif
 
 	sapi_flush();
diff --git a/win32/build/confutils.js b/win32/build/confutils.js
index 8c6d3f6..2fb4886 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 b30ff03..8e47d48 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 2898aad..ebd57f1 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)