February 2020 - tor-commits - lists.torproject.org

[tor/master] practracker: Make it happy
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit 3484608bda4d8c329ad886ddf98087d775c43a72 Author: David Goulet <dgoulet(a)torproject.org> Date: Tue Jan 14 12:04:39 2020 -0500 practracker: Make it happy Signed-off-by: David Goulet <dgoulet(a)torproject.org> --- scripts/maint/practracker/exceptions.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index 70e6a5519..09c496c6d 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -250,7 +250,7 @@ problem function-size /src/feature/hs/hs_cell.c:hs_cell_parse_introduce2() 152 problem function-size /src/feature/hs/hs_client.c:send_introduce1() 103 problem function-size /src/feature/hs/hs_client.c:hs_config_client_authorization() 107 problem function-size /src/feature/hs/hs_common.c:hs_get_responsible_hsdirs() 102 -problem function-size /src/feature/hs/hs_config.c:config_service_v3() 107 +problem function-size /src/feature/hs/hs_config.c:config_service_v3() 128 problem function-size /src/feature/hs/hs_config.c:config_generic_service() 138 problem function-size /src/feature/hs/hs_descriptor.c:desc_encode_v3() 101 problem function-size /src/feature/hs/hs_descriptor.c:decrypt_desc_layer() 111

1 0

