random_seed: use bcrypt instead of the old wincrypt API

Message ID 20180416145004.8406-1-martin@martin.st
State Committed
Commit abf806f7f1601c7e54de7f863bbb816af144a88c
Headers show
Series
  • random_seed: use bcrypt instead of the old wincrypt API
Related show

Commit Message

Martin Storsjö April 16, 2018, 2:50 p.m.
From: Steve Lhomme <robux4@ycbcr.xyz>

Remove the wincrypt API calls since we don't support XP anymore and
bcrypt is available since Vista, even on Windows Store builds.
---
Now with avutil_extralibs sorted alphabetically, and James' extended
configure check included.
---
 configure               |  7 ++++---
 libavutil/random_seed.c | 19 ++++++++++---------
 2 files changed, 14 insertions(+), 12 deletions(-)

Comments

Diego Biurrun April 17, 2018, 8:08 a.m. | #1
On Mon, Apr 16, 2018 at 05:50:04PM +0300, Martin Storsjö wrote:
> From: Steve Lhomme <robux4@ycbcr.xyz>
> 
> Remove the wincrypt API calls since we don't support XP anymore and
> bcrypt is available since Vista, even on Windows Store builds.
> ---
> Now with avutil_extralibs sorted alphabetically, and James' extended
> configure check included.
> 
> --- a/configure
> +++ b/configure
> @@ -4579,9 +4579,10 @@ check_header windows.h
>  
> +check_lib bcrypt   "windows.h bcrypt.h"   BCryptGenRandom      -lbcrypt &&
> +    check_cpp_condition bcrypt bcrypt.h "defined BCRYPT_RNG_ALGORITHM"

This is a workaround for an old, already-obsolete version of mingw64.
Before this shows up in a release it will be even more obsolete.  IMO
such workarounds are not worth the trouble; let the breakage occur where
the actual bugs are and do the fixes at the root. I consider that the
saner longterm strategy. Your call; push whichever version you prefer.

Diego
Martin Storsjö April 19, 2018, 7:56 a.m. | #2
On Tue, 17 Apr 2018, Diego Biurrun wrote:

> On Mon, Apr 16, 2018 at 05:50:04PM +0300, Martin Storsjö wrote:
>> From: Steve Lhomme <robux4@ycbcr.xyz>
>> 
>> Remove the wincrypt API calls since we don't support XP anymore and
>> bcrypt is available since Vista, even on Windows Store builds.
>> ---
>> Now with avutil_extralibs sorted alphabetically, and James' extended
>> configure check included.
>> 
>> --- a/configure
>> +++ b/configure
>> @@ -4579,9 +4579,10 @@ check_header windows.h
>> 
>> +check_lib bcrypt   "windows.h bcrypt.h"   BCryptGenRandom      -lbcrypt &&
>> +    check_cpp_condition bcrypt bcrypt.h "defined BCRYPT_RNG_ALGORITHM"
>
> This is a workaround for an old, already-obsolete version of mingw64.
> Before this shows up in a release it will be even more obsolete.  IMO
> such workarounds are not worth the trouble; let the breakage occur where
> the actual bugs are and do the fixes at the root. I consider that the
> saner longterm strategy. Your call; push whichever version you prefer.

If such versions regardless are common (which James fix would indicate), 
or even "aren't uncommon", I'd prefer to include the extended configure 
check. I wouldn't want to rule out building with a less-than-newest 
version of mingw-w64.

Thus pushed with the extra check.

// Martin

Patch

diff --git a/configure b/configure
index 3c7b6a0981..465fdcfb6d 100755
--- a/configure
+++ b/configure
@@ -1703,12 +1703,12 @@  SYSTEM_FUNCS="
 "
 
 SYSTEM_LIBRARIES="
+    bcrypt
     sdl
     vaapi_1
     vaapi_drm
     vaapi_x11
     vdpau_x11
-    wincrypt
 "
 
 TOOLCHAIN_FEATURES="
@@ -2610,7 +2610,7 @@  avdevice_extralibs="libm_extralibs"
 avformat_extralibs="libm_extralibs"
 avfilter_extralibs="pthreads_extralibs libm_extralibs"
 avresample_extralibs="libm_extralibs"
-avutil_extralibs="clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs wincrypt_extralibs"
+avutil_extralibs="bcrypt_extralibs clock_gettime_extralibs cuda_extralibs cuvid_extralibs d3d11va_extralibs libm_extralibs libmfx_extralibs nanosleep_extralibs pthreads_extralibs user32_extralibs vaapi_extralibs vaapi_drm_extralibs vaapi_x11_extralibs vdpau_x11_extralibs"
 swscale_extralibs="libm_extralibs"
 
 # programs
@@ -4579,9 +4579,10 @@  check_header windows.h
 # so we also check that atomics actually work here
 check_builtin stdatomic stdatomic.h "atomic_int foo; atomic_store(&foo, 0)"
 
+check_lib bcrypt   "windows.h bcrypt.h"   BCryptGenRandom      -lbcrypt &&
+    check_cpp_condition bcrypt bcrypt.h "defined BCRYPT_RNG_ALGORITHM"
 check_lib ole32    "windows.h"            CoTaskMemFree        -lole32
 check_lib shell32  "windows.h shellapi.h" CommandLineToArgvW   -lshell32
-check_lib wincrypt "windows.h wincrypt.h" CryptGenRandom       -ladvapi32
 check_lib psapi    "windows.h psapi.h"    GetProcessMemoryInfo -lpsapi
 
 check_struct "sys/time.h sys/resource.h" "struct rusage" ru_maxrss
diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
index 089d883916..388cb401ba 100644
--- a/libavutil/random_seed.c
+++ b/libavutil/random_seed.c
@@ -23,9 +23,9 @@ 
 #if HAVE_UNISTD_H
 #include <unistd.h>
 #endif
-#if HAVE_WINCRYPT
+#if HAVE_BCRYPT
 #include <windows.h>
-#include <wincrypt.h>
+#include <bcrypt.h>
 #endif
 #include <fcntl.h>
 #include <math.h>
@@ -96,13 +96,14 @@  uint32_t av_get_random_seed(void)
 {
     uint32_t seed;
 
-#if HAVE_WINCRYPT
-    HCRYPTPROV provider;
-    if (CryptAcquireContext(&provider, NULL, NULL, PROV_RSA_FULL,
-                            CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) {
-        BOOL ret = CryptGenRandom(provider, sizeof(seed), (PBYTE) &seed);
-        CryptReleaseContext(provider, 0);
-        if (ret)
+#if HAVE_BCRYPT
+    BCRYPT_ALG_HANDLE algo_handle;
+    NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
+                                               MS_PRIMITIVE_PROVIDER, 0);
+    if (BCRYPT_SUCCESS(ret)) {
+        NTSTATUS ret = BCryptGenRandom(algo_handle, (UCHAR*)&seed, sizeof(seed), 0);
+        BCryptCloseAlgorithmProvider(algo_handle, 0);
+        if (BCRYPT_SUCCESS(ret))
             return seed;
     }
 #endif