From da15feb0d358fe95394aed75fae672ad8459ceee Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 27 Jan 2020 17:06:36 +0200 Subject: [PATCH] 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. --- src/feature/hs/hs_cell.c | 17 +++------- src/feature/hs/hs_ob.c | 62 ++++++++++++++++++++++++++++--------- src/feature/hs/hs_ob.h | 9 ++++-- src/feature/hs/hs_service.c | 12 +++++++ src/feature/hs/hs_service.h | 9 ++++-- src/test/test_hs_ob.c | 3 +- 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