diff --git a/changes/ticket33119 b/changes/ticket33119
new file mode 100644
index 0000000000..11c20bc7a2
--- /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.
diff --git a/src/feature/dirparse/parsecommon.c b/src/feature/dirparse/parsecommon.c
index 036a51689c..6c2c4b06b7 100644
--- a/src/feature/dirparse/parsecommon.c
+++ b/src/feature/dirparse/parsecommon.c
@@ -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.");
}
diff --git a/src/lib/crypt_ops/crypto_rsa.c b/src/lib/crypt_ops/crypto_rsa.c
index c39d2e18d1..1289a82e62 100644
--- a/src/lib/crypt_ops/crypto_rsa.c
+++ b/src/lib/crypt_ops/crypto_rsa.c
@@ -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);
@@ -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,
@@ -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 len-byte string src
@@ -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 */
@@ -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));
@@ -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);
diff --git a/src/lib/crypt_ops/crypto_rsa.h b/src/lib/crypt_ops/crypto_rsa.h
index e9bfec2f85..01e6d10906 100644
--- a/src/lib/crypt_ops/crypto_rsa.h
+++ b/src/lib/crypt_ops/crypto_rsa.h
@@ -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);
@@ -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);
diff --git a/src/lib/crypt_ops/crypto_rsa_nss.c b/src/lib/crypt_ops/crypto_rsa_nss.c
index 612b7a0e64..c716126f95 100644
--- a/src/lib/crypt_ops/crypto_rsa_nss.c
+++ b/src/lib/crypt_ops/crypto_rsa_nss.c
@@ -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 str, decode and return the result on success, or NULL
* on failure.
+ *
+ * If max_bits 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);
@@ -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);
diff --git a/src/lib/crypt_ops/crypto_rsa_openssl.c b/src/lib/crypt_ops/crypto_rsa_openssl.c
index fbdc76ccd6..022a0dc093 100644
--- a/src/lib/crypt_ops/crypto_rsa_openssl.c
+++ b/src/lib/crypt_ops/crypto_rsa_openssl.c
@@ -566,9 +566,12 @@ crypto_pk_asn1_encode_private(const crypto_pk_t *pk, char *dest,
/** Decode an ASN.1-encoded private key from str; return the result on
* success and NULL on failure.
+ *
+ * If max_bits 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)
{
RSA *rsa;
unsigned char *buf;
@@ -578,7 +581,11 @@ crypto_pk_asn1_decode_private(const char *str, size_t len)
rsa = d2i_RSAPrivateKey(NULL, &cp, len);
tor_free(buf);
if (!rsa) {
- crypto_openssl_log_errors(LOG_WARN,"decoding public key");
+ crypto_openssl_log_errors(LOG_WARN,"decoding private key");
+ return NULL;
+ }
+ if (max_bits >= 0 && RSA_bits(rsa) > max_bits) {
+ log_info(LD_CRYPTO, "Private key longer than expected.");
return NULL;
}
crypto_pk_t *result = crypto_new_pk_from_openssl_rsa_(rsa);