[tor-commits] [tor/release-0.3.1] Make sure we always wind up checking i2d_*'s output.
nickm at torproject.org
nickm at torproject.org
Fri Aug 25 15:39:46 UTC 2017
commit 418f3d6298beb27e050618e2f59e01d6d3b2f45b
Author: Nick Mathewson <nickm at torproject.org>
Date: Wed Aug 9 09:24:16 2017 -0400
Make sure we always wind up checking i2d_*'s output.
The biggest offender here was sometimes not checking the output of
crypto_pk_get_digest.
Fixes bug 19418. Reported by Guido Vranken.
---
changes/bug19418 | 7 +++++++
src/common/tortls.c | 21 ++++++++++++++-------
src/or/connection_or.c | 4 +++-
src/or/hibernate.c | 5 ++++-
src/or/rendclient.c | 5 +++++
src/or/rendcommon.c | 5 ++++-
src/or/rendservice.c | 9 ++++++++-
src/or/router.c | 14 +++++++++++---
src/or/routerkeys.c | 4 +++-
src/or/routerparse.c | 5 ++++-
10 files changed, 63 insertions(+), 16 deletions(-)
diff --git a/changes/bug19418 b/changes/bug19418
new file mode 100644
index 000000000..fb5f6ad5d
--- /dev/null
+++ b/changes/bug19418
@@ -0,0 +1,7 @@
+ o Minor bugfixes (robustness, error handling):
+ - Improve our handling of the cases where OpenSSL encounters a
+ memory error while encoding keys and certificates. We haven't
+ observed these happening in the wild, but if they do happen,
+ we now detect and respond better. Fixes bug 19418; bugfix
+ on all versions of Tor. Reported by Guido Vranken.
+
diff --git a/src/common/tortls.c b/src/common/tortls.c
index d61cc2e58..bd41e0b7f 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -648,12 +648,7 @@ MOCK_IMPL(STATIC tor_x509_cert_t *,
length = i2d_X509(x509_cert, &buf);
cert = tor_malloc_zero(sizeof(tor_x509_cert_t));
if (length <= 0 || buf == NULL) {
- /* LCOV_EXCL_START for the same reason as the exclusion above */
- tor_free(cert);
- log_err(LD_CRYPTO, "Couldn't get length of encoded x509 certificate");
- X509_free(x509_cert);
- return NULL;
- /* LCOV_EXCL_STOP */
+ goto err;
}
cert->encoded_len = (size_t) length;
cert->encoded = tor_malloc(length);
@@ -668,13 +663,25 @@ MOCK_IMPL(STATIC tor_x509_cert_t *,
if ((pkey = X509_get_pubkey(x509_cert)) &&
(rsa = EVP_PKEY_get1_RSA(pkey))) {
crypto_pk_t *pk = crypto_new_pk_from_rsa_(rsa);
- crypto_pk_get_common_digests(pk, &cert->pkey_digests);
+ if (crypto_pk_get_common_digests(pk, &cert->pkey_digests) < 0) {
+ crypto_pk_free(pk);
+ EVP_PKEY_free(pkey);
+ goto err;
+ }
+
cert->pkey_digests_set = 1;
crypto_pk_free(pk);
EVP_PKEY_free(pkey);
}
return cert;
+ err:
+ /* LCOV_EXCL_START for the same reason as the exclusion above */
+ tor_free(cert);
+ log_err(LD_CRYPTO, "Couldn't wrap encoded X509 certificate.");
+ X509_free(x509_cert);
+ return NULL;
+ /* LCOV_EXCL_STOP */
}
/** Return a new copy of <b>cert</b>. */
diff --git a/src/or/connection_or.c b/src/or/connection_or.c
index dadfdc438..49d48d72e 100644
--- a/src/or/connection_or.c
+++ b/src/or/connection_or.c
@@ -1552,7 +1552,9 @@ connection_or_check_valid_tls_handshake(or_connection_t *conn,
}
if (identity_rcvd) {
- crypto_pk_get_digest(identity_rcvd, digest_rcvd_out);
+ if (crypto_pk_get_digest(identity_rcvd, digest_rcvd_out) < 0) {
+ return -1;
+ }
} else {
memset(digest_rcvd_out, 0, DIGEST_LEN);
}
diff --git a/src/or/hibernate.c b/src/or/hibernate.c
index e3c80b5f1..82196ef2c 100644
--- a/src/or/hibernate.c
+++ b/src/or/hibernate.c
@@ -587,7 +587,10 @@ accounting_set_wakeup_time(void)
char buf[ISO_TIME_LEN+1];
format_iso_time(buf, interval_start_time);
- crypto_pk_get_digest(get_server_identity_key(), digest);
+ if (crypto_pk_get_digest(get_server_identity_key(), digest) < 0) {
+ log_err(LD_BUG, "Error getting our key's digest.");
+ tor_assert(0);
+ }
d_env = crypto_digest_new();
crypto_digest_add_bytes(d_env, buf, ISO_TIME_LEN);
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index a93bc94a9..e54818c54 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -263,6 +263,11 @@ rend_client_send_introduction(origin_circuit_t *introcirc,
klen = crypto_pk_asn1_encode(extend_info->onion_key,
tmp+v3_shift+7+DIGEST_LEN+2,
sizeof(tmp)-(v3_shift+7+DIGEST_LEN+2));
+ if (klen < 0) {
+ log_warn(LD_BUG,"Internal error: can't encode public key.");
+ status = -2;
+ goto perm_err;
+ }
set_uint16(tmp+v3_shift+7+DIGEST_LEN, htons(klen));
memcpy(tmp+v3_shift+7+DIGEST_LEN+2+klen, rendcirc->rend_data->rend_cookie,
REND_COOKIE_LEN);
diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c
index d9d39b1f1..c01ce8f1d 100644
--- a/src/or/rendcommon.c
+++ b/src/or/rendcommon.c
@@ -475,7 +475,10 @@ rend_encode_v2_descriptors(smartlist_t *descs_out,
tor_assert(descriptor_cookie);
}
/* Obtain service_id from public key. */
- crypto_pk_get_digest(service_key, service_id);
+ if (crypto_pk_get_digest(service_key, service_id) < 0) {
+ log_warn(LD_BUG, "Couldn't compute service key digest.");
+ return -1;
+ }
/* Calculate current time-period. */
time_period = get_time_period(now, period, service_id);
/* Determine how many seconds the descriptor will be valid. */
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 4d04da02a..d4441b63c 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -2689,7 +2689,14 @@ rend_service_decrypt_intro(
/* Check that this cell actually matches this service key */
/* first DIGEST_LEN bytes of request is intro or service pk digest */
- crypto_pk_get_digest(key, (char *)key_digest);
+ if (crypto_pk_get_digest(key, (char *)key_digest) < 0) {
+ if (err_msg_out)
+ *err_msg_out = tor_strdup("Couldn't compute RSA digest.");
+ log_warn(LD_BUG, "Couldn't compute key digest.");
+ status = -7;
+ goto err;
+ }
+
if (tor_memneq(key_digest, intro->pk, DIGEST_LEN)) {
if (err_msg_out) {
base32_encode(service_id, REND_SERVICE_ID_LEN_BASE32 + 1,
diff --git a/src/or/router.c b/src/or/router.c
index 6d3a32a60..f7528c22d 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -212,7 +212,11 @@ set_server_identity_key(crypto_pk_t *k)
{
crypto_pk_free(server_identitykey);
server_identitykey = k;
- crypto_pk_get_digest(server_identitykey, server_identitykey_digest);
+ if (crypto_pk_get_digest(server_identitykey,
+ server_identitykey_digest) < 0) {
+ log_err(LD_BUG, "Couldn't compute our own identity key digest.");
+ tor_assert(0);
+ }
}
/** Make sure that we have set up our identity keys to match or not match as
@@ -871,8 +875,12 @@ init_keys(void)
}
cert = get_my_v3_authority_cert();
if (cert) {
- crypto_pk_get_digest(get_my_v3_authority_cert()->identity_key,
- v3_digest);
+ if (crypto_pk_get_digest(get_my_v3_authority_cert()->identity_key,
+ v3_digest) < 0) {
+ log_err(LD_BUG, "Couldn't compute my v3 authority identity key "
+ "digest.");
+ return -1;
+ }
v3_digest_set = 1;
}
}
diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c
index ca32228fc..5a41d6261 100644
--- a/src/or/routerkeys.c
+++ b/src/or/routerkeys.c
@@ -1079,7 +1079,9 @@ make_tap_onion_key_crosscert(const crypto_pk_t *onion_key,
uint8_t signed_data[DIGEST_LEN + ED25519_PUBKEY_LEN];
*len_out = 0;
- crypto_pk_get_digest(rsa_id_key, (char*)signed_data);
+ if (crypto_pk_get_digest(rsa_id_key, (char*)signed_data) < 0) {
+ return NULL;
+ }
memcpy(signed_data + DIGEST_LEN, master_id_key->pubkey, ED25519_PUBKEY_LEN);
int r = crypto_pk_private_sign(onion_key,
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index 521e237be..bfd027043 100644
--- a/src/or/routerparse.c
+++ b/src/or/routerparse.c
@@ -5967,7 +5967,10 @@ rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
"v2 rendezvous service descriptor") < 0)
goto err;
/* Verify that descriptor ID belongs to public key and secret ID part. */
- crypto_pk_get_digest(result->pk, public_key_hash);
+ if (crypto_pk_get_digest(result->pk, public_key_hash) < 0) {
+ log_warn(LD_REND, "Unable to compute rend descriptor public key digest");
+ goto err;
+ }
rend_get_descriptor_id_bytes(test_desc_id, public_key_hash,
secret_id_part);
if (tor_memneq(desc_id_out, test_desc_id, DIGEST_LEN)) {
More information about the tor-commits
mailing list