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.

diff --git a/changes/bug19418 b/changes/bug19418
new file mode 100644 (file)
index 0000000..fb5f6ad
--- /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 d61cc2e58ae091d730c765a68edff20e71eed3c1..bd41e0b7f10903cd7f13fa3479c671172e034a2a 100644 (file)
--- 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 dadfdc4380acbb784042115223d1c70de4c1e544..49d48d72e097f1f42db9d0a86b7e0eb54fab6bf3 100644 (file)
--- 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 e3c80b5f145bbfa401b29b65b222ad801eb15264..82196ef2c62c59eca62e9b80a34ac81c9d66f609 100644 (file)
--- 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 a93bc94a9cf15e2d05fb00b2fced0fc892616b61..e54818c543a4a68526053079a01b703e5a0e2edc 100644 (file)
--- 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 d9d39b1f1975db945a3a45f9a13c56841715a263..c01ce8f1de04344f99bb8e89deae0896004e3c50 100644 (file)
--- 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 4d04da02aa821a0505dc5c0189cc6a6f4d48a35d..d4441b63ce81ae0083656d4e9b4f8c577e399ba7 100644 (file)
--- 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 6d3a32a60c977c589ed5f2d0b864e7fdeff7ec15..f7528c22d0e4ce8b7bcafeed9866da741f05c475 100644 (file)
--- 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 ca32228fc7815e5d2b0a104449c799d409e510ef..5a41d6261897e98dcc143b2af06df206d03e4a55 100644 (file)
--- 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 521e237be2966778114859a7a7b4e5a92f6b1dd4..bfd02704343fb4106ffae1e4b41f2b4d52aa72c6 100644 (file)
--- 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)) {