Loading changes/ticket33119 0 → 100644 +8 −0 Original line number Diff line number Diff line 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. src/feature/dirparse/parsecommon.c +8 −1 Original line number Diff line number Diff line Loading @@ -403,12 +403,19 @@ get_next_token(memarea_t *area, } 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_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_asn1_decode_private(tok->object_body, tok->object_size); tok->object_size, 1024); if (! tok->key) RET_ERR("Couldn't parse private key."); } Loading src/lib/crypt_ops/crypto_rsa.c +21 −6 Original line number Diff line number Diff line Loading @@ -490,7 +490,7 @@ crypto_pk_write_private_key_to_string(crypto_pk_t *env, static int crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src, size_t len, int severity, bool private_key) bool private_key, int max_bits) { if (len == (size_t)-1) // "-1" indicates "use the length of the string." len = strlen(src); Loading @@ -510,7 +510,7 @@ crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src, } crypto_pk_t *pk = private_key ? crypto_pk_asn1_decode_private((const char*)buf, n) ? crypto_pk_asn1_decode_private((const char*)buf, n, max_bits) : crypto_pk_asn1_decode((const char*)buf, n); if (! pk) { log_fn(severity, LD_CRYPTO, Loading Loading @@ -539,7 +539,8 @@ int crypto_pk_read_public_key_from_string(crypto_pk_t *env, const char *src, size_t len) { return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, false); return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, false, -1); } /** Read a PEM-encoded private key from the <b>len</b>-byte string <b>src</b> Loading @@ -550,7 +551,21 @@ int crypto_pk_read_private_key_from_string(crypto_pk_t *env, const char *src, ssize_t len) { return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true); return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true, -1); } /** * As crypto_pk_read_private_key_from_string(), but reject any key * with a modulus longer than 1024 bits before doing any expensive * validation on it. */ int crypto_pk_read_private_key1024_from_string(crypto_pk_t *env, const char *src, ssize_t len) { return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true, 1024); } /** If a file is longer than this, we won't try to decode its private key */ Loading Loading @@ -578,7 +593,7 @@ crypto_pk_read_private_key_from_filename(crypto_pk_t *env, } int rv = crypto_pk_read_from_string_generic(env, buf, (ssize_t)st.st_size, LOG_WARN, true); LOG_WARN, true, -1); if (rv < 0) { log_warn(LD_CRYPTO, "Unable to decode private key from file %s", escaped(keyfile)); Loading Loading @@ -662,7 +677,7 @@ crypto_pk_base64_decode_private(const char *str, size_t len) goto out; } pk = crypto_pk_asn1_decode_private(der, der_len); pk = crypto_pk_asn1_decode_private(der, der_len, -1); out: memwipe(der, 0, len+1); Loading src/lib/crypt_ops/crypto_rsa.h +4 −1 Original line number Diff line number Diff line Loading @@ -61,6 +61,8 @@ int crypto_pk_read_public_key_from_string(crypto_pk_t *env, const char *src, size_t len); int crypto_pk_read_private_key_from_string(crypto_pk_t *env, const char *s, ssize_t len); int crypto_pk_read_private_key1024_from_string(crypto_pk_t *env, const char *src, ssize_t len); int crypto_pk_write_private_key_to_filename(crypto_pk_t *env, const char *fname); Loading Loading @@ -95,7 +97,8 @@ int crypto_pk_asn1_encode(const crypto_pk_t *pk, char *dest, size_t dest_len); crypto_pk_t *crypto_pk_asn1_decode(const char *str, size_t len); int crypto_pk_asn1_encode_private(const crypto_pk_t *pk, char *dest, size_t dest_len); crypto_pk_t *crypto_pk_asn1_decode_private(const char *str, size_t len); crypto_pk_t *crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits); int crypto_pk_get_fingerprint(crypto_pk_t *pk, char *fp_out,int add_space); int crypto_pk_get_hashed_fingerprint(crypto_pk_t *pk, char *fp_out); void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in); Loading src/lib/crypt_ops/crypto_rsa_nss.c +13 −1 Original line number Diff line number Diff line Loading @@ -679,9 +679,12 @@ crypto_pk_asn1_encode_private(const crypto_pk_t *pk, /** Given a buffer containing the DER representation of the * private key <b>str</b>, decode and return the result on success, or NULL * on failure. * * If <b>max_bits</b> is nonnegative, reject any key longer than max_bits * without performing any expensive validation on it. */ crypto_pk_t * crypto_pk_asn1_decode_private(const char *str, size_t len) crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits) { tor_assert(str); tor_assert(len < INT_MAX); Loading Loading @@ -731,6 +734,15 @@ crypto_pk_asn1_decode_private(const char *str, size_t len) output = NULL; } if (output) { const int bits = SECKEY_PublicKeyStrengthInBits(output->pubkey); if (max_bits >= 0 && bits > max_bits) { log_info(LD_CRYPTO, "Private key longer than expected."); crypto_pk_free(output); output = NULL; } } if (slot) PK11_FreeSlot(slot); Loading Loading
changes/ticket33119 0 → 100644 +8 −0 Original line number Diff line number Diff line 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.
src/feature/dirparse/parsecommon.c +8 −1 Original line number Diff line number Diff line Loading @@ -403,12 +403,19 @@ get_next_token(memarea_t *area, } 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_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_asn1_decode_private(tok->object_body, tok->object_size); tok->object_size, 1024); if (! tok->key) RET_ERR("Couldn't parse private key."); } Loading
src/lib/crypt_ops/crypto_rsa.c +21 −6 Original line number Diff line number Diff line Loading @@ -490,7 +490,7 @@ crypto_pk_write_private_key_to_string(crypto_pk_t *env, static int crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src, size_t len, int severity, bool private_key) bool private_key, int max_bits) { if (len == (size_t)-1) // "-1" indicates "use the length of the string." len = strlen(src); Loading @@ -510,7 +510,7 @@ crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src, } crypto_pk_t *pk = private_key ? crypto_pk_asn1_decode_private((const char*)buf, n) ? crypto_pk_asn1_decode_private((const char*)buf, n, max_bits) : crypto_pk_asn1_decode((const char*)buf, n); if (! pk) { log_fn(severity, LD_CRYPTO, Loading Loading @@ -539,7 +539,8 @@ int crypto_pk_read_public_key_from_string(crypto_pk_t *env, const char *src, size_t len) { return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, false); return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, false, -1); } /** Read a PEM-encoded private key from the <b>len</b>-byte string <b>src</b> Loading @@ -550,7 +551,21 @@ int crypto_pk_read_private_key_from_string(crypto_pk_t *env, const char *src, ssize_t len) { return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true); return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true, -1); } /** * As crypto_pk_read_private_key_from_string(), but reject any key * with a modulus longer than 1024 bits before doing any expensive * validation on it. */ int crypto_pk_read_private_key1024_from_string(crypto_pk_t *env, const char *src, ssize_t len) { return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true, 1024); } /** If a file is longer than this, we won't try to decode its private key */ Loading Loading @@ -578,7 +593,7 @@ crypto_pk_read_private_key_from_filename(crypto_pk_t *env, } int rv = crypto_pk_read_from_string_generic(env, buf, (ssize_t)st.st_size, LOG_WARN, true); LOG_WARN, true, -1); if (rv < 0) { log_warn(LD_CRYPTO, "Unable to decode private key from file %s", escaped(keyfile)); Loading Loading @@ -662,7 +677,7 @@ crypto_pk_base64_decode_private(const char *str, size_t len) goto out; } pk = crypto_pk_asn1_decode_private(der, der_len); pk = crypto_pk_asn1_decode_private(der, der_len, -1); out: memwipe(der, 0, len+1); Loading
src/lib/crypt_ops/crypto_rsa.h +4 −1 Original line number Diff line number Diff line Loading @@ -61,6 +61,8 @@ int crypto_pk_read_public_key_from_string(crypto_pk_t *env, const char *src, size_t len); int crypto_pk_read_private_key_from_string(crypto_pk_t *env, const char *s, ssize_t len); int crypto_pk_read_private_key1024_from_string(crypto_pk_t *env, const char *src, ssize_t len); int crypto_pk_write_private_key_to_filename(crypto_pk_t *env, const char *fname); Loading Loading @@ -95,7 +97,8 @@ int crypto_pk_asn1_encode(const crypto_pk_t *pk, char *dest, size_t dest_len); crypto_pk_t *crypto_pk_asn1_decode(const char *str, size_t len); int crypto_pk_asn1_encode_private(const crypto_pk_t *pk, char *dest, size_t dest_len); crypto_pk_t *crypto_pk_asn1_decode_private(const char *str, size_t len); crypto_pk_t *crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits); int crypto_pk_get_fingerprint(crypto_pk_t *pk, char *fp_out,int add_space); int crypto_pk_get_hashed_fingerprint(crypto_pk_t *pk, char *fp_out); void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in); Loading
src/lib/crypt_ops/crypto_rsa_nss.c +13 −1 Original line number Diff line number Diff line Loading @@ -679,9 +679,12 @@ crypto_pk_asn1_encode_private(const crypto_pk_t *pk, /** Given a buffer containing the DER representation of the * private key <b>str</b>, decode and return the result on success, or NULL * on failure. * * If <b>max_bits</b> is nonnegative, reject any key longer than max_bits * without performing any expensive validation on it. */ crypto_pk_t * crypto_pk_asn1_decode_private(const char *str, size_t len) crypto_pk_asn1_decode_private(const char *str, size_t len, int max_bits) { tor_assert(str); tor_assert(len < INT_MAX); Loading Loading @@ -731,6 +734,15 @@ crypto_pk_asn1_decode_private(const char *str, size_t len) output = NULL; } if (output) { const int bits = SECKEY_PublicKeyStrengthInBits(output->pubkey); if (max_bits >= 0 && bits > max_bits) { log_info(LD_CRYPTO, "Private key longer than expected."); crypto_pk_free(output); output = NULL; } } if (slot) PK11_FreeSlot(slot); Loading