From fcf830788d0c8dcd6bed7dcd532506aabf196523 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Tue, 7 Jul 2026 00:35:28 +0200 Subject: [PATCH] libgcrypt, mbedtls, openssl: validate DH public value Also add tests. Reported-by: yaotushaozhu on github Ref: GHSA-m9rg-h9jv-9gvh Closes #2239 --- src/crypto.h | 1 + src/libgcrypt.c | 32 ++++++++++++++++++ src/mbedtls.c | 46 +++++++++++++++++++++++++ src/openssl.c | 34 +++++++++++++++++++ src/userauth_kbd_packet.h | 2 +- tests/test_simple.c | 71 +++++++++++++++++++++++++++++++++++++-- 6 files changed, 183 insertions(+), 3 deletions(-) diff --git a/src/crypto.h b/src/crypto.h index ac864686..cc80ec2f 100644 --- a/src/crypto.h +++ b/src/crypto.h @@ -371,6 +371,7 @@ int ssh2_bn_to_bin(const ssh2_bn *bn, unsigned char *bin); void ssh2_dh_init(ssh2_dh_ctx *dhctx); int ssh2_dh_key_pair(ssh2_dh_ctx *dhctx, ssh2_bn *pub, ssh2_bn *g, ssh2_bn *p, int group_order, ssh2_bn_ctx *bnctx); +int ssh2_dh_is_valid(ssh2_bn *f, ssh2_bn *p); /* for unit tests */ int ssh2_dh_secret(ssh2_dh_ctx *dhctx, ssh2_bn *secret, ssh2_bn *f, ssh2_bn *p, ssh2_bn_ctx *bnctx); void ssh2_dh_dtor(ssh2_dh_ctx *dhctx); diff --git a/src/libgcrypt.c b/src/libgcrypt.c index 18262ed9..2e596f4c 100644 --- a/src/libgcrypt.c +++ b/src/libgcrypt.c @@ -788,10 +788,42 @@ int ssh2_dh_key_pair(ssh2_dh_ctx *dhctx, ssh2_bn *pub, ssh2_bn *g, return 0; } +int ssh2_dh_is_valid(ssh2_bn *f, ssh2_bn *p) +{ + gcry_mpi_t tmp; + unsigned int n, i, bits_set; + + if(gcry_mpi_cmp_ui(f, 1) <= 0) + return -1; /* f <= 1 */ + + tmp = gcry_mpi_new(0); + if(!tmp) + return -4; + gcry_mpi_sub_ui(tmp, p, 1); + if(gcry_mpi_cmp(f, tmp) >= 0) { + gcry_mpi_release(tmp); + return -2; /* f >= p - 1 (== f > p - 2) */ + } + gcry_mpi_release(tmp); + + for(i = 0, n = gcry_mpi_get_nbits(f), bits_set = 0; i < n; ++i) + if(gcry_mpi_test_bit(f, i)) + ++bits_set; + + if(bits_set < 4) + return -3; + + return 0; +} + int ssh2_dh_secret(ssh2_dh_ctx *dhctx, ssh2_bn *secret, ssh2_bn *f, ssh2_bn *p, ssh2_bn_ctx *bnctx) { (void)bnctx; + + if(ssh2_dh_is_valid(f, p)) /* Verify if parameters are valid */ + return -1; + /* Compute the shared secret */ gcry_mpi_powm(secret, f, *dhctx, p); return 0; diff --git a/src/mbedtls.c b/src/mbedtls.c index 50803818..cfae35b7 100644 --- a/src/mbedtls.c +++ b/src/mbedtls.c @@ -777,10 +777,56 @@ int ssh2_dh_key_pair(ssh2_dh_ctx *dhctx, ssh2_bn *pub, ssh2_bn *g, return 0; } +int ssh2_dh_is_valid(ssh2_bn *f, ssh2_bn *p) +{ + mbedtls_mpi one, tmp; + size_t n, i, bits_set; + + /* Verify if valid */ + mbedtls_mpi_init(&one); + if(mbedtls_mpi_lset(&one, 1)) { + mbedtls_mpi_free(&one); + return -4; + } + if(mbedtls_mpi_cmp_mpi(f, &one) != 1) { + mbedtls_mpi_free(&one); + return -1; /* f <= 1 */ + } + + mbedtls_mpi_init(&tmp); + if(mbedtls_mpi_copy(&tmp, p) || /* tmp = p */ + mbedtls_mpi_sub_int(&tmp, &tmp, 2)) { /* tmp -= 2 */ + mbedtls_mpi_free(&tmp); + mbedtls_mpi_free(&one); + return -5; + } + if(mbedtls_mpi_cmp_mpi(f, &tmp) == 1) { + mbedtls_mpi_free(&tmp); + mbedtls_mpi_free(&one); + return -2; /* f >= p - 1 (== f > p - 2) */ + } + + mbedtls_mpi_free(&tmp); + mbedtls_mpi_free(&one); + + for(i = 0, n = mbedtls_mpi_bitlen(f), bits_set = 0; i < n; ++i) + if(mbedtls_mpi_get_bit(f, i)) + ++bits_set; + + if(bits_set < 4) + return -3; + + return 0; +} + int ssh2_dh_secret(ssh2_dh_ctx *dhctx, ssh2_bn *secret, ssh2_bn *f, ssh2_bn *p, ssh2_bn_ctx *bnctx) { (void)bnctx; + + if(ssh2_dh_is_valid(f, p)) /* Verify if parameters are valid */ + return -1; + /* Compute the shared secret */ if(mbedtls_mpi_exp_mod(secret, f, *dhctx, p, NULL)) return -1; diff --git a/src/openssl.c b/src/openssl.c index f00aaf3e..d803c315 100644 --- a/src/openssl.c +++ b/src/openssl.c @@ -4283,9 +4283,43 @@ int ssh2_dh_key_pair(ssh2_dh_ctx *dhctx, ssh2_bn *pub, ssh2_bn *g, return 0; } +int ssh2_dh_is_valid(ssh2_bn *f, ssh2_bn *p) +{ + BIGNUM *tmp; + int n, i, bits_set; + + if(BN_cmp(f, BN_value_one()) != 1) + return -1; /* f <= 1 */ + + tmp = BN_new(); + if(!tmp) + return -4; + if(!BN_sub(tmp, p, BN_value_one())) { + BN_clear_free(tmp); + return -4; + } + if(BN_cmp(f, tmp) != -1) { + BN_clear_free(tmp); + return -2; /* f >= p - 1 (== f > p - 2) */ + } + BN_clear_free(tmp); + + for(i = 0, n = BN_num_bits(f), bits_set = 0; i < n; ++i) + if(BN_is_bit_set(f, i)) + ++bits_set; + + if(bits_set < 4) + return -3; + + return 0; +} + int ssh2_dh_secret(ssh2_dh_ctx *dhctx, ssh2_bn *secret, ssh2_bn *f, ssh2_bn *p, ssh2_bn_ctx *bnctx) { + if(ssh2_dh_is_valid(f, p)) /* Verify if parameters are valid */ + return -1; + /* Compute the shared secret */ if(!BN_mod_exp(secret, f, *dhctx, p, bnctx)) return -1; diff --git a/src/userauth_kbd_packet.h b/src/userauth_kbd_packet.h index 8ad8e875..1acc553b 100644 --- a/src/userauth_kbd_packet.h +++ b/src/userauth_kbd_packet.h @@ -34,6 +34,6 @@ #define LIBSSH2_USERAUTH_KBD_PACKET_H int userauth_keyboard_interactive_decode_info_request( - LIBSSH2_SESSION *session); + LIBSSH2_SESSION *session); /* for unit tests */ #endif /* LIBSSH2_USERAUTH_KBD_PACKET_H */ diff --git a/tests/test_simple.c b/tests/test_simple.c index baac63bd..bb8ec9da 100644 --- a/tests/test_simple.c +++ b/tests/test_simple.c @@ -34,7 +34,7 @@ #include "libssh2_priv.h" #include -#include +#include /* for atoi() */ static int test_ssh2_base64_decode(LIBSSH2_SESSION *session) { @@ -58,6 +58,72 @@ static int test_ssh2_base64_decode(LIBSSH2_SESSION *session) return 0; } +static int test_ssh2_dh_is_valid(void) +{ + struct tbn { + const char *f; const char *p; int expected; + }; + static const struct tbn tests[] = { + { "-1", "10", -1 }, + { "2", "10", -3 }, + { "1", "10", -1 }, + { "0", "10", -1 }, + { "9", "10", -2 }, /* f=9; p=10 => p-2=8, f>8 */ + { "6", "20", -3 }, /* f=6 (110); p=20 => p-2=18, 6<=18 */ + { "27", "100", 0 }, /* f=27 (11011); p=100 => p-2=98, 27<=98 */ + { "240", "242", 0 }, /* f=p-2, f=240 (11110000); p=f+2=242 */ + }; + + size_t i; + int err = 0; + + for(i = 0; i < SSH2_ARRAYSIZE(tests); i++) { + int got; +#ifdef LIBSSH2_LIBGCRYPT + int fi = atoi(tests[i].f); + gcry_mpi_t f = gcry_mpi_set_ui(NULL, fi >= 0 ? fi : -fi); + gcry_mpi_t p = gcry_mpi_set_ui(NULL, atoi(tests[i].p)); + if(tests[i].f[0] == '-') + gcry_mpi_neg(f, f); + got = ssh2_dh_is_valid(f, p); + gcry_mpi_release(f); + gcry_mpi_release(p); +#elif defined(LIBSSH2_MBEDTLS) + mbedtls_mpi f, p; + mbedtls_mpi_init(&f); + mbedtls_mpi_init(&p); + if(mbedtls_mpi_read_string(&f, 10, tests[i].f) || + mbedtls_mpi_read_string(&p, 10, tests[i].p)) + got = -9; + else + got = ssh2_dh_is_valid(&f, &p); + mbedtls_mpi_free(&f); + mbedtls_mpi_free(&p); +#elif defined(LIBSSH2_OPENSSL) || \ + (defined(LIBSSH2_WOLFSSL) && LIBWOLFSSL_VERSION_HEX >= 0x05006000) + BIGNUM *f = BN_new(), *p = BN_new(); + if(!BN_dec2bn(&f, tests[i].f) || + !BN_dec2bn(&p, tests[i].p)) + got = -9; + else + got = ssh2_dh_is_valid(f, p); + BN_free(f); + BN_free(p); +#else + got = tests[i].expected; +#endif + if(got != tests[i].expected) { + fprintf(stderr, + "ssh2_dh_is_valid/%lu: f=%s p=%s: expected %d got %d\n", + (unsigned long)i, + tests[i].f, tests[i].p, tests[i].expected, got); + err++; + } + } + + return err > 0; +} + int main(int argc, char *argv[]) { LIBSSH2_SESSION *session; @@ -65,7 +131,7 @@ int main(int argc, char *argv[]) (void)argv; (void)argc; - rc = libssh2_init(LIBSSH2_INIT_NO_CRYPTO); + rc = libssh2_init(0); if(rc) { fprintf(stderr, "libssh2_init() failed: %d\n", rc); return 1; @@ -78,6 +144,7 @@ int main(int argc, char *argv[]) } rc = test_ssh2_base64_decode(session); + rc |= test_ssh2_dh_is_valid(); libssh2_session_free(session);