summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/feature/dirparse/parsecommon.c9
-rw-r--r--src/feature/hs/hs_client.c6
-rw-r--r--src/feature/hs/hs_service.c6
-rw-r--r--src/lib/crypt_ops/crypto_ed25519.c2
-rw-r--r--src/lib/crypt_ops/crypto_rsa.c27
-rw-r--r--src/lib/crypt_ops/crypto_rsa.h5
-rw-r--r--src/lib/crypt_ops/crypto_rsa_nss.c14
-rw-r--r--src/lib/crypt_ops/crypto_rsa_openssl.c70
-rw-r--r--src/lib/tls/buffers_tls.c4
-rw-r--r--src/test/test_crypto.c39
-rw-r--r--src/test/testing_common.c15
-rw-r--r--src/win32/orconfig.h2
12 files changed, 181 insertions, 18 deletions
diff --git a/src/feature/dirparse/parsecommon.c b/src/feature/dirparse/parsecommon.c
index 632f5adff0..e8269f7ec7 100644
--- a/src/feature/dirparse/parsecommon.c
+++ b/src/feature/dirparse/parsecommon.c
@@ -384,12 +384,19 @@ 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_key_from_string(tok->key, obstart, eol-obstart))
+ if (crypto_pk_read_private_key1024_from_string(tok->key,
+ obstart, eol-obstart))
RET_ERR("Couldn't parse private key.");
} else { /* If it's something else, try to base64-decode it */
int r;
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index fd2d266453..0efe9fc28e 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -1275,7 +1275,7 @@ hs_client_decode_descriptor(const char *desc_str,
uint8_t subcredential[DIGEST256_LEN];
ed25519_public_key_t blinded_pubkey;
hs_client_service_authorization_t *client_auth = NULL;
- curve25519_secret_key_t *client_auht_sk = NULL;
+ curve25519_secret_key_t *client_auth_sk = NULL;
tor_assert(desc_str);
tor_assert(service_identity_pk);
@@ -1284,7 +1284,7 @@ hs_client_decode_descriptor(const char *desc_str,
/* Check if we have a client authorization for this service in the map. */
client_auth = find_client_auth(service_identity_pk);
if (client_auth) {
- client_auht_sk = &client_auth->enc_seckey;
+ client_auth_sk = &client_auth->enc_seckey;
}
/* Create subcredential for this HS so that we can decrypt */
@@ -1297,7 +1297,7 @@ hs_client_decode_descriptor(const char *desc_str,
/* Parse descriptor */
ret = hs_desc_decode_descriptor(desc_str, subcredential,
- client_auht_sk, desc);
+ client_auth_sk, desc);
memwipe(subcredential, 0, sizeof(subcredential));
if (ret < 0) {
goto err;
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 7e150599fc..6d32cae86c 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -3578,6 +3578,12 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports,
goto err;
}
+ if (ed25519_validate_pubkey(&service->keys.identity_pk) < 0) {
+ log_warn(LD_CONFIG, "Bad ed25519 private key was provided");
+ ret = RSAE_BADPRIVKEY;
+ goto err;
+ }
+
/* Make sure we have at least one port. */
if (smartlist_len(service->config.ports) == 0) {
log_warn(LD_CONFIG, "At least one VIRTPORT/TARGET must be specified "
diff --git a/src/lib/crypt_ops/crypto_ed25519.c b/src/lib/crypt_ops/crypto_ed25519.c
index 400f963898..0a442bb739 100644
--- a/src/lib/crypt_ops/crypto_ed25519.c
+++ b/src/lib/crypt_ops/crypto_ed25519.c
@@ -795,7 +795,7 @@ ed25519_point_is_identity_element(const uint8_t *point)
int
ed25519_validate_pubkey(const ed25519_public_key_t *pubkey)
{
- uint8_t result[32] = {9};
+ uint8_t result[32] = {0};
/* First check that we were not given the identity element */
if (ed25519_point_is_identity_element(pubkey->pubkey)) {
diff --git a/src/lib/crypt_ops/crypto_rsa.c b/src/lib/crypt_ops/crypto_rsa.c
index c9189b0dfc..8fd8a8aa7b 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 <b>len</b>-byte string <b>src</b>
@@ -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 c1ea767f85..6d9cc8d30e 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 ad2ad38b66..7abf6716f0 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 <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);
@@ -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..17eae24cc2 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,11 +565,71 @@ 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
+
+#if OPENSSL_VERSION_NUMBER >= OPENSSL_V_SERIES(1,1,1)
+ 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);
+#else
+ /* The accessors above did not exist in openssl 1.1.0. */
+ p = q = dmp1 = dmq1 = iqmp = NULL;
+ RSA_get0_key(rsa, &n, &e, &d);
+#endif
+
+ 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.
+ *
+ * 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)
{
RSA *rsa;
unsigned char *buf;
@@ -578,7 +639,12 @@ 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_private_key_too_long(rsa, max_bits)) {
+ 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);
diff --git a/src/lib/tls/buffers_tls.c b/src/lib/tls/buffers_tls.c
index e92cb9163f..b570216df0 100644
--- a/src/lib/tls/buffers_tls.c
+++ b/src/lib/tls/buffers_tls.c
@@ -146,10 +146,10 @@ buf_flush_to_tls(buf_t *buf, tor_tls_t *tls, size_t flushlen,
size_t flushed = 0;
ssize_t sz;
tor_assert(buf_flushlen);
- if (BUG(*buf_flushlen > buf->datalen)) {
+ IF_BUG_ONCE(*buf_flushlen > buf->datalen) {
*buf_flushlen = buf->datalen;
}
- if (BUG(flushlen > *buf_flushlen)) {
+ IF_BUG_ONCE(flushlen > *buf_flushlen) {
flushlen = *buf_flushlen;
}
sz = (ssize_t) flushlen;
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index fa79f4cc47..5af0cce130 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -1492,6 +1492,44 @@ 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 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);
+
+ done:
+ crypto_pk_free(pk1);
+ crypto_pk_free(pk2);
+}
+
+static void
test_crypto_pk_invalid_private_key(void *arg)
{
(void)arg;
@@ -3163,6 +3201,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),
diff --git a/src/test/testing_common.c b/src/test/testing_common.c
index 62d40a42fa..2c9c4538b9 100644
--- a/src/test/testing_common.c
+++ b/src/test/testing_common.c
@@ -348,6 +348,21 @@ main(int c, const char **v)
atexit(remove_directory);
+ /* Look for TOR_SKIP_TESTCASES: a space-separated list of tests to skip. */
+ const char *skip_tests = getenv("TOR_SKIP_TESTCASES");
+ if (skip_tests) {
+ smartlist_t *skip = smartlist_new();
+ smartlist_split_string(skip, skip_tests, NULL,
+ SPLIT_IGNORE_BLANK, -1);
+ int n = 0;
+ SMARTLIST_FOREACH_BEGIN(skip, char *, cp) {
+ n += tinytest_skip(testgroups, cp);
+ tor_free(cp);
+ } SMARTLIST_FOREACH_END(cp);
+ printf("Skipping %d testcases.\n", n);
+ smartlist_free(skip);
+ }
+
int have_failed = (tinytest_main(c, v, testgroups) != 0);
free_pregenerated_keys();
diff --git a/src/win32/orconfig.h b/src/win32/orconfig.h
index baad733d0f..2b21abe888 100644
--- a/src/win32/orconfig.h
+++ b/src/win32/orconfig.h
@@ -218,7 +218,7 @@
#define USING_TWOS_COMPLEMENT
/* Version number of package */
-#define VERSION "0.3.5.9-dev"
+#define VERSION "0.3.5.10-dev"
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion