[tor-commits] [tor/master] Unify INTRO2 handling codepaths in OB and normal cases.

nickm at torproject.org nickm at torproject.org
Mon Feb 24 12:48:35 UTC 2020


commit c731988cb2ba2164d7557a95e3410c2e12f85bb8
Author: George Kadianakis <desnacked at riseup.net>
Date:   Wed Jan 22 13:57:56 2020 +0200

    Unify INTRO2 handling codepaths in OB and normal cases.
    
    Now we use the exact same INTRO2 decrypt logic regardless of whether the
    service is an OB instance or not.
    
    The new get_subcredential_for_handling_intro2_cell() function is responsible
    for loading the right subcredentials in either case.
---
 src/feature/hs/hs_cell.c    | 67 +++++----------------------------------------
 src/feature/hs/hs_circuit.c | 46 ++++++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 64 deletions(-)

diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c
index 4a9044c98..fce6fe022 100644
--- a/src/feature/hs/hs_cell.c
+++ b/src/feature/hs/hs_cell.c
@@ -821,44 +821,6 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data,
   return intro_keys_result;
 }
 
-/** Return the newly allocated intro keys using the given service
- * configuration and INTRODUCE2 data for the cell encrypted section.
- *
- * Every onion balance configured master key will be tried. If NULL is
- * returned, no hit was found for the onion balance keys. */
-static hs_ntor_intro_cell_keys_t *
-get_intro2_keys_as_ob(const hs_service_t *service,
-                      const hs_cell_introduce2_data_t *data,
-                      const uint8_t *encrypted_section,
-                      size_t encrypted_section_len)
-{
-  hs_ntor_intro_cell_keys_t *intro_keys = NULL;
-
-  if (!service->ob_subcreds) {
-    /* We are _not_ an OB instance since no configured master onion key(s)
-     * were found and thus no subcredentials were built. */
-    goto end;
-  }
-
-  /* Copy current data into a new INTRO2 cell data. We will then change the
-   * subcredential in order to validate. */
-  hs_cell_introduce2_data_t new_data = *data;
-  new_data.n_subcredentials = (int) service->n_ob_subcreds;
-  new_data.subcredentials = service->ob_subcreds;
-
-  intro_keys = get_introduce2_keys_and_verify_mac(&new_data,
-                                                  encrypted_section,
-                                                  encrypted_section_len);
-  memwipe(&new_data, 0, sizeof(new_data));
-  if (intro_keys) {
-    /* It validates. We have a hit as an onion balance instance. */
-    goto end;
-  }
-
- end:
-  return intro_keys;
-}
-
 /** Parse the INTRODUCE2 cell using data which contains everything we need to
  * do so and contains the destination buffers of information we extract and
  * compute from the cell. Return 0 on success else a negative value. The
@@ -920,29 +882,14 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data,
   memcpy(&data->client_pk.public_key, encrypted_section,
          CURVE25519_PUBKEY_LEN);
 
-  /* If we are configured as an Onion Balance instance, we need to try
-   * validation with the configured master public keys given in the config
-   * file. This is because the master identity key and the blinded key is put
-   * in the INTRODUCE2 cell by the client thus it will never validate with
-   * this instance default public key. */
-  if (hs_ob_service_is_instance(service)) {
-    intro_keys = get_intro2_keys_as_ob(service, data,
-                                       encrypted_section,
-                                       encrypted_section_len);
-  }
+  /* Get the right INTRODUCE2 ntor keys and verify the cell MAC */
+  intro_keys = get_introduce2_keys_and_verify_mac(data, encrypted_section,
+                                                  encrypted_section_len);
   if (!intro_keys) {
-    /* We are not an onion balance instance or no keys matched, fallback to
-     * our default values. */
-
-    /* Get the right INTRODUCE2 ntor keys and verify the cell MAC */
-    intro_keys = get_introduce2_keys_and_verify_mac(data, encrypted_section,
-                                                    encrypted_section_len);
-    if (!intro_keys) {
-      log_info(LD_REND, "Could not get valid INTRO2 keys on circuit %u "
-                        "for service %s", TO_CIRCUIT(circ)->n_circ_id,
-               safe_str_client(service->onion_address));
-      goto done;
-    }
+    log_warn(LD_REND, "Could not get valid INTRO2 keys on circuit %u "
+             "for service %s", TO_CIRCUIT(circ)->n_circ_id,
+             safe_str_client(service->onion_address));
+    goto done;
   }
 
   {
diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index 48a92962f..febeec473 100644
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -19,6 +19,7 @@
 #include "feature/client/circpathbias.h"
 #include "feature/hs/hs_cell.h"
 #include "feature/hs/hs_circuit.h"
+#include "feature/hs/hs_ob.h"
 #include "feature/hs/hs_circuitmap.h"
 #include "feature/hs/hs_client.h"
 #include "feature/hs/hs_ident.h"
@@ -958,6 +959,42 @@ hs_circ_handle_intro_established(const hs_service_t *service,
   return ret;
 }
 
+/**
+ *  Go into <b>data</b> and add the right subcredential to be able to handle
+ *  this incoming cell.
+ *
+ *  <b>desc_subcred</b> is the subcredential of the descriptor that corresponds
+ *  to the intro point that received this intro request. This subcredential
+ *  should be used if we are not an onionbalance instance.
+ *
+ *  Return 0 if everything went well, or -1 in case of internal error.
+ */
+static int
+get_subcredential_for_handling_intro2_cell(const hs_service_t *service,
+                                        hs_cell_introduce2_data_t *data,
+                                        const hs_subcredential_t *desc_subcred)
+{
+  /* Handle the simple case first: We are not an onionbalance instance and we
+   * should just use the regular descriptor subcredential */
+  if (!hs_ob_service_is_instance(service)) {
+    data->n_subcredentials = 1;
+    data->subcredentials = desc_subcred;
+    return 0;
+  }
+
+  /* This should not happen since we should have made onionbalance
+   * subcredentials when we created our descriptors. */
+  if (BUG(!service->ob_subcreds)) {
+    return -1;
+  }
+
+  /* We are an onionbalance instance: */
+  data->n_subcredentials = (int) service->n_ob_subcreds;
+  data->subcredentials = service->ob_subcreds;
+
+  return 0;
+}
+
 /** We just received an INTRODUCE2 cell on the established introduction circuit
  * circ.  Handle the INTRODUCE2 payload of size payload_len for the given
  * circuit and service. This cell is associated with the intro point object ip
@@ -983,15 +1020,16 @@ hs_circ_handle_introduce2(const hs_service_t *service,
    * parsed, decrypted and key material computed correctly. */
   data.auth_pk = &ip->auth_key_kp.pubkey;
   data.enc_kp = &ip->enc_key_kp;
-  // XXXX We should replace these elements with something precomputed for
-  // XXXX the onionbalance case.
-  data.n_subcredentials = 1;
-  data.subcredentials = subcredential;
   data.payload = payload;
   data.payload_len = payload_len;
   data.link_specifiers = smartlist_new();
   data.replay_cache = ip->replay_cache;
 
+  if (get_subcredential_for_handling_intro2_cell(service,
+                                                 &data, subcredential)) {
+    goto done;
+  }
+
   if (hs_cell_parse_introduce2(&data, circ, service) < 0) {
     goto done;
   }





More information about the tor-commits mailing list