Refresh OB keys when we build a new descriptor.

We now assign OB subcredentials to the service instead of computing them on the
spot. See hs_ob_refresh_keys() for more details.

6 files changed, 81 insertions, 31 deletions

diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c
index c82ffd647b..4a9044c98f 100644
--- a/src/feature/hs/hs_cell.c
+++ b/src/feature/hs/hs_cell.c
@@ -827,17 +827,14 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data,
  * 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_config_t *config,
+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_subcredential_t *ob_subcreds = NULL;
-  size_t ob_num_subcreds;
   hs_ntor_intro_cell_keys_t *intro_keys = NULL;
 
-  ob_num_subcreds = hs_ob_get_subcredentials(config, &ob_subcreds);
-  if (!ob_num_subcreds) {
+  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;
@@ -846,11 +843,8 @@ get_intro2_keys_as_ob(const hs_service_config_t *config,
   /* 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;
-  /* XXXX This list should have been the descriptor's subcredentials all
-   * XXXX along.
-   */
-  new_data.n_subcredentials = (int)ob_num_subcreds;
-  new_data.subcredentials = ob_subcreds;
+  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,
@@ -862,7 +856,6 @@ get_intro2_keys_as_ob(const hs_service_config_t *config,
   }
 
  end:
-  tor_free(ob_subcreds);
   return intro_keys;
 }
 
@@ -933,7 +926,7 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data,
    * 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->config, data,
+    intro_keys = get_intro2_keys_as_ob(service, data,
                                        encrypted_section,
                                        encrypted_section_len);
   }
diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c
index ee54595f26..7552fbd16d 100644
--- a/src/feature/hs/hs_ob.c
+++ b/src/feature/hs/hs_ob.c
@@ -4,14 +4,15 @@
 /**
  * \file hs_ob.c
  * \brief Implement Onion Balance specific code.
- *
- * \details
- *
- * XXX:
  **/
 
 #define HS_OB_PRIVATE
 
+#include "feature/hs/hs_service.h"
+
+#include "feature/nodelist/networkstatus.h"
+#include "feature/nodelist/networkstatus_st.h"
+
 #include "lib/confmgt/confmgt.h"
 #include "lib/encoding/confline.h"
 
@@ -273,9 +274,9 @@ hs_ob_parse_config_file(hs_service_config_t *config)
  * returned and subcredentials is set to NULL.
  *
  * Otherwise, this can't fail. */
