March 2020 - tor-commits - lists.torproject.org

[tor/maint-0.4.3] Merge branch 'maint-0.4.1' into maint-0.4.2
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit 758deaa472b879645828c1ee585eb6ce8b8a9a50 Merge: 85141a3a7 4ad3f17c2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 18 08:20:16 2020 -0400 Merge branch 'maint-0.4.1' into maint-0.4.2 src/lib/crypt_ops/crypto_rsa_openssl.c | 7 +++++++ 1 file changed, 7 insertions(+)

1 0

[tor/master] Add a test for crypto_pk_asn1_decode_private maxbits.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit ab2e66ccdc4406a195d77b202bae21c17c634cb5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Mar 14 13:50:38 2020 -0400 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 fa79f4cc4..2373e5bf8 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -1492,6 +1492,29 @@ test_crypto_pk_pem_encrypted(void *arg) } 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) { (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),

1 0

[tor/master] When parsing tokens, reject early on spurious keys.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit f160212ee855b6899063f2e9355abd59105d6a04 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Feb 5 11:57:31 2020 -0500 When parsing tokens, reject early on spurious keys. --- src/feature/dirparse/parsecommon.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/feature/dirparse/parsecommon.c b/src/feature/dirparse/parsecommon.c index 5b753f0f2..e8269f7ec 100644 --- a/src/feature/dirparse/parsecommon.c +++ b/src/feature/dirparse/parsecommon.c @@ -384,10 +384,16 @@ get_next_token(memarea_t *area, RET_ERR("Couldn't parse object: missing footer or object much too big."); if (!strcmp(tok->object_type, "RSA PUBLIC KEY")) { /* If it's a public key */ + if (o_syn != NEED_KEY && o_syn != NEED_KEY_1024 && o_syn != OBJ_OK) { + RET_ERR("Unexpected public key."); + } tok->key = crypto_pk_new(); if (crypto_pk_read_public_key_from_string(tok->key, obstart, eol-obstart)) RET_ERR("Couldn't parse public key."); } else if (!strcmp(tok->object_type, "RSA PRIVATE KEY")) { /* private key */ + if (o_syn != NEED_SKEY_1024 && o_syn != OBJ_OK) { + RET_ERR("Unexpected private key."); + } tok->key = crypto_pk_new(); if (crypto_pk_read_private_key1024_from_string(tok->key, obstart, eol-obstart))

1 0

[tor/master] Merge branch 'trove_2020_002_035' into trove_2020_002_041
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit 1a375c3b193f73e73e7c9c640dccdf1eb027234b Merge: 7afb95d3e d0bce65ce Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Feb 5 12:06:24 2020 -0500 Merge branch 'trove_2020_002_035' into trove_2020_002_041 Resolved Conflicts: src/feature/dirparse/parsecommon.c changes/ticket33119 | 8 ++++++++ src/feature/dirparse/parsecommon.c | 9 ++++++++- src/lib/crypt_ops/crypto_rsa.c | 27 +++++++++++++++++++++------ src/lib/crypt_ops/crypto_rsa.h | 5 ++++- src/lib/crypt_ops/crypto_rsa_nss.c | 14 +++++++++++++- src/lib/crypt_ops/crypto_rsa_openssl.c | 11 +++++++++-- 6 files changed, 63 insertions(+), 11 deletions(-) diff --cc src/feature/dirparse/parsecommon.c index 036a51689,e8269f7ec..6c2c4b06b --- a/src/feature/dirparse/parsecommon.c +++ b/src/feature/dirparse/parsecommon.c @@@ -392,25 -383,28 +392,32 @@@ get_next_token(memarea_t *area if (next - *s > MAX_UNPARSED_OBJECT_SIZE) RET_ERR("Couldn't parse object: missing footer or object much too big."); + { + int r; + size_t maxsize = base64_decode_maxsize(next-*s); + tok->object_body = ALLOC(maxsize); + r = base64_decode(tok->object_body, maxsize, *s, next-*s); + if (r<0) + RET_ERR("Malformed object: bad base64-encoded data"); + tok->object_size = r; + } + if (!strcmp(tok->object_type, "RSA PUBLIC KEY")) { /* If it's a public key */ + if (o_syn != NEED_KEY && o_syn != NEED_KEY_1024 && o_syn != OBJ_OK) { + RET_ERR("Unexpected public key."); + } - tok->key = crypto_pk_new(); - if (crypto_pk_read_public_key_from_string(tok->key, obstart, eol-obstart)) + tok->key = crypto_pk_asn1_decode(tok->object_body, tok->object_size); + if (! tok->key) RET_ERR("Couldn't parse public key."); } else if (!strcmp(tok->object_type, "RSA PRIVATE KEY")) { /* private key */ + if (o_syn != NEED_SKEY_1024 && o_syn != OBJ_OK) { + RET_ERR("Unexpected private key."); + } - tok->key = crypto_pk_new(); - if (crypto_pk_read_private_key1024_from_string(tok->key, - obstart, eol-obstart)) + tok->key = crypto_pk_asn1_decode_private(tok->object_body, - tok->object_size); ++ tok->object_size, ++ 1024); + if (! tok->key) RET_ERR("Couldn't parse private key."); - } else { /* If it's something else, try to base64-decode it */ - int r; - tok->object_body = ALLOC(next-*s); /* really, this is too much RAM. */ - r = base64_decode(tok->object_body, next-*s, *s, next-*s); - if (r<0) - RET_ERR("Malformed object: bad base64-encoded data"); - tok->object_size = r; } *s = eol;

1 0

[tor/maint-0.4.3] Merge branch 'maint-0.3.5' into maint-0.4.1
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit 4ad3f17c26166b898cd768a6e1835fad2ef67005 Merge: 57b1d0848 0526801ed Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 18 08:20:16 2020 -0400 Merge branch 'maint-0.3.5' into maint-0.4.1 src/lib/crypt_ops/crypto_rsa_openssl.c | 7 +++++++ 1 file changed, 7 insertions(+)

1 0

[tor/master] changes file for 33119 aka TROVE-2020-002
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit d0bce65ce2426793a975e691204c3fb2ac667f66 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Feb 5 12:02:32 2020 -0500 changes file for 33119 aka TROVE-2020-002 --- changes/ticket33119 | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/changes/ticket33119 b/changes/ticket33119 new file mode 100644 index 000000000..11c20bc7a --- /dev/null +++ b/changes/ticket33119 @@ -0,0 +1,8 @@ + o Major bugfixes (security, denial-of-service): + - Fix a denial-of-service bug that could be used by anyone to consume a + bunch of CPU on any Tor relay or authority, or by directories to + consume a bunch of CPU on clients or hidden services. Because + of the potential for CPU consumption to introduce observable + timing patterns, we are treating this as a high-severity security + issue. Fixes bug 33119; bugfix on 0.2.1.5-alpha. We are also tracking + this issue as TROVE-2020-002.

1 0

[tor/master] Fix memory leak in crypto_pk_asn1_decode_private.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit 29c9675bdeb5a63564e9a76dcd7162bef884b240 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Mar 14 14:17:33 2020 -0400 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 39b7aaf0c..0db25f347 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);

1 0

[tor/master] Merge branch 'trove_2020_002_035' into trove_2020_002_041
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit 9163781039b96e859fc102f003a274e9716cc02d Merge: dd6e2277e f958b537a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 10:45:03 2020 -0400 Merge branch 'trove_2020_002_035' into trove_2020_002_041 src/lib/crypt_ops/crypto_rsa_nss.c | 2 +- src/lib/crypt_ops/crypto_rsa_openssl.c | 57 +++++++++++++++++++++++++++++++--- src/test/test_crypto.c | 15 +++++++++ 3 files changed, 68 insertions(+), 6 deletions(-)

1 0

[tor/master] Revise TROVE-2020-002 fix to work on older OpenSSL versions.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit be064f77b93bda370e4165e6ad6da17324835c9e Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Mar 14 13:38:53 2020 -0400 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 022a0dc09..39b7aaf0c 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; }

1 0

[tor/master] Extract key length check into a new function, and check more fields.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit 8abdb394893a1704f885278f5f5d7913cdf516c9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 10:04:38 2020 -0400 Extract key length check into a new function, and check more fields. In the openssl that I have, it should be safe to only check the size of n. But if I'm wrong, or if other openssls work differently, we should check whether any of the fields are too large. Issue spotted by Teor. --- src/lib/crypt_ops/crypto_rsa_openssl.c | 57 +++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/src/lib/crypt_ops/crypto_rsa_openssl.c b/src/lib/crypt_ops/crypto_rsa_openssl.c index 0db25f347..6f3ac6fde 100644 --- a/src/lib/crypt_ops/crypto_rsa_openssl.c +++ b/src/lib/crypt_ops/crypto_rsa_openssl.c @@ -33,6 +33,7 @@ ENABLE_GCC_WARNING(redundant-decls) #include "lib/encoding/binascii.h" #include <string.h> +#include <stdbool.h> /** Declaration for crypto_pk_t structure. */ struct crypto_pk_t @@ -564,6 +565,56 @@ crypto_pk_asn1_encode_private(const crypto_pk_t *pk, char *dest, return len; } +/** Check whether any component of a private key is too large in a way that + * seems likely to make verification too expensive. Return true if it's too + * long, and false otherwise. */ +static bool +rsa_private_key_too_long(RSA *rsa, int max_bits) +{ + const BIGNUM *n, *e, *p, *q, *d, *dmp1, *dmq1, *iqmp; +#ifdef OPENSSL_1_1_API + n = RSA_get0_n(rsa); + e = RSA_get0_e(rsa); + p = RSA_get0_p(rsa); + q = RSA_get0_q(rsa); + d = RSA_get0_d(rsa); + dmp1 = RSA_get0_dmp1(rsa); + dmq1 = RSA_get0_dmq1(rsa); + iqmp = RSA_get0_iqmp(rsa); + + if (RSA_bits(rsa) > max_bits) + return true; +#else + n = rsa->n; + e = rsa->e; + p = rsa->p; + q = rsa->q; + d = rsa->d; + dmp1 = rsa->dmp1; + dmq1 = rsa->dmq1; + iqmp = rsa->iqmp; +#endif + + if (n && BN_num_bits(n) > max_bits) + return true; + if (e && BN_num_bits(e) > max_bits) + return true; + if (p && BN_num_bits(p) > max_bits) + return true; + if (q && BN_num_bits(q) > max_bits) + return true; + if (d && BN_num_bits(d) > max_bits) + return true; + if (dmp1 && BN_num_bits(dmp1) > max_bits) + return true; + if (dmq1 && BN_num_bits(dmq1) > max_bits) + return true; + if (iqmp && BN_num_bits(iqmp) > max_bits) + return true; + + return false; +} + /** Decode an ASN.1-encoded private key from <b>str</b>; return the result on * success and NULL on failure. * @@ -584,11 +635,7 @@ 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 + if (max_bits >= 0 && rsa_private_key_too_long(rsa, max_bits)) { log_info(LD_CRYPTO, "Private key longer than expected."); RSA_free(rsa); return NULL;

1 0