July 2017 - tor-commits - lists.torproject.org

[tor/master] prop224: Add a function to check for invalid opts
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit f76f8731995917366b53f729befd450ed3d417d1 Author: David Goulet <dgoulet(a)torproject.org> Date: Fri Jul 7 15:34:36 2017 -0400 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 <dgoulet(a)torproject.org> --- 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 6bb422d..afc9631 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

1 0

[tor/master] prop224: Service address creation/validation
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit f3899acdbfe121521cbd8cc76983b1e1e149d38c Author: David Goulet <dgoulet(a)torproject.org> Date: Mon Jan 30 17:33:18 2017 -0500 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 <dgoulet(a)torproject.org> --- 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(+) diff --git a/src/or/hs_common.c b/src/or/hs_common.c index b524296..00befab 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 d1bc5ac..64bf89f 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 0000000..7be9c8b --- /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 17772f1..e081b7f 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 };

1 0

[tor/master] prop224: Add a clear configuration function
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit 1b048fbfaadec04f409ab8d120106025b009fec8 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jul 12 11:53:26 2017 -0400 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 <dgoulet(a)torproject.org> --- 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 bfce780..97d3288 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));

1 0

[tor/master] Merge remote-tracking branch 'dgoulet/ticket21979_032_04'
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit ef4ea864ea66059f7fd71ac68cfe6067f32149f1 Merge: 62d241a 965e3a6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 13 17:23:37 2017 -0400 Merge remote-tracking branch 'dgoulet/ticket21979_032_04' src/or/circuitlist.h | 1 + src/or/config.c | 7 +- src/or/hs_common.c | 216 ++++++++++++++ src/or/hs_common.h | 38 ++- src/or/hs_config.c | 582 +++++++++++++++++++++++++++++++++++++ src/or/hs_config.h | 24 ++ src/or/hs_descriptor.c | 6 +- src/or/hs_intropoint.h | 12 + src/or/hs_service.c | 665 ++++++++++++++++++++++++++++++++++++++++++- src/or/hs_service.h | 231 ++++++++++++++- src/or/include.am | 24 +- src/or/main.c | 10 +- src/or/rendservice.c | 356 ++++++++--------------- src/or/rendservice.h | 11 +- src/test/hs_build_address.py | 37 +++ 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.c | 22 +- src/test/test_hs_config.c | 477 +++++++++++++++++++++++++++++++ src/test/test_hs_service.c | 249 ++++++++++++++++ 23 files changed, 2730 insertions(+), 284 deletions(-)

1 0

[tor/master] prop224: Don't use char * for binary data
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit f64689f3f00d72033ff7544ada4ccdfb7c328b36 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jul 12 13:41:33 2017 -0400 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 <dgoulet(a)torproject.org> --- 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 87c29d5..22a845f 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 2b33914..203a5d0 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);

1 0

[tor/master] prop224: Don't use an array of config handlers
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit 750c684fff02fde3054261abbbdcc6a458bea8e0 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jul 12 11:17:26 2017 -0400 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 <dgoulet(a)torproject.org> --- 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 2e75a4e..c29315f 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(); }

1 0

[tor/master] prop224: Use a common function to parse uint64_t
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit cfa6f8358b4c5ea6c9c82a0818dc81b8aaf44a78 Author: David Goulet <dgoulet(a)torproject.org> Date: Wed Jul 12 10:37:10 2017 -0400 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 <dgoulet(a)torproject.org> --- 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 3baab46..f1e130b 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; } }

1 0

