[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.
From: Haoyang Chen chenhaoyang@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
From: Haoyang Chen chenhaoyang@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 );
What version of libgnutls requires this?
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
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?
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
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.
This merge request was closed by Haoyang Chen.
Thank you, okay then.
-
Hans Leidekker (＠hans) -
Haoyang Chen
-
Haoyang Chen (＠chenhaoyang)