[PATCH 0/2] MR9447: bcrypt: Complete the remaining four RSA parameters when importing the private key.
Some versions of libgnutls require passing all parameters when importing private keys, otherwise they will crash. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- configure | 103 ++++++++++++++++++++++++++++++++++++++++++++ configure.ac | 8 ++++ include/config.h.in | 6 +++ 3 files changed, 117 insertions(+) diff --git a/configure b/configure index 180fd1af6a3..b10c94dda4d 100755 --- a/configure +++ b/configure @@ -692,6 +692,8 @@ SANE_LIBS SANE_CFLAGS HWLOC_LIBS HWLOC_CFLAGS +GMP_LIBS +GMP_CFLAGS GNUTLS_LIBS GNUTLS_CFLAGS DBUS_LIBS @@ -1867,6 +1869,8 @@ GNUTLS_CFLAGS GNUTLS_LIBS HWLOC_CFLAGS HWLOC_LIBS +GMP_CFLAGS +GMP_LIBS SANE_CFLAGS SANE_LIBS USB_CFLAGS @@ -2725,6 +2729,8 @@ Some influential environment variables: HWLOC_CFLAGS C compiler flags for hwloc, overriding pkg-config HWLOC_LIBS Linker flags for hwloc, overriding pkg-config + GMP_CFLAGS C compiler flags for gmp, overriding pkg-config + GMP_LIBS Linker flags for gmp, overriding pkg-config SANE_CFLAGS C compiler flags for sane-backends, overriding pkg-config SANE_LIBS Linker flags for sane-backends, overriding pkg-config USB_CFLAGS C compiler flags for libusb-1.0, overriding pkg-config @@ -17208,6 +17214,101 @@ fi esac fi +if test "x$with_gnutls" != "xno" +then + rm -f conftest.err +if ${GMP_CFLAGS:+false} :; then : + if ${PKG_CONFIG+:} false; then : + GMP_CFLAGS=`$PKG_CONFIG --cflags gmp 2>conftest.err` +fi +fi + +if ${GMP_LIBS:+false} :; then : + if ${PKG_CONFIG+:} false; then : + GMP_LIBS=`$PKG_CONFIG --libs gmp 2>/dev/null` +fi +fi + +GMP_LIBS=${GMP_LIBS:-"-lgmp"} +$as_echo "$as_me:${as_lineno-$LINENO}: gmp cflags: $GMP_CFLAGS" >&5 +$as_echo "$as_me:${as_lineno-$LINENO}: gmp libs: $GMP_LIBS" >&5 +if test -s conftest.err; then + $as_echo_n "$as_me:${as_lineno-$LINENO}: gmp errors: " >&5 + cat conftest.err >&5 +fi +rm -f conftest.err +ac_save_CPPFLAGS=$CPPFLAGS +CPPFLAGS="$CPPFLAGS $GMP_CFLAGS" +for ac_header in gmp.h +do : + ac_fn_c_check_header_compile "$LINENO" "gmp.h" "ac_cv_header_gmp_h" "$ac_includes_default" +if test "x$ac_cv_header_gmp_h" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_GMP_H 1 +_ACEOF + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for -lgmp" >&5 +$as_echo_n "checking for -lgmp... " >&6; } +if ${ac_cv_lib_soname_gmp+:} false; then : + $as_echo_n "(cached) " >&6 +else + ac_check_soname_save_LIBS=$LIBS +LIBS="-lgmp $GMP_LIBS $LIBS" + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +/* Override any GCC internal prototype to avoid an error. + Use char because int might match the return type of a GCC + builtin and then its argument prototype would still apply. */ +#ifdef __cplusplus +extern "C" +#endif +char __gmpz_init (); +int +main () +{ +return __gmpz_init (); + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + case "$LIBEXT" in + dll) ac_cv_lib_soname_gmp=`$ac_cv_path_LDD conftest.exe | grep "gmp" | sed -e "s/dll.*/dll/"';2,$d'` ;; + dylib) ac_cv_lib_soname_gmp=`$OTOOL -L conftest$ac_exeext | grep "libgmp-*\\.[0-9A-Za-z.]*dylib" | sed -e "s/^.*\/\(libgmp-*\.[0-9A-Za-z.]*dylib\).*$/\1/"';2,$d'` ;; + *) ac_cv_lib_soname_gmp=`$READELF -d conftest$ac_exeext | grep "NEEDED.*libgmp-*\\.$LIBEXT" | sed -e "s/^.*\\[\\(libgmp-*\\.$LIBEXT[^ ]*\\)\\].*$/\1/"';2,$d'` + if ${ac_cv_lib_soname_gmp:+false} :; then : + ac_cv_lib_soname_gmp=`$LDD conftest$ac_exeext | grep "libgmp-*\\.$LIBEXT" | sed -e "s/^.*\(libgmp-*\.$LIBEXT[^ ]*\).*$/\1/"';2,$d'` +fi ;; + esac +else + ac_cv_lib_soname_gmp= +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext + LIBS=$ac_check_soname_save_LIBS +fi +if ${ac_cv_lib_soname_gmp:+false} :; then : + { $as_echo "$as_me:${as_lineno-$LINENO}: result: not found" >&5 +$as_echo "not found" >&6; } + GMP_CFLAGS="" +else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_soname_gmp" >&5 +$as_echo "$ac_cv_lib_soname_gmp" >&6; } + +cat >>confdefs.h <<_ACEOF +#define SONAME_LIBGMP "$ac_cv_lib_soname_gmp" +_ACEOF + + +fi +fi + +done + +CPPFLAGS=$ac_save_CPPFLAGS + +fi + if test "x$with_sane" != "xno" then rm -f conftest.err @@ -24523,6 +24624,8 @@ GNUTLS_CFLAGS = $GNUTLS_CFLAGS GNUTLS_LIBS = $GNUTLS_LIBS HWLOC_CFLAGS = $HWLOC_CFLAGS HWLOC_LIBS = $HWLOC_LIBS +GMP_CFLAGS = $GMP_CFLAGS +GMP_LIBS = $GMP_LIBS SANE_CFLAGS = $SANE_CFLAGS SANE_LIBS = $SANE_LIBS USB_CFLAGS = $USB_CFLAGS diff --git a/configure.ac b/configure.ac index a9db2abcce4..23c8781f36a 100644 --- a/configure.ac +++ b/configure.ac @@ -1537,6 +1537,14 @@ then esac fi +dnl **** Check for libgmp **** +if test "x$with_gnutls" != "xno" +then + WINE_PACKAGE_FLAGS(GMP,[gmp],[-lgmp],,, + [AC_CHECK_HEADERS([gmp.h], + [WINE_CHECK_SONAME(gmp,__gmpz_init,,[GMP_CFLAGS=""],[$GMP_LIBS],[[libgmp-*]])])]) +fi + dnl **** Check for SANE **** if test "x$with_sane" != "xno" then diff --git a/include/config.h.in b/include/config.h.in index bfc8c20d9d3..5b468596914 100644 --- a/include/config.h.in +++ b/include/config.h.in @@ -93,6 +93,9 @@ /* Define to 1 if you have the <gettext-po.h> header file. */ #undef HAVE_GETTEXT_PO_H +/* Define to 1 if you have the <gmp.h> header file. */ +#undef HAVE_GMP_H + /* Define to 1 if you have the 'gnutls_cipher_init' function. */ #undef HAVE_GNUTLS_CIPHER_INIT @@ -759,6 +762,9 @@ /* Define to the soname of the libgnutls library. */ #undef SONAME_LIBGNUTLS +/* Define to the soname of the libgmp library. */ +#undef SONAME_LIBGMP + /* Define to the soname of the libgssapi_krb5 library. */ #undef SONAME_LIBGSSAPI_KRB5 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9447
From: Haoyang Chen <chenhaoyang(a)kylinos.cn> --- dlls/bcrypt/Makefile.in | 2 +- dlls/bcrypt/gnutls.c | 197 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 197 insertions(+), 2 deletions(-) diff --git a/dlls/bcrypt/Makefile.in b/dlls/bcrypt/Makefile.in index a672ac8a3c9..a0ddbd08630 100644 --- a/dlls/bcrypt/Makefile.in +++ b/dlls/bcrypt/Makefile.in @@ -3,7 +3,7 @@ IMPORTLIB = bcrypt IMPORTS = $(TOMCRYPT_PE_LIBS) advapi32 EXTRAINCL = $(TOMCRYPT_PE_CFLAGS) UNIXLIB = bcrypt.so -UNIX_CFLAGS = $(GNUTLS_CFLAGS) +UNIX_CFLAGS = $(GNUTLS_CFLAGS) $(GMP_CFLAGS) SOURCES = \ bcrypt_main.c \ diff --git a/dlls/bcrypt/gnutls.c b/dlls/bcrypt/gnutls.c index 3e9efb677df..4620a570f9f 100644 --- a/dlls/bcrypt/gnutls.c +++ b/dlls/bcrypt/gnutls.c @@ -46,6 +46,10 @@ #include "bcrypt_internal.h" +#ifdef HAVE_GMP_H +#include <gmp.h> +#endif + #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(bcrypt); @@ -189,6 +193,20 @@ MAKE_FUNCPTR(gnutls_pubkey_deinit); MAKE_FUNCPTR(gnutls_pubkey_encrypt_data); MAKE_FUNCPTR(gnutls_pubkey_import_privkey); MAKE_FUNCPTR(gnutls_pubkey_init); +#if defined(HAVE_GMP_H) && defined(SONAME_LIBGMP) +static void *libgmp_handle; + +MAKE_FUNCPTR(mpz_init); +MAKE_FUNCPTR(mpz_clear); +MAKE_FUNCPTR(mpz_cmp); +MAKE_FUNCPTR(mpz_sizeinbase); +MAKE_FUNCPTR(mpz_import); +MAKE_FUNCPTR(mpz_export); +MAKE_FUNCPTR(mpz_mod); +MAKE_FUNCPTR(mpz_sub_ui); +MAKE_FUNCPTR(mpz_mul); +MAKE_FUNCPTR(mpz_invert); +#endif #undef MAKE_FUNCPTR static int compat_gnutls_cipher_tag(gnutls_cipher_hd_t handle, void *tag, size_t tag_size) @@ -458,6 +476,36 @@ static NTSTATUS gnutls_process_attach( void *args ) #undef LOAD_FUNCPTR_OPT +#if defined(HAVE_GMP_H) && defined(SONAME_LIBGMP) +#define LOAD_FUNCPTR_STR(f) #f +#define LOAD_FUNCPTR(f) \ + if (!(p##f = dlsym( libgmp_handle, LOAD_FUNCPTR_STR(f) ))) \ + { \ + ERR( "failed to load %s\n", LOAD_FUNCPTR_STR(f) ); \ + goto fail; \ + } + + if ((libgmp_handle = dlopen( SONAME_LIBGMP, RTLD_NOW ))) + { + LOAD_FUNCPTR(mpz_init); + LOAD_FUNCPTR(mpz_clear); + LOAD_FUNCPTR(mpz_cmp); + LOAD_FUNCPTR(mpz_sizeinbase); + LOAD_FUNCPTR(mpz_import); + LOAD_FUNCPTR(mpz_export); + LOAD_FUNCPTR(mpz_mod); + LOAD_FUNCPTR(mpz_sub_ui); + LOAD_FUNCPTR(mpz_mul); + LOAD_FUNCPTR(mpz_invert); + } + else + { + ERR_(winediag)( "failed to load libgmp, no support for parameter completion for asymmetric algorithms.\n" ); + goto fail; + } +#undef LOAD_FUNCPTR +#undef LOAD_FUNCPTR_STR +#endif if ((ret = pgnutls_global_init()) != GNUTLS_E_SUCCESS) { pgnutls_perror( ret ); @@ -477,6 +525,13 @@ static NTSTATUS gnutls_process_attach( void *args ) fail: dlclose( libgnutls_handle ); libgnutls_handle = NULL; +#if defined(HAVE_GMP_H) && defined(SONAME_LIBGMP) + if (libgmp_handle) + { + dlclose( libgmp_handle ); + libgmp_handle = NULL; + } +#endif return STATUS_DLL_NOT_FOUND; } @@ -488,6 +543,10 @@ static NTSTATUS gnutls_process_detach( void *args ) dlclose( libgnutls_handle ); libgnutls_handle = NULL; } +#if defined(HAVE_GMP_H) && defined(SONAME_LIBGMP) + dlclose( libgmp_handle ); + libgmp_handle = NULL; +#endif return STATUS_SUCCESS; } @@ -1481,12 +1540,133 @@ static NTSTATUS key_export_rsa( struct key *key, ULONG flags, UCHAR *buf, ULONG return STATUS_SUCCESS; } +#if defined(HAVE_GMP_H) && defined(SONAME_LIBGMP) +typedef struct { + gnutls_datum_t d; + gnutls_datum_t u; + gnutls_datum_t dp; + gnutls_datum_t dq; +} rsa_crt_params_t; + +static BOOL calculate_rsa_crt_params(const unsigned char* n_data, size_t n_len, + const unsigned char* e_data, size_t e_len, + const unsigned char* p_data, size_t p_len, + const unsigned char* q_data, size_t q_len, + rsa_crt_params_t* params) +{ + mpz_t n, e, p, q, phi_n, d, p_minus_1, q_minus_1, dp, dq, u, n_calc; + BOOL ret = FALSE; + size_t size; + + if(!n_data || !e_data || !p_data || !q_data || !params) return ret; + + pmpz_init(n); + pmpz_init(e); + pmpz_init(p); + pmpz_init(q); + pmpz_init(phi_n); + pmpz_init(d); + pmpz_init(p_minus_1); + pmpz_init(q_minus_1); + pmpz_init(dp); + pmpz_init(dq); + pmpz_init(u); + + pmpz_import(n, n_len, 1, sizeof(unsigned char), 0, 0, n_data); + pmpz_import(e, e_len, 1, sizeof(unsigned char), 0, 0, e_data); + pmpz_import(p, p_len, 1, sizeof(unsigned char), 0, 0, p_data); + pmpz_import(q, q_len, 1, sizeof(unsigned char), 0, 0, q_data); + + pmpz_init(n_calc); + pmpz_mul(n_calc, p, q); + if (pmpz_cmp(n, n_calc) != 0) + { + ERR("n != p * q input data is inconsistent.\n"); + pmpz_clear(n_calc); + goto done; + } + pmpz_clear(n_calc); + + pmpz_sub_ui(p_minus_1, p, 1); + pmpz_sub_ui(q_minus_1, q, 1); + pmpz_mul(phi_n, p_minus_1, q_minus_1); + + if (pmpz_invert(d, e, phi_n) == 0) + { + ERR("inverse does not exist (e and φ(n) are not coprime).\n"); + goto done; + } + + pmpz_mod(dp, d, p_minus_1); + + pmpz_mod(dq, d, q_minus_1); + + if (pmpz_invert(u, q, p) == 0) + { + ERR("inverse does not exist (q and p are not coprime - they should be primes!).\n"); + goto done; + } + + size = (pmpz_sizeinbase(d, 2) + 7) / 8; + params->d.data = malloc(size); + if (!params->d.data) goto done; + pmpz_export(params->d.data, &size, 1, sizeof(unsigned char), 0, 0, d); + params->d.size = size; + + size = (pmpz_sizeinbase(dp, 2) + 7) / 8; + params->dp.data = malloc(size); + if (!params->dp.data) goto done; + pmpz_export(params->dp.data, &size, 1, sizeof(unsigned char), 0, 0, dp); + params->dp.size = size; + + size = (pmpz_sizeinbase(dq, 2) + 7) / 8; + params->dq.data = malloc(size); + if (!params->dq.data) goto done; + pmpz_export(params->dq.data, &size, 1, sizeof(unsigned char), 0, 0, dq); + params->dq.size = size; + + size = (pmpz_sizeinbase(u, 2) + 7) / 8; + params->u.data = malloc(size); + if (!params->u.data) goto done; + pmpz_export(params->u.data, &size, 1, sizeof(unsigned char), 0, 0, u); + params->u.size = size; + + ret = TRUE; + +done: + if (!ret) + { + free(params->d.data); + free(params->dp.data); + free(params->dq.data); + free(params->u.data); + } + + pmpz_clear(n); + pmpz_clear(e); + pmpz_clear(p); + pmpz_clear(q); + pmpz_clear(phi_n); + pmpz_clear(d); + pmpz_clear(p_minus_1); + pmpz_clear(q_minus_1); + pmpz_clear(dp); + pmpz_clear(dq); + pmpz_clear(u); + + return ret; +} +#endif + static NTSTATUS key_import_rsa( struct key *key, UCHAR *buf, ULONG len ) { BCRYPT_RSAKEY_BLOB *rsa_blob = (BCRYPT_RSAKEY_BLOB *)buf; gnutls_datum_t m, e, p, q; gnutls_privkey_t handle; int ret; +#if defined(HAVE_GMP_H) && defined(SONAME_LIBGMP) + rsa_crt_params_t crt_params = {0}; +#endif if ((ret = pgnutls_privkey_init( &handle ))) { @@ -1503,7 +1683,22 @@ static NTSTATUS key_import_rsa( struct key *key, UCHAR *buf, ULONG len ) q.data = p.data + p.size; q.size = rsa_blob->cbPrime2; - if ((ret = pgnutls_privkey_import_rsa_raw( handle, &m, &e, NULL, &p, &q, NULL, NULL, NULL ))) +#if defined(HAVE_GMP_H) && defined(SONAME_LIBGMP) + if (calculate_rsa_crt_params(m.data, m.size, e.data, e.size, p.data, p.size, q.data, q.size, &crt_params)) + { + + ret = pgnutls_privkey_import_rsa_raw( handle, &m, &e, &crt_params.d, &p, &q, + &crt_params.u, &crt_params.dp, &crt_params.dq ); + + free(crt_params.d.data); + free(crt_params.u.data); + free(crt_params.dp.data); + free(crt_params.dq.data); + } else +#endif + ret = pgnutls_privkey_import_rsa_raw( handle, &m, &e, NULL, &p, &q, NULL, NULL, NULL ); + + if (ret) { pgnutls_perror( ret ); pgnutls_privkey_deinit( handle ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9447
What version of libgnutls requires this? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447#note_121774
This is the case with libgnutls30 (3.6.13) in my system. For example, if the d parameter is missing, it will crash. https://gitlab.com/gnutls/gnutls/-/blob/3.6.13/lib/x509/privkey.c#L903 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447#note_121776
Note that we're using gnutls_privkey_import_rsa_raw(), not gnutls_x509_privkey_import_rsa_raw2(). The documentation for the former says that these parameters are optional. What's the problem you're trying to fix? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447#note_121778
You're right, these parameters are optional in the latest version of libgnutls. Some older versions, such as version 3.6.13. They are necessary.The gnutls_privkey_import_rsa_raw will call gnutls_x509_privkey_import_rsa_raw2. https://gitlab.com/gnutls/gnutls/-/blob/3.6.13/lib/privkey_raw.c#L359 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447#note_121780
They were made optional in 3.7.0, released December 2, 2020. We want to avoid adding dependencies, especially when there's a fix available upstream, so I would recommend updating to a newer version of GnuTLS. Also note that there are plans to move away from GnuTLS in bcrypt which will sidestep this issue. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447#note_121781
This merge request was closed by Haoyang Chen. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447
Thank you, okay then. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9447#note_121782
participants (3)
-
Hans Leidekker (＠hans) -
Haoyang Chen
-
Haoyang Chen (＠chenhaoyang)