[tor/master] Use the return value for choosing intervals
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit f516c9ca99e4c3ef1bfeb3693e622befdd7954d2 Author: cypherpunks <cypherpunks(a)torproject.org> Date: Tue Jun 21 13:03:38 2016 +0000 Use the return value for choosing intervals --- src/or/main.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 5fa3869..4116740 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2071,14 +2071,19 @@ heartbeat_callback(time_t now, const or_options_t *options) return PERIODIC_EVENT_NO_UPDATE; } - /* Write the heartbeat message */ + /* Skip the first one. */ if (first) { - first = 0; /* Skip the first one. */ - } else { - log_heartbeat(now); + first = 0; + return options->HeartbeatPeriod; } - return options->HeartbeatPeriod; + /* Write the heartbeat message */ + int r = log_heartbeat(now); + if (r == 0) { + return options->HeartbeatPeriod; + } + + return PERIODIC_EVENT_NO_UPDATE; } #define CDM_CLEAN_CALLBACK_INTERVAL 600

1 0

[tor/master] Add changes file for #19476.
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit 175451d032026f1e1cbf45dfa55a408d5d4abeed Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed Jul 12 02:47:43 2017 +0000 Add changes file for #19476. --- changes/bug19476 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changes/bug19476 b/changes/bug19476 new file mode 100644 index 0000000..25a0578 --- /dev/null +++ b/changes/bug19476 @@ -0,0 +1,3 @@ + o Minor changes: + - If we fail to write a heartbeat message, schedule a retry for the minimum + heartbeat interval number of seconds in the future. Fixes bug 19476.

1 0

[tor/master] If writing a heartbeat message fails, retry after MIN_HEARTBEAT_PERIOD.
by nickm＠torproject.org 13 Jul '17

13 Jul '17

commit 9de12397cf4882e83f199616d52ec9c90e0d6d8e Author: Isis Lovecruft <isis(a)torproject.org> Date: Wed Jul 12 02:40:24 2017 +0000 If writing a heartbeat message fails, retry after MIN_HEARTBEAT_PERIOD. * FIXES #19476. --- src/or/config.c | 4 ---- src/or/config.h | 4 ++++ src/or/main.c | 12 ++++++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 5b5bb90..5da871d 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -2804,10 +2804,6 @@ compute_publishserverdescriptor(or_options_t *options) * will generate too many circuits and potentially overload the network. */ #define MIN_CIRCUIT_STREAM_TIMEOUT 10 -/** Lowest allowable value for HeartbeatPeriod; if this is too low, we might - * expose more information than we're comfortable with. */ -#define MIN_HEARTBEAT_PERIOD (30*60) - /** Lowest recommended value for CircuitBuildTimeout; if it is set too low * and LearnCircuitBuildTimeout is off, the failure rate for circuit * construction may be very high. In that case, if it is set below this diff --git a/src/or/config.h b/src/or/config.h index 27aec7f..3cfa7c4 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -18,6 +18,10 @@ #define KERNEL_MAY_SUPPORT_IPFW #endif +/** Lowest allowable value for HeartbeatPeriod; if this is too low, we might + * expose more information than we're comfortable with. */ +#define MIN_HEARTBEAT_PERIOD (30*60) + MOCK_DECL(const char*, get_dirportfrontpage, (void)); MOCK_DECL(const or_options_t *, get_options, (void)); MOCK_DECL(or_options_t *, get_options_mutable, (void)); diff --git a/src/or/main.c b/src/or/main.c index 4116740..f7b9cc8 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2060,6 +2060,9 @@ check_fw_helper_app_callback(time_t now, const or_options_t *options) /** * Periodic callback: write the heartbeat message in the logs. + * + * If writing the heartbeat message to the logs fails for some reason, retry + * again after <b>MIN_HEARTBEAT_PERIOD</b> seconds. */ static int heartbeat_callback(time_t now, const or_options_t *options) @@ -2078,12 +2081,13 @@ heartbeat_callback(time_t now, const or_options_t *options) } /* Write the heartbeat message */ - int r = log_heartbeat(now); - if (r == 0) { + if (log_heartbeat(now) == 0) { return options->HeartbeatPeriod; + } else { + /* If we couldn't write the heartbeat log message, try again in the minimum + * interval of time. */ + return MIN_HEARTBEAT_PERIOD; } - - return PERIODIC_EVENT_NO_UPDATE; } #define CDM_CLEAN_CALLBACK_INTERVAL 600

1 0