From a69184fb9d6fb2ad91903631fd1086220ed1e584 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 17 Jul 2023 11:43:47 +0200 Subject: [PATCH 1/5] wolfssl: Reject EC keys with explicitly encoded parameters These are not allowed in X.509 certificates according to RFC 5480 and some newer validations apparently explicitly check for this. Note that WolfSSL rejects such keys, by default. Only when compiled with WOLFSSL_NO_ASN_STRICT are they accepted. --- src/libstrongswan/plugins/wolfssl/wolfssl_ec_private_key.c | 3 ++- src/libstrongswan/plugins/wolfssl/wolfssl_ec_public_key.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_private_key.c b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_private_key.c index a08cc17e3..addd3bda2 100644 --- a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_private_key.c +++ b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_private_key.c @@ -449,7 +449,8 @@ wolfssl_ec_private_key_t *wolfssl_ec_private_key_load(key_type_t type, } idx = 0; - if (wc_EccPrivateKeyDecode(key.ptr, &idx, &this->ec, key.len) < 0) + if (wc_EccPrivateKeyDecode(key.ptr, &idx, &this->ec, key.len) < 0 || + this->ec.idx == -1) { destroy(this); return NULL; diff --git a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_public_key.c b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_public_key.c index 97abe950b..58fd6eded 100644 --- a/src/libstrongswan/plugins/wolfssl/wolfssl_ec_public_key.c +++ b/src/libstrongswan/plugins/wolfssl/wolfssl_ec_public_key.c @@ -378,7 +378,7 @@ wolfssl_ec_public_key_t *wolfssl_ec_public_key_load(key_type_t type, idx = 0; ret = wc_EccPublicKeyDecode(blob.ptr, &idx, &this->ec, blob.len); - if (ret < 0) + if (ret < 0 || this->ec.idx == -1) { destroy(this); return NULL; From 2bccdefc2c9231d8f74e6a587b19139589f92c51 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 17 Jul 2023 12:01:06 +0200 Subject: [PATCH 2/5] openssl: Reject EC keys with explicitly encoded parameters EC_KEY_decoded_from_explicit_params() was added with 1.1.1h but has been deprecated with 3.0. --- .../plugins/openssl/openssl_ec_private_key.c | 4 +++- .../plugins/openssl/openssl_ec_public_key.c | 23 ++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_private_key.c b/src/libstrongswan/plugins/openssl/openssl_ec_private_key.c index 512d624b7..21df4c035 100644 --- a/src/libstrongswan/plugins/openssl/openssl_ec_private_key.c +++ b/src/libstrongswan/plugins/openssl/openssl_ec_private_key.c @@ -62,6 +62,7 @@ struct private_openssl_ec_private_key_t { /* from openssl_ec_public_key */ bool openssl_check_ec_key_curve(EVP_PKEY *key, int nid_curve); +bool openssl_check_explicit_params(EVP_PKEY *key); /** * Build a DER encoded signature as in RFC 3279 @@ -474,8 +475,9 @@ openssl_ec_private_key_t *openssl_ec_private_key_load(key_type_t type, blob.len); } - if (!key) + if (!key || openssl_check_explicit_params(key)) { + EVP_PKEY_free(key); return NULL; } this = create_internal(key); diff --git a/src/libstrongswan/plugins/openssl/openssl_ec_public_key.c b/src/libstrongswan/plugins/openssl/openssl_ec_public_key.c index 142e91f9b..7c21902a7 100644 --- a/src/libstrongswan/plugins/openssl/openssl_ec_public_key.c +++ b/src/libstrongswan/plugins/openssl/openssl_ec_public_key.c @@ -299,6 +299,26 @@ METHOD(public_key_t, destroy, void, } } +/** + * Check whether the EC key was decoded with explicit curve parameters instead + * of a named curve. + */ +bool openssl_check_explicit_params(const EVP_PKEY *key) +{ + int explicit = 0; + +#if OPENSSL_VERSION_NUMBER >= 0x30000000L + if (!EVP_PKEY_get_int_param(key, OSSL_PKEY_PARAM_EC_DECODED_FROM_EXPLICIT_PARAMS, + &explicit)) + { + return FALSE; + } +#elif OPENSSL_VERSION_NUMBER >= 0x1010108fL + explicit = EC_KEY_decoded_from_explicit_params(EVP_PKEY_get0_EC_KEY((EVP_PKEY*)key)); +#endif + return explicit == 1; +} + /** * See header. */ @@ -324,7 +344,8 @@ openssl_ec_public_key_t *openssl_ec_public_key_load(key_type_t type, break; } key = d2i_PUBKEY(NULL, (const u_char**)&blob.ptr, blob.len); - if (!key || EVP_PKEY_base_id(key) != EVP_PKEY_EC) + if (!key || EVP_PKEY_base_id(key) != EVP_PKEY_EC || + openssl_check_explicit_params(key)) { EVP_PKEY_free(key); return NULL; From 0b989c7b20be8e575eda66bf1e107b38b187e08b Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 31 Aug 2023 14:27:09 +0200 Subject: [PATCH 3/5] botan: Reject EC keys with explicitly encoded parameters This requires a function that will be added in the upcoming Botan 3.2 release. --- configure.ac | 2 +- src/libstrongswan/plugins/botan/botan_ec_public_key.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index e89e702be..365f5cb96 100644 --- a/configure.ac +++ b/configure.ac @@ -1215,7 +1215,7 @@ if test x$botan = xtrue; then AC_SUBST(botan_LIBS) saved_LIBS=$LIBS LIBS="$botan_LIBS" - AC_CHECK_FUNCS(botan_rng_init_custom) + AC_CHECK_FUNCS(botan_rng_init_custom botan_pubkey_ecc_key_used_explicit_encoding) LIBS=$saved_LIBS fi diff --git a/src/libstrongswan/plugins/botan/botan_ec_public_key.c b/src/libstrongswan/plugins/botan/botan_ec_public_key.c index 95def4fa7..bd23bd0c8 100644 --- a/src/libstrongswan/plugins/botan/botan_ec_public_key.c +++ b/src/libstrongswan/plugins/botan/botan_ec_public_key.c @@ -235,6 +235,14 @@ botan_ec_public_key_t *botan_ec_public_key_adopt(botan_pubkey_t key) { private_botan_ec_public_key_t *this; +#ifdef HAVE_BOTAN_PUBKEY_ECC_KEY_USED_EXPLICIT_ENCODING + if (botan_pubkey_ecc_key_used_explicit_encoding(key)) + { + botan_pubkey_destroy(key); + return NULL; + } +#endif + INIT(this, .public = { .key = { From 578b561a22216736dab52ffe5e2dadda1b3f2091 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 31 Aug 2023 14:46:51 +0200 Subject: [PATCH 4/5] Use Botan 3.2.0 for tests This includes a change that allows checking EC keys for explicit param encoding. --- scripts/test.sh | 2 +- testing/scripts/recipes/011_botan.mk | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/test.sh b/scripts/test.sh index ea0db8ad7..d9a922417 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -4,7 +4,7 @@ build_botan() { # same revision used in the build recipe of the testing environment - BOTAN_REV=3.1.1 + BOTAN_REV=3.2.0 BOTAN_DIR=$DEPS_BUILD_DIR/botan if test -d "$BOTAN_DIR"; then diff --git a/testing/scripts/recipes/011_botan.mk b/testing/scripts/recipes/011_botan.mk index 315878c12..a4c747781 100644 --- a/testing/scripts/recipes/011_botan.mk +++ b/testing/scripts/recipes/011_botan.mk @@ -2,7 +2,7 @@ PKG = botan SRC = https://github.com/randombit/$(PKG).git -REV = 3.1.1 +REV = 3.2.0 NUM_CPUS := $(shell getconf _NPROCESSORS_ONLN) From bb14a2867198c65864946a38ca87eb82a680695f Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Mon, 17 Jul 2023 12:32:59 +0200 Subject: [PATCH 5/5] unit-tests: Add a test case for explicit ECDSA parameters Currently only warns about it as older OpenSSL versions (AppVeyor) don't reject them. --- src/libstrongswan/tests/suites/test_ecdsa.c | 80 +++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/src/libstrongswan/tests/suites/test_ecdsa.c b/src/libstrongswan/tests/suites/test_ecdsa.c index a3981ce0a..599a64083 100644 --- a/src/libstrongswan/tests/suites/test_ecdsa.c +++ b/src/libstrongswan/tests/suites/test_ecdsa.c @@ -339,6 +339,85 @@ START_TEST(test_load) } END_TEST +/** + * ECDSA-256 key from above, converted with: openssl ec -param_enc explicit + */ +static chunk_t explicit_params = chunk_from_chars( + 0x30,0x82,0x01,0x68,0x02,0x01,0x01,0x04,0x20,0x42,0xc6,0x8c,0xff,0x2b,0x8b,0x87, + 0xa1,0xfb,0x50,0xf6,0xfe,0xd6,0x88,0xb3,0x0a,0x48,0xb2,0xc5,0x8f,0x50,0xe0,0xcf, + 0x40,0xfa,0x57,0xd1,0xc6,0x6c,0x20,0x64,0xc5,0xa0,0x81,0xfa,0x30,0x81,0xf7,0x02, + 0x01,0x01,0x30,0x2c,0x06,0x07,0x2a,0x86,0x48,0xce,0x3d,0x01,0x01,0x02,0x21,0x00, + 0xff,0xff,0xff,0xff,0x00,0x00,0x00,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0x30,0x5b,0x04,0x20,0xff,0xff,0xff,0xff,0x00,0x00,0x00,0x01,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xfc,0x04,0x20,0x5a,0xc6,0x35,0xd8,0xaa,0x3a,0x93,0xe7,0xb3,0xeb, + 0xbd,0x55,0x76,0x98,0x86,0xbc,0x65,0x1d,0x06,0xb0,0xcc,0x53,0xb0,0xf6,0x3b,0xce, + 0x3c,0x3e,0x27,0xd2,0x60,0x4b,0x03,0x15,0x00,0xc4,0x9d,0x36,0x08,0x86,0xe7,0x04, + 0x93,0x6a,0x66,0x78,0xe1,0x13,0x9d,0x26,0xb7,0x81,0x9f,0x7e,0x90,0x04,0x41,0x04, + 0x6b,0x17,0xd1,0xf2,0xe1,0x2c,0x42,0x47,0xf8,0xbc,0xe6,0xe5,0x63,0xa4,0x40,0xf2, + 0x77,0x03,0x7d,0x81,0x2d,0xeb,0x33,0xa0,0xf4,0xa1,0x39,0x45,0xd8,0x98,0xc2,0x96, + 0x4f,0xe3,0x42,0xe2,0xfe,0x1a,0x7f,0x9b,0x8e,0xe7,0xeb,0x4a,0x7c,0x0f,0x9e,0x16, + 0x2b,0xce,0x33,0x57,0x6b,0x31,0x5e,0xce,0xcb,0xb6,0x40,0x68,0x37,0xbf,0x51,0xf5, + 0x02,0x21,0x00,0xff,0xff,0xff,0xff,0x00,0x00,0x00,0x00,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xbc,0xe6,0xfa,0xad,0xa7,0x17,0x9e,0x84,0xf3,0xb9,0xca,0xc2,0xfc, + 0x63,0x25,0x51,0x02,0x01,0x01,0xa1,0x44,0x03,0x42,0x00,0x04,0x9c,0xb2,0x52,0xcb, + 0xc0,0x5c,0xcf,0x97,0xdd,0xd6,0xe7,0x49,0x32,0x47,0x0c,0x8e,0xdb,0x6d,0xbf,0xc8, + 0x1a,0x0a,0x01,0xe8,0x5e,0x3f,0x8e,0x64,0x33,0xb4,0x15,0xbb,0x1b,0xa5,0xed,0xf9, + 0x4b,0xa7,0xe8,0x5e,0x6f,0x49,0x24,0xf7,0x32,0xf4,0x9b,0x4c,0x47,0xdc,0xf1,0x28, + 0x44,0x1c,0x37,0xdb,0xee,0xfb,0xd8,0xbd,0x4e,0x5c,0xeb,0x07); + +/** + * Public key of the above with: openssl ec -param_enc explicit -pubout + */ +static chunk_t explicit_params_pub = chunk_from_chars( + 0x30,0x82,0x01,0x4b,0x30,0x82,0x01,0x03,0x06,0x07,0x2a,0x86,0x48,0xce,0x3d,0x02, + 0x01,0x30,0x81,0xf7,0x02,0x01,0x01,0x30,0x2c,0x06,0x07,0x2a,0x86,0x48,0xce,0x3d, + 0x01,0x01,0x02,0x21,0x00,0xff,0xff,0xff,0xff,0x00,0x00,0x00,0x01,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xff,0xff,0x30,0x5b,0x04,0x20,0xff,0xff,0xff,0xff,0x00,0x00,0x00, + 0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xff,0xff,0xff, + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xfc,0x04,0x20,0x5a,0xc6,0x35,0xd8,0xaa, + 0x3a,0x93,0xe7,0xb3,0xeb,0xbd,0x55,0x76,0x98,0x86,0xbc,0x65,0x1d,0x06,0xb0,0xcc, + 0x53,0xb0,0xf6,0x3b,0xce,0x3c,0x3e,0x27,0xd2,0x60,0x4b,0x03,0x15,0x00,0xc4,0x9d, + 0x36,0x08,0x86,0xe7,0x04,0x93,0x6a,0x66,0x78,0xe1,0x13,0x9d,0x26,0xb7,0x81,0x9f, + 0x7e,0x90,0x04,0x41,0x04,0x6b,0x17,0xd1,0xf2,0xe1,0x2c,0x42,0x47,0xf8,0xbc,0xe6, + 0xe5,0x63,0xa4,0x40,0xf2,0x77,0x03,0x7d,0x81,0x2d,0xeb,0x33,0xa0,0xf4,0xa1,0x39, + 0x45,0xd8,0x98,0xc2,0x96,0x4f,0xe3,0x42,0xe2,0xfe,0x1a,0x7f,0x9b,0x8e,0xe7,0xeb, + 0x4a,0x7c,0x0f,0x9e,0x16,0x2b,0xce,0x33,0x57,0x6b,0x31,0x5e,0xce,0xcb,0xb6,0x40, + 0x68,0x37,0xbf,0x51,0xf5,0x02,0x21,0x00,0xff,0xff,0xff,0xff,0x00,0x00,0x00,0x00, + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xbc,0xe6,0xfa,0xad,0xa7,0x17,0x9e,0x84, + 0xf3,0xb9,0xca,0xc2,0xfc,0x63,0x25,0x51,0x02,0x01,0x01,0x03,0x42,0x00,0x04,0x9c, + 0xb2,0x52,0xcb,0xc0,0x5c,0xcf,0x97,0xdd,0xd6,0xe7,0x49,0x32,0x47,0x0c,0x8e,0xdb, + 0x6d,0xbf,0xc8,0x1a,0x0a,0x01,0xe8,0x5e,0x3f,0x8e,0x64,0x33,0xb4,0x15,0xbb,0x1b, + 0xa5,0xed,0xf9,0x4b,0xa7,0xe8,0x5e,0x6f,0x49,0x24,0xf7,0x32,0xf4,0x9b,0x4c,0x47, + 0xdc,0xf1,0x28,0x44,0x1c,0x37,0xdb,0xee,0xfb,0xd8,0xbd,0x4e,0x5c,0xeb,0x07); + +START_TEST(test_load_reject_explicit_params) +{ + private_key_t *privkey; + public_key_t *pubkey; + + pubkey = lib->creds->create(lib->creds, CRED_PUBLIC_KEY, KEY_ECDSA, + BUILD_BLOB_ASN1_DER, explicit_params_pub, + BUILD_END); + if (pubkey) + { + pubkey->destroy(pubkey); + warn("ECDSA public key with explicit parameters not rejected"); + } + + privkey = lib->creds->create(lib->creds, CRED_PRIVATE_KEY, KEY_ECDSA, + BUILD_BLOB_ASN1_DER, explicit_params, + BUILD_END); + if (privkey) + { + privkey->destroy(privkey); + warn("ECDSA private key with explicit parameters not rejected"); + } +} +END_TEST + Suite *ecdsa_suite_create() { Suite *s; @@ -358,6 +437,7 @@ Suite *ecdsa_suite_create() tc = tcase_create("load"); tcase_add_loop_test(tc, test_load, 0, countof(keys)); + tcase_add_test(tc, test_load_reject_explicit_params); suite_add_tcase(s, tc); return s;