From b50f39fb6fc9d9a4bbb86d760291d6e88bf0987a Mon Sep 17 00:00:00 2001 From: David Goulet Date: Sun, 15 Jan 2017 10:13:37 -0500 Subject: [PATCH 01/22] prop224: Add common intropoint object Groundwork for more prop224 service and client code. This object contains common data that both client and service uses. Signed-off-by: David Goulet --- src/or/hs_intropoint.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/or/hs_intropoint.h b/src/or/hs_intropoint.h index 163ed810e7..bfb1331ba0 100644 --- a/src/or/hs_intropoint.h +++ b/src/or/hs_intropoint.h @@ -9,6 +9,9 @@ #ifndef TOR_HS_INTRO_H #define TOR_HS_INTRO_H +#include "crypto_curve25519.h" +#include "torcert.h" + /* Authentication key type in an ESTABLISH_INTRO cell. */ enum hs_intro_auth_key_type { HS_INTRO_AUTH_KEY_TYPE_LEGACY0 = 0x00, @@ -24,6 +27,15 @@ typedef enum { HS_INTRO_ACK_STATUS_CANT_RELAY = 0x0003, } hs_intro_ack_status_t; +/* Object containing introduction point common data between the service and + * the client side. */ +typedef struct hs_intropoint_t { + /* Authentication key certificate from the descriptor. */ + tor_cert_t *auth_key_cert; + /* A list of link specifier. */ + smartlist_t *link_specifiers; +} hs_intropoint_t; + int hs_intro_received_establish_intro(or_circuit_t *circ, const uint8_t *request, size_t request_len); From b03853b65f109ed6a34ba2924fe3b00d56131ff5 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Sun, 15 Jan 2017 10:09:13 -0500 Subject: [PATCH 02/22] prop224: Initial import of hs_service_t This object is the foundation of proposal 224 service work. It will change and be adapted as it's being used more and more in the codebase. So, this version is just a basic skeleton one that *will* change. Signed-off-by: David Goulet --- src/or/hs_service.h | 184 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 182 insertions(+), 2 deletions(-) diff --git a/src/or/hs_service.h b/src/or/hs_service.h index 3302592762..fa5dd541e3 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -3,15 +3,195 @@ /** * \file hs_service.h - * \brief Header file for hs_service.c. + * \brief Header file containing service data for the HS subsytem. **/ #ifndef TOR_HS_SERVICE_H #define TOR_HS_SERVICE_H -#include "or.h" +#include "crypto_curve25519.h" +#include "crypto_ed25519.h" +#include "hs_descriptor.h" +#include "hs_intropoint.h" +#include "replaycache.h" + +/* Trunnel */ #include "hs/cell_establish_intro.h" +/* When loading and configuring a service, this is the default version it will + * be configured for as it is possible that no HiddenServiceVersion is + * present. */ +#define HS_SERVICE_DEFAULT_VERSION HS_VERSION_TWO + +/* Service side introduction point. */ +typedef struct hs_service_intro_point_t { + /* Top level intropoint "shared" data between client/service. */ + hs_intropoint_t base; + + /* Authentication keypair used to create the authentication certificate + * which is published in the descriptor. */ + ed25519_keypair_t auth_key_kp; + + /* Encryption private key. */ + curve25519_secret_key_t enc_key_sk; + + /* Amount of INTRODUCE2 cell accepted from this intro point. */ + uint64_t introduce2_count; + + /* Maximum number of INTRODUCE2 cell this intro point should accept. */ + uint64_t introduce2_max; + + /* The time at which this intro point should expire and stop being used. */ + time_t time_to_expire; + + /* The amount of circuit creation we've made to this intro point. This is + * incremented every time we do a circuit relaunch on this intro point which + * is triggered when the circuit dies but the node is still in the + * consensus. After MAX_INTRO_POINT_CIRCUIT_RETRIES, we give up on it. */ + uint32_t circuit_retries; + + /* Set if this intro point has an established circuit. */ + unsigned int circuit_established : 1; + + /* Replay cache recording the encrypted part of an INTRODUCE2 cell that the + * circuit associated with this intro point has received. This is used to + * prevent replay attacks. */ + replaycache_t *replay_cache; +} hs_service_intro_point_t; + +/* Object handling introduction points of a service. */ +typedef struct hs_service_intropoints_t { + /* The time at which we've started our retry period to build circuits. We + * don't want to stress circuit creation so we can only retry for a certain + * time and then after we stop and wait. */ + time_t retry_period_started; + + /* Number of circuit we've launched during a single retry period. */ + unsigned int num_circuits_launched; + + /* Contains the current hs_service_intro_point_t objects indexed by + * authentication public key. */ + digest256map_t *map; +} hs_service_intropoints_t; + +/* Representation of a service descriptor. */ +typedef struct hs_service_descriptor_t { + /* Decoded descriptor. This object is used for encoding when the service + * publishes the descriptor. */ + hs_descriptor_t *desc; + + /* Descriptor signing keypair. */ + ed25519_keypair_t signing_kp; + + /* Blinded keypair derived from the master identity public key. */ + ed25519_keypair_t blinded_kp; + + /* When is the next time when we should upload the descriptor. */ + time_t next_upload_time; + + /* Introduction points assign to this descriptor which contains + * hs_service_intropoints_t object indexed by authentication key (the RSA + * key if the node is legacy). */ + hs_service_intropoints_t intro_points; +} hs_service_descriptor_t; + +/* Service key material. */ +typedef struct hs_service_keys_t { + /* Master identify public key. */ + ed25519_public_key_t identity_pk; + /* Master identity private key. */ + ed25519_secret_key_t identity_sk; + /* True iff the key is kept offline which means the identity_sk MUST not be + * used in that case. */ + unsigned int is_identify_key_offline : 1; +} hs_service_keys_t; + +/* Service configuration. The following are set from the torrc options either + * set by the configuration file or by the control port. Nothing else should + * change those values. */ +typedef struct hs_service_config_t { + /* List of rend_service_port_config_t */ + smartlist_t *ports; + + /* Path on the filesystem where the service persistent data is stored. NULL + * if the service is ephemeral. Specified by HiddenServiceDir option. */ + char *directory_path; + + /* The time period after which a descriptor is uploaded to the directories + * in seconds. Specified by RendPostPeriod option. */ + uint32_t descriptor_post_period; + + /* The maximum number of simultaneous streams per rendezvous circuit that + * are allowed to be created. No limit if 0. Specified by + * HiddenServiceMaxStreams option. */ + uint64_t max_streams_per_rdv_circuit; + + /* If true, we close circuits that exceed the max_streams_per_rdv_circuit + * limit. Specified by HiddenServiceMaxStreamsCloseCircuit option. */ + unsigned int max_streams_close_circuit : 1; + + /* How many introduction points this service has. Specified by + * HiddenServiceNumIntroductionPoints option. */ + unsigned int num_intro_points; + + /* True iff we allow request made on unknown ports. Specified by + * HiddenServiceAllowUnknownPorts option. */ + unsigned int allow_unknown_ports : 1; + + /* If true, this service is a Single Onion Service. Specified by + * HiddenServiceSingleHopMode and HiddenServiceNonAnonymousMode options. */ + unsigned int is_single_onion : 1; + + /* If true, allow group read permissions on the directory_path. Specified by + * HiddenServiceDirGroupReadable option. */ + unsigned int dir_group_readable : 1; + + /* Is this service ephemeral? */ + unsigned int is_ephemeral : 1; +} hs_service_config_t; + +/* Service state. */ +typedef struct hs_service_state_t { + /* The time at which we've started our retry period to build circuits. We + * don't want to stress circuit creation so we can only retry for a certain + * time and then after we stop and wait. */ + time_t intro_circ_retry_started_time; + + /* Number of circuit we've launched during a single retry period. This + * should never go over MAX_INTRO_CIRCS_PER_PERIOD. */ + unsigned int num_intro_circ_launched; + + /* Indicate that the service has entered the overlap period. We use this + * flag to check for descriptor rotation. */ + unsigned int in_overlap_period : 1; +} hs_service_state_t; + +/* Representation of a service running on this tor instance. */ +typedef struct hs_service_t { + /* Protocol version of the service. Specified by HiddenServiceVersion. */ + uint32_t version; + + /* Service state which contains various flags and counters. */ + hs_service_state_t state; + + /* Key material of the service. */ + hs_service_keys_t keys; + + /* Configuration of the service. */ + hs_service_config_t config; + + /* Current descriptor. */ + hs_service_descriptor_t *desc_current; + /* Next descriptor that we need for the overlap period for which we have to + * keep two sets of opened introduction point circuits. */ + hs_service_descriptor_t *desc_next; + + /* XXX: Credential (client auth.) #20700. */ + +} hs_service_t; + +/* API */ + /* These functions are only used by unit tests and we need to expose them else * hs_service.o ends up with no symbols in libor.a which makes clang throw a * warning at compile time. See #21825. */ From 02e2edeb33224461d1fbb879722c0948171b9688 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 16 Jan 2017 13:19:44 -0500 Subject: [PATCH 03/22] prop224: Add hs_config.{c|h} with a refactoring Add the hs_config.{c|h} files contains everything that the HS subsystem needs to load and configure services. Ultimately, it should also contain client functions such as client authorization. This comes with a big refactoring of rend_config_services() which has now changed to only configure a single service and it is stripped down of the common directives which are now part of the generic handler. This is ground work for prop224 of course but only touches version 2 services and add XXX note for version 3. Signed-off-by: David Goulet --- src/or/config.c | 5 +- src/or/hs_common.h | 3 + src/or/hs_config.c | 292 ++++++++++++++++++++++++++++++++++++++++++ src/or/hs_config.h | 19 +++ src/or/hs_service.c | 85 +++++++++++++ src/or/hs_service.h | 6 + src/or/include.am | 26 ++-- src/or/main.c | 2 +- src/or/rendservice.c | 295 ++++++++++++++----------------------------- src/or/rendservice.h | 10 +- src/test/test_hs.c | 22 ++-- 11 files changed, 535 insertions(+), 230 deletions(-) create mode 100644 src/or/hs_config.c create mode 100644 src/or/hs_config.h diff --git a/src/or/config.c b/src/or/config.c index 5b5bb9049b..062ab27ae7 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -91,6 +91,7 @@ #include "relay.h" #include "rendclient.h" #include "rendservice.h" +#include "hs_config.h" #include "rephist.h" #include "router.h" #include "sandbox.h" @@ -1681,7 +1682,7 @@ options_act(const or_options_t *old_options) sweep_bridge_list(); } - if (running_tor && rend_config_services(options, 0)<0) { + if (running_tor && hs_config_service_all(options, 0)<0) { log_warn(LD_BUG, "Previously validated hidden services line could not be added!"); return -1; @@ -4009,7 +4010,7 @@ options_validate(or_options_t *old_options, or_options_t *options, COMPLAIN("V3AuthVotingInterval does not divide evenly into 24 hours."); } - if (rend_config_services(options, 1) < 0) + if (hs_config_service_all(options, 1) < 0) REJECT("Failed to configure rendezvous options. See logs for details."); /* Parse client-side authorization for hidden services. */ diff --git a/src/or/hs_common.h b/src/or/hs_common.h index 872fed763a..abc44c09d1 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -16,6 +16,9 @@ #define HS_VERSION_TWO 2 /* Version 3 of the protocol (prop224). */ #define HS_VERSION_THREE 3 +/* Earliest and latest version we support. */ +#define HS_VERSION_MIN HS_VERSION_TWO +#define HS_VERSION_MAX HS_VERSION_THREE /** Try to maintain this many intro points per service by default. */ #define NUM_INTRO_POINTS_DEFAULT 3 diff --git a/src/or/hs_config.c b/src/or/hs_config.c new file mode 100644 index 0000000000..6326e90324 --- /dev/null +++ b/src/or/hs_config.c @@ -0,0 +1,292 @@ +/* Copyright (c) 2017, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file hs_config.c + * \brief Implement hidden service configuration subsystem. + * + * \details + * + * This file has basically one main entry point: hs_config_service_all(). It + * takes the torrc options and configure hidden service from it. In validate + * mode, nothing is added to the global service list or keys are not generated + * nor loaded. + * + * A service is configured in two steps. It is first created using the tor + * options and then put in a staging list. It will stay there until + * hs_service_load_all_keys() is called. That function is responsible to + * load/generate the keys for the service in the staging list and if + * successful, transfert the service to the main global service list where + * at that point it is ready to be used. + * + * Configuration handlers are per-version (see config_service_handlers[]) and + * there is a main generic one for every option that is common to all version + * (config_generic_service). + **/ + +#define HS_CONFIG_PRIVATE + +#include "hs_common.h" +#include "hs_config.h" +#include "hs_service.h" +#include "rendservice.h" + +/* Configuration handler for a version 3 service. Return 0 on success else a + * negative value. */ +static int +config_service_v3(const config_line_t *line, + const or_options_t *options, int validate_only, + hs_service_t *service) +{ + (void) line; + (void) service; + (void) validate_only; + (void) options; + /* XXX: Configure a v3 service with specific options. */ + /* XXX: Add service to v3 list and pruning on reload. */ + return 0; +} + +/* Configure a service using the given options in line_ and options. This is + * called for any service regardless of its version which means that all + * directives in this function are generic to any service version. This + * function will also check the validity of the service directory path. + * + * The line_ must be pointing to the directive directly after a + * HiddenServiceDir. That way, when hitting the next HiddenServiceDir line or + * reaching the end of the list of lines, we know that we have to stop looking + * for more options. + * + * Return 0 on success else -1. */ +static int +config_generic_service(const config_line_t *line_, + const or_options_t *options, + hs_service_t *service) +{ + int ok, dir_seen = 0; + const config_line_t *line; + hs_service_config_t *config; + + tor_assert(line_); + tor_assert(options); + tor_assert(service); + + /* Makes thing easier. */ + config = &service->config; + memset(config, 0, sizeof(*config)); + + /* The first line starts with HiddenServiceDir so we consider what's next is + * the configuration of the service. */ + for (line = line_; line ; line = line->next) { + /* This indicate that we have a new service to configure. */ + if (!strcasecmp(line->key, "HiddenServiceDir")) { + /* This function only configures one service at a time so if we've + * already seen one, stop right now. */ + if (dir_seen) { + break; + } + /* Ok, we've seen one and we are about to configure it. */ + dir_seen = 1; + config->directory_path = tor_strdup(line->value); + log_info(LD_CONFIG, "HiddenServiceDir=%s. Configuring...", + escaped(config->directory_path)); + continue; + } + if (BUG(!dir_seen)) { + goto err; + } + /* Version of the service. */ + if (!strcasecmp(line->key, "HiddenServiceVersion")) { + service->version = (uint32_t) tor_parse_ulong(line->value, + 10, HS_VERSION_TWO, + HS_VERSION_MAX, + &ok, NULL); + if (!ok) { + log_warn(LD_CONFIG, + "HiddenServiceVersion be between %u and %u, not %s", + HS_VERSION_TWO, HS_VERSION_MAX, line->value); + goto err; + } + log_info(LD_CONFIG, "HiddenServiceVersion=%" PRIu32 " for %s", + service->version, escaped(config->directory_path)); + continue; + } + /* Virtual port. */ + if (!strcasecmp(line->key, "HiddenServicePort")) { + char *err_msg = NULL; + /* XXX: Can we rename this? */ + rend_service_port_config_t *portcfg = + rend_service_parse_port_config(line->value, " ", &err_msg); + if (!portcfg) { + if (err_msg) { + log_warn(LD_CONFIG, "%s", err_msg); + } + tor_free(err_msg); + goto err; + } + tor_assert(!err_msg); + smartlist_add(config->ports, portcfg); + log_info(LD_CONFIG, "HiddenServicePort=%s for %s", + line->value, escaped(config->directory_path)); + continue; + } + /* Do we allow unknown ports. */ + if (!strcasecmp(line->key, "HiddenServiceAllowUnknownPorts")) { + config->allow_unknown_ports = (unsigned int) tor_parse_long(line->value, + 10, 0, 1, + &ok, NULL); + if (!ok) { + log_warn(LD_CONFIG, + "HiddenServiceAllowUnknownPorts should be 0 or 1, not %s", + line->value); + goto err; + } + log_info(LD_CONFIG, + "HiddenServiceAllowUnknownPorts=%u for %s", + config->allow_unknown_ports, escaped(config->directory_path)); + continue; + } + /* Directory group readable. */ + if (!strcasecmp(line->key, "HiddenServiceDirGroupReadable")) { + config->dir_group_readable = (unsigned int) tor_parse_long(line->value, + 10, 0, 1, + &ok, NULL); + if (!ok) { + log_warn(LD_CONFIG, + "HiddenServiceDirGroupReadable should be 0 or 1, not %s", + line->value); + goto err; + } + log_info(LD_CONFIG, + "HiddenServiceDirGroupReadable=%u for %s", + config->dir_group_readable, escaped(config->directory_path)); + continue; + } + /* Maximum streams per circuit. */ + if (!strcasecmp(line->key, "HiddenServiceMaxStreams")) { + config->max_streams_per_rdv_circuit = tor_parse_uint64(line->value, + 10, 0, 65535, + &ok, NULL); + if (!ok) { + log_warn(LD_CONFIG, + "HiddenServiceMaxStreams should be between 0 and %d, not %s", + 65535, line->value); + goto err; + } + log_info(LD_CONFIG, + "HiddenServiceMaxStreams=%" PRIu64 " for %s", + config->max_streams_per_rdv_circuit, + escaped(config->directory_path)); + continue; + } + /* Maximum amount of streams before we close the circuit. */ + if (!strcasecmp(line->key, "HiddenServiceMaxStreamsCloseCircuit")) { + config->max_streams_close_circuit = + (unsigned int) tor_parse_long(line->value, 10, 0, 1, &ok, NULL); + if (!ok) { + log_warn(LD_CONFIG, + "HiddenServiceMaxStreamsCloseCircuit should be 0 or 1, " + "not %s", line->value); + goto err; + } + log_info(LD_CONFIG, + "HiddenServiceMaxStreamsCloseCircuit=%u for %s", + config->max_streams_close_circuit, + escaped(config->directory_path)); + continue; + } + } + + /* Check permission on service directory. */ + if (hs_check_service_private_dir(options->User, config->directory_path, + config->dir_group_readable, 0) < 0) { + goto err; + } + + /* Check if we are configured in non anonymous mode and single hop mode + * meaning every service become single onion. */ + if (rend_service_allow_non_anonymous_connection(options) && + rend_service_non_anonymous_mode_enabled(options)) { + config->is_single_onion = 1; + } + + /* Success */ + return 0; + err: + return -1; +} + +/* Configuration handler indexed by version number. */ +static int + (*config_service_handlers[])(const config_line_t *line, + const or_options_t *options, + int validate_only, + hs_service_t *service) = +{ + NULL, /* v0 */ + NULL, /* v1 */ + rend_config_service, /* v2 */ + config_service_v3, /* v3 */ +}; + +/* From a set of options, setup every hidden service found. Return 0 on + * success or -1 on failure. If validate_only is set, parse, warn and + * return as normal, but don't actually change the configured services. */ +int +hs_config_service_all(const or_options_t *options, int validate_only) +{ + int dir_option_seen = 0; + hs_service_t *service = NULL; + const config_line_t *line; + + tor_assert(options); + + for (line = options->RendConfigLines; line; line = line->next) { + if (!strcasecmp(line->key, "HiddenServiceDir")) { + /* We have a new hidden service. */ + service = hs_service_new(options); + /* We'll configure that service as a generic one and then pass it to the + * specific handler according to the configured version number. */ + if (config_generic_service(line, options, service) < 0) { + goto err; + } + tor_assert(service->version <= HS_VERSION_MAX); + /* The handler is in charge of specific options for a version. We start + * after this service directory line so once we hit another directory + * line, the handler knows that it has to stop. */ + if (config_service_handlers[service->version](line->next, options, + validate_only, + service) < 0) { + goto err; + } + /* Whatever happens, on success we loose the ownership of the service + * object so we nullify the pointer to be safe. */ + service = NULL; + /* Flag that we've seen a directory directive and we'll use that to make + * sure that the torrc options ordering are actually valid. */ + dir_option_seen = 1; + continue; + } + /* The first line must be a directory option else tor is misconfigured. */ + if (!dir_option_seen) { + log_warn(LD_CONFIG, "%s with no preceding HiddenServiceDir directive", + line->key); + goto err; + } + } + + if (!validate_only) { + /* Trigger service pruning which will make sure the just configured + * services end up in the main global list. This is v2 specific. */ + rend_service_prune_list(); + /* XXX: Need the v3 one. */ + } + + /* Success. */ + return 0; + err: + hs_service_free(service); + /* Tor main should call the free all function. */ + return -1; +} + diff --git a/src/or/hs_config.h b/src/or/hs_config.h new file mode 100644 index 0000000000..08072d18d2 --- /dev/null +++ b/src/or/hs_config.h @@ -0,0 +1,19 @@ +/* Copyright (c) 2016, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file hs_config.h + * \brief Header file containing configuration ABI/API for the HS subsytem. + **/ + +#ifndef TOR_HS_CONFIG_H +#define TOR_HS_CONFIG_H + +#include "or.h" + +/* API */ + +int hs_config_service_all(const or_options_t *options, int validate_only); + +#endif /* TOR_HS_CONFIG_H */ + diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 205ef11c92..c62aa8b075 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -19,6 +19,91 @@ #include "hs/cell_establish_intro.h" #include "hs/cell_common.h" +/* Set the default values for a service configuration object c. */ +static void +set_service_default_config(hs_service_config_t *c, + const or_options_t *options) +{ + tor_assert(c); + c->ports = smartlist_new(); + c->directory_path = NULL; + c->descriptor_post_period = options->RendPostPeriod; + c->max_streams_per_rdv_circuit = 0; + c->max_streams_close_circuit = 0; + c->num_intro_points = NUM_INTRO_POINTS_DEFAULT; + c->allow_unknown_ports = 0; + c->is_single_onion = 0; + c->dir_group_readable = 0; + c->is_ephemeral = 0; +} + +/* Allocate and initilize a service object. The service configuration will + * contain the default values. Return the newly allocated object pointer. This + * function can't fail. */ +hs_service_t * +hs_service_new(const or_options_t *options) +{ + hs_service_t *service = tor_malloc_zero(sizeof(hs_service_t)); + /* Set default configuration value. */ + set_service_default_config(&service->config, options); + /* Set the default service version. */ + service->version = HS_SERVICE_DEFAULT_VERSION; + return service; +} + +/* Free the given service object and all its content. This function + * also takes care of wiping service keys from memory. It is safe to pass a + * NULL pointer. */ +void +hs_service_free(hs_service_t *service) +{ + if (service == NULL) { + return; + } + + /* Free descriptors. */ + if (service->desc_current) { + hs_descriptor_free(service->desc_current->desc); + /* Wipe keys. */ + memwipe(&service->desc_current->signing_kp, 0, + sizeof(service->desc_current->signing_kp)); + memwipe(&service->desc_current->blinded_kp, 0, + sizeof(service->desc_current->blinded_kp)); + /* XXX: Free intro points. */ + tor_free(service->desc_current); + } + if (service->desc_next) { + hs_descriptor_free(service->desc_next->desc); + /* Wipe keys. */ + memwipe(&service->desc_next->signing_kp, 0, + sizeof(service->desc_next->signing_kp)); + memwipe(&service->desc_next->blinded_kp, 0, + sizeof(service->desc_next->blinded_kp)); + /* XXX: Free intro points. */ + tor_free(service->desc_next); + } + + /* Free service configuration. */ + tor_free(service->config.directory_path); + if (service->config.ports) { + SMARTLIST_FOREACH(service->config.ports, rend_service_port_config_t *, p, + rend_service_port_config_free(p);); + smartlist_free(service->config.ports); + } + + /* Wipe service keys. */ + memwipe(&service->keys.identity_sk, 0, sizeof(service->keys.identity_sk)); + + tor_free(service); +} + +/* Release all global the storage of hidden service subsystem. */ +void +hs_service_free_all(void) +{ + rend_service_free_all(); +} + /* XXX We don't currently use these functions, apart from generating unittest data. When we start implementing the service-side support for prop224 we should revisit these functions and use them. */ diff --git a/src/or/hs_service.h b/src/or/hs_service.h index fa5dd541e3..d29a4782cd 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -192,6 +192,12 @@ typedef struct hs_service_t { /* API */ +int hs_service_config_all(const or_options_t *options, int validate_only); +void hs_service_free_all(void); + +void hs_service_free(hs_service_t *service); +hs_service_t *hs_service_new(const or_options_t *options); + /* These functions are only used by unit tests and we need to expose them else * hs_service.o ends up with no symbols in libor.a which makes clang throw a * warning at compile time. See #21825. */ diff --git a/src/or/include.am b/src/or/include.am index 2f9f1a9c43..15b86ef50b 100644 --- a/src/or/include.am +++ b/src/or/include.am @@ -50,19 +50,20 @@ LIBTOR_A_SOURCES = \ src/or/dnsserv.c \ src/or/fp_pair.c \ src/or/geoip.c \ + src/or/entrynodes.c \ + src/or/ext_orport.c \ + src/or/hibernate.c \ src/or/hs_cache.c \ - src/or/hs_circuitmap.c \ - src/or/hs_common.c \ src/or/hs_circuit.c \ + src/or/hs_circuitmap.c \ + src/or/hs_client.c \ + src/or/hs_common.c \ + src/or/hs_config.c \ src/or/hs_descriptor.c \ src/or/hs_ident.c \ src/or/hs_intropoint.c \ src/or/hs_ntor.c \ src/or/hs_service.c \ - src/or/hs_client.c \ - src/or/entrynodes.c \ - src/or/ext_orport.c \ - src/or/hibernate.c \ src/or/keypin.c \ src/or/main.c \ src/or/microdesc.c \ @@ -183,15 +184,16 @@ ORHEADERS = \ src/or/entrynodes.h \ src/or/hibernate.h \ src/or/hs_cache.h \ - src/or/hs_common.h \ src/or/hs_circuit.h \ + src/or/hs_circuitmap.h \ + src/or/hs_client.h \ + src/or/hs_common.h \ + src/or/hs_config.h \ src/or/hs_descriptor.h \ src/or/hs_ident.h \ - src/or/hs_intropoint.h \ - src/or/hs_circuitmap.h \ - src/or/hs_ntor.h \ - src/or/hs_service.h \ - src/or/hs_client.h \ + src/or/hs_intropoint.h \ + src/or/hs_ntor.h \ + src/or/hs_service.h \ src/or/keypin.h \ src/or/main.h \ src/or/microdesc.h \ diff --git a/src/or/main.c b/src/or/main.c index 5fa3869ff8..8c269fd934 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -3216,7 +3216,7 @@ tor_free_all(int postfork) networkstatus_free_all(); addressmap_free_all(); dirserv_free_all(); - rend_service_free_all(); + hs_service_free_all(); rend_cache_free_all(); rend_service_authorization_free_all(); hs_cache_free_all(); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index b8e704e54b..695668bf60 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -236,13 +236,18 @@ rend_service_free(rend_service_t *service) void rend_service_free_all(void) { - if (!rend_service_list) - return; - - SMARTLIST_FOREACH(rend_service_list, rend_service_t*, ptr, - rend_service_free(ptr)); - smartlist_free(rend_service_list); - rend_service_list = NULL; + if (rend_service_list) { + SMARTLIST_FOREACH(rend_service_list, rend_service_t*, ptr, + rend_service_free(ptr)); + smartlist_free(rend_service_list); + rend_service_list = NULL; + } + if (rend_service_staging_list) { + SMARTLIST_FOREACH(rend_service_staging_list, rend_service_t*, ptr, + rend_service_free(ptr)); + smartlist_free(rend_service_staging_list); + rend_service_staging_list = NULL; + } } /* Validate a service. Use the service_list to make sure there @@ -335,6 +340,7 @@ rend_add_service(smartlist_t *service_list, rend_service_t *service) /* We must have a service list, even if it's a temporary one, so we can * check for duplicate services */ if (BUG(!s_list)) { + rend_service_free(service); return -1; } @@ -496,41 +502,6 @@ rend_service_port_config_free(rend_service_port_config_t *p) tor_free(p); } -/* Check the directory for service, and add the service to - * service_list, or to the global list if service_list is NULL. - * Only add the service to the list if validate_only is false. - * If validate_only is true, free the service. - * If service is NULL, ignore it, and return 0. - * Returns 0 on success, and -1 on failure. - * Takes ownership of service, either freeing it, or adding it to the - * global service list. - */ -STATIC int -rend_service_check_dir_and_add(smartlist_t *service_list, - const or_options_t *options, - rend_service_t *service, - int validate_only) -{ - if (!service) { - /* It is ok for a service to be NULL, this means there are no services */ - return 0; - } - - if (rend_service_check_private_dir(options, service, !validate_only) - < 0) { - rend_service_free(service); - return -1; - } - - smartlist_t *s_list = rend_get_service_list_mutable(service_list); - /* We must have a service list, even if it's a temporary one, so we can - * check for duplicate services */ - if (BUG(!s_list)) { - return -1; - } - return rend_add_service(s_list, service); -} - /* Helper: Actual implementation of the pruning on reload which we've * decoupled in order to make the unit test workeable without ugly hacks. * Furthermore, this function does NOT free any memory but will nullify the @@ -657,19 +628,51 @@ rend_service_prune_list(void) } } -/** Set up rend_service_list, based on the values of HiddenServiceDir and - * HiddenServicePort in options. Return 0 on success and -1 on - * failure. (If validate_only is set, parse, warn and return as - * normal, but don't actually change the configured services.) - */ -int -rend_config_services(const or_options_t *options, int validate_only) +/* Copy all the relevant data that the hs_service object contains over to the + * rend_service_t object. The reason to do so is because when configuring a + * service, we go through a generic handler that creates an hs_service_t + * object which so we have to copy the parsed values to a rend service object + * which is version 2 specific. */ +static void +service_shadow_copy(rend_service_t *service, hs_service_t *hs_service) { - config_line_t *line; + hs_service_config_t *config; + + tor_assert(service); + tor_assert(hs_service); + + config = &hs_service->config; + service->directory = tor_strdup(config->directory_path); + service->dir_group_readable = config->dir_group_readable; + service->allow_unknown_ports = config->allow_unknown_ports; + service->max_streams_per_circuit = config->max_streams_per_rdv_circuit; + service->max_streams_close_circuit = config->max_streams_close_circuit; + service->n_intro_points_wanted = config->num_intro_points; + /* Switching ownership of the ports to the rend service object. */ + smartlist_add_all(service->ports, config->ports); + smartlist_free(hs_service->config.ports); + hs_service->config.ports = NULL; +} + +/* Parse the hidden service configuration starting at line_ using the + * already configured generic service in hs_service. This function will + * translate the service object to a rend_service_t and add it to the + * temporary list if valid. If validate_only is set, parse, warn and + * return as normal but don't actually add the service to the list. */ +int +rend_config_service(const config_line_t *line_, + const or_options_t *options, + int validate_only, + hs_service_t *hs_service) +{ + (void) validate_only; + const config_line_t *line; rend_service_t *service = NULL; - rend_service_port_config_t *portcfg; - int ok = 0; - int rv = -1; + + /* line_ can be NULL which would mean that the service configuration only + * have one line that is the directory directive. */ + tor_assert(options); + tor_assert(hs_service); /* Use the staging service list so that we can check then do the pruning * process using the main list at the end. */ @@ -677,100 +680,23 @@ rend_config_services(const or_options_t *options, int validate_only) rend_service_staging_list = smartlist_new(); } - for (line = options->RendConfigLines; line; line = line->next) { + /* Initialize service. */ + service = tor_malloc_zero(sizeof(rend_service_t)); + service->intro_period_started = time(NULL); + service->ports = smartlist_new(); + /* From the hs_service object which has been used to load the generic + * options, we'll copy over the useful data to the rend_service_t object. */ + service_shadow_copy(service, hs_service); + + for (line = line_; line; line = line->next) { if (!strcasecmp(line->key, "HiddenServiceDir")) { - if (service) { - /* Validate and register the service we just finished parsing this - * code registers every service except the last one parsed, which is - * validated and registered below the loop. */ - if (rend_validate_service(rend_service_staging_list, service) < 0) { - goto free_and_return; - } - if (rend_service_check_dir_and_add(rend_service_staging_list, options, - service, validate_only) < 0) { - /* The above frees the service on error so nullify the pointer. */ - service = NULL; - goto free_and_return; - } - } - service = tor_malloc_zero(sizeof(rend_service_t)); - service->directory = tor_strdup(line->value); - service->ports = smartlist_new(); - service->intro_period_started = time(NULL); - service->n_intro_points_wanted = NUM_INTRO_POINTS_DEFAULT; - continue; + /* We just hit the next hidden service, stop right now. */ + break; } - if (!service) { - log_warn(LD_CONFIG, "%s with no preceding HiddenServiceDir directive", - line->key); - goto free_and_return; - } - if (!strcasecmp(line->key, "HiddenServicePort")) { - char *err_msg = NULL; - portcfg = rend_service_parse_port_config(line->value, " ", &err_msg); - if (!portcfg) { - if (err_msg) - log_warn(LD_CONFIG, "%s", err_msg); - tor_free(err_msg); - goto free_and_return; - } - tor_assert(!err_msg); - smartlist_add(service->ports, portcfg); - } else if (!strcasecmp(line->key, "HiddenServiceAllowUnknownPorts")) { - service->allow_unknown_ports = (int)tor_parse_long(line->value, - 10, 0, 1, &ok, NULL); - if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceAllowUnknownPorts should be 0 or 1, not %s", - line->value); - goto free_and_return; - } - log_info(LD_CONFIG, - "HiddenServiceAllowUnknownPorts=%d for %s", - (int)service->allow_unknown_ports, - rend_service_escaped_dir(service)); - } else if (!strcasecmp(line->key, - "HiddenServiceDirGroupReadable")) { - service->dir_group_readable = (int)tor_parse_long(line->value, - 10, 0, 1, &ok, NULL); - if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceDirGroupReadable should be 0 or 1, not %s", - line->value); - goto free_and_return; - } - log_info(LD_CONFIG, - "HiddenServiceDirGroupReadable=%d for %s", - service->dir_group_readable, - rend_service_escaped_dir(service)); - } else if (!strcasecmp(line->key, "HiddenServiceMaxStreams")) { - service->max_streams_per_circuit = (int)tor_parse_long(line->value, - 10, 0, 65535, &ok, NULL); - if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceMaxStreams should be between 0 and %d, not %s", - 65535, line->value); - goto free_and_return; - } - log_info(LD_CONFIG, - "HiddenServiceMaxStreams=%d for %s", - service->max_streams_per_circuit, - rend_service_escaped_dir(service)); - } else if (!strcasecmp(line->key, "HiddenServiceMaxStreamsCloseCircuit")) { - service->max_streams_close_circuit = (int)tor_parse_long(line->value, - 10, 0, 1, &ok, NULL); - if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceMaxStreamsCloseCircuit should be 0 or 1, " - "not %s", - line->value); - goto free_and_return; - } - log_info(LD_CONFIG, - "HiddenServiceMaxStreamsCloseCircuit=%d for %s", - (int)service->max_streams_close_circuit, - rend_service_escaped_dir(service)); - } else if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { + /* Number of introduction points. */ + if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { + int ok = 0; + /* Those are specific defaults for version 2. */ service->n_intro_points_wanted = (unsigned int) tor_parse_long(line->value, 10, 0, NUM_INTRO_POINTS_MAX, &ok, NULL); @@ -779,12 +705,13 @@ rend_config_services(const or_options_t *options, int validate_only) "HiddenServiceNumIntroductionPoints " "should be between %d and %d, not %s", 0, NUM_INTRO_POINTS_MAX, line->value); - goto free_and_return; + goto err; } log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", - service->n_intro_points_wanted, - rend_service_escaped_dir(service)); - } else if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) { + service->n_intro_points_wanted, escaped(service->directory)); + continue; + } + if (!strcasecmp(line->key, "HiddenServiceAuthorizeClient")) { /* Parse auth type and comma-separated list of client names and add a * rend_authorized_client_t for each client to the service's list * of authorized clients. */ @@ -794,7 +721,7 @@ rend_config_services(const or_options_t *options, int validate_only) if (service->auth_type != REND_NO_AUTH) { log_warn(LD_CONFIG, "Got multiple HiddenServiceAuthorizeClient " "lines for a single service."); - goto free_and_return; + goto err; } type_names_split = smartlist_new(); smartlist_split_string(type_names_split, line->value, " ", 0, 2); @@ -802,7 +729,8 @@ rend_config_services(const or_options_t *options, int validate_only) log_warn(LD_BUG, "HiddenServiceAuthorizeClient has no value. This " "should have been prevented when parsing the " "configuration."); - goto free_and_return; + smartlist_free(type_names_split); + goto err; } authname = smartlist_get(type_names_split, 0); if (!strcasecmp(authname, "basic")) { @@ -816,7 +744,7 @@ rend_config_services(const or_options_t *options, int validate_only) (char *) smartlist_get(type_names_split, 0)); SMARTLIST_FOREACH(type_names_split, char *, cp, tor_free(cp)); smartlist_free(type_names_split); - goto free_and_return; + goto err; } service->clients = smartlist_new(); if (smartlist_len(type_names_split) < 2) { @@ -853,7 +781,7 @@ rend_config_services(const or_options_t *options, int validate_only) client_name, REND_CLIENTNAME_MAX_LEN); SMARTLIST_FOREACH(clients, char *, cp, tor_free(cp)); smartlist_free(clients); - goto free_and_return; + goto err; } client = tor_malloc_zero(sizeof(rend_authorized_client_t)); client->client_name = tor_strdup(client_name); @@ -875,56 +803,29 @@ rend_config_services(const or_options_t *options, int validate_only) smartlist_len(service->clients), service->auth_type == REND_BASIC_AUTH ? 512 : 16, service->auth_type == REND_BASIC_AUTH ? "basic" : "stealth"); - goto free_and_return; - } - } else { - tor_assert(!strcasecmp(line->key, "HiddenServiceVersion")); - if (strcmp(line->value, "2")) { - log_warn(LD_CONFIG, - "The only supported HiddenServiceVersion is 2."); - goto free_and_return; + goto err; } + continue; } } - /* Validate the last service that we just parsed. */ - if (service && - rend_validate_service(rend_service_staging_list, service) < 0) { - goto free_and_return; + /* Validate the service just parsed. */ + if (rend_validate_service(rend_service_staging_list, service) < 0) { + /* Service is in the staging list so don't try to free it. */ + goto err; } - /* register the final service after we have finished parsing all services - * this code only registers the last service, other services are registered - * within the loop. It is ok for this service to be NULL, it is ignored. */ - if (rend_service_check_dir_and_add(rend_service_staging_list, options, - service, validate_only) < 0) { - /* Service object is freed on error so nullify pointer. */ + + /* Add it to the temporary list which we will use to prune our current + * list if any after configuring all services. */ + if (rend_add_service(rend_service_staging_list, service) < 0) { + /* The object has been freed on error already. */ service = NULL; - goto free_and_return; + goto err; } - /* The service is in the staging list so nullify pointer to avoid double - * free of this object in case of error because we lost ownership of it at - * this point. */ - service = NULL; - - /* Free the newly added services if validating */ - if (validate_only) { - rv = 0; - goto free_and_return; - } - - /* This could be a reload of configuration so try to prune the main list - * using the staging one. And we know we are not in validate mode here. - * After this, the main and staging list will point to the right place and - * be in a quiescent usable state. */ - rend_service_prune_list(); return 0; - free_and_return: + err: rend_service_free(service); - SMARTLIST_FOREACH(rend_service_staging_list, rend_service_t *, ptr, - rend_service_free(ptr)); - smartlist_free(rend_service_staging_list); - rend_service_staging_list = NULL; - return rv; + return -1; } /** Add the ephemeral service pk/ports if possible, using @@ -1548,9 +1449,9 @@ rend_service_load_keys(rend_service_t *s) char *fname = NULL; char buf[128]; - /* Make sure the directory was created and single onion poisoning was - * checked before calling this function */ - if (BUG(rend_service_check_private_dir(get_options(), s, 0) < 0)) + /* Create the directory if needed which will also poison it in case of + * single onion service. */ + if (rend_service_check_private_dir(get_options(), s, 1) < 0) goto err; /* Load key */ diff --git a/src/or/rendservice.h b/src/or/rendservice.h index 1583a6010b..90f854e4bc 100644 --- a/src/or/rendservice.h +++ b/src/or/rendservice.h @@ -13,6 +13,7 @@ #define TOR_RENDSERVICE_H #include "or.h" +#include "hs_service.h" typedef struct rend_intro_cell_s rend_intro_cell_t; typedef struct rend_service_port_config_s rend_service_port_config_t; @@ -119,10 +120,6 @@ typedef struct rend_service_t { STATIC void rend_service_free(rend_service_t *service); STATIC char *rend_service_sos_poison_path(const rend_service_t *service); -STATIC int rend_service_check_dir_and_add(smartlist_t *service_list, - const or_options_t *options, - rend_service_t *service, - int validate_only); STATIC int rend_service_verify_single_onion_poison( const rend_service_t *s, const or_options_t *options); @@ -144,7 +141,10 @@ STATIC void rend_service_prune_list_impl_(void); #endif /* RENDSERVICE_PRIVATE */ int num_rend_services(void); -int rend_config_services(const or_options_t *options, int validate_only); +int rend_config_service(const config_line_t *line_, + const or_options_t *options, + int validate_only, + hs_service_t *hs_service); void rend_service_prune_list(void); int rend_service_load_all_keys(const smartlist_t *service_list); void rend_services_add_filenames_to_lists(smartlist_t *open_lst, diff --git a/src/test/test_hs.c b/src/test/test_hs.c index 178f94d273..093c7ec35e 100644 --- a/src/test/test_hs.c +++ b/src/test/test_hs.c @@ -10,6 +10,7 @@ #define CIRCUITBUILD_PRIVATE #define RENDCOMMON_PRIVATE #define RENDSERVICE_PRIVATE +#define HS_SERVICE_PRIVATE #include "or.h" #include "test.h" @@ -661,17 +662,8 @@ test_single_onion_poisoning(void *arg) smartlist_t *services = smartlist_new(); char *poison_path = NULL; - /* No services, no service to verify, no problem! */ - mock_options->HiddenServiceSingleHopMode = 0; - mock_options->HiddenServiceNonAnonymousMode = 0; - ret = rend_config_services(mock_options, 1); - tt_assert(ret == 0); - - /* Either way, no problem. */ mock_options->HiddenServiceSingleHopMode = 1; mock_options->HiddenServiceNonAnonymousMode = 1; - ret = rend_config_services(mock_options, 1); - tt_assert(ret == 0); /* Create the data directory, and, if the correct bit in arg is set, * create a directory for that service. @@ -726,8 +718,10 @@ test_single_onion_poisoning(void *arg) tt_assert(ret == 0); /* Add the first service */ - ret = rend_service_check_dir_and_add(services, mock_options, service_1, 0); - tt_assert(ret == 0); + ret = hs_check_service_private_dir(mock_options->User, service_1->directory, + service_1->dir_group_readable, 1); + tt_int_op(ret, OP_EQ, 0); + smartlist_add(services, service_1); /* But don't add the second service yet. */ /* Service directories, but no previous keys, no problem! */ @@ -805,8 +799,10 @@ test_single_onion_poisoning(void *arg) tt_assert(ret == 0); /* Now add the second service: it has no key and no poison file */ - ret = rend_service_check_dir_and_add(services, mock_options, service_2, 0); - tt_assert(ret == 0); + ret = hs_check_service_private_dir(mock_options->User, service_2->directory, + service_2->dir_group_readable, 1); + tt_int_op(ret, OP_EQ, 0); + smartlist_add(services, service_2); /* A new service, and an existing poisoned service. Not ok. */ mock_options->HiddenServiceSingleHopMode = 0; From 765ed5dac160b28fb658560e8f39d1d7ab3d1c75 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 16 Jan 2017 13:29:03 -0500 Subject: [PATCH 04/22] prop224: Add a init/free_all function for the whole subsystem Introduces hs_init() located in hs_common.c which initialize the entire HS v3 subsystem. This is done _prior_ to the options being loaded because we need to allocate global data structure before we load the configuration. The hs_free_all() is added to release everything from tor_free_all(). Note that both functions do NOT handle v2 service subsystem but does handle the common interface that both v2 and v3 needs such as the cache and circuitmap. Signed-off-by: David Goulet --- src/or/hs_common.c | 22 ++++++++++++++++++++++ src/or/hs_common.h | 3 +++ src/or/hs_service.c | 7 +++++++ src/or/hs_service.h | 1 + src/or/main.c | 10 +++------- 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 42508126f8..b524296159 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -15,7 +15,9 @@ #include "config.h" #include "networkstatus.h" +#include "hs_cache.h" #include "hs_common.h" +#include "hs_service.h" #include "rendcommon.h" /* Make sure that the directory for service is private, using the config @@ -344,3 +346,23 @@ rend_data_get_pk_digest(const rend_data_t *rend_data, size_t *len_out) } } +/* Initialize the entire HS subsytem. This is called in tor_init() before any + * torrc options are loaded. Only for >= v3. */ +void +hs_init(void) +{ + hs_circuitmap_init(); + hs_service_init(); + hs_cache_init(); +} + +/* Release and cleanup all memory of the HS subsystem (all version). This is + * called by tor_free_all(). */ +void +hs_free_all(void) +{ + hs_circuitmap_free_all(); + hs_service_free_all(); + hs_cache_free_all(); +} + diff --git a/src/or/hs_common.h b/src/or/hs_common.h index abc44c09d1..8016535ca9 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -58,6 +58,9 @@ typedef enum { HS_AUTH_KEY_TYPE_ED25519 = 2, } hs_auth_key_type_t; +void hs_init(void); +void hs_free_all(void); + int hs_check_service_private_dir(const char *username, const char *path, unsigned int dir_group_readable, unsigned int create); diff --git a/src/or/hs_service.c b/src/or/hs_service.c index c62aa8b075..16ffc4885c 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -97,6 +97,13 @@ hs_service_free(hs_service_t *service) tor_free(service); } +/* Initialize the service HS subsystem. */ +void +hs_service_init(void) +{ + return; +} + /* Release all global the storage of hidden service subsystem. */ void hs_service_free_all(void) diff --git a/src/or/hs_service.h b/src/or/hs_service.h index d29a4782cd..ec47cb72ae 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -193,6 +193,7 @@ typedef struct hs_service_t { /* API */ int hs_service_config_all(const or_options_t *options, int validate_only); +void hs_service_init(void); void hs_service_free_all(void); void hs_service_free(hs_service_t *service); diff --git a/src/or/main.c b/src/or/main.c index 8c269fd934..204b3f3356 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2499,9 +2499,6 @@ do_main_loop(void) } } - /* Initialize relay-side HS circuitmap */ - hs_circuitmap_init(); - /* set up once-a-second callback. */ if (! second_timer) { struct timeval one_second; @@ -3014,9 +3011,10 @@ tor_init(int argc, char *argv[]) rep_hist_init(); /* Initialize the service cache. */ rend_cache_init(); - hs_cache_init(); addressmap_init(); /* Init the client dns cache. Do it always, since it's * cheap. */ + /* Initialize the HS subsystem. */ + hs_init(); { /* We search for the "quiet" option first, since it decides whether we @@ -3216,10 +3214,8 @@ tor_free_all(int postfork) networkstatus_free_all(); addressmap_free_all(); dirserv_free_all(); - hs_service_free_all(); rend_cache_free_all(); rend_service_authorization_free_all(); - hs_cache_free_all(); rep_hist_free_all(); dns_free_all(); clear_pending_onions(); @@ -3232,7 +3228,6 @@ tor_free_all(int postfork) connection_edge_free_all(); scheduler_free_all(); nodelist_free_all(); - hs_circuitmap_free_all(); microdesc_free_all(); routerparse_free_all(); ext_orport_free_all(); @@ -3241,6 +3236,7 @@ tor_free_all(int postfork) protover_free_all(); bridges_free_all(); consdiffmgr_free_all(); + hs_free_all(); if (!postfork) { config_free_all(); or_state_free_all(); From 74193b932115a82417dc312721ffe0a10a7ed6dc Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 10 May 2017 09:37:41 -0400 Subject: [PATCH 05/22] hs: Use v3 maximum intro points value when decoding v3 Signed-off-by: David Goulet --- src/or/hs_config.h | 3 +++ src/or/hs_descriptor.c | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/or/hs_config.h b/src/or/hs_config.h index 08072d18d2..f4207917e5 100644 --- a/src/or/hs_config.h +++ b/src/or/hs_config.h @@ -11,6 +11,9 @@ #include "or.h" +/* Maximum number of intro points per version 3 services. */ +#define HS_CONFIG_V3_MAX_INTRO_POINTS 20 + /* API */ int hs_config_service_all(const or_options_t *options, int validate_only); diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index b55f9663bc..2393eac252 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -62,6 +62,7 @@ #include "parsecommon.h" #include "rendcache.h" #include "hs_cache.h" +#include "hs_config.h" #include "torcert.h" /* tor_cert_encode_ed22519() */ /* Constant string value used for the descriptor format. */ @@ -2035,10 +2036,11 @@ desc_decode_encrypted_v3(const hs_descriptor_t *desc, decode_intro_points(desc, desc_encrypted_out, message); /* Validation of maximum introduction points allowed. */ - if (smartlist_len(desc_encrypted_out->intro_points) > MAX_INTRO_POINTS) { + if (smartlist_len(desc_encrypted_out->intro_points) > + HS_CONFIG_V3_MAX_INTRO_POINTS) { log_warn(LD_REND, "Service descriptor contains too many introduction " "points. Maximum allowed is %d but we have %d", - MAX_INTRO_POINTS, + HS_CONFIG_V3_MAX_INTRO_POINTS, smartlist_len(desc_encrypted_out->intro_points)); goto err; } From 93774dcb5458115652e0be5cdfaf198967b8a31e Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 13 Jan 2017 11:20:31 -0500 Subject: [PATCH 06/22] test: Add HS v2 service configuration unit tests Signed-off-by: David Goulet --- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_helpers.c | 42 +++++++- src/test/test_helpers.h | 1 + src/test/test_hs_config.c | 198 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 src/test/test_hs_config.c diff --git a/src/test/include.am b/src/test/include.am index e7a2e0278b..2e448c8b39 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -114,6 +114,7 @@ src_test_test_SOURCES = \ src/test/test_guardfraction.c \ src/test/test_extorport.c \ src/test/test_hs.c \ + src/test/test_hs_config.c \ src/test/test_hs_service.c \ src/test/test_hs_client.c \ src/test/test_hs_intropoint.c \ diff --git a/src/test/test.c b/src/test/test.c index 31b3db3a4f..b6b11ce94a 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1213,6 +1213,7 @@ struct testgroup_t testgroups[] = { { "extorport/", extorport_tests }, { "legacy_hs/", hs_tests }, { "hs_cache/", hs_cache }, + { "hs_config/", hs_config_tests }, { "hs_descriptor/", hs_descriptor }, { "hs_service/", hs_service_tests }, { "hs_client/", hs_client_tests }, diff --git a/src/test/test.h b/src/test/test.h index 4de0da99fb..9b2a0b842f 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -207,6 +207,7 @@ extern struct testcase_t guardfraction_tests[]; extern struct testcase_t extorport_tests[]; extern struct testcase_t hs_tests[]; extern struct testcase_t hs_cache[]; +extern struct testcase_t hs_config_tests[]; extern struct testcase_t hs_descriptor[]; extern struct testcase_t hs_service_tests[]; extern struct testcase_t hs_client_tests[]; diff --git a/src/test/test_helpers.c b/src/test/test_helpers.c index 22d9de3f5b..e885d27815 100644 --- a/src/test/test_helpers.c +++ b/src/test/test_helpers.c @@ -7,18 +7,21 @@ */ #define ROUTERLIST_PRIVATE +#define CONFIG_PRIVATE #define CONNECTION_PRIVATE #define MAIN_PRIVATE #include "orconfig.h" #include "or.h" +#include "buffers.h" +#include "config.h" +#include "confparse.h" #include "connection.h" #include "main.h" +#include "nodelist.h" #include "relay.h" #include "routerlist.h" -#include "nodelist.h" -#include "buffers.h" #include "test.h" #include "test_helpers.h" @@ -239,3 +242,38 @@ test_conn_get_connection(uint8_t state, uint8_t type, uint8_t purpose) return NULL; } +/* Helper function to parse a set of torrc options in a text format and return + * a newly allocated or_options_t object containing the configuration. On + * error, NULL is returned indicating that the conf couldn't be parsed + * properly. */ +or_options_t * +helper_parse_options(const char *conf) +{ + int ret = 0; + char *msg = NULL; + or_options_t *opt = NULL; + config_line_t *line = NULL; + + /* Kind of pointless to call this with a NULL value. */ + tt_assert(conf); + + opt = options_new(); + tt_assert(opt); + ret = config_get_lines(conf, &line, 1); + if (ret != 0) { + goto done; + } + ret = config_assign(&options_format, opt, line, 0, &msg); + if (ret != 0) { + goto done; + } + + done: + config_free_lines(line); + if (ret != 0) { + or_options_free(opt); + opt = NULL; + } + return opt; +} + diff --git a/src/test/test_helpers.h b/src/test/test_helpers.h index 96a4b594eb..847104a40a 100644 --- a/src/test/test_helpers.h +++ b/src/test/test_helpers.h @@ -24,6 +24,7 @@ int mock_tor_addr_lookup__fail_on_bad_addrs(const char *name, connection_t *test_conn_get_connection(uint8_t state, uint8_t type, uint8_t purpose); +or_options_t *helper_parse_options(const char *conf); extern const char TEST_DESCRIPTORS[]; diff --git a/src/test/test_hs_config.c b/src/test/test_hs_config.c new file mode 100644 index 0000000000..18b11948a4 --- /dev/null +++ b/src/test/test_hs_config.c @@ -0,0 +1,198 @@ +/* Copyright (c) 2016, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file test_hs_config.c + * \brief Test hidden service configuration functionality. + */ + +#define CONFIG_PRIVATE + +#include "test.h" +#include "test_helpers.h" +#include "log_test_helpers.h" +#include "hs_config.h" +#include "config.h" + +static int +helper_config_service_v2(const char *conf, int validate_only) +{ + int ret = 0; + or_options_t *options = NULL; + tt_assert(conf); + options = helper_parse_options(conf); + tt_assert(options); + ret = hs_config_service_all(options, validate_only); + done: + or_options_free(options); + return ret; +} + +static void +test_invalid_service_v2(void *arg) +{ + int validate_only = 1, ret; + + (void) arg; + + /* Try with a missing port configuration. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service_v2(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("with no ports configured."); + teardown_capture_of_logs(); + } + + /* Out of order directives. */ + { + const char *conf = + "HiddenServiceVersion 2\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServicePort 80\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service_v2(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceVersion with no preceding " + "HiddenServiceDir directive"); + teardown_capture_of_logs(); + } + + /* Bad port. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 65536\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service_v2(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("Missing or invalid port"); + teardown_capture_of_logs(); + } + + /* Too many introduction points. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceNumIntroductionPoints 11\n"; /* One too many. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service_v2(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceNumIntroductionPoints should " + "be between 0 and 10, not 11"); + teardown_capture_of_logs(); + } + + /* Too much max streams. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceMaxStreams 65536\n"; /* One too many. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service_v2(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceMaxStreams should be between " + "0 and 65535, not 65536"); + teardown_capture_of_logs(); + } + + /* Bad authorized client type. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceAuthorizeClient blah alice,bob\n"; /* blah is no good. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service_v2(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceAuthorizeClient contains " + "unrecognized auth-type"); + teardown_capture_of_logs(); + } + + /* Duplicate directory directive. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 81\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service_v2(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("Another hidden service is already " + "configured for directory"); + teardown_capture_of_logs(); + } + + done: + ; +} + +static void +test_valid_service_v2(void *arg) +{ + int ret; + + (void) arg; + + /* Valid complex configuration. Basic client authorization. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServicePort 22 localhost:22\n" + "HiddenServicePort 42 unix:/path/to/socket\n" + "HiddenServiceAuthorizeClient basic alice,bob,eve\n" + "HiddenServiceAllowUnknownPorts 1\n" + "HiddenServiceMaxStreams 42\n" + "HiddenServiceMaxStreamsCloseCircuit 0\n" + "HiddenServiceDirGroupReadable 1\n" + "HiddenServiceNumIntroductionPoints 7\n"; + ret = helper_config_service_v2(conf, 1); + tt_int_op(ret, OP_EQ, 0); + } + + /* Valid complex configuration. Stealth client authorization. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs2\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 65535\n" + "HiddenServicePort 22 1.1.1.1:22\n" + "HiddenServicePort 9000 unix:/path/to/socket\n" + "HiddenServiceAuthorizeClient stealth charlie,romeo\n" + "HiddenServiceAllowUnknownPorts 0\n" + "HiddenServiceMaxStreams 42\n" + "HiddenServiceMaxStreamsCloseCircuit 0\n" + "HiddenServiceDirGroupReadable 1\n" + "HiddenServiceNumIntroductionPoints 8\n"; + ret = helper_config_service_v2(conf, 1); + tt_int_op(ret, OP_EQ, 0); + } + + done: + ; +} + +struct testcase_t hs_config_tests[] = { + { "invalid_service_v2", test_invalid_service_v2, TT_FORK, + NULL, NULL }, + { "valid_service_v2", test_valid_service_v2, TT_FORK, + NULL, NULL }, + + END_OF_TESTCASES +}; + From c086a59ea1fe63e38b6f83fa0c2c19bf495e977d Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 13 Jan 2017 16:00:07 -0500 Subject: [PATCH 07/22] prop224: Configure v3 service from options This commit adds the support in the HS subsystem for loading a service from a set of or_options_t and put them in a staging list. To achieve this, service accessors have been created and a global hash map containing service object indexed by master public key. However, this is not used for now. It's ground work for registration process. Signed-off-by: David Goulet --- src/or/circuitlist.h | 1 + src/or/hs_common.h | 2 +- src/or/hs_config.c | 318 +++++++++++++++++++++++++++++++------- src/or/hs_config.h | 2 + src/or/hs_service.c | 360 ++++++++++++++++++++++++++++++++++++++++++- src/or/hs_service.h | 15 +- src/or/rendservice.c | 55 ++----- src/or/rendservice.h | 2 +- 8 files changed, 648 insertions(+), 107 deletions(-) diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index d647062f46..2f76252563 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -48,6 +48,7 @@ origin_circuit_t *circuit_get_ready_rend_circ_by_rend_data( origin_circuit_t *circuit_get_next_by_pk_and_purpose(origin_circuit_t *start, const uint8_t *digest, uint8_t purpose); origin_circuit_t *circuit_get_next_service_intro_circ(origin_circuit_t *start); +origin_circuit_t *circuit_get_next_service_hsdir_circ(origin_circuit_t *start); origin_circuit_t *circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, int flags); void circuit_mark_all_unused_circs(void); diff --git a/src/or/hs_common.h b/src/or/hs_common.h index 8016535ca9..d1bc5ac7ef 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -22,7 +22,7 @@ /** Try to maintain this many intro points per service by default. */ #define NUM_INTRO_POINTS_DEFAULT 3 -/** Maximum number of intro points per service. */ +/** Maximum number of intro points per generic and version 2 service. */ #define NUM_INTRO_POINTS_MAX 10 /** Number of extra intro points we launch if our set of intro nodes is empty. * See proposal 155, section 4. */ diff --git a/src/or/hs_config.c b/src/or/hs_config.c index 6326e90324..6bb422dbf1 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -31,20 +31,171 @@ #include "hs_service.h" #include "rendservice.h" -/* Configuration handler for a version 3 service. Return 0 on success else a - * negative value. */ +/* Using the given list of services, stage them into our global state. Every + * service version are handled. This function can remove entries in the given + * service_list. + * + * Staging a service means that we take all services in service_list and we + * put them in the staging list (global) which acts as a temporary list that + * is used by the service loading key process. In other words, staging a + * service puts it in a list to be considered when loading the keys and then + * moved to the main global list. */ +static void +stage_services(smartlist_t *service_list) +{ + tor_assert(service_list); + + /* This is v2 specific. Trigger service pruning which will make sure the + * just configured services end up in the main global list. It should only + * be done in non validation mode because v2 subsystem handles service + * object differently. */ + rend_service_prune_list(); + + /* Cleanup v2 service from the list, we don't need those object anymore + * because we validated them all against the others and we want to stage + * only >= v3 service. And remember, v2 has a different object type which is + * shadow copied from an hs_service_t type. */ + SMARTLIST_FOREACH_BEGIN(service_list, hs_service_t *, s) { + if (s->version == HS_VERSION_TWO) { + SMARTLIST_DEL_CURRENT(service_list, s); + hs_service_free(s); + } + } SMARTLIST_FOREACH_END(s); + + /* This is >= v3 specific. Using the newly configured service list, stage + * them into our global state. Every object ownership is lost after. */ + hs_service_stage_services(service_list); +} + +/* Validate the given service against all service in the given list. If the + * service is ephemeral, this function ignores it. Services with the same + * directory path aren't allowed and will return an error. If a duplicate is + * found, 1 is returned else 0 if none found. */ static int -config_service_v3(const config_line_t *line, - const or_options_t *options, int validate_only, +service_is_duplicate_in_list(const smartlist_t *service_list, + const hs_service_t *service) +{ + int ret = 0; + + tor_assert(service_list); + tor_assert(service); + + /* Ephemeral service don't have a directory configured so no need to check + * for a service in the list having the same path. */ + if (service->config.is_ephemeral) { + goto end; + } + + /* XXX: Validate if we have any service that has the given service dir path. + * This has two problems: + * + * a) It's O(n^2), but the same comment from the bottom of + * rend_config_services() should apply. + * + * b) We only compare directory paths as strings, so we can't + * detect two distinct paths that specify the same directory + * (which can arise from symlinks, case-insensitivity, bind + * mounts, etc.). + * + * It also can't detect that two separate Tor instances are trying + * to use the same HiddenServiceDir; for that, we would need a + * lock file. But this is enough to detect a simple mistake that + * at least one person has actually made. */ + SMARTLIST_FOREACH_BEGIN(service_list, const hs_service_t *, s) { + if (!strcmp(s->config.directory_path, service->config.directory_path)) { + log_warn(LD_REND, "Another hidden service is already configured " + "for directory %s", + escaped(service->config.directory_path)); + ret = 1; + goto end; + } + } SMARTLIST_FOREACH_END(s); + + end: + return ret; +} + +/* Validate service configuration. This is used when loading the configuration + * and once we've setup a service object, it's config object is passed to this + * function for further validation. This does not validate service key + * material. Return 0 if valid else -1 if invalid. */ +static int +config_validate_service(const hs_service_config_t *config) +{ + tor_assert(config); + + /* Amount of ports validation. */ + if (!config->ports || smartlist_len(config->ports) == 0) { + log_warn(LD_CONFIG, "Hidden service (%s) with no ports configured.", + escaped(config->directory_path)); + goto invalid; + } + + /* Valid. */ + return 0; + invalid: + return -1; +} + +/* Configuration handler for a version 3 service. The line_ must be pointing + * to the directive directly after a HiddenServiceDir. That way, when hitting + * the next HiddenServiceDir line or reaching the end of the list of lines, we + * know that we have to stop looking for more options. The given service + * object must be already allocated and passed through + * config_generic_service() prior to calling this function. + * + * Return 0 on success else a negative value. */ +static int +config_service_v3(const config_line_t *line_, + const or_options_t *options, hs_service_t *service) { - (void) line; - (void) service; - (void) validate_only; (void) options; - /* XXX: Configure a v3 service with specific options. */ - /* XXX: Add service to v3 list and pruning on reload. */ + const config_line_t *line; + hs_service_config_t *config; + + tor_assert(service); + + config = &service->config; + + for (line = line_; line; line = line->next) { + if (!strcasecmp(line->key, "HiddenServiceDir")) { + /* We just hit the next hidden service, stop right now. */ + break; + } + /* Number of introduction points. */ + if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { + int ok = 0; + config->num_intro_points = + (unsigned int) tor_parse_ulong(line->value, 10, + NUM_INTRO_POINTS_DEFAULT, + HS_CONFIG_V3_MAX_INTRO_POINTS, + &ok, NULL); + if (!ok) { + log_warn(LD_CONFIG, "HiddenServiceNumIntroductionPoints " + "should be between %d and %d, not %s", + NUM_INTRO_POINTS_DEFAULT, HS_CONFIG_V3_MAX_INTRO_POINTS, + line->value); + goto err; + } + log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", + config->num_intro_points, escaped(config->directory_path)); + continue; + } + } + + /* We do not load the key material for the service at this stage. This is + * done later once tor can confirm that it is in a running state. */ + + /* We are about to return a fully configured service so do one last pass of + * validation at it. */ + if (config_validate_service(config) < 0) { + goto err; + } + return 0; + err: + return -1; } /* Configure a service using the given options in line_ and options. This is @@ -98,7 +249,7 @@ config_generic_service(const config_line_t *line_, /* Version of the service. */ if (!strcasecmp(line->key, "HiddenServiceVersion")) { service->version = (uint32_t) tor_parse_ulong(line->value, - 10, HS_VERSION_TWO, + 10, HS_VERSION_MIN, HS_VERSION_MAX, &ok, NULL); if (!ok) { @@ -164,13 +315,13 @@ config_generic_service(const config_line_t *line_, } /* Maximum streams per circuit. */ if (!strcasecmp(line->key, "HiddenServiceMaxStreams")) { - config->max_streams_per_rdv_circuit = tor_parse_uint64(line->value, - 10, 0, 65535, - &ok, NULL); + config->max_streams_per_rdv_circuit = + tor_parse_uint64(line->value, 10, 0, + HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT, &ok, NULL); if (!ok) { log_warn(LD_CONFIG, "HiddenServiceMaxStreams should be between 0 and %d, not %s", - 65535, line->value); + HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT, line->value); goto err; } log_info(LD_CONFIG, @@ -197,12 +348,6 @@ config_generic_service(const config_line_t *line_, } } - /* Check permission on service directory. */ - if (hs_check_service_private_dir(options->User, config->directory_path, - config->dir_group_readable, 0) < 0) { - goto err; - } - /* Check if we are configured in non anonymous mode and single hop mode * meaning every service become single onion. */ if (rend_service_allow_non_anonymous_connection(options) && @@ -220,7 +365,6 @@ config_generic_service(const config_line_t *line_, static int (*config_service_handlers[])(const config_line_t *line, const or_options_t *options, - int validate_only, hs_service_t *service) = { NULL, /* v0 */ @@ -229,64 +373,124 @@ static int config_service_v3, /* v3 */ }; +/* Configure a service using the given line and options. This function will + * call the corresponding version handler and validate the service against the + * other one. On success, add the service to the given list and return 0. On + * error, nothing is added to the list and a negative value is returned. */ +static int +config_service(const config_line_t *line, const or_options_t *options, + smartlist_t *service_list) +{ + hs_service_t *service = NULL; + + tor_assert(line); + tor_assert(options); + tor_assert(service_list); + + /* We have a new hidden service. */ + service = hs_service_new(options); + /* We'll configure that service as a generic one and then pass it to the + * specific handler according to the configured version number. */ + if (config_generic_service(line, options, service) < 0) { + goto err; + } + tor_assert(service->version <= HS_VERSION_MAX); + /* Check permission on service directory that was just parsed. And this must + * be done regardless of the service version. Do not ask for the directory + * to be created, this is done when the keys are loaded because we could be + * in validation mode right now. */ + if (hs_check_service_private_dir(options->User, + service->config.directory_path, + service->config.dir_group_readable, + 0) < 0) { + goto err; + } + /* The handler is in charge of specific options for a version. We start + * after this service directory line so once we hit another directory + * line, the handler knows that it has to stop. */ + if (config_service_handlers[service->version](line->next, options, + service) < 0) { + goto err; + } + /* We'll check if this service can be kept depending on the others + * configured previously. */ + if (service_is_duplicate_in_list(service_list, service)) { + goto err; + } + /* Passes, add it to the given list. */ + smartlist_add(service_list, service); + return 0; + + err: + hs_service_free(service); + return -1; +} + /* From a set of options, setup every hidden service found. Return 0 on * success or -1 on failure. If validate_only is set, parse, warn and * return as normal, but don't actually change the configured services. */ int hs_config_service_all(const or_options_t *options, int validate_only) { - int dir_option_seen = 0; - hs_service_t *service = NULL; + int dir_option_seen = 0, ret = -1; const config_line_t *line; + smartlist_t *new_service_list = NULL; tor_assert(options); + /* Newly configured service are put in that list which is then used for + * validation and staging for >= v3. */ + new_service_list = smartlist_new(); + for (line = options->RendConfigLines; line; line = line->next) { - if (!strcasecmp(line->key, "HiddenServiceDir")) { - /* We have a new hidden service. */ - service = hs_service_new(options); - /* We'll configure that service as a generic one and then pass it to the - * specific handler according to the configured version number. */ - if (config_generic_service(line, options, service) < 0) { + /* Ignore all directives that aren't the start of a service. */ + if (strcasecmp(line->key, "HiddenServiceDir")) { + if (!dir_option_seen) { + log_warn(LD_CONFIG, "%s with no preceding HiddenServiceDir directive", + line->key); goto err; } - tor_assert(service->version <= HS_VERSION_MAX); - /* The handler is in charge of specific options for a version. We start - * after this service directory line so once we hit another directory - * line, the handler knows that it has to stop. */ - if (config_service_handlers[service->version](line->next, options, - validate_only, - service) < 0) { - goto err; - } - /* Whatever happens, on success we loose the ownership of the service - * object so we nullify the pointer to be safe. */ - service = NULL; - /* Flag that we've seen a directory directive and we'll use that to make - * sure that the torrc options ordering are actually valid. */ - dir_option_seen = 1; continue; } - /* The first line must be a directory option else tor is misconfigured. */ - if (!dir_option_seen) { - log_warn(LD_CONFIG, "%s with no preceding HiddenServiceDir directive", - line->key); + /* Flag that we've seen a directory directive and we'll use it to make + * sure that the torrc options ordering is actually valid. */ + dir_option_seen = 1; + + /* Try to configure this service now. On success, it will be added to the + * list and validated against the service in that same list. */ + if (config_service(line, options, new_service_list) < 0) { goto err; } } + /* In non validation mode, we'll stage those services we just successfully + * configured. Service ownership is transfered from the list to the global + * state. If any service is invalid, it will be removed from the list and + * freed. All versions are handled in that function. */ if (!validate_only) { - /* Trigger service pruning which will make sure the just configured - * services end up in the main global list. This is v2 specific. */ - rend_service_prune_list(); - /* XXX: Need the v3 one. */ + stage_services(new_service_list); + } else { + /* We've just validated that we were able to build a clean working list of + * services. We don't need those objects anymore. */ + SMARTLIST_FOREACH(new_service_list, hs_service_t *, s, + hs_service_free(s)); + /* For the v2 subsystem, the configuration handler adds the service object + * to the staging list and it is transferred in the main list through the + * prunning process. In validation mode, we thus have to purge the staging + * list so it's not kept in memory as valid service. */ + rend_service_free_staging_list(); } - /* Success. */ - return 0; + /* Success. Note that the service list has no ownership of its content. */ + ret = 0; + goto end; + err: - hs_service_free(service); - /* Tor main should call the free all function. */ - return -1; + SMARTLIST_FOREACH(new_service_list, hs_service_t *, s, hs_service_free(s)); + + end: + smartlist_free(new_service_list); + /* Tor main should call the free all function on error. */ + return ret; } diff --git a/src/or/hs_config.h b/src/or/hs_config.h index f4207917e5..2f8cbdc130 100644 --- a/src/or/hs_config.h +++ b/src/or/hs_config.h @@ -11,6 +11,8 @@ #include "or.h" +/* Max value for HiddenServiceMaxStreams */ +#define HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT 65535 /* Maximum number of intro points per version 3 services. */ #define HS_CONFIG_V3_MAX_INTRO_POINTS 20 diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 16ffc4885c..b267e1d028 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -7,18 +7,127 @@ **/ #include "or.h" +#include "circuitlist.h" +#include "config.h" #include "relay.h" #include "rendservice.h" -#include "circuitlist.h" -#include "circpathbias.h" +#include "router.h" +#include "hs_common.h" +#include "hs_config.h" #include "hs_intropoint.h" #include "hs_service.h" -#include "hs_common.h" #include "hs/cell_establish_intro.h" #include "hs/cell_common.h" +/* Staging list of service object. When configuring service, we add them to + * this list considered a staging area and they will get added to our global + * map once the keys have been loaded. These two steps are seperated because + * loading keys requires that we are an actual running tor process. */ +static smartlist_t *hs_service_staging_list; + +/* Helper: Function to compare two objects in the service map. Return 1 if the + * two service have the same master public identity key. */ +static inline int +hs_service_ht_eq(const hs_service_t *first, const hs_service_t *second) +{ + tor_assert(first); + tor_assert(second); + /* Simple key compare. */ + return ed25519_pubkey_eq(&first->keys.identity_pk, + &second->keys.identity_pk); +} + +/* Helper: Function for the service hash table code below. The key used is the + * master public identity key which is ultimately the onion address. */ +static inline unsigned int +hs_service_ht_hash(const hs_service_t *service) +{ + tor_assert(service); + return (unsigned int) siphash24g(service->keys.identity_pk.pubkey, + sizeof(service->keys.identity_pk.pubkey)); +} + +/* For the service global hash map, we define a specific type for it which + * will make it safe to use and specific to some controlled parameters such as + * the hashing function and how to compare services. */ +typedef HT_HEAD(hs_service_ht, hs_service_t) hs_service_ht; + +/* This is _the_ global hash map of hidden services which indexed the service + * contained in it by master public identity key which is roughly the onion + * address of the service. */ +static struct hs_service_ht *hs_service_map; + +/* Register the service hash table. */ +HT_PROTOTYPE(hs_service_ht, /* Name of hashtable. */ + hs_service_t, /* Object contained in the map. */ + hs_service_node, /* The name of the HT_ENTRY member. */ + hs_service_ht_hash, /* Hashing function. */ + hs_service_ht_eq) /* Compare function for objects. */ + +HT_GENERATE2(hs_service_ht, hs_service_t, hs_service_node, + hs_service_ht_hash, hs_service_ht_eq, + 0.6, tor_reallocarray, tor_free_) + +/* Query the given service map with a public key and return a service object + * if found else NULL. It is also possible to set a directory path in the + * search query. If pk is NULL, then it will be set to zero indicating the + * hash table to compare the directory path instead. */ +static hs_service_t * +find_service(hs_service_ht *map, const ed25519_public_key_t *pk) +{ + hs_service_t dummy_service = {0}; + tor_assert(map); + tor_assert(pk); + ed25519_pubkey_copy(&dummy_service.keys.identity_pk, pk); + return HT_FIND(hs_service_ht, map, &dummy_service); +} + +/* Register the given service in the given map. If the service already exists + * in the map, -1 is returned. On success, 0 is returned and the service + * ownership has been transfered to the global map. */ +static int +register_service(hs_service_ht *map, hs_service_t *service) +{ + tor_assert(map); + tor_assert(service); + tor_assert(!ed25519_public_key_is_zero(&service->keys.identity_pk)); + + if (find_service(map, &service->keys.identity_pk)) { + /* Existing service with the same key. Do not register it. */ + return -1; + } + /* Taking ownership of the object at this point. */ + HT_INSERT(hs_service_ht, map, service); + return 0; +} + +/* Remove a given service from the given map. If service is NULL or the + * service key is unset, return gracefully. */ +static void +remove_service(hs_service_ht *map, hs_service_t *service) +{ + hs_service_t *elm; + + tor_assert(map); + + /* Ignore if no service or key is zero. */ + if (BUG(service == NULL) || + BUG(ed25519_public_key_is_zero(&service->keys.identity_pk))) { + return; + } + + elm = HT_REMOVE(hs_service_ht, map, service); + if (elm) { + tor_assert(elm == service); + } else { + log_warn(LD_BUG, "Could not find service in the global map " + "while removing service %s", + escaped(service->config.directory_path)); + } +} + /* Set the default values for a service configuration object c. */ static void set_service_default_config(hs_service_config_t *c, @@ -37,6 +146,239 @@ set_service_default_config(hs_service_config_t *c, c->is_ephemeral = 0; } +/* Helper: Function that needs to return 1 for the HT for each loop which + * frees every service in an hash map. */ +static int +ht_free_service_(struct hs_service_t *service, void *data) +{ + (void) data; + hs_service_free(service); + /* This function MUST return 1 so the given object is then removed from the + * service map leading to this free of the object being safe. */ + return 1; +} + +/* Free every service that can be found in the global map. Once done, clear + * and free the global map. */ +static void +service_free_all(void) +{ + if (hs_service_map == NULL) { + return; + } + /* The free helper function returns 1 so this is safe. */ + hs_service_ht_HT_FOREACH_FN(hs_service_map, ht_free_service_, NULL); + HT_CLEAR(hs_service_ht, hs_service_map); + tor_free(hs_service_map); + hs_service_map = NULL; + /* Cleanup staging list. */ + SMARTLIST_FOREACH(hs_service_staging_list, hs_service_t *, s, + hs_service_free(s)); + smartlist_free(hs_service_staging_list); + hs_service_staging_list = NULL; +} + +/* Close all rendezvous circuits for the given service. */ +static void +close_service_rp_circuits(hs_service_t *service) +{ + tor_assert(service); + /* XXX: To implement. */ + return; +} + +/* Close the circuit(s) for the given map of introduction points. */ +static void +close_intro_circuits(hs_service_intropoints_t *intro_points) +{ + tor_assert(intro_points); + + DIGEST256MAP_FOREACH(intro_points->map, key, + const hs_service_intro_point_t *, ip) { + origin_circuit_t *ocirc = + hs_circuitmap_get_intro_circ_v3_service_side( + &ip->auth_key_kp.pubkey); + if (ocirc) { + hs_circuitmap_remove_circuit(TO_CIRCUIT(ocirc)); + /* Reason is FINISHED because service has been removed and thus the + * circuit is considered old/uneeded. */ + circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED); + } + } DIGEST256MAP_FOREACH_END; +} + +/* Close all introduction circuits for the given service. */ +static void +close_service_intro_circuits(hs_service_t *service) +{ + tor_assert(service); + + if (service->desc_current) { + close_intro_circuits(&service->desc_current->intro_points); + } + if (service->desc_next) { + close_intro_circuits(&service->desc_next->intro_points); + } +} + +/* Close any circuits related to the given service. */ +static void +close_service_circuits(hs_service_t *service) +{ + tor_assert(service); + + /* Only support for version >= 3. */ + if (BUG(service->version < HS_VERSION_THREE)) { + return; + } + /* Close intro points. */ + close_service_intro_circuits(service); + /* Close rendezvous points. */ + close_service_rp_circuits(service); +} + +/* Move introduction points from the src descriptor to the dst descriptor. The + * destination service intropoints are wiped out if any before moving. */ +static void +move_descriptor_intro_points(hs_service_descriptor_t *src, + hs_service_descriptor_t *dst) +{ + tor_assert(src); + tor_assert(dst); + + /* XXX: Free dst introduction points. */ + dst->intro_points.map = src->intro_points.map; + /* Nullify the source. */ + src->intro_points.map = NULL; +} + +/* Move introduction points from the src service to the dst service. The + * destination service intropoints are wiped out if any before moving. */ +static void +move_intro_points(hs_service_t *src, hs_service_t *dst) +{ + tor_assert(src); + tor_assert(dst); + + /* Cleanup destination. */ + if (src->desc_current && dst->desc_current) { + move_descriptor_intro_points(src->desc_current, dst->desc_current); + } + if (src->desc_next && dst->desc_next) { + move_descriptor_intro_points(src->desc_next, dst->desc_next); + } +} + +/* Move every ephemeral services from the src service map to the dst service + * map. It is possible that a service can't be register to the dst map which + * won't stop the process of moving them all but will trigger a log warn. */ +static void +move_ephemeral_services(hs_service_ht *src, hs_service_ht *dst) +{ + hs_service_t **iter, **next; + + tor_assert(src); + tor_assert(dst); + + /* Iterate over the map to find ephemeral service and move them to the other + * map. We loop using this method to have a safe removal process. */ + for (iter = HT_START(hs_service_ht, src); iter != NULL; iter = next) { + hs_service_t *s = *iter; + if (!s->config.is_ephemeral) { + /* Yeah, we are in a very manual loop :). */ + next = HT_NEXT(hs_service_ht, src, iter); + continue; + } + /* Remove service from map and then register to it to the other map. + * Reminder that "*iter" and "s" are the same thing. */ + next = HT_NEXT_RMV(hs_service_ht, src, iter); + if (register_service(dst, s) < 0) { + log_warn(LD_BUG, "Ephemeral service key is already being used. " + "Skipping."); + } + } +} + +/* Register services that are in the list. Once this function returns, the + * global service map will be set with the right content and all non surviving + * services will be cleaned up. */ +void +hs_service_register_services(smartlist_t *new_service_list) +{ + struct hs_service_ht *new_service_map; + hs_service_t *s, **iter; + + tor_assert(new_service_list); + + /* We'll save us some allocation and computing time. */ + if (smartlist_len(new_service_list) == 0) { + return; + } + + /* Allocate a new map that will replace the current one. */ + new_service_map = tor_malloc_zero(sizeof(*new_service_map)); + HT_INIT(hs_service_ht, new_service_map); + + /* First step is to transfer all ephemeral services from the current global + * map to the new one we are constructing. We do not prune ephemeral + * services as the only way to kill them is by deleting it from the control + * port or stopping the tor daemon. */ + move_ephemeral_services(hs_service_map, new_service_map); + + SMARTLIST_FOREACH_BEGIN(new_service_list, hs_service_t *, snew) { + /* Check if that service is already in our global map and if so, we'll + * transfer the intro points to it. */ + s = find_service(hs_service_map, &snew->keys.identity_pk); + if (s) { + /* Pass ownership of intro points from s (the current service) to snew + * (the newly configured one). */ + move_intro_points(s, snew); + /* Remove the service from the global map because after this, we need to + * go over the remaining service in that map that aren't surviving the + * reload to close their circuits. */ + remove_service(hs_service_map, s); + } + /* Great, this service is now ready to be added to our new map. */ + if (BUG(register_service(new_service_map, snew) < 0)) { + /* This should never happen because prior to registration, we validate + * every service against the entire set. Not being able to register a + * service means we failed to validate correctly. In that case, don't + * break tor and ignore the service but tell user. */ + log_warn(LD_BUG, "Unable to register service with directory %s", + snew->config.directory_path); + SMARTLIST_DEL_CURRENT(new_service_list, snew); + hs_service_free(snew); + } + } SMARTLIST_FOREACH_END(snew); + + /* Close any circuits associated with the non surviving services. Every + * service in the current global map are roaming. */ + HT_FOREACH(iter, hs_service_ht, hs_service_map) { + close_service_circuits(*iter); + } + + /* Time to make the switch. We'll wipe the current list and switch. */ + service_free_all(); + hs_service_map = new_service_map; +} + +/* Put all service object in the given service list. After this, the caller + * looses ownership of every elements in the list and responsible to free the + * list pointer. */ +void +hs_service_stage_services(const smartlist_t *service_list) +{ + tor_assert(service_list); + /* This list is freed at registration time but this function can be called + * multiple time. */ + if (hs_service_staging_list == NULL) { + hs_service_staging_list = smartlist_new(); + } + /* Add all service object to our staging list. Caller is responsible for + * freeing the service_list. */ + smartlist_add_all(hs_service_staging_list, service_list); +} + /* Allocate and initilize a service object. The service configuration will * contain the default values. Return the newly allocated object pointer. This * function can't fail. */ @@ -101,14 +443,22 @@ hs_service_free(hs_service_t *service) void hs_service_init(void) { - return; + /* Should never be called twice. */ + tor_assert(!hs_service_map); + tor_assert(!hs_service_staging_list); + + hs_service_map = tor_malloc_zero(sizeof(struct hs_service_ht)); + HT_INIT(hs_service_ht, hs_service_map); + + hs_service_staging_list = smartlist_new(); } -/* Release all global the storage of hidden service subsystem. */ +/* Release all global storage of the hidden service subsystem. */ void hs_service_free_all(void) { rend_service_free_all(); + service_free_all(); } /* XXX We don't currently use these functions, apart from generating unittest diff --git a/src/or/hs_service.h b/src/or/hs_service.h index ec47cb72ae..8d92e75072 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -11,9 +11,10 @@ #include "crypto_curve25519.h" #include "crypto_ed25519.h" +#include "replaycache.h" + #include "hs_descriptor.h" #include "hs_intropoint.h" -#include "replaycache.h" /* Trunnel */ #include "hs/cell_establish_intro.h" @@ -171,6 +172,10 @@ typedef struct hs_service_t { /* Protocol version of the service. Specified by HiddenServiceVersion. */ uint32_t version; + /* Hashtable node: use to look up the service by its master public identity + * key in the service global map. */ + HT_ENTRY(hs_service_t) hs_service_node; + /* Service state which contains various flags and counters. */ hs_service_state_t state; @@ -192,12 +197,16 @@ typedef struct hs_service_t { /* API */ -int hs_service_config_all(const or_options_t *options, int validate_only); +/* Global initializer and cleanup function. */ void hs_service_init(void); void hs_service_free_all(void); -void hs_service_free(hs_service_t *service); +/* Service new/free functions. */ hs_service_t *hs_service_new(const or_options_t *options); +void hs_service_free(hs_service_t *service); + +void hs_service_register_services(smartlist_t *new_service_list); +void hs_service_stage_services(const smartlist_t *service_list); /* These functions are only used by unit tests and we need to expose them else * hs_service.o ends up with no symbols in libor.a which makes clang throw a diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 695668bf60..e86119f2d0 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -231,8 +231,20 @@ rend_service_free(rend_service_t *service) tor_free(service); } -/** Release all the storage held in rend_service_list. - */ +/* Release all the storage held in rend_service_staging_list. */ +void +rend_service_free_staging_list(void) +{ + if (rend_service_staging_list) { + SMARTLIST_FOREACH(rend_service_staging_list, rend_service_t*, ptr, + rend_service_free(ptr)); + smartlist_free(rend_service_staging_list); + rend_service_staging_list = NULL; + } +} + +/** Release all the storage held in both rend_service_list and + * rend_service_staging_list. */ void rend_service_free_all(void) { @@ -242,12 +254,7 @@ rend_service_free_all(void) smartlist_free(rend_service_list); rend_service_list = NULL; } - if (rend_service_staging_list) { - SMARTLIST_FOREACH(rend_service_staging_list, rend_service_t*, ptr, - rend_service_free(ptr)); - smartlist_free(rend_service_staging_list); - rend_service_staging_list = NULL; - } + rend_service_free_staging_list(); } /* Validate a service. Use the service_list to make sure there @@ -257,8 +264,6 @@ static int rend_validate_service(const smartlist_t *service_list, const rend_service_t *service) { - int dupe = 0; - tor_assert(service_list); tor_assert(service); @@ -291,34 +296,6 @@ rend_validate_service(const smartlist_t *service_list, goto invalid; } - /* XXX This duplicate check has two problems: - * - * a) It's O(n^2), but the same comment from the bottom of - * rend_config_services() should apply. - * - * b) We only compare directory paths as strings, so we can't - * detect two distinct paths that specify the same directory - * (which can arise from symlinks, case-insensitivity, bind - * mounts, etc.). - * - * It also can't detect that two separate Tor instances are trying - * to use the same HiddenServiceDir; for that, we would need a - * lock file. But this is enough to detect a simple mistake that - * at least one person has actually made. - */ - if (!rend_service_is_ephemeral(service)) { - /* Skip dupe for ephemeral services. */ - SMARTLIST_FOREACH(service_list, rend_service_t *, ptr, - dupe = dupe || - !strcmp(ptr->directory, service->directory)); - if (dupe) { - log_warn(LD_REND, "Another hidden service is already configured for " - "directory %s.", - rend_service_escaped_dir(service)); - goto invalid; - } - } - /* Valid. */ return 0; invalid: @@ -662,10 +639,8 @@ service_shadow_copy(rend_service_t *service, hs_service_t *hs_service) int rend_config_service(const config_line_t *line_, const or_options_t *options, - int validate_only, hs_service_t *hs_service) { - (void) validate_only; const config_line_t *line; rend_service_t *service = NULL; diff --git a/src/or/rendservice.h b/src/or/rendservice.h index 90f854e4bc..361a119e49 100644 --- a/src/or/rendservice.h +++ b/src/or/rendservice.h @@ -143,9 +143,9 @@ STATIC void rend_service_prune_list_impl_(void); int num_rend_services(void); int rend_config_service(const config_line_t *line_, const or_options_t *options, - int validate_only, hs_service_t *hs_service); void rend_service_prune_list(void); +void rend_service_free_staging_list(void); int rend_service_load_all_keys(const smartlist_t *service_list); void rend_services_add_filenames_to_lists(smartlist_t *open_lst, smartlist_t *stat_lst); From f3899acdbfe121521cbd8cc76983b1e1e149d38c Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 30 Jan 2017 17:33:18 -0500 Subject: [PATCH 08/22] prop224: Service address creation/validation This also adds unit test and a small python script generating a deterministic test vector that a unit test tries to match. Signed-off-by: David Goulet --- src/or/hs_common.c | 178 +++++++++++++++++++++++++++++++++++ src/or/hs_common.h | 29 ++++++ src/test/hs_build_address.py | 37 ++++++++ src/test/test_hs_service.c | 83 ++++++++++++++++ 4 files changed, 327 insertions(+) create mode 100644 src/test/hs_build_address.py diff --git a/src/or/hs_common.c b/src/or/hs_common.c index b524296159..00befabbc2 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -346,6 +346,184 @@ rend_data_get_pk_digest(const rend_data_t *rend_data, size_t *len_out) } } +/* Using an ed25519 public key and version to build the checksum of an + * address. Put in checksum_out. Format is: + * SHA3-256(".onion checksum" || PUBKEY || VERSION) + * + * checksum_out must be large enough to receive 32 bytes (DIGEST256_LEN). */ +static void +build_hs_checksum(const ed25519_public_key_t *key, uint8_t version, + char *checksum_out) +{ + size_t offset = 0; + char data[HS_SERVICE_ADDR_CHECKSUM_INPUT_LEN]; + + /* Build checksum data. */ + memcpy(data, HS_SERVICE_ADDR_CHECKSUM_PREFIX, + HS_SERVICE_ADDR_CHECKSUM_PREFIX_LEN); + offset += HS_SERVICE_ADDR_CHECKSUM_PREFIX_LEN; + memcpy(data + offset, key->pubkey, ED25519_PUBKEY_LEN); + offset += ED25519_PUBKEY_LEN; + set_uint8(data + offset, version); + offset += sizeof(version); + tor_assert(offset == HS_SERVICE_ADDR_CHECKSUM_INPUT_LEN); + + /* Hash the data payload to create the checksum. */ + crypto_digest256(checksum_out, data, sizeof(data), DIGEST_SHA3_256); +} + +/* Using an ed25519 public key, checksum and version to build the binary + * representation of a service address. Put in addr_out. Format is: + * addr_out = PUBKEY || CHECKSUM || VERSION + * + * addr_out must be large enough to receive HS_SERVICE_ADDR_LEN bytes. */ +static void +build_hs_address(const ed25519_public_key_t *key, const char *checksum, + uint8_t version, char *addr_out) +{ + size_t offset = 0; + + tor_assert(key); + tor_assert(checksum); + + memcpy(addr_out, key->pubkey, ED25519_PUBKEY_LEN); + offset += ED25519_PUBKEY_LEN; + memcpy(addr_out + offset, checksum, HS_SERVICE_ADDR_CHECKSUM_LEN_USED); + offset += HS_SERVICE_ADDR_CHECKSUM_LEN_USED; + set_uint8(addr_out + offset, version); + offset += sizeof(uint8_t); + tor_assert(offset == HS_SERVICE_ADDR_LEN); +} + +/* Helper for hs_parse_address(): Using a binary representation of a service + * address, parse its content into the key_out, checksum_out and version_out. + * Any out variable can be NULL in case the caller would want only one field. + * checksum_out MUST at least be 2 bytes long. address must be at least + * HS_SERVICE_ADDR_LEN bytes but doesn't need to be NUL terminated. */ +static void +hs_parse_address_impl(const char *address, ed25519_public_key_t *key_out, + char *checksum_out, uint8_t *version_out) +{ + size_t offset = 0; + + tor_assert(address); + + if (key_out) { + /* First is the key. */ + memcpy(key_out->pubkey, address, ED25519_PUBKEY_LEN); + } + offset += ED25519_PUBKEY_LEN; + if (checksum_out) { + /* Followed by a 2 bytes checksum. */ + memcpy(checksum_out, address + offset, HS_SERVICE_ADDR_CHECKSUM_LEN_USED); + } + offset += HS_SERVICE_ADDR_CHECKSUM_LEN_USED; + if (version_out) { + /* Finally, version value is 1 byte. */ + *version_out = get_uint8(address + offset); + } + offset += sizeof(uint8_t); + /* Extra safety. */ + tor_assert(offset == HS_SERVICE_ADDR_LEN); +} + +/* Using a base32 representation of a service address, parse its content into + * the key_out, checksum_out and version_out. Any out variable can be NULL in + * 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. */ +int +hs_parse_address(const char *address, ed25519_public_key_t *key_out, + char *checksum_out, uint8_t *version_out) +{ + char decoded[HS_SERVICE_ADDR_LEN]; + + tor_assert(address); + + /* Obvious length check. */ + if (strlen(address) != HS_SERVICE_ADDR_LEN_BASE32) { + log_warn(LD_REND, "Service address %s has an invalid length. " + "Expected %ld but got %lu.", + escaped_safe_str(address), HS_SERVICE_ADDR_LEN_BASE32, + strlen(address)); + goto invalid; + } + + /* Decode address so we can extract needed fields. */ + if (base32_decode(decoded, sizeof(decoded), address, strlen(address)) < 0) { + log_warn(LD_REND, "Service address %s can't be decoded.", + escaped_safe_str(address)); + goto invalid; + } + + /* Parse the decoded address into the fields we need. */ + hs_parse_address_impl(decoded, key_out, checksum_out, version_out); + + return 0; + invalid: + return -1; +} + +/* Validate a given onion address. The length, the base32 decoding and + * checksum are validated. Return 1 if valid else 0. */ +int +hs_address_is_valid(const char *address) +{ + uint8_t version; + char checksum[HS_SERVICE_ADDR_CHECKSUM_LEN_USED]; + char target_checksum[DIGEST256_LEN]; + ed25519_public_key_t key; + + /* Parse the decoded address into the fields we need. */ + if (hs_parse_address(address, &key, checksum, &version) < 0) { + goto invalid; + } + + /* Get the checksum it's suppose to be and compare it with what we have + * encoded in the address. */ + build_hs_checksum(&key, version, target_checksum); + if (tor_memcmp(checksum, target_checksum, sizeof(checksum))) { + log_warn(LD_REND, "Service address %s invalid checksum.", + escaped_safe_str(address)); + goto invalid; + } + + /* Valid address. */ + return 1; + invalid: + return 0; +} + +/* Build a service address using an ed25519 public key and a given version. + * The returned address is base32 encoded and put in addr_out. The caller MUST + * make sure the addr_out is at least HS_SERVICE_ADDR_LEN_BASE32 + 1 long. + * + * Format is as follow: + * base32(PUBKEY || CHECKSUM || VERSION) + * CHECKSUM = H(".onion checksum" || PUBKEY || VERSION) + * */ +void +hs_build_address(const ed25519_public_key_t *key, uint8_t version, + char *addr_out) +{ + char checksum[DIGEST256_LEN], address[HS_SERVICE_ADDR_LEN]; + + tor_assert(key); + tor_assert(addr_out); + + /* Get the checksum of the address. */ + build_hs_checksum(key, version, checksum); + /* Get the binary address representation. */ + build_hs_address(key, checksum, version, address); + + /* Encode the address. addr_out will be NUL terminated after this. */ + base32_encode(addr_out, HS_SERVICE_ADDR_LEN_BASE32 + 1, address, + sizeof(address)); + /* Validate what we just built. */ + tor_assert(hs_address_is_valid(addr_out)); +} + /* Initialize the entire HS subsytem. This is called in tor_init() before any * torrc options are loaded. Only for >= v3. */ void diff --git a/src/or/hs_common.h b/src/or/hs_common.h index d1bc5ac7ef..64bf89f398 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -52,6 +52,30 @@ /* The time period rotation offset as seen in prop224 section [TIME-PERIODS] */ #define HS_TIME_PERIOD_ROTATION_OFFSET (12 * 60) /* minutes */ +/* Prefix of the onion address checksum. */ +#define HS_SERVICE_ADDR_CHECKSUM_PREFIX ".onion checksum" +/* Length of the checksum prefix minus the NUL terminated byte. */ +#define HS_SERVICE_ADDR_CHECKSUM_PREFIX_LEN \ + (sizeof(HS_SERVICE_ADDR_CHECKSUM_PREFIX) - 1) +/* Length of the resulting checksum of the address. The construction of this + * checksum looks like: + * CHECKSUM = ".onion checksum" || PUBKEY || VERSION + * where VERSION is 1 byte. This is pre-hashing. */ +#define HS_SERVICE_ADDR_CHECKSUM_INPUT_LEN \ + (HS_SERVICE_ADDR_CHECKSUM_PREFIX_LEN + ED25519_PUBKEY_LEN + sizeof(uint8_t)) +/* The amount of bytes we use from the address checksum. */ +#define HS_SERVICE_ADDR_CHECKSUM_LEN_USED 2 +/* Length of the binary encoded service address which is of course before the + * base32 encoding. Construction is: + * PUBKEY || CHECKSUM || VERSION + * with 1 byte VERSION and 2 bytes CHECKSUM. The following is 35 bytes. */ +#define HS_SERVICE_ADDR_LEN \ + (ED25519_PUBKEY_LEN + HS_SERVICE_ADDR_CHECKSUM_LEN_USED + sizeof(uint8_t)) +/* Length of 'y' portion of 'y.onion' URL. This is base32 encoded and the + * length ends up to 56 bytes (not counting the terminated NUL byte.) */ +#define HS_SERVICE_ADDR_LEN_BASE32 \ + (CEIL_DIV(HS_SERVICE_ADDR_LEN * 8, 5)) + /* Type of authentication key used by an introduction point. */ typedef enum { HS_AUTH_KEY_TYPE_LEGACY = 1, @@ -64,6 +88,11 @@ void hs_free_all(void); int hs_check_service_private_dir(const char *username, const char *path, unsigned int dir_group_readable, unsigned int create); +void hs_build_address(const ed25519_public_key_t *key, uint8_t version, + char *addr_out); +int hs_address_is_valid(const char *address); +int hs_parse_address(const char *address, ed25519_public_key_t *key_out, + char *checksum_out, uint8_t *version_out); void rend_data_free(rend_data_t *data); rend_data_t *rend_data_dup(const rend_data_t *data); diff --git a/src/test/hs_build_address.py b/src/test/hs_build_address.py new file mode 100644 index 0000000000..7be9c8b85a --- /dev/null +++ b/src/test/hs_build_address.py @@ -0,0 +1,37 @@ +import sys +import hashlib +import struct +import base64 + +# Python 3.6+, the SHA3 is available in hashlib natively. Else this requires +# the pysha3 package (pip install pysha3). +if sys.version_info < (3, 6): + import sha3 + +# Test vector to make sure the right sha3 version will be used. pysha3 < 1.0 +# used the old Keccak implementation. During the finalization of SHA3, NIST +# changed the delimiter suffix from 0x01 to 0x06. The Keccak sponge function +# stayed the same. pysha3 1.0 provides the previous Keccak hash, too. +TEST_VALUE = "e167f68d6563d75bb25f3aa49c29ef612d41352dc00606de7cbd630bb2665f51" +if TEST_VALUE != sha3.sha3_256(b"Hello World").hexdigest(): + print("pysha3 version is < 1.0. Please install from:") + print("https://github.com/tiran/pysha3https://github.com/tiran/pysha3") + sys.exit(1) + +# Checksum is built like so: +# CHECKSUM = SHA3(".onion checksum" || PUBKEY || VERSION) +PREFIX = ".onion checksum".encode() +# 32 bytes ed25519 pubkey. +PUBKEY = ("\x42" * 32).encode() +# Version 3 is proposal224 +VERSION = 3 + +data = struct.pack('15s32sb', PREFIX, PUBKEY, VERSION) +checksum = hashlib.sha3_256(data).digest() + +# Onion address is built like so: +# onion_address = base32(PUBKEY || CHECKSUM || VERSION) + ".onion" +address = struct.pack('!32s2sb', PUBKEY, checksum, VERSION) +onion_addr = base64.b32encode(address).decode().lower() + +print("%s" % (onion_addr)) diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 17772f1df0..e081b7f2f3 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -207,6 +207,85 @@ test_hs_ntor(void *arg) tt_mem_op(client_hs_ntor_rend_cell_keys.ntor_key_seed, OP_EQ, service_hs_ntor_rend_cell_keys.ntor_key_seed, DIGEST256_LEN); + done: + ; +} + +static void +test_validate_address(void *arg) +{ + int ret; + + (void) arg; + + /* Address too short and too long. */ + 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"); + 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"); + teardown_capture_of_logs(); + + /* Invalid checksum (taken from prop224) */ + setup_full_capture_of_logs(LOG_WARN); + ret = hs_address_is_valid( + "l5satjgud6gucryazcyvyvhuxhr74u6ygigiuyixe3a6ysis67ororad"); + tt_int_op(ret, OP_EQ, 0); + expect_log_msg_containing("invalid checksum"); + teardown_capture_of_logs(); + + setup_full_capture_of_logs(LOG_WARN); + ret = hs_address_is_valid( + "btojiu7nu5y5iwut64eufevogqdw4wmqzugnoluw232r4t3ecsfv37ad"); + tt_int_op(ret, OP_EQ, 0); + expect_log_msg_containing("invalid checksum"); + teardown_capture_of_logs(); + + /* Non base32 decodable string. */ + setup_full_capture_of_logs(LOG_WARN); + ret = hs_address_is_valid( + "????????????????????????????????????????????????????????"); + tt_int_op(ret, OP_EQ, 0); + expect_log_msg_containing("can't be decoded"); + teardown_capture_of_logs(); + + /* Valid address. */ + ret = hs_address_is_valid( + "p3xnclpu4mu22dwaurjtsybyqk4xfjmcfz6z62yl24uwmhjatiwnlnad"); + tt_int_op(ret, OP_EQ, 1); + + done: + ; +} + +static void +test_build_address(void *arg) +{ + int ret; + char onion_addr[HS_SERVICE_ADDR_LEN_BASE32 + 1]; + ed25519_public_key_t pubkey; + + (void) arg; + + /* The following has been created with hs_build_address.py script that + * follows proposal 224 specification to build an onion address. */ + static const char *test_addr = + "ijbeeqscijbeeqscijbeeqscijbeeqscijbeeqscijbeeqscijbezhid"; + + /* Let's try to build the same onion address that the script can do. Key is + * a long set of very random \x42 :). */ + memset(&pubkey, '\x42', sizeof(pubkey)); + hs_build_address(&pubkey, HS_VERSION_THREE, onion_addr); + tt_str_op(test_addr, OP_EQ, onion_addr); + /* Validate that address. */ + ret = hs_address_is_valid(onion_addr); + tt_int_op(ret, OP_EQ, 1); done: ; @@ -326,6 +405,10 @@ struct testcase_t hs_service_tests[] = { NULL, NULL }, { "e2e_rend_circuit_setup", test_e2e_rend_circuit_setup, TT_FORK, NULL, NULL }, + { "build_address", test_build_address, TT_FORK, + NULL, NULL }, + { "validate_address", test_validate_address, TT_FORK, + NULL, NULL }, END_OF_TESTCASES }; From 138e03c488bfa05504b69ced48ddf8f0afd1310c Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 1 Feb 2017 09:18:58 -0500 Subject: [PATCH 09/22] prop224: Load and/or generate v3 service keys Try to load or/and generate service keys for v3. This write both the public and private key file to disk along with the hostname file containing the onion address. Signed-off-by: David Goulet --- src/or/config.c | 2 +- src/or/hs_common.c | 14 +++ src/or/hs_common.h | 1 + src/or/hs_service.c | 215 ++++++++++++++++++++++++++++++++++++++----- src/or/hs_service.h | 7 +- src/or/rendservice.c | 9 +- 6 files changed, 215 insertions(+), 33 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 062ab27ae7..f460ec6f3c 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1799,7 +1799,7 @@ options_act(const or_options_t *old_options) monitor_owning_controller_process(options->OwningControllerProcess); /* reload keys as needed for rendezvous services. */ - if (rend_service_load_all_keys(NULL)<0) { + if (hs_service_load_all_keys() < 0) { log_warn(LD_GENERAL,"Error loading rendezvous service keys"); return -1; } diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 00befabbc2..87c29d5819 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -20,6 +20,20 @@ #include "hs_service.h" #include "rendcommon.h" +/* Allocate and return a string containing the path to filename in directory. + * This function will never return NULL. The caller must free this path. */ +char * +hs_path_from_filename(const char *directory, const char *filename) +{ + char *file_path = NULL; + + tor_assert(directory); + tor_assert(filename); + + tor_asprintf(&file_path, "%s%s%s", directory, PATH_SEPARATOR, filename); + return file_path; +} + /* Make sure that the directory for service is private, using the config * username. * If create is true: diff --git a/src/or/hs_common.h b/src/or/hs_common.h index 64bf89f398..2b33914275 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -88,6 +88,7 @@ void hs_free_all(void); int hs_check_service_private_dir(const char *username, const char *path, unsigned int dir_group_readable, unsigned int create); +char *hs_path_from_filename(const char *directory, const char *filename); void hs_build_address(const ed25519_public_key_t *key, uint8_t version, char *addr_out); int hs_address_is_valid(const char *address); diff --git a/src/or/hs_service.c b/src/or/hs_service.c index b267e1d028..bce976ee8c 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -12,6 +12,7 @@ #include "relay.h" #include "rendservice.h" #include "router.h" +#include "routerkeys.h" #include "hs_common.h" #include "hs_config.h" @@ -21,6 +22,11 @@ #include "hs/cell_establish_intro.h" #include "hs/cell_common.h" +/* Onion service directory file names. */ +static const char *fname_keyfile_prefix = "hs_ed25519"; +static const char *fname_hostname = "hostname"; +static const char *address_tld = "onion"; + /* Staging list of service object. When configuring service, we add them to * this list considered a staging area and they will get added to our global * map once the keys have been loaded. These two steps are seperated because @@ -163,19 +169,21 @@ ht_free_service_(struct hs_service_t *service, void *data) static void service_free_all(void) { - if (hs_service_map == NULL) { - return; + if (hs_service_map) { + /* The free helper function returns 1 so this is safe. */ + hs_service_ht_HT_FOREACH_FN(hs_service_map, ht_free_service_, NULL); + HT_CLEAR(hs_service_ht, hs_service_map); + tor_free(hs_service_map); + hs_service_map = NULL; + } + + if (hs_service_staging_list) { + /* Cleanup staging list. */ + SMARTLIST_FOREACH(hs_service_staging_list, hs_service_t *, s, + hs_service_free(s)); + smartlist_free(hs_service_staging_list); + hs_service_staging_list = NULL; } - /* The free helper function returns 1 so this is safe. */ - hs_service_ht_HT_FOREACH_FN(hs_service_map, ht_free_service_, NULL); - HT_CLEAR(hs_service_ht, hs_service_map); - tor_free(hs_service_map); - hs_service_map = NULL; - /* Cleanup staging list. */ - SMARTLIST_FOREACH(hs_service_staging_list, hs_service_t *, s, - hs_service_free(s)); - smartlist_free(hs_service_staging_list); - hs_service_staging_list = NULL; } /* Close all rendezvous circuits for the given service. */ @@ -299,19 +307,29 @@ move_ephemeral_services(hs_service_ht *src, hs_service_ht *dst) } } -/* Register services that are in the list. Once this function returns, the - * global service map will be set with the right content and all non surviving - * services will be cleaned up. */ -void -hs_service_register_services(smartlist_t *new_service_list) +/* Return a const string of the directory path escaped. If this is an + * ephemeral service, it returns "[EPHEMERAL]". This can only be called from + * the main thread because escaped() uses a static variable. */ +static const char * +service_escaped_dir(const hs_service_t *s) +{ + return (s->config.is_ephemeral) ? "[EPHEMERAL]" : + escaped(s->config.directory_path); +} + +/* Register services that are in the staging list. Once this function returns, + * the global service map will be set with the right content and all non + * surviving services will be cleaned up. */ +static void +register_all_services(void) { struct hs_service_ht *new_service_map; hs_service_t *s, **iter; - tor_assert(new_service_list); + tor_assert(hs_service_staging_list); /* We'll save us some allocation and computing time. */ - if (smartlist_len(new_service_list) == 0) { + if (smartlist_len(hs_service_staging_list) == 0) { return; } @@ -325,7 +343,7 @@ hs_service_register_services(smartlist_t *new_service_list) * port or stopping the tor daemon. */ move_ephemeral_services(hs_service_map, new_service_map); - SMARTLIST_FOREACH_BEGIN(new_service_list, hs_service_t *, snew) { + SMARTLIST_FOREACH_BEGIN(hs_service_staging_list, hs_service_t *, snew) { /* Check if that service is already in our global map and if so, we'll * transfer the intro points to it. */ s = find_service(hs_service_map, &snew->keys.identity_pk); @@ -345,8 +363,8 @@ hs_service_register_services(smartlist_t *new_service_list) * service means we failed to validate correctly. In that case, don't * break tor and ignore the service but tell user. */ log_warn(LD_BUG, "Unable to register service with directory %s", - snew->config.directory_path); - SMARTLIST_DEL_CURRENT(new_service_list, snew); + service_escaped_dir(snew)); + SMARTLIST_DEL_CURRENT(hs_service_staging_list, snew); hs_service_free(snew); } } SMARTLIST_FOREACH_END(snew); @@ -357,11 +375,162 @@ hs_service_register_services(smartlist_t *new_service_list) close_service_circuits(*iter); } - /* Time to make the switch. We'll wipe the current list and switch. */ + /* Time to make the switch. We'll clear the staging list because its content + * has now changed ownership to the map. */ + smartlist_clear(hs_service_staging_list); service_free_all(); hs_service_map = new_service_map; } +/* Write the onion address of a given service to the given filename fname_ in + * the service directory. Return 0 on success else -1 on error. */ +static int +write_address_to_file(const hs_service_t *service, const char *fname_) +{ + int ret = -1; + char *fname = NULL; + /* Length of an address plus the sizeof the address tld (onion) which counts + * the NUL terminated byte so we keep it for the "." and the newline. */ + char buf[HS_SERVICE_ADDR_LEN_BASE32 + sizeof(address_tld) + 1]; + + tor_assert(service); + tor_assert(fname_); + + /* Construct the full address with the onion tld and write the hostname file + * to disk. */ + tor_snprintf(buf, sizeof(buf), "%s.%s\n", service->onion_address, + address_tld); + /* Notice here that we use the given "fname_". */ + fname = hs_path_from_filename(service->config.directory_path, fname_); + if (write_str_to_file(fname, buf, 0) < 0) { + log_warn(LD_REND, "Could not write onion address to hostname file %s", + escaped(fname)); + goto end; + } + +#ifndef _WIN32 + if (service->config.dir_group_readable) { + /* Mode to 0640. */ + if (chmod(fname, S_IRUSR | S_IWUSR | S_IRGRP) < 0) { + log_warn(LD_FS, "Unable to make onion service hostname file %s " + "group-readable.", escaped(fname)); + } + } +#endif /* _WIN32 */ + + /* Success. */ + ret = 0; + end: + tor_free(fname); + return ret; +} + +/* Load and/or generate private keys for the given service. On success, the + * hostname file will be written to disk along with the master private key iff + * the service is not configured for offline keys. Return 0 on success else -1 + * on failure. */ +static int +load_service_keys(hs_service_t *service) +{ + int ret = -1; + char *fname = NULL; + ed25519_keypair_t *kp; + const hs_service_config_t *config; + + tor_assert(service); + + config = &service->config; + + /* Create and fix permission on service directory. We are about to write + * files to that directory so make sure it exists and has the right + * permissions. We do this here because at this stage we know that Tor is + * actually running and the service we have has been validated. */ + if (BUG(hs_check_service_private_dir(get_options()->User, + config->directory_path, + config->dir_group_readable, 1) < 0)) { + goto end; + } + + /* Try to load the keys from file or generate it if not found. */ + fname = hs_path_from_filename(config->directory_path, fname_keyfile_prefix); + /* Don't ask for key creation, we want to know if we were able to load it or + * we had to generate it. Better logging! */ + kp = ed_key_init_from_file(fname, 0, LOG_INFO, NULL, 0, 0, 0, NULL); + if (!kp) { + log_info(LD_REND, "Unable to load keys from %s. Generating it...", fname); + /* We'll now try to generate the keys and for it we want the strongest + * randomness for it. The keypair will be written in different files. */ + uint32_t key_flags = INIT_ED_KEY_CREATE | INIT_ED_KEY_EXTRA_STRONG | + INIT_ED_KEY_SPLIT; + kp = ed_key_init_from_file(fname, key_flags, LOG_WARN, NULL, 0, 0, 0, + NULL); + if (!kp) { + log_warn(LD_REND, "Unable to generate keys and save in %s.", fname); + goto end; + } + } + + /* Copy loaded or generated keys to service object. */ + ed25519_pubkey_copy(&service->keys.identity_pk, &kp->pubkey); + memcpy(&service->keys.identity_sk, &kp->seckey, + sizeof(service->keys.identity_sk)); + /* This does a proper memory wipe. */ + ed25519_keypair_free(kp); + + /* Build onion address from the newly loaded keys. */ + tor_assert(service->version <= UINT8_MAX); + hs_build_address(&service->keys.identity_pk, (uint8_t) service->version, + service->onion_address); + + /* Write onion address to hostname file. */ + if (write_address_to_file(service, fname_hostname) < 0) { + goto end; + } + + /* Succes. */ + ret = 0; + end: + tor_free(fname); + return ret; +} + +/* Load and/or generate keys for all onion services including the client + * authorization if any. Return 0 on success, -1 on failure. */ +int +hs_service_load_all_keys(void) +{ + /* Load v2 service keys if we have v2. */ + if (num_rend_services() != 0) { + if (rend_service_load_all_keys(NULL) < 0) { + goto err; + } + } + + /* Load or/and generate them for v3+. */ + SMARTLIST_FOREACH_BEGIN(hs_service_staging_list, hs_service_t *, service) { + /* Ignore ephemeral service, they already have their keys set. */ + if (service->config.is_ephemeral) { + continue; + } + log_info(LD_REND, "Loading v3 onion service keys from %s", + service_escaped_dir(service)); + if (load_service_keys(service) < 0) { + goto err; + } + /* XXX: Load/Generate client authorization keys. (#20700) */ + } SMARTLIST_FOREACH_END(service); + + /* Final step, the staging list contains service in a quiescent state that + * is ready to be used. Register them to the global map. Once this is over, + * the staging list will be cleaned up. */ + register_all_services(); + + /* All keys have been loaded successfully. */ + return 0; + err: + return -1; +} + /* Put all service object in the given service list. After this, the caller * looses ownership of every elements in the list and responsible to free the * list pointer. */ diff --git a/src/or/hs_service.h b/src/or/hs_service.h index 8d92e75072..90606acb1c 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -13,6 +13,7 @@ #include "crypto_ed25519.h" #include "replaycache.h" +#include "hs_common.h" #include "hs_descriptor.h" #include "hs_intropoint.h" @@ -172,6 +173,10 @@ typedef struct hs_service_t { /* Protocol version of the service. Specified by HiddenServiceVersion. */ uint32_t version; + /* Onion address base32 encoded and NUL terminated. We keep it for logging + * purposes so we don't have to build it everytime. */ + char onion_address[HS_SERVICE_ADDR_LEN_BASE32 + 1]; + /* Hashtable node: use to look up the service by its master public identity * key in the service global map. */ HT_ENTRY(hs_service_t) hs_service_node; @@ -205,8 +210,8 @@ void hs_service_free_all(void); hs_service_t *hs_service_new(const or_options_t *options); void hs_service_free(hs_service_t *service); -void hs_service_register_services(smartlist_t *new_service_list); void hs_service_stage_services(const smartlist_t *service_list); +int hs_service_load_all_keys(void); /* These functions are only used by unit tests and we need to expose them else * hs_service.o ends up with no symbols in libor.a which makes clang throw a diff --git a/src/or/rendservice.c b/src/or/rendservice.c index e86119f2d0..358efd0ed9 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1046,15 +1046,8 @@ rend_service_update_descriptor(rend_service_t *service) static char * rend_service_path(const rend_service_t *service, const char *file_name) { - char *file_path = NULL; - tor_assert(service->directory); - - /* Can never fail: asserts rather than leaving file_path NULL. */ - tor_asprintf(&file_path, "%s%s%s", - service->directory, PATH_SEPARATOR, file_name); - - return file_path; + return hs_path_from_filename(service->directory, file_name); } /* Allocate and return a string containing the path to the single onion From f76f8731995917366b53f729befd450ed3d417d1 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Fri, 7 Jul 2017 15:34:36 -0400 Subject: [PATCH 10/22] prop224: Add a function to check for invalid opts Every hidden service option don't apply to every version so this new function makes sure we don't have for instance an option that is only for v2 in a v3 configured service. This works using an exclude lists for a specific version. Right now, there is only one option that is not allowed in v3. The rest is common. Signed-off-by: David Goulet --- src/or/hs_config.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/or/hs_config.c b/src/or/hs_config.c index 6bb422dbf1..afc9631cd4 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -115,6 +115,69 @@ service_is_duplicate_in_list(const smartlist_t *service_list, return ret; } +/* Return true iff the given options starting at line_ for a hidden service + * contains at least one invalid option. Each hidden service option don't + * apply to all versions so this function can find out. The line_ MUST start + * right after the HiddenServiceDir line of this service. + * + * This is mainly for usability so we can inform the user of any invalid + * option for the hidden service version instead of silently ignoring. */ +static int +config_has_invalid_options(const config_line_t *line_, + const hs_service_t *service) +{ + int ret = 0; + const char **optlist; + const config_line_t *line; + + tor_assert(service); + tor_assert(service->version <= HS_VERSION_MAX); + + /* List of options that a v3 service doesn't support thus must exclude from + * its configuration. */ + const char *opts_exclude_v3[] = { + "HiddenServiceAuthorizeClient", + NULL /* End marker. */ + }; + + /* Defining the size explicitly allows us to take advantage of the compiler + * which warns us if we ever bump the max version but forget to grow this + * array. The plus one is because we have a version 0 :). */ + struct { + const char **list; + } exclude_lists[HS_VERSION_MAX + 1] = { + { NULL }, /* v0. */ + { NULL }, /* v1. */ + { NULL }, /* v2 */ + { opts_exclude_v3 }, /* v3. */ + }; + + optlist = exclude_lists[service->version].list; + if (optlist == NULL) { + /* No exclude options to look at for this version. */ + goto end; + } + for (int i = 0; optlist[i]; i++) { + const char *opt = optlist[i]; + for (line = line_; line; line = line->next) { + if (!strcasecmp(line->key, "HiddenServiceDir")) { + /* We just hit the next hidden service, stop right now. */ + goto end; + } + if (!strcasecmp(line->key, opt)) { + log_warn(LD_CONFIG, "Hidden service option %s is incompatible with " + "version %" PRIu32 " of service in %s", + opt, service->version, service->config.directory_path); + ret = 1; + /* Continue the loop so we can find all possible options. */ + continue; + } + } + } + end: + return ret; +} + /* Validate service configuration. This is used when loading the configuration * and once we've setup a service object, it's config object is passed to this * function for further validation. This does not validate service key @@ -395,6 +458,12 @@ config_service(const config_line_t *line, const or_options_t *options, goto err; } tor_assert(service->version <= HS_VERSION_MAX); + /* Before we configure the service with the per-version handler, we'll make + * sure that this set of options for a service are valid that is for + * instance an option only for v2 is not used for v3. */ + if (config_has_invalid_options(line->next, service)) { + goto err; + } /* Check permission on service directory that was just parsed. And this must * be done regardless of the service version. Do not ask for the directory * to be created, this is done when the keys are loaded because we could be From 87f6f96f4707cc18a58c5de8be0ee10f1893673d Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 29 Jun 2017 11:18:24 -0400 Subject: [PATCH 11/22] hs: Add rend_service_init() Initialize both the global and staging service lists. Signed-off-by: David Goulet --- src/or/hs_service.c | 3 +++ src/or/rendservice.c | 11 +++++++++++ src/or/rendservice.h | 1 + 3 files changed, 15 insertions(+) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index bce976ee8c..eb58c768bb 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -616,6 +616,9 @@ hs_service_init(void) tor_assert(!hs_service_map); tor_assert(!hs_service_staging_list); + /* v2 specific. */ + rend_service_init(); + hs_service_map = tor_malloc_zero(sizeof(struct hs_service_ht)); HT_INIT(hs_service_ht, hs_service_map); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 358efd0ed9..67da760069 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -257,6 +257,17 @@ rend_service_free_all(void) rend_service_free_staging_list(); } +/* Initialize the subsystem. */ +void +rend_service_init(void) +{ + tor_assert(!rend_service_list); + tor_assert(!rend_service_staging_list); + + rend_service_list = smartlist_new(); + rend_service_staging_list = smartlist_new(); +} + /* Validate a service. Use the service_list to make sure there * is no duplicate entry for the given service object. Return 0 if valid else * -1 if not.*/ diff --git a/src/or/rendservice.h b/src/or/rendservice.h index 361a119e49..20e827d2aa 100644 --- a/src/or/rendservice.h +++ b/src/or/rendservice.h @@ -179,6 +179,7 @@ int rend_service_set_connection_addr_port(edge_connection_t *conn, origin_circuit_t *circ); void rend_service_dump_stats(int severity); void rend_service_free_all(void); +void rend_service_init(void); rend_service_port_config_t *rend_service_parse_port_config(const char *string, const char *sep, From 418059dd96f5f427eceffff1daeb2a2f6c4adbeb Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 17 Jan 2017 12:09:54 -0500 Subject: [PATCH 12/22] test: Add v3 service config and registration test This tests our hs_config.c API to properly load v3 services and register them to the global map. It does NOT test the service object validity, that will be the hs service unit test later on. At this commit, we have 100% code coverage of hs_config.c. Signed-off-by: David Goulet --- src/or/hs_service.c | 20 ++ src/or/hs_service.h | 11 ++ src/test/test_hs_config.c | 391 ++++++++++++++++++++++++++++++++------ 3 files changed, 366 insertions(+), 56 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index eb58c768bb..854ce9e541 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -6,6 +6,8 @@ * \brief Implement next generation hidden service functionality **/ +#define HS_SERVICE_PRIVATE + #include "or.h" #include "circuitlist.h" #include "config.h" @@ -786,3 +788,21 @@ generate_establish_intro_cell(const uint8_t *circuit_key_material, return NULL; } +#ifdef TOR_UNIT_TESTS + +/* Return the global service map size. Only used by unit test. */ +STATIC unsigned int +get_hs_service_map_size(void) +{ + return HT_SIZE(hs_service_map); +} + +/* Return the staging list size. Only used by unit test. */ +STATIC int +get_hs_service_staging_list_size(void) +{ + return smartlist_len(hs_service_staging_list); +} + +#endif /* TOR_UNIT_TESTS */ + diff --git a/src/or/hs_service.h b/src/or/hs_service.h index 90606acb1c..cd154d3fe9 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -224,5 +224,16 @@ ssize_t get_establish_intro_payload(uint8_t *buf, size_t buf_len, const trn_cell_establish_intro_t *cell); +#ifdef HS_SERVICE_PRIVATE + +#ifdef TOR_UNIT_TESTS + +STATIC unsigned int get_hs_service_map_size(void); +STATIC int get_hs_service_staging_list_size(void); + +#endif /* TOR_UNIT_TESTS */ + +#endif /* HS_SERVICE_PRIVATE */ + #endif /* TOR_HS_SERVICE_H */ diff --git a/src/test/test_hs_config.c b/src/test/test_hs_config.c index 18b11948a4..343ce9f2f8 100644 --- a/src/test/test_hs_config.c +++ b/src/test/test_hs_config.c @@ -7,15 +7,20 @@ */ #define CONFIG_PRIVATE +#define HS_SERVICE_PRIVATE #include "test.h" #include "test_helpers.h" #include "log_test_helpers.h" -#include "hs_config.h" + #include "config.h" +#include "hs_common.h" +#include "hs_config.h" +#include "hs_service.h" +#include "rendservice.h" static int -helper_config_service_v2(const char *conf, int validate_only) +helper_config_service(const char *conf, int validate_only) { int ret = 0; or_options_t *options = NULL; @@ -28,6 +33,157 @@ helper_config_service_v2(const char *conf, int validate_only) return ret; } +static void +test_invalid_service(void *arg) +{ + int ret; + + (void) arg; + + /* Try with a missing port configuration. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 1\n"; /* Wrong not supported version. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceVersion must be between 2 and 3"); + teardown_capture_of_logs(); + } + + /* Bad value of HiddenServiceAllowUnknownPorts. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServiceAllowUnknownPorts 2\n"; /* Should be 0 or 1. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceAllowUnknownPorts must be " + "between 0 and 1, not 2"); + teardown_capture_of_logs(); + } + + /* Bad value of HiddenServiceDirGroupReadable */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServiceDirGroupReadable 2\n"; /* Should be 0 or 1. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceDirGroupReadable must be " + "between 0 and 1, not 2"); + teardown_capture_of_logs(); + } + + /* Bad value of HiddenServiceMaxStreamsCloseCircuit */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServiceMaxStreamsCloseCircuit 2\n"; /* Should be 0 or 1. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceMaxStreamsCloseCircuit must " + "be between 0 and 1, not 2"); + teardown_capture_of_logs(); + } + + /* Too much max streams. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceMaxStreams 65536\n"; /* One too many. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceMaxStreams must be between " + "0 and 65535, not 65536"); + teardown_capture_of_logs(); + } + + /* Duplicate directory directive. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 81\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("Another hidden service is already " + "configured for directory"); + teardown_capture_of_logs(); + } + + /* Bad port. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 65536\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("Missing or invalid port"); + teardown_capture_of_logs(); + } + + /* Out of order directives. */ + { + const char *conf = + "HiddenServiceVersion 2\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServicePort 80\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceVersion with no preceding " + "HiddenServiceDir directive"); + teardown_capture_of_logs(); + } + + done: + ; +} + +static void +test_valid_service(void *arg) +{ + int ret; + + (void) arg; + + /* Mix of v2 and v3. Still valid. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs2\n" + "HiddenServiceVersion 3\n" + "HiddenServicePort 81\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs3\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 82\n"; + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, 0); + } + + done: + ; +} + static void test_invalid_service_v2(void *arg) { @@ -41,39 +197,12 @@ test_invalid_service_v2(void *arg) "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" "HiddenServiceVersion 2\n"; setup_full_capture_of_logs(LOG_WARN); - ret = helper_config_service_v2(conf, validate_only); + ret = helper_config_service(conf, validate_only); tt_int_op(ret, OP_EQ, -1); expect_log_msg_containing("with no ports configured."); teardown_capture_of_logs(); } - /* Out of order directives. */ - { - const char *conf = - "HiddenServiceVersion 2\n" - "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" - "HiddenServicePort 80\n"; - setup_full_capture_of_logs(LOG_WARN); - ret = helper_config_service_v2(conf, validate_only); - tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceVersion with no preceding " - "HiddenServiceDir directive"); - teardown_capture_of_logs(); - } - - /* Bad port. */ - { - const char *conf = - "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" - "HiddenServiceVersion 2\n" - "HiddenServicePort 65536\n"; - setup_full_capture_of_logs(LOG_WARN); - ret = helper_config_service_v2(conf, validate_only); - tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("Missing or invalid port"); - teardown_capture_of_logs(); - } - /* Too many introduction points. */ { const char *conf = @@ -82,25 +211,25 @@ test_invalid_service_v2(void *arg) "HiddenServicePort 80\n" "HiddenServiceNumIntroductionPoints 11\n"; /* One too many. */ setup_full_capture_of_logs(LOG_WARN); - ret = helper_config_service_v2(conf, validate_only); + ret = helper_config_service(conf, validate_only); tt_int_op(ret, OP_EQ, -1); expect_log_msg_containing("HiddenServiceNumIntroductionPoints should " "be between 0 and 10, not 11"); teardown_capture_of_logs(); } - /* Too much max streams. */ + /* Too little introduction points. */ { const char *conf = "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" "HiddenServiceVersion 2\n" "HiddenServicePort 80\n" - "HiddenServiceMaxStreams 65536\n"; /* One too many. */ + "HiddenServiceNumIntroductionPoints -1\n"; setup_full_capture_of_logs(LOG_WARN); - ret = helper_config_service_v2(conf, validate_only); + ret = helper_config_service(conf, validate_only); tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("HiddenServiceMaxStreams should be between " - "0 and 65535, not 65536"); + expect_log_msg_containing("HiddenServiceNumIntroductionPoints should " + "be between 0 and 10, not -1"); teardown_capture_of_logs(); } @@ -112,30 +241,13 @@ test_invalid_service_v2(void *arg) "HiddenServicePort 80\n" "HiddenServiceAuthorizeClient blah alice,bob\n"; /* blah is no good. */ setup_full_capture_of_logs(LOG_WARN); - ret = helper_config_service_v2(conf, validate_only); + ret = helper_config_service(conf, validate_only); tt_int_op(ret, OP_EQ, -1); expect_log_msg_containing("HiddenServiceAuthorizeClient contains " "unrecognized auth-type"); teardown_capture_of_logs(); } - /* Duplicate directory directive. */ - { - const char *conf = - "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" - "HiddenServiceVersion 2\n" - "HiddenServicePort 80\n" - "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" - "HiddenServiceVersion 2\n" - "HiddenServicePort 81\n"; - setup_full_capture_of_logs(LOG_WARN); - ret = helper_config_service_v2(conf, validate_only); - tt_int_op(ret, OP_EQ, -1); - expect_log_msg_containing("Another hidden service is already " - "configured for directory"); - teardown_capture_of_logs(); - } - done: ; } @@ -161,7 +273,7 @@ test_valid_service_v2(void *arg) "HiddenServiceMaxStreamsCloseCircuit 0\n" "HiddenServiceDirGroupReadable 1\n" "HiddenServiceNumIntroductionPoints 7\n"; - ret = helper_config_service_v2(conf, 1); + ret = helper_config_service(conf, 1); tt_int_op(ret, OP_EQ, 0); } @@ -179,7 +291,7 @@ test_valid_service_v2(void *arg) "HiddenServiceMaxStreamsCloseCircuit 0\n" "HiddenServiceDirGroupReadable 1\n" "HiddenServiceNumIntroductionPoints 8\n"; - ret = helper_config_service_v2(conf, 1); + ret = helper_config_service(conf, 1); tt_int_op(ret, OP_EQ, 0); } @@ -187,12 +299,179 @@ test_valid_service_v2(void *arg) ; } +static void +test_invalid_service_v3(void *arg) +{ + int validate_only = 1, ret; + + (void) arg; + + /* Try with a missing port configuration. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 3\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("with no ports configured."); + teardown_capture_of_logs(); + } + + /* Too many introduction points. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 3\n" + "HiddenServicePort 80\n" + "HiddenServiceNumIntroductionPoints 21\n"; /* One too many. */ + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceNumIntroductionPoints must " + "be between 3 and 20, not 21."); + teardown_capture_of_logs(); + } + + /* Too little introduction points. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 3\n" + "HiddenServicePort 80\n" + "HiddenServiceNumIntroductionPoints 1\n"; + setup_full_capture_of_logs(LOG_WARN); + ret = helper_config_service(conf, validate_only); + tt_int_op(ret, OP_EQ, -1); + expect_log_msg_containing("HiddenServiceNumIntroductionPoints must " + "be between 3 and 20, not 1."); + teardown_capture_of_logs(); + } + + done: + ; +} + +static void +test_valid_service_v3(void *arg) +{ + int ret; + + (void) arg; + + /* Valid complex configuration. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 3\n" + "HiddenServicePort 80\n" + "HiddenServicePort 22 localhost:22\n" + "HiddenServicePort 42 unix:/path/to/socket\n" + "HiddenServiceAllowUnknownPorts 1\n" + "HiddenServiceMaxStreams 42\n" + "HiddenServiceMaxStreamsCloseCircuit 0\n" + "HiddenServiceDirGroupReadable 1\n" + "HiddenServiceNumIntroductionPoints 7\n"; + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, 0); + } + + /* Valid complex configuration. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs2\n" + "HiddenServiceVersion 3\n" + "HiddenServicePort 65535\n" + "HiddenServicePort 22 1.1.1.1:22\n" + "HiddenServicePort 9000 unix:/path/to/socket\n" + "HiddenServiceAllowUnknownPorts 0\n" + "HiddenServiceMaxStreams 42\n" + "HiddenServiceMaxStreamsCloseCircuit 0\n" + "HiddenServiceDirGroupReadable 1\n" + "HiddenServiceNumIntroductionPoints 20\n"; + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, 0); + } + + /* Mix of v2 and v3. Still valid. */ + { + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs1\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 80\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs2\n" + "HiddenServiceVersion 3\n" + "HiddenServicePort 81\n" + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs3\n" + "HiddenServiceVersion 2\n" + "HiddenServicePort 82\n"; + ret = helper_config_service(conf, 1); + tt_int_op(ret, OP_EQ, 0); + } + + done: + ; +} + +static void +test_staging_service_v3(void *arg) +{ + int ret; + + (void) arg; + + /* We don't validate a service object, this is the service test that are in + * charge of doing so. We just check for the stable state after + * registration. */ + + hs_init(); + + /* Time for a valid v3 service that should get staged. */ + const char *conf = + "HiddenServiceDir /tmp/tor-test-hs-RANDOM/hs2\n" + "HiddenServiceVersion 3\n" + "HiddenServicePort 65535\n" + "HiddenServicePort 22 1.1.1.1:22\n" + "HiddenServicePort 9000 unix:/path/to/socket\n" + "HiddenServiceAllowUnknownPorts 0\n" + "HiddenServiceMaxStreams 42\n" + "HiddenServiceMaxStreamsCloseCircuit 0\n" + "HiddenServiceDirGroupReadable 1\n" + "HiddenServiceNumIntroductionPoints 20\n"; + ret = helper_config_service(conf, 0); + tt_int_op(ret, OP_EQ, 0); + /* Ok, we have a service in our map! Registration went well. */ + tt_int_op(get_hs_service_staging_list_size(), OP_EQ, 1); + /* Make sure we don't have a magic v2 service out of this. */ + tt_int_op(num_rend_services(), OP_EQ, 0); + + done: + hs_free_all(); +} + struct testcase_t hs_config_tests[] = { + /* Invalid service not specific to any version. */ + { "invalid_service", test_invalid_service, TT_FORK, + NULL, NULL }, + { "valid_service", test_valid_service, TT_FORK, + NULL, NULL }, + + /* Test case only for version 2. */ { "invalid_service_v2", test_invalid_service_v2, TT_FORK, NULL, NULL }, { "valid_service_v2", test_valid_service_v2, TT_FORK, NULL, NULL }, + /* Test case only for version 3. */ + { "invalid_service_v3", test_invalid_service_v3, TT_FORK, + NULL, NULL }, + { "valid_service_v3", test_valid_service_v3, TT_FORK, + NULL, NULL }, + + /* Test service staging. */ + { "staging_service_v3", test_staging_service_v3, TT_FORK, + NULL, NULL }, + END_OF_TESTCASES }; From 09b12c40947ea496c0bfaeeafba7540925c17a32 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 2 Feb 2017 15:26:04 -0500 Subject: [PATCH 13/22] test: Add v3 service load keys and accessors Signed-off-by: David Goulet --- src/or/hs_service.c | 27 ++++-- src/or/hs_service.h | 14 ++++ src/test/test_hs_service.c | 166 +++++++++++++++++++++++++++++++++++++ 3 files changed, 199 insertions(+), 8 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 854ce9e541..bfce7804ab 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -57,11 +57,6 @@ hs_service_ht_hash(const hs_service_t *service) sizeof(service->keys.identity_pk.pubkey)); } -/* For the service global hash map, we define a specific type for it which - * will make it safe to use and specific to some controlled parameters such as - * the hashing function and how to compare services. */ -typedef HT_HEAD(hs_service_ht, hs_service_t) hs_service_ht; - /* This is _the_ global hash map of hidden services which indexed the service * contained in it by master public identity key which is roughly the onion * address of the service. */ @@ -82,7 +77,7 @@ HT_GENERATE2(hs_service_ht, hs_service_t, hs_service_node, * if found else NULL. It is also possible to set a directory path in the * search query. If pk is NULL, then it will be set to zero indicating the * hash table to compare the directory path instead. */ -static hs_service_t * +STATIC hs_service_t * find_service(hs_service_ht *map, const ed25519_public_key_t *pk) { hs_service_t dummy_service = {0}; @@ -95,7 +90,7 @@ find_service(hs_service_ht *map, const ed25519_public_key_t *pk) /* Register the given service in the given map. If the service already exists * in the map, -1 is returned. On success, 0 is returned and the service * ownership has been transfered to the global map. */ -static int +STATIC int register_service(hs_service_ht *map, hs_service_t *service) { tor_assert(map); @@ -113,7 +108,7 @@ register_service(hs_service_ht *map, hs_service_t *service) /* Remove a given service from the given map. If service is NULL or the * service key is unset, return gracefully. */ -static void +STATIC void remove_service(hs_service_ht *map, hs_service_t *service) { hs_service_t *elm; @@ -804,5 +799,21 @@ get_hs_service_staging_list_size(void) return smartlist_len(hs_service_staging_list); } +STATIC hs_service_ht * +get_hs_service_map(void) +{ + return hs_service_map; +} + +STATIC hs_service_t * +get_first_service(void) +{ + hs_service_t **obj = HT_START(hs_service_ht, hs_service_map); + if (obj == NULL) { + return NULL; + } + return *obj; +} + #endif /* TOR_UNIT_TESTS */ diff --git a/src/or/hs_service.h b/src/or/hs_service.h index cd154d3fe9..a98884f6bc 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -200,6 +200,11 @@ typedef struct hs_service_t { } hs_service_t; +/* For the service global hash map, we define a specific type for it which + * will make it safe to use and specific to some controlled parameters such as + * the hashing function and how to compare services. */ +typedef HT_HEAD(hs_service_ht, hs_service_t) hs_service_ht; + /* API */ /* Global initializer and cleanup function. */ @@ -228,8 +233,17 @@ get_establish_intro_payload(uint8_t *buf, size_t buf_len, #ifdef TOR_UNIT_TESTS +/* Useful getters for unit tests. */ STATIC unsigned int get_hs_service_map_size(void); STATIC int get_hs_service_staging_list_size(void); +STATIC hs_service_ht *get_hs_service_map(void); +STATIC hs_service_t *get_first_service(void); + +/* Service accessors. */ +STATIC hs_service_t *find_service(hs_service_ht *map, + const ed25519_public_key_t *pk); +STATIC void remove_service(hs_service_ht *map, hs_service_t *service); +STATIC int register_service(hs_service_ht *map, hs_service_t *service); #endif /* TOR_UNIT_TESTS */ diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index e081b7f2f3..c695b90bc9 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -8,14 +8,17 @@ #define CIRCUITBUILD_PRIVATE #define CIRCUITLIST_PRIVATE +#define CONFIG_PRIVATE #define CONNECTION_PRIVATE #define CRYPTO_PRIVATE #define HS_COMMON_PRIVATE +#define HS_SERVICE_PRIVATE #define HS_INTROPOINT_PRIVATE #define MAIN_PRIVATE #define TOR_CHANNEL_INTERNAL_ #include "test.h" +#include "test_helpers.h" #include "log_test_helpers.h" #include "rend_test_helpers.h" @@ -26,8 +29,10 @@ #include "circuituse.h" #include "config.h" #include "connection.h" +#include "crypto.h" #include "hs_circuit.h" #include "hs_common.h" +#include "hs_config.h" #include "hs_ident.h" #include "hs_intropoint.h" #include "hs_ntor.h" @@ -35,6 +40,25 @@ #include "main.h" #include "rendservice.h" +/* Trunnel */ +#include "hs/cell_establish_intro.h" + +/* Helper: from a set of options in conf, configure a service which will add + * it to the staging list of the HS subsytem. */ +static int +helper_config_service(const char *conf) +{ + int ret = 0; + or_options_t *options = NULL; + tt_assert(conf); + options = helper_parse_options(conf); + tt_assert(options); + ret = hs_config_service_all(options, 0); + done: + or_options_free(options); + return ret; +} + /** We simulate the creation of an outgoing ESTABLISH_INTRO cell, and then we * parse it from the receiver side. */ static void @@ -394,6 +418,144 @@ test_e2e_rend_circuit_setup(void *arg) circuit_free(TO_CIRCUIT(or_circ)); } +static void +test_load_keys(void *arg) +{ + int ret; + char *conf = NULL; + char *hsdir_v2 = tor_strdup(get_fname("hs2")); + char *hsdir_v3 = tor_strdup(get_fname("hs3")); + char addr[HS_SERVICE_ADDR_LEN_BASE32 + 1]; + + (void) arg; + + /* We'll register two services, a v2 and a v3, then we'll load keys and + * validate that both are in a correct state. */ + + hs_init(); + +#define conf_fmt \ + "HiddenServiceDir %s\n" \ + "HiddenServiceVersion %d\n" \ + "HiddenServicePort 65535\n" + + /* v2 service. */ + tor_asprintf(&conf, conf_fmt, hsdir_v2, HS_VERSION_TWO); + ret = helper_config_service(conf); + tor_free(conf); + tt_int_op(ret, OP_EQ, 0); + /* This one should now be registered into the v2 list. */ + tt_int_op(get_hs_service_staging_list_size(), OP_EQ, 0); + tt_int_op(num_rend_services(), OP_EQ, 1); + + /* v3 service. */ + tor_asprintf(&conf, conf_fmt, hsdir_v3, HS_VERSION_THREE); + ret = helper_config_service(conf); + tor_free(conf); + tt_int_op(ret, OP_EQ, 0); + /* It's in staging? */ + tt_int_op(get_hs_service_staging_list_size(), OP_EQ, 1); + + /* Load the keys for these. After that, the v3 service should be registered + * in the global map. */ + hs_service_load_all_keys(); + tt_int_op(get_hs_service_map_size(), OP_EQ, 1); + hs_service_t *s = get_first_service(); + tt_assert(s); + + /* Ok we have the service object. Validate few things. */ + tt_assert(!tor_mem_is_zero(s->onion_address, sizeof(s->onion_address))); + tt_int_op(hs_address_is_valid(s->onion_address), OP_EQ, 1); + tt_assert(!tor_mem_is_zero((char *) s->keys.identity_sk.seckey, + ED25519_SECKEY_LEN)); + tt_assert(!tor_mem_is_zero((char *) s->keys.identity_pk.pubkey, + ED25519_PUBKEY_LEN)); + /* Check onion address from identity key. */ + hs_build_address(&s->keys.identity_pk, s->version, addr); + tt_int_op(hs_address_is_valid(addr), OP_EQ, 1); + tt_str_op(addr, OP_EQ, s->onion_address); + + done: + tor_free(hsdir_v2); + tor_free(hsdir_v3); + hs_free_all(); +} + +static void +test_access_service(void *arg) +{ + int ret; + char *conf = NULL; + char *hsdir_v3 = tor_strdup(get_fname("hs3")); + hs_service_ht *global_map; + + (void) arg; + + /* We'll register two services, a v2 and a v3, then we'll load keys and + * validate that both are in a correct state. */ + + hs_init(); + +#define conf_fmt \ + "HiddenServiceDir %s\n" \ + "HiddenServiceVersion %d\n" \ + "HiddenServicePort 65535\n" + + /* v3 service. */ + tor_asprintf(&conf, conf_fmt, hsdir_v3, HS_VERSION_THREE); + ret = helper_config_service(conf); + tor_free(conf); + tt_int_op(ret, OP_EQ, 0); + /* It's in staging? */ + tt_int_op(get_hs_service_staging_list_size(), OP_EQ, 1); + + /* Load the keys for these. After that, the v3 service should be registered + * in the global map. */ + hs_service_load_all_keys(); + tt_int_op(get_hs_service_map_size(), OP_EQ, 1); + hs_service_t *s = get_first_service(); + tt_assert(s); + global_map = get_hs_service_map(); + tt_assert(global_map); + + /* From here, we'll try the service accessors. */ + hs_service_t *query = find_service(global_map, &s->keys.identity_pk); + tt_assert(query); + tt_mem_op(query, OP_EQ, s, sizeof(hs_service_t)); + /* Remove service, check if it actually works and then put it back. */ + remove_service(global_map, s); + tt_int_op(get_hs_service_map_size(), OP_EQ, 0); + query = find_service(global_map, &s->keys.identity_pk); + tt_assert(!query); + + /* Register back the service in the map. */ + ret = register_service(global_map, s); + tt_int_op(ret, OP_EQ, 0); + tt_int_op(get_hs_service_map_size(), OP_EQ, 1); + /* Twice should fail. */ + ret = register_service(global_map, s); + tt_int_op(ret, OP_EQ, -1); + /* Modify key of service and we should be able to put it back in. */ + s->keys.identity_pk.pubkey[1] = '\x42'; + ret = register_service(global_map, s); + tt_int_op(ret, OP_EQ, 0); + tt_int_op(get_hs_service_map_size(), OP_EQ, 2); + /* Remove service from map so we don't double free on cleanup. */ + remove_service(global_map, s); + tt_int_op(get_hs_service_map_size(), OP_EQ, 1); + query = find_service(global_map, &s->keys.identity_pk); + tt_assert(!query); + /* Let's try to remove twice for fun. */ + setup_full_capture_of_logs(LOG_WARN); + remove_service(global_map, s); + expect_log_msg_containing("Could not find service in the global map"); + teardown_capture_of_logs(); + + done: + tor_free(hsdir_v3); + hs_free_all(); +} + struct testcase_t hs_service_tests[] = { { "gen_establish_intro_cell", test_gen_establish_intro_cell, TT_FORK, NULL, NULL }, @@ -409,6 +571,10 @@ struct testcase_t hs_service_tests[] = { NULL, NULL }, { "validate_address", test_validate_address, TT_FORK, NULL, NULL }, + { "load_keys", test_load_keys, TT_FORK, + NULL, NULL }, + { "access_service", test_access_service, TT_FORK, + NULL, NULL }, END_OF_TESTCASES }; From 28f643139951ee6b60cefa7d7a77bbb257c3af50 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 11 Jul 2017 16:17:58 -0400 Subject: [PATCH 14/22] Revert "fixup! prop224: Add hs_config.{c|h} with a refactoring" This reverts commit e2497e2ba038133026a475f0f93c9054187b2a1d. --- src/or/hs_config.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/or/hs_config.c b/src/or/hs_config.c index afc9631cd4..3baab4688b 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -287,7 +287,6 @@ config_generic_service(const config_line_t *line_, /* Makes thing easier. */ config = &service->config; - memset(config, 0, sizeof(*config)); /* The first line starts with HiddenServiceDir so we consider what's next is * the configuration of the service. */ From cfa6f8358b4c5ea6c9c82a0818dc81b8aaf44a78 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 12 Jul 2017 10:37:10 -0400 Subject: [PATCH 15/22] prop224: Use a common function to parse uint64_t Add a helper function to parse uint64_t and also does logging so we can reduce the amount of duplicate code. Signed-off-by: David Goulet --- src/or/hs_config.c | 100 ++++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 56 deletions(-) diff --git a/src/or/hs_config.c b/src/or/hs_config.c index 3baab4688b..f1e130bf22 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -115,6 +115,33 @@ service_is_duplicate_in_list(const smartlist_t *service_list, return ret; } +/* Helper function: Given an configuration option name, its value, a minimum + * min and a maxium max, parse the value as a uint64_t. On success, ok is set + * to 1 and ret is the parsed value. On error, ok is set to 0 and ret must be + * ignored. This function logs both on error and success. */ +static uint64_t +helper_parse_uint64(const char *opt, const char *value, uint64_t min, + uint64_t max, int *ok) +{ + uint64_t ret = 0; + + tor_assert(opt); + tor_assert(value); + tor_assert(ok); + + *ok = 0; + ret = tor_parse_uint64(value, 10, min, max, ok, NULL); + if (!*ok) { + log_warn(LD_CONFIG, "%s must be between %" PRIu64 " and %"PRIu64 + ", not %s.", + opt, min, max, value); + goto err; + } + log_info(LD_CONFIG, "%s was parsed to %" PRIu64, opt, ret); + err: + return ret; +} + /* Return true iff the given options starting at line_ for a hidden service * contains at least one invalid option. Each hidden service option don't * apply to all versions so this function can find out. The line_ MUST start @@ -222,27 +249,21 @@ config_service_v3(const config_line_t *line_, config = &service->config; for (line = line_; line; line = line->next) { + int ok = 0; if (!strcasecmp(line->key, "HiddenServiceDir")) { /* We just hit the next hidden service, stop right now. */ break; } /* Number of introduction points. */ if (!strcasecmp(line->key, "HiddenServiceNumIntroductionPoints")) { - int ok = 0; config->num_intro_points = - (unsigned int) tor_parse_ulong(line->value, 10, - NUM_INTRO_POINTS_DEFAULT, - HS_CONFIG_V3_MAX_INTRO_POINTS, - &ok, NULL); + (unsigned int) helper_parse_uint64(line->key, line->value, + NUM_INTRO_POINTS_DEFAULT, + HS_CONFIG_V3_MAX_INTRO_POINTS, + &ok); if (!ok) { - log_warn(LD_CONFIG, "HiddenServiceNumIntroductionPoints " - "should be between %d and %d, not %s", - NUM_INTRO_POINTS_DEFAULT, HS_CONFIG_V3_MAX_INTRO_POINTS, - line->value); goto err; } - log_info(LD_CONFIG, "HiddenServiceNumIntroductionPoints=%d for %s", - config->num_intro_points, escaped(config->directory_path)); continue; } } @@ -277,7 +298,7 @@ config_generic_service(const config_line_t *line_, const or_options_t *options, hs_service_t *service) { - int ok, dir_seen = 0; + int dir_seen = 0; const config_line_t *line; hs_service_config_t *config; @@ -291,6 +312,7 @@ config_generic_service(const config_line_t *line_, /* The first line starts with HiddenServiceDir so we consider what's next is * the configuration of the service. */ for (line = line_; line ; line = line->next) { + int ok = 0; /* This indicate that we have a new service to configure. */ if (!strcasecmp(line->key, "HiddenServiceDir")) { /* This function only configures one service at a time so if we've @@ -310,18 +332,12 @@ config_generic_service(const config_line_t *line_, } /* Version of the service. */ if (!strcasecmp(line->key, "HiddenServiceVersion")) { - service->version = (uint32_t) tor_parse_ulong(line->value, - 10, HS_VERSION_MIN, - HS_VERSION_MAX, - &ok, NULL); + service->version = + (uint32_t) helper_parse_uint64(line->key, line->value, HS_VERSION_MIN, + HS_VERSION_MAX, &ok); if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceVersion be between %u and %u, not %s", - HS_VERSION_TWO, HS_VERSION_MAX, line->value); goto err; } - log_info(LD_CONFIG, "HiddenServiceVersion=%" PRIu32 " for %s", - service->version, escaped(config->directory_path)); continue; } /* Virtual port. */ @@ -345,67 +361,39 @@ config_generic_service(const config_line_t *line_, } /* Do we allow unknown ports. */ if (!strcasecmp(line->key, "HiddenServiceAllowUnknownPorts")) { - config->allow_unknown_ports = (unsigned int) tor_parse_long(line->value, - 10, 0, 1, - &ok, NULL); + config->allow_unknown_ports = + (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceAllowUnknownPorts should be 0 or 1, not %s", - line->value); goto err; } - log_info(LD_CONFIG, - "HiddenServiceAllowUnknownPorts=%u for %s", - config->allow_unknown_ports, escaped(config->directory_path)); continue; } /* Directory group readable. */ if (!strcasecmp(line->key, "HiddenServiceDirGroupReadable")) { - config->dir_group_readable = (unsigned int) tor_parse_long(line->value, - 10, 0, 1, - &ok, NULL); + config->dir_group_readable = + (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceDirGroupReadable should be 0 or 1, not %s", - line->value); goto err; } - log_info(LD_CONFIG, - "HiddenServiceDirGroupReadable=%u for %s", - config->dir_group_readable, escaped(config->directory_path)); continue; } /* Maximum streams per circuit. */ if (!strcasecmp(line->key, "HiddenServiceMaxStreams")) { config->max_streams_per_rdv_circuit = - tor_parse_uint64(line->value, 10, 0, - HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT, &ok, NULL); + helper_parse_uint64(line->key, line->value, 0, + HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT, &ok); if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceMaxStreams should be between 0 and %d, not %s", - HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT, line->value); goto err; } - log_info(LD_CONFIG, - "HiddenServiceMaxStreams=%" PRIu64 " for %s", - config->max_streams_per_rdv_circuit, - escaped(config->directory_path)); continue; } /* Maximum amount of streams before we close the circuit. */ if (!strcasecmp(line->key, "HiddenServiceMaxStreamsCloseCircuit")) { config->max_streams_close_circuit = - (unsigned int) tor_parse_long(line->value, 10, 0, 1, &ok, NULL); + (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); if (!ok) { - log_warn(LD_CONFIG, - "HiddenServiceMaxStreamsCloseCircuit should be 0 or 1, " - "not %s", line->value); goto err; } - log_info(LD_CONFIG, - "HiddenServiceMaxStreamsCloseCircuit=%u for %s", - config->max_streams_close_circuit, - escaped(config->directory_path)); continue; } } From e9dd4ed16d8606dc0aa0cd6eadc99aa959f93392 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 12 Jul 2017 11:02:59 -0400 Subject: [PATCH 16/22] prop224: Detect duplicate configuration options Signed-off-by: David Goulet --- src/or/hs_config.c | 46 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/src/or/hs_config.c b/src/or/hs_config.c index f1e130bf22..2e75a4ef0b 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -241,6 +241,8 @@ config_service_v3(const config_line_t *line_, hs_service_t *service) { (void) options; + int have_num_ip = 0; + const char *dup_opt_seen = NULL; const config_line_t *line; hs_service_config_t *config; @@ -261,9 +263,12 @@ config_service_v3(const config_line_t *line_, NUM_INTRO_POINTS_DEFAULT, HS_CONFIG_V3_MAX_INTRO_POINTS, &ok); - if (!ok) { + if (!ok || have_num_ip) { + if (have_num_ip) + dup_opt_seen = line->key; goto err; } + have_num_ip = 1; continue; } } @@ -279,6 +284,9 @@ config_service_v3(const config_line_t *line_, return 0; err: + if (dup_opt_seen) { + log_warn(LD_CONFIG, "Duplicate directive %s.", dup_opt_seen); + } return -1; } @@ -301,6 +309,13 @@ config_generic_service(const config_line_t *line_, int dir_seen = 0; const config_line_t *line; hs_service_config_t *config; + /* If this is set, we've seen a duplicate of this option. Keep the string + * so we can log the directive. */ + const char *dup_opt_seen = NULL; + /* These variables will tell us if we ever have duplicate. */ + int have_version = 0, have_allow_unknown_ports = 0; + int have_dir_group_read = 0, have_max_streams = 0; + int have_max_streams_close = 0; tor_assert(line_); tor_assert(options); @@ -313,6 +328,7 @@ config_generic_service(const config_line_t *line_, * the configuration of the service. */ for (line = line_; line ; line = line->next) { int ok = 0; + /* This indicate that we have a new service to configure. */ if (!strcasecmp(line->key, "HiddenServiceDir")) { /* This function only configures one service at a time so if we've @@ -335,9 +351,12 @@ config_generic_service(const config_line_t *line_, service->version = (uint32_t) helper_parse_uint64(line->key, line->value, HS_VERSION_MIN, HS_VERSION_MAX, &ok); - if (!ok) { + if (!ok || have_version) { + if (have_version) + dup_opt_seen = line->key; goto err; } + have_version = 1; continue; } /* Virtual port. */ @@ -363,18 +382,24 @@ config_generic_service(const config_line_t *line_, if (!strcasecmp(line->key, "HiddenServiceAllowUnknownPorts")) { config->allow_unknown_ports = (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); - if (!ok) { + if (!ok || have_allow_unknown_ports) { + if (have_allow_unknown_ports) + dup_opt_seen = line->key; goto err; } + have_allow_unknown_ports = 1; continue; } /* Directory group readable. */ if (!strcasecmp(line->key, "HiddenServiceDirGroupReadable")) { config->dir_group_readable = (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); - if (!ok) { + if (!ok || have_dir_group_read) { + if (have_dir_group_read) + dup_opt_seen = line->key; goto err; } + have_dir_group_read = 1; continue; } /* Maximum streams per circuit. */ @@ -382,18 +407,24 @@ config_generic_service(const config_line_t *line_, config->max_streams_per_rdv_circuit = helper_parse_uint64(line->key, line->value, 0, HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT, &ok); - if (!ok) { + if (!ok || have_max_streams) { + if (have_max_streams) + dup_opt_seen = line->key; goto err; } + have_max_streams = 1; continue; } /* Maximum amount of streams before we close the circuit. */ if (!strcasecmp(line->key, "HiddenServiceMaxStreamsCloseCircuit")) { config->max_streams_close_circuit = (unsigned int) helper_parse_uint64(line->key, line->value, 0, 1, &ok); - if (!ok) { + if (!ok || have_max_streams_close) { + if (have_max_streams_close) + dup_opt_seen = line->key; goto err; } + have_max_streams_close = 1; continue; } } @@ -408,6 +439,9 @@ config_generic_service(const config_line_t *line_, /* Success */ return 0; err: + if (dup_opt_seen) { + log_warn(LD_CONFIG, "Duplicate directive %s.", dup_opt_seen); + } return -1; } From 750c684fff02fde3054261abbbdcc6a458bea8e0 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 12 Jul 2017 11:17:26 -0400 Subject: [PATCH 17/22] prop224: Don't use an array of config handlers As per nickm suggestion, an array of config handlers will not play well with our callgraph tool. Instead, we'll go with a switch case on the version which has a good side effect of allowing us to control what we pass to the function intead of a fix set of parameters. Signed-off-by: David Goulet --- src/or/hs_config.c | 62 +++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/src/or/hs_config.c b/src/or/hs_config.c index 2e75a4ef0b..c29315f6cf 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -19,9 +19,8 @@ * successful, transfert the service to the main global service list where * at that point it is ready to be used. * - * Configuration handlers are per-version (see config_service_handlers[]) and - * there is a main generic one for every option that is common to all version - * (config_generic_service). + * Configuration functions are per-version and there is a main generic one for + * every option that is common to all version (config_generic_service). **/ #define HS_CONFIG_PRIVATE @@ -227,7 +226,7 @@ config_validate_service(const hs_service_config_t *config) return -1; } -/* Configuration handler for a version 3 service. The line_ must be pointing +/* Configuration funcion for a version 3 service. The line_ must be pointing * to the directive directly after a HiddenServiceDir. That way, when hitting * the next HiddenServiceDir line or reaching the end of the list of lines, we * know that we have to stop looking for more options. The given service @@ -445,26 +444,16 @@ config_generic_service(const config_line_t *line_, return -1; } -/* Configuration handler indexed by version number. */ -static int - (*config_service_handlers[])(const config_line_t *line, - const or_options_t *options, - hs_service_t *service) = -{ - NULL, /* v0 */ - NULL, /* v1 */ - rend_config_service, /* v2 */ - config_service_v3, /* v3 */ -}; - /* Configure a service using the given line and options. This function will - * call the corresponding version handler and validate the service against the - * other one. On success, add the service to the given list and return 0. On - * error, nothing is added to the list and a negative value is returned. */ + * call the corresponding configuration function for a specific service + * version and validate the service against the other ones. On success, add + * the service to the given list and return 0. On error, nothing is added to + * the list and a negative value is returned. */ static int config_service(const config_line_t *line, const or_options_t *options, smartlist_t *service_list) { + int ret; hs_service_t *service = NULL; tor_assert(line); @@ -473,13 +462,13 @@ config_service(const config_line_t *line, const or_options_t *options, /* We have a new hidden service. */ service = hs_service_new(options); - /* We'll configure that service as a generic one and then pass it to the - * specific handler according to the configured version number. */ + /* We'll configure that service as a generic one and then pass it to a + * specific function according to the configured version number. */ if (config_generic_service(line, options, service) < 0) { goto err; } tor_assert(service->version <= HS_VERSION_MAX); - /* Before we configure the service with the per-version handler, we'll make + /* Before we configure the service on a per-version basis, we'll make * sure that this set of options for a service are valid that is for * instance an option only for v2 is not used for v3. */ if (config_has_invalid_options(line->next, service)) { @@ -495,11 +484,22 @@ config_service(const config_line_t *line, const or_options_t *options, 0) < 0) { goto err; } - /* The handler is in charge of specific options for a version. We start - * after this service directory line so once we hit another directory - * line, the handler knows that it has to stop. */ - if (config_service_handlers[service->version](line->next, options, - service) < 0) { + /* Different functions are in charge of specific options for a version. We + * start just after the service directory line so once we hit another + * directory line, the function knows that it has to stop parsing. */ + switch (service->version) { + case HS_VERSION_TWO: + ret = rend_config_service(line->next, options, service); + break; + case HS_VERSION_THREE: + ret = config_service_v3(line->next, options, service); + break; + default: + /* We do validate before if we support the parsed version. */ + tor_assert_nonfatal_unreached(); + goto err; + } + if (ret < 0) { goto err; } /* We'll check if this service can be kept depending on the others @@ -564,10 +564,10 @@ hs_config_service_all(const or_options_t *options, int validate_only) * services. We don't need those objects anymore. */ SMARTLIST_FOREACH(new_service_list, hs_service_t *, s, hs_service_free(s)); - /* For the v2 subsystem, the configuration handler adds the service object - * to the staging list and it is transferred in the main list through the - * prunning process. In validation mode, we thus have to purge the staging - * list so it's not kept in memory as valid service. */ + /* For the v2 subsystem, the configuration function adds the service + * object to the staging list and it is transferred in the main list + * through the prunning process. In validation mode, we thus have to purge + * the staging list so it's not kept in memory as valid service. */ rend_service_free_staging_list(); } From 1b048fbfaadec04f409ab8d120106025b009fec8 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 12 Jul 2017 11:53:26 -0400 Subject: [PATCH 18/22] prop224: Add a clear configuration function The added function frees any allocated pointers in a service configuration object and reset all values to 0. Signed-off-by: David Goulet --- src/or/hs_service.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index bfce7804ab..97d3288d17 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -149,6 +149,23 @@ set_service_default_config(hs_service_config_t *c, c->is_ephemeral = 0; } +/* From a service configuration object config, clear everything from it + * meaning free allocated pointers and reset the values. */ +static void +service_clear_config(hs_service_config_t *config) +{ + if (config == NULL) { + return; + } + tor_free(config->directory_path); + if (config->ports) { + SMARTLIST_FOREACH(config->ports, rend_service_port_config_t *, p, + rend_service_port_config_free(p);); + smartlist_free(config->ports); + } + memset(config, 0, sizeof(*config)); +} + /* Helper: Function that needs to return 1 for the HT for each loop which * frees every service in an hash map. */ static int @@ -592,12 +609,7 @@ hs_service_free(hs_service_t *service) } /* Free service configuration. */ - tor_free(service->config.directory_path); - if (service->config.ports) { - SMARTLIST_FOREACH(service->config.ports, rend_service_port_config_t *, p, - rend_service_port_config_free(p);); - smartlist_free(service->config.ports); - } + service_clear_config(&service->config); /* Wipe service keys. */ memwipe(&service->keys.identity_sk, 0, sizeof(service->keys.identity_sk)); From f64689f3f00d72033ff7544ada4ccdfb7c328b36 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 12 Jul 2017 13:41:33 -0400 Subject: [PATCH 19/22] prop224: Don't use char * for binary data It turns out that some char * sneaked in our hs_common.c code. Replace those by uint8_t *. Signed-off-by: David Goulet --- src/or/hs_common.c | 18 ++++++++++-------- src/or/hs_common.h | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 87c29d5819..22a845f911 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -367,7 +367,7 @@ rend_data_get_pk_digest(const rend_data_t *rend_data, size_t *len_out) * checksum_out must be large enough to receive 32 bytes (DIGEST256_LEN). */ static void build_hs_checksum(const ed25519_public_key_t *key, uint8_t version, - char *checksum_out) + uint8_t *checksum_out) { size_t offset = 0; char data[HS_SERVICE_ADDR_CHECKSUM_INPUT_LEN]; @@ -383,7 +383,8 @@ build_hs_checksum(const ed25519_public_key_t *key, uint8_t version, tor_assert(offset == HS_SERVICE_ADDR_CHECKSUM_INPUT_LEN); /* Hash the data payload to create the checksum. */ - crypto_digest256(checksum_out, data, sizeof(data), DIGEST_SHA3_256); + crypto_digest256((char *) checksum_out, data, sizeof(data), + DIGEST_SHA3_256); } /* Using an ed25519 public key, checksum and version to build the binary @@ -392,7 +393,7 @@ build_hs_checksum(const ed25519_public_key_t *key, uint8_t version, * * addr_out must be large enough to receive HS_SERVICE_ADDR_LEN bytes. */ static void -build_hs_address(const ed25519_public_key_t *key, const char *checksum, +build_hs_address(const ed25519_public_key_t *key, const uint8_t *checksum, uint8_t version, char *addr_out) { size_t offset = 0; @@ -416,7 +417,7 @@ build_hs_address(const ed25519_public_key_t *key, const char *checksum, * HS_SERVICE_ADDR_LEN bytes but doesn't need to be NUL terminated. */ static void hs_parse_address_impl(const char *address, ed25519_public_key_t *key_out, - char *checksum_out, uint8_t *version_out) + uint8_t *checksum_out, uint8_t *version_out) { size_t offset = 0; @@ -449,7 +450,7 @@ hs_parse_address_impl(const char *address, ed25519_public_key_t *key_out, * Return 0 if parsing went well; return -1 in case of error. */ int hs_parse_address(const char *address, ed25519_public_key_t *key_out, - char *checksum_out, uint8_t *version_out) + uint8_t *checksum_out, uint8_t *version_out) { char decoded[HS_SERVICE_ADDR_LEN]; @@ -485,8 +486,8 @@ int hs_address_is_valid(const char *address) { uint8_t version; - char checksum[HS_SERVICE_ADDR_CHECKSUM_LEN_USED]; - char target_checksum[DIGEST256_LEN]; + uint8_t checksum[HS_SERVICE_ADDR_CHECKSUM_LEN_USED]; + uint8_t target_checksum[DIGEST256_LEN]; ed25519_public_key_t key; /* Parse the decoded address into the fields we need. */ @@ -521,7 +522,8 @@ void hs_build_address(const ed25519_public_key_t *key, uint8_t version, char *addr_out) { - char checksum[DIGEST256_LEN], address[HS_SERVICE_ADDR_LEN]; + uint8_t checksum[DIGEST256_LEN]; + char address[HS_SERVICE_ADDR_LEN]; tor_assert(key); tor_assert(addr_out); diff --git a/src/or/hs_common.h b/src/or/hs_common.h index 2b33914275..203a5d0818 100644 --- a/src/or/hs_common.h +++ b/src/or/hs_common.h @@ -93,7 +93,7 @@ void hs_build_address(const ed25519_public_key_t *key, uint8_t version, char *addr_out); int hs_address_is_valid(const char *address); int hs_parse_address(const char *address, ed25519_public_key_t *key_out, - char *checksum_out, uint8_t *version_out); + uint8_t *checksum_out, uint8_t *version_out); void rend_data_free(rend_data_t *data); rend_data_t *rend_data_dup(const rend_data_t *data); From 3eeebd1b0ca43bc2523fb39349078277a40d4116 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 12 Jul 2017 13:52:19 -0400 Subject: [PATCH 20/22] prop224: Use the service config object when configuring Both configuration function now takes the service config object instead of the service itself. Signed-off-by: David Goulet --- src/or/hs_config.c | 13 ++++--------- src/or/rendservice.c | 26 ++++++++++++-------------- src/or/rendservice.h | 2 +- 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/or/hs_config.c b/src/or/hs_config.c index c29315f6cf..7e0124bb34 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -236,18 +236,13 @@ config_validate_service(const hs_service_config_t *config) * Return 0 on success else a negative value. */ static int config_service_v3(const config_line_t *line_, - const or_options_t *options, - hs_service_t *service) + hs_service_config_t *config) { - (void) options; int have_num_ip = 0; const char *dup_opt_seen = NULL; const config_line_t *line; - hs_service_config_t *config; - tor_assert(service); - - config = &service->config; + tor_assert(config); for (line = line_; line; line = line->next) { int ok = 0; @@ -489,10 +484,10 @@ config_service(const config_line_t *line, const or_options_t *options, * directory line, the function knows that it has to stop parsing. */ switch (service->version) { case HS_VERSION_TWO: - ret = rend_config_service(line->next, options, service); + ret = rend_config_service(line->next, options, &service->config); break; case HS_VERSION_THREE: - ret = config_service_v3(line->next, options, service); + ret = config_service_v3(line->next, &service->config); break; default: /* We do validate before if we support the parsed version. */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 67da760069..67de636de4 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -622,14 +622,12 @@ rend_service_prune_list(void) * object which so we have to copy the parsed values to a rend service object * which is version 2 specific. */ static void -service_shadow_copy(rend_service_t *service, hs_service_t *hs_service) +service_config_shadow_copy(rend_service_t *service, + hs_service_config_t *config) { - hs_service_config_t *config; - tor_assert(service); - tor_assert(hs_service); + tor_assert(config); - config = &hs_service->config; service->directory = tor_strdup(config->directory_path); service->dir_group_readable = config->dir_group_readable; service->allow_unknown_ports = config->allow_unknown_ports; @@ -638,19 +636,19 @@ service_shadow_copy(rend_service_t *service, hs_service_t *hs_service) service->n_intro_points_wanted = config->num_intro_points; /* Switching ownership of the ports to the rend service object. */ smartlist_add_all(service->ports, config->ports); - smartlist_free(hs_service->config.ports); - hs_service->config.ports = NULL; + smartlist_free(config->ports); + config->ports = NULL; } /* Parse the hidden service configuration starting at line_ using the - * already configured generic service in hs_service. This function will - * translate the service object to a rend_service_t and add it to the - * temporary list if valid. If validate_only is set, parse, warn and - * return as normal but don't actually add the service to the list. */ + * already configured generic service configuration in config. This + * function will translate the config object to a rend_service_t and add it to + * the temporary list if valid. If validate_only is set, parse, warn + * and return as normal but don't actually add the service to the list. */ int rend_config_service(const config_line_t *line_, const or_options_t *options, - hs_service_t *hs_service) + hs_service_config_t *config) { const config_line_t *line; rend_service_t *service = NULL; @@ -658,7 +656,7 @@ rend_config_service(const config_line_t *line_, /* line_ can be NULL which would mean that the service configuration only * have one line that is the directory directive. */ tor_assert(options); - tor_assert(hs_service); + tor_assert(config); /* Use the staging service list so that we can check then do the pruning * process using the main list at the end. */ @@ -672,7 +670,7 @@ rend_config_service(const config_line_t *line_, service->ports = smartlist_new(); /* From the hs_service object which has been used to load the generic * options, we'll copy over the useful data to the rend_service_t object. */ - service_shadow_copy(service, hs_service); + service_config_shadow_copy(service, config); for (line = line_; line; line = line->next) { if (!strcasecmp(line->key, "HiddenServiceDir")) { diff --git a/src/or/rendservice.h b/src/or/rendservice.h index 20e827d2aa..ffed21d14e 100644 --- a/src/or/rendservice.h +++ b/src/or/rendservice.h @@ -143,7 +143,7 @@ STATIC void rend_service_prune_list_impl_(void); int num_rend_services(void); int rend_config_service(const config_line_t *line_, const or_options_t *options, - hs_service_t *hs_service); + hs_service_config_t *config); void rend_service_prune_list(void); void rend_service_free_staging_list(void); int rend_service_load_all_keys(const smartlist_t *service_list); From 5d64ceb12defdc7db8402088fb2946c35274636a Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 13 Jul 2017 08:51:14 -0400 Subject: [PATCH 21/22] prop224: Move service version into config object It makes more sense to have the version in the configuration object of the service because it is afterall a torrc option (HiddenServiceVersion). Signed-off-by: David Goulet --- src/or/hs_config.c | 15 ++++++++------- src/or/hs_service.c | 9 +++++---- src/or/hs_service.h | 7 ++++--- src/test/test_hs_service.c | 2 +- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/or/hs_config.c b/src/or/hs_config.c index 7e0124bb34..5f9282ea79 100644 --- a/src/or/hs_config.c +++ b/src/or/hs_config.c @@ -55,7 +55,7 @@ stage_services(smartlist_t *service_list) * only >= v3 service. And remember, v2 has a different object type which is * shadow copied from an hs_service_t type. */ SMARTLIST_FOREACH_BEGIN(service_list, hs_service_t *, s) { - if (s->version == HS_VERSION_TWO) { + if (s->config.version == HS_VERSION_TWO) { SMARTLIST_DEL_CURRENT(service_list, s); hs_service_free(s); } @@ -157,7 +157,7 @@ config_has_invalid_options(const config_line_t *line_, const config_line_t *line; tor_assert(service); - tor_assert(service->version <= HS_VERSION_MAX); + tor_assert(service->config.version <= HS_VERSION_MAX); /* List of options that a v3 service doesn't support thus must exclude from * its configuration. */ @@ -178,7 +178,7 @@ config_has_invalid_options(const config_line_t *line_, { opts_exclude_v3 }, /* v3. */ }; - optlist = exclude_lists[service->version].list; + optlist = exclude_lists[service->config.version].list; if (optlist == NULL) { /* No exclude options to look at for this version. */ goto end; @@ -193,7 +193,8 @@ config_has_invalid_options(const config_line_t *line_, if (!strcasecmp(line->key, opt)) { log_warn(LD_CONFIG, "Hidden service option %s is incompatible with " "version %" PRIu32 " of service in %s", - opt, service->version, service->config.directory_path); + opt, service->config.version, + service->config.directory_path); ret = 1; /* Continue the loop so we can find all possible options. */ continue; @@ -342,7 +343,7 @@ config_generic_service(const config_line_t *line_, } /* Version of the service. */ if (!strcasecmp(line->key, "HiddenServiceVersion")) { - service->version = + service->config.version = (uint32_t) helper_parse_uint64(line->key, line->value, HS_VERSION_MIN, HS_VERSION_MAX, &ok); if (!ok || have_version) { @@ -462,7 +463,7 @@ config_service(const config_line_t *line, const or_options_t *options, if (config_generic_service(line, options, service) < 0) { goto err; } - tor_assert(service->version <= HS_VERSION_MAX); + tor_assert(service->config.version <= HS_VERSION_MAX); /* Before we configure the service on a per-version basis, we'll make * sure that this set of options for a service are valid that is for * instance an option only for v2 is not used for v3. */ @@ -482,7 +483,7 @@ config_service(const config_line_t *line, const or_options_t *options, /* Different functions are in charge of specific options for a version. We * start just after the service directory line so once we hit another * directory line, the function knows that it has to stop parsing. */ - switch (service->version) { + switch (service->config.version) { case HS_VERSION_TWO: ret = rend_config_service(line->next, options, &service->config); break; diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 97d3288d17..d8b87d1f2c 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -250,7 +250,7 @@ close_service_circuits(hs_service_t *service) tor_assert(service); /* Only support for version >= 3. */ - if (BUG(service->version < HS_VERSION_THREE)) { + if (BUG(service->config.version < HS_VERSION_THREE)) { return; } /* Close intro points. */ @@ -492,8 +492,9 @@ load_service_keys(hs_service_t *service) ed25519_keypair_free(kp); /* Build onion address from the newly loaded keys. */ - tor_assert(service->version <= UINT8_MAX); - hs_build_address(&service->keys.identity_pk, (uint8_t) service->version, + tor_assert(service->config.version <= UINT8_MAX); + hs_build_address(&service->keys.identity_pk, + (uint8_t) service->config.version, service->onion_address); /* Write onion address to hostname file. */ @@ -572,7 +573,7 @@ hs_service_new(const or_options_t *options) /* Set default configuration value. */ set_service_default_config(&service->config, options); /* Set the default service version. */ - service->version = HS_SERVICE_DEFAULT_VERSION; + service->config.version = HS_SERVICE_DEFAULT_VERSION; return service; } diff --git a/src/or/hs_service.h b/src/or/hs_service.h index a98884f6bc..54b9e69724 100644 --- a/src/or/hs_service.h +++ b/src/or/hs_service.h @@ -112,6 +112,10 @@ typedef struct hs_service_keys_t { * set by the configuration file or by the control port. Nothing else should * change those values. */ typedef struct hs_service_config_t { + /* Protocol version of the service. Specified by HiddenServiceVersion + * option. */ + uint32_t version; + /* List of rend_service_port_config_t */ smartlist_t *ports; @@ -170,9 +174,6 @@ typedef struct hs_service_state_t { /* Representation of a service running on this tor instance. */ typedef struct hs_service_t { - /* Protocol version of the service. Specified by HiddenServiceVersion. */ - uint32_t version; - /* Onion address base32 encoded and NUL terminated. We keep it for logging * purposes so we don't have to build it everytime. */ char onion_address[HS_SERVICE_ADDR_LEN_BASE32 + 1]; diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index c695b90bc9..24fca08a14 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -471,7 +471,7 @@ test_load_keys(void *arg) tt_assert(!tor_mem_is_zero((char *) s->keys.identity_pk.pubkey, ED25519_PUBKEY_LEN)); /* Check onion address from identity key. */ - hs_build_address(&s->keys.identity_pk, s->version, addr); + hs_build_address(&s->keys.identity_pk, s->config.version, addr); tt_int_op(hs_address_is_valid(addr), OP_EQ, 1); tt_str_op(addr, OP_EQ, s->onion_address); From 965e3a6628f26d5fb1422fb04aa12e807537a32a Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 13 Jul 2017 17:18:11 -0400 Subject: [PATCH 22/22] prop224: Fix clang warnings Signed-off-by: David Goulet --- src/or/hs_service.c | 3 ++- src/or/rendservice.c | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/or/hs_service.c b/src/or/hs_service.c index d8b87d1f2c..5fde42ddbb 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -80,9 +80,10 @@ HT_GENERATE2(hs_service_ht, hs_service_t, hs_service_node, STATIC hs_service_t * find_service(hs_service_ht *map, const ed25519_public_key_t *pk) { - hs_service_t dummy_service = {0}; + hs_service_t dummy_service; tor_assert(map); tor_assert(pk); + memset(&dummy_service, 0, sizeof(dummy_service)); ed25519_pubkey_copy(&dummy_service.keys.identity_pk, pk); return HT_FIND(hs_service_ht, map, &dummy_service); } diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 67de636de4..2a5d2b775c 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -18,6 +18,7 @@ #include "control.h" #include "directory.h" #include "hs_common.h" +#include "hs_config.h" #include "main.h" #include "networkstatus.h" #include "nodelist.h" @@ -631,7 +632,14 @@ service_config_shadow_copy(rend_service_t *service, service->directory = tor_strdup(config->directory_path); service->dir_group_readable = config->dir_group_readable; service->allow_unknown_ports = config->allow_unknown_ports; - service->max_streams_per_circuit = config->max_streams_per_rdv_circuit; + /* This value can't go above HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT (65535) + * if the code flow is right so this cast is safe. But just in case, we'll + * check it. */ + service->max_streams_per_circuit = (int) config->max_streams_per_rdv_circuit; + if (BUG(config->max_streams_per_rdv_circuit > + HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT)) { + service->max_streams_per_circuit = HS_CONFIG_MAX_STREAMS_PER_RDV_CIRCUIT; + } service->max_streams_close_circuit = config->max_streams_close_circuit; service->n_intro_points_wanted = config->num_intro_points; /* Switching ownership of the ports to the rend service object. */