-size_t
-hs_ob_get_subcredentials(const hs_service_config_t *config,
-                         hs_subcredential_t **subcredentials)
+STATIC size_t
+compute_subcredentials(const hs_service_t *service,
+                       hs_subcredential_t **subcredentials)
 {
   unsigned int num_pkeys, idx = 0;
   hs_subcredential_t *subcreds = NULL;
@@ -286,10 +287,9 @@ hs_ob_get_subcredentials(const hs_service_config_t *config,
   tor_assert(config);
   tor_assert(subcredentials);
 
+  /* Our caller made sure that we are an OB instance */
   num_pkeys = smartlist_len(config->ob_master_pubkeys);
-  if (!num_pkeys) {
-    goto end;
-  }
+  tor_assert(num_pkeys > 0);
 
   /* Time to build all the subcredentials for each time period: the previous
    * one (-1), the current one (0) and the next one (1) for each configured
@@ -310,9 +310,7 @@ hs_ob_get_subcredentials(const hs_service_config_t *config,
    * our chance of success. */
 
   /* We use a flat array, not a smartlist_t, in order to minimize memory
-   * allocation. This function is called for _each_ INTRODUCE2 cell arriving
-   * on this instance and thus the less we allocate small chunks often,
-   * usually the healthier our overall memory will be.
+   * allocation.
    *
    * Size of array is: length of a single subcredential multiplied by the
    * number of time period we need to compute and finally multiplied by the
@@ -329,7 +327,43 @@ hs_ob_get_subcredentials(const hs_service_config_t *config,
     } SMARTLIST_FOREACH_END(pkey);
   }
 
- end:
   *subcredentials = subcreds;
   return idx;
 }
+
+/**
+ *  If we are an Onionbalance instance, refresh our keys.
+ *
+ *  If we are not an Onionbalance instance or we are not ready to do so, this
+ *  is a NOP.
+ *
+ *  This function is called everytime we build a new descriptor. That's because
+ *  we want our Onionbalance keys to always use up-to-date subcredentials both
+ *  for the instance (ourselves) and for the onionbalance frontend.
+ */
+void
+hs_ob_refresh_keys(hs_service_t *service)
+{
+  const networkstatus_t *ns;
+  hs_subcredential_t *ob_subcreds = NULL;
+  size_t num_subcreds;
+
+  tor_assert(service);
+
+  /* Don't do any of this if we are not configured as an OB instance */
+  if (!hs_ob_service_is_instance(service)) {
+    return;
+  }
+
+  /* Get a new set of subcreds */
+  num_subcreds = compute_subcredentials(service, &ob_subcreds);
+  tor_assert(num_subcreds > 0);
+
+  /* Delete old subcredentials if any */
+  if (service->ob_subcreds) {
+    tor_free(service->ob_subcreds);
+  }
+
+  service->ob_subcreds = ob_subcreds;
+  service->n_ob_subcreds = num_subcreds;
+}
diff --git a/src/feature/hs/hs_ob.h b/src/feature/hs/hs_ob.h
index 37e94808c2..d5b5504be7 100644
--- a/src/feature/hs/hs_ob.h
+++ b/src/feature/hs/hs_ob.h
@@ -16,11 +16,16 @@ bool hs_ob_service_is_instance(const hs_service_t *service);
 int hs_ob_parse_config_file(hs_service_config_t *config);
 
 struct hs_subcredential_t;
-size_t hs_ob_get_subcredentials(const hs_service_config_t *config,
-                                struct hs_subcredential_t **subcredentials);
+
+void hs_ob_free_all(void);
+
+void hs_ob_refresh_keys(hs_service_t *service);
 
 #ifdef HS_OB_PRIVATE
 
+STATIC size_t compute_subcredentials(const hs_service_t *service,
+                                   struct hs_subcredential_t **subcredentials);
+
 typedef struct ob_options_t {
   /** Magic number to identify the structure in memory. */
   uint32_t magic_;
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 87b9625f54..f0c791f21d 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -41,6 +41,7 @@
 #include "feature/hs/hs_intropoint.h"
 #include "feature/hs/hs_service.h"
 #include "feature/hs/hs_stats.h"
+#include "feature/hs/hs_ob.h"
 
 #include "feature/dircommon/dir_connection_st.h"
 #include "core/or/edge_connection_st.h"
@@ -1986,9 +1987,15 @@ build_service_descriptor(hs_service_t *service, uint64_t time_period_num,
 
   /* Assign newly built descriptor to the next slot. */
   *desc_out = desc;
+
   /* Fire a CREATED control port event. */
   hs_control_desc_event_created(service->onion_address,
                                 &desc->blinded_kp.pubkey);
+
+  /* If we are an onionbalance instance, we refresh our keys when we rotate
+   * descriptors. */
+  hs_ob_refresh_keys(service);
+
   return;
 
  err:
@@ -4048,6 +4055,11 @@ hs_service_free_(hs_service_t *service)
     replaycache_free(service->state.replay_cache_rend_cookie);
   }
 
+  /* Free onionbalance subcredentials (if any) */
+  if (service->ob_subcreds) {
+    tor_free(service->ob_subcreds);
+  }
+
   /* Wipe service keys. */
   memwipe(&service->keys.identity_sk, 0, sizeof(service->keys.identity_sk));
 
diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h
index bdc2bc3b64..94a73b2fec 100644
--- a/src/feature/hs/hs_service.h
+++ b/src/feature/hs/hs_service.h
@@ -305,8 +305,13 @@ typedef struct hs_service_t {
   /** Next descriptor. */
   hs_service_descriptor_t *desc_next;
 
-  /* XXX: Credential (client auth.) #20700. */
-
+  /* If this is an onionbalance instance, this is an array of subcredentials
+   * that should be used when decrypting an INTRO2 cell. If this is not an
+   * onionbalance instance, this is NULL.
+   * See [ONIONBALANCE] section in rend-spec-v3.txt for more details . */
+  hs_subcredential_t *ob_subcreds;
+  /* Number of OB subcredentials */
+  size_t n_ob_subcreds;
 } hs_service_t;
 
 /** For the service global hash map, we define a specific type for it which
diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c
index 848a488be2..c2d62e354a 100644
--- a/src/test/test_hs_ob.c
+++ b/src/test/test_hs_ob.c
@@ -8,6 +8,7 @@
 
 #define CONFIG_PRIVATE
 #define HS_SERVICE_PRIVATE
+#define HS_OB_PRIVATE
 
 #include "test/test.h"
 #include "test/test_helpers.h"
@@ -191,7 +192,7 @@ test_get_subcredentials(void *arg)
   smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey);
 
   hs_subcredential_t *subcreds = NULL;
-  size_t num = hs_ob_get_subcredentials(&config, &subcreds);
+  size_t num = compute_subcredentials(&config, &subcreds);
   tt_uint_op(num, OP_EQ, 3);
 
   /* Validate the subcredentials we just got. We'll build them oursevles with