From 16a201e7039577070201828750e0b092f0e8eb7f Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 13 Jan 2020 12:15:14 -0500 Subject: [PATCH 01/22] hs-v3: Implement hs_parse_address_no_log() The hs_parse_address() can not be used without an options_t object existing since on error it uses the escaped_safe_str() that looks at the options. This new function won't log and returns an error message in case of failure that can then be used to log. Signed-off-by: David Goulet --- src/feature/hs/hs_common.c | 41 ++++++++++++++++++++++++++++---------- src/feature/hs/hs_common.h | 4 ++++ src/test/test_hs_common.c | 6 +++--- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index f8b031cc26..47594b582e 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -909,30 +909,35 @@ hs_set_conn_addr_port(const smartlist_t *ports, edge_connection_t *conn) * case the caller would want only one field. checksum_out MUST at least be 2 * bytes long. * - * Return 0 if parsing went well; return -1 in case of error. */ + * Return 0 if parsing went well; return -1 in case of error and if errmsg is + * non NULL, a human readable string message is set. */ int -hs_parse_address(const char *address, ed25519_public_key_t *key_out, - uint8_t *checksum_out, uint8_t *version_out) +hs_parse_address_no_log(const char *address, ed25519_public_key_t *key_out, + uint8_t *checksum_out, uint8_t *version_out, + const char **errmsg) { char decoded[HS_SERVICE_ADDR_LEN]; tor_assert(address); + if (errmsg) { + *errmsg = NULL; + } + /* Obvious length check. */ if (strlen(address) != HS_SERVICE_ADDR_LEN_BASE32) { - log_warn(LD_REND, "Service address %s has an invalid length. " - "Expected %lu but got %lu.", - escaped_safe_str(address), - (unsigned long) HS_SERVICE_ADDR_LEN_BASE32, - (unsigned long) strlen(address)); + if (errmsg) { + *errmsg = "Invalid length"; + } goto invalid; } /* Decode address so we can extract needed fields. */ if (base32_decode(decoded, sizeof(decoded), address, strlen(address)) != sizeof(decoded)) { - log_warn(LD_REND, "Service address %s can't be decoded.", - escaped_safe_str(address)); + if (errmsg) { + *errmsg = "Unable to base32 decode"; + } goto invalid; } @@ -944,6 +949,22 @@ hs_parse_address(const char *address, ed25519_public_key_t *key_out, return -1; } +/** Same has hs_parse_address_no_log() but emits a log warning on parsing + * failure. */ +int +hs_parse_address(const char *address, ed25519_public_key_t *key_out, + uint8_t *checksum_out, uint8_t *version_out) +{ + const char *errmsg = NULL; + int ret = hs_parse_address_no_log(address, key_out, checksum_out, + version_out, &errmsg); + if (ret < 0) { + log_warn(LD_REND, "Service address %s failed to be parsed: %s", + escaped_safe_str(address), errmsg); + } + return ret; +} + /** Validate a given onion address. The length, the base32 decoding, and * checksum are validated. Return 1 if valid else 0. */ int diff --git a/src/feature/hs/hs_common.h b/src/feature/hs/hs_common.h index 8f743d4d37..2bcf0f67c4 100644 --- a/src/feature/hs/hs_common.h +++ b/src/feature/hs/hs_common.h @@ -179,6 +179,10 @@ void hs_build_address(const struct ed25519_public_key_t *key, uint8_t version, int hs_address_is_valid(const char *address); int hs_parse_address(const char *address, struct ed25519_public_key_t *key_out, uint8_t *checksum_out, uint8_t *version_out); +int hs_parse_address_no_log(const char *address, + struct ed25519_public_key_t *key_out, + uint8_t *checksum_out, uint8_t *version_out, + const char **errmsg); void hs_build_blinded_pubkey(const struct ed25519_public_key_t *pubkey, const uint8_t *secret, size_t secret_len, diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index 61306778d4..20f88b637b 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -53,14 +53,14 @@ test_validate_address(void *arg) setup_full_capture_of_logs(LOG_WARN); ret = hs_address_is_valid("blah"); tt_int_op(ret, OP_EQ, 0); - expect_log_msg_containing("has an invalid length"); + expect_log_msg_containing("Invalid length"); teardown_capture_of_logs(); setup_full_capture_of_logs(LOG_WARN); ret = hs_address_is_valid( "p3xnclpu4mu22dwaurjtsybyqk4xfjmcfz6z62yl24uwmhjatiwnlnadb"); tt_int_op(ret, OP_EQ, 0); - expect_log_msg_containing("has an invalid length"); + expect_log_msg_containing("Invalid length"); teardown_capture_of_logs(); /* Invalid checksum (taken from prop224) */ @@ -83,7 +83,7 @@ test_validate_address(void *arg) ret = hs_address_is_valid( "????????????????????????????????????????????????????????"); tt_int_op(ret, OP_EQ, 0); - expect_log_msg_containing("can't be decoded"); + expect_log_msg_containing("Unable to base32 decode"); teardown_capture_of_logs(); /* Valid address. */ From f1498e75ddf8e493edecf940616703c36fa17de8 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Tue, 10 Dec 2019 13:54:47 +0200 Subject: [PATCH 02/22] hs-v3: Extract INTRO2 key computation to its own function. Part of #32709 Signed-off-by: David Goulet --- src/feature/hs/hs_cell.c | 92 ++++++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 31 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 52bd663200..d59ea9edb9 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -747,6 +747,61 @@ hs_cell_parse_intro_established(const uint8_t *payload, size_t payload_len) return ret; } +/** For the encrypted INTRO2 cell in encrypted_section, use the crypto + * material in data to compute the right ntor keys. Also validate the + * INTRO2 MAC to ensure that the keys are the right ones. + * + * Return NULL on failure to either produce the key material or on MAC + * valication. Else a newly allocated intro keys object. */ +static hs_ntor_intro_cell_keys_t * +get_introduce2_keys_and_verify_mac(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; + + /* Build the key material out of the key material found in the cell. */ + intro_keys = get_introduce2_key_material(data->auth_pk, data->enc_kp, + data->subcredential, + encrypted_section, + &data->client_pk); + if (intro_keys == NULL) { + log_info(LD_REND, "Invalid INTRODUCE2 encrypted data. Unable to " + "compute key material"); + goto err; + } + + /* Validate MAC from the cell and our computed key material. The MAC field + * in the cell is at the end of the encrypted section. */ + { + uint8_t mac[DIGEST256_LEN]; + /* The MAC field is at the very end of the ENCRYPTED section. */ + size_t mac_offset = encrypted_section_len - sizeof(mac); + /* Compute the MAC. Use the entire encoded payload with a length up to the + * ENCRYPTED section. */ + compute_introduce_mac(data->payload, + data->payload_len - encrypted_section_len, + encrypted_section, encrypted_section_len, + intro_keys->mac_key, sizeof(intro_keys->mac_key), + mac, sizeof(mac)); + if (tor_memcmp(mac, encrypted_section + mac_offset, sizeof(mac))) { + log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); + goto err; + } + } + + goto done; + + err: + if (intro_keys) { + memwipe(intro_keys, 0, sizeof(hs_ntor_intro_cell_keys_t)); + tor_free(intro_keys); + } + + done: + 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 @@ -801,41 +856,16 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data, goto done; } - /* Build the key material out of the key material found in the cell. */ - intro_keys = get_introduce2_key_material(data->auth_pk, data->enc_kp, - data->subcredential, - encrypted_section, - &data->client_pk); - if (intro_keys == NULL) { - log_info(LD_REND, "Invalid INTRODUCE2 encrypted data. Unable to " - "compute key material on circuit %u for service %s", - TO_CIRCUIT(circ)->n_circ_id, + /* 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; } - /* Validate MAC from the cell and our computed key material. The MAC field - * in the cell is at the end of the encrypted section. */ - { - uint8_t mac[DIGEST256_LEN]; - /* The MAC field is at the very end of the ENCRYPTED section. */ - size_t mac_offset = encrypted_section_len - sizeof(mac); - /* Compute the MAC. Use the entire encoded payload with a length up to the - * ENCRYPTED section. */ - compute_introduce_mac(data->payload, - data->payload_len - encrypted_section_len, - encrypted_section, encrypted_section_len, - intro_keys->mac_key, sizeof(intro_keys->mac_key), - mac, sizeof(mac)); - if (tor_memcmp(mac, encrypted_section + mac_offset, sizeof(mac))) { - log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell on " - "circuit %u for service %s", - TO_CIRCUIT(circ)->n_circ_id, - safe_str_client(service->onion_address)); - goto done; - } - } - { /* The ENCRYPTED_DATA section starts just after the CLIENT_PK. */ const uint8_t *encrypted_data = From ef28afa2551a6827d85ceb00d8fe2a69d1605795 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 9 Jan 2020 14:51:56 -0500 Subject: [PATCH 03/22] hs-v3: Add the Onion Balance config file option At this commit, the service reads the config file and parse it to finally set the service config object with the options. Part of #32709 Signed-off-by: David Goulet --- src/app/config/config.c | 2 + src/feature/hs/hs_config.c | 25 +++- src/feature/hs/hs_ob.c | 222 ++++++++++++++++++++++++++++++++++++ src/feature/hs/hs_ob.h | 29 +++++ src/feature/hs/hs_service.c | 5 + src/feature/hs/hs_service.h | 6 +- src/feature/hs/include.am | 2 + 7 files changed, 289 insertions(+), 2 deletions(-) create mode 100644 src/feature/hs/hs_ob.c create mode 100644 src/feature/hs/hs_ob.h diff --git a/src/app/config/config.c b/src/app/config/config.c index bbf984ad08..57aa055e73 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -510,6 +510,8 @@ static const config_var_t option_vars_[] = { LINELIST_S, RendConfigLines, NULL), VAR("HiddenServiceEnableIntroDoSBurstPerSec", LINELIST_S, RendConfigLines, NULL), + VAR("HiddenServiceOnionBalanceInstance", + LINELIST_S, RendConfigLines, NULL), VAR("HiddenServiceStatistics", BOOL, HiddenServiceStatistics_option, "1"), V(HidServAuth, LINELIST, NULL), V(ClientOnionAuthDir, FILENAME, NULL), diff --git a/src/feature/hs/hs_config.c b/src/feature/hs/hs_config.c index 64656b1935..684f7bc975 100644 --- a/src/feature/hs/hs_config.c +++ b/src/feature/hs/hs_config.c @@ -26,6 +26,7 @@ #include "feature/hs/hs_common.h" #include "feature/hs/hs_config.h" #include "feature/hs/hs_client.h" +#include "feature/hs/hs_ob.h" #include "feature/hs/hs_service.h" #include "feature/rend/rendclient.h" #include "feature/rend/rendservice.h" @@ -219,6 +220,7 @@ config_has_invalid_options(const config_line_t *line_, "HiddenServiceEnableIntroDoSDefense", "HiddenServiceEnableIntroDoSRatePerSec", "HiddenServiceEnableIntroDoSBurstPerSec", + "HiddenServiceOnionBalanceInstance", NULL /* End marker. */ }; @@ -317,7 +319,7 @@ config_service_v3(const config_line_t *line_, int have_num_ip = 0; bool export_circuit_id = false; /* just to detect duplicate options */ bool dos_enabled = false, dos_rate_per_sec = false; - bool dos_burst_per_sec = false; + bool dos_burst_per_sec = false, ob_instance = false; const char *dup_opt_seen = NULL; const config_line_t *line; @@ -402,6 +404,27 @@ config_service_v3(const config_line_t *line_, config->intro_dos_burst_per_sec); continue; } + if (!strcasecmp(line->key, "HiddenServiceOnionBalanceInstance")) { + bool enabled = !!helper_parse_uint64(line->key, line->value, + 0, 1, &ok); + if (!ok || ob_instance) { + if (ob_instance) { + dup_opt_seen = line->key; + } + goto err; + } + ob_instance = true; + if (!enabled) { + /* Skip if this is disabled. */ + continue; + } + /* Option is enabled, parse config file. */ + ok = hs_ob_parse_config_file(config); + if (!ok) { + goto err; + } + continue; + } } /* We do not load the key material for the service at this stage. This is diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c new file mode 100644 index 0000000000..633b157a09 --- /dev/null +++ b/src/feature/hs/hs_ob.c @@ -0,0 +1,222 @@ +/* Copyright (c) 2017-2020, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file hs_ob.c + * \brief Implement Onion Balance specific code. + * + * \details + * + * XXX: + **/ + +#define HS_OB_PRIVATE + +#include "lib/confmgt/confmgt.h" +#include "lib/encoding/confline.h" + +#include "hs_ob.h" + +/* Options config magic number. */ +#define OB_OPTIONS_MAGIC 0x631DE7EA + +/* Helper macros. */ +#define VAR(varname, conftype, member, initvalue) \ + CONFIG_VAR_ETYPE(ob_options_t, varname, conftype, member, 0, initvalue) +#define V(member,conftype,initvalue) \ + VAR(#member, conftype, member, initvalue) + +/* Dummy instance of ob_options_t, used for type-checking its members with + * CONF_CHECK_VAR_TYPE. */ +DUMMY_TYPECHECK_INSTANCE(ob_options_t); + +/* Array of variables for the config file options. */ +static const config_var_t config_vars[] = { + V(MasterOnionAddress, LINELIST, NULL), + + END_OF_CONFIG_VARS +}; + +/* "Extra" variable in the state that receives lines we can't parse. This + * lets us preserve options from versions of Tor newer than us. */ +static const struct_member_t config_extra_vars = { + .name = "__extra", + .type = CONFIG_TYPE_LINELIST, + .offset = offsetof(ob_options_t, ExtraLines), +}; + +/* Configuration format of ob_options_t. */ +static const config_format_t config_format = { + .size = sizeof(ob_options_t), + .magic = { + "ob_options_t", + OB_OPTIONS_MAGIC, + offsetof(ob_options_t, magic_), + }, + .vars = config_vars, + .extra = &config_extra_vars, +}; + +/* Global configuration manager for the config file. */ +static config_mgr_t *config_options_mgr = NULL; + +/* Return the configuration manager for the config file. */ +static const config_mgr_t * +get_config_options_mgr(void) +{ + if (PREDICT_UNLIKELY(config_options_mgr == NULL)) { + config_options_mgr = config_mgr_new(&config_format); + config_mgr_freeze(config_options_mgr); + } + return config_options_mgr; +} + +#define ob_option_free(val) \ + FREE_AND_NULL(ob_options_t, ob_option_free_, (val)) + +/** Helper: Free a config options object. */ +static void +ob_option_free_(ob_options_t *opts) +{ + if (opts == NULL) { + return; + } + config_free(get_config_options_mgr(), opts); +} + +/** Return an allocated config options object. */ +static ob_options_t * +ob_option_new(void) +{ + ob_options_t *opts = config_new(get_config_options_mgr()); + config_init(get_config_options_mgr(), opts); + return opts; +} + +/** Helper function: From the configuration line value which is an onion + * address with the ".onion" extension, find the public key and put it in + * pkey_out. + * + * On success, true is returned. Else, false and pkey is untouched. */ +static bool +get_onion_public_key(const char *value, ed25519_public_key_t *pkey_out) +{ + char address[HS_SERVICE_ADDR_LEN_BASE32 + 1]; + + tor_assert(value); + tor_assert(pkey_out); + + if (strcmpend(value, ".onion")) { + /* Not a .onion extension, bad format. */ + return false; + } + + /* Length validation. The -1 is because sizeof() counts the NUL byte. */ + if (strlen(value) > + (HS_SERVICE_ADDR_LEN_BASE32 + sizeof(".onion") - 1)) { + /* Too long, bad format. */ + return false; + } + + /* We don't want the .onion so we add 2 because size - 1 is copied with + * strlcpy() in order to accomodate the NUL byte and sizeof() counts the NUL + * byte so we need to remove them from the equation. */ + strlcpy(address, value, strlen(value) - sizeof(".onion") + 2); + + if (hs_parse_address_no_log(address, pkey_out, NULL, NULL, NULL) < 0) { + return false; + } + + /* Success. */ + return true; +} + +/** Parse the given ob options in opts and set the service config object + * accordingly. + * + * Return 1 on success else 0. */ +static int +ob_option_parse(hs_service_config_t *config, const ob_options_t *opts) +{ + int ret = 0; + config_line_t *line; + + tor_assert(config); + tor_assert(opts); + + for (line = opts->MasterOnionAddress; line; line = line->next) { + /* Allocate config list if need be. */ + if (!config->ob_master_pubkeys) { + config->ob_master_pubkeys = smartlist_new(); + } + ed25519_public_key_t *pubkey = tor_malloc_zero(sizeof(*pubkey)); + + if (!get_onion_public_key(line->value, pubkey)) { + log_warn(LD_REND, "OnionBalance: MasterOnionAddress %s is invalid", + line->value); + tor_free(pubkey); + goto end; + } + smartlist_add(config->ob_master_pubkeys, pubkey); + } + /* Success. */ + ret = 1; + + end: + /* No keys added, we free the list since no list means no onion balance + * support for this tor instance. */ + if (smartlist_len(config->ob_master_pubkeys) == 0) { + smartlist_free(config->ob_master_pubkeys); + } + return ret; +} + +/** Read and parse the config file at fname on disk. The service config object + * is populated with the options if any. + * + * Return 1 on success else 0. This is to follow the "ok" convention in + * hs_config.c. */ +int +hs_ob_parse_config_file(hs_service_config_t *config) +{ + static const char *fname = "ob_config"; + int ret = 0; + char *content = NULL, *errmsg = NULL, *config_file_path = NULL; + ob_options_t *options = NULL; + config_line_t *lines = NULL; + + tor_assert(config); + + /* Read file from disk. */ + config_file_path = hs_path_from_filename(config->directory_path, fname); + content = read_file_to_str(config_file_path, 0, NULL); + if (!content) { + log_warn(LD_FS, "OnionBalance: Unable to read config file %s", + escaped(config_file_path)); + goto end; + } + + /* Parse lines. */ + if (config_get_lines(content, &lines, 0) < 0) { + goto end; + } + + options = ob_option_new(); + config_assign(get_config_options_mgr(), options, lines, 0, &errmsg); + if (errmsg) { + log_warn(LD_REND, "OnionBalance: Unable to parse config file: %s", + errmsg); + tor_free(errmsg); + goto end; + } + + /* Parse the options and set the service config object with the details. */ + ret = ob_option_parse(config, options); + + end: + config_free_lines(lines); + ob_option_free(options); + tor_free(content); + tor_free(config_file_path); + return ret; +} diff --git a/src/feature/hs/hs_ob.h b/src/feature/hs/hs_ob.h new file mode 100644 index 0000000000..d16896f2ed --- /dev/null +++ b/src/feature/hs/hs_ob.h @@ -0,0 +1,29 @@ +/* Copyright (c) 2020, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file hs_ob.h + * \brief Header file for the specific code for onion balance. + **/ + +#ifndef TOR_HS_OB_H +#define TOR_HS_OB_H + +#include "hs_service.h" + +int hs_ob_parse_config_file(hs_service_config_t *config); + +#ifdef HS_OB_PRIVATE + +typedef struct ob_options_t { + /** Magic number to identify the structure in memory. */ + uint32_t magic_; + /** Master Onion Address(es). */ + struct config_line_t *MasterOnionAddress; + /** Extra Lines for configuration we might not know. */ + struct config_line_t *ExtraLines; +} ob_options_t; + +#endif /* defined(HS_OB_PRIVATE) */ + +#endif /* !defined(TOR_HS_OB_H) */ diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 81b37eab40..d0d9d65790 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -267,6 +267,11 @@ service_clear_config(hs_service_config_t *config) service_authorized_client_free(p)); smartlist_free(config->clients); } + if (config->ob_master_pubkeys) { + SMARTLIST_FOREACH(config->ob_master_pubkeys, ed25519_public_key_t *, k, + tor_free(k)); + smartlist_free(config->ob_master_pubkeys); + } memset(config, 0, sizeof(*config)); } diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index 8809411e01..bdc2bc3b64 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -248,10 +248,14 @@ typedef struct hs_service_config_t { /** Does this service export the circuit ID of its clients? */ hs_circuit_id_protocol_t circuit_id_protocol; - /* DoS defenses. For the ESTABLISH_INTRO cell extension. */ + /** DoS defenses. For the ESTABLISH_INTRO cell extension. */ unsigned int has_dos_defense_enabled : 1; uint32_t intro_dos_rate_per_sec; uint32_t intro_dos_burst_per_sec; + + /** If set, contains the Onion Balance master ed25519 public key (taken from + * an .onion addresses) that this tor instance serves as backend. */ + smartlist_t *ob_master_pubkeys; } hs_service_config_t; /** Service state. */ diff --git a/src/feature/hs/include.am b/src/feature/hs/include.am index 5e69607e59..f83907c76b 100644 --- a/src/feature/hs/include.am +++ b/src/feature/hs/include.am @@ -13,6 +13,7 @@ LIBTOR_APP_A_SOURCES += \ src/feature/hs/hs_dos.c \ src/feature/hs/hs_ident.c \ src/feature/hs/hs_intropoint.c \ + src/feature/hs/hs_ob.c \ src/feature/hs/hs_service.c \ src/feature/hs/hs_stats.c @@ -30,6 +31,7 @@ noinst_HEADERS += \ src/feature/hs/hs_dos.h \ src/feature/hs/hs_ident.h \ src/feature/hs/hs_intropoint.h \ + src/feature/hs/hs_ob.h \ src/feature/hs/hs_service.h \ src/feature/hs/hs_stats.h \ src/feature/hs/hsdir_index_st.h From 02f1caa583ca0e09e4c75ff6d9399f5d53931d2b Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 9 Jan 2020 15:18:32 -0500 Subject: [PATCH 04/22] hs-v3: Validate INTRO2 cells for onion balance Closes #32709 Signed-off-by: David Goulet --- src/feature/hs/hs_cell.c | 78 +++++++++++++++++++++++++++++--- src/feature/hs/hs_ob.c | 97 ++++++++++++++++++++++++++++++++++++++++ src/feature/hs/hs_ob.h | 3 ++ 3 files changed, 171 insertions(+), 7 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index d59ea9edb9..680897cf90 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -13,6 +13,7 @@ #include "feature/hs_common/replaycache.h" #include "feature/hs/hs_cell.h" +#include "feature/hs/hs_ob.h" #include "core/crypto/hs_ntor.h" #include "core/or/origin_circuit_st.h" @@ -802,6 +803,47 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, return intro_keys; } +/** 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_config_t *config, + const hs_cell_introduce2_data_t *data, + const uint8_t *encrypted_section, + size_t encrypted_section_len) +{ + uint8_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) { + /* We are _not_ an OB instance since no configured master onion key(s) + * were found and thus no subcredentials were built. */ + goto end; + } + + for (size_t idx = 0; idx < ob_num_subcreds; idx++) { + /* 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.subcredential = &(ob_subcreds[idx * DIGEST256_LEN]); + intro_keys = get_introduce2_keys_and_verify_mac(&new_data, + encrypted_section, + encrypted_section_len); + if (intro_keys) { + /* It validates. We have a hit as an onion balance instance. */ + goto end; + } + } + + end: + tor_free(ob_subcreds); + 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 @@ -856,14 +898,36 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data, goto done; } - /* 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); + /* First bytes of the ENCRYPTED section are the client public key (they are + * guaranteed to exist because of the length check above). We are gonna use + * the client public key to compute the ntor keys and decrypt the payload: + */ + 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 (service->config.ob_master_pubkeys) { + intro_keys = get_intro2_keys_as_ob(&service->config, 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; + /* 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; + } } { diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 633b157a09..7e84af3d99 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -158,6 +158,8 @@ ob_option_parse(hs_service_config_t *config, const ob_options_t *opts) goto end; } smartlist_add(config->ob_master_pubkeys, pubkey); + log_info(LD_REND, "OnionBalance: MasterOnionAddress %s registered", + line->value); } /* Success. */ ret = 1; @@ -171,6 +173,26 @@ ob_option_parse(hs_service_config_t *config, const ob_options_t *opts) return ret; } +/** For the given master public key and time period, compute the subcredential + * and put them into subcredential. The subcredential parameter needs to be at + * least DIGEST256_LEN in size. */ +static void +build_subcredential(const ed25519_public_key_t *pkey, uint64_t tp, + uint8_t *subcredential) +{ + ed25519_public_key_t blinded_pubkey; + + tor_assert(pkey); + tor_assert(subcredential); + + hs_build_blinded_pubkey(pkey, NULL, 0, tp, &blinded_pubkey); + hs_get_subcredential(pkey, &blinded_pubkey, subcredential); +} + +/* + * Public API. + */ + /** Read and parse the config file at fname on disk. The service config object * is populated with the options if any. * @@ -220,3 +242,78 @@ hs_ob_parse_config_file(hs_service_config_t *config) tor_free(config_file_path); return ret; } + +/** Compute all possible subcredentials for every onion master key in the + * given service config object. The subcredentials is allocated and set as an + * continous array containing all possible values. + * + * On success, return the number of subcredential put in the array which will + * correspond to an arry of size: n * DIGEST256_LEN where DIGEST256_LEN is the + * length of a single subcredential. + * + * If the given configuration object has no OB master keys configured, 0 is + * returned and subcredentials is set to NULL. + * + * Otherwise, this can't fail. */ +size_t +hs_ob_get_subcredentials(const hs_service_config_t *config, + uint8_t **subcredentials) +{ + unsigned int num_pkeys, idx = 0; + uint8_t *subcreds = NULL; + const int steps[3] = {0, -1, 1}; + const unsigned int num_steps = ARRAY_LENGTH(steps); + const size_t subcred_len = DIGEST256_LEN; + const uint64_t tp = hs_get_time_period_num(0); + + tor_assert(config); + tor_assert(subcredentials); + + num_pkeys = smartlist_len(config->ob_master_pubkeys); + if (!num_pkeys) { + goto end; + } + + /* 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 + * key in order to accomodate client and service consensus skew. + * + * If the client consensus after_time is at 23:00 but the service one is at + * 01:00, the client will be using the previous time period where the + * service will think it is the client next time period. Thus why we have + * to try them all. + * + * The normal use case works because the service gets the descriptor object + * that corresponds to the intro point's request, and because each + * descriptor corresponds to a specific subcredential, we get the right + * subcredential out of it, and use that to do the decryption. + * + * As a slight optimization, statistically, the current time period (0) will + * be the one to work first so we'll put them first in the array to maximize + * 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. + * + * 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 + * total number of keys we are about to process. In other words, for each + * key, we allocate 3 subcredential slots. */ + subcreds = tor_malloc_zero(subcred_len * num_steps * num_pkeys); + + /* For each time period step. */ + for (unsigned int i = 0; i < num_steps; i++) { + SMARTLIST_FOREACH_BEGIN(config->ob_master_pubkeys, + const ed25519_public_key_t *, pkey) { + build_subcredential(pkey, tp + steps[i], + &(subcreds[idx * subcred_len])); + idx++; + } SMARTLIST_FOREACH_END(pkey); + } + + end: + *subcredentials = subcreds; + return idx; +} diff --git a/src/feature/hs/hs_ob.h b/src/feature/hs/hs_ob.h index d16896f2ed..8ad6aabc4f 100644 --- a/src/feature/hs/hs_ob.h +++ b/src/feature/hs/hs_ob.h @@ -13,6 +13,9 @@ int hs_ob_parse_config_file(hs_service_config_t *config); +size_t hs_ob_get_subcredentials(const hs_service_config_t *config, + uint8_t **subcredentials); + #ifdef HS_OB_PRIVATE typedef struct ob_options_t { From 780e498f760b139fb540d2e050de08df60714f4a Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 14 Jan 2020 12:42:09 -0500 Subject: [PATCH 05/22] hs-v3: Code improvement for INTRO2 MAC validation Pointed by nickm during the review of #32709. Signed-off-by: David Goulet --- src/feature/hs/hs_cell.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 680897cf90..021a41825d 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -776,6 +776,12 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, * in the cell is at the end of the encrypted section. */ { uint8_t mac[DIGEST256_LEN]; + + /* Make sure we are now about to underflow. */ + if (encrypted_section_len < sizeof(mac)) { + goto err; + } + /* The MAC field is at the very end of the ENCRYPTED section. */ size_t mac_offset = encrypted_section_len - sizeof(mac); /* Compute the MAC. Use the entire encoded payload with a length up to the @@ -785,7 +791,7 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, encrypted_section, encrypted_section_len, intro_keys->mac_key, sizeof(intro_keys->mac_key), mac, sizeof(mac)); - if (tor_memcmp(mac, encrypted_section + mac_offset, sizeof(mac))) { + if (tor_memneq(mac, encrypted_section + mac_offset, sizeof(mac))) { log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); goto err; } From faada6af8d77eee40499be06acfab80d0ffac97f Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 14 Jan 2020 12:54:56 -0500 Subject: [PATCH 06/22] hs-v3: Implement hs_ob_service_is_instance() Signed-off-by: David Goulet --- src/feature/hs/hs_cell.c | 2 +- src/feature/hs/hs_ob.c | 18 ++++++++++++++++++ src/feature/hs/hs_ob.h | 2 ++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 021a41825d..aabc558599 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -916,7 +916,7 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data, * 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 (service->config.ob_master_pubkeys) { + if (hs_ob_service_is_instance(service)) { intro_keys = get_intro2_keys_as_ob(&service->config, data, encrypted_section, encrypted_section_len); diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 7e84af3d99..62db3bd434 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -193,6 +193,24 @@ build_subcredential(const ed25519_public_key_t *pkey, uint64_t tp, * Public API. */ +/** Return true iff the given service is configured as an onion balance + * instance. To satisfy that condition, there must at least be one master + * ed25519 public key configured. */ +bool +hs_ob_service_is_instance(const hs_service_t *service) +{ + if (BUG(service == NULL)) { + return false; + } + + /* No list, we are not an instance. */ + if (!service->config.ob_master_pubkeys) { + return false; + } + + return smartlist_len(service->config.ob_master_pubkeys) > 0; +} + /** Read and parse the config file at fname on disk. The service config object * is populated with the options if any. * diff --git a/src/feature/hs/hs_ob.h b/src/feature/hs/hs_ob.h index 8ad6aabc4f..fea6a737d7 100644 --- a/src/feature/hs/hs_ob.h +++ b/src/feature/hs/hs_ob.h @@ -11,6 +11,8 @@ #include "hs_service.h" +bool hs_ob_service_is_instance(const hs_service_t *service); + int hs_ob_parse_config_file(hs_service_config_t *config); size_t hs_ob_get_subcredentials(const hs_service_config_t *config, From 7c18860c3e0403246af0d9d79cecc79f97c7f2d6 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 14 Jan 2020 11:29:08 -0500 Subject: [PATCH 07/22] test: Add HS onion balance tests Signed-off-by: David Goulet --- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_hs_ob.c | 231 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 234 insertions(+) create mode 100644 src/test/test_hs_ob.c diff --git a/src/test/include.am b/src/test/include.am index 90e50752ce..3590745ba7 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -167,6 +167,7 @@ src_test_test_SOURCES += \ src/test/test_hs_client.c \ src/test/test_hs_intropoint.c \ src/test/test_hs_control.c \ + src/test/test_hs_ob.c \ src/test/test_handles.c \ src/test/test_hs_cache.c \ src/test/test_hs_descriptor.c \ diff --git a/src/test/test.c b/src/test/test.c index 1742f1d952..4b6082ce4f 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -721,6 +721,7 @@ struct testgroup_t testgroups[] = { { "hs_dos/", hs_dos_tests }, { "hs_intropoint/", hs_intropoint_tests }, { "hs_ntor/", hs_ntor_tests }, + { "hs_ob/", hs_ob_tests }, { "hs_service/", hs_service_tests }, { "introduce/", introduce_tests }, { "keypin/", keypin_tests }, diff --git a/src/test/test.h b/src/test/test.h index 63e2faff95..18987719d0 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -141,6 +141,7 @@ extern struct testcase_t hs_descriptor[]; extern struct testcase_t hs_dos_tests[]; extern struct testcase_t hs_intropoint_tests[]; extern struct testcase_t hs_ntor_tests[]; +extern struct testcase_t hs_ob_tests[]; extern struct testcase_t hs_service_tests[]; extern struct testcase_t hs_tests[]; extern struct testcase_t introduce_tests[]; diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c new file mode 100644 index 0000000000..37c57f795e --- /dev/null +++ b/src/test/test_hs_ob.c @@ -0,0 +1,231 @@ +/* Copyright (c) 2020, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file test_hs_ob.c + * \brief Test hidden service onion balance functionality. + */ + +#define CONFIG_PRIVATE +#define HS_SERVICE_PRIVATE + +#include "test/test.h" +#include "test/test_helpers.h" +#include "test/log_test_helpers.h" + +#include "app/config/config.h" +#include "feature/hs/hs_config.h" +#include "feature/hs/hs_ob.h" +#include "feature/hs/hs_service.h" +#include "feature/nodelist/networkstatus.h" +#include "feature/nodelist/networkstatus_st.h" + +static ed25519_keypair_t onion_addr_kp_1; +static char onion_addr_1[HS_SERVICE_ADDR_LEN_BASE32 + 1]; + +static ed25519_keypair_t onion_addr_kp_2; +static char onion_addr_2[HS_SERVICE_ADDR_LEN_BASE32 + 1]; + +static bool config_is_good = true; + +static int +helper_tor_config(const char *conf) +{ + int ret = -1; + or_options_t *options = helper_parse_options(conf); + tt_assert(options); + ret = hs_config_service_all(options, 0); + done: + or_options_free(options); + return ret; +} + +static networkstatus_t mock_ns; + +static networkstatus_t * +mock_networkstatus_get_live_consensus(time_t now) +{ + (void) now; + return &mock_ns; +} + +static char * +mock_read_file_to_str(const char *filename, int flags, struct stat *stat_out) +{ + char *ret = NULL; + + (void) flags; + (void) stat_out; + + if (!strcmp(filename, get_fname("hs3" PATH_SEPARATOR "ob_config"))) { + if (config_is_good) { + tor_asprintf(&ret, "MasterOnionAddress %s.onion\n" + "MasterOnionAddress %s.onion\n", + onion_addr_1, onion_addr_2); + } else { + tor_asprintf(&ret, "MasterOnionAddress JUNKJUNKJUNK.onion\n" + "UnknownOption BLAH\n"); + } + goto done; + } + + done: + return ret; +} + +static void +test_parse_config_file(void *arg) +{ + int ret; + char *conf = NULL; + const ed25519_public_key_t *pkey; + + (void) arg; + + hs_init(); + + MOCK(read_file_to_str, mock_read_file_to_str); + +#define fmt_conf \ + "HiddenServiceDir %s\n" \ + "HiddenServicePort 22\n" \ + "HiddenServiceOnionBalanceInstance 1\n" + tor_asprintf(&conf, fmt_conf, get_fname("hs3")); +#undef fmt_conf + + /* Build the OB frontend onion addresses. */ + ed25519_keypair_generate(&onion_addr_kp_1, 0); + hs_build_address(&onion_addr_kp_1.pubkey, HS_VERSION_THREE, onion_addr_1); + ed25519_keypair_generate(&onion_addr_kp_2, 0); + hs_build_address(&onion_addr_kp_2.pubkey, HS_VERSION_THREE, onion_addr_2); + + ret = helper_tor_config(conf); + tor_free(conf); + tt_int_op(ret, OP_EQ, 0); + + /* Load the keys for the service. After that, the v3 service should be + * registered in the global map and we'll be able to access it. */ + tt_int_op(get_hs_service_staging_list_size(), OP_EQ, 1); + hs_service_load_all_keys(); + tt_int_op(get_hs_service_map_size(), OP_EQ, 1); + const hs_service_t *s = get_first_service(); + tt_assert(s); + tt_assert(s->config.ob_master_pubkeys); + tt_assert(hs_ob_service_is_instance(s)); + tt_assert(smartlist_len(s->config.ob_master_pubkeys) == 2); + + /* Test the public keys we've added. */ + pkey = smartlist_get(s->config.ob_master_pubkeys, 0); + tt_mem_op(&onion_addr_kp_1.pubkey, OP_EQ, pkey, ED25519_PUBKEY_LEN); + pkey = smartlist_get(s->config.ob_master_pubkeys, 1); + tt_mem_op(&onion_addr_kp_2.pubkey, OP_EQ, pkey, ED25519_PUBKEY_LEN); + + done: + hs_free_all(); + + UNMOCK(read_file_to_str); +} + +static void +test_parse_config_file_bad(void *arg) +{ + int ret; + char *conf = NULL; + + (void) arg; + + hs_init(); + + MOCK(read_file_to_str, mock_read_file_to_str); + + /* Indicate mock_read_file_to_str() to use the bad config. */ + config_is_good = false; + +#define fmt_conf \ + "HiddenServiceDir %s\n" \ + "HiddenServicePort 22\n" \ + "HiddenServiceOnionBalanceInstance 1\n" + tor_asprintf(&conf, fmt_conf, get_fname("hs3")); +#undef fmt_conf + + setup_full_capture_of_logs(LOG_INFO); + ret = helper_tor_config(conf); + tor_free(conf); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("OnionBalance: MasterOnionAddress " + "JUNKJUNKJUNK.onion is invalid"); + expect_log_msg_containing("Found unrecognized option \'UnknownOption\'; " + "saving it."); + teardown_capture_of_logs(); + + done: + hs_free_all(); + + UNMOCK(read_file_to_str); +} + +static void +test_get_subcredentials(void *arg) +{ + int ret; + hs_service_config_t config; + + (void) arg; + + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); + + /* Setup consensus with proper time so we can compute the time period. */ + ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC", + &mock_ns.valid_after); + tt_int_op(ret, OP_EQ, 0); + ret = parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC", + &mock_ns.fresh_until); + tt_int_op(ret, OP_EQ, 0); + + config.ob_master_pubkeys = smartlist_new(); + tt_assert(config.ob_master_pubkeys); + + /* Generate a keypair to add to the list. */ + ed25519_keypair_generate(&onion_addr_kp_1, 0); + smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); + + uint8_t *subcreds = NULL; + size_t num = hs_ob_get_subcredentials(&config, &subcreds); + tt_uint_op(num, OP_EQ, 3); + + /* Validate the subcredentials we just got. We'll build them oursevles with + * the right time period steps and compare. */ + const uint64_t tp = hs_get_time_period_num(0); + const int steps[3] = {0, -1, 1}; + + for (unsigned int i = 0; i < num; i++) { + uint8_t subcredential[DIGEST256_LEN]; + ed25519_public_key_t blinded_pubkey; + hs_build_blinded_pubkey(&onion_addr_kp_1.pubkey, NULL, 0, tp + steps[i], + &blinded_pubkey); + hs_get_subcredential(&onion_addr_kp_1.pubkey, &blinded_pubkey, + subcredential); + tt_mem_op(&(subcreds[i * sizeof(subcredential)]), OP_EQ, subcredential, + sizeof(subcredential)); + } + + done: + tor_free(subcreds); + smartlist_free(config.ob_master_pubkeys); + + UNMOCK(networkstatus_get_live_consensus); +} + +struct testcase_t hs_ob_tests[] = { + { "parse_config_file", test_parse_config_file, TT_FORK, + NULL, NULL }, + { "parse_config_file_bad", test_parse_config_file_bad, TT_FORK, + NULL, NULL }, + + { "get_subcredentials", test_get_subcredentials, TT_FORK, + NULL, NULL }, + + END_OF_TESTCASES +}; + From 3484608bda4d8c329ad886ddf98087d775c43a72 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 14 Jan 2020 12:04:39 -0500 Subject: [PATCH 08/22] practracker: Make it happy Signed-off-by: David Goulet --- scripts/maint/practracker/exceptions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 70e6a55199..09c496c6dd 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -250,7 +250,7 @@ problem function-size /src/feature/hs/hs_cell.c:hs_cell_parse_introduce2() 152 problem function-size /src/feature/hs/hs_client.c:send_introduce1() 103 problem function-size /src/feature/hs/hs_client.c:hs_config_client_authorization() 107 problem function-size /src/feature/hs/hs_common.c:hs_get_responsible_hsdirs() 102 -problem function-size /src/feature/hs/hs_config.c:config_service_v3() 107 +problem function-size /src/feature/hs/hs_config.c:config_service_v3() 128 problem function-size /src/feature/hs/hs_config.c:config_generic_service() 138 problem function-size /src/feature/hs/hs_descriptor.c:desc_encode_v3() 101 problem function-size /src/feature/hs/hs_descriptor.c:decrypt_desc_layer() 111 From 4532c7ef6aa96d502412cbc61da91369bc3eaa44 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 16 Jan 2020 18:27:25 -0500 Subject: [PATCH 09/22] Turn hs_subcredential_t into a proper struct. --- src/core/crypto/hs_ntor.c | 8 ++++---- src/core/crypto/hs_ntor.h | 13 +++++++++++-- src/feature/hs/hs_cell.c | 6 +++--- src/feature/hs/hs_cell.h | 6 ++++-- src/feature/hs/hs_circuit.c | 6 +++--- src/feature/hs/hs_circuit.h | 5 +++-- src/feature/hs/hs_client.c | 12 ++++++------ src/feature/hs/hs_common.c | 7 ++++--- src/feature/hs/hs_common.h | 3 ++- src/feature/hs/hs_descriptor.c | 24 +++++++++++++----------- src/feature/hs/hs_descriptor.h | 7 ++++--- src/feature/hs/hs_ob.c | 12 +++++------- src/feature/hs/hs_ob.h | 3 ++- src/feature/hs/hs_service.c | 9 +++++---- src/test/fuzz/fuzz_hsdescv3.c | 7 +++---- src/test/hs_test_helpers.c | 6 +++--- src/test/hs_test_helpers.h | 7 +++---- src/test/test_hs_cache.c | 21 ++++++++++++--------- src/test/test_hs_client.c | 7 ++++--- src/test/test_hs_descriptor.c | 28 ++++++++++++++-------------- src/test/test_hs_ntor.c | 8 ++++---- src/test/test_hs_ntor_cl.c | 16 ++++++++-------- src/test/test_hs_ob.c | 11 +++++------ 23 files changed, 125 insertions(+), 107 deletions(-) diff --git a/src/core/crypto/hs_ntor.c b/src/core/crypto/hs_ntor.c index 2bd4c32446..0422e72795 100644 --- a/src/core/crypto/hs_ntor.c +++ b/src/core/crypto/hs_ntor.c @@ -170,7 +170,7 @@ get_rendezvous1_key_material(const uint8_t *rend_secret_hs_input, * necessary key material, and return 0. */ static void get_introduce1_key_material(const uint8_t *secret_input, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { uint8_t keystream[CIPHER256_KEY_LEN + DIGEST256_LEN]; @@ -181,7 +181,7 @@ get_introduce1_key_material(const uint8_t *secret_input, /* Let's build info */ ptr = info_blob; APPEND(ptr, M_HSEXPAND, strlen(M_HSEXPAND)); - APPEND(ptr, subcredential, DIGEST256_LEN); + APPEND(ptr, subcredential->subcred, SUBCRED_LEN); tor_assert(ptr == info_blob + sizeof(info_blob)); /* Let's build the input to the KDF */ @@ -317,7 +317,7 @@ hs_ntor_client_get_introduce1_keys( const ed25519_public_key_t *intro_auth_pubkey, const curve25519_public_key_t *intro_enc_pubkey, const curve25519_keypair_t *client_ephemeral_enc_keypair, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { int bad = 0; @@ -450,7 +450,7 @@ hs_ntor_service_get_introduce1_keys( const ed25519_public_key_t *intro_auth_pubkey, const curve25519_keypair_t *intro_enc_keypair, const curve25519_public_key_t *client_ephemeral_enc_pubkey, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { int bad = 0; diff --git a/src/core/crypto/hs_ntor.h b/src/core/crypto/hs_ntor.h index 2bce5686cd..b78bc4e802 100644 --- a/src/core/crypto/hs_ntor.h +++ b/src/core/crypto/hs_ntor.h @@ -35,11 +35,20 @@ typedef struct hs_ntor_rend_cell_keys_t { uint8_t ntor_key_seed[DIGEST256_LEN]; } hs_ntor_rend_cell_keys_t; +#define SUBCRED_LEN DIGEST256_LEN + +/** + * A 'subcredential' used to prove knowledge of a hidden service. + **/ +typedef struct hs_subcredential_t { + uint8_t subcred[SUBCRED_LEN]; +} hs_subcredential_t; + int hs_ntor_client_get_introduce1_keys( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_public_key_t *intro_enc_pubkey, const struct curve25519_keypair_t *client_ephemeral_enc_keypair, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out); int hs_ntor_client_get_rendezvous1_keys( @@ -53,7 +62,7 @@ int hs_ntor_service_get_introduce1_keys( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_keypair_t *intro_enc_keypair, const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out); int hs_ntor_service_get_rendezvous1_keys( diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index aabc558599..d377e9134d 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -75,7 +75,7 @@ compute_introduce_mac(const uint8_t *encoded_cell, size_t encoded_cell_len, static hs_ntor_intro_cell_keys_t * get_introduce2_key_material(const ed25519_public_key_t *auth_key, const curve25519_keypair_t *enc_key, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const uint8_t *encrypted_section, curve25519_public_key_t *client_pk) { @@ -820,7 +820,7 @@ get_intro2_keys_as_ob(const hs_service_config_t *config, const uint8_t *encrypted_section, size_t encrypted_section_len) { - uint8_t *ob_subcreds = NULL; + hs_subcredential_t *ob_subcreds = NULL; size_t ob_num_subcreds; hs_ntor_intro_cell_keys_t *intro_keys = NULL; @@ -835,7 +835,7 @@ 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; - new_data.subcredential = &(ob_subcreds[idx * DIGEST256_LEN]); + new_data.subcredential = &ob_subcreds[idx]; intro_keys = get_introduce2_keys_and_verify_mac(&new_data, encrypted_section, encrypted_section_len); diff --git a/src/feature/hs/hs_cell.h b/src/feature/hs/hs_cell.h index 80f37057d2..58cc401cc0 100644 --- a/src/feature/hs/hs_cell.h +++ b/src/feature/hs/hs_cell.h @@ -16,6 +16,8 @@ * 3.2.2 of the specification). Below this value, the cell must be padded. */ #define HS_CELL_INTRODUCE1_MIN_SIZE 246 +struct hs_subcredential_t; + /** This data structure contains data that we need to build an INTRODUCE1 cell * used by the INTRODUCE1 build function. */ typedef struct hs_cell_introduce1_data_t { @@ -29,7 +31,7 @@ typedef struct hs_cell_introduce1_data_t { /** Introduction point encryption public key. */ const curve25519_public_key_t *enc_pk; /** Subcredentials of the service. */ - const uint8_t *subcredential; + const struct hs_subcredential_t *subcredential; /** Onion public key for the ntor handshake. */ const curve25519_public_key_t *onion_pk; /** Rendezvous cookie. */ @@ -57,7 +59,7 @@ typedef struct hs_cell_introduce2_data_t { const curve25519_keypair_t *enc_kp; /** Subcredentials of the service. Pointer owned by the descriptor that owns the introduction point through which we received the INTRO2 cell. */ - const uint8_t *subcredential; + const struct hs_subcredential_t *subcredential; /** Payload of the received encoded cell. */ const uint8_t *payload; /** Size of the payload of the received encoded cell. */ diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 90805a98b7..fb3694b2d4 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -578,7 +578,7 @@ retry_service_rendezvous_point(const origin_circuit_t *circ) static int setup_introduce1_data(const hs_desc_intro_point_t *ip, const node_t *rp_node, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_cell_introduce1_data_t *intro1_data) { int ret = -1; @@ -966,7 +966,7 @@ int hs_circ_handle_introduce2(const hs_service_t *service, const origin_circuit_t *circ, hs_service_intro_point_t *ip, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const uint8_t *payload, size_t payload_len) { int ret = -1; @@ -1092,7 +1092,7 @@ int hs_circ_send_introduce1(origin_circuit_t *intro_circ, origin_circuit_t *rend_circ, const hs_desc_intro_point_t *ip, - const uint8_t *subcredential) + const hs_subcredential_t *subcredential) { int ret = -1; ssize_t payload_len; diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h index 92231369c6..5c20e38187 100644 --- a/src/feature/hs/hs_circuit.h +++ b/src/feature/hs/hs_circuit.h @@ -46,15 +46,16 @@ int hs_circ_handle_intro_established(const hs_service_t *service, origin_circuit_t *circ, const uint8_t *payload, size_t payload_len); +struct hs_subcredential_t; int hs_circ_handle_introduce2(const hs_service_t *service, const origin_circuit_t *circ, hs_service_intro_point_t *ip, - const uint8_t *subcredential, + const struct hs_subcredential_t *subcredential, const uint8_t *payload, size_t payload_len); int hs_circ_send_introduce1(origin_circuit_t *intro_circ, origin_circuit_t *rend_circ, const hs_desc_intro_point_t *ip, - const uint8_t *subcredential); + const struct hs_subcredential_t *subcredential); int hs_circ_send_establish_rendezvous(origin_circuit_t *circ); /* e2e circuit API. */ diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index bcb0495c6f..601062190d 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -641,7 +641,7 @@ send_introduce1(origin_circuit_t *intro_circ, /* Send the INTRODUCE1 cell. */ if (hs_circ_send_introduce1(intro_circ, rend_circ, ip, - desc->subcredential) < 0) { + &desc->subcredential) < 0) { if (TO_CIRCUIT(intro_circ)->marked_for_close) { /* If the introduction circuit was closed, we were unable to send the * cell for some reasons. In any case, the intro circuit has to be @@ -1817,7 +1817,7 @@ hs_client_decode_descriptor(const char *desc_str, hs_descriptor_t **desc) { hs_desc_decode_status_t ret; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; ed25519_public_key_t blinded_pubkey; hs_client_service_authorization_t *client_auth = NULL; curve25519_secret_key_t *client_auht_sk = NULL; @@ -1837,13 +1837,14 @@ hs_client_decode_descriptor(const char *desc_str, uint64_t current_time_period = hs_get_time_period_num(0); hs_build_blinded_pubkey(service_identity_pk, NULL, 0, current_time_period, &blinded_pubkey); - hs_get_subcredential(service_identity_pk, &blinded_pubkey, subcredential); + hs_get_subcredential(service_identity_pk, &blinded_pubkey, & + subcredential); } /* Parse descriptor */ - ret = hs_desc_decode_descriptor(desc_str, subcredential, + ret = hs_desc_decode_descriptor(desc_str, &subcredential, client_auht_sk, desc); - memwipe(subcredential, 0, sizeof(subcredential)); + memwipe(&subcredential, 0, sizeof(subcredential)); if (ret != HS_DESC_DECODE_OK) { goto err; } @@ -2456,4 +2457,3 @@ set_hs_client_auths_map(digest256map_t *map) } #endif /* defined(TOR_UNIT_TESTS) */ - diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index 47594b582e..b2b52c480e 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -808,12 +808,12 @@ hs_parse_address_impl(const char *address, ed25519_public_key_t *key_out, } /** Using the given identity public key and a blinded public key, compute the - * subcredential and put it in subcred_out (must be of size DIGEST256_LEN). + * subcredential and put it in subcred_out. * This can't fail. */ void hs_get_subcredential(const ed25519_public_key_t *identity_pk, const ed25519_public_key_t *blinded_pk, - uint8_t *subcred_out) + hs_subcredential_t *subcred_out) { uint8_t credential[DIGEST256_LEN]; crypto_digest_t *digest; @@ -841,7 +841,8 @@ hs_get_subcredential(const ed25519_public_key_t *identity_pk, sizeof(credential)); crypto_digest_add_bytes(digest, (const char *) blinded_pk->pubkey, ED25519_PUBKEY_LEN); - crypto_digest_get_digest(digest, (char *) subcred_out, DIGEST256_LEN); + crypto_digest_get_digest(digest, (char *) subcred_out->subcred, + SUBCRED_LEN); crypto_digest_free(digest); memwipe(credential, 0, sizeof(credential)); diff --git a/src/feature/hs/hs_common.h b/src/feature/hs/hs_common.h index 2bcf0f67c4..997b7298a6 100644 --- a/src/feature/hs/hs_common.h +++ b/src/feature/hs/hs_common.h @@ -214,9 +214,10 @@ const uint8_t *rend_data_get_pk_digest(const rend_data_t *rend_data, routerstatus_t *pick_hsdir(const char *desc_id, const char *desc_id_base32); +struct hs_subcredential_t; void hs_get_subcredential(const struct ed25519_public_key_t *identity_pk, const struct ed25519_public_key_t *blinded_pk, - uint8_t *subcred_out); + struct hs_subcredential_t *subcred_out); uint64_t hs_get_previous_time_period_num(time_t now); uint64_t hs_get_time_period_num(time_t now); diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index 65d6c7a581..7f28e1c341 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -211,7 +211,7 @@ build_secret_input(const hs_descriptor_t *desc, memcpy(secret_input, secret_data, secret_data_len); offset += secret_data_len; /* Copy subcredential. */ - memcpy(secret_input + offset, desc->subcredential, DIGEST256_LEN); + memcpy(secret_input + offset, desc->subcredential.subcred, DIGEST256_LEN); offset += DIGEST256_LEN; /* Copy revision counter value. */ set_uint64(secret_input + offset, @@ -1018,9 +1018,11 @@ desc_encode_v3(const hs_descriptor_t *desc, tor_assert(encoded_out); tor_assert(desc->plaintext_data.version == 3); + /* This is impossible; this is a member of desc. if (BUG(desc->subcredential == NULL)) { goto err; } + */ /* Build the non-encrypted values. */ { @@ -1366,8 +1368,7 @@ encrypted_data_length_is_valid(size_t len) * and return the buffer's length. The caller should wipe and free its content * once done with it. This function can't fail. */ static size_t -build_descriptor_cookie_keys(const uint8_t *subcredential, - size_t subcredential_len, +build_descriptor_cookie_keys(const hs_subcredential_t *subcredential, const curve25519_secret_key_t *sk, const curve25519_public_key_t *pk, uint8_t **keys_out) @@ -1389,7 +1390,7 @@ build_descriptor_cookie_keys(const uint8_t *subcredential, /* Calculate KEYS = KDF(subcredential | SECRET_SEED, 40) */ xof = crypto_xof_new(); - crypto_xof_add_bytes(xof, subcredential, subcredential_len); + crypto_xof_add_bytes(xof, subcredential->subcred, SUBCRED_LEN); crypto_xof_add_bytes(xof, secret_seed, sizeof(secret_seed)); crypto_xof_squeeze_bytes(xof, keystream, keystream_len); crypto_xof_free(xof); @@ -1426,11 +1427,12 @@ decrypt_descriptor_cookie(const hs_descriptor_t *desc, sizeof(desc->superencrypted_data.auth_ephemeral_pubkey))); tor_assert(!fast_mem_is_zero((char *) client_auth_sk, sizeof(*client_auth_sk))); - tor_assert(!fast_mem_is_zero((char *) desc->subcredential, DIGEST256_LEN)); + tor_assert(!fast_mem_is_zero((char *) desc->subcredential.subcred, + DIGEST256_LEN)); /* Get the KEYS component to derive the CLIENT-ID and COOKIE-KEY. */ keystream_length = - build_descriptor_cookie_keys(desc->subcredential, DIGEST256_LEN, + build_descriptor_cookie_keys(&desc->subcredential, client_auth_sk, &desc->superencrypted_data.auth_ephemeral_pubkey, &keystream); @@ -2558,7 +2560,7 @@ hs_desc_decode_plaintext(const char *encoded, * set to NULL. */ hs_desc_decode_status_t hs_desc_decode_descriptor(const char *encoded, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const curve25519_secret_key_t *client_auth_sk, hs_descriptor_t **desc_out) { @@ -2576,7 +2578,7 @@ hs_desc_decode_descriptor(const char *encoded, goto err; } - memcpy(desc->subcredential, subcredential, sizeof(desc->subcredential)); + memcpy(&desc->subcredential, subcredential, sizeof(desc->subcredential)); ret = hs_desc_decode_plaintext(encoded, &desc->plaintext_data); if (ret != HS_DESC_DECODE_OK) { @@ -2666,7 +2668,7 @@ hs_desc_encode_descriptor,(const hs_descriptor_t *desc, * symmetric only if the client auth is disabled. That is, the descriptor * cookie will be NULL. */ if (!descriptor_cookie) { - ret = hs_desc_decode_descriptor(*encoded_out, desc->subcredential, + ret = hs_desc_decode_descriptor(*encoded_out, &desc->subcredential, NULL, NULL); if (BUG(ret != HS_DESC_DECODE_OK)) { ret = -1; @@ -2870,7 +2872,7 @@ hs_desc_build_fake_authorized_client(void) * key, and descriptor cookie, build the auth client so we can then encode the * descriptor for publication. client_out must be already allocated. */ void -hs_desc_build_authorized_client(const uint8_t *subcredential, +hs_desc_build_authorized_client(const hs_subcredential_t *subcredential, const curve25519_public_key_t *client_auth_pk, const curve25519_secret_key_t * auth_ephemeral_sk, @@ -2898,7 +2900,7 @@ hs_desc_build_authorized_client(const uint8_t *subcredential, /* Get the KEYS part so we can derive the CLIENT-ID and COOKIE-KEY. */ keystream_length = - build_descriptor_cookie_keys(subcredential, DIGEST256_LEN, + build_descriptor_cookie_keys(subcredential, auth_ephemeral_sk, client_auth_pk, &keystream); tor_assert(keystream_length > 0); diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h index 639dd31c8f..08daa904b6 100644 --- a/src/feature/hs/hs_descriptor.h +++ b/src/feature/hs/hs_descriptor.h @@ -14,6 +14,7 @@ #include "core/or/or.h" #include "trunnel/ed25519_cert.h" /* needed for trunnel */ #include "feature/nodelist/torcert.h" +#include "core/crypto/hs_ntor.h" /* for hs_subcredential_t */ /* Trunnel */ struct link_specifier_t; @@ -238,7 +239,7 @@ typedef struct hs_descriptor_t { /** Subcredentials of a service, used by the client and service to decrypt * the encrypted data. */ - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; } hs_descriptor_t; /** Return true iff the given descriptor format version is supported. */ @@ -277,7 +278,7 @@ MOCK_DECL(int, char **encoded_out)); int hs_desc_decode_descriptor(const char *encoded, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const curve25519_secret_key_t *client_auth_sk, hs_descriptor_t **desc_out); int hs_desc_decode_plaintext(const char *encoded, @@ -302,7 +303,7 @@ void hs_desc_authorized_client_free_(hs_desc_authorized_client_t *client); hs_desc_authorized_client_t *hs_desc_build_fake_authorized_client(void); -void hs_desc_build_authorized_client(const uint8_t *subcredential, +void hs_desc_build_authorized_client(const hs_subcredential_t *subcredential, const curve25519_public_key_t * client_auth_pk, const curve25519_secret_key_t * diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 62db3bd434..ee54595f26 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -178,7 +178,7 @@ ob_option_parse(hs_service_config_t *config, const ob_options_t *opts) * least DIGEST256_LEN in size. */ static void build_subcredential(const ed25519_public_key_t *pkey, uint64_t tp, - uint8_t *subcredential) + hs_subcredential_t *subcredential) { ed25519_public_key_t blinded_pubkey; @@ -275,13 +275,12 @@ hs_ob_parse_config_file(hs_service_config_t *config) * Otherwise, this can't fail. */ size_t hs_ob_get_subcredentials(const hs_service_config_t *config, - uint8_t **subcredentials) + hs_subcredential_t **subcredentials) { unsigned int num_pkeys, idx = 0; - uint8_t *subcreds = NULL; + hs_subcredential_t *subcreds = NULL; const int steps[3] = {0, -1, 1}; const unsigned int num_steps = ARRAY_LENGTH(steps); - const size_t subcred_len = DIGEST256_LEN; const uint64_t tp = hs_get_time_period_num(0); tor_assert(config); @@ -319,14 +318,13 @@ hs_ob_get_subcredentials(const hs_service_config_t *config, * number of time period we need to compute and finally multiplied by the * total number of keys we are about to process. In other words, for each * key, we allocate 3 subcredential slots. */ - subcreds = tor_malloc_zero(subcred_len * num_steps * num_pkeys); + subcreds = tor_calloc(num_steps * num_pkeys, sizeof(hs_subcredential_t)); /* For each time period step. */ for (unsigned int i = 0; i < num_steps; i++) { SMARTLIST_FOREACH_BEGIN(config->ob_master_pubkeys, const ed25519_public_key_t *, pkey) { - build_subcredential(pkey, tp + steps[i], - &(subcreds[idx * subcred_len])); + build_subcredential(pkey, tp + steps[i], &subcreds[idx]); idx++; } SMARTLIST_FOREACH_END(pkey); } diff --git a/src/feature/hs/hs_ob.h b/src/feature/hs/hs_ob.h index fea6a737d7..37e94808c2 100644 --- a/src/feature/hs/hs_ob.h +++ b/src/feature/hs/hs_ob.h @@ -15,8 +15,9 @@ 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, - uint8_t **subcredentials); + struct hs_subcredential_t **subcredentials); #ifdef HS_OB_PRIVATE diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index d0d9d65790..87b9625f54 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -1769,7 +1769,8 @@ build_service_desc_superencrypted(const hs_service_t *service, sizeof(curve25519_public_key_t)); /* Test that subcred is not zero because we might use it below */ - if (BUG(fast_mem_is_zero((char*)desc->desc->subcredential, DIGEST256_LEN))) { + if (BUG(fast_mem_is_zero((char*)desc->desc->subcredential.subcred, + DIGEST256_LEN))) { return -1; } @@ -1786,7 +1787,7 @@ build_service_desc_superencrypted(const hs_service_t *service, /* Prepare the client for descriptor and then add to the list in the * superencrypted part of the descriptor */ - hs_desc_build_authorized_client(desc->desc->subcredential, + hs_desc_build_authorized_client(&desc->desc->subcredential, &client->client_pk, &desc->auth_ephemeral_kp.seckey, desc->descriptor_cookie, desc_client); @@ -1842,7 +1843,7 @@ build_service_desc_plaintext(const hs_service_t *service, /* Set the subcredential. */ hs_get_subcredential(&service->keys.identity_pk, &desc->blinded_kp.pubkey, - desc->desc->subcredential); + &desc->desc->subcredential); plaintext = &desc->desc->plaintext_data; @@ -3374,7 +3375,7 @@ service_handle_introduce2(origin_circuit_t *circ, const uint8_t *payload, /* The following will parse, decode and launch the rendezvous point circuit. * Both current and legacy cells are handled. */ - if (hs_circ_handle_introduce2(service, circ, ip, desc->desc->subcredential, + if (hs_circ_handle_introduce2(service, circ, ip, &desc->desc->subcredential, payload, payload_len) < 0) { goto err; } diff --git a/src/test/fuzz/fuzz_hsdescv3.c b/src/test/fuzz/fuzz_hsdescv3.c index 3955241389..8d7eab1a8d 100644 --- a/src/test/fuzz/fuzz_hsdescv3.c +++ b/src/test/fuzz/fuzz_hsdescv3.c @@ -85,12 +85,12 @@ int fuzz_main(const uint8_t *data, size_t sz) { hs_descriptor_t *desc = NULL; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; char *fuzzing_data = tor_memdup_nulterm(data, sz); - memset(subcredential, 'A', sizeof(subcredential)); + memset(&subcredential, 'A', sizeof(subcredential)); - hs_desc_decode_descriptor(fuzzing_data, subcredential, NULL, &desc); + hs_desc_decode_descriptor(fuzzing_data, &subcredential, NULL, &desc); if (desc) { log_debug(LD_GENERAL, "Decoding okay"); hs_descriptor_free(desc); @@ -101,4 +101,3 @@ fuzz_main(const uint8_t *data, size_t sz) tor_free(fuzzing_data); return 0; } - diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index e8b99aaac8..eb4af1c9f7 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -140,7 +140,7 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, desc->plaintext_data.lifetime_sec = 3 * 60 * 60; hs_get_subcredential(&signing_kp->pubkey, &blinded_kp.pubkey, - desc->subcredential); + &desc->subcredential); /* Setup superencrypted data section. */ ret = curve25519_keypair_generate(&auth_ephemeral_kp, 0); @@ -186,7 +186,7 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, * an HS. Used to decrypt descriptors in unittests. */ void hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp, - uint8_t *subcred_out) + hs_subcredential_t *subcred_out) { ed25519_keypair_t blinded_kp; uint64_t current_time_period = hs_get_time_period_num(approx_time()); @@ -233,7 +233,7 @@ hs_helper_build_hs_desc_with_client_auth( memcpy(&desc->superencrypted_data.auth_ephemeral_pubkey, &auth_ephemeral_kp.pubkey, sizeof(curve25519_public_key_t)); - hs_desc_build_authorized_client(desc->subcredential, client_pk, + hs_desc_build_authorized_client(&desc->subcredential, client_pk, &auth_ephemeral_kp.seckey, descriptor_cookie, desc_client); smartlist_add(desc->superencrypted_data.clients, desc_client); diff --git a/src/test/hs_test_helpers.h b/src/test/hs_test_helpers.h index a01fd45d63..cad4532826 100644 --- a/src/test/hs_test_helpers.h +++ b/src/test/hs_test_helpers.h @@ -21,12 +21,11 @@ hs_descriptor_t *hs_helper_build_hs_desc_with_client_auth( const ed25519_keypair_t *signing_kp); void hs_helper_desc_equal(const hs_descriptor_t *desc1, const hs_descriptor_t *desc2); -void -hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp, - uint8_t *subcred_out); +struct hs_subcredential_t; +void hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp, + struct hs_subcredential_t *subcred_out); void hs_helper_add_client_auth(const ed25519_public_key_t *service_pk, const curve25519_secret_key_t *client_sk); #endif /* !defined(TOR_HS_TEST_HELPERS_H) */ - diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 9e0094d250..c1bff6eb7f 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -370,7 +370,7 @@ test_hsdir_revision_counter_check(void *arg) hs_descriptor_t *published_desc = NULL; char *published_desc_str = NULL; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; char *received_desc_str = NULL; hs_descriptor_t *received_desc = NULL; @@ -407,11 +407,11 @@ test_hsdir_revision_counter_check(void *arg) const ed25519_public_key_t *blinded_key; blinded_key = &published_desc->plaintext_data.blinded_pubkey; - hs_get_subcredential(&signing_kp.pubkey, blinded_key, subcredential); + hs_get_subcredential(&signing_kp.pubkey, blinded_key, &subcredential); received_desc_str = helper_fetch_desc_from_hsdir(blinded_key); retval = hs_desc_decode_descriptor(received_desc_str, - subcredential, NULL, &received_desc); + &subcredential, NULL, &received_desc); tt_int_op(retval, OP_EQ, HS_DESC_DECODE_OK); tt_assert(received_desc); @@ -444,7 +444,7 @@ test_hsdir_revision_counter_check(void *arg) received_desc_str = helper_fetch_desc_from_hsdir(blinded_key); retval = hs_desc_decode_descriptor(received_desc_str, - subcredential, NULL, &received_desc); + &subcredential, NULL, &received_desc); tt_int_op(retval, OP_EQ, HS_DESC_DECODE_OK); tt_assert(received_desc); @@ -476,7 +476,7 @@ test_client_cache(void *arg) ed25519_keypair_t signing_kp; hs_descriptor_t *published_desc = NULL; char *published_desc_str = NULL; - uint8_t wanted_subcredential[DIGEST256_LEN]; + hs_subcredential_t wanted_subcredential; response_handler_args_t *args = NULL; dir_connection_t *conn = NULL; @@ -505,8 +505,10 @@ test_client_cache(void *arg) retval = hs_desc_encode_descriptor(published_desc, &signing_kp, NULL, &published_desc_str); tt_int_op(retval, OP_EQ, 0); - memcpy(wanted_subcredential, published_desc->subcredential, DIGEST256_LEN); - tt_assert(!fast_mem_is_zero((char*)wanted_subcredential, DIGEST256_LEN)); + memcpy(&wanted_subcredential, &published_desc->subcredential, + sizeof(hs_subcredential_t)); + tt_assert(!fast_mem_is_zero((char*)wanted_subcredential.subcred, + DIGEST256_LEN)); } /* Test handle_response_fetch_hsdesc_v3() */ @@ -540,8 +542,9 @@ test_client_cache(void *arg) const hs_descriptor_t *cached_desc = NULL; cached_desc = hs_cache_lookup_as_client(&signing_kp.pubkey); tt_assert(cached_desc); - tt_mem_op(cached_desc->subcredential, OP_EQ, wanted_subcredential, - DIGEST256_LEN); + tt_mem_op(cached_desc->subcredential.subcred, + OP_EQ, wanted_subcredential.subcred, + SUBCRED_LEN); } /* Progress time to next TP and check that desc was cleaned */ diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 9f09cc3ecb..88910e8ead 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -416,9 +416,10 @@ test_client_pick_intro(void *arg) const hs_descriptor_t *fetched_desc = hs_cache_lookup_as_client(&service_kp.pubkey); tt_assert(fetched_desc); - tt_mem_op(fetched_desc->subcredential, OP_EQ, desc->subcredential, - DIGEST256_LEN); - tt_assert(!fast_mem_is_zero((char*)fetched_desc->subcredential, + tt_mem_op(fetched_desc->subcredential.subcred, + OP_EQ, desc->subcredential.subcred, + SUBCRED_LEN); + tt_assert(!fast_mem_is_zero((char*)fetched_desc->subcredential.subcred, DIGEST256_LEN)); tor_free(encoded); } diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index 43ac5490a1..61ccd3f919 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -221,7 +221,7 @@ test_decode_descriptor(void *arg) hs_descriptor_t *desc = NULL; hs_descriptor_t *decoded = NULL; hs_descriptor_t *desc_no_ip = NULL; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; (void) arg; @@ -230,10 +230,10 @@ test_decode_descriptor(void *arg) desc = hs_helper_build_hs_desc_with_ip(&signing_kp); hs_helper_get_subcred_from_identity_keypair(&signing_kp, - subcredential); + &subcredential); /* Give some bad stuff to the decoding function. */ - ret = hs_desc_decode_descriptor("hladfjlkjadf", subcredential, + ret = hs_desc_decode_descriptor("hladfjlkjadf", &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_PLAINTEXT_ERROR); @@ -241,7 +241,7 @@ test_decode_descriptor(void *arg) tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(encoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, NULL, &decoded); + ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(decoded); @@ -253,7 +253,7 @@ test_decode_descriptor(void *arg) ret = ed25519_keypair_generate(&signing_kp_no_ip, 0); tt_int_op(ret, OP_EQ, 0); hs_helper_get_subcred_from_identity_keypair(&signing_kp_no_ip, - subcredential); + &subcredential); desc_no_ip = hs_helper_build_hs_desc_no_ip(&signing_kp_no_ip); tt_assert(desc_no_ip); tor_free(encoded); @@ -262,7 +262,7 @@ test_decode_descriptor(void *arg) tt_int_op(ret, OP_EQ, 0); tt_assert(encoded); hs_descriptor_free(decoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, NULL, &decoded); + ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(decoded); } @@ -286,14 +286,14 @@ test_decode_descriptor(void *arg) &auth_ephemeral_kp.pubkey, CURVE25519_PUBKEY_LEN); hs_helper_get_subcred_from_identity_keypair(&signing_kp, - subcredential); + &subcredential); /* Build and add the auth client to the descriptor. */ clients = desc->superencrypted_data.clients; if (!clients) { clients = smartlist_new(); } - hs_desc_build_authorized_client(subcredential, + hs_desc_build_authorized_client(&subcredential, &client_kp.pubkey, &auth_ephemeral_kp.seckey, descriptor_cookie, client); @@ -315,21 +315,21 @@ test_decode_descriptor(void *arg) /* If we do not have the client secret key, the decoding must fail. */ hs_descriptor_free(decoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, + ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_NEED_CLIENT_AUTH); tt_assert(!decoded); /* If we have an invalid client secret key, the decoding must fail. */ hs_descriptor_free(decoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, + ret = hs_desc_decode_descriptor(encoded, &subcredential, &invalid_client_kp.seckey, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_BAD_CLIENT_AUTH); tt_assert(!decoded); /* If we have the client secret key, the decoding must succeed and the * decoded descriptor must be correct. */ - ret = hs_desc_decode_descriptor(encoded, subcredential, + ret = hs_desc_decode_descriptor(encoded, &subcredential, &client_kp.seckey, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(decoded); @@ -762,7 +762,7 @@ test_build_authorized_client(void *arg) "07d087f1d8c68393721f6e70316d3b29"; const char client_pubkey_b16[] = "8c1298fa6050e372f8598f6deca32e27b0ad457741422c2629ebb132cf7fae37"; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; char *mem_op_hex_tmp=NULL; (void) arg; @@ -774,7 +774,7 @@ test_build_authorized_client(void *arg) tt_int_op(ret, OP_EQ, 0); curve25519_public_key_generate(&client_auth_pk, &client_auth_sk); - memset(subcredential, 42, sizeof(subcredential)); + memset(subcredential.subcred, 42, sizeof(subcredential)); desc_client = tor_malloc_zero(sizeof(hs_desc_authorized_client_t)); @@ -795,7 +795,7 @@ test_build_authorized_client(void *arg) testing_enable_prefilled_rng("\x01", 1); - hs_desc_build_authorized_client(subcredential, + hs_desc_build_authorized_client(&subcredential, &client_auth_pk, &auth_ephemeral_sk, descriptor_cookie, desc_client); diff --git a/src/test/test_hs_ntor.c b/src/test/test_hs_ntor.c index 4f98bc85dc..7867740a1a 100644 --- a/src/test/test_hs_ntor.c +++ b/src/test/test_hs_ntor.c @@ -23,7 +23,7 @@ test_hs_ntor(void *arg) { int retval; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; ed25519_keypair_t service_intro_auth_keypair; curve25519_keypair_t service_intro_enc_keypair; @@ -42,7 +42,7 @@ test_hs_ntor(void *arg) /* Generate fake data for this unittest */ { /* Generate fake subcredential */ - memset(subcredential, 'Z', DIGEST256_LEN); + memset(subcredential.subcred, 'Z', DIGEST256_LEN); /* service */ curve25519_keypair_generate(&service_intro_enc_keypair, 0); @@ -57,7 +57,7 @@ test_hs_ntor(void *arg) hs_ntor_client_get_introduce1_keys(&service_intro_auth_keypair.pubkey, &service_intro_enc_keypair.pubkey, &client_ephemeral_enc_keypair, - subcredential, + &subcredential, &client_hs_ntor_intro_cell_keys); tt_int_op(retval, OP_EQ, 0); @@ -66,7 +66,7 @@ test_hs_ntor(void *arg) hs_ntor_service_get_introduce1_keys(&service_intro_auth_keypair.pubkey, &service_intro_enc_keypair, &client_ephemeral_enc_keypair.pubkey, - subcredential, + &subcredential, &service_hs_ntor_intro_cell_keys); tt_int_op(retval, OP_EQ, 0); diff --git a/src/test/test_hs_ntor_cl.c b/src/test/test_hs_ntor_cl.c index a7cebc6af4..3acd7ef0bc 100644 --- a/src/test/test_hs_ntor_cl.c +++ b/src/test/test_hs_ntor_cl.c @@ -53,7 +53,7 @@ client1(int argc, char **argv) curve25519_public_key_t intro_enc_pubkey; ed25519_public_key_t intro_auth_pubkey; curve25519_keypair_t client_ephemeral_enc_keypair; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; /* Output */ hs_ntor_intro_cell_keys_t hs_ntor_intro_cell_keys; @@ -65,7 +65,7 @@ client1(int argc, char **argv) BASE16(3, intro_enc_pubkey.public_key, CURVE25519_PUBKEY_LEN); BASE16(4, client_ephemeral_enc_keypair.seckey.secret_key, CURVE25519_SECKEY_LEN); - BASE16(5, subcredential, DIGEST256_LEN); + BASE16(5, subcredential.subcred, DIGEST256_LEN); /* Generate keypair */ curve25519_public_key_generate(&client_ephemeral_enc_keypair.pubkey, @@ -74,7 +74,7 @@ client1(int argc, char **argv) retval = hs_ntor_client_get_introduce1_keys(&intro_auth_pubkey, &intro_enc_pubkey, &client_ephemeral_enc_keypair, - subcredential, + &subcredential, &hs_ntor_intro_cell_keys); if (retval < 0) { goto done; @@ -106,7 +106,7 @@ server1(int argc, char **argv) curve25519_keypair_t intro_enc_keypair; ed25519_public_key_t intro_auth_pubkey; curve25519_public_key_t client_ephemeral_enc_pubkey; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; /* Output */ hs_ntor_intro_cell_keys_t hs_ntor_intro_cell_keys; @@ -119,7 +119,7 @@ server1(int argc, char **argv) BASE16(2, intro_auth_pubkey.pubkey, ED25519_PUBKEY_LEN); BASE16(3, intro_enc_keypair.seckey.secret_key, CURVE25519_SECKEY_LEN); BASE16(4, client_ephemeral_enc_pubkey.public_key, CURVE25519_PUBKEY_LEN); - BASE16(5, subcredential, DIGEST256_LEN); + BASE16(5, subcredential.subcred, DIGEST256_LEN); /* Generate keypair */ curve25519_public_key_generate(&intro_enc_keypair.pubkey, @@ -130,7 +130,7 @@ server1(int argc, char **argv) retval = hs_ntor_service_get_introduce1_keys(&intro_auth_pubkey, &intro_enc_keypair, &client_ephemeral_enc_pubkey, - subcredential, + &subcredential, &hs_ntor_intro_cell_keys); if (retval < 0) { goto done; @@ -188,7 +188,7 @@ client2(int argc, char **argv) ed25519_public_key_t intro_auth_pubkey; curve25519_keypair_t client_ephemeral_enc_keypair; curve25519_public_key_t service_ephemeral_rend_pubkey; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; /* Output */ hs_ntor_rend_cell_keys_t hs_ntor_rend_cell_keys; @@ -201,7 +201,7 @@ client2(int argc, char **argv) CURVE25519_SECKEY_LEN); BASE16(4, intro_enc_pubkey.public_key, CURVE25519_PUBKEY_LEN); BASE16(5, service_ephemeral_rend_pubkey.public_key, CURVE25519_PUBKEY_LEN); - BASE16(6, subcredential, DIGEST256_LEN); + BASE16(6, subcredential.subcred, DIGEST256_LEN); /* Generate keypair */ curve25519_public_key_generate(&client_ephemeral_enc_keypair.pubkey, diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c index 37c57f795e..848a488be2 100644 --- a/src/test/test_hs_ob.c +++ b/src/test/test_hs_ob.c @@ -190,7 +190,7 @@ test_get_subcredentials(void *arg) ed25519_keypair_generate(&onion_addr_kp_1, 0); smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); - uint8_t *subcreds = NULL; + hs_subcredential_t *subcreds = NULL; size_t num = hs_ob_get_subcredentials(&config, &subcreds); tt_uint_op(num, OP_EQ, 3); @@ -200,14 +200,14 @@ test_get_subcredentials(void *arg) const int steps[3] = {0, -1, 1}; for (unsigned int i = 0; i < num; i++) { - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; ed25519_public_key_t blinded_pubkey; hs_build_blinded_pubkey(&onion_addr_kp_1.pubkey, NULL, 0, tp + steps[i], &blinded_pubkey); hs_get_subcredential(&onion_addr_kp_1.pubkey, &blinded_pubkey, - subcredential); - tt_mem_op(&(subcreds[i * sizeof(subcredential)]), OP_EQ, subcredential, - sizeof(subcredential)); + &subcredential); + tt_mem_op(subcreds[i].subcred, OP_EQ, subcredential.subcred, + SUBCRED_LEN); } done: @@ -228,4 +228,3 @@ struct testcase_t hs_ob_tests[] = { END_OF_TESTCASES }; - From bd0efb270219d02c75b8a6a1f7964cd986e2cdc6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 16 Jan 2020 18:28:25 -0500 Subject: [PATCH 10/22] Remove a dead BUG() check. --- src/feature/hs/hs_descriptor.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index 7f28e1c341..c274ed7581 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -1018,12 +1018,6 @@ desc_encode_v3(const hs_descriptor_t *desc, tor_assert(encoded_out); tor_assert(desc->plaintext_data.version == 3); - /* This is impossible; this is a member of desc. - if (BUG(desc->subcredential == NULL)) { - goto err; - } - */ - /* Build the non-encrypted values. */ { char *encoded_cert; From 46e6a4819aefb09b26924026833ead3eda533328 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 16 Jan 2020 18:40:50 -0500 Subject: [PATCH 11/22] Define a variant of hs_ntor that takes multiple subcredentials. --- src/core/crypto/hs_ntor.c | 36 +++++++++++++++++++++++++++++++----- src/core/crypto/hs_ntor.h | 8 ++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/core/crypto/hs_ntor.c b/src/core/crypto/hs_ntor.c index 0422e72795..4bd11ef98e 100644 --- a/src/core/crypto/hs_ntor.c +++ b/src/core/crypto/hs_ntor.c @@ -452,6 +452,28 @@ hs_ntor_service_get_introduce1_keys( const curve25519_public_key_t *client_ephemeral_enc_pubkey, const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) +{ + return hs_ntor_service_get_introduce1_keys_multi( + intro_auth_pubkey, + intro_enc_keypair, + client_ephemeral_enc_pubkey, + 1, + subcredential, + hs_ntor_intro_cell_keys_out); +} + +/** + * As hs_ntor_service_get_introduce1_keys(), but take multiple subcredentials + * as input, and yield multiple sets of keys as output. + **/ +int +hs_ntor_service_get_introduce1_keys_multi( + const struct ed25519_public_key_t *intro_auth_pubkey, + const struct curve25519_keypair_t *intro_enc_keypair, + const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, + int n_subcredentials, + const hs_subcredential_t *subcredentials, + hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { int bad = 0; uint8_t secret_input[INTRO_SECRET_HS_INPUT_LEN]; @@ -460,7 +482,8 @@ hs_ntor_service_get_introduce1_keys( tor_assert(intro_auth_pubkey); tor_assert(intro_enc_keypair); tor_assert(client_ephemeral_enc_pubkey); - tor_assert(subcredential); + tor_assert(n_subcredentials >= 1); + tor_assert(subcredentials); tor_assert(hs_ntor_intro_cell_keys_out); /* Compute EXP(X, b) */ @@ -476,13 +499,16 @@ hs_ntor_service_get_introduce1_keys( secret_input); bad |= safe_mem_is_zero(secret_input, CURVE25519_OUTPUT_LEN); - /* Get ENC_KEY and MAC_KEY! */ - get_introduce1_key_material(secret_input, subcredential, - hs_ntor_intro_cell_keys_out); + for (int i = 0; i < n_subcredentials; ++i) { + /* Get ENC_KEY and MAC_KEY! */ + get_introduce1_key_material(secret_input, &subcredentials[i], + &hs_ntor_intro_cell_keys_out[i]); + } memwipe(secret_input, 0, sizeof(secret_input)); if (bad) { - memwipe(hs_ntor_intro_cell_keys_out, 0, sizeof(hs_ntor_intro_cell_keys_t)); + memwipe(hs_ntor_intro_cell_keys_out, 0, + sizeof(hs_ntor_intro_cell_keys_t) * n_subcredentials); } return bad ? -1 : 0; diff --git a/src/core/crypto/hs_ntor.h b/src/core/crypto/hs_ntor.h index b78bc4e802..2ed357f02d 100644 --- a/src/core/crypto/hs_ntor.h +++ b/src/core/crypto/hs_ntor.h @@ -58,6 +58,14 @@ int hs_ntor_client_get_rendezvous1_keys( const struct curve25519_public_key_t *service_ephemeral_rend_pubkey, hs_ntor_rend_cell_keys_t *hs_ntor_rend_cell_keys_out); +int hs_ntor_service_get_introduce1_keys_multi( + const struct ed25519_public_key_t *intro_auth_pubkey, + const struct curve25519_keypair_t *intro_enc_keypair, + const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, + int n_subcredentials, + const hs_subcredential_t *subcredentials, + hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out); + int hs_ntor_service_get_introduce1_keys( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_keypair_t *intro_enc_keypair, From b6250236a2427b116c819be3305727fcbefdb424 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 16 Jan 2020 19:10:26 -0500 Subject: [PATCH 12/22] Pass multiple subcredentials all the way down to hs_ntor. This approach saves us a pair of curve25519 operations for every subcredential but the first. It is not yet constant-time. I've noted a few places where IMO we should refactor the code so that the complete list of subcredentials is passed in earlier. --- src/feature/hs/hs_cell.c | 91 ++++++++++++++++++++++++------------- src/feature/hs/hs_cell.h | 11 +++-- src/feature/hs/hs_circuit.c | 5 +- 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index d377e9134d..2b83453838 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -68,14 +68,17 @@ compute_introduce_mac(const uint8_t *encoded_cell, size_t encoded_cell_len, memwipe(mac_msg, 0, sizeof(mac_msg)); } -/** From a set of keys, subcredential and the ENCRYPTED section of an - * INTRODUCE2 cell, return a newly allocated intro cell keys structure. - * Finally, the client public key is copied in client_pk. On error, return - * NULL. */ +/** + * From a set of keys, a list of subcredentials, and the ENCRYPTED section of + * an INTRODUCE2 cell, return an array of newly allocated intro cell keys + * structures. Finally, the client public key is copied in client_pk. On + * error, return NULL. + **/ static hs_ntor_intro_cell_keys_t * get_introduce2_key_material(const ed25519_public_key_t *auth_key, const curve25519_keypair_t *enc_key, - const hs_subcredential_t *subcredential, + int n_subcredentials, + const hs_subcredential_t *subcredentials, const uint8_t *encrypted_section, curve25519_public_key_t *client_pk) { @@ -83,17 +86,19 @@ get_introduce2_key_material(const ed25519_public_key_t *auth_key, tor_assert(auth_key); tor_assert(enc_key); - tor_assert(subcredential); + tor_assert(n_subcredentials > 0); + tor_assert(subcredentials); tor_assert(encrypted_section); tor_assert(client_pk); - keys = tor_malloc_zero(sizeof(*keys)); + keys = tor_calloc(n_subcredentials, sizeof(hs_ntor_intro_cell_keys_t)); /* First bytes of the ENCRYPTED section are the client public key. */ memcpy(client_pk->public_key, encrypted_section, CURVE25519_PUBKEY_LEN); - if (hs_ntor_service_get_introduce1_keys(auth_key, enc_key, client_pk, - subcredential, keys) < 0) { + if (hs_ntor_service_get_introduce1_keys_multi(auth_key, enc_key, client_pk, + n_subcredentials, + subcredentials, keys) < 0) { /* Don't rely on the caller to wipe this on error. */ memwipe(client_pk, 0, sizeof(curve25519_public_key_t)); tor_free(keys); @@ -760,10 +765,12 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, size_t encrypted_section_len) { hs_ntor_intro_cell_keys_t *intro_keys = NULL; + hs_ntor_intro_cell_keys_t *intro_keys_result = NULL; /* Build the key material out of the key material found in the cell. */ intro_keys = get_introduce2_key_material(data->auth_pk, data->enc_kp, - data->subcredential, + data->n_subcredentials, + data->subcredentials, encrypted_section, &data->client_pk); if (intro_keys == NULL) { @@ -774,10 +781,11 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, /* Validate MAC from the cell and our computed key material. The MAC field * in the cell is at the end of the encrypted section. */ - { + int found_idx = -1; + for (int i = 0; i < data->n_subcredentials; ++i) { uint8_t mac[DIGEST256_LEN]; - /* Make sure we are now about to underflow. */ + /* Make sure we are not about to underflow. */ if (encrypted_section_len < sizeof(mac)) { goto err; } @@ -789,24 +797,39 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, compute_introduce_mac(data->payload, data->payload_len - encrypted_section_len, encrypted_section, encrypted_section_len, - intro_keys->mac_key, sizeof(intro_keys->mac_key), + intro_keys[i].mac_key, + sizeof(intro_keys[i].mac_key), mac, sizeof(mac)); - if (tor_memneq(mac, encrypted_section + mac_offset, sizeof(mac))) { - log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); - goto err; + if (tor_memeq(mac, encrypted_section + mac_offset, sizeof(mac))) { + found_idx = i; + break; } } - goto done; + if (found_idx == -1) { + log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); + goto err; + } + + /* We found a match! */ + if (data->n_subcredentials == 1) { + /* There was only one; steal it. */ + intro_keys_result = intro_keys; + intro_keys = NULL; + } else { + /* Copy out the one we wanted. */ + intro_keys_result = tor_memdup(&intro_keys[found_idx], + sizeof(hs_ntor_intro_cell_keys_t)); + } err: if (intro_keys) { - memwipe(intro_keys, 0, sizeof(hs_ntor_intro_cell_keys_t)); + memwipe(intro_keys, 0, + sizeof(hs_ntor_intro_cell_keys_t) * data->n_subcredentials); tor_free(intro_keys); } - done: - return intro_keys; + return intro_keys_result; } /** Return the newly allocated intro keys using the given service @@ -831,18 +854,22 @@ get_intro2_keys_as_ob(const hs_service_config_t *config, goto end; } - for (size_t idx = 0; idx < ob_num_subcreds; idx++) { - /* 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.subcredential = &ob_subcreds[idx]; - intro_keys = get_introduce2_keys_and_verify_mac(&new_data, - encrypted_section, - encrypted_section_len); - if (intro_keys) { - /* It validates. We have a hit as an onion balance instance. */ - 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; + /* 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; + + 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: diff --git a/src/feature/hs/hs_cell.h b/src/feature/hs/hs_cell.h index 58cc401cc0..cc2e7b5817 100644 --- a/src/feature/hs/hs_cell.h +++ b/src/feature/hs/hs_cell.h @@ -57,9 +57,14 @@ typedef struct hs_cell_introduce2_data_t { owned by the introduction point object through which we received the INTRO2 cell*/ const curve25519_keypair_t *enc_kp; - /** Subcredentials of the service. Pointer owned by the descriptor that owns - the introduction point through which we received the INTRO2 cell. */ - const struct hs_subcredential_t *subcredential; + /** + * Length of the subcredentials array below. + **/ + int n_subcredentials; + /** Array of n_subcredentials subcredentials for the service. Pointer + * owned by the descriptor that owns the introduction point through which we + * received the INTRO2 cell. */ + const struct hs_subcredential_t *subcredentials; /** Payload of the received encoded cell. */ const uint8_t *payload; /** Size of the payload of the received encoded cell. */ diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index fb3694b2d4..48a92962f4 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -983,7 +983,10 @@ 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; - data.subcredential = subcredential; + // 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(); From 4269ab97c685cb3bef9607cbada8af5b6b84640c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 16 Jan 2020 19:23:20 -0500 Subject: [PATCH 13/22] Add a function to maybe memcpy() a value, in constant time. --- src/lib/ctime/di_ops.c | 27 +++++++++++++++++++++++++++ src/lib/ctime/di_ops.h | 2 ++ src/test/test_util.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/lib/ctime/di_ops.c b/src/lib/ctime/di_ops.c index 7448a9973e..0859dd8bee 100644 --- a/src/lib/ctime/di_ops.c +++ b/src/lib/ctime/di_ops.c @@ -279,3 +279,30 @@ select_array_member_cumulative_timei(const uint64_t *entries, int n_entries, return i_chosen; } + +/** + * If s is true, then copy n bytes from src to + * dest. Otherwise leave dest alone. + * + * This function behaves the same as + * + * if (s) + * memcpy(dest, src, n); + * + * except that it tries to run in the same amount of time whether s is + * true or not. + **/ +void +memcpy_if_true_timei(bool s, void *dest, const void *src, size_t n) +{ + // If s is true, mask will be ~0. If s is false, mask will be 0. + const char mask = (char) -(signed char)s; + + char *destp = dest; + const char *srcp = src; + for (size_t i = 0; i < n; ++i) { + *destp = (*destp & ~mask) | (*srcp & mask); + ++destp; + ++srcp; + } +} diff --git a/src/lib/ctime/di_ops.h b/src/lib/ctime/di_ops.h index 4ff8f03165..9fe2884ecc 100644 --- a/src/lib/ctime/di_ops.h +++ b/src/lib/ctime/di_ops.h @@ -73,4 +73,6 @@ int select_array_member_cumulative_timei(const uint64_t *entries, int n_entries, uint64_t total, uint64_t rand_val); +void memcpy_if_true_timei(bool s, void *dest, const void *src, size_t n); + #endif /* !defined(TOR_DI_OPS_H) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 07c31f02d6..7a375b06be 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4571,6 +4571,35 @@ test_util_di_ops(void *arg) ; } +static void +test_util_memcpy_iftrue_timei(void *arg) +{ + (void)arg; + char buf1[25]; + char buf2[25]; + char buf3[25]; + + for (int i = 0; i < 100; ++i) { + crypto_rand(buf1, sizeof(buf1)); + crypto_rand(buf2, sizeof(buf2)); + memcpy(buf3, buf1, sizeof(buf1)); + + /* We just copied buf1 into buf3. Now we're going to copy buf2 into buf2, + iff our coin flip comes up heads. */ + bool coinflip = crypto_rand_int(2) == 0; + + memcpy_if_true_timei(coinflip, buf3, buf2, sizeof(buf3)); + + if (coinflip) { + tt_mem_op(buf3, OP_EQ, buf2, sizeof(buf2)); + } else { + tt_mem_op(buf3, OP_EQ, buf1, sizeof(buf1)); + } + } + done: + ; +} + static void test_util_di_map(void *arg) { @@ -6386,6 +6415,7 @@ struct testcase_t util_tests[] = { UTIL_LEGACY(path_is_relative), UTIL_LEGACY(strtok), UTIL_LEGACY(di_ops), + UTIL_TEST(memcpy_iftrue_timei, 0), UTIL_TEST(di_map, 0), UTIL_TEST(round_to_next_multiple_of, 0), UTIL_TEST(laplace, 0), From 942543253a30b8231c46eeaeb44f7ba174152113 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 16 Jan 2020 19:52:01 -0500 Subject: [PATCH 14/22] Use time-invariant conditional memcpy to make onionbalance loop safer --- src/feature/hs/hs_cell.c | 51 ++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 2b83453838..c82ffd647b 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -776,20 +776,20 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, if (intro_keys == NULL) { log_info(LD_REND, "Invalid INTRODUCE2 encrypted data. Unable to " "compute key material"); - goto err; + return NULL; + } + + /* Make sure we are not about to underflow. */ + if (BUG(encrypted_section_len < DIGEST256_LEN)) { + return NULL; } /* Validate MAC from the cell and our computed key material. The MAC field * in the cell is at the end of the encrypted section. */ - int found_idx = -1; + intro_keys_result = tor_malloc_zero(sizeof(*intro_keys_result)); for (int i = 0; i < data->n_subcredentials; ++i) { uint8_t mac[DIGEST256_LEN]; - /* Make sure we are not about to underflow. */ - if (encrypted_section_len < sizeof(mac)) { - goto err; - } - /* The MAC field is at the very end of the ENCRYPTED section. */ size_t mac_offset = encrypted_section_len - sizeof(mac); /* Compute the MAC. Use the entire encoded payload with a length up to the @@ -800,33 +800,22 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, intro_keys[i].mac_key, sizeof(intro_keys[i].mac_key), mac, sizeof(mac)); - if (tor_memeq(mac, encrypted_section + mac_offset, sizeof(mac))) { - found_idx = i; - break; - } + /* Time-invariant conditional copy: if the MAC is what we expected, then + * set intro_keys_result to intro_keys[i]. Otherwise, don't: but don't + * leak which one it was! */ + bool equal = tor_memeq(mac, encrypted_section + mac_offset, sizeof(mac)); + memcpy_if_true_timei(equal, intro_keys_result, &intro_keys[i], + sizeof(*intro_keys_result)); } - if (found_idx == -1) { + /* We no longer need intro_keys. */ + memwipe(intro_keys, 0, + sizeof(hs_ntor_intro_cell_keys_t) * data->n_subcredentials); + tor_free(intro_keys); + + if (safe_mem_is_zero(intro_keys_result, sizeof(*intro_keys_result))) { log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); - goto err; - } - - /* We found a match! */ - if (data->n_subcredentials == 1) { - /* There was only one; steal it. */ - intro_keys_result = intro_keys; - intro_keys = NULL; - } else { - /* Copy out the one we wanted. */ - intro_keys_result = tor_memdup(&intro_keys[found_idx], - sizeof(hs_ntor_intro_cell_keys_t)); - } - - err: - if (intro_keys) { - memwipe(intro_keys, 0, - sizeof(hs_ntor_intro_cell_keys_t) * data->n_subcredentials); - tor_free(intro_keys); + tor_free(intro_keys_result); /* sets intro_keys_result to NULL */ } return intro_keys_result; From da15feb0d358fe95394aed75fae672ad8459ceee Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 27 Jan 2020 17:06:36 +0200 Subject: [PATCH 15/22] 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 From c731988cb2ba2164d7557a95e3410c2e12f85bb8 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Wed, 22 Jan 2020 13:57:56 +0200 Subject: [PATCH 16/22] 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 4a9044c98f..fce6fe0220 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 48a92962f4..febeec4730 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 data and add the right subcredential to be able to handle + * this incoming cell. + * + * desc_subcred 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; } From 0133169481edd4094ec422da09bb68547bca4b50 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 27 Jan 2020 17:03:38 +0200 Subject: [PATCH 17/22] Allow clients to connect to the instance even with OB enabled. We do this by including the instance's subcredentials to the list of subcredentials that are used during INTRO2 decryption. --- src/feature/hs/hs_ob.c | 47 +++++++++++++++++++++++++++++++++--------- src/test/test_hs_ob.c | 27 +++++++++++++++++++++--- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 7552fbd16d..69fc51a8a0 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -284,16 +284,20 @@ compute_subcredentials(const hs_service_t *service, const unsigned int num_steps = ARRAY_LENGTH(steps); const uint64_t tp = hs_get_time_period_num(0); - tor_assert(config); + tor_assert(service); tor_assert(subcredentials); + /* Our caller has checked these too */ + tor_assert(service->desc_current); + tor_assert(service->desc_next); /* Our caller made sure that we are an OB instance */ - num_pkeys = smartlist_len(config->ob_master_pubkeys); + num_pkeys = smartlist_len(service->config.ob_master_pubkeys); 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 - * key in order to accomodate client and service consensus skew. + /* Time to build all the subcredentials for each time period: two for each + * instance descriptor plus three for the onionbalance frontend service: the + * previous one (-1), the current one (0) and the next one (1) for each + * configured key in order to accomodate client and service consensus skew. * * If the client consensus after_time is at 23:00 but the service one is at * 01:00, the client will be using the previous time period where the @@ -315,18 +319,30 @@ compute_subcredentials(const hs_service_t *service, * 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 * total number of keys we are about to process. In other words, for each - * key, we allocate 3 subcredential slots. */ - subcreds = tor_calloc(num_steps * num_pkeys, sizeof(hs_subcredential_t)); + * key, we allocate 3 subcredential slots. Then in the end we also add two + * subcredentials for this instance's active descriptors. */ + subcreds = + tor_calloc((num_steps * num_pkeys) + 2, sizeof(hs_subcredential_t)); - /* For each time period step. */ + /* For each master pubkey we add 3 subcredentials: */ for (unsigned int i = 0; i < num_steps; i++) { - SMARTLIST_FOREACH_BEGIN(config->ob_master_pubkeys, + SMARTLIST_FOREACH_BEGIN(service->config.ob_master_pubkeys, const ed25519_public_key_t *, pkey) { build_subcredential(pkey, tp + steps[i], &subcreds[idx]); idx++; } SMARTLIST_FOREACH_END(pkey); } + /* And then in the end we add the two subcredentials of the current active + * instance descriptors */ + memcpy(&subcreds[idx++], + service->desc_current->desc->subcredential.subcred, SUBCRED_LEN); + memcpy(&subcreds[idx++], + service->desc_next->desc->subcredential.subcred, SUBCRED_LEN); + + log_info(LD_REND, "Refreshing %u onionbalance keys (TP #%d).", + idx, (int)tp); + *subcredentials = subcreds; return idx; } @@ -344,7 +360,6 @@ compute_subcredentials(const hs_service_t *service, void hs_ob_refresh_keys(hs_service_t *service) { - const networkstatus_t *ns; hs_subcredential_t *ob_subcreds = NULL; size_t num_subcreds; @@ -355,6 +370,18 @@ hs_ob_refresh_keys(hs_service_t *service) return; } + /* We need both service descriptors created to make onionbalance keys. + * + * That's because we fetch our own (the instance's) subcredentials from our + * own descriptors which should always include the latest subcredentials that + * clients would use. + * + * This function is called with each descriptor build, so we will be + * eventually be called when both descriptors are created. */ + if (!service->desc_current || !service->desc_next) { + return; + } + /* Get a new set of subcreds */ num_subcreds = compute_subcredentials(service, &ob_subcreds); tor_assert(num_subcreds > 0); diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c index c2d62e354a..c4d9d239d2 100644 --- a/src/test/test_hs_ob.c +++ b/src/test/test_hs_ob.c @@ -191,16 +191,34 @@ test_get_subcredentials(void *arg) ed25519_keypair_generate(&onion_addr_kp_1, 0); smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); + /* Set up an instance */ + hs_service_t *service = tor_malloc_zero(sizeof(hs_service_t)); + service->config = config; + service->desc_current = service_descriptor_new(); + service->desc_next = service_descriptor_new(); + + /* Set up the instance subcredentials */ + char current_subcred[SUBCRED_LEN]; + char next_subcred[SUBCRED_LEN]; + memset(current_subcred, 'C', SUBCRED_LEN); + memset(next_subcred, 'N', SUBCRED_LEN); + memcpy(service->desc_current->desc->subcredential.subcred, current_subcred, + SUBCRED_LEN); + memcpy(service->desc_next->desc->subcredential.subcred, next_subcred, + SUBCRED_LEN); + hs_subcredential_t *subcreds = NULL; - size_t num = compute_subcredentials(&config, &subcreds); - tt_uint_op(num, OP_EQ, 3); + size_t num = compute_subcredentials(service, &subcreds); + /* 5 subcredentials: 3 for the frontend, 2 for the instance */ + tt_uint_op(num, OP_EQ, 5); /* Validate the subcredentials we just got. We'll build them oursevles with * the right time period steps and compare. */ const uint64_t tp = hs_get_time_period_num(0); const int steps[3] = {0, -1, 1}; - for (unsigned int i = 0; i < num; i++) { + unsigned int i; + for (i = 0; i < 3; i++) { hs_subcredential_t subcredential; ed25519_public_key_t blinded_pubkey; hs_build_blinded_pubkey(&onion_addr_kp_1.pubkey, NULL, 0, tp + steps[i], @@ -211,6 +229,9 @@ test_get_subcredentials(void *arg) SUBCRED_LEN); } + tt_mem_op(subcreds[i++].subcred, OP_EQ, current_subcred, SUBCRED_LEN); + tt_mem_op(subcreds[i++].subcred, OP_EQ, next_subcred, SUBCRED_LEN); + done: tor_free(subcreds); smartlist_free(config.ob_master_pubkeys); From 635f58bad23282e27fbc5833dbaae978dab25934 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Thu, 23 Jan 2020 00:31:29 +0200 Subject: [PATCH 18/22] Introduce an hs_ob_free_all() function. --- src/feature/hs/hs_common.c | 2 ++ src/feature/hs/hs_ob.c | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index b2b52c480e..4639cdb68a 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -22,6 +22,7 @@ #include "feature/hs/hs_client.h" #include "feature/hs/hs_common.h" #include "feature/hs/hs_dos.h" +#include "feature/hs/hs_ob.h" #include "feature/hs/hs_ident.h" #include "feature/hs/hs_service.h" #include "feature/hs_common/shared_random_client.h" @@ -1829,6 +1830,7 @@ hs_free_all(void) hs_service_free_all(); hs_cache_free_all(); hs_client_free_all(); + hs_ob_free_all(); } /** For the given origin circuit circ, decrement the number of rendezvous diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 69fc51a8a0..6a2e43f050 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -394,3 +394,10 @@ hs_ob_refresh_keys(hs_service_t *service) service->ob_subcreds = ob_subcreds; service->n_ob_subcreds = num_subcreds; } + +/** Free any memory allocated by the onionblance subsystem. */ +void +hs_ob_free_all(void) +{ + config_mgr_free(config_options_mgr); +} From ba99287d13782048f58a88dc5d18780fad9f2034 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 27 Jan 2020 17:26:47 +0200 Subject: [PATCH 19/22] Write unittest that covers cases of INTRODUCE1 handling. Also fix some memleaks of other OB unittests. --- src/core/or/circuitbuild.c | 4 +- src/core/or/circuitbuild.h | 3 +- src/feature/hs/hs_cell.c | 2 +- src/feature/hs/hs_circuit.c | 8 +- src/feature/hs/hs_circuit.h | 6 + src/feature/nodelist/nodelist.c | 4 +- src/feature/nodelist/nodelist.h | 4 +- src/test/hs_test_helpers.c | 45 +++- src/test/hs_test_helpers.h | 8 +- src/test/test_hs_ob.c | 8 +- src/test/test_hs_service.c | 356 +++++++++++++++++++++++++++++++- 11 files changed, 421 insertions(+), 27 deletions(-) diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 03ed2c7d29..003b91af8d 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -2819,8 +2819,8 @@ extend_info_dup(extend_info_t *info) * If there is no chosen exit, or if we don't know the node_t for * the chosen exit, return NULL. */ -const node_t * -build_state_get_exit_node(cpath_build_state_t *state) +MOCK_IMPL(const node_t *, +build_state_get_exit_node,(cpath_build_state_t *state)) { if (!state || !state->chosen_exit) return NULL; diff --git a/src/core/or/circuitbuild.h b/src/core/or/circuitbuild.h index f5a3439064..48592dd346 100644 --- a/src/core/or/circuitbuild.h +++ b/src/core/or/circuitbuild.h @@ -66,7 +66,8 @@ int circuit_can_use_tap(const origin_circuit_t *circ); int circuit_has_usable_onion_key(const origin_circuit_t *circ); int extend_info_has_preferred_onion_key(const extend_info_t* ei); const uint8_t *build_state_get_exit_rsa_id(cpath_build_state_t *state); -const node_t *build_state_get_exit_node(cpath_build_state_t *state); +MOCK_DECL(const node_t *, + build_state_get_exit_node,(cpath_build_state_t *state)); const char *build_state_get_exit_nickname(cpath_build_state_t *state); struct circuit_guard_state_t; diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index fce6fe0220..97a1691f16 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -869,7 +869,7 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data, /* Check our replay cache for this introduction point. */ if (replaycache_add_test_and_elapsed(data->replay_cache, encrypted_section, encrypted_section_len, &elapsed)) { - log_warn(LD_REND, "Possible replay detected! An INTRODUCE2 cell with the" + log_warn(LD_REND, "Possible replay detected! An INTRODUCE2 cell with the " "same ENCRYPTED section was seen %ld seconds ago. " "Dropping cell.", (long int) elapsed); goto done; diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index febeec4730..97507df9f7 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -368,10 +368,10 @@ get_service_anonymity_string(const hs_service_t *service) * success, a circuit identifier is attached to the circuit with the needed * data. This function will try to open a circuit for a maximum value of * MAX_REND_FAILURES then it will give up. */ -static void -launch_rendezvous_point_circuit(const hs_service_t *service, - const hs_service_intro_point_t *ip, - const hs_cell_introduce2_data_t *data) +MOCK_IMPL(STATIC void, +launch_rendezvous_point_circuit,(const hs_service_t *service, + const hs_service_intro_point_t *ip, + const hs_cell_introduce2_data_t *data)) { int circ_needs_uptime; time_t now = time(NULL); diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h index 5c20e38187..22e936e685 100644 --- a/src/feature/hs/hs_circuit.h +++ b/src/feature/hs/hs_circuit.h @@ -79,6 +79,12 @@ create_rp_circuit_identifier(const hs_service_t *service, const curve25519_public_key_t *server_pk, const struct hs_ntor_rend_cell_keys_t *keys); +struct hs_cell_introduce2_data_t; +MOCK_DECL(STATIC void, +launch_rendezvous_point_circuit,(const hs_service_t *service, + const hs_service_intro_point_t *ip, + const struct hs_cell_introduce2_data_t *data)); + #endif /* defined(HS_CIRCUIT_PRIVATE) */ #endif /* !defined(TOR_HS_CIRCUIT_H) */ diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 94ff08826f..1d0b1a0d4b 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -1213,8 +1213,8 @@ node_get_rsa_id_digest(const node_t *node) * If node is NULL, returns an empty smartlist. * * The smartlist must be freed using link_specifier_smartlist_free(). */ -smartlist_t * -node_get_link_specifier_smartlist(const node_t *node, bool direct_conn) +MOCK_IMPL(smartlist_t *, +node_get_link_specifier_smartlist,(const node_t *node, bool direct_conn)) { link_specifier_t *ls; tor_addr_port_t ap; diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index 9742e3dff2..40aed0067f 100644 --- a/src/feature/nodelist/nodelist.h +++ b/src/feature/nodelist/nodelist.h @@ -78,8 +78,8 @@ int node_supports_ed25519_hs_intro(const node_t *node); int node_supports_v3_rendezvous_point(const node_t *node); int node_supports_establish_intro_dos_extension(const node_t *node); const uint8_t *node_get_rsa_id_digest(const node_t *node); -smartlist_t *node_get_link_specifier_smartlist(const node_t *node, - bool direct_conn); +MOCK_DECL(smartlist_t *,node_get_link_specifier_smartlist,(const node_t *node, + bool direct_conn)); void link_specifier_smartlist_free_(smartlist_t *ls_list); #define link_specifier_smartlist_free(ls_list) \ FREE_AND_NULL(smartlist_t, link_specifier_smartlist_free_, (ls_list)) diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index eb4af1c9f7..5116fc7169 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -13,9 +13,22 @@ #include "feature/hs/hs_service.h" #include "test/hs_test_helpers.h" +/** + * Create an introduction point taken straight out of an HSv3 descriptor. + * + * Use 'signing_kp' to sign the introduction point certificates. + * + * If 'intro_auth_kp' is provided use that as the introduction point + * authentication keypair, otherwise generate one on the fly. + * + * If 'intro_enc_kp' is provided use that as the introduction point encryption + * keypair, otherwise generate one on the fly. + */ hs_desc_intro_point_t * hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, - const char *addr, int legacy) + const char *addr, int legacy, + const ed25519_keypair_t *intro_auth_kp, + const curve25519_keypair_t *intro_enc_kp) { int ret; ed25519_keypair_t auth_kp; @@ -56,8 +69,12 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, smartlist_add(ip->link_specifiers, ls_ip); } - ret = ed25519_keypair_generate(&auth_kp, 0); - tt_int_op(ret, OP_EQ, 0); + if (intro_auth_kp) { + memcpy(&auth_kp, intro_auth_kp, sizeof(ed25519_keypair_t)); + } else { + ret = ed25519_keypair_generate(&auth_kp, 0); + tt_int_op(ret, OP_EQ, 0); + } ip->auth_key_cert = tor_cert_create(signing_kp, CERT_TYPE_AUTH_HS_IP_KEY, &auth_kp.pubkey, now, HS_DESC_CERT_LIFETIME, @@ -85,8 +102,12 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, ed25519_keypair_t ed25519_kp; tor_cert_t *cross_cert; - ret = curve25519_keypair_generate(&curve25519_kp, 0); - tt_int_op(ret, OP_EQ, 0); + if (intro_enc_kp) { + memcpy(&curve25519_kp, intro_enc_kp, sizeof(curve25519_keypair_t)); + } else { + ret = curve25519_keypair_generate(&curve25519_kp, 0); + tt_int_op(ret, OP_EQ, 0); + } ed25519_keypair_from_curve25519_keypair(&ed25519_kp, &signbit, &curve25519_kp); cross_cert = tor_cert_create(signing_kp, CERT_TYPE_CROSS_HS_IP_KEYS, @@ -95,6 +116,8 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, CERT_FLAG_INCLUDE_SIGNING_KEY); tt_assert(cross_cert); ip->enc_key_cert = cross_cert; + memcpy(ip->enc_key.public_key, curve25519_kp.pubkey.public_key, + CURVE25519_PUBKEY_LEN); } intro_point = ip; @@ -165,13 +188,17 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, if (!no_ip) { /* Add four intro points. */ smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "1.2.3.4", 0)); + hs_helper_build_intro_point(signing_kp, now, "1.2.3.4", 0, + NULL, NULL)); smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "[2600::1]", 0)); + hs_helper_build_intro_point(signing_kp, now, "[2600::1]", 0, + NULL, NULL)); smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "3.2.1.4", 1)); + hs_helper_build_intro_point(signing_kp, now, "3.2.1.4", 1, + NULL, NULL)); smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "5.6.7.8", 1)); + hs_helper_build_intro_point(signing_kp, now, "5.6.7.8", 1, + NULL, NULL)); } descp = desc; diff --git a/src/test/hs_test_helpers.h b/src/test/hs_test_helpers.h index cad4532826..23d11f2a4a 100644 --- a/src/test/hs_test_helpers.h +++ b/src/test/hs_test_helpers.h @@ -8,9 +8,11 @@ #include "feature/hs/hs_descriptor.h" /* Set of functions to help build and test descriptors. */ -hs_desc_intro_point_t *hs_helper_build_intro_point( - const ed25519_keypair_t *signing_kp, time_t now, - const char *addr, int legacy); +hs_desc_intro_point_t * +hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, + const char *addr, int legacy, + const ed25519_keypair_t *intro_auth_kp, + const curve25519_keypair_t *intro_enc_kp); hs_descriptor_t *hs_helper_build_hs_desc_no_ip( const ed25519_keypair_t *signing_kp); hs_descriptor_t *hs_helper_build_hs_desc_with_ip( diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c index c4d9d239d2..b84cef9dec 100644 --- a/src/test/test_hs_ob.c +++ b/src/test/test_hs_ob.c @@ -169,6 +169,7 @@ static void test_get_subcredentials(void *arg) { int ret; + hs_service_t *service = NULL; hs_service_config_t config; (void) arg; @@ -192,7 +193,7 @@ test_get_subcredentials(void *arg) smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); /* Set up an instance */ - hs_service_t *service = tor_malloc_zero(sizeof(hs_service_t)); + service = tor_malloc_zero(sizeof(hs_service_t)); service->config = config; service->desc_current = service_descriptor_new(); service->desc_next = service_descriptor_new(); @@ -234,7 +235,12 @@ test_get_subcredentials(void *arg) done: tor_free(subcreds); + smartlist_free(config.ob_master_pubkeys); + if (service) { + memset(&service->config, 0, sizeof(hs_service_config_t)); + hs_service_free(service); + } UNMOCK(networkstatus_get_live_consensus); } diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index e33d593d94..1767648bbe 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -51,6 +51,8 @@ #include "feature/hs/hs_common.h" #include "feature/hs/hs_config.h" #include "feature/hs/hs_ident.h" +#include "feature/hs/hs_ob.h" +#include "feature/hs/hs_cell.h" #include "feature/hs/hs_intropoint.h" #include "feature/hs/hs_service.h" #include "feature/nodelist/networkstatus.h" @@ -109,6 +111,9 @@ mock_circuit_mark_for_close(circuit_t *circ, int reason, int line, return; } +static size_t relay_payload_len; +static char relay_payload[RELAY_PAYLOAD_SIZE]; + static int mock_relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, uint8_t relay_command, const char *payload, @@ -124,6 +129,10 @@ mock_relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, (void) cpath_layer; (void) filename; (void) lineno; + + memcpy(relay_payload, payload, payload_len); + relay_payload_len = payload_len; + return 0; } @@ -1160,7 +1169,7 @@ test_closing_intro_circs(void *arg) /** Test sending and receiving introduce2 cells */ static void -test_introduce2(void *arg) +test_bad_introduce2(void *arg) { int ret; int flags = CIRCLAUNCH_NEED_UPTIME | CIRCLAUNCH_IS_INTERNAL; @@ -2169,6 +2178,348 @@ test_export_client_circuit_id(void *arg) tor_free(cp2); } +static smartlist_t * +mock_node_get_link_specifier_smartlist(const node_t *node, bool direct_conn) +{ + (void) node; + (void) direct_conn; + + smartlist_t *lspecs = smartlist_new(); + link_specifier_t *ls_legacy = link_specifier_new(); + smartlist_add(lspecs, ls_legacy); + + return lspecs; +} + +static node_t *fake_node = NULL; + +static const node_t * +mock_build_state_get_exit_node(cpath_build_state_t *state) +{ + (void) state; + + if (!fake_node) { + curve25519_secret_key_t seckey; + curve25519_secret_key_generate(&seckey, 0); + + fake_node = tor_malloc_zero(sizeof(node_t)); + fake_node->ri = tor_malloc_zero(sizeof(routerinfo_t)); + fake_node->ri->onion_curve25519_pkey = + tor_malloc_zero(sizeof(curve25519_public_key_t)); + curve25519_public_key_generate(fake_node->ri->onion_curve25519_pkey, + &seckey); + } + + return fake_node; +} + +static void +mock_launch_rendezvous_point_circuit(const hs_service_t *service, + const hs_service_intro_point_t *ip, + const hs_cell_introduce2_data_t *data) +{ + (void) service; + (void) ip; + (void) data; + return; +} + +/** + * Test that INTRO2 cells are handled well by onion services in the normal + * case and also when onionbalance is enabled. + */ +static void +test_intro2_handling(void *arg) +{ + (void)arg; + + MOCK(build_state_get_exit_node, mock_build_state_get_exit_node); + MOCK(relay_send_command_from_edge_, mock_relay_send_command_from_edge); + MOCK(node_get_link_specifier_smartlist, + mock_node_get_link_specifier_smartlist); + MOCK(launch_rendezvous_point_circuit, mock_launch_rendezvous_point_circuit); + + memset(relay_payload, 0, sizeof(relay_payload)); + + int retval; + time_t now = 0101010101; + update_approx_time(now); + + /** OK this is the play: + * + * In Act I, we have a standalone onion service X (without onionbalance + * enabled). We test that X can properly handle INTRO2 cells sent by a + * client Alice. + * + * In Act II, we create an onionbalance setup with frontend being Z which + * includes instances X and Y. We then setup onionbalance on X and test that + * Alice who addresses Z can communicate with X through INTRO2 cells. + * + * In Act III, we test that Alice can also communicate with X + * directly even tho onionbalance is enabled. + * + * And finally in Act IV, we check various cases where the INTRO2 cell + * should not go through because the subcredentials don't line up + * (e.g. Alice sends INTRO2 to X using Y's subcredential). + */ + + /** Let's start with some setup! Create the instances and the frontend + service, create Alice, etc: */ + + /* Create instance X */ + hs_service_t x_service; + memset(&x_service, 0, sizeof(hs_service_t)); + /* Disable onionbalance */ + x_service.config.ob_master_pubkeys = NULL; + x_service.state.replay_cache_rend_cookie = replaycache_new(0,0); + + /* Create subcredential for x: */ + ed25519_keypair_t x_identity_keypair; + hs_subcredential_t x_subcred; + ed25519_keypair_generate(&x_identity_keypair, 0); + hs_helper_get_subcred_from_identity_keypair(&x_identity_keypair, + &x_subcred); + + /* Create the x instance's intro point */ + hs_service_intro_point_t *x_ip = NULL; + { + curve25519_secret_key_t seckey; + curve25519_public_key_t pkey; + curve25519_secret_key_generate(&seckey, 0); + curve25519_public_key_generate(&pkey, &seckey); + + node_t intro_node; + memset(&intro_node, 0, sizeof(intro_node)); + routerinfo_t ri; + memset(&ri, 0, sizeof(routerinfo_t)); + ri.onion_curve25519_pkey = &pkey; + intro_node.ri = &ri; + + x_ip = service_intro_point_new(&intro_node); + } + + /* Create z frontend's subcredential */ + ed25519_keypair_t z_identity_keypair; + hs_subcredential_t z_subcred; + ed25519_keypair_generate(&z_identity_keypair, 0); + hs_helper_get_subcred_from_identity_keypair(&z_identity_keypair, + &z_subcred); + + /* Create y instance's subcredential */ + ed25519_keypair_t y_identity_keypair; + hs_subcredential_t y_subcred; + ed25519_keypair_generate(&y_identity_keypair, 0); + hs_helper_get_subcred_from_identity_keypair(&y_identity_keypair, + &y_subcred); + + /* Create Alice's intro point */ + hs_desc_intro_point_t *alice_ip; + ed25519_keypair_t signing_kp; + ed25519_keypair_generate(&signing_kp, 0); + alice_ip = hs_helper_build_intro_point(&signing_kp, now, "1.2.3.4", 0, + &x_ip->auth_key_kp, + &x_ip->enc_key_kp); + + /* Create Alice's intro and rend circuits */ + origin_circuit_t *intro_circ = origin_circuit_new(); + intro_circ->cpath = tor_malloc_zero(sizeof(crypt_path_t)); + intro_circ->cpath->prev = intro_circ->cpath; + intro_circ->hs_ident = tor_malloc_zero(sizeof(*intro_circ->hs_ident)); + origin_circuit_t rend_circ; + rend_circ.hs_ident = tor_malloc_zero(sizeof(*rend_circ.hs_ident)); + curve25519_keypair_generate(&rend_circ.hs_ident->rendezvous_client_kp, 0); + memset(rend_circ.hs_ident->rendezvous_cookie, 'r', HS_REND_COOKIE_LEN); + + /* ************************************************************ */ + + /* Act I: + * + * Where Alice connects to X without onionbalance in the picture */ + + /* Create INTRODUCE1 */ + tt_assert(fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &x_subcred); + + /* Check that the payload was written successfully */ + tt_int_op(retval, OP_EQ, 0); + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + /* Handle the cell */ + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload,relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + /* ************************************************************ */ + + /* Act II: + * + * We now create an onionbalance setup with Z being the frontend and X and Y + * being the backend instances. Make sure that Alice can talk with the + * backend instance X even tho she thinks she is talking to the frontend Z. + */ + + /* Now configure the X instance to do onionbalance with Z as the frontend */ + x_service.config.ob_master_pubkeys = smartlist_new(); + smartlist_add(x_service.config.ob_master_pubkeys, + &z_identity_keypair.pubkey); + + /* Create descriptors for x and load next descriptor with the x's + * subcredential so that it can accept connections for itself. */ + x_service.desc_current = service_descriptor_new(); + memset(x_service.desc_current->desc->subcredential.subcred, 'C',SUBCRED_LEN); + x_service.desc_next = service_descriptor_new(); + memcpy(&x_service.desc_next->desc->subcredential, &x_subcred, SUBCRED_LEN); + + /* Refresh OB keys */ + hs_ob_refresh_keys(&x_service); + + /* Create INTRODUCE1 from Alice to X through Z */ + memset(relay_payload, 0, sizeof(relay_payload)); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &z_subcred); + + /* Check that the payload was written successfully */ + tt_int_op(retval, OP_EQ, 0); + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + /* Deliver INTRODUCE1 to X even tho it carries Z's subcredential */ + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &z_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + replaycache_free(x_ip->replay_cache); + x_ip->replay_cache = replaycache_new(0, 0); + + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + + /* ************************************************************ */ + + /* Act III: + * + * Now send a direct INTRODUCE cell from Alice to X using X's subcredential + * and check that it succeeds even with onionbalance enabled. + */ + + /* Refresh OB keys (just to check for memleaks) */ + hs_ob_refresh_keys(&x_service); + + /* Create INTRODUCE1 from Alice to X using X's subcred. */ + memset(relay_payload, 0, sizeof(relay_payload)); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &x_subcred); + + /* Check that the payload was written successfully */ + tt_int_op(retval, OP_EQ, 0); + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + /* Send INTRODUCE1 to X with X's subcredential (should succeed) */ + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + /* ************************************************************ */ + + /* Act IV: + * + * Test cases where the INTRO2 cell should not be able to decode. + */ + + /* Try sending the exact same INTRODUCE2 cell again and see that the intro + * point replay cache triggers: */ + setup_full_capture_of_logs(LOG_WARN); + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, -1); + expect_log_msg_containing("with the same ENCRYPTED section"); + teardown_capture_of_logs(); + + /* Now cleanup the intro point replay cache but not the service replay cache + and see that this one triggers this time. */ + replaycache_free(x_ip->replay_cache); + x_ip->replay_cache = replaycache_new(0, 0); + setup_full_capture_of_logs(LOG_INFO); + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, -1); + expect_log_msg_containing("with same REND_COOKIE"); + teardown_capture_of_logs(); + + /* Now just to make sure cleanup both replay caches and make sure that the + cell gets through */ + replaycache_free(x_ip->replay_cache); + x_ip->replay_cache = replaycache_new(0, 0); + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + /* As a final thing, create an INTRODUCE1 cell from Alice to X using Y's + * subcred (should fail since Y is just another instance and not the frontend + * service!) */ + memset(relay_payload, 0, sizeof(relay_payload)); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &y_subcred); + tt_int_op(retval, OP_EQ, 0); + + /* Check that the payload was written successfully */ + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &y_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, -1); + + done: + /* Start cleaning up X */ + replaycache_free(x_service.state.replay_cache_rend_cookie); + smartlist_free(x_service.config.ob_master_pubkeys); + tor_free(x_service.ob_subcreds); + service_descriptor_free(x_service.desc_current); + service_descriptor_free(x_service.desc_next); + service_intro_point_free(x_ip); + + /* Clean up Alice */ + hs_desc_intro_point_free(alice_ip); + tor_free(rend_circ.hs_ident); + + if (fake_node) { + tor_free(fake_node->ri->onion_curve25519_pkey); + tor_free(fake_node->ri); + tor_free(fake_node); + } + + UNMOCK(build_state_get_exit_node); + UNMOCK(relay_send_command_from_edge_); + UNMOCK(node_get_link_specifier_smartlist); + UNMOCK(launch_rendezvous_point_circuit); +} + struct testcase_t hs_service_tests[] = { { "e2e_rend_circuit_setup", test_e2e_rend_circuit_setup, TT_FORK, NULL, NULL }, @@ -2194,7 +2545,7 @@ struct testcase_t hs_service_tests[] = { NULL, NULL }, { "rdv_circuit_opened", test_rdv_circuit_opened, TT_FORK, NULL, NULL }, - { "introduce2", test_introduce2, TT_FORK, + { "bad_introduce2", test_bad_introduce2, TT_FORK, NULL, NULL }, { "service_event", test_service_event, TT_FORK, NULL, NULL }, @@ -2212,6 +2563,7 @@ struct testcase_t hs_service_tests[] = { TT_FORK, NULL, NULL }, { "export_client_circuit_id", test_export_client_circuit_id, TT_FORK, NULL, NULL }, + { "intro2_handling", test_intro2_handling, TT_FORK, NULL, NULL }, END_OF_TESTCASES }; From 901ed35709ac79e841b89d947a0d84526f4d8daf Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Thu, 6 Feb 2020 16:28:21 +0200 Subject: [PATCH 20/22] Make n_subcredentials a size_t . Based on David's review. --- src/core/crypto/hs_ntor.c | 4 ++-- src/core/crypto/hs_ntor.h | 2 +- src/feature/hs/hs_cell.c | 4 ++-- src/feature/hs/hs_cell.h | 2 +- src/feature/hs/hs_circuit.c | 2 +- src/lib/ctime/di_ops.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/crypto/hs_ntor.c b/src/core/crypto/hs_ntor.c index 4bd11ef98e..07bcdc566c 100644 --- a/src/core/crypto/hs_ntor.c +++ b/src/core/crypto/hs_ntor.c @@ -471,7 +471,7 @@ hs_ntor_service_get_introduce1_keys_multi( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_keypair_t *intro_enc_keypair, const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, - int n_subcredentials, + size_t n_subcredentials, const hs_subcredential_t *subcredentials, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { @@ -499,7 +499,7 @@ hs_ntor_service_get_introduce1_keys_multi( secret_input); bad |= safe_mem_is_zero(secret_input, CURVE25519_OUTPUT_LEN); - for (int i = 0; i < n_subcredentials; ++i) { + for (unsigned i = 0; i < n_subcredentials; ++i) { /* Get ENC_KEY and MAC_KEY! */ get_introduce1_key_material(secret_input, &subcredentials[i], &hs_ntor_intro_cell_keys_out[i]); diff --git a/src/core/crypto/hs_ntor.h b/src/core/crypto/hs_ntor.h index 2ed357f02d..9a975dd83f 100644 --- a/src/core/crypto/hs_ntor.h +++ b/src/core/crypto/hs_ntor.h @@ -62,7 +62,7 @@ int hs_ntor_service_get_introduce1_keys_multi( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_keypair_t *intro_enc_keypair, const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, - int n_subcredentials, + size_t n_subcredentials, const hs_subcredential_t *subcredentials, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out); diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 97a1691f16..dd5fefd7e7 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -77,7 +77,7 @@ compute_introduce_mac(const uint8_t *encoded_cell, size_t encoded_cell_len, static hs_ntor_intro_cell_keys_t * get_introduce2_key_material(const ed25519_public_key_t *auth_key, const curve25519_keypair_t *enc_key, - int n_subcredentials, + size_t n_subcredentials, const hs_subcredential_t *subcredentials, const uint8_t *encrypted_section, curve25519_public_key_t *client_pk) @@ -787,7 +787,7 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, /* Validate MAC from the cell and our computed key material. The MAC field * in the cell is at the end of the encrypted section. */ intro_keys_result = tor_malloc_zero(sizeof(*intro_keys_result)); - for (int i = 0; i < data->n_subcredentials; ++i) { + for (unsigned i = 0; i < data->n_subcredentials; ++i) { uint8_t mac[DIGEST256_LEN]; /* The MAC field is at the very end of the ENCRYPTED section. */ diff --git a/src/feature/hs/hs_cell.h b/src/feature/hs/hs_cell.h index cc2e7b5817..2b28c44c50 100644 --- a/src/feature/hs/hs_cell.h +++ b/src/feature/hs/hs_cell.h @@ -60,7 +60,7 @@ typedef struct hs_cell_introduce2_data_t { /** * Length of the subcredentials array below. **/ - int n_subcredentials; + size_t n_subcredentials; /** Array of n_subcredentials subcredentials for the service. Pointer * owned by the descriptor that owns the introduction point through which we * received the INTRO2 cell. */ diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 97507df9f7..fdd226ba79 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -989,7 +989,7 @@ get_subcredential_for_handling_intro2_cell(const hs_service_t *service, } /* We are an onionbalance instance: */ - data->n_subcredentials = (int) service->n_ob_subcreds; + data->n_subcredentials = service->n_ob_subcreds; data->subcredentials = service->ob_subcreds; return 0; diff --git a/src/lib/ctime/di_ops.c b/src/lib/ctime/di_ops.c index 0859dd8bee..e1ac0943fb 100644 --- a/src/lib/ctime/di_ops.c +++ b/src/lib/ctime/di_ops.c @@ -281,7 +281,7 @@ select_array_member_cumulative_timei(const uint64_t *entries, int n_entries, } /** - * If s is true, then copy n bytes from src to + * If s is true, then copy n bytes from src to * dest. Otherwise leave dest alone. * * This function behaves the same as From 975102869a3b5957bc0a1f2103697371fbe04cd3 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Tue, 18 Feb 2020 12:37:34 +0200 Subject: [PATCH 21/22] Fix issues pointed out by Nick. - Loose the asserts on num_pkeys. - Straighten some dangling &. - Fix some unpredictable memcpys. --- src/feature/hs/hs_client.c | 3 +-- src/feature/hs/hs_ob.c | 29 +++++++++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 601062190d..58a143650b 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1837,8 +1837,7 @@ hs_client_decode_descriptor(const char *desc_str, uint64_t current_time_period = hs_get_time_period_num(0); hs_build_blinded_pubkey(service_identity_pk, NULL, 0, current_time_period, &blinded_pubkey); - hs_get_subcredential(service_identity_pk, &blinded_pubkey, & - subcredential); + hs_get_subcredential(service_identity_pk, &blinded_pubkey, &subcredential); } /* Parse descriptor */ diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 6a2e43f050..49e01099a1 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -262,8 +262,8 @@ hs_ob_parse_config_file(hs_service_config_t *config) return ret; } -/** Compute all possible subcredentials for every onion master key in the - * given service config object. The subcredentials is allocated and set as an +/** Compute all possible subcredentials for every onion master key in the given + * service config object. subcredentials_out is allocated and set as an * continous array containing all possible values. * * On success, return the number of subcredential put in the array which will @@ -271,12 +271,12 @@ hs_ob_parse_config_file(hs_service_config_t *config) * length of a single subcredential. * * If the given configuration object has no OB master keys configured, 0 is - * returned and subcredentials is set to NULL. + * returned and subcredentials_out is set to NULL. * * Otherwise, this can't fail. */ STATIC size_t compute_subcredentials(const hs_service_t *service, - hs_subcredential_t **subcredentials) + hs_subcredential_t **subcredentials_out) { unsigned int num_pkeys, idx = 0; hs_subcredential_t *subcreds = NULL; @@ -285,14 +285,17 @@ compute_subcredentials(const hs_service_t *service, const uint64_t tp = hs_get_time_period_num(0); tor_assert(service); - tor_assert(subcredentials); + tor_assert(subcredentials_out); /* Our caller has checked these too */ tor_assert(service->desc_current); tor_assert(service->desc_next); /* Our caller made sure that we are an OB instance */ num_pkeys = smartlist_len(service->config.ob_master_pubkeys); - tor_assert(num_pkeys > 0); + if (!num_pkeys) { + subcredentials_out = NULL; + return 0; + } /* Time to build all the subcredentials for each time period: two for each * instance descriptor plus three for the onionbalance frontend service: the @@ -335,15 +338,15 @@ compute_subcredentials(const hs_service_t *service, /* And then in the end we add the two subcredentials of the current active * instance descriptors */ - memcpy(&subcreds[idx++], - service->desc_current->desc->subcredential.subcred, SUBCRED_LEN); - memcpy(&subcreds[idx++], - service->desc_next->desc->subcredential.subcred, SUBCRED_LEN); + memcpy(&subcreds[idx++], &service->desc_current->desc->subcredential, + sizeof(hs_subcredential_t)); + memcpy(&subcreds[idx++], &service->desc_next->desc->subcredential, + sizeof(hs_subcredential_t)); log_info(LD_REND, "Refreshing %u onionbalance keys (TP #%d).", idx, (int)tp); - *subcredentials = subcreds; + *subcredentials_out = subcreds; return idx; } @@ -384,7 +387,9 @@ hs_ob_refresh_keys(hs_service_t *service) /* Get a new set of subcreds */ num_subcreds = compute_subcredentials(service, &ob_subcreds); - tor_assert(num_subcreds > 0); + if (BUG(!num_subcreds)) { + return; + } /* Delete old subcredentials if any */ if (service->ob_subcreds) { From 93cb8072becb4213525d08a87fdf7284e6257168 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 24 Feb 2020 12:15:35 +0200 Subject: [PATCH 22/22] Final touches to #32709 based on Nick's feedback. - Fix a bug and add unittest. - Add changes file. - Add man page entry. --- changes/bug32709 | 4 ++++ doc/tor.1.txt | 13 +++++++++++++ src/feature/hs/hs_ob.c | 4 ++-- src/test/test_hs_ob.c | 22 ++++++++++++++++------ 4 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 changes/bug32709 diff --git a/changes/bug32709 b/changes/bug32709 new file mode 100644 index 0000000000..d00b112be6 --- /dev/null +++ b/changes/bug32709 @@ -0,0 +1,4 @@ + o Major features (v3 onion services): + - Allow v3 onion services to act as OnionBalance backend instances using + the HiddenServiceOnionBalanceInstance torrc option. Closes ticket 32709. + diff --git a/doc/tor.1.txt b/doc/tor.1.txt index a5108df805..4aa09e7f3e 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -3128,6 +3128,19 @@ The next section describes the per service options that can only be set The HAProxy version 1 protocol is described in detail at https://www.haproxy.org/download/1.8/doc/proxy-protocol.txt +[[HiddenServiceOnionBalanceInstance]] **HiddenServiceOnionBalanceInstance** **0**|**1**:: + + If set to 1, this onion service becomes an OnionBalance instance and will + accept client connections destined to an OnionBalance frontend. In this + case, Tor expects to find a file named "ob_config" inside the + **HiddenServiceDir** directory with content: + + + MasterOnionAddress + + + where is the onion address of the OnionBalance + frontend (e.g. wrxdvcaqpuzakbfww5sxs6r2uybczwijzfn2ezy2osaj7iox7kl7nhad.onion). + + [[HiddenServiceMaxStreams]] **HiddenServiceMaxStreams** __N__:: The maximum number of simultaneous streams (connections) per rendezvous circuit. The maximum value allowed is 65535. (Setting this to 0 will allow diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 49e01099a1..c18a789013 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -290,10 +290,10 @@ compute_subcredentials(const hs_service_t *service, tor_assert(service->desc_current); tor_assert(service->desc_next); - /* Our caller made sure that we are an OB instance */ + /* Make sure we are an OB instance, or bail out. */ num_pkeys = smartlist_len(service->config.ob_master_pubkeys); if (!num_pkeys) { - subcredentials_out = NULL; + *subcredentials_out = NULL; return 0; } diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c index b84cef9dec..7f40187b5f 100644 --- a/src/test/test_hs_ob.c +++ b/src/test/test_hs_ob.c @@ -171,6 +171,7 @@ test_get_subcredentials(void *arg) int ret; hs_service_t *service = NULL; hs_service_config_t config; + hs_subcredential_t *subcreds = NULL; (void) arg; @@ -188,16 +189,24 @@ test_get_subcredentials(void *arg) config.ob_master_pubkeys = smartlist_new(); tt_assert(config.ob_master_pubkeys); - /* Generate a keypair to add to the list. */ - ed25519_keypair_generate(&onion_addr_kp_1, 0); - smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); - /* Set up an instance */ service = tor_malloc_zero(sizeof(hs_service_t)); service->config = config; + /* Setup the service descriptors */ service->desc_current = service_descriptor_new(); service->desc_next = service_descriptor_new(); + /* First try to compute subcredentials but with no OB keys. Make sure that + * subcreds get NULLed. To do this check we first poison subcreds. */ + subcreds = (void*)999; + tt_ptr_op(subcreds, OP_NE, NULL); + size_t num = compute_subcredentials(service, &subcreds); + tt_ptr_op(subcreds, OP_EQ, NULL); + + /* Generate a keypair to add to the OB keys list. */ + ed25519_keypair_generate(&onion_addr_kp_1, 0); + smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); + /* Set up the instance subcredentials */ char current_subcred[SUBCRED_LEN]; char next_subcred[SUBCRED_LEN]; @@ -208,10 +217,11 @@ test_get_subcredentials(void *arg) memcpy(service->desc_next->desc->subcredential.subcred, next_subcred, SUBCRED_LEN); - hs_subcredential_t *subcreds = NULL; - size_t num = compute_subcredentials(service, &subcreds); + /* See that subcreds are computed properly */ + num = compute_subcredentials(service, &subcreds); /* 5 subcredentials: 3 for the frontend, 2 for the instance */ tt_uint_op(num, OP_EQ, 5); + tt_ptr_op(subcreds, OP_NE, NULL); /* Validate the subcredentials we just got. We'll build them oursevles with * the right time period steps and compare. */