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 44db3aec5..1c47cf988 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -676,12 +676,7 @@ tor_x509_cert_new,(X509 *x509_cert)) 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); @@ -696,13 +691,25 @@ tor_x509_cert_new,(X509 *x509_cert)) 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 cert. */ diff --git a/src/or/connection_or.c b/src/or/connection_or.c index 753148291..59a9bdffd 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -1549,7 +1549,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 f54109fc9..8c48a6f47 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 9bc2d6289..9e2daf038 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -265,6 +265,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 e1236bdd0..d18356e67 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -477,7 +477,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 f3b78c466..ede887945 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2812,7 +2812,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 2187a76b4..249140114 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -257,7 +257,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 @@ -956,8 +960,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 71889d272..fd4c6ce0d 100644 --- a/src/or/routerkeys.c +++ b/src/or/routerkeys.c @@ -1236,7 +1236,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 0f6113ccf..22521a306 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -5293,7 +5293,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)) {