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

[tor/maint-0.4.1] Use >= consistently with max_bits.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit f958b537abc1285dd627c03f091dc94a5d17995a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 10:09:58 2020 -0400 Use >= consistently with max_bits. --- src/lib/crypt_ops/crypto_rsa_nss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/crypt_ops/crypto_rsa_nss.c b/src/lib/crypt_ops/crypto_rsa_nss.c index 4cf699067..7abf6716f 100644 --- a/src/lib/crypt_ops/crypto_rsa_nss.c +++ b/src/lib/crypt_ops/crypto_rsa_nss.c @@ -736,7 +736,7 @@ crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits) if (output) { const int bits = SECKEY_PublicKeyStrengthInBits(output->pubkey); - if (max_bits > 0 && bits > max_bits) { + if (max_bits >= 0 && bits > max_bits) { log_info(LD_CRYPTO, "Private key longer than expected."); crypto_pk_free(output); output = NULL;

1 0

[tor/maint-0.4.2] 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/maint-0.4.3] 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] 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

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

18 Mar '20

commit dd6e2277e0d336f3d519f88d792b832d04e2c323 Merge: 1a375c3b1 29c9675bd Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Mar 14 14:20:51 2020 -0400 Merge branch 'trove_2020_002_035' into trove_2020_002_041 src/lib/crypt_ops/crypto_rsa_openssl.c | 5 +++++ src/test/test_crypto.c | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+)

1 0

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

18 Mar '20

commit fe3d8ec38e3c4c2a992280c2847cdd8e05f81d36 Merge: 089e57d22 f958b537a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 15:21:48 2020 -0400 Merge branch 'trove_2020_002_035' into maint-0.3.5 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 | 63 ++++++++++++++++++++++++++++++++-- src/test/test_crypto.c | 39 +++++++++++++++++++++ 7 files changed, 154 insertions(+), 11 deletions(-)

1 0

[tor/maint-0.4.3] Use >= consistently with max_bits.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit f958b537abc1285dd627c03f091dc94a5d17995a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 10:09:58 2020 -0400 Use >= consistently with max_bits. --- src/lib/crypt_ops/crypto_rsa_nss.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/crypt_ops/crypto_rsa_nss.c b/src/lib/crypt_ops/crypto_rsa_nss.c index 4cf699067..7abf6716f 100644 --- a/src/lib/crypt_ops/crypto_rsa_nss.c +++ b/src/lib/crypt_ops/crypto_rsa_nss.c @@ -736,7 +736,7 @@ crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits) if (output) { const int bits = SECKEY_PublicKeyStrengthInBits(output->pubkey); - if (max_bits > 0 && bits > max_bits) { + if (max_bits >= 0 && bits > max_bits) { log_info(LD_CRYPTO, "Private key longer than expected."); crypto_pk_free(output); output = NULL;

1 0

[tor/maint-0.4.3] Add off-by-one checks for key length.
by nickm＠torproject.org 18 Mar '20

18 Mar '20

commit 2328c79a5fbc2f1995390dd08002244bc952246d Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 10:07:54 2020 -0400 Add off-by-one checks for key length. --- src/test/test_crypto.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 2373e5bf8..5af0cce13 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -1505,6 +1505,21 @@ test_crypto_pk_bad_size(void *arg) pk2 = crypto_pk_asn1_decode_private(buf, n, 1020); tt_assert(! pk2); + /* Set the max bit count one bit smaller: we should refuse to decode the + key.*/ + pk2 = crypto_pk_asn1_decode_private(buf, n, 1023); + tt_assert(! pk2); + + /* Correct size: should work. */ + pk2 = crypto_pk_asn1_decode_private(buf, n, 1024); + tt_assert(pk2); + crypto_pk_free(pk2); + + /* One bit larger: should work. */ + pk2 = crypto_pk_asn1_decode_private(buf, n, 1025); + tt_assert(pk2); + crypto_pk_free(pk2); + /* Set the max bit count larger: it should decode fine. */ pk2 = crypto_pk_asn1_decode_private(buf, n, 2048); tt_assert(pk2);

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 57b1d0848e9b42670d6f33e37a2095740726e2b1 Merge: 3c8a4b8fb fe3d8ec38 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 15:22:36 2020 -0400 Merge branch 'maint-0.3.5' into maint-0.4.1

1 0

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

18 Mar '20

commit 3c8a4b8fbdb42b2fa98ddec8b6d7ed3202201eb1 Merge: cec647ff3 916378103 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 15:22:02 2020 -0400 Merge branch 'trove_2020_002_041' into maint-0.4.1 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 | 63 ++++++++++++++++++++++++++++++++-- src/test/test_crypto.c | 39 +++++++++++++++++++++ 7 files changed, 154 insertions(+), 11 deletions(-)

1 0