tor-commits March 2019

tor-commits@lists.torproject.org

20 participants
3265 discussions

[tor/master] hs: Onion services put IPv6 addresses in service descriptors
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 6170d3fcf12f4345ec13561d0f444930f2f12e84 Author: teor <teor(a)torproject.org> Date: Tue Jul 31 14:30:17 2018 +1000 hs: Onion services put IPv6 addresses in service descriptors Rewrite service_intro_point_new() to take a node_t. Since node_get_link_specifier_smartlist() supports IPv6 link specifiers, this refactor adds IPv6 addresses to onion service descriptors. Part of 23576, implements 26992. --- changes/bug23576 | 7 ++++ src/feature/hs/hs_common.c | 18 +++++++- src/feature/hs/hs_service.c | 96 ++++++++----------------------------------- src/feature/hs/hs_service.h | 5 +-- src/test/test_hs_cell.c | 4 +- src/test/test_hs_intropoint.c | 4 +- src/test/test_hs_service.c | 2 +- 7 files changed, 48 insertions(+), 88 deletions(-) diff --git a/changes/bug23576 b/changes/bug23576 new file mode 100644 index 000000000..edcae02e5 --- /dev/null +++ b/changes/bug23576 @@ -0,0 +1,7 @@ + o Minor features (IPv6, v3 onion services): + - Make v3 onion services put IPv6 addresses in service + descriptors. Before this change, service descriptors only + contained IPv4 addressesd. Implements 26992. + o Code simplification and refactoring: + - Simplify v3 onion service link specifier handling code. + Fixes bug 23576; bugfix on 0.3.2.1-alpha. diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index ebe49f09a..5d8f54230 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -1697,6 +1697,12 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, tor_assert(lspecs); + if (smartlist_len(lspecs) == 0) { + log_fn(LOG_PROTOCOL_WARN, LD_REND, "Empty link specifier list."); + /* Return NULL. */ + goto done; + } + SMARTLIST_FOREACH_BEGIN(lspecs, const link_specifier_t *, ls) { switch (link_specifier_get_ls_type(ls)) { case LS_IPV4: @@ -1730,6 +1736,12 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, /* Legacy ID is mandatory, and we require IPv4. */ if (!have_v4 || !have_legacy_id) { + bool both = !have_v4 && !have_legacy_id; + log_fn(LOG_PROTOCOL_WARN, LD_REND, "Missing %s%s%s link specifier%s.", + !have_v4 ? "IPv4" : "", + both ? " and " : "", + !have_legacy_id ? "legacy ID" : "", + both ? "s" : ""); goto done; } @@ -1748,6 +1760,10 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, * release. */ } else { /* If we can't reach IPv4, return NULL. */ + log_fn(LOG_PROTOCOL_WARN, LD_REND, + "Received an IPv4 link specifier, " + "but the address is not reachable: %s:%u", + fmt_addr(&addr_v4), port_v4); goto done; } @@ -1755,7 +1771,7 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs, validate: /* We'll validate now that the address we've picked isn't a private one. If - * it is, are we allowing to extend to private address? */ + * it is, are we allowed to extend to private addresses? */ if (!extend_info_addr_is_allowed(&addr_v4)) { log_fn(LOG_PROTOCOL_WARN, LD_REND, "Requested address is private and we are not allowed to extend to " diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index b94dd9a48..6e4da8856 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -426,23 +426,16 @@ service_intro_point_free_void(void *obj) } /* Return a newly allocated service intro point and fully initialized from the - * given extend_info_t ei if non NULL. - * If is_legacy is true, we also generate the legacy key. - * If supports_ed25519_link_handshake_any is true, we add the relay's ed25519 - * key to the link specifiers. + * given node_t node, if non NULL. * - * If ei is NULL, returns a hs_service_intro_point_t with an empty link + * If node is NULL, returns a hs_service_intro_point_t with an empty link * specifier list and no onion key. (This is used for testing.) * On any other error, NULL is returned. * - * ei must be an extend_info_t containing an IPv4 address. (We will add supoort - * for IPv6 in a later release.) When calling extend_info_from_node(), pass - * 0 in for_direct_connection to make sure ei always has an IPv4 address. */ + * node must be an node_t with an IPv4 address. */ STATIC hs_service_intro_point_t * -service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, - unsigned int supports_ed25519_link_handshake_any) +service_intro_point_new(const node_t *node) { - hs_desc_link_specifier_t *ls; hs_service_intro_point_t *ip; ip = tor_malloc_zero(sizeof(*ip)); @@ -472,12 +465,17 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, ip->replay_cache = replaycache_new(0, 0); /* Initialize the base object. We don't need the certificate object. */ - ip->base.link_specifiers = smartlist_new(); + ip->base.link_specifiers = node_get_link_specifier_smartlist(node, 0); + + if (node == NULL) { + goto done; + } /* Generate the encryption key for this intro point. */ curve25519_keypair_generate(&ip->enc_key_kp, 0); - /* Figure out if this chosen node supports v3 or is legacy only. */ - if (is_legacy) { + /* Figure out if this chosen node supports v3 or is legacy only. + * NULL nodes are used in the unit tests. */ + if (!node_supports_ed25519_hs_intro(node)) { ip->base.is_only_legacy = 1; /* Legacy mode that is doesn't support v3+ with ed25519 auth key. */ ip->legacy_key = crypto_pk_new(); @@ -490,40 +488,9 @@ service_intro_point_new(const extend_info_t *ei, unsigned int is_legacy, } } - if (ei == NULL) { - goto done; - } - - /* We'll try to add all link specifiers. Legacy is mandatory. - * IPv4 or IPv6 is required, and we always send IPv4. */ - ls = hs_desc_link_specifier_new(ei, LS_IPV4); - /* It is impossible to have an extend info object without a v4. */ - if (BUG(!ls)) { - goto err; - } - smartlist_add(ip->base.link_specifiers, ls); - - ls = hs_desc_link_specifier_new(ei, LS_LEGACY_ID); - /* It is impossible to have an extend info object without an identity - * digest. */ - if (BUG(!ls)) { - goto err; - } - smartlist_add(ip->base.link_specifiers, ls); - - /* ed25519 identity key is optional for intro points. If the node supports - * ed25519 link authentication, we include it. */ - if (supports_ed25519_link_handshake_any) { - ls = hs_desc_link_specifier_new(ei, LS_ED25519_ID); - if (ls) { - smartlist_add(ip->base.link_specifiers, ls); - } - } - - /* IPv6 is not supported in this release. */ - - /* Finally, copy onion key from the extend_info_t object. */ - memcpy(&ip->onion_key, &ei->curve25519_onion_key, sizeof(ip->onion_key)); + /* Finally, copy onion key from the node. */ + memcpy(&ip->onion_key, node_get_curve25519_onion_key(node), + sizeof(ip->onion_key)); done: return ip; @@ -2106,7 +2073,6 @@ static hs_service_intro_point_t * pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes) { const node_t *node; - extend_info_t *info = NULL; hs_service_intro_point_t *ip = NULL; /* Normal 3-hop introduction point flags. */ router_crn_flags_t flags = CRN_NEED_UPTIME | CRN_NEED_DESC; @@ -2127,43 +2093,17 @@ pick_intro_point(unsigned int direct_conn, smartlist_t *exclude_nodes) * we don't want to use that node anymore. */ smartlist_add(exclude_nodes, (void *) node); - /* We do this to ease our life but also this call makes appropriate checks - * of the node object such as validating ntor support for instance. - * - * We must provide an extend_info for clients to connect over a 3-hop path, - * so we don't pass direct_conn here. */ - info = extend_info_from_node(node, 0); - if (BUG(info == NULL)) { - goto err; - } - - /* Let's do a basic sanity check here so that we don't end up advertising the - * ed25519 identity key of relays that don't actually support the link - * protocol */ - if (!node_supports_ed25519_link_authentication(node, 0)) { - tor_assert_nonfatal(ed25519_public_key_is_zero(&info->ed_identity)); - } else { - /* Make sure we *do* have an ed key if we support the link authentication. - * Sending an empty key would result in a failure to extend. */ - tor_assert_nonfatal(!ed25519_public_key_is_zero(&info->ed_identity)); - } + /* Create our objects and populate them with the node information. */ + ip = service_intro_point_new(node); - /* Create our objects and populate them with the node information. - * We don't care if the intro's link auth is compatible with us, because - * we are sending the ed25519 key to a remote client via the descriptor. */ - ip = service_intro_point_new(info, !node_supports_ed25519_hs_intro(node), - node_supports_ed25519_link_authentication(node, - 0)); if (ip == NULL) { goto err; } - log_info(LD_REND, "Picked intro point: %s", extend_info_describe(info)); - extend_info_free(info); + log_info(LD_REND, "Picked intro point: %s", node_describe(node)); return ip; err: service_intro_point_free(ip); - extend_info_free(info); return NULL; } diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index ec53f2f23..8d7f77321 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -369,10 +369,7 @@ STATIC hs_service_t *find_service(hs_service_ht *map, STATIC void remove_service(hs_service_ht *map, hs_service_t *service); STATIC int register_service(hs_service_ht *map, hs_service_t *service); /* Service introduction point functions. */ -STATIC hs_service_intro_point_t *service_intro_point_new( - const extend_info_t *ei, - unsigned int is_legacy, - unsigned int supports_ed25519_link_handshake_any); +STATIC hs_service_intro_point_t *service_intro_point_new(const node_t *node); STATIC void service_intro_point_free_(hs_service_intro_point_t *ip); #define service_intro_point_free(ip) \ FREE_AND_NULL(hs_service_intro_point_t, \ diff --git a/src/test/test_hs_cell.c b/src/test/test_hs_cell.c index 0c93f593c..6e00e8807 100644 --- a/src/test/test_hs_cell.c +++ b/src/test/test_hs_cell.c @@ -39,7 +39,7 @@ test_gen_establish_intro_cell(void *arg) attempt to parse it. */ { /* We only need the auth key pair here. */ - hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0); + hs_service_intro_point_t *ip = service_intro_point_new(NULL); /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ ret = hs_cell_build_establish_intro(circ_nonce, ip, buf); @@ -107,7 +107,7 @@ test_gen_establish_intro_cell_bad(void *arg) ed25519_sign_prefixed() function and make it fail. */ cell = trn_cell_establish_intro_new(); tt_assert(cell); - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, NULL); service_intro_point_free(ip); expect_log_msg_containing("Unable to make signature for " diff --git a/src/test/test_hs_intropoint.c b/src/test/test_hs_intropoint.c index 660f21ffd..b7163c5c1 100644 --- a/src/test/test_hs_intropoint.c +++ b/src/test/test_hs_intropoint.c @@ -50,7 +50,7 @@ new_establish_intro_cell(const char *circ_nonce, /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, buf); tt_i64_op(cell_len, OP_GT, 0); @@ -76,7 +76,7 @@ new_establish_intro_encoded_cell(const char *circ_nonce, uint8_t *cell_out) /* Auth key pair is generated in the constructor so we are all set for * using this IP object. */ - ip = service_intro_point_new(NULL, 0, 0); + ip = service_intro_point_new(NULL); tt_assert(ip); cell_len = hs_cell_build_establish_intro(circ_nonce, ip, cell_out); tt_i64_op(cell_len, OP_GT, 0); diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 43bf89438..e8bdcd86e 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -329,7 +329,7 @@ static hs_service_intro_point_t * helper_create_service_ip(void) { hs_desc_link_specifier_t *ls; - hs_service_intro_point_t *ip = service_intro_point_new(NULL, 0, 0); + hs_service_intro_point_t *ip = service_intro_point_new(NULL); tor_assert(ip); /* Add a first unused link specifier. */ ls = tor_malloc_zero(sizeof(*ls));

1 0

[tor/master] test/hs: minor hs test fixes
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 257cea8876e00b18c6791f86ebe5c24bab3b72bf Author: teor <teor(a)torproject.org> Date: Wed Jan 30 21:52:19 2019 +0100 test/hs: minor hs test fixes Cleanup some bugs discovered during 23576: * stop copying the first 20 characters of a 40-character hex string to a binary fingerprint * stop putting IPv6 addresses in a variable called "ipv4" * explain why we do a duplicate tt_int_op() to deliberately fail and print a value Fixes bug 29243; bugfix on 0.3.2.1-alpha. --- changes/bug29243 | 3 +++ src/test/hs_test_helpers.c | 30 ++++++++++++++---------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/changes/bug29243 b/changes/bug29243 new file mode 100644 index 000000000..b5694f756 --- /dev/null +++ b/changes/bug29243 @@ -0,0 +1,3 @@ + o Minor bugfixes (testing, v3 onion services): + - Fix some incorrect code in the v3 onion service unit tests. + Fixes bug 29243; bugfix on 0.3.2.1-alpha. diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index 1c47c7d7d..c57bdc730 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -24,34 +24,32 @@ hs_helper_build_intro_point(const ed25519_keypair_t *signing_kp, time_t now, tor_addr_t a; tor_addr_make_unspec(&a); link_specifier_t *ls_legacy = link_specifier_new(); - /* TODO: this name is wrong: it is sometimes an IPv6 address */ - link_specifier_t *ls_v4 = link_specifier_new(); + link_specifier_t *ls_ip = link_specifier_new(); link_specifier_set_ls_type(ls_legacy, LS_LEGACY_ID); - memcpy(link_specifier_getarray_un_legacy_id(ls_legacy), - /* TODO: this code is wrong: it copies hex into binary */ - "0299F268FCA9D55CD157976D39AE92B4B455B3A8", + memset(link_specifier_getarray_un_legacy_id(ls_legacy), 'C', link_specifier_getlen_un_legacy_id(ls_legacy)); int family = tor_addr_parse(&a, addr); switch (family) { case AF_INET: - link_specifier_set_ls_type(ls_v4, LS_IPV4); - link_specifier_set_un_ipv4_addr(ls_v4, tor_addr_to_ipv4h(&a)); - link_specifier_set_un_ipv4_port(ls_v4, 9001); + link_specifier_set_ls_type(ls_ip, LS_IPV4); + link_specifier_set_un_ipv4_addr(ls_ip, tor_addr_to_ipv4h(&a)); + link_specifier_set_un_ipv4_port(ls_ip, 9001); break; case AF_INET6: - link_specifier_set_ls_type(ls_v4, LS_IPV6); - memcpy(link_specifier_getarray_un_ipv6_addr(ls_v4), + link_specifier_set_ls_type(ls_ip, LS_IPV6); + memcpy(link_specifier_getarray_un_ipv6_addr(ls_ip), tor_addr_to_in6_addr8(&a), - link_specifier_getlen_un_ipv6_addr(ls_v4)); - link_specifier_set_un_ipv6_port(ls_v4, 9001); + link_specifier_getlen_un_ipv6_addr(ls_ip)); + link_specifier_set_un_ipv6_port(ls_ip, 9001); break; default: - /* Stop the test, not suppose to have an error. */ - /* TODO: just tt_fail(), because this code is confusing */ - tt_int_op(family, OP_EQ, AF_INET); + /* Stop the test, not supposed to have an error. + * Compare with -1 to show the actual family. + */ + tt_int_op(family, OP_EQ, -1); } smartlist_add(ip->link_specifiers, ls_legacy); - smartlist_add(ip->link_specifiers, ls_v4); + smartlist_add(ip->link_specifiers, ls_ip); } ret = ed25519_keypair_generate(&auth_kp, 0);

1 0

[tor/master] Merge branch 'bug23576-041-rebased-squashed'
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 61adcb22c53843c0fd12125d1a388dcd7b7e7df7 Merge: c01c6123f 680b2afd8 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 12 11:10:01 2019 -0400 Merge branch 'bug23576-041-rebased-squashed' changes/bug22781 | 4 + changes/bug23576 | 7 ++ changes/bug29243 | 3 + src/feature/hs/hs_cell.c | 9 +- src/feature/hs/hs_circuit.c | 86 +------------------ src/feature/hs/hs_client.c | 21 ++--- src/feature/hs/hs_common.c | 75 ++++++++++++----- src/feature/hs/hs_common.h | 4 +- src/feature/hs/hs_descriptor.c | 178 +++------------------------------------ src/feature/hs/hs_descriptor.h | 29 +------ src/feature/hs/hs_intropoint.c | 4 +- src/feature/hs/hs_service.c | 146 +++++++++----------------------- src/feature/hs/hs_service.h | 5 +- src/feature/nodelist/nodelist.c | 96 +++++++++++++++++++++ src/feature/nodelist/nodelist.h | 5 ++ src/test/hs_test_helpers.c | 89 ++++++++++++++------ src/test/test_hs_cell.c | 4 +- src/test/test_hs_client.c | 4 + src/test/test_hs_descriptor.c | 111 ------------------------ src/test/test_hs_intropoint.c | 4 +- src/test/test_hs_service.c | 22 ++--- src/trunnel/ed25519_cert.trunnel | 6 -- 22 files changed, 332 insertions(+), 580 deletions(-)

1 0

[tor/master] Revert "Walk back from requiring bash"
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 4773fa647434eba582f775cb371894b7e36cdb34 Author: rl1987 <rl1987(a)sdf.lonestar.org> Date: Sun Mar 10 18:16:58 2019 +0200 Revert "Walk back from requiring bash" This reverts commit c346eff223e94b5fbeb6e751a99393fc5f7dd4b0. --- src/test/test-network.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/test-network.sh b/src/test/test-network.sh index 372c8cbac..4d56e8380 100755 --- a/src/test/test-network.sh +++ b/src/test/test-network.sh @@ -1,4 +1,4 @@ -#!/bin/sh +#!/usr/bin/env bash # This script calls the equivalent script in chutney/tools @@ -18,6 +18,10 @@ ECHO="${ECHO:-echo}" # Output is prefixed with the name of the script myname=$(basename "$0") +# Save the arguments before we destroy them +# This might not preserve arguments with spaces in them +ORIGINAL_ARGS=( "$@" ) + # We need to find CHUTNEY_PATH, so that we can call the version of this script # in chutney/tools with the same arguments. We also need to respect --quiet. until [ -z "$1" ] @@ -95,7 +99,7 @@ if [ -d "$CHUTNEY_PATH" ] && [ -x "$TEST_NETWORK" ]; then # this may fail if some arguments have spaces in them # if so, set CHUTNEY_PATH before calling test-network.sh, and spaces # will be handled correctly - exec "$TEST_NETWORK" "$@" + exec "$TEST_NETWORK" "${ORIGINAL_ARGS[@]}" # $ORIGINAL_ARGS else $ECHO "$myname: Could not find tools/test-network.sh in CHUTNEY_PATH." $ECHO "$myname: Please update your chutney using 'git pull'."

1 0

[tor/master] Merge remote-tracking branch 'tor-github/pr/779'
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit c01c6123fa0ef207ca55f6ce9b4896b1f3de8b15 Merge: 1c9b62928 4773fa647 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 12 11:05:27 2019 -0400 Merge remote-tracking branch 'tor-github/pr/779' src/test/test-network.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

1 0

[tor/maint-0.4.0] sr: Free SRVs before replacing them in state_query_put_()
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 26e6f56023e749800d95bbc5669c60197d481314 Author: teor <teor(a)torproject.org> Date: Sat Mar 9 10:50:55 2019 +1000 sr: Free SRVs before replacing them in state_query_put_() Refactor the shared random state's memory management so that it actually takes ownership of the shared random value pointers. Fixes bug 29706; bugfix on 0.2.9.1-alpha. --- changes/bug29706_refactor | 4 ++++ src/or/shared_random.c | 2 +- src/or/shared_random.h | 3 ++- src/or/shared_random_state.c | 20 ++++++++++++++------ src/test/test_shared_random.c | 25 +++++++++++++++---------- 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/changes/bug29706_refactor b/changes/bug29706_refactor new file mode 100644 index 000000000..ba1d0c7ed --- /dev/null +++ b/changes/bug29706_refactor @@ -0,0 +1,4 @@ + o Minor bugfixes (memory management): + - Refactor the shared random state's memory management so that it actually + takes ownership of the shared random value pointers. + Fixes bug 29706; bugfix on 0.2.9.1-alpha. diff --git a/src/or/shared_random.c b/src/or/shared_random.c index 5f6b03f1b..c3e640977 100644 --- a/src/or/shared_random.c +++ b/src/or/shared_random.c @@ -112,7 +112,7 @@ static const char sr_flag_ns_str[] = "shared-rand-participate"; static int32_t num_srv_agreements_from_vote; /* Return a heap allocated copy of the SRV orig. */ -STATIC sr_srv_t * +sr_srv_t * srv_dup(const sr_srv_t *orig) { sr_srv_t *duplicate = NULL; diff --git a/src/or/shared_random.h b/src/or/shared_random.h index 9885934cc..68a101979 100644 --- a/src/or/shared_random.h +++ b/src/or/shared_random.h @@ -129,6 +129,8 @@ const char *sr_commit_get_rsa_fpr(const sr_commit_t *commit) void sr_compute_srv(void); sr_commit_t *sr_generate_our_commit(time_t timestamp, const authority_cert_t *my_rsa_cert); +sr_srv_t *srv_dup(const sr_srv_t *orig); + #ifdef SHARED_RANDOM_PRIVATE /* Encode */ @@ -146,7 +148,6 @@ STATIC sr_srv_t *get_majority_srv_from_votes(const smartlist_t *votes, int current); STATIC void save_commit_to_state(sr_commit_t *commit); -STATIC sr_srv_t *srv_dup(const sr_srv_t *orig); STATIC int commitments_are_the_same(const sr_commit_t *commit_one, const sr_commit_t *commit_two); STATIC int commit_is_authoritative(const sr_commit_t *commit, diff --git a/src/or/shared_random_state.c b/src/or/shared_random_state.c index f27eccafc..e99817106 100644 --- a/src/or/shared_random_state.c +++ b/src/or/shared_random_state.c @@ -92,6 +92,8 @@ static const config_format_t state_format = { &state_extra_var, }; +static void state_query_del_(sr_state_object_t obj_type, void *data); + /* Return a string representation of a protocol phase. */ STATIC const char * get_phase_str(sr_phase_t phase) @@ -883,7 +885,8 @@ state_query_get_(sr_state_object_t obj_type, const void *data) } /* Helper function: This handles the PUT state action using an - * obj_type and data needed for the action. */ + * obj_type and data needed for the action. + * PUT frees the previous data before replacing it, if needed. */ static void state_query_put_(sr_state_object_t obj_type, void *data) { @@ -892,13 +895,18 @@ state_query_put_(sr_state_object_t obj_type, void *data) { sr_commit_t *commit = data; tor_assert(commit); + /* commit_add_to_state() frees the old commit, if there is one */ commit_add_to_state(commit, sr_state); break; } case SR_STATE_OBJ_CURSRV: + /* Always free the old SRV */ + state_query_del_(SR_STATE_OBJ_CURSRV, NULL); sr_state->current_srv = (sr_srv_t *) data; break; case SR_STATE_OBJ_PREVSRV: + /* Always free the old SRV */ + state_query_del_(SR_STATE_OBJ_PREVSRV, NULL); sr_state->previous_srv = (sr_srv_t *) data; break; case SR_STATE_OBJ_VALID_AFTER: @@ -1021,16 +1029,16 @@ state_del_previous_srv(void) state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_PREVSRV, NULL, NULL); } -/* Rotate SRV value by freeing the previous value, assigning the current - * value to the previous one and nullifying the current one. */ +/* Rotate SRV value by setting the previous SRV to the current SRV, and + * clearing the current SRV. */ STATIC void state_rotate_srv(void) { /* First delete previous SRV from the state. Object will be freed. */ state_del_previous_srv(); - /* Set previous SRV with the current one. */ - sr_state_set_previous_srv(sr_state_get_current_srv()); - /* Nullify the current srv. */ + /* Set previous SRV to a copy of the current one. */ + sr_state_set_previous_srv(srv_dup(sr_state_get_current_srv())); + /* Free and NULL the current srv. */ sr_state_set_current_srv(NULL); } diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index 0a3c2e119..5d3e574fa 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -1013,6 +1013,7 @@ test_state_transition(void *arg) { sr_state_t *state = NULL; time_t now = time(NULL); + sr_srv_t *cur = NULL; (void) arg; @@ -1051,44 +1052,47 @@ test_state_transition(void *arg) /* Test SRV rotation in our state. */ { - const sr_srv_t *cur, *prev; test_sr_setup_srv(1); - cur = sr_state_get_current_srv(); + tt_assert(sr_state_get_current_srv()); + /* Take a copy of the data, because the state owns the pointer */ + cur = srv_dup(sr_state_get_current_srv()); tt_assert(cur); - /* After, current srv should be the previous and then set to NULL. */ + /* After, the previous SRV should be the same as the old current SRV, and + * the current SRV should be set to NULL */ state_rotate_srv(); - prev = sr_state_get_previous_srv(); - tt_assert(prev == cur); + tt_mem_op(sr_state_get_previous_srv(), OP_EQ, cur, sizeof(sr_srv_t)); tt_assert(!sr_state_get_current_srv()); sr_state_clean_srvs(); + tor_free(cur); } /* New protocol run. */ { - const sr_srv_t *cur; /* Setup some new SRVs so we can confirm that a new protocol run * actually makes them rotate and compute new ones. */ test_sr_setup_srv(1); - cur = sr_state_get_current_srv(); - tt_assert(cur); + tt_assert(sr_state_get_current_srv()); + /* Take a copy of the data, because the state owns the pointer */ + cur = srv_dup(sr_state_get_current_srv()); set_sr_phase(SR_PHASE_REVEAL); MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m); new_protocol_run(now); UNMOCK(get_my_v3_authority_cert); /* Rotation happened. */ - tt_assert(sr_state_get_previous_srv() == cur); + tt_mem_op(sr_state_get_previous_srv(), OP_EQ, cur, sizeof(sr_srv_t)); /* We are going into COMMIT phase so we had to rotate our SRVs. Usually * our current SRV would be NULL but a new protocol run should make us * compute a new SRV. */ tt_assert(sr_state_get_current_srv()); /* Also, make sure we did change the current. */ - tt_assert(sr_state_get_current_srv() != cur); + tt_mem_op(sr_state_get_current_srv(), OP_NE, cur, sizeof(sr_srv_t)); /* We should have our commitment alone. */ tt_int_op(digestmap_size(state->commits), ==, 1); tt_int_op(state->n_reveal_rounds, ==, 0); tt_int_op(state->n_commit_rounds, ==, 0); /* 46 here since we were at 45 just before. */ tt_u64_op(state->n_protocol_runs, ==, 46); + tor_free(cur); } /* Cleanup of SRVs. */ @@ -1099,6 +1103,7 @@ test_state_transition(void *arg) } done: + tor_free(cur); sr_state_free(); }

1 0

[tor/maint-0.4.0] sr: rename srv_dup() to sr_srv_dup()
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 593fde930fa52e0f4ed7aaf70c4ea9e4615d2a0e Author: teor <teor(a)torproject.org> Date: Sat Mar 9 11:48:05 2019 +1000 sr: rename srv_dup() to sr_srv_dup() --- src/or/shared_random.c | 6 +++--- src/or/shared_random.h | 2 +- src/or/shared_random_state.c | 2 +- src/test/test_shared_random.c | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/or/shared_random.c b/src/or/shared_random.c index c3e640977..ddb6de1a5 100644 --- a/src/or/shared_random.c +++ b/src/or/shared_random.c @@ -113,7 +113,7 @@ static int32_t num_srv_agreements_from_vote; /* Return a heap allocated copy of the SRV orig. */ sr_srv_t * -srv_dup(const sr_srv_t *orig) +sr_srv_dup(const sr_srv_t *orig) { sr_srv_t *duplicate = NULL; @@ -1318,8 +1318,8 @@ sr_act_post_consensus(const networkstatus_t *consensus) * decided by the majority. */ sr_state_unset_fresh_srv(); /* Set the SR values from the given consensus. */ - sr_state_set_previous_srv(srv_dup(consensus->sr_info.previous_srv)); - sr_state_set_current_srv(srv_dup(consensus->sr_info.current_srv)); + sr_state_set_previous_srv(sr_srv_dup(consensus->sr_info.previous_srv)); + sr_state_set_current_srv(sr_srv_dup(consensus->sr_info.current_srv)); } /* Prepare our state so that it's ready for the next voting period. */ diff --git a/src/or/shared_random.h b/src/or/shared_random.h index 68a101979..c640470c1 100644 --- a/src/or/shared_random.h +++ b/src/or/shared_random.h @@ -129,7 +129,7 @@ const char *sr_commit_get_rsa_fpr(const sr_commit_t *commit) void sr_compute_srv(void); sr_commit_t *sr_generate_our_commit(time_t timestamp, const authority_cert_t *my_rsa_cert); -sr_srv_t *srv_dup(const sr_srv_t *orig); +sr_srv_t *sr_srv_dup(const sr_srv_t *orig); #ifdef SHARED_RANDOM_PRIVATE diff --git a/src/or/shared_random_state.c b/src/or/shared_random_state.c index e99817106..50c6d3dd7 100644 --- a/src/or/shared_random_state.c +++ b/src/or/shared_random_state.c @@ -1037,7 +1037,7 @@ state_rotate_srv(void) /* First delete previous SRV from the state. Object will be freed. */ state_del_previous_srv(); /* Set previous SRV to a copy of the current one. */ - sr_state_set_previous_srv(srv_dup(sr_state_get_current_srv())); + sr_state_set_previous_srv(sr_srv_dup(sr_state_get_current_srv())); /* Free and NULL the current srv. */ sr_state_set_current_srv(NULL); } diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index 5d3e574fa..93125e513 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -932,7 +932,7 @@ test_utils(void *arg) { (void) arg; - /* Testing srv_dup(). */ + /* Testing sr_srv_dup(). */ { sr_srv_t *srv = NULL, *dup_srv = NULL; const char *srv_value = @@ -940,7 +940,7 @@ test_utils(void *arg) srv = tor_malloc_zero(sizeof(*srv)); srv->num_reveals = 42; memcpy(srv->value, srv_value, sizeof(srv->value)); - dup_srv = srv_dup(srv); + dup_srv = sr_srv_dup(srv); tt_assert(dup_srv); tt_u64_op(dup_srv->num_reveals, ==, srv->num_reveals); tt_mem_op(dup_srv->value, OP_EQ, srv->value, sizeof(srv->value)); @@ -1055,7 +1055,7 @@ test_state_transition(void *arg) test_sr_setup_srv(1); tt_assert(sr_state_get_current_srv()); /* Take a copy of the data, because the state owns the pointer */ - cur = srv_dup(sr_state_get_current_srv()); + cur = sr_srv_dup(sr_state_get_current_srv()); tt_assert(cur); /* After, the previous SRV should be the same as the old current SRV, and * the current SRV should be set to NULL */ @@ -1073,7 +1073,7 @@ test_state_transition(void *arg) test_sr_setup_srv(1); tt_assert(sr_state_get_current_srv()); /* Take a copy of the data, because the state owns the pointer */ - cur = srv_dup(sr_state_get_current_srv()); + cur = sr_srv_dup(sr_state_get_current_srv()); set_sr_phase(SR_PHASE_REVEAL); MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m); new_protocol_run(now);

1 0

[tor/maint-0.4.0] sr: BUG() on NULL sr_state before doing a state_query_*()
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 9eeff921ae7b786d960ea4286d5bba56edaef47f Author: teor <teor(a)torproject.org> Date: Tue Mar 12 11:09:48 2019 +1000 sr: BUG() on NULL sr_state before doing a state_query_*() Part of #29706. --- src/or/shared_random_state.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/or/shared_random_state.c b/src/or/shared_random_state.c index 22a503b89..5011b5c7f 100644 --- a/src/or/shared_random_state.c +++ b/src/or/shared_random_state.c @@ -857,6 +857,9 @@ state_query_get_commit(const char *rsa_fpr) static void * state_query_get_(sr_state_object_t obj_type, const void *data) { + if (BUG(!sr_state)) + return NULL; + void *obj = NULL; switch (obj_type) { @@ -890,6 +893,9 @@ state_query_get_(sr_state_object_t obj_type, const void *data) static void state_query_put_(sr_state_object_t obj_type, void *data) { + if (BUG(!sr_state)) + return; + switch (obj_type) { case SR_STATE_OBJ_COMMIT: { @@ -939,6 +945,9 @@ state_query_put_(sr_state_object_t obj_type, void *data) static void state_query_del_all_(sr_state_object_t obj_type) { + if (BUG(!sr_state)) + return; + switch (obj_type) { case SR_STATE_OBJ_COMMIT: { @@ -967,6 +976,9 @@ state_query_del_(sr_state_object_t obj_type, void *data) { (void) data; + if (BUG(!sr_state)) + return; + switch (obj_type) { case SR_STATE_OBJ_PREVSRV: tor_free(sr_state->previous_srv);

1 0

[tor/maint-0.4.0] sr: Check for replacing a SRV pointer with the same pointer
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 0cca55411074a78096dab1b84a4781e790e9aece Author: teor <teor(a)torproject.org> Date: Tue Mar 12 10:24:34 2019 +1000 sr: Check for replacing a SRV pointer with the same pointer Check if the new pointer is the same as the old one: if it is, it's probably a bug: * the caller may have confused current and previous, or * they may have forgotten to sr_srv_dup(). Putting NULL multiple times is allowed. Part of 29706. --- src/or/shared_random_state.c | 32 +++++-- src/test/test_shared_random.c | 212 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 230 insertions(+), 14 deletions(-) diff --git a/src/or/shared_random_state.c b/src/or/shared_random_state.c index 50c6d3dd7..22a503b89 100644 --- a/src/or/shared_random_state.c +++ b/src/or/shared_random_state.c @@ -900,14 +900,26 @@ state_query_put_(sr_state_object_t obj_type, void *data) break; } case SR_STATE_OBJ_CURSRV: - /* Always free the old SRV */ - state_query_del_(SR_STATE_OBJ_CURSRV, NULL); - sr_state->current_srv = (sr_srv_t *) data; + /* Check if the new pointer is the same as the old one: if it is, it's + * probably a bug. The caller may have confused current and previous, + * or they may have forgotten to sr_srv_dup(). + * Putting NULL multiple times is allowed. */ + if (!BUG(data && sr_state->current_srv == (sr_srv_t *) data)) { + /* We own the old SRV, so we need to free it. */ + state_query_del_(SR_STATE_OBJ_CURSRV, NULL); + sr_state->current_srv = (sr_srv_t *) data; + } break; case SR_STATE_OBJ_PREVSRV: - /* Always free the old SRV */ - state_query_del_(SR_STATE_OBJ_PREVSRV, NULL); - sr_state->previous_srv = (sr_srv_t *) data; + /* Check if the new pointer is the same as the old one: if it is, it's + * probably a bug. The caller may have confused current and previous, + * or they may have forgotten to sr_srv_dup(). + * Putting NULL multiple times is allowed. */ + if (!BUG(data && sr_state->previous_srv == (sr_srv_t *) data)) { + /* We own the old SRV, so we need to free it. */ + state_query_del_(SR_STATE_OBJ_PREVSRV, NULL); + sr_state->previous_srv = (sr_srv_t *) data; + } break; case SR_STATE_OBJ_VALID_AFTER: sr_state->valid_after = *((time_t *) data); @@ -1059,7 +1071,9 @@ sr_state_get_phase(void) return *(sr_phase_t *) ptr; } -/* Return the previous SRV value from our state. Value CAN be NULL. */ +/* Return the previous SRV value from our state. Value CAN be NULL. + * The state object owns the SRV, so the calling code should not free the SRV. + * Use sr_srv_dup() if you want to keep a copy of the SRV. */ const sr_srv_t * sr_state_get_previous_srv(void) { @@ -1078,7 +1092,9 @@ sr_state_set_previous_srv(const sr_srv_t *srv) NULL); } -/* Return the current SRV value from our state. Value CAN be NULL. */ +/* Return the current SRV value from our state. Value CAN be NULL. + * The state object owns the SRV, so the calling code should not free the SRV. + * Use sr_srv_dup() if you want to keep a copy of the SRV. */ const sr_srv_t * sr_state_get_current_srv(void) { diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index 93125e513..c81b6ec2d 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -38,7 +38,8 @@ trusteddirserver_get_by_v3_auth_digest_m(const char *digest) } /* Setup a minimal dirauth environment by initializing the SR state and - * making sure the options are set to be an authority directory. */ + * making sure the options are set to be an authority directory. + * You must only call this function once per process. */ static void init_authority_state(void) { @@ -451,7 +452,9 @@ test_encoding(void *arg) /** Setup some SRVs in our SR state. If also_current is set, then set * both current and previous SRVs. - * Helper of test_vote() and test_sr_compute_srv(). */ + * Helper of test_vote() and test_sr_compute_srv(). + * You must call sr_state_free() to free the state at the end of each test + * function (on pass or fail). */ static void test_sr_setup_srv(int also_current) { @@ -927,8 +930,9 @@ test_sr_get_majority_srv_from_votes(void *arg) smartlist_free(votes); } +/* Test utils that don't depend on authority state */ static void -test_utils(void *arg) +test_utils_general(void *arg) { (void) arg; @@ -991,9 +995,19 @@ test_utils(void *arg) tt_str_op(get_phase_str(SR_PHASE_COMMIT), ==, "commit"); } + done: + return; +} + +/* Test utils that depend on authority state */ +static void +test_utils_auth(void *arg) +{ + (void)arg; + init_authority_state(); + /* Testing phase transition */ { - init_authority_state(); set_sr_phase(SR_PHASE_COMMIT); tt_int_op(is_phase_transition(SR_PHASE_REVEAL), ==, 1); tt_int_op(is_phase_transition(SR_PHASE_COMMIT), ==, 0); @@ -1004,8 +1018,193 @@ test_utils(void *arg) tt_int_op(is_phase_transition(42), ==, 1); } + /* Testing get, set, delete, clean SRVs */ + + { + /* Just set the previous SRV */ + test_sr_setup_srv(0); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + } + + { + /* Delete the SRVs one at a time */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And in the opposite order */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And both at once */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And do the gets and sets multiple times */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + state_del_previous_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + state_del_current_srv(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + } + + { + /* Now set the SRVs to NULL instead */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_current_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + sr_state_set_previous_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And in the opposite order */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_previous_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_current_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And both at once */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_clean_srvs(); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + + /* And do the gets and sets multiple times */ + test_sr_setup_srv(1); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_previous_srv(NULL); + sr_state_set_previous_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + sr_state_set_current_srv(NULL); + sr_state_set_previous_srv(NULL); + sr_state_set_current_srv(NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); + } + + { + /* Now copy the values across */ + test_sr_setup_srv(1); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Check that the content is different */ + tt_mem_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Set the current to the previous: the protocol goes the other way */ + sr_state_set_current_srv(sr_srv_dup(sr_state_get_previous_srv())); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Check that the content is the same */ + tt_mem_op(sr_state_get_previous_srv(), OP_EQ, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + } + + { + /* Now copy a value onto itself */ + test_sr_setup_srv(1); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Take a copy of the old value */ + sr_srv_t old_current_srv; + memcpy(&old_current_srv, sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Check that the content is different */ + tt_mem_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Set the current to the current: the protocol never replaces an SRV with + * the same value */ + sr_state_set_current_srv(sr_srv_dup(sr_state_get_current_srv())); + /* Check that the pointers are non-NULL, and different from each other */ + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL); + tt_ptr_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv()); + /* Check that the content is different between current and previous */ + tt_mem_op(sr_state_get_previous_srv(), OP_NE, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + /* Check that the content is the same as the old content */ + tt_mem_op(&old_current_srv, OP_EQ, + sr_state_get_current_srv(), sizeof(sr_srv_t)); + } + + /* I don't think we can say "expect a BUG()" in our tests. */ +#if 0 + { + /* Now copy a value onto itself without sr_srv_dup(). + * This should fail with a BUG() warning. */ + test_sr_setup_srv(1); + sr_state_set_current_srv(sr_state_get_current_srv()); + sr_state_set_previous_srv(sr_state_get_previous_srv()); + } +#endif + done: - return; + sr_state_free(); } static void @@ -1293,7 +1492,8 @@ struct testcase_t sr_tests[] = { { "sr_compute_srv", test_sr_compute_srv, TT_FORK, NULL, NULL }, { "sr_get_majority_srv_from_votes", test_sr_get_majority_srv_from_votes, TT_FORK, NULL, NULL }, - { "utils", test_utils, TT_FORK, NULL, NULL }, + { "utils_general", test_utils_general, TT_FORK, NULL, NULL }, + { "utils_auth", test_utils_auth, TT_FORK, NULL, NULL }, { "state_transition", test_state_transition, TT_FORK, NULL, NULL }, { "state_update", test_state_update, TT_FORK, NULL, NULL },

1 0

[tor/maint-0.4.0] test/sr: update sr_state_free() to sr_state_free_all()
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit dfc3e552a3b7617add0bd576a5c34dca59f42649 Author: teor <teor(a)torproject.org> Date: Tue Mar 12 11:34:52 2019 +1000 test/sr: update sr_state_free() to sr_state_free_all() The function name changed between 0.2.9 and 0.3.4. --- src/test/test_shared_random.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index 45f0e1db7..991d73128 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -548,7 +548,7 @@ test_encoding(void *arg) /** Setup some SRVs in our SR state. If also_current is set, then set * both current and previous SRVs. * Helper of test_vote() and test_sr_compute_srv(). - * You must call sr_state_free() to free the state at the end of each test + * You must call sr_state_free_all() to free the state at the end of each test * function (on pass or fail). */ static void test_sr_setup_srv(int also_current) @@ -1299,7 +1299,7 @@ test_utils_auth(void *arg) #endif done: - sr_state_free(); + sr_state_free_all(); } static void

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

tor-commits March 2019