[tor/master] Turn hs_subcredential_t into a proper struct.
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit 4532c7ef6aa96d502412cbc61da91369bc3eaa44 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 16 18:27:25 2020 -0500 Turn hs_subcredential_t into a proper struct. --- src/core/crypto/hs_ntor.c | 8 ++++---- src/core/crypto/hs_ntor.h | 13 +++++++++++-- src/feature/hs/hs_cell.c | 6 +++--- src/feature/hs/hs_cell.h | 6 ++++-- src/feature/hs/hs_circuit.c | 6 +++--- src/feature/hs/hs_circuit.h | 5 +++-- src/feature/hs/hs_client.c | 12 ++++++------ src/feature/hs/hs_common.c | 7 ++++--- src/feature/hs/hs_common.h | 3 ++- src/feature/hs/hs_descriptor.c | 24 +++++++++++++----------- src/feature/hs/hs_descriptor.h | 7 ++++--- src/feature/hs/hs_ob.c | 12 +++++------- src/feature/hs/hs_ob.h | 3 ++- src/feature/hs/hs_service.c | 9 +++++---- src/test/fuzz/fuzz_hsdescv3.c | 7 +++---- src/test/hs_test_helpers.c | 6 +++--- src/test/hs_test_helpers.h | 7 +++---- src/test/test_hs_cache.c | 21 ++++++++++++--------- src/test/test_hs_client.c | 7 ++++--- src/test/test_hs_descriptor.c | 28 ++++++++++++++-------------- src/test/test_hs_ntor.c | 8 ++++---- src/test/test_hs_ntor_cl.c | 16 ++++++++-------- src/test/test_hs_ob.c | 11 +++++------ 23 files changed, 125 insertions(+), 107 deletions(-) diff --git a/src/core/crypto/hs_ntor.c b/src/core/crypto/hs_ntor.c index 2bd4c3244..0422e7279 100644 --- a/src/core/crypto/hs_ntor.c +++ b/src/core/crypto/hs_ntor.c @@ -170,7 +170,7 @@ get_rendezvous1_key_material(const uint8_t *rend_secret_hs_input, * necessary key material, and return 0. */ static void get_introduce1_key_material(const uint8_t *secret_input, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { uint8_t keystream[CIPHER256_KEY_LEN + DIGEST256_LEN]; @@ -181,7 +181,7 @@ get_introduce1_key_material(const uint8_t *secret_input, /* Let's build info */ ptr = info_blob; APPEND(ptr, M_HSEXPAND, strlen(M_HSEXPAND)); - APPEND(ptr, subcredential, DIGEST256_LEN); + APPEND(ptr, subcredential->subcred, SUBCRED_LEN); tor_assert(ptr == info_blob + sizeof(info_blob)); /* Let's build the input to the KDF */ @@ -317,7 +317,7 @@ hs_ntor_client_get_introduce1_keys( const ed25519_public_key_t *intro_auth_pubkey, const curve25519_public_key_t *intro_enc_pubkey, const curve25519_keypair_t *client_ephemeral_enc_keypair, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { int bad = 0; @@ -450,7 +450,7 @@ hs_ntor_service_get_introduce1_keys( const ed25519_public_key_t *intro_auth_pubkey, const curve25519_keypair_t *intro_enc_keypair, const curve25519_public_key_t *client_ephemeral_enc_pubkey, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { int bad = 0; diff --git a/src/core/crypto/hs_ntor.h b/src/core/crypto/hs_ntor.h index 2bce5686c..b78bc4e80 100644 --- a/src/core/crypto/hs_ntor.h +++ b/src/core/crypto/hs_ntor.h @@ -35,11 +35,20 @@ typedef struct hs_ntor_rend_cell_keys_t { uint8_t ntor_key_seed[DIGEST256_LEN]; } hs_ntor_rend_cell_keys_t; +#define SUBCRED_LEN DIGEST256_LEN + +/** + * A 'subcredential' used to prove knowledge of a hidden service. + **/ +typedef struct hs_subcredential_t { + uint8_t subcred[SUBCRED_LEN]; +} hs_subcredential_t; + int hs_ntor_client_get_introduce1_keys( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_public_key_t *intro_enc_pubkey, const struct curve25519_keypair_t *client_ephemeral_enc_keypair, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out); int hs_ntor_client_get_rendezvous1_keys( @@ -53,7 +62,7 @@ int hs_ntor_service_get_introduce1_keys( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_keypair_t *intro_enc_keypair, const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out); int hs_ntor_service_get_rendezvous1_keys( diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index aabc55859..d377e9134 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -75,7 +75,7 @@ compute_introduce_mac(const uint8_t *encoded_cell, size_t encoded_cell_len, static hs_ntor_intro_cell_keys_t * get_introduce2_key_material(const ed25519_public_key_t *auth_key, const curve25519_keypair_t *enc_key, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const uint8_t *encrypted_section, curve25519_public_key_t *client_pk) { @@ -820,7 +820,7 @@ get_intro2_keys_as_ob(const hs_service_config_t *config, const uint8_t *encrypted_section, size_t encrypted_section_len) { - uint8_t *ob_subcreds = NULL; + hs_subcredential_t *ob_subcreds = NULL; size_t ob_num_subcreds; hs_ntor_intro_cell_keys_t *intro_keys = NULL; @@ -835,7 +835,7 @@ get_intro2_keys_as_ob(const hs_service_config_t *config, /* Copy current data into a new INTRO2 cell data. We will then change the * subcredential in order to validate. */ hs_cell_introduce2_data_t new_data = *data; - new_data.subcredential = &(ob_subcreds[idx * DIGEST256_LEN]); + new_data.subcredential = &ob_subcreds[idx]; intro_keys = get_introduce2_keys_and_verify_mac(&new_data, encrypted_section, encrypted_section_len); diff --git a/src/feature/hs/hs_cell.h b/src/feature/hs/hs_cell.h index 80f37057d..58cc401cc 100644 --- a/src/feature/hs/hs_cell.h +++ b/src/feature/hs/hs_cell.h @@ -16,6 +16,8 @@ * 3.2.2 of the specification). Below this value, the cell must be padded. */ #define HS_CELL_INTRODUCE1_MIN_SIZE 246 +struct hs_subcredential_t; + /** This data structure contains data that we need to build an INTRODUCE1 cell * used by the INTRODUCE1 build function. */ typedef struct hs_cell_introduce1_data_t { @@ -29,7 +31,7 @@ typedef struct hs_cell_introduce1_data_t { /** Introduction point encryption public key. */ const curve25519_public_key_t *enc_pk; /** Subcredentials of the service. */ - const uint8_t *subcredential; + const struct hs_subcredential_t *subcredential; /** Onion public key for the ntor handshake. */ const curve25519_public_key_t *onion_pk; /** Rendezvous cookie. */ @@ -57,7 +59,7 @@ typedef struct hs_cell_introduce2_data_t { const curve25519_keypair_t *enc_kp; /** Subcredentials of the service. Pointer owned by the descriptor that owns the introduction point through which we received the INTRO2 cell. */ - const uint8_t *subcredential; + const struct hs_subcredential_t *subcredential; /** Payload of the received encoded cell. */ const uint8_t *payload; /** Size of the payload of the received encoded cell. */ diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 90805a98b..fb3694b2d 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -578,7 +578,7 @@ retry_service_rendezvous_point(const origin_circuit_t *circ) static int setup_introduce1_data(const hs_desc_intro_point_t *ip, const node_t *rp_node, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, hs_cell_introduce1_data_t *intro1_data) { int ret = -1; @@ -966,7 +966,7 @@ int hs_circ_handle_introduce2(const hs_service_t *service, const origin_circuit_t *circ, hs_service_intro_point_t *ip, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const uint8_t *payload, size_t payload_len) { int ret = -1; @@ -1092,7 +1092,7 @@ int hs_circ_send_introduce1(origin_circuit_t *intro_circ, origin_circuit_t *rend_circ, const hs_desc_intro_point_t *ip, - const uint8_t *subcredential) + const hs_subcredential_t *subcredential) { int ret = -1; ssize_t payload_len; diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h index 92231369c..5c20e3818 100644 --- a/src/feature/hs/hs_circuit.h +++ b/src/feature/hs/hs_circuit.h @@ -46,15 +46,16 @@ int hs_circ_handle_intro_established(const hs_service_t *service, origin_circuit_t *circ, const uint8_t *payload, size_t payload_len); +struct hs_subcredential_t; int hs_circ_handle_introduce2(const hs_service_t *service, const origin_circuit_t *circ, hs_service_intro_point_t *ip, - const uint8_t *subcredential, + const struct hs_subcredential_t *subcredential, const uint8_t *payload, size_t payload_len); int hs_circ_send_introduce1(origin_circuit_t *intro_circ, origin_circuit_t *rend_circ, const hs_desc_intro_point_t *ip, - const uint8_t *subcredential); + const struct hs_subcredential_t *subcredential); int hs_circ_send_establish_rendezvous(origin_circuit_t *circ); /* e2e circuit API. */ diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index bcb0495c6..601062190 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -641,7 +641,7 @@ send_introduce1(origin_circuit_t *intro_circ, /* Send the INTRODUCE1 cell. */ if (hs_circ_send_introduce1(intro_circ, rend_circ, ip, - desc->subcredential) < 0) { + &desc->subcredential) < 0) { if (TO_CIRCUIT(intro_circ)->marked_for_close) { /* If the introduction circuit was closed, we were unable to send the * cell for some reasons. In any case, the intro circuit has to be @@ -1817,7 +1817,7 @@ hs_client_decode_descriptor(const char *desc_str, hs_descriptor_t **desc) { hs_desc_decode_status_t ret; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; ed25519_public_key_t blinded_pubkey; hs_client_service_authorization_t *client_auth = NULL; curve25519_secret_key_t *client_auht_sk = NULL; @@ -1837,13 +1837,14 @@ hs_client_decode_descriptor(const char *desc_str, uint64_t current_time_period = hs_get_time_period_num(0); hs_build_blinded_pubkey(service_identity_pk, NULL, 0, current_time_period, &blinded_pubkey); - hs_get_subcredential(service_identity_pk, &blinded_pubkey, subcredential); + hs_get_subcredential(service_identity_pk, &blinded_pubkey, & + subcredential); } /* Parse descriptor */ - ret = hs_desc_decode_descriptor(desc_str, subcredential, + ret = hs_desc_decode_descriptor(desc_str, &subcredential, client_auht_sk, desc); - memwipe(subcredential, 0, sizeof(subcredential)); + memwipe(&subcredential, 0, sizeof(subcredential)); if (ret != HS_DESC_DECODE_OK) { goto err; } @@ -2456,4 +2457,3 @@ set_hs_client_auths_map(digest256map_t *map) } #endif /* defined(TOR_UNIT_TESTS) */ - diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index 47594b582..b2b52c480 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -808,12 +808,12 @@ hs_parse_address_impl(const char *address, ed25519_public_key_t *key_out, } /** Using the given identity public key and a blinded public key, compute the - * subcredential and put it in subcred_out (must be of size DIGEST256_LEN). + * subcredential and put it in subcred_out. * This can't fail. */ void hs_get_subcredential(const ed25519_public_key_t *identity_pk, const ed25519_public_key_t *blinded_pk, - uint8_t *subcred_out) + hs_subcredential_t *subcred_out) { uint8_t credential[DIGEST256_LEN]; crypto_digest_t *digest; @@ -841,7 +841,8 @@ hs_get_subcredential(const ed25519_public_key_t *identity_pk, sizeof(credential)); crypto_digest_add_bytes(digest, (const char *) blinded_pk->pubkey, ED25519_PUBKEY_LEN); - crypto_digest_get_digest(digest, (char *) subcred_out, DIGEST256_LEN); + crypto_digest_get_digest(digest, (char *) subcred_out->subcred, + SUBCRED_LEN); crypto_digest_free(digest); memwipe(credential, 0, sizeof(credential)); diff --git a/src/feature/hs/hs_common.h b/src/feature/hs/hs_common.h index 2bcf0f67c..997b7298a 100644 --- a/src/feature/hs/hs_common.h +++ b/src/feature/hs/hs_common.h @@ -214,9 +214,10 @@ const uint8_t *rend_data_get_pk_digest(const rend_data_t *rend_data, routerstatus_t *pick_hsdir(const char *desc_id, const char *desc_id_base32); +struct hs_subcredential_t; void hs_get_subcredential(const struct ed25519_public_key_t *identity_pk, const struct ed25519_public_key_t *blinded_pk, - uint8_t *subcred_out); + struct hs_subcredential_t *subcred_out); uint64_t hs_get_previous_time_period_num(time_t now); uint64_t hs_get_time_period_num(time_t now); diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index 65d6c7a58..7f28e1c34 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -211,7 +211,7 @@ build_secret_input(const hs_descriptor_t *desc, memcpy(secret_input, secret_data, secret_data_len); offset += secret_data_len; /* Copy subcredential. */ - memcpy(secret_input + offset, desc->subcredential, DIGEST256_LEN); + memcpy(secret_input + offset, desc->subcredential.subcred, DIGEST256_LEN); offset += DIGEST256_LEN; /* Copy revision counter value. */ set_uint64(secret_input + offset, @@ -1018,9 +1018,11 @@ desc_encode_v3(const hs_descriptor_t *desc, tor_assert(encoded_out); tor_assert(desc->plaintext_data.version == 3); + /* This is impossible; this is a member of desc. if (BUG(desc->subcredential == NULL)) { goto err; } + */ /* Build the non-encrypted values. */ { @@ -1366,8 +1368,7 @@ encrypted_data_length_is_valid(size_t len) * and return the buffer's length. The caller should wipe and free its content * once done with it. This function can't fail. */ static size_t -build_descriptor_cookie_keys(const uint8_t *subcredential, - size_t subcredential_len, +build_descriptor_cookie_keys(const hs_subcredential_t *subcredential, const curve25519_secret_key_t *sk, const curve25519_public_key_t *pk, uint8_t **keys_out) @@ -1389,7 +1390,7 @@ build_descriptor_cookie_keys(const uint8_t *subcredential, /* Calculate KEYS = KDF(subcredential | SECRET_SEED, 40) */ xof = crypto_xof_new(); - crypto_xof_add_bytes(xof, subcredential, subcredential_len); + crypto_xof_add_bytes(xof, subcredential->subcred, SUBCRED_LEN); crypto_xof_add_bytes(xof, secret_seed, sizeof(secret_seed)); crypto_xof_squeeze_bytes(xof, keystream, keystream_len); crypto_xof_free(xof); @@ -1426,11 +1427,12 @@ decrypt_descriptor_cookie(const hs_descriptor_t *desc, sizeof(desc->superencrypted_data.auth_ephemeral_pubkey))); tor_assert(!fast_mem_is_zero((char *) client_auth_sk, sizeof(*client_auth_sk))); - tor_assert(!fast_mem_is_zero((char *) desc->subcredential, DIGEST256_LEN)); + tor_assert(!fast_mem_is_zero((char *) desc->subcredential.subcred, + DIGEST256_LEN)); /* Get the KEYS component to derive the CLIENT-ID and COOKIE-KEY. */ keystream_length = - build_descriptor_cookie_keys(desc->subcredential, DIGEST256_LEN, + build_descriptor_cookie_keys(&desc->subcredential, client_auth_sk, &desc->superencrypted_data.auth_ephemeral_pubkey, &keystream); @@ -2558,7 +2560,7 @@ hs_desc_decode_plaintext(const char *encoded, * set to NULL. */ hs_desc_decode_status_t hs_desc_decode_descriptor(const char *encoded, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const curve25519_secret_key_t *client_auth_sk, hs_descriptor_t **desc_out) { @@ -2576,7 +2578,7 @@ hs_desc_decode_descriptor(const char *encoded, goto err; } - memcpy(desc->subcredential, subcredential, sizeof(desc->subcredential)); + memcpy(&desc->subcredential, subcredential, sizeof(desc->subcredential)); ret = hs_desc_decode_plaintext(encoded, &desc->plaintext_data); if (ret != HS_DESC_DECODE_OK) { @@ -2666,7 +2668,7 @@ hs_desc_encode_descriptor,(const hs_descriptor_t *desc, * symmetric only if the client auth is disabled. That is, the descriptor * cookie will be NULL. */ if (!descriptor_cookie) { - ret = hs_desc_decode_descriptor(*encoded_out, desc->subcredential, + ret = hs_desc_decode_descriptor(*encoded_out, &desc->subcredential, NULL, NULL); if (BUG(ret != HS_DESC_DECODE_OK)) { ret = -1; @@ -2870,7 +2872,7 @@ hs_desc_build_fake_authorized_client(void) * key, and descriptor cookie, build the auth client so we can then encode the * descriptor for publication. client_out must be already allocated. */ void -hs_desc_build_authorized_client(const uint8_t *subcredential, +hs_desc_build_authorized_client(const hs_subcredential_t *subcredential, const curve25519_public_key_t *client_auth_pk, const curve25519_secret_key_t * auth_ephemeral_sk, @@ -2898,7 +2900,7 @@ hs_desc_build_authorized_client(const uint8_t *subcredential, /* Get the KEYS part so we can derive the CLIENT-ID and COOKIE-KEY. */ keystream_length = - build_descriptor_cookie_keys(subcredential, DIGEST256_LEN, + build_descriptor_cookie_keys(subcredential, auth_ephemeral_sk, client_auth_pk, &keystream); tor_assert(keystream_length > 0); diff --git a/src/feature/hs/hs_descriptor.h b/src/feature/hs/hs_descriptor.h index 639dd31c8..08daa904b 100644 --- a/src/feature/hs/hs_descriptor.h +++ b/src/feature/hs/hs_descriptor.h @@ -14,6 +14,7 @@ #include "core/or/or.h" #include "trunnel/ed25519_cert.h" /* needed for trunnel */ #include "feature/nodelist/torcert.h" +#include "core/crypto/hs_ntor.h" /* for hs_subcredential_t */ /* Trunnel */ struct link_specifier_t; @@ -238,7 +239,7 @@ typedef struct hs_descriptor_t { /** Subcredentials of a service, used by the client and service to decrypt * the encrypted data. */ - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; } hs_descriptor_t; /** Return true iff the given descriptor format version is supported. */ @@ -277,7 +278,7 @@ MOCK_DECL(int, char **encoded_out)); int hs_desc_decode_descriptor(const char *encoded, - const uint8_t *subcredential, + const hs_subcredential_t *subcredential, const curve25519_secret_key_t *client_auth_sk, hs_descriptor_t **desc_out); int hs_desc_decode_plaintext(const char *encoded, @@ -302,7 +303,7 @@ void hs_desc_authorized_client_free_(hs_desc_authorized_client_t *client); hs_desc_authorized_client_t *hs_desc_build_fake_authorized_client(void); -void hs_desc_build_authorized_client(const uint8_t *subcredential, +void hs_desc_build_authorized_client(const hs_subcredential_t *subcredential, const curve25519_public_key_t * client_auth_pk, const curve25519_secret_key_t * diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 62db3bd43..ee54595f2 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -178,7 +178,7 @@ ob_option_parse(hs_service_config_t *config, const ob_options_t *opts) * least DIGEST256_LEN in size. */ static void build_subcredential(const ed25519_public_key_t *pkey, uint64_t tp, - uint8_t *subcredential) + hs_subcredential_t *subcredential) { ed25519_public_key_t blinded_pubkey; @@ -275,13 +275,12 @@ hs_ob_parse_config_file(hs_service_config_t *config) * Otherwise, this can't fail. */ size_t hs_ob_get_subcredentials(const hs_service_config_t *config, - uint8_t **subcredentials) + hs_subcredential_t **subcredentials) { unsigned int num_pkeys, idx = 0; - uint8_t *subcreds = NULL; + hs_subcredential_t *subcreds = NULL; const int steps[3] = {0, -1, 1}; const unsigned int num_steps = ARRAY_LENGTH(steps); - const size_t subcred_len = DIGEST256_LEN; const uint64_t tp = hs_get_time_period_num(0); tor_assert(config); @@ -319,14 +318,13 @@ hs_ob_get_subcredentials(const hs_service_config_t *config, * number of time period we need to compute and finally multiplied by the * total number of keys we are about to process. In other words, for each * key, we allocate 3 subcredential slots. */ - subcreds = tor_malloc_zero(subcred_len * num_steps * num_pkeys); + subcreds = tor_calloc(num_steps * num_pkeys, sizeof(hs_subcredential_t)); /* For each time period step. */ for (unsigned int i = 0; i < num_steps; i++) { SMARTLIST_FOREACH_BEGIN(config->ob_master_pubkeys, const ed25519_public_key_t *, pkey) { - build_subcredential(pkey, tp + steps[i], - &(subcreds[idx * subcred_len])); + build_subcredential(pkey, tp + steps[i], &subcreds[idx]); idx++; } SMARTLIST_FOREACH_END(pkey); } diff --git a/src/feature/hs/hs_ob.h b/src/feature/hs/hs_ob.h index fea6a737d..37e94808c 100644 --- a/src/feature/hs/hs_ob.h +++ b/src/feature/hs/hs_ob.h @@ -15,8 +15,9 @@ bool hs_ob_service_is_instance(const hs_service_t *service); int hs_ob_parse_config_file(hs_service_config_t *config); +struct hs_subcredential_t; size_t hs_ob_get_subcredentials(const hs_service_config_t *config, - uint8_t **subcredentials); + struct hs_subcredential_t **subcredentials); #ifdef HS_OB_PRIVATE diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index d0d9d6579..87b9625f5 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -1769,7 +1769,8 @@ build_service_desc_superencrypted(const hs_service_t *service, sizeof(curve25519_public_key_t)); /* Test that subcred is not zero because we might use it below */ - if (BUG(fast_mem_is_zero((char*)desc->desc->subcredential, DIGEST256_LEN))) { + if (BUG(fast_mem_is_zero((char*)desc->desc->subcredential.subcred, + DIGEST256_LEN))) { return -1; } @@ -1786,7 +1787,7 @@ build_service_desc_superencrypted(const hs_service_t *service, /* Prepare the client for descriptor and then add to the list in the * superencrypted part of the descriptor */ - hs_desc_build_authorized_client(desc->desc->subcredential, + hs_desc_build_authorized_client(&desc->desc->subcredential, &client->client_pk, &desc->auth_ephemeral_kp.seckey, desc->descriptor_cookie, desc_client); @@ -1842,7 +1843,7 @@ build_service_desc_plaintext(const hs_service_t *service, /* Set the subcredential. */ hs_get_subcredential(&service->keys.identity_pk, &desc->blinded_kp.pubkey, - desc->desc->subcredential); + &desc->desc->subcredential); plaintext = &desc->desc->plaintext_data; @@ -3374,7 +3375,7 @@ service_handle_introduce2(origin_circuit_t *circ, const uint8_t *payload, /* The following will parse, decode and launch the rendezvous point circuit. * Both current and legacy cells are handled. */ - if (hs_circ_handle_introduce2(service, circ, ip, desc->desc->subcredential, + if (hs_circ_handle_introduce2(service, circ, ip, &desc->desc->subcredential, payload, payload_len) < 0) { goto err; } diff --git a/src/test/fuzz/fuzz_hsdescv3.c b/src/test/fuzz/fuzz_hsdescv3.c index 395524138..8d7eab1a8 100644 --- a/src/test/fuzz/fuzz_hsdescv3.c +++ b/src/test/fuzz/fuzz_hsdescv3.c @@ -85,12 +85,12 @@ int fuzz_main(const uint8_t *data, size_t sz) { hs_descriptor_t *desc = NULL; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; char *fuzzing_data = tor_memdup_nulterm(data, sz); - memset(subcredential, 'A', sizeof(subcredential)); + memset(&subcredential, 'A', sizeof(subcredential)); - hs_desc_decode_descriptor(fuzzing_data, subcredential, NULL, &desc); + hs_desc_decode_descriptor(fuzzing_data, &subcredential, NULL, &desc); if (desc) { log_debug(LD_GENERAL, "Decoding okay"); hs_descriptor_free(desc); @@ -101,4 +101,3 @@ fuzz_main(const uint8_t *data, size_t sz) tor_free(fuzzing_data); return 0; } - diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index e8b99aaac..eb4af1c9f 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -140,7 +140,7 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, desc->plaintext_data.lifetime_sec = 3 * 60 * 60; hs_get_subcredential(&signing_kp->pubkey, &blinded_kp.pubkey, - desc->subcredential); + &desc->subcredential); /* Setup superencrypted data section. */ ret = curve25519_keypair_generate(&auth_ephemeral_kp, 0); @@ -186,7 +186,7 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, * an HS. Used to decrypt descriptors in unittests. */ void hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp, - uint8_t *subcred_out) + hs_subcredential_t *subcred_out) { ed25519_keypair_t blinded_kp; uint64_t current_time_period = hs_get_time_period_num(approx_time()); @@ -233,7 +233,7 @@ hs_helper_build_hs_desc_with_client_auth( memcpy(&desc->superencrypted_data.auth_ephemeral_pubkey, &auth_ephemeral_kp.pubkey, sizeof(curve25519_public_key_t)); - hs_desc_build_authorized_client(desc->subcredential, client_pk, + hs_desc_build_authorized_client(&desc->subcredential, client_pk, &auth_ephemeral_kp.seckey, descriptor_cookie, desc_client); smartlist_add(desc->superencrypted_data.clients, desc_client); diff --git a/src/test/hs_test_helpers.h b/src/test/hs_test_helpers.h index a01fd45d6..cad453282 100644 --- a/src/test/hs_test_helpers.h +++ b/src/test/hs_test_helpers.h @@ -21,12 +21,11 @@ hs_descriptor_t *hs_helper_build_hs_desc_with_client_auth( const ed25519_keypair_t *signing_kp); void hs_helper_desc_equal(const hs_descriptor_t *desc1, const hs_descriptor_t *desc2); -void -hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp, - uint8_t *subcred_out); +struct hs_subcredential_t; +void hs_helper_get_subcred_from_identity_keypair(ed25519_keypair_t *signing_kp, + struct hs_subcredential_t *subcred_out); void hs_helper_add_client_auth(const ed25519_public_key_t *service_pk, const curve25519_secret_key_t *client_sk); #endif /* !defined(TOR_HS_TEST_HELPERS_H) */ - diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 9e0094d25..c1bff6eb7 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -370,7 +370,7 @@ test_hsdir_revision_counter_check(void *arg) hs_descriptor_t *published_desc = NULL; char *published_desc_str = NULL; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; char *received_desc_str = NULL; hs_descriptor_t *received_desc = NULL; @@ -407,11 +407,11 @@ test_hsdir_revision_counter_check(void *arg) const ed25519_public_key_t *blinded_key; blinded_key = &published_desc->plaintext_data.blinded_pubkey; - hs_get_subcredential(&signing_kp.pubkey, blinded_key, subcredential); + hs_get_subcredential(&signing_kp.pubkey, blinded_key, &subcredential); received_desc_str = helper_fetch_desc_from_hsdir(blinded_key); retval = hs_desc_decode_descriptor(received_desc_str, - subcredential, NULL, &received_desc); + &subcredential, NULL, &received_desc); tt_int_op(retval, OP_EQ, HS_DESC_DECODE_OK); tt_assert(received_desc); @@ -444,7 +444,7 @@ test_hsdir_revision_counter_check(void *arg) received_desc_str = helper_fetch_desc_from_hsdir(blinded_key); retval = hs_desc_decode_descriptor(received_desc_str, - subcredential, NULL, &received_desc); + &subcredential, NULL, &received_desc); tt_int_op(retval, OP_EQ, HS_DESC_DECODE_OK); tt_assert(received_desc); @@ -476,7 +476,7 @@ test_client_cache(void *arg) ed25519_keypair_t signing_kp; hs_descriptor_t *published_desc = NULL; char *published_desc_str = NULL; - uint8_t wanted_subcredential[DIGEST256_LEN]; + hs_subcredential_t wanted_subcredential; response_handler_args_t *args = NULL; dir_connection_t *conn = NULL; @@ -505,8 +505,10 @@ test_client_cache(void *arg) retval = hs_desc_encode_descriptor(published_desc, &signing_kp, NULL, &published_desc_str); tt_int_op(retval, OP_EQ, 0); - memcpy(wanted_subcredential, published_desc->subcredential, DIGEST256_LEN); - tt_assert(!fast_mem_is_zero((char*)wanted_subcredential, DIGEST256_LEN)); + memcpy(&wanted_subcredential, &published_desc->subcredential, + sizeof(hs_subcredential_t)); + tt_assert(!fast_mem_is_zero((char*)wanted_subcredential.subcred, + DIGEST256_LEN)); } /* Test handle_response_fetch_hsdesc_v3() */ @@ -540,8 +542,9 @@ test_client_cache(void *arg) const hs_descriptor_t *cached_desc = NULL; cached_desc = hs_cache_lookup_as_client(&signing_kp.pubkey); tt_assert(cached_desc); - tt_mem_op(cached_desc->subcredential, OP_EQ, wanted_subcredential, - DIGEST256_LEN); + tt_mem_op(cached_desc->subcredential.subcred, + OP_EQ, wanted_subcredential.subcred, + SUBCRED_LEN); } /* Progress time to next TP and check that desc was cleaned */ diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c index 9f09cc3ec..88910e8ea 100644 --- a/src/test/test_hs_client.c +++ b/src/test/test_hs_client.c @@ -416,9 +416,10 @@ test_client_pick_intro(void *arg) const hs_descriptor_t *fetched_desc = hs_cache_lookup_as_client(&service_kp.pubkey); tt_assert(fetched_desc); - tt_mem_op(fetched_desc->subcredential, OP_EQ, desc->subcredential, - DIGEST256_LEN); - tt_assert(!fast_mem_is_zero((char*)fetched_desc->subcredential, + tt_mem_op(fetched_desc->subcredential.subcred, + OP_EQ, desc->subcredential.subcred, + SUBCRED_LEN); + tt_assert(!fast_mem_is_zero((char*)fetched_desc->subcredential.subcred, DIGEST256_LEN)); tor_free(encoded); } diff --git a/src/test/test_hs_descriptor.c b/src/test/test_hs_descriptor.c index 43ac5490a..61ccd3f91 100644 --- a/src/test/test_hs_descriptor.c +++ b/src/test/test_hs_descriptor.c @@ -221,7 +221,7 @@ test_decode_descriptor(void *arg) hs_descriptor_t *desc = NULL; hs_descriptor_t *decoded = NULL; hs_descriptor_t *desc_no_ip = NULL; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; (void) arg; @@ -230,10 +230,10 @@ test_decode_descriptor(void *arg) desc = hs_helper_build_hs_desc_with_ip(&signing_kp); hs_helper_get_subcred_from_identity_keypair(&signing_kp, - subcredential); + &subcredential); /* Give some bad stuff to the decoding function. */ - ret = hs_desc_decode_descriptor("hladfjlkjadf", subcredential, + ret = hs_desc_decode_descriptor("hladfjlkjadf", &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_PLAINTEXT_ERROR); @@ -241,7 +241,7 @@ test_decode_descriptor(void *arg) tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(encoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, NULL, &decoded); + ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(decoded); @@ -253,7 +253,7 @@ test_decode_descriptor(void *arg) ret = ed25519_keypair_generate(&signing_kp_no_ip, 0); tt_int_op(ret, OP_EQ, 0); hs_helper_get_subcred_from_identity_keypair(&signing_kp_no_ip, - subcredential); + &subcredential); desc_no_ip = hs_helper_build_hs_desc_no_ip(&signing_kp_no_ip); tt_assert(desc_no_ip); tor_free(encoded); @@ -262,7 +262,7 @@ test_decode_descriptor(void *arg) tt_int_op(ret, OP_EQ, 0); tt_assert(encoded); hs_descriptor_free(decoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, NULL, &decoded); + ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(decoded); } @@ -286,14 +286,14 @@ test_decode_descriptor(void *arg) &auth_ephemeral_kp.pubkey, CURVE25519_PUBKEY_LEN); hs_helper_get_subcred_from_identity_keypair(&signing_kp, - subcredential); + &subcredential); /* Build and add the auth client to the descriptor. */ clients = desc->superencrypted_data.clients; if (!clients) { clients = smartlist_new(); } - hs_desc_build_authorized_client(subcredential, + hs_desc_build_authorized_client(&subcredential, &client_kp.pubkey, &auth_ephemeral_kp.seckey, descriptor_cookie, client); @@ -315,21 +315,21 @@ test_decode_descriptor(void *arg) /* If we do not have the client secret key, the decoding must fail. */ hs_descriptor_free(decoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, + ret = hs_desc_decode_descriptor(encoded, &subcredential, NULL, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_NEED_CLIENT_AUTH); tt_assert(!decoded); /* If we have an invalid client secret key, the decoding must fail. */ hs_descriptor_free(decoded); - ret = hs_desc_decode_descriptor(encoded, subcredential, + ret = hs_desc_decode_descriptor(encoded, &subcredential, &invalid_client_kp.seckey, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_BAD_CLIENT_AUTH); tt_assert(!decoded); /* If we have the client secret key, the decoding must succeed and the * decoded descriptor must be correct. */ - ret = hs_desc_decode_descriptor(encoded, subcredential, + ret = hs_desc_decode_descriptor(encoded, &subcredential, &client_kp.seckey, &decoded); tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); tt_assert(decoded); @@ -762,7 +762,7 @@ test_build_authorized_client(void *arg) "07d087f1d8c68393721f6e70316d3b29"; const char client_pubkey_b16[] = "8c1298fa6050e372f8598f6deca32e27b0ad457741422c2629ebb132cf7fae37"; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; char *mem_op_hex_tmp=NULL; (void) arg; @@ -774,7 +774,7 @@ test_build_authorized_client(void *arg) tt_int_op(ret, OP_EQ, 0); curve25519_public_key_generate(&client_auth_pk, &client_auth_sk); - memset(subcredential, 42, sizeof(subcredential)); + memset(subcredential.subcred, 42, sizeof(subcredential)); desc_client = tor_malloc_zero(sizeof(hs_desc_authorized_client_t)); @@ -795,7 +795,7 @@ test_build_authorized_client(void *arg) testing_enable_prefilled_rng("\x01", 1); - hs_desc_build_authorized_client(subcredential, + hs_desc_build_authorized_client(&subcredential, &client_auth_pk, &auth_ephemeral_sk, descriptor_cookie, desc_client); diff --git a/src/test/test_hs_ntor.c b/src/test/test_hs_ntor.c index 4f98bc85d..7867740a1 100644 --- a/src/test/test_hs_ntor.c +++ b/src/test/test_hs_ntor.c @@ -23,7 +23,7 @@ test_hs_ntor(void *arg) { int retval; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; ed25519_keypair_t service_intro_auth_keypair; curve25519_keypair_t service_intro_enc_keypair; @@ -42,7 +42,7 @@ test_hs_ntor(void *arg) /* Generate fake data for this unittest */ { /* Generate fake subcredential */ - memset(subcredential, 'Z', DIGEST256_LEN); + memset(subcredential.subcred, 'Z', DIGEST256_LEN); /* service */ curve25519_keypair_generate(&service_intro_enc_keypair, 0); @@ -57,7 +57,7 @@ test_hs_ntor(void *arg) hs_ntor_client_get_introduce1_keys(&service_intro_auth_keypair.pubkey, &service_intro_enc_keypair.pubkey, &client_ephemeral_enc_keypair, - subcredential, + &subcredential, &client_hs_ntor_intro_cell_keys); tt_int_op(retval, OP_EQ, 0); @@ -66,7 +66,7 @@ test_hs_ntor(void *arg) hs_ntor_service_get_introduce1_keys(&service_intro_auth_keypair.pubkey, &service_intro_enc_keypair, &client_ephemeral_enc_keypair.pubkey, - subcredential, + &subcredential, &service_hs_ntor_intro_cell_keys); tt_int_op(retval, OP_EQ, 0); diff --git a/src/test/test_hs_ntor_cl.c b/src/test/test_hs_ntor_cl.c index a7cebc6af..3acd7ef0b 100644 --- a/src/test/test_hs_ntor_cl.c +++ b/src/test/test_hs_ntor_cl.c @@ -53,7 +53,7 @@ client1(int argc, char **argv) curve25519_public_key_t intro_enc_pubkey; ed25519_public_key_t intro_auth_pubkey; curve25519_keypair_t client_ephemeral_enc_keypair; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; /* Output */ hs_ntor_intro_cell_keys_t hs_ntor_intro_cell_keys; @@ -65,7 +65,7 @@ client1(int argc, char **argv) BASE16(3, intro_enc_pubkey.public_key, CURVE25519_PUBKEY_LEN); BASE16(4, client_ephemeral_enc_keypair.seckey.secret_key, CURVE25519_SECKEY_LEN); - BASE16(5, subcredential, DIGEST256_LEN); + BASE16(5, subcredential.subcred, DIGEST256_LEN); /* Generate keypair */ curve25519_public_key_generate(&client_ephemeral_enc_keypair.pubkey, @@ -74,7 +74,7 @@ client1(int argc, char **argv) retval = hs_ntor_client_get_introduce1_keys(&intro_auth_pubkey, &intro_enc_pubkey, &client_ephemeral_enc_keypair, - subcredential, + &subcredential, &hs_ntor_intro_cell_keys); if (retval < 0) { goto done; @@ -106,7 +106,7 @@ server1(int argc, char **argv) curve25519_keypair_t intro_enc_keypair; ed25519_public_key_t intro_auth_pubkey; curve25519_public_key_t client_ephemeral_enc_pubkey; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; /* Output */ hs_ntor_intro_cell_keys_t hs_ntor_intro_cell_keys; @@ -119,7 +119,7 @@ server1(int argc, char **argv) BASE16(2, intro_auth_pubkey.pubkey, ED25519_PUBKEY_LEN); BASE16(3, intro_enc_keypair.seckey.secret_key, CURVE25519_SECKEY_LEN); BASE16(4, client_ephemeral_enc_pubkey.public_key, CURVE25519_PUBKEY_LEN); - BASE16(5, subcredential, DIGEST256_LEN); + BASE16(5, subcredential.subcred, DIGEST256_LEN); /* Generate keypair */ curve25519_public_key_generate(&intro_enc_keypair.pubkey, @@ -130,7 +130,7 @@ server1(int argc, char **argv) retval = hs_ntor_service_get_introduce1_keys(&intro_auth_pubkey, &intro_enc_keypair, &client_ephemeral_enc_pubkey, - subcredential, + &subcredential, &hs_ntor_intro_cell_keys); if (retval < 0) { goto done; @@ -188,7 +188,7 @@ client2(int argc, char **argv) ed25519_public_key_t intro_auth_pubkey; curve25519_keypair_t client_ephemeral_enc_keypair; curve25519_public_key_t service_ephemeral_rend_pubkey; - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; /* Output */ hs_ntor_rend_cell_keys_t hs_ntor_rend_cell_keys; @@ -201,7 +201,7 @@ client2(int argc, char **argv) CURVE25519_SECKEY_LEN); BASE16(4, intro_enc_pubkey.public_key, CURVE25519_PUBKEY_LEN); BASE16(5, service_ephemeral_rend_pubkey.public_key, CURVE25519_PUBKEY_LEN); - BASE16(6, subcredential, DIGEST256_LEN); + BASE16(6, subcredential.subcred, DIGEST256_LEN); /* Generate keypair */ curve25519_public_key_generate(&client_ephemeral_enc_keypair.pubkey, diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c index 37c57f795..848a488be 100644 --- a/src/test/test_hs_ob.c +++ b/src/test/test_hs_ob.c @@ -190,7 +190,7 @@ test_get_subcredentials(void *arg) ed25519_keypair_generate(&onion_addr_kp_1, 0); smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); - uint8_t *subcreds = NULL; + hs_subcredential_t *subcreds = NULL; size_t num = hs_ob_get_subcredentials(&config, &subcreds); tt_uint_op(num, OP_EQ, 3); @@ -200,14 +200,14 @@ test_get_subcredentials(void *arg) const int steps[3] = {0, -1, 1}; for (unsigned int i = 0; i < num; i++) { - uint8_t subcredential[DIGEST256_LEN]; + hs_subcredential_t subcredential; ed25519_public_key_t blinded_pubkey; hs_build_blinded_pubkey(&onion_addr_kp_1.pubkey, NULL, 0, tp + steps[i], &blinded_pubkey); hs_get_subcredential(&onion_addr_kp_1.pubkey, &blinded_pubkey, - subcredential); - tt_mem_op(&(subcreds[i * sizeof(subcredential)]), OP_EQ, subcredential, - sizeof(subcredential)); + &subcredential); + tt_mem_op(subcreds[i].subcred, OP_EQ, subcredential.subcred, + SUBCRED_LEN); } done: @@ -228,4 +228,3 @@ struct testcase_t hs_ob_tests[] = { END_OF_TESTCASES }; -

1 0

[tor/master] Use time-invariant conditional memcpy to make onionbalance loop safer
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit 942543253a30b8231c46eeaeb44f7ba174152113 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 16 19:52:01 2020 -0500 Use time-invariant conditional memcpy to make onionbalance loop safer --- src/feature/hs/hs_cell.c | 51 +++++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 2b8345383..c82ffd647 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -776,20 +776,20 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, if (intro_keys == NULL) { log_info(LD_REND, "Invalid INTRODUCE2 encrypted data. Unable to " "compute key material"); - goto err; + return NULL; + } + + /* Make sure we are not about to underflow. */ + if (BUG(encrypted_section_len < DIGEST256_LEN)) { + return NULL; } /* Validate MAC from the cell and our computed key material. The MAC field * in the cell is at the end of the encrypted section. */ - int found_idx = -1; + intro_keys_result = tor_malloc_zero(sizeof(*intro_keys_result)); for (int i = 0; i < data->n_subcredentials; ++i) { uint8_t mac[DIGEST256_LEN]; - /* Make sure we are not about to underflow. */ - if (encrypted_section_len < sizeof(mac)) { - goto err; - } - /* The MAC field is at the very end of the ENCRYPTED section. */ size_t mac_offset = encrypted_section_len - sizeof(mac); /* Compute the MAC. Use the entire encoded payload with a length up to the @@ -800,33 +800,22 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, intro_keys[i].mac_key, sizeof(intro_keys[i].mac_key), mac, sizeof(mac)); - if (tor_memeq(mac, encrypted_section + mac_offset, sizeof(mac))) { - found_idx = i; - break; - } - } - - if (found_idx == -1) { - log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); - goto err; + /* Time-invariant conditional copy: if the MAC is what we expected, then + * set intro_keys_result to intro_keys[i]. Otherwise, don't: but don't + * leak which one it was! */ + bool equal = tor_memeq(mac, encrypted_section + mac_offset, sizeof(mac)); + memcpy_if_true_timei(equal, intro_keys_result, &intro_keys[i], + sizeof(*intro_keys_result)); } - /* We found a match! */ - if (data->n_subcredentials == 1) { - /* There was only one; steal it. */ - intro_keys_result = intro_keys; - intro_keys = NULL; - } else { - /* Copy out the one we wanted. */ - intro_keys_result = tor_memdup(&intro_keys[found_idx], - sizeof(hs_ntor_intro_cell_keys_t)); - } + /* We no longer need intro_keys. */ + memwipe(intro_keys, 0, + sizeof(hs_ntor_intro_cell_keys_t) * data->n_subcredentials); + tor_free(intro_keys); - err: - if (intro_keys) { - memwipe(intro_keys, 0, - sizeof(hs_ntor_intro_cell_keys_t) * data->n_subcredentials); - tor_free(intro_keys); + if (safe_mem_is_zero(intro_keys_result, sizeof(*intro_keys_result))) { + log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); + tor_free(intro_keys_result); /* sets intro_keys_result to NULL */ } return intro_keys_result;

1 0

[tor/master] Refresh OB keys when we build a new descriptor.
by nickm＠torproject.org 24 Feb '20

24 Feb '20

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

1 0

[tor/master] Pass multiple subcredentials all the way down to hs_ntor.
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit b6250236a2427b116c819be3305727fcbefdb424 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 16 19:10:26 2020 -0500 Pass multiple subcredentials all the way down to hs_ntor. This approach saves us a pair of curve25519 operations for every subcredential but the first. It is not yet constant-time. I've noted a few places where IMO we should refactor the code so that the complete list of subcredentials is passed in earlier. --- src/feature/hs/hs_cell.c | 91 +++++++++++++++++++++++++++++---------------- src/feature/hs/hs_cell.h | 11 ++++-- src/feature/hs/hs_circuit.c | 5 ++- 3 files changed, 71 insertions(+), 36 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index d377e9134..2b8345383 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -68,14 +68,17 @@ compute_introduce_mac(const uint8_t *encoded_cell, size_t encoded_cell_len, memwipe(mac_msg, 0, sizeof(mac_msg)); } -/** From a set of keys, subcredential and the ENCRYPTED section of an - * INTRODUCE2 cell, return a newly allocated intro cell keys structure. - * Finally, the client public key is copied in client_pk. On error, return - * NULL. */ +/** + * From a set of keys, a list of subcredentials, and the ENCRYPTED section of + * an INTRODUCE2 cell, return an array of newly allocated intro cell keys + * structures. Finally, the client public key is copied in client_pk. On + * error, return NULL. + **/ static hs_ntor_intro_cell_keys_t * get_introduce2_key_material(const ed25519_public_key_t *auth_key, const curve25519_keypair_t *enc_key, - const hs_subcredential_t *subcredential, + int n_subcredentials, + const hs_subcredential_t *subcredentials, const uint8_t *encrypted_section, curve25519_public_key_t *client_pk) { @@ -83,17 +86,19 @@ get_introduce2_key_material(const ed25519_public_key_t *auth_key, tor_assert(auth_key); tor_assert(enc_key); - tor_assert(subcredential); + tor_assert(n_subcredentials > 0); + tor_assert(subcredentials); tor_assert(encrypted_section); tor_assert(client_pk); - keys = tor_malloc_zero(sizeof(*keys)); + keys = tor_calloc(n_subcredentials, sizeof(hs_ntor_intro_cell_keys_t)); /* First bytes of the ENCRYPTED section are the client public key. */ memcpy(client_pk->public_key, encrypted_section, CURVE25519_PUBKEY_LEN); - if (hs_ntor_service_get_introduce1_keys(auth_key, enc_key, client_pk, - subcredential, keys) < 0) { + if (hs_ntor_service_get_introduce1_keys_multi(auth_key, enc_key, client_pk, + n_subcredentials, + subcredentials, keys) < 0) { /* Don't rely on the caller to wipe this on error. */ memwipe(client_pk, 0, sizeof(curve25519_public_key_t)); tor_free(keys); @@ -760,10 +765,12 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, size_t encrypted_section_len) { hs_ntor_intro_cell_keys_t *intro_keys = NULL; + hs_ntor_intro_cell_keys_t *intro_keys_result = NULL; /* Build the key material out of the key material found in the cell. */ intro_keys = get_introduce2_key_material(data->auth_pk, data->enc_kp, - data->subcredential, + data->n_subcredentials, + data->subcredentials, encrypted_section, &data->client_pk); if (intro_keys == NULL) { @@ -774,10 +781,11 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, /* Validate MAC from the cell and our computed key material. The MAC field * in the cell is at the end of the encrypted section. */ - { + int found_idx = -1; + for (int i = 0; i < data->n_subcredentials; ++i) { uint8_t mac[DIGEST256_LEN]; - /* Make sure we are now about to underflow. */ + /* Make sure we are not about to underflow. */ if (encrypted_section_len < sizeof(mac)) { goto err; } @@ -789,24 +797,39 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, compute_introduce_mac(data->payload, data->payload_len - encrypted_section_len, encrypted_section, encrypted_section_len, - intro_keys->mac_key, sizeof(intro_keys->mac_key), + intro_keys[i].mac_key, + sizeof(intro_keys[i].mac_key), mac, sizeof(mac)); - if (tor_memneq(mac, encrypted_section + mac_offset, sizeof(mac))) { - log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); - goto err; + if (tor_memeq(mac, encrypted_section + mac_offset, sizeof(mac))) { + found_idx = i; + break; } } - goto done; + if (found_idx == -1) { + log_info(LD_REND, "Invalid MAC validation for INTRODUCE2 cell"); + goto err; + } + + /* We found a match! */ + if (data->n_subcredentials == 1) { + /* There was only one; steal it. */ + intro_keys_result = intro_keys; + intro_keys = NULL; + } else { + /* Copy out the one we wanted. */ + intro_keys_result = tor_memdup(&intro_keys[found_idx], + sizeof(hs_ntor_intro_cell_keys_t)); + } err: if (intro_keys) { - memwipe(intro_keys, 0, sizeof(hs_ntor_intro_cell_keys_t)); + memwipe(intro_keys, 0, + sizeof(hs_ntor_intro_cell_keys_t) * data->n_subcredentials); tor_free(intro_keys); } - done: - return intro_keys; + return intro_keys_result; } /** Return the newly allocated intro keys using the given service @@ -831,18 +854,22 @@ get_intro2_keys_as_ob(const hs_service_config_t *config, goto end; } - for (size_t idx = 0; idx < ob_num_subcreds; idx++) { - /* Copy current data into a new INTRO2 cell data. We will then change the - * subcredential in order to validate. */ - hs_cell_introduce2_data_t new_data = *data; - new_data.subcredential = &ob_subcreds[idx]; - intro_keys = get_introduce2_keys_and_verify_mac(&new_data, - encrypted_section, - encrypted_section_len); - if (intro_keys) { - /* It validates. We have a hit as an onion balance instance. */ - goto end; - } + /* Copy current data into a new INTRO2 cell data. We will then change the + * subcredential in order to validate. */ + hs_cell_introduce2_data_t new_data = *data; + /* XXXX This list should have been the descriptor's subcredentials all + * XXXX along. + */ + new_data.n_subcredentials = (int)ob_num_subcreds; + new_data.subcredentials = ob_subcreds; + + intro_keys = get_introduce2_keys_and_verify_mac(&new_data, + encrypted_section, + encrypted_section_len); + memwipe(&new_data, 0, sizeof(new_data)); + if (intro_keys) { + /* It validates. We have a hit as an onion balance instance. */ + goto end; } end: diff --git a/src/feature/hs/hs_cell.h b/src/feature/hs/hs_cell.h index 58cc401cc..cc2e7b581 100644 --- a/src/feature/hs/hs_cell.h +++ b/src/feature/hs/hs_cell.h @@ -57,9 +57,14 @@ typedef struct hs_cell_introduce2_data_t { owned by the introduction point object through which we received the INTRO2 cell*/ const curve25519_keypair_t *enc_kp; - /** Subcredentials of the service. Pointer owned by the descriptor that owns - the introduction point through which we received the INTRO2 cell. */ - const struct hs_subcredential_t *subcredential; + /** + * Length of the subcredentials array below. + **/ + int n_subcredentials; + /** Array of n_subcredentials subcredentials for the service. Pointer + * owned by the descriptor that owns the introduction point through which we + * received the INTRO2 cell. */ + const struct hs_subcredential_t *subcredentials; /** Payload of the received encoded cell. */ const uint8_t *payload; /** Size of the payload of the received encoded cell. */ diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index fb3694b2d..48a92962f 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -983,7 +983,10 @@ hs_circ_handle_introduce2(const hs_service_t *service, * parsed, decrypted and key material computed correctly. */ data.auth_pk = &ip->auth_key_kp.pubkey; data.enc_kp = &ip->enc_key_kp; - data.subcredential = subcredential; + // XXXX We should replace these elements with something precomputed for + // XXXX the onionbalance case. + data.n_subcredentials = 1; + data.subcredentials = subcredential; data.payload = payload; data.payload_len = payload_len; data.link_specifiers = smartlist_new();

1 0

[tor/master] Add a function to maybe memcpy() a value, in constant time.
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit 4269ab97c685cb3bef9607cbada8af5b6b84640c Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jan 16 19:23:20 2020 -0500 Add a function to maybe memcpy() a value, in constant time. --- src/lib/ctime/di_ops.c | 27 +++++++++++++++++++++++++++ src/lib/ctime/di_ops.h | 2 ++ src/test/test_util.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/lib/ctime/di_ops.c b/src/lib/ctime/di_ops.c index 7448a9973..0859dd8be 100644 --- a/src/lib/ctime/di_ops.c +++ b/src/lib/ctime/di_ops.c @@ -279,3 +279,30 @@ select_array_member_cumulative_timei(const uint64_t *entries, int n_entries, return i_chosen; } + +/** + * If s is true, then copy n bytes from src</d> to + * dest. Otherwise leave dest alone. + * + * This function behaves the same as + * + * if (s) + * memcpy(dest, src, n); + * + * except that it tries to run in the same amount of time whether s is + * true or not. + **/ +void +memcpy_if_true_timei(bool s, void *dest, const void *src, size_t n) +{ + // If s is true, mask will be ~0. If s is false, mask will be 0. + const char mask = (char) -(signed char)s; + + char *destp = dest; + const char *srcp = src; + for (size_t i = 0; i < n; ++i) { + *destp = (*destp & ~mask) | (*srcp & mask); + ++destp; + ++srcp; + } +} diff --git a/src/lib/ctime/di_ops.h b/src/lib/ctime/di_ops.h index 4ff8f0316..9fe2884ec 100644 --- a/src/lib/ctime/di_ops.h +++ b/src/lib/ctime/di_ops.h @@ -73,4 +73,6 @@ int select_array_member_cumulative_timei(const uint64_t *entries, int n_entries, uint64_t total, uint64_t rand_val); +void memcpy_if_true_timei(bool s, void *dest, const void *src, size_t n); + #endif /* !defined(TOR_DI_OPS_H) */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 07c31f02d..7a375b06b 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -4572,6 +4572,35 @@ test_util_di_ops(void *arg) } static void +test_util_memcpy_iftrue_timei(void *arg) +{ + (void)arg; + char buf1[25]; + char buf2[25]; + char buf3[25]; + + for (int i = 0; i < 100; ++i) { + crypto_rand(buf1, sizeof(buf1)); + crypto_rand(buf2, sizeof(buf2)); + memcpy(buf3, buf1, sizeof(buf1)); + + /* We just copied buf1 into buf3. Now we're going to copy buf2 into buf2, + iff our coin flip comes up heads. */ + bool coinflip = crypto_rand_int(2) == 0; + + memcpy_if_true_timei(coinflip, buf3, buf2, sizeof(buf3)); + + if (coinflip) { + tt_mem_op(buf3, OP_EQ, buf2, sizeof(buf2)); + } else { + tt_mem_op(buf3, OP_EQ, buf1, sizeof(buf1)); + } + } + done: + ; +} + +static void test_util_di_map(void *arg) { (void)arg; @@ -6386,6 +6415,7 @@ struct testcase_t util_tests[] = { UTIL_LEGACY(path_is_relative), UTIL_LEGACY(strtok), UTIL_LEGACY(di_ops), + UTIL_TEST(memcpy_iftrue_timei, 0), UTIL_TEST(di_map, 0), UTIL_TEST(round_to_next_multiple_of, 0), UTIL_TEST(laplace, 0),

1 0

[tor/master] Write unittest that covers cases of INTRODUCE1 handling.
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit ba99287d13782048f58a88dc5d18780fad9f2034 Author: George Kadianakis <desnacked(a)riseup.net> Date: Mon Jan 27 17:26:47 2020 +0200 Write unittest that covers cases of INTRODUCE1 handling. Also fix some memleaks of other OB unittests. --- src/core/or/circuitbuild.c | 4 +- src/core/or/circuitbuild.h | 3 +- src/feature/hs/hs_cell.c | 2 +- src/feature/hs/hs_circuit.c | 8 +- src/feature/hs/hs_circuit.h | 6 + src/feature/nodelist/nodelist.c | 4 +- src/feature/nodelist/nodelist.h | 4 +- src/test/hs_test_helpers.c | 45 ++++- src/test/hs_test_helpers.h | 8 +- src/test/test_hs_ob.c | 8 +- src/test/test_hs_service.c | 356 +++++++++++++++++++++++++++++++++++++++- 11 files changed, 421 insertions(+), 27 deletions(-) diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 03ed2c7d2..003b91af8 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -2819,8 +2819,8 @@ extend_info_dup(extend_info_t *info) * If there is no chosen exit, or if we don't know the node_t for * the chosen exit, return NULL. */ -const node_t * -build_state_get_exit_node(cpath_build_state_t *state) +MOCK_IMPL(const node_t *, +build_state_get_exit_node,(cpath_build_state_t *state)) { if (!state || !state->chosen_exit) return NULL; diff --git a/src/core/or/circuitbuild.h b/src/core/or/circuitbuild.h index f5a343906..48592dd34 100644 --- a/src/core/or/circuitbuild.h +++ b/src/core/or/circuitbuild.h @@ -66,7 +66,8 @@ int circuit_can_use_tap(const origin_circuit_t *circ); int circuit_has_usable_onion_key(const origin_circuit_t *circ); int extend_info_has_preferred_onion_key(const extend_info_t* ei); const uint8_t *build_state_get_exit_rsa_id(cpath_build_state_t *state); -const node_t *build_state_get_exit_node(cpath_build_state_t *state); +MOCK_DECL(const node_t *, + build_state_get_exit_node,(cpath_build_state_t *state)); const char *build_state_get_exit_nickname(cpath_build_state_t *state); struct circuit_guard_state_t; diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index fce6fe022..97a1691f1 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -869,7 +869,7 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data, /* Check our replay cache for this introduction point. */ if (replaycache_add_test_and_elapsed(data->replay_cache, encrypted_section, encrypted_section_len, &elapsed)) { - log_warn(LD_REND, "Possible replay detected! An INTRODUCE2 cell with the" + log_warn(LD_REND, "Possible replay detected! An INTRODUCE2 cell with the " "same ENCRYPTED section was seen %ld seconds ago. " "Dropping cell.", (long int) elapsed); goto done; diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index febeec473..97507df9f 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -368,10 +368,10 @@ get_service_anonymity_string(const hs_service_t *service) * success, a circuit identifier is attached to the circuit with the needed * data. This function will try to open a circuit for a maximum value of * MAX_REND_FAILURES then it will give up. */ -static void -launch_rendezvous_point_circuit(const hs_service_t *service, - const hs_service_intro_point_t *ip, - const hs_cell_introduce2_data_t *data) +MOCK_IMPL(STATIC void, +launch_rendezvous_point_circuit,(const hs_service_t *service, + const hs_service_intro_point_t *ip, + const hs_cell_introduce2_data_t *data)) { int circ_needs_uptime; time_t now = time(NULL); diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h index 5c20e3818..22e936e68 100644 --- a/src/feature/hs/hs_circuit.h +++ b/src/feature/hs/hs_circuit.h @@ -79,6 +79,12 @@ create_rp_circuit_identifier(const hs_service_t *service, const curve25519_public_key_t *server_pk, const struct hs_ntor_rend_cell_keys_t *keys); +struct hs_cell_introduce2_data_t; +MOCK_DECL(STATIC void, +launch_rendezvous_point_circuit,(const hs_service_t *service, + const hs_service_intro_point_t *ip, + const struct hs_cell_introduce2_data_t *data)); + #endif /* defined(HS_CIRCUIT_PRIVATE) */ #endif /* !defined(TOR_HS_CIRCUIT_H) */ diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index 94ff08826..1d0b1a0d4 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -1213,8 +1213,8 @@ node_get_rsa_id_digest(const node_t *node) * If node is NULL, returns an empty smartlist. * * The smartlist must be freed using link_specifier_smartlist_free(). */ -smartlist_t * -node_get_link_specifier_smartlist(const node_t *node, bool direct_conn) +MOCK_IMPL(smartlist_t *, +node_get_link_specifier_smartlist,(const node_t *node, bool direct_conn)) { link_specifier_t *ls; tor_addr_port_t ap; diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index 9742e3dff..40aed0067 100644 --- a/src/feature/nodelist/nodelist.h +++ b/src/feature/nodelist/nodelist.h @@ -78,8 +78,8 @@ int node_supports_ed25519_hs_intro(const node_t *node); int node_supports_v3_rendezvous_point(const node_t *node); int node_supports_establish_intro_dos_extension(const node_t *node); const uint8_t *node_get_rsa_id_digest(const node_t *node); -smartlist_t *node_get_link_specifier_smartlist(const node_t *node, - bool direct_conn); +MOCK_DECL(smartlist_t *,node_get_link_specifier_smartlist,(const node_t *node, + bool direct_conn)); void link_specifier_smartlist_free_(smartlist_t *ls_list); #define link_specifier_smartlist_free(ls_list) \ FREE_AND_NULL(smartlist_t, link_specifier_smartlist_free_, (ls_list)) diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index eb4af1c9f..5116fc716 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -13,9 +13,22 @@ #include "feature/hs/hs_service.h" #include "test/hs_test_helpers.h" +/** + * Create an introduction point taken straight out of an HSv3 descriptor. + * + * Use 'signing_kp' to sign the introduction point certificates. + * + * If 'intro_auth_kp' is provided use that as the introduction point + * authentication keypair, otherwise generate one on the fly. + * + * If 'intro_enc_kp' is provided use that as the introduction point encryption + * keypair, otherwise generate one on the fly. + */ hs_desc_intro_point_t * hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, - const char *addr, int legacy) + const char *addr, int legacy, + const ed25519_keypair_t *intro_auth_kp, + const curve25519_keypair_t *intro_enc_kp) { int ret; ed25519_keypair_t auth_kp; @@ -56,8 +69,12 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, smartlist_add(ip->link_specifiers, ls_ip); } - ret = ed25519_keypair_generate(&auth_kp, 0); - tt_int_op(ret, OP_EQ, 0); + if (intro_auth_kp) { + memcpy(&auth_kp, intro_auth_kp, sizeof(ed25519_keypair_t)); + } else { + ret = ed25519_keypair_generate(&auth_kp, 0); + tt_int_op(ret, OP_EQ, 0); + } ip->auth_key_cert = tor_cert_create(signing_kp, CERT_TYPE_AUTH_HS_IP_KEY, &auth_kp.pubkey, now, HS_DESC_CERT_LIFETIME, @@ -85,8 +102,12 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, ed25519_keypair_t ed25519_kp; tor_cert_t *cross_cert; - ret = curve25519_keypair_generate(&curve25519_kp, 0); - tt_int_op(ret, OP_EQ, 0); + if (intro_enc_kp) { + memcpy(&curve25519_kp, intro_enc_kp, sizeof(curve25519_keypair_t)); + } else { + ret = curve25519_keypair_generate(&curve25519_kp, 0); + tt_int_op(ret, OP_EQ, 0); + } ed25519_keypair_from_curve25519_keypair(&ed25519_kp, &signbit, &curve25519_kp); cross_cert = tor_cert_create(signing_kp, CERT_TYPE_CROSS_HS_IP_KEYS, @@ -95,6 +116,8 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, CERT_FLAG_INCLUDE_SIGNING_KEY); tt_assert(cross_cert); ip->enc_key_cert = cross_cert; + memcpy(ip->enc_key.public_key, curve25519_kp.pubkey.public_key, + CURVE25519_PUBKEY_LEN); } intro_point = ip; @@ -165,13 +188,17 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, if (!no_ip) { /* Add four intro points. */ smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "1.2.3.4", 0)); + hs_helper_build_intro_point(signing_kp, now, "1.2.3.4", 0, + NULL, NULL)); smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "[2600::1]", 0)); + hs_helper_build_intro_point(signing_kp, now, "[2600::1]", 0, + NULL, NULL)); smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "3.2.1.4", 1)); + hs_helper_build_intro_point(signing_kp, now, "3.2.1.4", 1, + NULL, NULL)); smartlist_add(desc->encrypted_data.intro_points, - hs_helper_build_intro_point(signing_kp, now, "5.6.7.8", 1)); + hs_helper_build_intro_point(signing_kp, now, "5.6.7.8", 1, + NULL, NULL)); } descp = desc; diff --git a/src/test/hs_test_helpers.h b/src/test/hs_test_helpers.h index cad453282..23d11f2a4 100644 --- a/src/test/hs_test_helpers.h +++ b/src/test/hs_test_helpers.h @@ -8,9 +8,11 @@ #include "feature/hs/hs_descriptor.h" /* Set of functions to help build and test descriptors. */ -hs_desc_intro_point_t *hs_helper_build_intro_point( - const ed25519_keypair_t *signing_kp, time_t now, - const char *addr, int legacy); +hs_desc_intro_point_t * +hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, + const char *addr, int legacy, + const ed25519_keypair_t *intro_auth_kp, + const curve25519_keypair_t *intro_enc_kp); hs_descriptor_t *hs_helper_build_hs_desc_no_ip( const ed25519_keypair_t *signing_kp); hs_descriptor_t *hs_helper_build_hs_desc_with_ip( diff --git a/src/test/test_hs_ob.c b/src/test/test_hs_ob.c index c4d9d239d..b84cef9de 100644 --- a/src/test/test_hs_ob.c +++ b/src/test/test_hs_ob.c @@ -169,6 +169,7 @@ static void test_get_subcredentials(void *arg) { int ret; + hs_service_t *service = NULL; hs_service_config_t config; (void) arg; @@ -192,7 +193,7 @@ test_get_subcredentials(void *arg) smartlist_add(config.ob_master_pubkeys, &onion_addr_kp_1.pubkey); /* Set up an instance */ - hs_service_t *service = tor_malloc_zero(sizeof(hs_service_t)); + service = tor_malloc_zero(sizeof(hs_service_t)); service->config = config; service->desc_current = service_descriptor_new(); service->desc_next = service_descriptor_new(); @@ -234,7 +235,12 @@ test_get_subcredentials(void *arg) done: tor_free(subcreds); + smartlist_free(config.ob_master_pubkeys); + if (service) { + memset(&service->config, 0, sizeof(hs_service_config_t)); + hs_service_free(service); + } UNMOCK(networkstatus_get_live_consensus); } diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index e33d593d9..1767648bb 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -51,6 +51,8 @@ #include "feature/hs/hs_common.h" #include "feature/hs/hs_config.h" #include "feature/hs/hs_ident.h" +#include "feature/hs/hs_ob.h" +#include "feature/hs/hs_cell.h" #include "feature/hs/hs_intropoint.h" #include "feature/hs/hs_service.h" #include "feature/nodelist/networkstatus.h" @@ -109,6 +111,9 @@ mock_circuit_mark_for_close(circuit_t *circ, int reason, int line, return; } +static size_t relay_payload_len; +static char relay_payload[RELAY_PAYLOAD_SIZE]; + static int mock_relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, uint8_t relay_command, const char *payload, @@ -124,6 +129,10 @@ mock_relay_send_command_from_edge(streamid_t stream_id, circuit_t *circ, (void) cpath_layer; (void) filename; (void) lineno; + + memcpy(relay_payload, payload, payload_len); + relay_payload_len = payload_len; + return 0; } @@ -1160,7 +1169,7 @@ test_closing_intro_circs(void *arg) /** Test sending and receiving introduce2 cells */ static void -test_introduce2(void *arg) +test_bad_introduce2(void *arg) { int ret; int flags = CIRCLAUNCH_NEED_UPTIME | CIRCLAUNCH_IS_INTERNAL; @@ -2169,6 +2178,348 @@ test_export_client_circuit_id(void *arg) tor_free(cp2); } +static smartlist_t * +mock_node_get_link_specifier_smartlist(const node_t *node, bool direct_conn) +{ + (void) node; + (void) direct_conn; + + smartlist_t *lspecs = smartlist_new(); + link_specifier_t *ls_legacy = link_specifier_new(); + smartlist_add(lspecs, ls_legacy); + + return lspecs; +} + +static node_t *fake_node = NULL; + +static const node_t * +mock_build_state_get_exit_node(cpath_build_state_t *state) +{ + (void) state; + + if (!fake_node) { + curve25519_secret_key_t seckey; + curve25519_secret_key_generate(&seckey, 0); + + fake_node = tor_malloc_zero(sizeof(node_t)); + fake_node->ri = tor_malloc_zero(sizeof(routerinfo_t)); + fake_node->ri->onion_curve25519_pkey = + tor_malloc_zero(sizeof(curve25519_public_key_t)); + curve25519_public_key_generate(fake_node->ri->onion_curve25519_pkey, + &seckey); + } + + return fake_node; +} + +static void +mock_launch_rendezvous_point_circuit(const hs_service_t *service, + const hs_service_intro_point_t *ip, + const hs_cell_introduce2_data_t *data) +{ + (void) service; + (void) ip; + (void) data; + return; +} + +/** + * Test that INTRO2 cells are handled well by onion services in the normal + * case and also when onionbalance is enabled. + */ +static void +test_intro2_handling(void *arg) +{ + (void)arg; + + MOCK(build_state_get_exit_node, mock_build_state_get_exit_node); + MOCK(relay_send_command_from_edge_, mock_relay_send_command_from_edge); + MOCK(node_get_link_specifier_smartlist, + mock_node_get_link_specifier_smartlist); + MOCK(launch_rendezvous_point_circuit, mock_launch_rendezvous_point_circuit); + + memset(relay_payload, 0, sizeof(relay_payload)); + + int retval; + time_t now = 0101010101; + update_approx_time(now); + + /** OK this is the play: + * + * In Act I, we have a standalone onion service X (without onionbalance + * enabled). We test that X can properly handle INTRO2 cells sent by a + * client Alice. + * + * In Act II, we create an onionbalance setup with frontend being Z which + * includes instances X and Y. We then setup onionbalance on X and test that + * Alice who addresses Z can communicate with X through INTRO2 cells. + * + * In Act III, we test that Alice can also communicate with X + * directly even tho onionbalance is enabled. + * + * And finally in Act IV, we check various cases where the INTRO2 cell + * should not go through because the subcredentials don't line up + * (e.g. Alice sends INTRO2 to X using Y's subcredential). + */ + + /** Let's start with some setup! Create the instances and the frontend + service, create Alice, etc: */ + + /* Create instance X */ + hs_service_t x_service; + memset(&x_service, 0, sizeof(hs_service_t)); + /* Disable onionbalance */ + x_service.config.ob_master_pubkeys = NULL; + x_service.state.replay_cache_rend_cookie = replaycache_new(0,0); + + /* Create subcredential for x: */ + ed25519_keypair_t x_identity_keypair; + hs_subcredential_t x_subcred; + ed25519_keypair_generate(&x_identity_keypair, 0); + hs_helper_get_subcred_from_identity_keypair(&x_identity_keypair, + &x_subcred); + + /* Create the x instance's intro point */ + hs_service_intro_point_t *x_ip = NULL; + { + curve25519_secret_key_t seckey; + curve25519_public_key_t pkey; + curve25519_secret_key_generate(&seckey, 0); + curve25519_public_key_generate(&pkey, &seckey); + + node_t intro_node; + memset(&intro_node, 0, sizeof(intro_node)); + routerinfo_t ri; + memset(&ri, 0, sizeof(routerinfo_t)); + ri.onion_curve25519_pkey = &pkey; + intro_node.ri = &ri; + + x_ip = service_intro_point_new(&intro_node); + } + + /* Create z frontend's subcredential */ + ed25519_keypair_t z_identity_keypair; + hs_subcredential_t z_subcred; + ed25519_keypair_generate(&z_identity_keypair, 0); + hs_helper_get_subcred_from_identity_keypair(&z_identity_keypair, + &z_subcred); + + /* Create y instance's subcredential */ + ed25519_keypair_t y_identity_keypair; + hs_subcredential_t y_subcred; + ed25519_keypair_generate(&y_identity_keypair, 0); + hs_helper_get_subcred_from_identity_keypair(&y_identity_keypair, + &y_subcred); + + /* Create Alice's intro point */ + hs_desc_intro_point_t *alice_ip; + ed25519_keypair_t signing_kp; + ed25519_keypair_generate(&signing_kp, 0); + alice_ip = hs_helper_build_intro_point(&signing_kp, now, "1.2.3.4", 0, + &x_ip->auth_key_kp, + &x_ip->enc_key_kp); + + /* Create Alice's intro and rend circuits */ + origin_circuit_t *intro_circ = origin_circuit_new(); + intro_circ->cpath = tor_malloc_zero(sizeof(crypt_path_t)); + intro_circ->cpath->prev = intro_circ->cpath; + intro_circ->hs_ident = tor_malloc_zero(sizeof(*intro_circ->hs_ident)); + origin_circuit_t rend_circ; + rend_circ.hs_ident = tor_malloc_zero(sizeof(*rend_circ.hs_ident)); + curve25519_keypair_generate(&rend_circ.hs_ident->rendezvous_client_kp, 0); + memset(rend_circ.hs_ident->rendezvous_cookie, 'r', HS_REND_COOKIE_LEN); + + /* ************************************************************ */ + + /* Act I: + * + * Where Alice connects to X without onionbalance in the picture */ + + /* Create INTRODUCE1 */ + tt_assert(fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &x_subcred); + + /* Check that the payload was written successfully */ + tt_int_op(retval, OP_EQ, 0); + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + /* Handle the cell */ + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload,relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + /* ************************************************************ */ + + /* Act II: + * + * We now create an onionbalance setup with Z being the frontend and X and Y + * being the backend instances. Make sure that Alice can talk with the + * backend instance X even tho she thinks she is talking to the frontend Z. + */ + + /* Now configure the X instance to do onionbalance with Z as the frontend */ + x_service.config.ob_master_pubkeys = smartlist_new(); + smartlist_add(x_service.config.ob_master_pubkeys, + &z_identity_keypair.pubkey); + + /* Create descriptors for x and load next descriptor with the x's + * subcredential so that it can accept connections for itself. */ + x_service.desc_current = service_descriptor_new(); + memset(x_service.desc_current->desc->subcredential.subcred, 'C',SUBCRED_LEN); + x_service.desc_next = service_descriptor_new(); + memcpy(&x_service.desc_next->desc->subcredential, &x_subcred, SUBCRED_LEN); + + /* Refresh OB keys */ + hs_ob_refresh_keys(&x_service); + + /* Create INTRODUCE1 from Alice to X through Z */ + memset(relay_payload, 0, sizeof(relay_payload)); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &z_subcred); + + /* Check that the payload was written successfully */ + tt_int_op(retval, OP_EQ, 0); + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + /* Deliver INTRODUCE1 to X even tho it carries Z's subcredential */ + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &z_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + replaycache_free(x_ip->replay_cache); + x_ip->replay_cache = replaycache_new(0, 0); + + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + + /* ************************************************************ */ + + /* Act III: + * + * Now send a direct INTRODUCE cell from Alice to X using X's subcredential + * and check that it succeeds even with onionbalance enabled. + */ + + /* Refresh OB keys (just to check for memleaks) */ + hs_ob_refresh_keys(&x_service); + + /* Create INTRODUCE1 from Alice to X using X's subcred. */ + memset(relay_payload, 0, sizeof(relay_payload)); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &x_subcred); + + /* Check that the payload was written successfully */ + tt_int_op(retval, OP_EQ, 0); + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + /* Send INTRODUCE1 to X with X's subcredential (should succeed) */ + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + /* ************************************************************ */ + + /* Act IV: + * + * Test cases where the INTRO2 cell should not be able to decode. + */ + + /* Try sending the exact same INTRODUCE2 cell again and see that the intro + * point replay cache triggers: */ + setup_full_capture_of_logs(LOG_WARN); + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, -1); + expect_log_msg_containing("with the same ENCRYPTED section"); + teardown_capture_of_logs(); + + /* Now cleanup the intro point replay cache but not the service replay cache + and see that this one triggers this time. */ + replaycache_free(x_ip->replay_cache); + x_ip->replay_cache = replaycache_new(0, 0); + setup_full_capture_of_logs(LOG_INFO); + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, -1); + expect_log_msg_containing("with same REND_COOKIE"); + teardown_capture_of_logs(); + + /* Now just to make sure cleanup both replay caches and make sure that the + cell gets through */ + replaycache_free(x_ip->replay_cache); + x_ip->replay_cache = replaycache_new(0, 0); + replaycache_free(x_service.state.replay_cache_rend_cookie); + x_service.state.replay_cache_rend_cookie = replaycache_new(0, 0); + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &x_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, 0); + + /* As a final thing, create an INTRODUCE1 cell from Alice to X using Y's + * subcred (should fail since Y is just another instance and not the frontend + * service!) */ + memset(relay_payload, 0, sizeof(relay_payload)); + retval = hs_circ_send_introduce1(intro_circ, &rend_circ, + alice_ip, &y_subcred); + tt_int_op(retval, OP_EQ, 0); + + /* Check that the payload was written successfully */ + tt_assert(!fast_mem_is_zero(relay_payload, sizeof(relay_payload))); + tt_int_op(relay_payload_len, OP_NE, 0); + + retval = hs_circ_handle_introduce2(&x_service, + intro_circ, x_ip, + &y_subcred, + (uint8_t*)relay_payload, relay_payload_len); + tt_int_op(retval, OP_EQ, -1); + + done: + /* Start cleaning up X */ + replaycache_free(x_service.state.replay_cache_rend_cookie); + smartlist_free(x_service.config.ob_master_pubkeys); + tor_free(x_service.ob_subcreds); + service_descriptor_free(x_service.desc_current); + service_descriptor_free(x_service.desc_next); + service_intro_point_free(x_ip); + + /* Clean up Alice */ + hs_desc_intro_point_free(alice_ip); + tor_free(rend_circ.hs_ident); + + if (fake_node) { + tor_free(fake_node->ri->onion_curve25519_pkey); + tor_free(fake_node->ri); + tor_free(fake_node); + } + + UNMOCK(build_state_get_exit_node); + UNMOCK(relay_send_command_from_edge_); + UNMOCK(node_get_link_specifier_smartlist); + UNMOCK(launch_rendezvous_point_circuit); +} + struct testcase_t hs_service_tests[] = { { "e2e_rend_circuit_setup", test_e2e_rend_circuit_setup, TT_FORK, NULL, NULL }, @@ -2194,7 +2545,7 @@ struct testcase_t hs_service_tests[] = { NULL, NULL }, { "rdv_circuit_opened", test_rdv_circuit_opened, TT_FORK, NULL, NULL }, - { "introduce2", test_introduce2, TT_FORK, + { "bad_introduce2", test_bad_introduce2, TT_FORK, NULL, NULL }, { "service_event", test_service_event, TT_FORK, NULL, NULL }, @@ -2212,6 +2563,7 @@ struct testcase_t hs_service_tests[] = { TT_FORK, NULL, NULL }, { "export_client_circuit_id", test_export_client_circuit_id, TT_FORK, NULL, NULL }, + { "intro2_handling", test_intro2_handling, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/master] Make n_subcredentials a size_t .
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit 901ed35709ac79e841b89d947a0d84526f4d8daf Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Feb 6 16:28:21 2020 +0200 Make n_subcredentials a size_t . Based on David's review. --- src/core/crypto/hs_ntor.c | 4 ++-- src/core/crypto/hs_ntor.h | 2 +- src/feature/hs/hs_cell.c | 4 ++-- src/feature/hs/hs_cell.h | 2 +- src/feature/hs/hs_circuit.c | 2 +- src/lib/ctime/di_ops.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/crypto/hs_ntor.c b/src/core/crypto/hs_ntor.c index 4bd11ef98..07bcdc566 100644 --- a/src/core/crypto/hs_ntor.c +++ b/src/core/crypto/hs_ntor.c @@ -471,7 +471,7 @@ hs_ntor_service_get_introduce1_keys_multi( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_keypair_t *intro_enc_keypair, const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, - int n_subcredentials, + size_t n_subcredentials, const hs_subcredential_t *subcredentials, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out) { @@ -499,7 +499,7 @@ hs_ntor_service_get_introduce1_keys_multi( secret_input); bad |= safe_mem_is_zero(secret_input, CURVE25519_OUTPUT_LEN); - for (int i = 0; i < n_subcredentials; ++i) { + for (unsigned i = 0; i < n_subcredentials; ++i) { /* Get ENC_KEY and MAC_KEY! */ get_introduce1_key_material(secret_input, &subcredentials[i], &hs_ntor_intro_cell_keys_out[i]); diff --git a/src/core/crypto/hs_ntor.h b/src/core/crypto/hs_ntor.h index 2ed357f02..9a975dd83 100644 --- a/src/core/crypto/hs_ntor.h +++ b/src/core/crypto/hs_ntor.h @@ -62,7 +62,7 @@ int hs_ntor_service_get_introduce1_keys_multi( const struct ed25519_public_key_t *intro_auth_pubkey, const struct curve25519_keypair_t *intro_enc_keypair, const struct curve25519_public_key_t *client_ephemeral_enc_pubkey, - int n_subcredentials, + size_t n_subcredentials, const hs_subcredential_t *subcredentials, hs_ntor_intro_cell_keys_t *hs_ntor_intro_cell_keys_out); diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 97a1691f1..dd5fefd7e 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -77,7 +77,7 @@ compute_introduce_mac(const uint8_t *encoded_cell, size_t encoded_cell_len, static hs_ntor_intro_cell_keys_t * get_introduce2_key_material(const ed25519_public_key_t *auth_key, const curve25519_keypair_t *enc_key, - int n_subcredentials, + size_t n_subcredentials, const hs_subcredential_t *subcredentials, const uint8_t *encrypted_section, curve25519_public_key_t *client_pk) @@ -787,7 +787,7 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, /* Validate MAC from the cell and our computed key material. The MAC field * in the cell is at the end of the encrypted section. */ intro_keys_result = tor_malloc_zero(sizeof(*intro_keys_result)); - for (int i = 0; i < data->n_subcredentials; ++i) { + for (unsigned i = 0; i < data->n_subcredentials; ++i) { uint8_t mac[DIGEST256_LEN]; /* The MAC field is at the very end of the ENCRYPTED section. */ diff --git a/src/feature/hs/hs_cell.h b/src/feature/hs/hs_cell.h index cc2e7b581..2b28c44c5 100644 --- a/src/feature/hs/hs_cell.h +++ b/src/feature/hs/hs_cell.h @@ -60,7 +60,7 @@ typedef struct hs_cell_introduce2_data_t { /** * Length of the subcredentials array below. **/ - int n_subcredentials; + size_t n_subcredentials; /** Array of n_subcredentials subcredentials for the service. Pointer * owned by the descriptor that owns the introduction point through which we * received the INTRO2 cell. */ diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 97507df9f..fdd226ba7 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -989,7 +989,7 @@ get_subcredential_for_handling_intro2_cell(const hs_service_t *service, } /* We are an onionbalance instance: */ - data->n_subcredentials = (int) service->n_ob_subcreds; + data->n_subcredentials = service->n_ob_subcreds; data->subcredentials = service->ob_subcreds; return 0; diff --git a/src/lib/ctime/di_ops.c b/src/lib/ctime/di_ops.c index 0859dd8be..e1ac0943f 100644 --- a/src/lib/ctime/di_ops.c +++ b/src/lib/ctime/di_ops.c @@ -281,7 +281,7 @@ select_array_member_cumulative_timei(const uint64_t *entries, int n_entries, } /** - * If s is true, then copy n bytes from src</d> to + * If s is true, then copy n bytes from src to * dest. Otherwise leave dest alone. * * This function behaves the same as

1 0

[tor/master] Unify INTRO2 handling codepaths in OB and normal cases.
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit c731988cb2ba2164d7557a95e3410c2e12f85bb8 Author: George Kadianakis <desnacked(a)riseup.net> Date: Wed Jan 22 13:57:56 2020 +0200 Unify INTRO2 handling codepaths in OB and normal cases. Now we use the exact same INTRO2 decrypt logic regardless of whether the service is an OB instance or not. The new get_subcredential_for_handling_intro2_cell() function is responsible for loading the right subcredentials in either case. --- src/feature/hs/hs_cell.c | 67 +++++---------------------------------------- src/feature/hs/hs_circuit.c | 46 ++++++++++++++++++++++++++++--- 2 files changed, 49 insertions(+), 64 deletions(-) diff --git a/src/feature/hs/hs_cell.c b/src/feature/hs/hs_cell.c index 4a9044c98..fce6fe022 100644 --- a/src/feature/hs/hs_cell.c +++ b/src/feature/hs/hs_cell.c @@ -821,44 +821,6 @@ get_introduce2_keys_and_verify_mac(hs_cell_introduce2_data_t *data, return intro_keys_result; } -/** Return the newly allocated intro keys using the given service - * configuration and INTRODUCE2 data for the cell encrypted section. - * - * Every onion balance configured master key will be tried. If NULL is - * returned, no hit was found for the onion balance keys. */ -static hs_ntor_intro_cell_keys_t * -get_intro2_keys_as_ob(const hs_service_t *service, - const hs_cell_introduce2_data_t *data, - const uint8_t *encrypted_section, - size_t encrypted_section_len) -{ - hs_ntor_intro_cell_keys_t *intro_keys = NULL; - - if (!service->ob_subcreds) { - /* We are _not_ an OB instance since no configured master onion key(s) - * were found and thus no subcredentials were built. */ - goto end; - } - - /* Copy current data into a new INTRO2 cell data. We will then change the - * subcredential in order to validate. */ - hs_cell_introduce2_data_t new_data = *data; - new_data.n_subcredentials = (int) service->n_ob_subcreds; - new_data.subcredentials = service->ob_subcreds; - - intro_keys = get_introduce2_keys_and_verify_mac(&new_data, - encrypted_section, - encrypted_section_len); - memwipe(&new_data, 0, sizeof(new_data)); - if (intro_keys) { - /* It validates. We have a hit as an onion balance instance. */ - goto end; - } - - end: - return intro_keys; -} - /** Parse the INTRODUCE2 cell using data which contains everything we need to * do so and contains the destination buffers of information we extract and * compute from the cell. Return 0 on success else a negative value. The @@ -920,29 +882,14 @@ hs_cell_parse_introduce2(hs_cell_introduce2_data_t *data, memcpy(&data->client_pk.public_key, encrypted_section, CURVE25519_PUBKEY_LEN); - /* If we are configured as an Onion Balance instance, we need to try - * validation with the configured master public keys given in the config - * file. This is because the master identity key and the blinded key is put - * in the INTRODUCE2 cell by the client thus it will never validate with - * this instance default public key. */ - if (hs_ob_service_is_instance(service)) { - intro_keys = get_intro2_keys_as_ob(service, data, - encrypted_section, - encrypted_section_len); - } + /* Get the right INTRODUCE2 ntor keys and verify the cell MAC */ + intro_keys = get_introduce2_keys_and_verify_mac(data, encrypted_section, + encrypted_section_len); if (!intro_keys) { - /* We are not an onion balance instance or no keys matched, fallback to - * our default values. */ - - /* Get the right INTRODUCE2 ntor keys and verify the cell MAC */ - intro_keys = get_introduce2_keys_and_verify_mac(data, encrypted_section, - encrypted_section_len); - if (!intro_keys) { - log_info(LD_REND, "Could not get valid INTRO2 keys on circuit %u " - "for service %s", TO_CIRCUIT(circ)->n_circ_id, - safe_str_client(service->onion_address)); - goto done; - } + log_warn(LD_REND, "Could not get valid INTRO2 keys on circuit %u " + "for service %s", TO_CIRCUIT(circ)->n_circ_id, + safe_str_client(service->onion_address)); + goto done; } { diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index 48a92962f..febeec473 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -19,6 +19,7 @@ #include "feature/client/circpathbias.h" #include "feature/hs/hs_cell.h" #include "feature/hs/hs_circuit.h" +#include "feature/hs/hs_ob.h" #include "feature/hs/hs_circuitmap.h" #include "feature/hs/hs_client.h" #include "feature/hs/hs_ident.h" @@ -958,6 +959,42 @@ hs_circ_handle_intro_established(const hs_service_t *service, return ret; } +/** + * Go into data and add the right subcredential to be able to handle + * this incoming cell. + * + * desc_subcred is the subcredential of the descriptor that corresponds + * to the intro point that received this intro request. This subcredential + * should be used if we are not an onionbalance instance. + * + * Return 0 if everything went well, or -1 in case of internal error. + */ +static int +get_subcredential_for_handling_intro2_cell(const hs_service_t *service, + hs_cell_introduce2_data_t *data, + const hs_subcredential_t *desc_subcred) +{ + /* Handle the simple case first: We are not an onionbalance instance and we + * should just use the regular descriptor subcredential */ + if (!hs_ob_service_is_instance(service)) { + data->n_subcredentials = 1; + data->subcredentials = desc_subcred; + return 0; + } + + /* This should not happen since we should have made onionbalance + * subcredentials when we created our descriptors. */ + if (BUG(!service->ob_subcreds)) { + return -1; + } + + /* We are an onionbalance instance: */ + data->n_subcredentials = (int) service->n_ob_subcreds; + data->subcredentials = service->ob_subcreds; + + return 0; +} + /** We just received an INTRODUCE2 cell on the established introduction circuit * circ. Handle the INTRODUCE2 payload of size payload_len for the given * circuit and service. This cell is associated with the intro point object ip @@ -983,15 +1020,16 @@ hs_circ_handle_introduce2(const hs_service_t *service, * parsed, decrypted and key material computed correctly. */ data.auth_pk = &ip->auth_key_kp.pubkey; data.enc_kp = &ip->enc_key_kp; - // XXXX We should replace these elements with something precomputed for - // XXXX the onionbalance case. - data.n_subcredentials = 1; - data.subcredentials = subcredential; data.payload = payload; data.payload_len = payload_len; data.link_specifiers = smartlist_new(); data.replay_cache = ip->replay_cache; + if (get_subcredential_for_handling_intro2_cell(service, + &data, subcredential)) { + goto done; + } + if (hs_cell_parse_introduce2(&data, circ, service) < 0) { goto done; }

1 0

[tor/master] Introduce an hs_ob_free_all() function.
by nickm＠torproject.org 24 Feb '20

24 Feb '20

commit 635f58bad23282e27fbc5833dbaae978dab25934 Author: George Kadianakis <desnacked(a)riseup.net> Date: Thu Jan 23 00:31:29 2020 +0200 Introduce an hs_ob_free_all() function. --- src/feature/hs/hs_common.c | 2 ++ src/feature/hs/hs_ob.c | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index b2b52c480..4639cdb68 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -22,6 +22,7 @@ #include "feature/hs/hs_client.h" #include "feature/hs/hs_common.h" #include "feature/hs/hs_dos.h" +#include "feature/hs/hs_ob.h" #include "feature/hs/hs_ident.h" #include "feature/hs/hs_service.h" #include "feature/hs_common/shared_random_client.h" @@ -1829,6 +1830,7 @@ hs_free_all(void) hs_service_free_all(); hs_cache_free_all(); hs_client_free_all(); + hs_ob_free_all(); } /** For the given origin circuit circ, decrement the number of rendezvous diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index 69fc51a8a..6a2e43f05 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -394,3 +394,10 @@ hs_ob_refresh_keys(hs_service_t *service) service->ob_subcreds = ob_subcreds; service->n_ob_subcreds = num_subcreds; } + +/** Free any memory allocated by the onionblance subsystem. */ +void +hs_ob_free_all(void) +{ + config_mgr_free(config_options_mgr); +}

1 0