[PATCH] Windows port add support to BCryptGenRandom
Hi,
According to microsoft documentation at:
https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom
The function CryptGenRandom is deprecated, and may can be removed in future release.
This patch add support to use BCryptGenRandom.
BCryptGenRandom apparently works without having to set up an environment before calling.
The drawback, its change causes need to link to bcrypt.lib.
regards,
Ranier Vilela
Attachments:
pg_strong_random_v1.patchtext/x-patch; name=pg_strong_random_v1.patchDownload
diff --git a/src/port/pg_strong_random.c b/src/port/pg_strong_random.c
index 6be5874cbf..8199f89a37 100644
--- a/src/port/pg_strong_random.c
+++ b/src/port/pg_strong_random.c
@@ -28,10 +28,20 @@
#include <openssl/rand.h>
#endif
#ifdef USE_WIN32_RANDOM
-#include <wincrypt.h>
+#if defined(_MSC_VER) && _MSC_VER >= 1900 \
+ && defined(MIN_WINNT) && MIN_WINNT >= 0x0600
+#define USE_WIN32_BCRYPTGENRANDOM
+#endif
#endif
-#ifdef USE_WIN32_RANDOM
+#ifdef USE_WIN32_BCRYPTGENRANDOM
+#include <bcrypt.h>
+#ifndef STATUS_SUCCESS
+ #define STATUS_SUCCESS ((NTSTATUS)0x00000000L)
+#endif
+#elif USE_WIN32_RANDOM
+#include <wincrypt.h>
+
/*
* Cache a global crypto provider that only gets freed when the process
* exits, in case we need random numbers more than once.
@@ -85,8 +95,9 @@ random_from_file(const char *filename, void *buf, size_t len)
* We support a number of sources:
*
* 1. OpenSSL's RAND_bytes()
- * 2. Windows' CryptGenRandom() function
- * 3. /dev/urandom
+ * 2. Windows' BCryptGenRandom() function
+ * 3. Windows' CryptGenRandom() function
+ * 4. /dev/urandom
*
* The configure script will choose which one to use, and set
* a USE_*_RANDOM flag accordingly.
@@ -139,6 +150,10 @@ pg_strong_random(void *buf, size_t len)
/*
* Windows has CryptoAPI for strong cryptographic numbers.
*/
+#elif defined(USE_WIN32_BCRYPTGENRANDOM)
+ return (BCryptGenRandom(NULL, buf, len,
+ BCRYPT_USE_SYSTEM_PREFERRED_RNG) == STATUS_SUCCESS);
+
#elif defined(USE_WIN32_RANDOM)
if (hProvider == 0)
{
Forget Mkvcbuild_v1.patch
regards,
Ranier Vilela
Attachments:
Mkvcbuild_v1.patchtext/x-patch; name=Mkvcbuild_v1.patchDownload
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 275f3bb699..33dc9bf5ad 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -65,7 +65,7 @@ my @frontend_uselibpgcommon = (
my $frontend_extralibs = {
'initdb' => ['ws2_32.lib'],
'pg_restore' => ['ws2_32.lib'],
- 'pgbench' => ['ws2_32.lib'],
+ 'pgbench' => ['ws2_32.lib', 'bcrypt.lib'],
'psql' => ['ws2_32.lib']
};
my $frontend_extraincludes = {
@@ -184,6 +184,7 @@ sub mkvcbuild
$postgres->AddFiles('src/backend/utils/adt', 'jsonpath_scan.l',
'jsonpath_gram.y');
$postgres->AddDefine('BUILDING_DLL');
+ $postgres->AddLibrary('bcrypt.lib') if ($vsVersion > '12.00');
$postgres->AddLibrary('secur32.lib');
$postgres->AddLibrary('ws2_32.lib');
$postgres->AddLibrary('wldap32.lib') if ($solution->{options}->{ldap});
@@ -246,6 +247,7 @@ sub mkvcbuild
$libpq->AddDefine('FRONTEND');
$libpq->AddDefine('UNSAFE_STAT_OK');
$libpq->AddIncludeDir('src/port');
+ $libpq->AddLibrary('bcrypt.lib') if ($vsVersion > '12.00');
$libpq->AddLibrary('secur32.lib');
$libpq->AddLibrary('ws2_32.lib');
$libpq->AddLibrary('wldap32.lib') if ($solution->{options}->{ldap});
On Mon, Dec 16, 2019 at 09:18:10PM +0000, Ranier Vilela wrote:
According to microsoft documentation at:
https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom
The function CryptGenRandom is deprecated, and may can be removed in future release.
This patch add support to use BCryptGenRandom.
+#if defined(_MSC_VER) && _MSC_VER >= 1900 \
+ && defined(MIN_WINNT) && MIN_WINNT >= 0x0600
+#define USE_WIN32_BCRYPTGENRANDOM
[...]
+ $postgres->AddLibrary('bcrypt.lib') if ($vsVersion > '12.00');And looking at this page, it is said that the minimum version
supported by this function is Windows 2008:
https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom
Now, your changes in MkvcBuild.pm and the backend code assume that
we need to include bcrypt.lib since MSVC 2015 (at least version
14.00 or _MSC_VER >= 1900. Do you have a reference about when this
has been introduced in VS? The MS docs don't seem to hold a hint
about that..
--
Michael
De: Michael Paquier
Enviadas: Terça-feira, 17 de Dezembro de 2019 03:43
Para: Ranier Vilela
Cc: pgsql-hackers@lists.postgresql.org
Assunto: Re: [PATCH] Windows port add support to BCryptGenRandom
And looking at this page, it is said that the minimum version
supported by this function is Windows 2008:
https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom<https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom>
Now, your changes in MkvcBuild.pm and the backend code assume that
we need to include bcrypt.lib since MSVC 2015 (at least version
14.00 or _MSC_VER >= 1900. Do you have a reference about when this
has been introduced in VS? The MS docs don't seem to hold a hint
about that..
Sorry Perl I understand a little bit.
Windows Vista I believe.
https://github.com/openssl/openssl/blob/master/crypto/rand/rand_win.c
is the primary font and have more information.
Best regards,
Ranier Vilela
[https://avatars0.githubusercontent.com/u/3279138?s=400&v=4]<https://github.com/openssl/openssl/blob/master/crypto/rand/rand_win.c>
openssl/rand_win.c at master · openssl/openssl · GitHub<https://github.com/openssl/openssl/blob/master/crypto/rand/rand_win.c>
TLS/SSL and crypto library. Contribute to openssl/openssl development by creating an account on GitHub.
github.com
Import Notes
Resolved by subject fallback
On Tue, Dec 17, 2019 at 03:57:56AM +0000, Ranier Vilela wrote:
Windows Vista I believe.
https://github.com/openssl/openssl/blob/master/crypto/rand/rand_win.c
is the primary font and have more information.
So, this basically matches with what the MS documents tell us, and my
impression: this API is available down to at least MSVC 2008, which is
much more than what we support on HEAD where one can use MSVC 2013 and
newer versions. Note that for the minimal platforms supported our
documentation cite Windows Server 2008 R2 SP1 and Windows 7, implying
_WIN32_WINNT >= 0x0600.
In short, this means two things:
- Your patch, as presented, is wrong.
- There is no need to make conditional the use of BCryptGenRandom.
--
Michael
De: Michael Paquier
Enviadas: Terça-feira, 17 de Dezembro de 2019 04:34
So, this basically matches with what the MS documents tell us, and my
impression: this API is available down to at least MSVC 2008, which is
much more than what we support on HEAD where one can use MSVC 2013 and
newer versions. Note that for the minimal platforms supported our
documentation cite Windows Server 2008 R2 SP1 and Windows 7, implying
_WIN32_WINNT >= 0x0600.
As concern [1], at src/include/port/win32.h, the comments still references Windows XP and claims about possible MingW break.
In short, this means two things:
- Your patch, as presented, is wrong.
Well, I try correct him to target MSVC 2013.
There is no need to make conditional the use of BCryptGenRandom.
If legacy Windows Crypto API still remain, and the patch can broken MingW, I believe as necessary conditional use of BCryptGenRandom.
Best regards,
Ranier Vilela
Import Notes
Resolved by subject fallback
On Tue, Dec 17, 2019 at 02:20:20PM +0000, Ranier Vilela wrote:
As concern [1], at src/include/port/win32.h, the comments still
references Windows XP and claims about possible MingW break.
This looks like a leftover of d9dd406, which has made the code to
require C99. As we don't support compilation with Windows XP and
require Windows 7, we should be able to remove all the dance around
MIN_WINNT in win32.h, don't you think?
--
Michael
On Wed, Dec 18, 2019 at 3:20 AM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Dec 17, 2019 at 02:20:20PM +0000, Ranier Vilela wrote:
As concern [1], at src/include/port/win32.h, the comments still
references Windows XP and claims about possible MingW break.This looks like a leftover of d9dd406, which has made the code to
require C99. As we don't support compilation with Windows XP and
require Windows 7, we should be able to remove all the dance around
MIN_WINNT in win32.h, don't you think?
+1, there is a reference in [1]https://www.postgresql.org/docs/current/install-windows.html about that is possible to build PostgreSQL
using the GNU compiler tools for older versions of Windows, that should be
also updated.
[1]: https://www.postgresql.org/docs/current/install-windows.html
Regards,
Juan José Santamaría Flecha
De: Michael Paquier
Enviadas: Quarta-feira, 18 de Dezembro de 2019 02:19
This looks like a leftover of d9dd406, which has made the code to
require C99. As we don't support compilation with Windows XP and
require Windows 7, we should be able to remove all the dance around
MIN_WINNT in win32.h, don't you think?
It would be a good thing since there is no support for these old systems.
And whenever there is a patch that touches windows, someone could complain that it would be breaking something.
Can You help improve the support of BCryptoGenRandom?
I still have doubts about:
1. This break MingW?
2. Legacy API, have to stay?
3. Perl support, pgbench specifically.
If legacy API, have to stay, I have no doubt that it needs to be guarded by conditionals.
Best regards,
Ranier Vilela
1.
Import Notes
Resolved by subject fallback
On Wed, Dec 18, 2019 at 09:52:07AM +0100, Juan José Santamaría Flecha wrote:
+1, there is a reference in [1] about that is possible to build PostgreSQL
using the GNU compiler tools for older versions of Windows, that should be
also updated.
There is actually a little bit more which could be cleaned up. I am
going to begin a new thread on that after finishing looking.
[1] https://www.postgresql.org/docs/current/install-windows.html
Are you referring to the part about cygwin? We could remove all the
paragraph..
--
Michael