From be064f77b93bda370e4165e6ad6da17324835c9e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 14 Mar 2020 13:38:53 -0400 Subject: Revise TROVE-2020-002 fix to work on older OpenSSL versions. Although OpenSSL before 1.1.1 is no longer supported, it's possible that somebody is still using it with 0.3.5, so we probably shouldn't break it with this fix. --- src/lib/crypt_ops/crypto_rsa_openssl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/crypt_ops/crypto_rsa_openssl.c b/src/lib/crypt_ops/crypto_rsa_openssl.c index 022a0dc093..39b7aaf0cf 100644 --- a/src/lib/crypt_ops/crypto_rsa_openssl.c +++ b/src/lib/crypt_ops/crypto_rsa_openssl.c @@ -584,7 +584,11 @@ crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits) crypto_openssl_log_errors(LOG_WARN,"decoding private key"); return NULL; } +#ifdef OPENSSL_1_1_API if (max_bits >= 0 && RSA_bits(rsa) > max_bits) { +#else + if (max_bits >= 0 && rsa->n && BN_num_bits(rsa->n) > max_bits) { +#endif log_info(LD_CRYPTO, "Private key longer than expected."); return NULL; } -- cgit v1.2.3-54-g00ecf From ab2e66ccdc4406a195d77b202bae21c17c634cb5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 14 Mar 2020 13:50:38 -0400 Subject: Add a test for crypto_pk_asn1_decode_private maxbits. --- src/test/test_crypto.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index fa79f4cc47..2373e5bf86 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -1491,6 +1491,29 @@ test_crypto_pk_pem_encrypted(void *arg) crypto_pk_free(pk); } +static void +test_crypto_pk_bad_size(void *arg) +{ + (void)arg; + crypto_pk_t *pk1 = pk_generate(0); + crypto_pk_t *pk2 = NULL; + char buf[2048]; + int n = crypto_pk_asn1_encode_private(pk1, buf, sizeof(buf)); + tt_int_op(n, OP_GT, 0); + + /* Set the max bit count smaller: we should refuse to decode the key.*/ + pk2 = crypto_pk_asn1_decode_private(buf, n, 1020); + tt_assert(! pk2); + + /* Set the max bit count larger: it should decode fine. */ + pk2 = crypto_pk_asn1_decode_private(buf, n, 2048); + tt_assert(pk2); + + done: + crypto_pk_free(pk1); + crypto_pk_free(pk2); +} + static void test_crypto_pk_invalid_private_key(void *arg) { @@ -3163,6 +3186,7 @@ struct testcase_t crypto_tests[] = { { "pk_fingerprints", test_crypto_pk_fingerprints, TT_FORK, NULL, NULL }, { "pk_base64", test_crypto_pk_base64, TT_FORK, NULL, NULL }, { "pk_pem_encrypted", test_crypto_pk_pem_encrypted, TT_FORK, NULL, NULL }, + { "pk_bad_size", test_crypto_pk_bad_size, 0, NULL, NULL }, { "pk_invalid_private_key", test_crypto_pk_invalid_private_key, 0, NULL, NULL }, CRYPTO_LEGACY(digests), -- cgit v1.2.3-54-g00ecf From 29c9675bdeb5a63564e9a76dcd7162bef884b240 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 14 Mar 2020 14:17:33 -0400 Subject: Fix memory leak in crypto_pk_asn1_decode_private. (Deep, deep thanks to Taylor for reminding me to test this!) --- src/lib/crypt_ops/crypto_rsa_openssl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/crypt_ops/crypto_rsa_openssl.c b/src/lib/crypt_ops/crypto_rsa_openssl.c index 39b7aaf0cf..0db25f3473 100644 --- a/src/lib/crypt_ops/crypto_rsa_openssl.c +++ b/src/lib/crypt_ops/crypto_rsa_openssl.c @@ -590,6 +590,7 @@ crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits) if (max_bits >= 0 && rsa->n && BN_num_bits(rsa->n) > max_bits) { #endif log_info(LD_CRYPTO, "Private key longer than expected."); + RSA_free(rsa); return NULL; } crypto_pk_t *result = crypto_new_pk_from_openssl_rsa_(rsa); -- cgit v1.2.3-54-g00ecf