The branch master has been updated via eafd3e9d07e99583a1439bb027e4d6af43e2df27 (commit) via 2c0f7d46b8449423446cfe1e52fc1e1ecd506b62 (commit) from f6f4d1cc00a557232955867b6c04f767e8b5a12e (commit)
- Log ----------------------------------------------------------------- commit eafd3e9d07e99583a1439bb027e4d6af43e2df27 Author: Tomas Mraz <to...@openssl.org> Date: Thu Feb 3 16:30:21 2022 +0100 Add testcases for EVP_PKEY_set1_encoded_public_key() Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/17630) commit 2c0f7d46b8449423446cfe1e52fc1e1ecd506b62 Author: Tomas Mraz <to...@openssl.org> Date: Wed Feb 2 17:47:26 2022 +0100 Replace size check with more meaningful pubkey check It does not make sense to check the size because this function can be used in other contexts than in TLS-1.3 and the value might not be padded to the size of p. However it makes sense to do the partial pubkey check because there is no valid reason having the pubkey value outside the 1 < pubkey < p-1 bounds. Fixes #15465 Reviewed-by: Paul Dale <pa...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/17630) ----------------------------------------------------------------------- Summary of changes: crypto/dh/dh_key.c | 11 +-- test/evp_pkey_dparams_test.c | 183 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 181 insertions(+), 13 deletions(-) diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c index 6b8cd550f2..c78ed618bf 100644 --- a/crypto/dh/dh_key.c +++ b/crypto/dh/dh_key.c @@ -375,20 +375,17 @@ int ossl_dh_buf2key(DH *dh, const unsigned char *buf, size_t len) int err_reason = DH_R_BN_ERROR; BIGNUM *pubkey = NULL; const BIGNUM *p; - size_t p_size; + int ret; if ((pubkey = BN_bin2bn(buf, len, NULL)) == NULL) goto err; DH_get0_pqg(dh, &p, NULL, NULL); - if (p == NULL || (p_size = BN_num_bytes(p)) == 0) { + if (p == NULL || BN_num_bytes(p) == 0) { err_reason = DH_R_NO_PARAMETERS_SET; goto err; } - /* - * As per Section 4.2.8.1 of RFC 8446 fail if DHE's - * public key is of size not equal to size of p - */ - if (BN_is_zero(pubkey) || p_size != len) { + /* Prevent small subgroup attacks per RFC 8446 Section 4.2.8.1 */ + if (!ossl_dh_check_pub_key_partial(dh, pubkey, &ret)) { err_reason = DH_R_INVALID_PUBKEY; goto err; } diff --git a/test/evp_pkey_dparams_test.c b/test/evp_pkey_dparams_test.c index 2b6bd31a66..c6eea7548e 100644 --- a/test/evp_pkey_dparams_test.c +++ b/test/evp_pkey_dparams_test.c @@ -26,6 +26,13 @@ #endif #ifndef OPENSSL_NO_KEYPARAMS + +struct pubkey { + int bad; + const unsigned char *key_bin; + size_t key_bin_len; +}; + # ifndef OPENSSL_NO_DH static const unsigned char dhparam_bin[] = { 0x30,0x82,0x01,0x08,0x02,0x82,0x01,0x01,0x00,0xc0,0xd1,0x2e,0x14,0x18,0xbd,0x03, @@ -46,6 +53,79 @@ static const unsigned char dhparam_bin[] = { 0x06,0x7f,0x7f,0xd7,0x7b,0x42,0x5b,0xba,0x93,0x7a,0xeb,0x43,0x5f,0xce,0x59,0x26, 0xe8,0x76,0xdc,0xee,0xe2,0xbe,0x36,0x7a,0x83,0x02,0x01,0x02 }; +static const unsigned char dhkey_1[] = { + 0x7a, 0x49, 0xcb, 0xc3, 0x25, 0x67, 0x7a, 0x61, + 0xd0, 0x60, 0x81, 0x0f, 0xf6, 0xbd, 0x38, 0x82, + 0xe7, 0x38, 0x8c, 0xe9, 0xd1, 0x04, 0x33, 0xbf, + 0x8a, 0x03, 0x63, 0xb3, 0x05, 0x04, 0xb5, 0x1f, + 0xba, 0x9f, 0x1a, 0x5f, 0x31, 0x3e, 0x96, 0x79, + 0x88, 0x7d, 0x3f, 0x59, 0x6d, 0x3b, 0xf3, 0x2f, + 0xf2, 0xa6, 0x43, 0x48, 0x64, 0x5a, 0x6a, 0x32, + 0x1f, 0x24, 0x37, 0x62, 0x54, 0x3a, 0x7d, 0xab, + 0x26, 0x77, 0x7c, 0xec, 0x57, 0x3c, 0xa4, 0xbd, + 0x96, 0x9d, 0xaa, 0x3b, 0x0e, 0x9a, 0x55, 0x7e, + 0x1d, 0xb4, 0x47, 0x5b, 0xea, 0x20, 0x3c, 0x6d, + 0xbe, 0xd6, 0x70, 0x7d, 0xa8, 0x9e, 0x84, 0xb4, + 0x03, 0x52, 0xf2, 0x08, 0x4c, 0x98, 0xd3, 0x4f, + 0x58, 0xb3, 0xdf, 0xb4, 0xe6, 0xdc, 0x2c, 0x43, + 0x55, 0xd1, 0xce, 0x2a, 0xb3, 0xfc, 0xe0, 0x29, + 0x97, 0xd8, 0xd8, 0x62, 0xc6, 0x87, 0x0a, 0x1b, + 0xfd, 0x72, 0x74, 0xe0, 0xa9, 0xfb, 0xfa, 0x91, + 0xf2, 0xc1, 0x09, 0x93, 0xea, 0x63, 0xf6, 0x9a, + 0x4b, 0xdf, 0x4e, 0xdf, 0x6b, 0xf9, 0xeb, 0xf6, + 0x66, 0x3c, 0xfd, 0x6f, 0x68, 0xcb, 0xdb, 0x6e, + 0x40, 0x65, 0xf7, 0xf2, 0x46, 0xe5, 0x0d, 0x9a, + 0xd9, 0x6f, 0xcf, 0x28, 0x22, 0x8f, 0xca, 0x0b, + 0x30, 0xa0, 0x9e, 0xa5, 0x13, 0xba, 0x72, 0x7f, + 0x85, 0x3d, 0x02, 0x9c, 0x97, 0x8e, 0x6f, 0xea, + 0x6d, 0x35, 0x4e, 0xd1, 0x78, 0x7d, 0x73, 0x60, + 0x92, 0xa9, 0x12, 0xf4, 0x2a, 0xac, 0x17, 0x97, + 0xf3, 0x7b, 0x79, 0x08, 0x69, 0xd1, 0x9e, 0xb5, + 0xf8, 0x2a, 0x0a, 0x2b, 0x00, 0x7b, 0x16, 0x8d, + 0x41, 0x82, 0x3a, 0x72, 0x58, 0x57, 0x80, 0x65, + 0xae, 0x17, 0xbc, 0x3a, 0x5b, 0x7e, 0x5c, 0x2d, + 0xae, 0xb2, 0xc2, 0x26, 0x20, 0x9a, 0xaa, 0x57, + 0x4b, 0x7d, 0x43, 0x41, 0x96, 0x3f, 0xf0, 0x0d +}; +/* smaller but still valid key */ +static const unsigned char dhkey_2[] = { + 0x73, 0xb2, 0x22, 0x91, 0x27, 0xb9, 0x45, 0xb0, + 0xfd, 0x17, 0x66, 0x79, 0x9b, 0x32, 0x71, 0x92, + 0x97, 0x1d, 0x70, 0x02, 0x37, 0x70, 0x79, 0x63, + 0xed, 0x11, 0x22, 0xe9, 0xe6, 0xf8, 0xeb, 0xd7, + 0x90, 0x00, 0xe6, 0x5c, 0x47, 0x02, 0xfb, 0x13, + 0xca, 0x29, 0x14, 0x1e, 0xf4, 0x61, 0x58, 0xf6, + 0xaa, 0xbb, 0xcf, 0xa7, 0x82, 0x9a, 0x9e, 0x7c, + 0x4a, 0x05, 0x42, 0xed, 0x55, 0xd8, 0x08, 0x37, + 0x06, 0x49, 0x9b, 0xda, 0xb3, 0xb9, 0xc9, 0xc0, + 0x56, 0x26, 0xda, 0x60, 0x1d, 0xbc, 0x06, 0x0b, + 0xb0, 0x94, 0x4b, 0x4e, 0x95, 0xf9, 0xb4, 0x2f, + 0x4e, 0xad, 0xf8, 0xab, 0x2d, 0x19, 0xa2, 0xe6, + 0x6d, 0x11, 0xfd, 0x9b, 0x5a, 0x2a, 0xb0, 0x81, + 0x42, 0x4d, 0x86, 0x76, 0xd5, 0x9e, 0xaf, 0xf9, + 0x6f, 0x79, 0xab, 0x1d, 0xfe, 0xd8, 0xc8, 0xba, + 0xb6, 0xce, 0x03, 0x61, 0x48, 0x53, 0xd8, 0x0b, + 0x83, 0xf0, 0xb0, 0x46, 0xa0, 0xea, 0x46, 0x60, + 0x7a, 0x39, 0x4e, 0x46, 0x6a, 0xbb, 0x07, 0x6c, + 0x8c, 0x7d, 0xb7, 0x7d, 0x5b, 0xe5, 0x24, 0xa5, + 0xab, 0x41, 0x8a, 0xc4, 0x63, 0xf9, 0xce, 0x20, + 0x6f, 0x58, 0x4f, 0x0e, 0x42, 0x82, 0x9e, 0x17, + 0x53, 0xa6, 0xd6, 0x42, 0x3e, 0x80, 0x66, 0x6f, + 0x2a, 0x1c, 0x30, 0x08, 0x01, 0x99, 0x5a, 0x4f, + 0x72, 0x16, 0xed, 0xb0, 0xd6, 0x8c, 0xf0, 0x7a, + 0x33, 0x15, 0xc4, 0x95, 0x65, 0xba, 0x11, 0x37, + 0xa0, 0xcc, 0xe7, 0x45, 0x65, 0x4f, 0x17, 0x0a, + 0x2c, 0x62, 0xc0, 0x65, 0x3b, 0x65, 0x2a, 0x56, + 0xf7, 0x29, 0x8a, 0x9b, 0x1b, 0xbb, 0x0c, 0x40, + 0xcd, 0x66, 0x4b, 0x4f, 0x2f, 0xba, 0xdb, 0x59, + 0x93, 0x6d, 0x34, 0xf3, 0x8d, 0xde, 0x68, 0x99, + 0x78, 0xfc, 0xac, 0x95, 0xd9, 0xa3, 0x74, 0xe6, + 0x24, 0x96, 0x98, 0x6f, 0x64, 0x71, 0x76 +}; +/* 1 is not a valid key */ +static const unsigned char dhkey_3[] = { + 0x01 +}; # endif # ifndef OPENSSL_NO_DSA @@ -92,21 +172,73 @@ static const unsigned char dsaparam_bin[] = { static const unsigned char ecparam_bin[] = { 0x06,0x08,0x2a,0x86,0x48,0xce,0x3d,0x03,0x01,0x07 }; +static const unsigned char eckey_1[] = { + 0x04, 0xc8, 0x65, 0x45, 0x63, 0x73, 0xe5, 0x0a, + 0x61, 0x1d, 0xcf, 0x60, 0x76, 0x2c, 0xe7, 0x36, + 0x0b, 0x76, 0xc2, 0x92, 0xfc, 0xa4, 0x56, 0xee, + 0xc2, 0x62, 0x05, 0x00, 0x80, 0xe4, 0x4f, 0x07, + 0x3b, 0xf4, 0x59, 0xb8, 0xc3, 0xb3, 0x1f, 0x77, + 0x36, 0x16, 0x4c, 0x72, 0x2a, 0xc0, 0x89, 0x89, + 0xd6, 0x16, 0x14, 0xee, 0x2f, 0x5a, 0xde, 0x9e, + 0x83, 0xc5, 0x78, 0xd0, 0x0b, 0x69, 0xb4, 0xb9, + 0xf1 +}; +/* a modified key */ +static const unsigned char eckey_2[] = { + 0x04, 0xc8, 0x65, 0x45, 0x63, 0x73, 0xe5, 0x0a, + 0x61, 0x1d, 0xcf, 0x60, 0x76, 0x2c, 0xe7, 0x36, + 0x0b, 0x77, 0xc2, 0x92, 0xfc, 0xa4, 0x56, 0xee, + 0xc2, 0x62, 0x05, 0x00, 0x80, 0xe4, 0x4f, 0x07, + 0x3b, 0xf4, 0x59, 0xb8, 0xc3, 0xb3, 0x1f, 0x77, + 0x36, 0x16, 0x4c, 0x72, 0x2a, 0xc0, 0x89, 0x89, + 0xd6, 0x16, 0x14, 0xee, 0x2f, 0x5a, 0xde, 0x9e, + 0x83, 0xc5, 0x78, 0xd0, 0x0b, 0x69, 0xb4, 0xb9, + 0xf1 +}; +/* an added byte */ +static const unsigned char eckey_3[] = { + 0x04, 0xc8, 0x65, 0x45, 0x63, 0x73, 0xe5, 0x0a, + 0x61, 0x1d, 0xcf, 0x60, 0x76, 0x2c, 0xe7, 0x36, + 0x0b, 0x76, 0xc2, 0x92, 0xfc, 0xa4, 0x56, 0xee, + 0xc2, 0x62, 0x05, 0x00, 0x80, 0xe4, 0x4f, 0x07, + 0x3b, 0xf4, 0x59, 0xb8, 0xc3, 0xb3, 0x1f, 0x77, + 0x36, 0x16, 0x4c, 0x72, 0x2a, 0xc0, 0x89, 0x89, + 0xd6, 0x16, 0x14, 0xee, 0x2f, 0x5a, 0xde, 0x9e, + 0x83, 0xc5, 0x78, 0xd0, 0x0b, 0x69, 0xb4, 0xb9, + 0xf1, 0xaa +}; # endif +#define NUM_KEYS 10 + static const struct { int type; const unsigned char *param_bin; size_t param_bin_len; + struct pubkey keys[NUM_KEYS]; } pkey_params [] = { # ifndef OPENSSL_NO_DH - { EVP_PKEY_DH, dhparam_bin, sizeof(dhparam_bin) }, + { EVP_PKEY_DH, dhparam_bin, sizeof(dhparam_bin), + { { 0, dhkey_1, sizeof(dhkey_1) }, + { 0, dhkey_2, sizeof(dhkey_2) }, + { 1, dhkey_3, sizeof(dhkey_3) }, + { 1, dhkey_1, 0 }, + { 1, dhparam_bin, sizeof(dhparam_bin) } + } + }, # endif # ifndef OPENSSL_NO_DSA { EVP_PKEY_DSA, dsaparam_bin, sizeof(dsaparam_bin) }, # endif # ifndef OPENSSL_NO_EC - { EVP_PKEY_EC, ecparam_bin, sizeof(ecparam_bin) } + { EVP_PKEY_EC, ecparam_bin, sizeof(ecparam_bin), + { { 0, eckey_1, sizeof(eckey_1) }, + { 1, eckey_2, sizeof(eckey_2) }, + { 1, eckey_3, sizeof(eckey_3) }, + { 1, eckey_1, 0 }, + { 1, eckey_1, sizeof(eckey_1) - 1 } + } + } # endif }; @@ -114,13 +246,11 @@ static int params_bio_test(int id) { int ret, out_len; BIO *in = NULL, *out = NULL; - EVP_PKEY_CTX *ctx = NULL; EVP_PKEY *in_key = NULL, *out_key = NULL; unsigned char *out_bin; int type = pkey_params[id].type; - ret = TEST_ptr(ctx = EVP_PKEY_CTX_new_id(type, NULL)) - && TEST_ptr(in = BIO_new_mem_buf(pkey_params[id].param_bin, + ret = TEST_ptr(in = BIO_new_mem_buf(pkey_params[id].param_bin, (int)pkey_params[id].param_bin_len)) /* Load in pkey params from binary */ && TEST_ptr(d2i_KeyParams_bio(type, &in_key, in)) @@ -137,7 +267,47 @@ static int params_bio_test(int id) BIO_free(out); EVP_PKEY_free(in_key); EVP_PKEY_free(out_key); - EVP_PKEY_CTX_free(ctx); + return ret; +} + +static int set_enc_pubkey_test(int id) +{ + int ret, i; + BIO *in = NULL; + EVP_PKEY *in_key = NULL; + int type = pkey_params[id].type; + const struct pubkey *keys = pkey_params[id].keys; + + if (keys[0].key_bin == NULL) + return TEST_skip("Not applicable test"); + + ret = TEST_ptr(in = BIO_new_mem_buf(pkey_params[id].param_bin, + (int)pkey_params[id].param_bin_len)) + /* Load in pkey params from binary */ + && TEST_ptr(d2i_KeyParams_bio(type, &in_key, in)); + + for (i = 0; ret && i < NUM_KEYS && keys[i].key_bin != NULL; i++) { + if (keys[i].bad) { + ERR_set_mark(); + ret = ret + && TEST_int_le(EVP_PKEY_set1_encoded_public_key(in_key, + keys[i].key_bin, + keys[i].key_bin_len), + 0); + ERR_pop_to_mark(); + } else { + ret = ret + && TEST_int_gt(EVP_PKEY_set1_encoded_public_key(in_key, + keys[i].key_bin, + keys[i].key_bin_len), + 0); + } + if (!ret) + TEST_info("Test key index #%d", i); + } + + BIO_free(in); + EVP_PKEY_free(in_key); return ret; } #endif @@ -148,6 +318,7 @@ int setup_tests(void) TEST_note("No DH/DSA/EC support"); #else ADD_ALL_TESTS(params_bio_test, OSSL_NELEM(pkey_params)); + ADD_ALL_TESTS(set_enc_pubkey_test, OSSL_NELEM(pkey_params)); #endif return 1; }