From 18111bf49b10200a86c5029a6eb1b52ff9ba1fd2 Mon Sep 17 00:00:00 2001 From: sebastian-carpenter Date: Tue, 16 Dec 2025 17:40:19 -0700 Subject: [PATCH] rsa key validation test and additions to rsa fromdata test --- src/wp_rsa_kmgmt.c | 92 +++++++++++--- test/test_rsa.c | 299 +++++++++++++++++++++++++++++++++++++++++++++ test/unit.c | 1 + test/unit.h | 1 + 4 files changed, 379 insertions(+), 14 deletions(-) diff --git a/src/wp_rsa_kmgmt.c b/src/wp_rsa_kmgmt.c index c66a5dee..d2d89540 100644 --- a/src/wp_rsa_kmgmt.c +++ b/src/wp_rsa_kmgmt.c @@ -893,13 +893,23 @@ static int wp_rsa_get_params_key_data(wp_Rsa* rsa, OSSL_PARAM params[]) if (p != NULL) { size_t oLen; mp_int* mp = (mp_int*)(((byte*)&rsa->key) + wp_rsa_offset[i]); - oLen = mp_unsigned_bin_size(mp); - if (oLen > p->data_size) { - ok = 0; + if (mp_iszero(mp) == MP_YES) { + /* not sure if this should be set to NULL or not. + * setting this to NULL can follow OSSL's output better, but it + * feels like a memory leak + * Would be good to check OSSL's implementation */ + p->data = NULL; + oLen = 0; } - if (ok && (p->data != NULL) && - (!wp_mp_to_unsigned_bin_le(mp, p->data, oLen))) { - ok = 0; + else { + oLen = mp_unsigned_bin_size(mp); + if (oLen > p->data_size) { + ok = 0; + } + if (ok && (p->data != NULL) && + (!wp_mp_to_unsigned_bin_le(mp, p->data, oLen))) { + ok = 0; + } } p->return_size = oLen; } @@ -1074,6 +1084,27 @@ static int wp_rsa_match(const wp_Rsa* rsa1, const wp_Rsa* rsa2, int selection) return ok; } +#define VALIDATE_PRIMES_SIZE 133 +static const mp_digit validate_primes[VALIDATE_PRIMES_SIZE] = { + 0x0002, 0x0003, 0x0005, 0x0007, 0x000B, 0x000D, 0x0011, 0x0013, + 0x0017, 0x001D, 0x001F, 0x0025, 0x0029, 0x002B, 0x002F, 0x0035, + 0x003B, 0x003D, 0x0043, 0x0047, 0x0049, 0x004F, 0x0053, 0x0059, + 0x0061, 0x0065, 0x0067, 0x006B, 0x006D, 0x0071, 0x007F, 0x0083, + 0x0089, 0x008B, 0x0095, 0x0097, 0x009D, 0x00A3, 0x00A7, 0x00AD, + 0x00B3, 0x00B5, 0x00BF, 0x00C1, 0x00C5, 0x00C7, 0x00D3, 0x00DF, + 0x00E3, 0x00E5, 0x00E9, 0x00EF, 0x00F1, 0x00FB, 0x0101, 0x0107, + 0x010D, 0x010F, 0x0115, 0x0119, 0x011B, 0x0125, 0x0133, 0x0137, + 0x0139, 0x013D, 0x014B, 0x0151, 0x015B, 0x015D, 0x0161, 0x0167, + 0x016F, 0x0175, 0x017B, 0x017F, 0x0185, 0x018D, 0x0191, 0x0199, + 0x01A3, 0x01A5, 0x01AF, 0x01B1, 0x01B7, 0x01BB, 0x01C1, 0x01C9, + 0x01CD, 0x01CF, 0x01D3, 0x01DF, 0x01E7, 0x01EB, 0x01F3, 0x01F7, + 0x01FD, 0x0209, 0x020B, 0x021D, 0x0223, 0x022D, 0x0233, 0x0239, + 0x023B, 0x0241, 0x024B, 0x0251, 0x0257, 0x0259, 0x025F, 0x0265, + 0x0269, 0x026B, 0x0277, 0x0281, 0x0283, 0x0287, 0x028D, 0x0293, + 0x0295, 0x02A1, 0x02A5, 0x02AB, 0x02B3, 0x02BD, 0x02C5, 0x02CF, + 0x02D7, 0x02DD, 0x02E3, 0x02E7, 0x02EF, +}; + /** * Validate the RSA key. * @@ -1106,16 +1137,38 @@ static int wp_rsa_validate(const wp_Rsa* rsa, int selection, int checkType) else #endif if (checkPriv) { - if (mp_isone(&rsa->key.d) || mp_iszero((mp_int*)&rsa->key.d) || + if (mp_iszero((mp_int*)&rsa->key.d) || (mp_cmp((mp_int*)&rsa->key.d, (mp_int*)&rsa->key.n) != MP_LT)) { ok = 0; } } else if (checkPub) { - if (mp_iseven(&rsa->key.e) || mp_iszero((mp_int*)&rsa->key.e) || - mp_isone(&rsa->key.e)) { + int prime; + mp_int res; + + if (mp_iszero((mp_int*)&rsa->key.e) || mp_iszero((mp_int*)&rsa->key.n) || + mp_isone((mp_int*)&rsa->key.e) || mp_isone((mp_int*)&rsa->key.n) || + mp_iseven((mp_int*)&rsa->key.e) || mp_iseven((mp_int*)&rsa->key.n)) { ok = 0; } + + if (ok && mp_init(&res) != MP_OKAY){ + ok = 0; + } + else if (ok){ + /* seems like it would be better to use mp_prime_is_prime but I + * could not get it to work + * Still need miller-rabin test, GCD, and size constraints on e + * though it passes the test for now */ + for(prime = 0; prime < VALIDATE_PRIMES_SIZE; prime++) { + if (mp_set_int(&res, validate_primes[prime]) != MP_OKAY || + mp_mod((mp_int*)&rsa->key.n, &res, &res) != MP_OKAY || + mp_iszero(&res)) { + ok = 0; + break; + } + } + } } WOLFPROV_LEAVE(WP_LOG_COMP_RSA, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok); @@ -1144,8 +1197,12 @@ static int wp_rsa_import_key_data(wp_Rsa* rsa, const OSSL_PARAM params[], /* N and E params are the only ones required by OSSL, so match that. * See ossl_rsa_fromdata() and RSA_set0_key() in OpenSSL. */ - if (OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_N) == NULL || - OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_E) == NULL) { + /* May actually be fine if p->data is NULL, not sure if OSSL fails if + * N and/or E are NULL */ + if ((p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_N)) == NULL || + p->data == NULL || + (p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_E)) == NULL || + p->data == NULL){ WOLFPROV_MSG(WP_LOG_COMP_RSA, "Param N or E is missing"); ok = 0; } @@ -1160,7 +1217,7 @@ static int wp_rsa_import_key_data(wp_Rsa* rsa, const OSSL_PARAM params[], index = -1; for (j = 0; j < (int)ARRAY_SIZE(wp_rsa_param_key); j++) { if (XSTRNCMP(p->key, wp_rsa_param_key[j], XSTRLEN(p->key)) == 0) { - index = j; + index = j; break; } } @@ -1170,11 +1227,18 @@ static int wp_rsa_import_key_data(wp_Rsa* rsa, const OSSL_PARAM params[], p->key); continue; } - /* Read the value into the rsa struct */ if (ok) { mp = (mp_int*)(((byte*)&rsa->key) + wp_rsa_offset[index]); - if (!wp_mp_read_unsigned_bin_le(mp, p->data, p->data_size)) { + /* OSSL doesn't fail when p->data is NULL, input data is + * converted as follows by OSSL: + * NULL -> 0 + * negative -> NULL + * Mimic this behavior by setting both to 0 */ + if (p->data == NULL || p->data_type != OSSL_PARAM_UNSIGNED_INTEGER){ + ok = mp_set(mp, 0) == MP_OKAY; + } + else if (!wp_mp_read_unsigned_bin_le(mp, p->data, p->data_size)) { WOLFPROV_MSG(WP_LOG_COMP_RSA, "Failed to read %s from parameters", p->key); ok = 0; diff --git a/test/test_rsa.c b/test/test_rsa.c index c7042121..7860e30a 100644 --- a/test/test_rsa.c +++ b/test/test_rsa.c @@ -24,6 +24,7 @@ #include #include +#include #ifdef WP_HAVE_RSA @@ -1379,6 +1380,7 @@ int test_rsa_fromdata(void* data) #ifdef EVP_PKEY_PRIVATE_KEY EVP_PKEY_PRIVATE_KEY, /* added in 3.0.12 and 3.1.4 */ #endif + EVP_PKEY_KEY_PARAMETERS, }; /* Parameter data fields */ @@ -1441,6 +1443,22 @@ int test_rsa_fromdata(void* data) { "bar", OSSL_PARAM_UTF8_STRING, (void *)&bar, sizeof(bar) - 1, 0 }, OSSL_PARAM_END }; + OSSL_PARAM params_null[] = { + OSSL_PARAM_ulong("n", NULL), + OSSL_PARAM_ulong("e", NULL), + OSSL_PARAM_ulong("d", NULL), + { "foo", OSSL_PARAM_UTF8_PTR, NULL, foo_l, 0 }, + { "bar", OSSL_PARAM_UTF8_STRING, NULL, sizeof(bar) - 1, 0 }, + OSSL_PARAM_END + }; + OSSL_PARAM params_ne_null[] = { + OSSL_PARAM_ulong("n", &rsa_n), + OSSL_PARAM_ulong("e", &rsa_e), + OSSL_PARAM_ulong("d", NULL), + { "foo", OSSL_PARAM_UTF8_PTR, NULL, foo_l, 0 }, + { "bar", OSSL_PARAM_UTF8_STRING, NULL, sizeof(bar) - 1, 0 }, + OSSL_PARAM_END + }; OSSL_PARAM* params_table[] = { params_none, params_n, @@ -1452,6 +1470,8 @@ int test_rsa_fromdata(void* data) params_ned, params_extra_ulong, params_extra_str, + params_null, + params_ne_null, }; for (unsigned i = 0; i < ARRAY_SIZE(selections); i++) { @@ -1867,4 +1887,283 @@ int test_rsa_null_init(void* data) return err; } +static int test_rsa_key_integrity_helper(OSSL_PARAM_BLD* bld, BIGNUM* p, + BIGNUM* q, BIGNUM* n, BIGNUM* e, BIGNUM* d) +{ + int err = 0; + int ossl_err; + int wolf_err; + OSSL_PARAM* params = NULL; + EVP_PKEY_CTX* ctx_ossl = NULL; + EVP_PKEY_CTX* ctx_wolf = NULL; + EVP_PKEY* pkey_ossl = NULL; + EVP_PKEY* pkey_wolf = NULL; + + if (err == 0) { + err |= OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_RSA_FACTOR1, p) != 1; + err |= OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_RSA_FACTOR2, q) != 1; + err |= OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_RSA_N, n) != 1; + err |= OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_RSA_E, e) != 1; + err |= OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_RSA_D, d) != 1; + } + if (err == 0) { + params = OSSL_PARAM_BLD_to_param(bld); + err = params == NULL; + } + + if (err == 0) { + ctx_ossl = EVP_PKEY_CTX_new_from_name(osslLibCtx, "RSA", NULL); + ctx_wolf = EVP_PKEY_CTX_new_from_name(wpLibCtx, "RSA", NULL); + err = (ctx_ossl == NULL) || (ctx_wolf == NULL); + } + if (err == 0) { + err |= EVP_PKEY_fromdata_init(ctx_ossl) != 1; + err |= EVP_PKEY_fromdata_init(ctx_wolf) != 1; + } + if (err == 0) { + ossl_err = EVP_PKEY_fromdata(ctx_ossl, &pkey_ossl, EVP_PKEY_KEYPAIR, + params); + wolf_err = EVP_PKEY_fromdata(ctx_wolf, &pkey_wolf, EVP_PKEY_KEYPAIR, + params); + + if (ossl_err != wolf_err) { + PRINT_MSG("EVP_PKEY_fromdata status mismatch"); + err = 1; + } + else if (wolf_err == 0) { + PRINT_MSG("Could not load keys with EVP_PKEY_fromdata"); + err = 1; + } + else if (wolf_err == 1) { + if (EVP_PKEY_eq(pkey_ossl, pkey_wolf) != 1 || + EVP_PKEY_parameters_eq(pkey_ossl, pkey_wolf) != 1) { + PRINT_MSG("EVP_PKEY_eq / EVP_PKEY_parameters_eq failed"); + err = 1; + } + } + } + + if (err == 0) { + EVP_PKEY_CTX_free(ctx_ossl); + EVP_PKEY_CTX_free(ctx_wolf); + ctx_ossl = EVP_PKEY_CTX_new_from_pkey(osslLibCtx, pkey_ossl, NULL); + ctx_wolf = EVP_PKEY_CTX_new_from_pkey(wpLibCtx, pkey_wolf, NULL); + err = (ctx_ossl == NULL) || (ctx_wolf == NULL); + } + if (err == 0) { + ossl_err = EVP_PKEY_param_check(ctx_ossl) != 1; + wolf_err = EVP_PKEY_param_check(ctx_wolf) != 1; + if (ossl_err != wolf_err){ + PRINT_MSG("param_check err: OSSL %d WOLF %d", ossl_err, wolf_err); + err |= 1; + } + + ossl_err = EVP_PKEY_public_check(ctx_ossl) != 1; + wolf_err = EVP_PKEY_public_check(ctx_wolf) != 1; + if (ossl_err != wolf_err){ + PRINT_MSG("public_check err: OSSL %d WOLF %d", ossl_err, wolf_err); + err |= 1; + } + + ossl_err = EVP_PKEY_private_check(ctx_ossl) != 1; + wolf_err = EVP_PKEY_private_check(ctx_wolf) != 1; + if (ossl_err != wolf_err){ + PRINT_MSG("private_check err: OSSL %d WOLF %d", ossl_err, wolf_err); + err |= 1; + } + +#ifdef WOLFSSL_RSA_KEY_CHECK + ossl_err = EVP_PKEY_pairwise_check(ctx_ossl) != 1; + wolf_err = EVP_PKEY_pairwise_check(ctx_wolf) != 1; + if (ossl_err != wolf_err){ + PRINT_MSG("pairwise_check err: OSSL %d WOLF %d", ossl_err, wolf_err); + err |= 1; + } +#endif + } + + EVP_PKEY_free(pkey_ossl); + EVP_PKEY_free(pkey_wolf); + EVP_PKEY_CTX_free(ctx_ossl); + EVP_PKEY_CTX_free(ctx_wolf); + OSSL_PARAM_free(params); + + return err; +} + +int test_rsa_key_integrity(void* data) +{ + (void)data; + int err = 0; + PKCS8_PRIV_KEY_INFO* p8inf = NULL; + OSSL_PARAM_BLD* bld = NULL; + EVP_PKEY* pkey = NULL; + BIGNUM* p = NULL; + BIGNUM* q = NULL; + BIGNUM* n = NULL; + BIGNUM* e = NULL; + BIGNUM* d = NULL; + BIGNUM* bignums[5]; + size_t loop; + BIGNUM* num; + BN_ULONG num_val; + const unsigned char* pder = rsa_key_der_2048_pkcs8; + + /* steal parameters from existing key so they can be fuzzed */ + p8inf = d2i_PKCS8_PRIV_KEY_INFO(NULL, (const unsigned char **)&pder, + sizeof(rsa_key_der_2048_pkcs8)); + err = p8inf == NULL; + if (err == 0) { + pkey = EVP_PKCS82PKEY_ex(p8inf, osslLibCtx, NULL); + err = pkey == NULL; + } + if (err == 0) { + err |= EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_FACTOR1, &p) != 1; + err |= EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_FACTOR2, &q) != 1; + err |= EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_N, &n) != 1; + err |= EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_E, &e) != 1; + err |= EVP_PKEY_get_bn_param(pkey, OSSL_PKEY_PARAM_RSA_D, &d) != 1; + err |= (p == NULL) || (q == NULL) || (n == NULL) || + (e == NULL) || (d == NULL); + bignums[0] = p; + bignums[1] = q; + bignums[2] = n; + bignums[3] = e; + bignums[4] = d; + } + if (err == 0) { + bld = OSSL_PARAM_BLD_new(); + err = bld == NULL; + } + + if (err == 0) { + PRINT_MSG("Validate with valid key"); + err = test_rsa_key_integrity_helper(bld, p, q, n, e, d); + } + + if (err == 0) { + num = BN_new(); + err = num == NULL; + } + + for (loop = 0; err == 0 && loop < ARRAY_SIZE(bignums); loop++) { + switch (loop) { + case 0: + PRINT_MSG("Validate with p -= 1, 2"); + break; + case 1: + PRINT_MSG("Validate with q -= 1, 2"); + break; + case 2: + PRINT_MSG("Validate with n -= 1, 2"); + break; + case 3: + PRINT_MSG("Validate with e -= 1, 2"); + break; + case 4: + PRINT_MSG("Validate with d -= 1, 2"); + break; + default: + err = 1; + break; + } + + for (num_val = 1; err == 0 && num_val <= 2; num_val++) { + BIGNUM* bignum = bignums[loop]; + err = BN_set_word(num, num_val) != 1; + + PRINT_MSG("%lu:", num_val); + if (BN_sub(bignum, bignum, num) != 1) { + err = 1; + } + else if ((err = test_rsa_key_integrity_helper(bld, p, q, n, e, d)) == 0) { + err = BN_add(bignum, bignum, num) != 1; + } + } + } + + for (loop = 0; err == 0 && loop < ARRAY_SIZE(bignums); loop++) { + switch (loop) { + case 0: + PRINT_MSG("Validate with p = 0, 1, 2, 3"); + break; + case 1: + PRINT_MSG("Validate with q = 0, 1, 2, 3"); + break; + case 2: + PRINT_MSG("Validate with n = 0, 1, 2, 3"); + break; + case 3: + PRINT_MSG("Validate with e = 0, 1, 2, 3"); + break; + case 4: + PRINT_MSG("Validate with d = 0, 1, 2, 3"); + break; + default: + err = 1; + break; + } + + for (num_val = 0; err == 0 && num_val <= 3; num_val++) { + BIGNUM* bignum = bignums[loop]; + err = BN_copy(num, bignum) == NULL; + + PRINT_MSG("%lu:", num_val); + if (err != 1 && (BN_set_word(bignum, num_val) != 1 || + test_rsa_key_integrity_helper(bld, p, q, n, e, d) != 0)) { + err = 1; + } + if (err != 1 && BN_copy(bignum, num) == NULL) { + err = 1; + } + } + } + + for (loop = 0; err == 0 && loop < ARRAY_SIZE(bignums); loop++) { + switch (loop) { + case 0: + PRINT_MSG("Validate with negative p"); + break; + case 1: + PRINT_MSG("Validate with negative q"); + break; + case 2: + PRINT_MSG("Validate with negative n"); + break; + case 3: + PRINT_MSG("Validate with negative e"); + break; + case 4: + PRINT_MSG("Validate with negative d"); + break; + default: + err = 1; + break; + } + + BIGNUM* bignum = bignums[loop]; + BN_set_negative(bignum, 1); + + if (err != 1 && test_rsa_key_integrity_helper(bld, p, q, n, e, d) != 0) { + err = 1; + } + + BN_set_negative(bignum, 0); + } + + if (err == 0) { + PRINT_MSG("Validate with NULL values"); + err = test_rsa_key_integrity_helper(bld, NULL, NULL, NULL, NULL, NULL); + } + + BN_free(p); + BN_free(q); + BN_free(n); + BN_free(e); + BN_free(d); + BN_free(num); + + return err; +} + #endif /* WP_HAVE_RSA */ diff --git a/test/unit.c b/test/unit.c index a55a5bcf..3b213dd0 100644 --- a/test/unit.c +++ b/test/unit.c @@ -308,6 +308,7 @@ TEST_CASE test_case[] = { TEST_DECL(test_rsa_fromdata, NULL), TEST_DECL(test_rsa_decode, NULL), TEST_DECL(test_rsa_null_init, NULL), + TEST_DECL(test_rsa_key_integrity, NULL), #endif /* WP_HAVE_RSA */ #ifdef WP_HAVE_EC_P192 #ifdef WP_HAVE_ECKEYGEN diff --git a/test/unit.h b/test/unit.h index 5579bf70..35297653 100644 --- a/test/unit.h +++ b/test/unit.h @@ -268,6 +268,7 @@ int test_rsa_load_cert(void* data); int test_rsa_fromdata(void* data); int test_rsa_decode(void* data); int test_rsa_null_init(void* data); +int test_rsa_key_integrity(void* data); #endif /* WP_HAVE_RSA */ #ifdef WP_HAVE_DH