| Message ID | 20200421074851.9328-1-robux4@ycbcr.xyz |
|---|---|
| State | New |
| Headers | show |
| Series | favor bcrypt over wincrypt for the random generator on Windows | expand |
Hello, Any update on this ? This prevents libssp from being usable in UWP apps. (BTW the name of the old API is not wincrypt, the header, but CryptoAPI or CAPI) On 2020-04-21 9:48, Steve Lhomme wrote: > BCrypt is more modern and supported in Universal Apps, Wincrypt is not and > CryptGenRandom is deprecated: > https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom > > BCrypt is available since Vista > https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptopenalgorithmprovider > > It requires linking with bcrypt rather than advapi32 for wincrypt. > --- > libssp/configure.ac | 16 ++++++++++++++++ > libssp/ssp.c | 20 ++++++++++++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/libssp/configure.ac b/libssp/configure.ac > index f30f81c54f6..a39d9e9c992 100644 > --- a/libssp/configure.ac > +++ b/libssp/configure.ac > @@ -158,6 +158,22 @@ else > fi > AC_SUBST(ssp_have_usable_vsnprintf) > > +AC_ARG_ENABLE(bcrypt, > +AS_HELP_STRING([--disable-bcrypt], > + [use bcrypt for random generator on Windows (otherwise wincrypt)]), > + use_win_bcrypt=$enableval, > + use_win_bcrypt=yes) > +if test "x$use_win_bcrypt" != xno; then > + case "$target_os" in > + win32 | pe | mingw32*) > + AC_CHECK_TYPES([BCRYPT_ALG_HANDLE],[ > + LDFLAGS="$LDFLAGS -lbcrypt" > +],[],[#include <windows.h> > +#include <bcrypt.h>]) > + ;; > + esac > +fi > + > AM_PROG_LIBTOOL > ACX_LT_HOST_FLAGS > AC_SUBST(enable_shared) > diff --git a/libssp/ssp.c b/libssp/ssp.c > index 28f3e9cc64a..f07cc41fd4f 100644 > --- a/libssp/ssp.c > +++ b/libssp/ssp.c > @@ -56,7 +56,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see > to the console using "CONOUT$" */ > #if defined (_WIN32) && !defined (__CYGWIN__) > #include <windows.h> > +#ifdef HAVE_BCRYPT_ALG_HANDLE > +#include <bcrypt.h> > +#else > #include <wincrypt.h> > +#endif > # define _PATH_TTY "CONOUT$" > #else > # define _PATH_TTY "/dev/tty" > @@ -77,6 +81,21 @@ __guard_setup (void) > return; > > #if defined (_WIN32) && !defined (__CYGWIN__) > +#ifdef HAVE_BCRYPT_ALG_HANDLE > + BCRYPT_ALG_HANDLE algo = 0; > + NTSTATUS err = BCryptOpenAlgorithmProvider(&algo, BCRYPT_RNG_ALGORITHM, > + NULL, 0); > + if (BCRYPT_SUCCESS(err)) > + { > + if (BCryptGenRandom(algo, (BYTE *)&__stack_chk_guard, > + sizeof (__stack_chk_guard), 0) && __stack_chk_guard != 0) > + { > + BCryptCloseAlgorithmProvider(algo, 0); > + return; > + } > + BCryptCloseAlgorithmProvider(algo, 0); > + } > +#else /* !HAVE_BCRYPT_ALG_HANDLE */ > HCRYPTPROV hprovider = 0; > if (CryptAcquireContext(&hprovider, NULL, NULL, PROV_RSA_FULL, > CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) > @@ -89,6 +108,7 @@ __guard_setup (void) > } > CryptReleaseContext(hprovider, 0); > } > +#endif /* !HAVE_BCRYPT_ALG_HANDLE */ > #else > int fd = open ("/dev/urandom", O_RDONLY); > if (fd != -1) > -- > 2.17.1 >
Steve Lhomme <robux4@ycbcr.xyz> writes: > Hello, > > Any update on this ? This prevents libssp from being usable in UWP apps. > > (BTW the name of the old API is not wincrypt, the header, but CryptoAPI > or CAPI) Sorry for the slow review. I fear most global reviewers would have no idea whether the patch is right or not. Maybe Jon (cc:ed) could comment. Thanks, Richard > > On 2020-04-21 9:48, Steve Lhomme wrote: >> BCrypt is more modern and supported in Universal Apps, Wincrypt is not and >> CryptGenRandom is deprecated: >> https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom >> >> BCrypt is available since Vista >> https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptopenalgorithmprovider >> >> It requires linking with bcrypt rather than advapi32 for wincrypt. >> --- >> libssp/configure.ac | 16 ++++++++++++++++ >> libssp/ssp.c | 20 ++++++++++++++++++++ >> 2 files changed, 36 insertions(+) >> >> diff --git a/libssp/configure.ac b/libssp/configure.ac >> index f30f81c54f6..a39d9e9c992 100644 >> --- a/libssp/configure.ac >> +++ b/libssp/configure.ac >> @@ -158,6 +158,22 @@ else >> fi >> AC_SUBST(ssp_have_usable_vsnprintf) >> >> +AC_ARG_ENABLE(bcrypt, >> +AS_HELP_STRING([--disable-bcrypt], >> + [use bcrypt for random generator on Windows (otherwise wincrypt)]), >> + use_win_bcrypt=$enableval, >> + use_win_bcrypt=yes) >> +if test "x$use_win_bcrypt" != xno; then >> + case "$target_os" in >> + win32 | pe | mingw32*) >> + AC_CHECK_TYPES([BCRYPT_ALG_HANDLE],[ >> + LDFLAGS="$LDFLAGS -lbcrypt" >> +],[],[#include <windows.h> >> +#include <bcrypt.h>]) >> + ;; >> + esac >> +fi >> + >> AM_PROG_LIBTOOL >> ACX_LT_HOST_FLAGS >> AC_SUBST(enable_shared) >> diff --git a/libssp/ssp.c b/libssp/ssp.c >> index 28f3e9cc64a..f07cc41fd4f 100644 >> --- a/libssp/ssp.c >> +++ b/libssp/ssp.c >> @@ -56,7 +56,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >> to the console using "CONOUT$" */ >> #if defined (_WIN32) && !defined (__CYGWIN__) >> #include <windows.h> >> +#ifdef HAVE_BCRYPT_ALG_HANDLE >> +#include <bcrypt.h> >> +#else >> #include <wincrypt.h> >> +#endif >> # define _PATH_TTY "CONOUT$" >> #else >> # define _PATH_TTY "/dev/tty" >> @@ -77,6 +81,21 @@ __guard_setup (void) >> return; >> >> #if defined (_WIN32) && !defined (__CYGWIN__) >> +#ifdef HAVE_BCRYPT_ALG_HANDLE >> + BCRYPT_ALG_HANDLE algo = 0; >> + NTSTATUS err = BCryptOpenAlgorithmProvider(&algo, BCRYPT_RNG_ALGORITHM, >> + NULL, 0); >> + if (BCRYPT_SUCCESS(err)) >> + { >> + if (BCryptGenRandom(algo, (BYTE *)&__stack_chk_guard, >> + sizeof (__stack_chk_guard), 0) && __stack_chk_guard != 0) >> + { >> + BCryptCloseAlgorithmProvider(algo, 0); >> + return; >> + } >> + BCryptCloseAlgorithmProvider(algo, 0); >> + } >> +#else /* !HAVE_BCRYPT_ALG_HANDLE */ >> HCRYPTPROV hprovider = 0; >> if (CryptAcquireContext(&hprovider, NULL, NULL, PROV_RSA_FULL, >> CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) >> @@ -89,6 +108,7 @@ __guard_setup (void) >> } >> CryptReleaseContext(hprovider, 0); >> } >> +#endif /* !HAVE_BCRYPT_ALG_HANDLE */ >> #else >> int fd = open ("/dev/urandom", O_RDONLY); >> if (fd != -1) >> -- >> 2.17.1 >>
Actually I found a but in the patch. The BCryptGenRandom() doesn't return a BOOL like CryptGenRandom so the success needs to be checked differently. I'm sending an updated patch with the wincrypt name change as well. On 2020-06-01 18:30, Richard Sandiford wrote: > Steve Lhomme <robux4@ycbcr.xyz> writes: >> Hello, >> >> Any update on this ? This prevents libssp from being usable in UWP apps. >> >> (BTW the name of the old API is not wincrypt, the header, but CryptoAPI >> or CAPI) > > Sorry for the slow review. I fear most global reviewers would have > no idea whether the patch is right or not. Maybe Jon (cc:ed) could > comment. > > Thanks, > Richard > >> >> On 2020-04-21 9:48, Steve Lhomme wrote: >>> BCrypt is more modern and supported in Universal Apps, Wincrypt is not and >>> CryptGenRandom is deprecated: >>> https://docs.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cryptgenrandom >>> >>> BCrypt is available since Vista >>> https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptopenalgorithmprovider >>> >>> It requires linking with bcrypt rather than advapi32 for wincrypt. >>> --- >>> libssp/configure.ac | 16 ++++++++++++++++ >>> libssp/ssp.c | 20 ++++++++++++++++++++ >>> 2 files changed, 36 insertions(+) >>> >>> diff --git a/libssp/configure.ac b/libssp/configure.ac >>> index f30f81c54f6..a39d9e9c992 100644 >>> --- a/libssp/configure.ac >>> +++ b/libssp/configure.ac >>> @@ -158,6 +158,22 @@ else >>> fi >>> AC_SUBST(ssp_have_usable_vsnprintf) >>> >>> +AC_ARG_ENABLE(bcrypt, >>> +AS_HELP_STRING([--disable-bcrypt], >>> + [use bcrypt for random generator on Windows (otherwise wincrypt)]), >>> + use_win_bcrypt=$enableval, >>> + use_win_bcrypt=yes) >>> +if test "x$use_win_bcrypt" != xno; then >>> + case "$target_os" in >>> + win32 | pe | mingw32*) >>> + AC_CHECK_TYPES([BCRYPT_ALG_HANDLE],[ >>> + LDFLAGS="$LDFLAGS -lbcrypt" >>> +],[],[#include <windows.h> >>> +#include <bcrypt.h>]) >>> + ;; >>> + esac >>> +fi >>> + >>> AM_PROG_LIBTOOL >>> ACX_LT_HOST_FLAGS >>> AC_SUBST(enable_shared) >>> diff --git a/libssp/ssp.c b/libssp/ssp.c >>> index 28f3e9cc64a..f07cc41fd4f 100644 >>> --- a/libssp/ssp.c >>> +++ b/libssp/ssp.c >>> @@ -56,7 +56,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see >>> to the console using "CONOUT$" */ >>> #if defined (_WIN32) && !defined (__CYGWIN__) >>> #include <windows.h> >>> +#ifdef HAVE_BCRYPT_ALG_HANDLE >>> +#include <bcrypt.h> >>> +#else >>> #include <wincrypt.h> >>> +#endif >>> # define _PATH_TTY "CONOUT$" >>> #else >>> # define _PATH_TTY "/dev/tty" >>> @@ -77,6 +81,21 @@ __guard_setup (void) >>> return; >>> >>> #if defined (_WIN32) && !defined (__CYGWIN__) >>> +#ifdef HAVE_BCRYPT_ALG_HANDLE >>> + BCRYPT_ALG_HANDLE algo = 0; >>> + NTSTATUS err = BCryptOpenAlgorithmProvider(&algo, BCRYPT_RNG_ALGORITHM, >>> + NULL, 0); >>> + if (BCRYPT_SUCCESS(err)) >>> + { >>> + if (BCryptGenRandom(algo, (BYTE *)&__stack_chk_guard, >>> + sizeof (__stack_chk_guard), 0) && __stack_chk_guard != 0) >>> + { >>> + BCryptCloseAlgorithmProvider(algo, 0); >>> + return; >>> + } >>> + BCryptCloseAlgorithmProvider(algo, 0); >>> + } >>> +#else /* !HAVE_BCRYPT_ALG_HANDLE */ >>> HCRYPTPROV hprovider = 0; >>> if (CryptAcquireContext(&hprovider, NULL, NULL, PROV_RSA_FULL, >>> CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) >>> @@ -89,6 +108,7 @@ __guard_setup (void) >>> } >>> CryptReleaseContext(hprovider, 0); >>> } >>> +#endif /* !HAVE_BCRYPT_ALG_HANDLE */ >>> #else >>> int fd = open ("/dev/urandom", O_RDONLY); >>> if (fd != -1) >>> -- >>> 2.17.1 >>>
diff --git a/libssp/configure.ac b/libssp/configure.ac index f30f81c54f6..a39d9e9c992 100644 --- a/libssp/configure.ac +++ b/libssp/configure.ac @@ -158,6 +158,22 @@ else fi AC_SUBST(ssp_have_usable_vsnprintf) +AC_ARG_ENABLE(bcrypt, +AS_HELP_STRING([--disable-bcrypt], + [use bcrypt for random generator on Windows (otherwise wincrypt)]), + use_win_bcrypt=$enableval, + use_win_bcrypt=yes) +if test "x$use_win_bcrypt" != xno; then + case "$target_os" in + win32 | pe | mingw32*) + AC_CHECK_TYPES([BCRYPT_ALG_HANDLE],[ + LDFLAGS="$LDFLAGS -lbcrypt" +],[],[#include <windows.h> +#include <bcrypt.h>]) + ;; + esac +fi + AM_PROG_LIBTOOL ACX_LT_HOST_FLAGS AC_SUBST(enable_shared) diff --git a/libssp/ssp.c b/libssp/ssp.c index 28f3e9cc64a..f07cc41fd4f 100644 --- a/libssp/ssp.c +++ b/libssp/ssp.c @@ -56,7 +56,11 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see to the console using "CONOUT$" */ #if defined (_WIN32) && !defined (__CYGWIN__) #include <windows.h> +#ifdef HAVE_BCRYPT_ALG_HANDLE +#include <bcrypt.h> +#else #include <wincrypt.h> +#endif # define _PATH_TTY "CONOUT$" #else # define _PATH_TTY "/dev/tty" @@ -77,6 +81,21 @@ __guard_setup (void) return; #if defined (_WIN32) && !defined (__CYGWIN__) +#ifdef HAVE_BCRYPT_ALG_HANDLE + BCRYPT_ALG_HANDLE algo = 0; + NTSTATUS err = BCryptOpenAlgorithmProvider(&algo, BCRYPT_RNG_ALGORITHM, + NULL, 0); + if (BCRYPT_SUCCESS(err)) + { + if (BCryptGenRandom(algo, (BYTE *)&__stack_chk_guard, + sizeof (__stack_chk_guard), 0) && __stack_chk_guard != 0) + { + BCryptCloseAlgorithmProvider(algo, 0); + return; + } + BCryptCloseAlgorithmProvider(algo, 0); + } +#else /* !HAVE_BCRYPT_ALG_HANDLE */ HCRYPTPROV hprovider = 0; if (CryptAcquireContext(&hprovider, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT | CRYPT_SILENT)) @@ -89,6 +108,7 @@ __guard_setup (void) } CryptReleaseContext(hprovider, 0); } +#endif /* !HAVE_BCRYPT_ALG_HANDLE */ #else int fd = open ("/dev/urandom", O_RDONLY); if (fd != -1)