March 2019 - tor-commits - lists.torproject.org

[tor/master] 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] Merge remote-tracking branch 'tor-github/pr/776' into maint-0.4.0
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 9c9214f2c9f4e9511a2e09238e1cc90d81ae02de Merge: a9c84bfd3 dfc3e552a Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 12 11:03:37 2019 -0400 Merge remote-tracking branch 'tor-github/pr/776' into maint-0.4.0 changes/bug29706_minimal | 4 + changes/bug29706_refactor | 4 + src/feature/dirauth/shared_random.c | 8 +- src/feature/dirauth/shared_random.h | 2 +- src/feature/dirauth/shared_random_state.c | 60 ++++++-- src/feature/dirauth/shared_random_state.h | 2 + src/test/test_shared_random.c | 245 +++++++++++++++++++++++++++--- 7 files changed, 290 insertions(+), 35 deletions(-) diff --cc src/feature/dirauth/shared_random.c index 34b228325,000000000..137c49800 mode 100644,000000..100644 --- a/src/feature/dirauth/shared_random.c +++ b/src/feature/dirauth/shared_random.c @@@ -1,1291 -1,0 +1,1291 @@@ +/* Copyright (c) 2016-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file shared_random.c + * + * \brief Functions and data structure needed to accomplish the shared + * random protocol as defined in proposal #250. + * + * \details + * + * This file implements the dirauth-only commit-and-reveal protocol specified + * by proposal #250. The protocol has two phases (sr_phase_t): the commitment + * phase and the reveal phase (see get_sr_protocol_phase()). + * + * During the protocol, directory authorities keep state in memory (using + * sr_state_t) and in disk (using sr_disk_state_t). The synchronization between + * these two data structures happens in disk_state_update() and + * disk_state_parse(). + * + * Here is a rough protocol outline: + * + * 1) In the beginning of the commitment phase, dirauths generate a + * commitment/reveal value for the current protocol run (see + * new_protocol_run() and sr_generate_our_commit()). + * + * 2) During voting, dirauths publish their commits in their votes + * depending on the current phase. Dirauths also include the two + * latest shared random values (SRV) in their votes. + * (see sr_get_string_for_vote()) + * + * 3) Upon receiving a commit from a vote, authorities parse it, verify + * it, and attempt to save any new commitment or reveal information in + * their state file (see extract_shared_random_commits() and + * sr_handle_received_commits()). They also parse SRVs from votes to + * decide which SRV should be included in the final consensus (see + * extract_shared_random_srvs()). + * + * 3) After voting is done, we count the SRVs we extracted from the votes, + * to find the one voted by the majority of dirauths which should be + * included in the final consensus (see get_majority_srv_from_votes()). + * If an appropriate SRV is found, it is embedded in the consensus (see + * sr_get_string_for_consensus()). + * + * 4) At the end of the reveal phase, dirauths compute a fresh SRV for the + * day using the active commits (see sr_compute_srv()). This new SRV + * is embedded in the votes as described above. + * + * Some more notes: + * + * - To support rebooting authorities and to avoid double voting, each dirauth + * saves the current state of the protocol on disk so that it can resume + * normally in case of reboot. The disk state (sr_disk_state_t) is managed by + * shared_random_state.c:state_query() and we go to extra lengths to ensure + * that the state is flushed on disk everytime we receive any useful + * information like commits or SRVs. + * + * - When we receive a commit from a vote, we examine it to see if it's useful + * to us and whether it's appropriate to receive it according to the current + * phase of the protocol (see should_keep_commit()). If the commit is useful + * to us, we save it in our disk state using save_commit_to_state(). When we + * receive the reveal information corresponding to a commitment, we verify + * that they indeed match using verify_commit_and_reveal(). + * + * - We treat consensuses as the ground truth, so everytime we generate a new + * consensus we update our SR state accordingly even if our local view was + * different (see sr_act_post_consensus()). + * + * - After a consensus has been composed, the SR protocol state gets prepared + * for the next voting session using sr_state_update(). That function takes + * care of housekeeping and also rotates the SRVs and commits in case a new + * protocol run is coming up. We also call sr_state_update() on bootup (in + * sr_state_init()), to prepare the state for the very first voting session. + * + * Terminology: + * + * - "Commitment" is the commitment value of the commit-and-reveal protocol. + * + * - "Reveal" is the reveal value of the commit-and-reveal protocol. + * + * - "Commit" is a struct (sr_commit_t) that contains a commitment value and + * optionally also a corresponding reveal value. + * + * - "SRV" is the Shared Random Value that gets generated as the result of the + * commit-and-reveal protocol. + **/ + +#define SHARED_RANDOM_PRIVATE + +#include "core/or/or.h" +#include "feature/dirauth/shared_random.h" +#include "app/config/config.h" +#include "app/config/confparse.h" +#include "lib/crypt_ops/crypto_rand.h" +#include "lib/crypt_ops/crypto_util.h" +#include "feature/nodelist/networkstatus.h" +#include "feature/relay/router.h" +#include "feature/relay/routerkeys.h" +#include "feature/nodelist/dirlist.h" +#include "feature/hs_common/shared_random_client.h" +#include "feature/dirauth/shared_random_state.h" +#include "feature/dircommon/voting_schedule.h" + +#include "feature/dirauth/dirvote.h" +#include "feature/dirauth/authmode.h" + +#include "feature/nodelist/authority_cert_st.h" +#include "feature/nodelist/networkstatus_st.h" + +/* String prefix of shared random values in votes/consensuses. */ +static const char previous_srv_str[] = "shared-rand-previous-value"; +static const char current_srv_str[] = "shared-rand-current-value"; +static const char commit_ns_str[] = "shared-rand-commit"; +static const char sr_flag_ns_str[] = "shared-rand-participate"; + +/* The value of the consensus param AuthDirNumSRVAgreements found in the + * vote. This is set once the consensus creation subsystem requests the + * SRV(s) that should be put in the consensus. We use this value to decide + * if we keep or not an SRV. */ +static int32_t num_srv_agreements_from_vote; + +/* Return a heap allocated copy of the SRV orig. */ - STATIC sr_srv_t * - srv_dup(const sr_srv_t *orig) ++sr_srv_t * ++sr_srv_dup(const sr_srv_t *orig) +{ + sr_srv_t *duplicate = NULL; + + if (!orig) { + return NULL; + } + + duplicate = tor_malloc_zero(sizeof(sr_srv_t)); + duplicate->num_reveals = orig->num_reveals; + memcpy(duplicate->value, orig->value, sizeof(duplicate->value)); + return duplicate; +} + +/* Allocate a new commit object and initializing it with rsa_identity + * that MUST be provided. The digest algorithm is set to the default one + * that is supported. The rest is uninitialized. This never returns NULL. */ +static sr_commit_t * +commit_new(const char *rsa_identity) +{ + sr_commit_t *commit; + + tor_assert(rsa_identity); + + commit = tor_malloc_zero(sizeof(*commit)); + commit->alg = SR_DIGEST_ALG; + memcpy(commit->rsa_identity, rsa_identity, sizeof(commit->rsa_identity)); + base16_encode(commit->rsa_identity_hex, sizeof(commit->rsa_identity_hex), + commit->rsa_identity, sizeof(commit->rsa_identity)); + return commit; +} + +/* Issue a log message describing commit. */ +static void +commit_log(const sr_commit_t *commit) +{ + tor_assert(commit); + + log_debug(LD_DIR, "SR: Commit from %s", sr_commit_get_rsa_fpr(commit)); + log_debug(LD_DIR, "SR: Commit: [TS: %" PRIu64 "] [Encoded: %s]", + commit->commit_ts, commit->encoded_commit); + log_debug(LD_DIR, "SR: Reveal: [TS: %" PRIu64 "] [Encoded: %s]", + commit->reveal_ts, safe_str(commit->encoded_reveal)); +} + +/* Make sure that the commitment and reveal information in commit + * match. If they match return 0, return -1 otherwise. This function MUST be + * used everytime we receive a new reveal value. Furthermore, the commit + * object MUST have a reveal value and the hash of the reveal value. */ +STATIC int +verify_commit_and_reveal(const sr_commit_t *commit) +{ + tor_assert(commit); + + log_debug(LD_DIR, "SR: Validating commit from authority %s", + sr_commit_get_rsa_fpr(commit)); + + /* Check that the timestamps match. */ + if (commit->commit_ts != commit->reveal_ts) { + log_warn(LD_BUG, "SR: Commit timestamp %" PRIu64 " doesn't match reveal " + "timestamp %" PRIu64, commit->commit_ts, + commit->reveal_ts); + goto invalid; + } + + /* Verify that the hashed_reveal received in the COMMIT message, matches + * the reveal we just received. */ + { + /* We first hash the reveal we just received. */ + char received_hashed_reveal[sizeof(commit->hashed_reveal)]; + + /* Only sha3-256 is supported. */ + if (commit->alg != SR_DIGEST_ALG) { + goto invalid; + } + + /* Use the invariant length since the encoded reveal variable has an + * extra byte for the NUL terminated byte. */ + if (crypto_digest256(received_hashed_reveal, commit->encoded_reveal, + SR_REVEAL_BASE64_LEN, commit->alg) < 0) { + /* Unable to digest the reveal blob, this is unlikely. */ + goto invalid; + } + + /* Now compare that with the hashed_reveal we received in COMMIT. */ + if (fast_memneq(received_hashed_reveal, commit->hashed_reveal, + sizeof(received_hashed_reveal))) { + log_warn(LD_BUG, "SR: Received reveal value from authority %s " + "doesn't match the commit value.", + sr_commit_get_rsa_fpr(commit)); + goto invalid; + } + } + + return 0; + invalid: + return -1; +} + +/* Return true iff the commit contains an encoded reveal value. */ +STATIC int +commit_has_reveal_value(const sr_commit_t *commit) +{ + return !tor_mem_is_zero(commit->encoded_reveal, + sizeof(commit->encoded_reveal)); +} + +/* Parse the encoded commit. The format is: + * base64-encode( TIMESTAMP || H(REVEAL) ) + * + * If successfully decoded and parsed, commit is updated and 0 is returned. + * On error, return -1. */ +STATIC int +commit_decode(const char *encoded, sr_commit_t *commit) +{ + int decoded_len = 0; + size_t offset = 0; + char b64_decoded[SR_COMMIT_LEN]; + + tor_assert(encoded); + tor_assert(commit); + + if (strlen(encoded) > SR_COMMIT_BASE64_LEN) { + /* This means that if we base64 decode successfully the reveiced commit, + * we'll end up with a bigger decoded commit thus unusable. */ + goto error; + } + + /* Decode our encoded commit. Let's be careful here since _encoded_ is + * coming from the network in a dirauth vote so we expect nothing more + * than the base64 encoded length of a commit. */ + decoded_len = base64_decode(b64_decoded, sizeof(b64_decoded), + encoded, strlen(encoded)); + if (decoded_len < 0) { + log_warn(LD_BUG, "SR: Commit from authority %s can't be decoded.", + sr_commit_get_rsa_fpr(commit)); + goto error; + } + + if (decoded_len != SR_COMMIT_LEN) { + log_warn(LD_BUG, "SR: Commit from authority %s decoded length doesn't " + "match the expected length (%d vs %u).", + sr_commit_get_rsa_fpr(commit), decoded_len, + (unsigned)SR_COMMIT_LEN); + goto error; + } + + /* First is the timestamp (8 bytes). */ + commit->commit_ts = tor_ntohll(get_uint64(b64_decoded)); + offset += sizeof(uint64_t); + /* Next is hashed reveal. */ + memcpy(commit->hashed_reveal, b64_decoded + offset, + sizeof(commit->hashed_reveal)); + /* Copy the base64 blob to the commit. Useful for voting. */ + strlcpy(commit->encoded_commit, encoded, sizeof(commit->encoded_commit)); + + return 0; + + error: + return -1; +} + +/* Parse the b64 blob at encoded containing reveal information and + * store the information in-place in commit. Return 0 on success else + * a negative value. */ +STATIC int +reveal_decode(const char *encoded, sr_commit_t *commit) +{ + int decoded_len = 0; + char b64_decoded[SR_REVEAL_LEN]; + + tor_assert(encoded); + tor_assert(commit); + + if (strlen(encoded) > SR_REVEAL_BASE64_LEN) { + /* This means that if we base64 decode successfully the received reveal + * value, we'll end up with a bigger decoded value thus unusable. */ + goto error; + } + + /* Decode our encoded reveal. Let's be careful here since _encoded_ is + * coming from the network in a dirauth vote so we expect nothing more + * than the base64 encoded length of our reveal. */ + decoded_len = base64_decode(b64_decoded, sizeof(b64_decoded), + encoded, strlen(encoded)); + if (decoded_len < 0) { + log_warn(LD_BUG, "SR: Reveal from authority %s can't be decoded.", + sr_commit_get_rsa_fpr(commit)); + goto error; + } + + if (decoded_len != SR_REVEAL_LEN) { + log_warn(LD_BUG, "SR: Reveal from authority %s decoded length is " + "doesn't match the expected length (%d vs %u)", + sr_commit_get_rsa_fpr(commit), decoded_len, + (unsigned)SR_REVEAL_LEN); + goto error; + } + + commit->reveal_ts = tor_ntohll(get_uint64(b64_decoded)); + /* Copy the last part, the random value. */ + memcpy(commit->random_number, b64_decoded + 8, + sizeof(commit->random_number)); + /* Also copy the whole message to use during verification */ + strlcpy(commit->encoded_reveal, encoded, sizeof(commit->encoded_reveal)); + + return 0; + + error: + return -1; +} + +/* Encode a reveal element using a given commit object to dst which is a + * buffer large enough to put the base64-encoded reveal construction. The + * format is as follow: + * REVEAL = base64-encode( TIMESTAMP || H(RN) ) + * Return base64 encoded length on success else a negative value. + */ +STATIC int +reveal_encode(const sr_commit_t *commit, char *dst, size_t len) +{ + int ret; + size_t offset = 0; + char buf[SR_REVEAL_LEN] = {0}; + + tor_assert(commit); + tor_assert(dst); + + set_uint64(buf, tor_htonll(commit->reveal_ts)); + offset += sizeof(uint64_t); + memcpy(buf + offset, commit->random_number, + sizeof(commit->random_number)); + + /* Let's clean the buffer and then b64 encode it. */ + memset(dst, 0, len); + ret = base64_encode(dst, len, buf, sizeof(buf), 0); + /* Wipe this buffer because it contains our random value. */ + memwipe(buf, 0, sizeof(buf)); + return ret; +} + +/* Encode the given commit object to dst which is a buffer large enough to + * put the base64-encoded commit. The format is as follow: + * COMMIT = base64-encode( TIMESTAMP || H(H(RN)) ) + * Return base64 encoded length on success else a negative value. + */ +STATIC int +commit_encode(const sr_commit_t *commit, char *dst, size_t len) +{ + size_t offset = 0; + char buf[SR_COMMIT_LEN] = {0}; + + tor_assert(commit); + tor_assert(dst); + + /* First is the timestamp (8 bytes). */ + set_uint64(buf, tor_htonll(commit->commit_ts)); + offset += sizeof(uint64_t); + /* and then the hashed reveal. */ + memcpy(buf + offset, commit->hashed_reveal, + sizeof(commit->hashed_reveal)); + + /* Clean the buffer and then b64 encode it. */ + memset(dst, 0, len); + return base64_encode(dst, len, buf, sizeof(buf), 0); +} + +/* Cleanup both our global state and disk state. */ +static void +sr_cleanup(void) +{ + sr_state_free_all(); +} + +/* Using commit, return a newly allocated string containing the commit + * information that should be used during SRV calculation. It's the caller + * responsibility to free the memory. Return NULL if this is not a commit to be + * used for SRV calculation. */ +static char * +get_srv_element_from_commit(const sr_commit_t *commit) +{ + char *element; + tor_assert(commit); + + if (!commit_has_reveal_value(commit)) { + return NULL; + } + + tor_asprintf(&element, "%s%s", sr_commit_get_rsa_fpr(commit), + commit->encoded_reveal); + return element; +} + +/* Return a srv object that is built with the construction: + * SRV = SHA3-256("shared-random" | INT_8(reveal_num) | + * INT_4(version) | HASHED_REVEALS | previous_SRV) + * This function cannot fail. */ +static sr_srv_t * +generate_srv(const char *hashed_reveals, uint64_t reveal_num, + const sr_srv_t *previous_srv) +{ + char msg[DIGEST256_LEN + SR_SRV_MSG_LEN] = {0}; + size_t offset = 0; + sr_srv_t *srv; + + tor_assert(hashed_reveals); + + /* Add the invariant token. */ + memcpy(msg, SR_SRV_TOKEN, SR_SRV_TOKEN_LEN); + offset += SR_SRV_TOKEN_LEN; + set_uint64(msg + offset, tor_htonll(reveal_num)); + offset += sizeof(uint64_t); + set_uint32(msg + offset, htonl(SR_PROTO_VERSION)); + offset += sizeof(uint32_t); + memcpy(msg + offset, hashed_reveals, DIGEST256_LEN); + offset += DIGEST256_LEN; + if (previous_srv != NULL) { + memcpy(msg + offset, previous_srv->value, sizeof(previous_srv->value)); + } + + /* Ok we have our message and key for the HMAC computation, allocate our + * srv object and do the last step. */ + srv = tor_malloc_zero(sizeof(*srv)); + crypto_digest256((char *) srv->value, msg, sizeof(msg), SR_DIGEST_ALG); + srv->num_reveals = reveal_num; + + { + /* Debugging. */ + char srv_hash_encoded[SR_SRV_VALUE_BASE64_LEN + 1]; + sr_srv_encode(srv_hash_encoded, sizeof(srv_hash_encoded), srv); + log_info(LD_DIR, "SR: Generated SRV: %s", srv_hash_encoded); + } + return srv; +} + +/* Compare reveal values and return the result. This should exclusively be + * used by smartlist_sort(). */ +static int +compare_reveal_(const void **_a, const void **_b) +{ + const sr_commit_t *a = *_a, *b = *_b; + return fast_memcmp(a->hashed_reveal, b->hashed_reveal, + sizeof(a->hashed_reveal)); +} + +/* Given commit give the line that we should place in our votes. + * It's the responsibility of the caller to free the string. */ +static char * +get_vote_line_from_commit(const sr_commit_t *commit, sr_phase_t phase) +{ + char *vote_line = NULL; + + switch (phase) { + case SR_PHASE_COMMIT: + tor_asprintf(&vote_line, "%s %u %s %s %s\n", + commit_ns_str, + SR_PROTO_VERSION, + crypto_digest_algorithm_get_name(commit->alg), + sr_commit_get_rsa_fpr(commit), + commit->encoded_commit); + break; + case SR_PHASE_REVEAL: + { + /* Send a reveal value for this commit if we have one. */ + const char *reveal_str = commit->encoded_reveal; + if (tor_mem_is_zero(commit->encoded_reveal, + sizeof(commit->encoded_reveal))) { + reveal_str = ""; + } + tor_asprintf(&vote_line, "%s %u %s %s %s %s\n", + commit_ns_str, + SR_PROTO_VERSION, + crypto_digest_algorithm_get_name(commit->alg), + sr_commit_get_rsa_fpr(commit), + commit->encoded_commit, reveal_str); + break; + } + default: + tor_assert(0); + } + + log_debug(LD_DIR, "SR: Commit vote line: %s", vote_line); + return vote_line; +} + +/* Return a heap allocated string that contains the given srv string + * representation formatted for a networkstatus document using the + * key as the start of the line. This doesn't return NULL. */ +static char * +srv_to_ns_string(const sr_srv_t *srv, const char *key) +{ + char *srv_str; + char srv_hash_encoded[SR_SRV_VALUE_BASE64_LEN + 1]; + tor_assert(srv); + tor_assert(key); + + sr_srv_encode(srv_hash_encoded, sizeof(srv_hash_encoded), srv); + tor_asprintf(&srv_str, "%s %" PRIu64 " %s\n", key, + srv->num_reveals, srv_hash_encoded); + log_debug(LD_DIR, "SR: Consensus SRV line: %s", srv_str); + return srv_str; +} + +/* Given the previous SRV and the current SRV, return a heap allocated + * string with their data that could be put in a vote or a consensus. Caller + * must free the returned string. Return NULL if no SRVs were provided. */ +static char * +get_ns_str_from_sr_values(const sr_srv_t *prev_srv, const sr_srv_t *cur_srv) +{ + smartlist_t *chunks = NULL; + char *srv_str; + + if (!prev_srv && !cur_srv) { + return NULL; + } + + chunks = smartlist_new(); + + if (prev_srv) { + char *srv_line = srv_to_ns_string(prev_srv, previous_srv_str); + smartlist_add(chunks, srv_line); + } + + if (cur_srv) { + char *srv_line = srv_to_ns_string(cur_srv, current_srv_str); + smartlist_add(chunks, srv_line); + } + + /* Join the line(s) here in one string to return. */ + srv_str = smartlist_join_strings(chunks, "", 0, NULL); + SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); + smartlist_free(chunks); + + return srv_str; +} + +/* Return 1 iff the two commits have the same commitment values. This + * function does not care about reveal values. */ +STATIC int +commitments_are_the_same(const sr_commit_t *commit_one, + const sr_commit_t *commit_two) +{ + tor_assert(commit_one); + tor_assert(commit_two); + + if (strcmp(commit_one->encoded_commit, commit_two->encoded_commit)) { + return 0; + } + return 1; +} + +/* We just received a commit from the vote of authority with + * identity_digest. Return 1 if this commit is authorititative that + * is, it belongs to the authority that voted it. Else return 0 if not. */ +STATIC int +commit_is_authoritative(const sr_commit_t *commit, + const char *voter_key) +{ + tor_assert(commit); + tor_assert(voter_key); + + return fast_memeq(commit->rsa_identity, voter_key, + sizeof(commit->rsa_identity)); +} + +/* Decide if the newly received commit should be kept depending on + * the current phase and state of the protocol. The voter_key is the + * RSA identity key fingerprint of the authority's vote from which the + * commit comes from. The phase is the phase we should be validating + * the commit for. Return 1 if the commit should be added to our state or 0 + * if not. */ +STATIC int +should_keep_commit(const sr_commit_t *commit, const char *voter_key, + sr_phase_t phase) +{ + const sr_commit_t *saved_commit; + + tor_assert(commit); + tor_assert(voter_key); + + log_debug(LD_DIR, "SR: Inspecting commit from %s (voter: %s)?", + sr_commit_get_rsa_fpr(commit), + hex_str(voter_key, DIGEST_LEN)); + + /* For a commit to be considered, it needs to be authoritative (it should + * be the voter's own commit). */ + if (!commit_is_authoritative(commit, voter_key)) { + log_debug(LD_DIR, "SR: Ignoring non-authoritative commit."); + goto ignore; + } + + /* Let's make sure, for extra safety, that this fingerprint is known to + * us. Even though this comes from a vote, doesn't hurt to be + * extracareful. */ + if (trusteddirserver_get_by_v3_auth_digest(commit->rsa_identity) == NULL) { + log_warn(LD_DIR, "SR: Fingerprint %s is not from a recognized " + "authority. Discarding commit.", + escaped(commit->rsa_identity)); + goto ignore; + } + + /* Check if the authority that voted for commit has already posted + * a commit before. */ + saved_commit = sr_state_get_commit(commit->rsa_identity); + + switch (phase) { + case SR_PHASE_COMMIT: + /* Already having a commit for an authority so ignore this one. */ + if (saved_commit) { + /* Receiving known commits should happen naturally since commit phase + lasts multiple rounds. However if the commitment value changes + during commit phase, it might be a bug so log more loudly. */ + if (!commitments_are_the_same(commit, saved_commit)) { + log_info(LD_DIR, + "SR: Received altered commit from %s in commit phase.", + sr_commit_get_rsa_fpr(commit)); + } else { + log_debug(LD_DIR, "SR: Ignoring known commit during commit phase."); + } + goto ignore; + } + + /* A commit with a reveal value during commitment phase is very wrong. */ + if (commit_has_reveal_value(commit)) { + log_warn(LD_DIR, "SR: Commit from authority %s has a reveal value " + "during COMMIT phase. (voter: %s)", + sr_commit_get_rsa_fpr(commit), + hex_str(voter_key, DIGEST_LEN)); + goto ignore; + } + break; + case SR_PHASE_REVEAL: + /* We are now in reveal phase. We keep a commit if and only if: + * + * - We have already seen a commit by this auth, AND + * - the saved commit has the same commitment value as this one, AND + * - the saved commit has no reveal information, AND + * - this commit does have reveal information, AND + * - the reveal & commit information are matching. + * + * If all the above are true, then we are interested in this new commit + * for its reveal information. */ + + if (!saved_commit) { + log_debug(LD_DIR, "SR: Ignoring commit first seen in reveal phase."); + goto ignore; + } + + if (!commitments_are_the_same(commit, saved_commit)) { + log_warn(LD_DIR, "SR: Commit from authority %s is different from " + "previous commit in our state (voter: %s)", + sr_commit_get_rsa_fpr(commit), + hex_str(voter_key, DIGEST_LEN)); + goto ignore; + } + + if (commit_has_reveal_value(saved_commit)) { + log_debug(LD_DIR, "SR: Ignoring commit with known reveal info."); + goto ignore; + } + + if (!commit_has_reveal_value(commit)) { + log_debug(LD_DIR, "SR: Ignoring commit without reveal value."); + goto ignore; + } + + if (verify_commit_and_reveal(commit) < 0) { + log_warn(LD_BUG, "SR: Commit from authority %s has an invalid " + "reveal value. (voter: %s)", + sr_commit_get_rsa_fpr(commit), + hex_str(voter_key, DIGEST_LEN)); + goto ignore; + } + break; + default: + tor_assert(0); + } + + return 1; + + ignore: + return 0; +} + +/* We are in reveal phase and we found a valid and verified commit in + * a vote that contains reveal values that we could use. Update the commit + * we have in our state. Never call this with an unverified commit. */ +STATIC void +save_commit_during_reveal_phase(const sr_commit_t *commit) +{ + sr_commit_t *saved_commit; + + tor_assert(commit); + + /* Get the commit from our state. */ + saved_commit = sr_state_get_commit(commit->rsa_identity); + tor_assert(saved_commit); + /* Safety net. They can not be different commitments at this point. */ + int same_commits = commitments_are_the_same(commit, saved_commit); + tor_assert(same_commits); + + /* Copy reveal information to our saved commit. */ + sr_state_copy_reveal_info(saved_commit, commit); +} + +/* Save commit to our persistent state. Depending on the current + * phase, different actions are taken. Steals reference of commit. + * The commit object MUST be valid and verified before adding it to the + * state. */ +STATIC void +save_commit_to_state(sr_commit_t *commit) +{ + sr_phase_t phase = sr_state_get_phase(); + + ASSERT_COMMIT_VALID(commit); + + switch (phase) { + case SR_PHASE_COMMIT: + /* During commit phase, just save any new authoritative commit */ + sr_state_add_commit(commit); + break; + case SR_PHASE_REVEAL: + save_commit_during_reveal_phase(commit); + sr_commit_free(commit); + break; + default: + tor_assert(0); + } +} + +/* Return 1 if we should we keep an SRV voted by n_agreements auths. + * Return 0 if we should ignore it. */ +static int +should_keep_srv(int n_agreements) +{ + /* Check if the most popular SRV has reached majority. */ + int n_voters = get_n_authorities(V3_DIRINFO); + int votes_required_for_majority = (n_voters / 2) + 1; + + /* We need at the very least majority to keep a value. */ + if (n_agreements < votes_required_for_majority) { + log_notice(LD_DIR, "SR: SRV didn't reach majority [%d/%d]!", + n_agreements, votes_required_for_majority); + return 0; + } + + /* When we just computed a new SRV, we need to have super majority in order + * to keep it. */ + if (sr_state_srv_is_fresh()) { + /* Check if we have super majority for this new SRV value. */ + if (n_agreements < num_srv_agreements_from_vote) { + log_notice(LD_DIR, "SR: New SRV didn't reach agreement [%d/%d]!", + n_agreements, num_srv_agreements_from_vote); + return 0; + } + } + + return 1; +} + +/* Helper: compare two DIGEST256_LEN digests. */ +static int +compare_srvs_(const void **_a, const void **_b) +{ + const sr_srv_t *a = *_a, *b = *_b; + return tor_memcmp(a->value, b->value, sizeof(a->value)); +} + +/* Return the most frequent member of the sorted list of DIGEST256_LEN + * digests in sl with the count of that most frequent element. */ +static sr_srv_t * +smartlist_get_most_frequent_srv(const smartlist_t *sl, int *count_out) +{ + return smartlist_get_most_frequent_(sl, compare_srvs_, count_out); +} + +/** Compare two SRVs. Used in smartlist sorting. */ +static int +compare_srv_(const void **_a, const void **_b) +{ + const sr_srv_t *a = *_a, *b = *_b; + return fast_memcmp(a->value, b->value, + sizeof(a->value)); +} + +/* Using a list of votes, return the SRV object from them that has + * been voted by the majority of dirauths. If current is set, we look + * for the current SRV value else the previous one. The returned pointer is + * an object located inside a vote. NULL is returned if no appropriate value + * could be found. */ +STATIC sr_srv_t * +get_majority_srv_from_votes(const smartlist_t *votes, int current) +{ + int count = 0; + sr_srv_t *most_frequent_srv = NULL; + sr_srv_t *the_srv = NULL; + smartlist_t *srv_list; + + tor_assert(votes); + + srv_list = smartlist_new(); + + /* Walk over votes and register any SRVs found. */ + SMARTLIST_FOREACH_BEGIN(votes, networkstatus_t *, v) { + sr_srv_t *srv_tmp = NULL; + + if (!v->sr_info.participate) { + /* Ignore vote that do not participate. */ + continue; + } + /* Do we want previous or current SRV? */ + srv_tmp = current ? v->sr_info.current_srv : v->sr_info.previous_srv; + if (!srv_tmp) { + continue; + } + + smartlist_add(srv_list, srv_tmp); + } SMARTLIST_FOREACH_END(v); + + smartlist_sort(srv_list, compare_srv_); + most_frequent_srv = smartlist_get_most_frequent_srv(srv_list, &count); + if (!most_frequent_srv) { + goto end; + } + + /* Was this SRV voted by enough auths for us to keep it? */ + if (!should_keep_srv(count)) { + goto end; + } + + /* We found an SRV that we can use! Habemus SRV! */ + the_srv = most_frequent_srv; + + { + /* Debugging */ + char encoded[SR_SRV_VALUE_BASE64_LEN + 1]; + sr_srv_encode(encoded, sizeof(encoded), the_srv); + log_debug(LD_DIR, "SR: Chosen SRV by majority: %s (%d votes)", encoded, + count); + } + + end: + /* We do not free any sr_srv_t values, we don't have the ownership. */ + smartlist_free(srv_list); + return the_srv; +} + +/* Free a commit object. */ +void +sr_commit_free_(sr_commit_t *commit) +{ + if (commit == NULL) { + return; + } + /* Make sure we do not leave OUR random number in memory. */ + memwipe(commit->random_number, 0, sizeof(commit->random_number)); + tor_free(commit); +} + +/* Generate the commitment/reveal value for the protocol run starting at + * timestamp. my_rsa_cert is our authority RSA certificate. */ +sr_commit_t * +sr_generate_our_commit(time_t timestamp, const authority_cert_t *my_rsa_cert) +{ + sr_commit_t *commit = NULL; + char digest[DIGEST_LEN]; + + tor_assert(my_rsa_cert); + + /* Get our RSA identity fingerprint */ + if (crypto_pk_get_digest(my_rsa_cert->identity_key, digest) < 0) { + goto error; + } + + /* New commit with our identity key. */ + commit = commit_new(digest); + + /* Generate the reveal random value */ + crypto_strongest_rand(commit->random_number, + sizeof(commit->random_number)); + commit->commit_ts = commit->reveal_ts = timestamp; + + /* Now get the base64 blob that corresponds to our reveal */ + if (reveal_encode(commit, commit->encoded_reveal, + sizeof(commit->encoded_reveal)) < 0) { + log_err(LD_DIR, "SR: Unable to encode our reveal value!"); + goto error; + } + + /* Now let's create the commitment */ + tor_assert(commit->alg == SR_DIGEST_ALG); + /* The invariant length is used here since the encoded reveal variable + * has an extra byte added for the NULL terminated byte. */ + if (crypto_digest256(commit->hashed_reveal, commit->encoded_reveal, + SR_REVEAL_BASE64_LEN, commit->alg) < 0) { + goto error; + } + + /* Now get the base64 blob that corresponds to our commit. */ + if (commit_encode(commit, commit->encoded_commit, + sizeof(commit->encoded_commit)) < 0) { + log_err(LD_DIR, "SR: Unable to encode our commit value!"); + goto error; + } + + log_debug(LD_DIR, "SR: Generated our commitment:"); + commit_log(commit); + /* Our commit better be valid :). */ + commit->valid = 1; + return commit; + + error: + sr_commit_free(commit); + return NULL; +} + +/* Compute the shared random value based on the active commits in our state. */ +void +sr_compute_srv(void) +{ + uint64_t reveal_num = 0; + char *reveals = NULL; + smartlist_t *chunks, *commits; + digestmap_t *state_commits; + + /* Computing a shared random value in the commit phase is very wrong. This + * should only happen at the very end of the reveal phase when a new + * protocol run is about to start. */ + if (BUG(sr_state_get_phase() != SR_PHASE_REVEAL)) + return; + state_commits = sr_state_get_commits(); + + commits = smartlist_new(); + chunks = smartlist_new(); + + /* We must make a list of commit ordered by authority fingerprint in + * ascending order as specified by proposal 250. */ + DIGESTMAP_FOREACH(state_commits, key, sr_commit_t *, c) { + /* Extra safety net, make sure we have valid commit before using it. */ + ASSERT_COMMIT_VALID(c); + /* Let's not use a commit from an authority that we don't know. It's + * possible that an authority could be removed during a protocol run so + * that commit value should never be used in the SRV computation. */ + if (trusteddirserver_get_by_v3_auth_digest(c->rsa_identity) == NULL) { + log_warn(LD_DIR, "SR: Fingerprint %s is not from a recognized " + "authority. Discarding commit for the SRV computation.", + sr_commit_get_rsa_fpr(c)); + continue; + } + /* We consider this commit valid. */ + smartlist_add(commits, c); + } DIGESTMAP_FOREACH_END; + smartlist_sort(commits, compare_reveal_); + + /* Now for each commit for that sorted list in ascending order, we'll + * build the element for each authority that needs to go into the srv + * computation. */ + SMARTLIST_FOREACH_BEGIN(commits, const sr_commit_t *, c) { + char *element = get_srv_element_from_commit(c); + if (element) { + smartlist_add(chunks, element); + reveal_num++; + } + } SMARTLIST_FOREACH_END(c); + smartlist_free(commits); + + { + /* Join all reveal values into one giant string that we'll hash so we + * can generated our shared random value. */ + sr_srv_t *current_srv; + char hashed_reveals[DIGEST256_LEN]; + reveals = smartlist_join_strings(chunks, "", 0, NULL); + SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); + smartlist_free(chunks); + if (crypto_digest256(hashed_reveals, reveals, strlen(reveals), + SR_DIGEST_ALG) < 0) { + goto end; + } + current_srv = generate_srv(hashed_reveals, reveal_num, + sr_state_get_previous_srv()); + sr_state_set_current_srv(current_srv); + /* We have a fresh SRV, flag our state. */ + sr_state_set_fresh_srv(); + } + + end: + tor_free(reveals); +} + +/* Parse a commit from a vote or from our disk state and return a newly + * allocated commit object. NULL is returned on error. + * + * The commit's data is in args and the order matters very much: + * version, algname, RSA fingerprint, commit value[, reveal value] + */ +sr_commit_t * +sr_parse_commit(const smartlist_t *args) +{ + uint32_t version; + char *value, digest[DIGEST_LEN]; + digest_algorithm_t alg; + const char *rsa_identity_fpr; + sr_commit_t *commit = NULL; + + if (smartlist_len(args) < 4) { + goto error; + } + + /* First is the version number of the SR protocol which indicates at which + * version that commit was created. */ + value = smartlist_get(args, 0); + version = (uint32_t) tor_parse_ulong(value, 10, 1, UINT32_MAX, NULL, NULL); + if (version > SR_PROTO_VERSION) { + log_info(LD_DIR, "SR: Commit version %" PRIu32 " (%s) is not supported.", + version, escaped(value)); + goto error; + } + + /* Second is the algorithm. */ + value = smartlist_get(args, 1); + alg = crypto_digest_algorithm_parse_name(value); + if (alg != SR_DIGEST_ALG) { + log_warn(LD_BUG, "SR: Commit algorithm %s is not recognized.", + escaped(value)); + goto error; + } + + /* Third argument is the RSA fingerprint of the auth and turn it into a + * digest value. */ + rsa_identity_fpr = smartlist_get(args, 2); + if (base16_decode(digest, DIGEST_LEN, rsa_identity_fpr, + HEX_DIGEST_LEN) < 0) { + log_warn(LD_DIR, "SR: RSA fingerprint %s not decodable", + escaped(rsa_identity_fpr)); + goto error; + } + + /* Allocate commit since we have a valid identity now. */ + commit = commit_new(digest); + + /* Fourth argument is the commitment value base64-encoded. */ + value = smartlist_get(args, 3); + if (commit_decode(value, commit) < 0) { + goto error; + } + + /* (Optional) Fifth argument is the revealed value. */ + if (smartlist_len(args) > 4) { + value = smartlist_get(args, 4); + if (reveal_decode(value, commit) < 0) { + goto error; + } + } + + return commit; + + error: + sr_commit_free(commit); + return NULL; +} + +/* Called when we are done parsing a vote by voter_key that might + * contain some useful commits. Find if any of them should be kept + * and update our state accordingly. Once done, the list of commitments will + * be empty. */ +void +sr_handle_received_commits(smartlist_t *commits, crypto_pk_t *voter_key) +{ + char rsa_identity[DIGEST_LEN]; + + tor_assert(voter_key); + + /* It's possible that the vote has _NO_ commits. */ + if (commits == NULL) { + return; + } + + /* Get the RSA identity fingerprint of this voter */ + if (crypto_pk_get_digest(voter_key, rsa_identity) < 0) { + return; + } + + SMARTLIST_FOREACH_BEGIN(commits, sr_commit_t *, commit) { + /* We won't need the commit in this list anymore, kept or not. */ + SMARTLIST_DEL_CURRENT(commits, commit); + /* Check if this commit is valid and should be stored in our state. */ + if (!should_keep_commit(commit, rsa_identity, + sr_state_get_phase())) { + sr_commit_free(commit); + continue; + } + /* Ok, we have a valid commit now that we are about to put in our state. + * so flag it valid from now on. */ + commit->valid = 1; + /* Everything lines up: save this commit to state then! */ + save_commit_to_state(commit); + } SMARTLIST_FOREACH_END(commit); +} + +/* Return a heap-allocated string containing commits that should be put in + * the votes. It's the responsibility of the caller to free the string. + * This always return a valid string, either empty or with line(s). */ +char * +sr_get_string_for_vote(void) +{ + char *vote_str = NULL; + digestmap_t *state_commits; + smartlist_t *chunks = smartlist_new(); + const or_options_t *options = get_options(); + + /* Are we participating in the protocol? */ + if (!options->AuthDirSharedRandomness) { + goto end; + } + + log_debug(LD_DIR, "SR: Preparing our vote info:"); + + /* First line, put in the vote the participation flag. */ + { + char *sr_flag_line; + tor_asprintf(&sr_flag_line, "%s\n", sr_flag_ns_str); + smartlist_add(chunks, sr_flag_line); + } + + /* In our vote we include every commitment in our permanent state. */ + state_commits = sr_state_get_commits(); + smartlist_t *state_commit_vote_lines = smartlist_new(); + DIGESTMAP_FOREACH(state_commits, key, const sr_commit_t *, commit) { + char *line = get_vote_line_from_commit(commit, sr_state_get_phase()); + smartlist_add(state_commit_vote_lines, line); + } DIGESTMAP_FOREACH_END; + + /* Sort the commit strings by version (string, not numeric), algorithm, + * and fingerprint. This makes sure the commit lines in votes are in a + * recognisable, stable order. */ + smartlist_sort_strings(state_commit_vote_lines); + + /* Now add the sorted list of commits to the vote */ + smartlist_add_all(chunks, state_commit_vote_lines); + smartlist_free(state_commit_vote_lines); + + /* Add the SRV value(s) if any. */ + { + char *srv_lines = get_ns_str_from_sr_values(sr_state_get_previous_srv(), + sr_state_get_current_srv()); + if (srv_lines) { + smartlist_add(chunks, srv_lines); + } + } + + end: + vote_str = smartlist_join_strings(chunks, "", 0, NULL); + SMARTLIST_FOREACH(chunks, char *, s, tor_free(s)); + smartlist_free(chunks); + return vote_str; +} + +/* Return a heap-allocated string that should be put in the consensus and + * contains the shared randomness values. It's the responsibility of the + * caller to free the string. NULL is returned if no SRV(s) available. + * + * This is called when a consensus (any flavor) is bring created thus it + * should NEVER change the state nor the state should be changed in between + * consensus creation. + * + * num_srv_agreements is taken from the votes thus the voted value + * that should be used. + * */ +char * +sr_get_string_for_consensus(const smartlist_t *votes, + int32_t num_srv_agreements) +{ + char *srv_str; + const or_options_t *options = get_options(); + + tor_assert(votes); + + /* Not participating, avoid returning anything. */ + if (!options->AuthDirSharedRandomness) { + log_info(LD_DIR, "SR: Support disabled (AuthDirSharedRandomness %d)", + options->AuthDirSharedRandomness); + goto end; + } + + /* Set the global value of AuthDirNumSRVAgreements found in the votes. */ + num_srv_agreements_from_vote = num_srv_agreements; + + /* Check the votes and figure out if SRVs should be included in the final + * consensus. */ + sr_srv_t *prev_srv = get_majority_srv_from_votes(votes, 0); + sr_srv_t *cur_srv = get_majority_srv_from_votes(votes, 1); + srv_str = get_ns_str_from_sr_values(prev_srv, cur_srv); + if (!srv_str) { + goto end; + } + + return srv_str; + end: + return NULL; +} + +/* We just computed a new consensus. Update our state with the SRVs + * from the consensus (might be NULL as well). Register the SRVs in our SR + * state and prepare for the upcoming protocol round. */ +void +sr_act_post_consensus(const networkstatus_t *consensus) +{ + const or_options_t *options = get_options(); + + /* Don't act if our state hasn't been initialized. We can be called during + * boot time when loading consensus from disk which is prior to the + * initialization of the SR subsystem. We also should not be doing + * anything if we are _not_ a directory authority and if we are a bridge + * authority. */ + if (!sr_state_is_initialized() || !authdir_mode_v3(options) || + authdir_mode_bridge(options)) { + return; + } + + /* Set the majority voted SRVs in our state even if both are NULL. It + * doesn't matter this is what the majority has decided. Obviously, we can + * only do that if we have a consensus. */ + if (consensus) { + /* Start by freeing the current SRVs since the SRVs we believed during + * voting do not really matter. Now that all the votes are in, we use the + * majority's opinion on which are the active SRVs. */ + sr_state_clean_srvs(); + /* Reset the fresh flag of the SRV so we know that from now on we don't + * have a new SRV to vote for. We just used the one from the 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. */ + sr_state_update(voting_schedule_get_next_valid_after_time()); +} + +/* Initialize shared random subsystem. This MUST be called early in the boot + * process of tor. Return 0 on success else -1 on error. */ +int +sr_init(int save_to_disk) +{ + return sr_state_init(save_to_disk, 1); +} + +/* Save our state to disk and cleanup everything. */ +void +sr_save_and_cleanup(void) +{ + sr_state_save(); + sr_cleanup(); +} + +#ifdef TOR_UNIT_TESTS + +/* Set the global value of number of SRV agreements so the test can play + * along by calling specific functions that don't parse the votes prior for + * the AuthDirNumSRVAgreements value. */ +void +set_num_srv_agreements(int32_t value) +{ + num_srv_agreements_from_vote = value; +} + +#endif /* defined(TOR_UNIT_TESTS) */ diff --cc src/feature/dirauth/shared_random.h index 25d95ebbc,000000000..0b45ad1ed mode 100644,000000..100644 --- a/src/feature/dirauth/shared_random.h +++ b/src/feature/dirauth/shared_random.h @@@ -1,194 -1,0 +1,194 @@@ +/* Copyright (c) 2016-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#ifndef TOR_SHARED_RANDOM_H +#define TOR_SHARED_RANDOM_H + +/* + * This file contains ABI/API of the shared random protocol defined in + * proposal #250. Every public functions and data structure are namespaced + * with "sr_" which stands for shared random. + */ + +#include "core/or/or.h" + +/* Protocol version */ +#define SR_PROTO_VERSION 1 +/* Default digest algorithm. */ +#define SR_DIGEST_ALG DIGEST_SHA3_256 +/* Invariant token in the SRV calculation. */ +#define SR_SRV_TOKEN "shared-random" +/* Don't count the NUL terminated byte even though the TOKEN has it. */ +#define SR_SRV_TOKEN_LEN (sizeof(SR_SRV_TOKEN) - 1) + +/* Length of the random number (in bytes). */ +#define SR_RANDOM_NUMBER_LEN 32 +/* Size of a decoded commit value in a vote or state. It's a hash and a + * timestamp. It adds up to 40 bytes. */ +#define SR_COMMIT_LEN (sizeof(uint64_t) + DIGEST256_LEN) +/* Size of a decoded reveal value from a vote or state. It's a 64 bit + * timestamp and the hashed random number. This adds up to 40 bytes. */ +#define SR_REVEAL_LEN (sizeof(uint64_t) + DIGEST256_LEN) +/* Size of SRV message length. The construction is has follow: + * "shared-random" | INT_8(reveal_num) | INT_4(version) | PREV_SRV */ +#define SR_SRV_MSG_LEN \ + (SR_SRV_TOKEN_LEN + sizeof(uint64_t) + sizeof(uint32_t) + DIGEST256_LEN) + +/* Length of base64 encoded commit NOT including the NUL terminated byte. + * Formula is taken from base64_encode_size. This adds up to 56 bytes. */ +#define SR_COMMIT_BASE64_LEN (BASE64_LEN(SR_COMMIT_LEN)) +/* Length of base64 encoded reveal NOT including the NUL terminated byte. + * Formula is taken from base64_encode_size. This adds up to 56 bytes. */ +#define SR_REVEAL_BASE64_LEN (BASE64_LEN(SR_REVEAL_LEN)) +/* Length of base64 encoded shared random value. It's 32 bytes long so 44 + * bytes from the base64_encode_size formula. That includes the '=' + * character at the end. */ +#define SR_SRV_VALUE_BASE64_LEN (BASE64_LEN(DIGEST256_LEN)) + +/* Assert if commit valid flag is not set. */ +#define ASSERT_COMMIT_VALID(c) tor_assert((c)->valid) + +/* Protocol phase. */ +typedef enum { + /* Commitment phase */ + SR_PHASE_COMMIT = 1, + /* Reveal phase */ + SR_PHASE_REVEAL = 2, +} sr_phase_t; + +/* A shared random value (SRV). */ +typedef struct sr_srv_t { + /* The number of reveal values used to derive this SRV. */ + uint64_t num_reveals; + /* The actual value. This is the stored result of SHA3-256. */ + uint8_t value[DIGEST256_LEN]; +} sr_srv_t; + +/* A commit (either ours or from another authority). */ +typedef struct sr_commit_t { + /* Hashing algorithm used. */ + digest_algorithm_t alg; + /* Indicate if this commit has been verified thus valid. */ + unsigned int valid:1; + + /* Commit owner info */ + + /* The RSA identity key of the authority and its base16 representation, + * which includes the NUL terminated byte. */ + char rsa_identity[DIGEST_LEN]; + char rsa_identity_hex[HEX_DIGEST_LEN + 1]; + + /* Commitment information */ + + /* Timestamp of reveal. Correspond to TIMESTAMP. */ + uint64_t reveal_ts; + /* H(REVEAL) as found in COMMIT message. */ + char hashed_reveal[DIGEST256_LEN]; + /* Base64 encoded COMMIT. We use this to put it in our vote. */ + char encoded_commit[SR_COMMIT_BASE64_LEN + 1]; + + /* Reveal information */ + + /* H(RN) which is what we used as the random value for this commit. We + * don't use the raw bytes since those are sent on the network thus + * avoiding possible information leaks of our PRNG. */ + uint8_t random_number[SR_RANDOM_NUMBER_LEN]; + /* Timestamp of commit. Correspond to TIMESTAMP. */ + uint64_t commit_ts; + /* This is the whole reveal message. We use it during verification */ + char encoded_reveal[SR_REVEAL_BASE64_LEN + 1]; +} sr_commit_t; + +/* API */ + +/* Public methods used _outside_ of the module. + * + * We need to nullify them if the module is disabled. */ +#ifdef HAVE_MODULE_DIRAUTH + +int sr_init(int save_to_disk); +void sr_save_and_cleanup(void); +void sr_act_post_consensus(const networkstatus_t *consensus); + +#else /* HAVE_MODULE_DIRAUTH */ + +static inline int +sr_init(int save_to_disk) +{ + (void) save_to_disk; + /* Always return success. */ + return 0; +} + +static inline void +sr_save_and_cleanup(void) +{ +} + +static inline void +sr_act_post_consensus(const networkstatus_t *consensus) +{ + (void) consensus; +} + +#endif /* HAVE_MODULE_DIRAUTH */ + +/* Public methods used only by dirauth code. */ + +void sr_handle_received_commits(smartlist_t *commits, + crypto_pk_t *voter_key); +sr_commit_t *sr_parse_commit(const smartlist_t *args); +char *sr_get_string_for_vote(void); +char *sr_get_string_for_consensus(const smartlist_t *votes, + int32_t num_srv_agreements); +void sr_commit_free_(sr_commit_t *commit); +#define sr_commit_free(sr) FREE_AND_NULL(sr_commit_t, sr_commit_free_, (sr)) + +/* Private methods (only used by shared_random_state.c): */ +static inline +const char *sr_commit_get_rsa_fpr(const sr_commit_t *commit) +{ + return commit->rsa_identity_hex; +} + +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 *sr_srv_dup(const sr_srv_t *orig); + +#ifdef SHARED_RANDOM_PRIVATE + +/* Encode */ +STATIC int reveal_encode(const sr_commit_t *commit, char *dst, size_t len); +STATIC int commit_encode(const sr_commit_t *commit, char *dst, size_t len); +/* Decode. */ +STATIC int commit_decode(const char *encoded, sr_commit_t *commit); +STATIC int reveal_decode(const char *encoded, sr_commit_t *commit); + +STATIC int commit_has_reveal_value(const sr_commit_t *commit); + +STATIC int verify_commit_and_reveal(const sr_commit_t *commit); + +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, + const char *voter_key); +STATIC int should_keep_commit(const sr_commit_t *commit, + const char *voter_key, + sr_phase_t phase); +STATIC void save_commit_during_reveal_phase(const sr_commit_t *commit); + +#endif /* defined(SHARED_RANDOM_PRIVATE) */ + +#ifdef TOR_UNIT_TESTS + +void set_num_srv_agreements(int32_t value); + +#endif /* TOR_UNIT_TESTS */ + +#endif /* !defined(TOR_SHARED_RANDOM_H) */ + diff --cc src/feature/dirauth/shared_random_state.c index 92f0b3e73,000000000..a7b7480ed mode 100644,000000..100644 --- a/src/feature/dirauth/shared_random_state.c +++ b/src/feature/dirauth/shared_random_state.c @@@ -1,1341 -1,0 +1,1377 @@@ +/* Copyright (c) 2016-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file shared_random_state.c + * + * \brief Functions and data structures for the state of the random protocol + * as defined in proposal #250. + **/ + +#define SHARED_RANDOM_STATE_PRIVATE + +#include "core/or/or.h" +#include "app/config/config.h" +#include "app/config/confparse.h" +#include "lib/crypt_ops/crypto_util.h" +#include "feature/dirauth/dirvote.h" +#include "feature/nodelist/networkstatus.h" +#include "feature/relay/router.h" +#include "feature/dirauth/shared_random.h" +#include "feature/hs_common/shared_random_client.h" +#include "feature/dirauth/shared_random_state.h" +#include "feature/dircommon/voting_schedule.h" +#include "lib/encoding/confline.h" +#include "lib/version/torversion.h" + +#include "app/config/or_state_st.h" + +/* Default filename of the shared random state on disk. */ +static const char default_fname[] = "sr-state"; + +/* String representation of a protocol phase. */ +static const char *phase_str[] = { "unknown", "commit", "reveal" }; + +/* Our shared random protocol state. There is only one possible state per + * protocol run so this is the global state which is reset at every run once + * the shared random value has been computed. */ +static sr_state_t *sr_state = NULL; + +/* Representation of our persistent state on disk. The sr_state above + * contains the data parsed from this state. When we save to disk, we + * translate the sr_state to this sr_disk_state. */ +static sr_disk_state_t *sr_disk_state = NULL; + +/* Disk state file keys. */ +static const char dstate_commit_key[] = "Commit"; +static const char dstate_prev_srv_key[] = "SharedRandPreviousValue"; +static const char dstate_cur_srv_key[] = "SharedRandCurrentValue"; + +/** dummy instance of sr_disk_state_t, used for type-checking its + * members with CONF_CHECK_VAR_TYPE. */ +DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t); + +/* These next two are duplicates or near-duplicates from config.c */ +#define VAR(name, conftype, member, initvalue) \ + { name, CONFIG_TYPE_ ## conftype, offsetof(sr_disk_state_t, member), \ + initvalue CONF_TEST_MEMBERS(sr_disk_state_t, conftype, member) } +/* As VAR, but the option name and member name are the same. */ +#define V(member, conftype, initvalue) \ + VAR(#member, conftype, member, initvalue) +/* Our persistent state magic number. */ +#define SR_DISK_STATE_MAGIC 0x98AB1254 + +static int +disk_state_validate_cb(void *old_state, void *state, void *default_state, + int from_setconf, char **msg); +static void disk_state_free_cb(void *); + +/* Array of variables that are saved to disk as a persistent state. */ +static config_var_t state_vars[] = { + V(Version, UINT, "0"), + V(TorVersion, STRING, NULL), + V(ValidAfter, ISOTIME, NULL), + V(ValidUntil, ISOTIME, NULL), + + V(Commit, LINELIST, NULL), + + V(SharedRandValues, LINELIST_V, NULL), + VAR("SharedRandPreviousValue",LINELIST_S, SharedRandValues, NULL), + VAR("SharedRandCurrentValue", LINELIST_S, SharedRandValues, NULL), + END_OF_CONFIG_VARS +}; + +/* "Extra" variable in the state that receives lines we can't parse. This + * lets us preserve options from versions of Tor newer than us. */ +static config_var_t state_extra_var = { + "__extra", CONFIG_TYPE_LINELIST, + offsetof(sr_disk_state_t, ExtraLines), NULL + CONF_TEST_MEMBERS(sr_disk_state_t, LINELIST, ExtraLines) +}; + +/* Configuration format of sr_disk_state_t. */ +static const config_format_t state_format = { + sizeof(sr_disk_state_t), + SR_DISK_STATE_MAGIC, + offsetof(sr_disk_state_t, magic_), + NULL, + NULL, + state_vars, + disk_state_validate_cb, + disk_state_free_cb, + &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) +{ + const char *the_string = NULL; + + switch (phase) { + case SR_PHASE_COMMIT: + case SR_PHASE_REVEAL: + the_string = phase_str[phase]; + break; + default: + /* Unknown phase shouldn't be possible. */ + tor_assert(0); + } + + return the_string; +} +/* Return the time we should expire the state file created at now. + * We expire the state file in the beginning of the next protocol run. */ +STATIC time_t +get_state_valid_until_time(time_t now) +{ + int total_rounds = SHARED_RANDOM_N_ROUNDS * SHARED_RANDOM_N_PHASES; + int current_round, voting_interval, rounds_left; + time_t valid_until, beginning_of_current_round; + + voting_interval = get_voting_interval(); + /* Find the time the current round started. */ + beginning_of_current_round = get_start_time_of_current_round(); + + /* Find how many rounds are left till the end of the protocol run */ + current_round = (now / voting_interval) % total_rounds; + rounds_left = total_rounds - current_round; + + /* To find the valid-until time now, take the start time of the current + * round and add to it the time it takes for the leftover rounds to + * complete. */ + valid_until = beginning_of_current_round + (rounds_left * voting_interval); + + { /* Logging */ + char tbuf[ISO_TIME_LEN + 1]; + format_iso_time(tbuf, valid_until); + log_debug(LD_DIR, "SR: Valid until time for state set to %s.", tbuf); + } + + return valid_until; +} + +/* Given the consensus 'valid-after' time, return the protocol phase we should + * be in. */ +STATIC sr_phase_t +get_sr_protocol_phase(time_t valid_after) +{ + /* Shared random protocol has two phases, commit and reveal. */ + int total_periods = SHARED_RANDOM_N_ROUNDS * SHARED_RANDOM_N_PHASES; + int current_slot; + + /* Split time into slots of size 'voting_interval'. See which slot we are + * currently into, and find which phase it corresponds to. */ + current_slot = (valid_after / get_voting_interval()) % total_periods; + + if (current_slot < SHARED_RANDOM_N_ROUNDS) { + return SR_PHASE_COMMIT; + } else { + return SR_PHASE_REVEAL; + } +} + +/* Add the given commit to state. It MUST be a valid commit + * and there shouldn't be a commit from the same authority in the state + * already else verification hasn't been done prior. This takes ownership of + * the commit once in our state. */ +static void +commit_add_to_state(sr_commit_t *commit, sr_state_t *state) +{ + sr_commit_t *saved_commit; + + tor_assert(commit); + tor_assert(state); + + saved_commit = digestmap_set(state->commits, commit->rsa_identity, + commit); + if (saved_commit != NULL) { + /* This means we already have that commit in our state so adding twice + * the same commit is either a code flow error, a corrupted disk state + * or some new unknown issue. */ + log_warn(LD_DIR, "SR: Commit from %s exists in our state while " + "adding it: '%s'", sr_commit_get_rsa_fpr(commit), + commit->encoded_commit); + sr_commit_free(saved_commit); + } +} + +/* Helper: deallocate a commit object. (Used with digestmap_free(), which + * requires a function pointer whose argument is void *). */ +static void +commit_free_(void *p) +{ + sr_commit_free_(p); +} + +#define state_free(val) \ + FREE_AND_NULL(sr_state_t, state_free_, (val)) + +/* Free a state that was allocated with state_new(). */ +static void +state_free_(sr_state_t *state) +{ + if (state == NULL) { + return; + } + tor_free(state->fname); + digestmap_free(state->commits, commit_free_); + tor_free(state->current_srv); + tor_free(state->previous_srv); + tor_free(state); +} + +/* Allocate an sr_state_t object and returns it. If no fname, the + * default file name is used. This function does NOT initialize the state + * timestamp, phase or shared random value. NULL is never returned. */ +static sr_state_t * +state_new(const char *fname, time_t now) +{ + sr_state_t *new_state = tor_malloc_zero(sizeof(*new_state)); + /* If file name is not provided, use default. */ + if (fname == NULL) { + fname = default_fname; + } + new_state->fname = tor_strdup(fname); + new_state->version = SR_PROTO_VERSION; + new_state->commits = digestmap_new(); + new_state->phase = get_sr_protocol_phase(now); + new_state->valid_until = get_state_valid_until_time(now); + return new_state; +} + +/* Set our global state pointer with the one given. */ +static void +state_set(sr_state_t *state) +{ + tor_assert(state); + if (sr_state != NULL) { + state_free(sr_state); + } + sr_state = state; +} + +#define disk_state_free(val) \ + FREE_AND_NULL(sr_disk_state_t, disk_state_free_, (val)) + +/* Free an allocated disk state. */ +static void +disk_state_free_(sr_disk_state_t *state) +{ + if (state == NULL) { + return; + } + config_free(&state_format, state); +} + +/* Allocate a new disk state, initialize it and return it. */ +static sr_disk_state_t * +disk_state_new(time_t now) +{ + sr_disk_state_t *new_state = tor_malloc_zero(sizeof(*new_state)); + + new_state->magic_ = SR_DISK_STATE_MAGIC; + new_state->Version = SR_PROTO_VERSION; + new_state->TorVersion = tor_strdup(get_version()); + new_state->ValidUntil = get_state_valid_until_time(now); + new_state->ValidAfter = now; + + /* Init config format. */ + config_init(&state_format, new_state); + return new_state; +} + +/* Set our global disk state with the given state. */ +static void +disk_state_set(sr_disk_state_t *state) +{ + tor_assert(state); + if (sr_disk_state != NULL) { + disk_state_free(sr_disk_state); + } + sr_disk_state = state; +} + +/* Return -1 if the disk state is invalid (something in there that we can't or + * shouldn't use). Return 0 if everything checks out. */ +static int +disk_state_validate(const sr_disk_state_t *state) +{ + time_t now; + + tor_assert(state); + + /* Do we support the protocol version in the state or is it 0 meaning + * Version wasn't found in the state file or bad anyway ? */ + if (state->Version == 0 || state->Version > SR_PROTO_VERSION) { + goto invalid; + } + + /* If the valid until time is before now, we shouldn't use that state. */ + now = time(NULL); + if (state->ValidUntil < now) { + log_info(LD_DIR, "SR: Disk state has expired. Ignoring it."); + goto invalid; + } + + /* Make sure we don't have a valid after time that is earlier than a valid + * until time which would make things not work well. */ + if (state->ValidAfter >= state->ValidUntil) { + log_info(LD_DIR, "SR: Disk state valid after/until times are invalid."); + goto invalid; + } + + return 0; + + invalid: + return -1; +} + +/* Validate the disk state (NOP for now). */ +static int +disk_state_validate_cb(void *old_state, void *state, void *default_state, + int from_setconf, char **msg) +{ + /* We don't use these; only options do. */ + (void) from_setconf; + (void) default_state; + (void) old_state; + + /* This is called by config_dump which is just before we are about to + * write it to disk. At that point, our global memory state has been + * copied to the disk state so it's fair to assume it's trustable. */ + (void) state; + (void) msg; + return 0; +} + +static void +disk_state_free_cb(void *state) +{ + disk_state_free_(state); +} + +/* Parse the Commit line(s) in the disk state and translate them to the + * the memory state. Return 0 on success else -1 on error. */ +static int +disk_state_parse_commits(sr_state_t *state, + const sr_disk_state_t *disk_state) +{ + config_line_t *line; + smartlist_t *args = NULL; + + tor_assert(state); + tor_assert(disk_state); + + for (line = disk_state->Commit; line; line = line->next) { + sr_commit_t *commit = NULL; + + /* Extra safety. */ + if (strcasecmp(line->key, dstate_commit_key) || + line->value == NULL) { + /* Ignore any lines that are not commits. */ + tor_fragile_assert(); + continue; + } + args = smartlist_new(); + smartlist_split_string(args, line->value, " ", + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + if (smartlist_len(args) < 3) { + log_warn(LD_BUG, "SR: Too few arguments in Commit Line: %s", + escaped(line->value)); + goto error; + } + commit = sr_parse_commit(args); + if (commit == NULL) { + /* Ignore badly formed commit. It could also be a authority + * fingerprint that we don't know about so it shouldn't be used. */ + smartlist_free(args); + continue; + } + /* We consider parseable commit from our disk state to be valid because + * they need to be in the first place to get in there. */ + commit->valid = 1; + /* Add commit to our state pointer. */ + commit_add_to_state(commit, state); + + SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); + smartlist_free(args); + } + + return 0; + + error: + SMARTLIST_FOREACH(args, char *, cp, tor_free(cp)); + smartlist_free(args); + return -1; +} + +/* Parse a share random value line from the disk state and save it to dst + * which is an allocated srv object. Return 0 on success else -1. */ +static int +disk_state_parse_srv(const char *value, sr_srv_t *dst) +{ + int ret = -1; + smartlist_t *args; + sr_srv_t *srv; + + tor_assert(value); + tor_assert(dst); + + args = smartlist_new(); + smartlist_split_string(args, value, " ", + SPLIT_SKIP_SPACE|SPLIT_IGNORE_BLANK, 0); + if (smartlist_len(args) < 2) { + log_warn(LD_BUG, "SR: Too few arguments in shared random value. " + "Line: %s", escaped(value)); + goto error; + } + srv = sr_parse_srv(args); + if (srv == NULL) { + goto error; + } + dst->num_reveals = srv->num_reveals; + memcpy(dst->value, srv->value, sizeof(dst->value)); + tor_free(srv); + ret = 0; + + error: + SMARTLIST_FOREACH(args, char *, s, tor_free(s)); + smartlist_free(args); + return ret; +} + +/* Parse both SharedRandCurrentValue and SharedRandPreviousValue line from + * the state. Return 0 on success else -1. */ +static int +disk_state_parse_sr_values(sr_state_t *state, + const sr_disk_state_t *disk_state) +{ + /* Only one value per type (current or previous) is allowed so we keep + * track of it with these flag. */ + unsigned int seen_previous = 0, seen_current = 0; + config_line_t *line; + sr_srv_t *srv = NULL; + + tor_assert(state); + tor_assert(disk_state); + + for (line = disk_state->SharedRandValues; line; line = line->next) { + if (line->value == NULL) { + continue; + } + srv = tor_malloc_zero(sizeof(*srv)); + if (disk_state_parse_srv(line->value, srv) < 0) { + log_warn(LD_BUG, "SR: Broken current SRV line in state %s", + escaped(line->value)); + goto bad; + } + if (!strcasecmp(line->key, dstate_prev_srv_key)) { + if (seen_previous) { + log_warn(LD_DIR, "SR: Second previous SRV value seen. Bad state"); + goto bad; + } + state->previous_srv = srv; + seen_previous = 1; + } else if (!strcasecmp(line->key, dstate_cur_srv_key)) { + if (seen_current) { + log_warn(LD_DIR, "SR: Second current SRV value seen. Bad state"); + goto bad; + } + state->current_srv = srv; + seen_current = 1; + } else { + /* Unknown key. Ignoring. */ + tor_free(srv); + } + } + + return 0; + bad: + tor_free(srv); + return -1; +} + +/* Parse the given disk state and set a newly allocated state. On success, + * return that state else NULL. */ +static sr_state_t * +disk_state_parse(const sr_disk_state_t *new_disk_state) +{ + sr_state_t *new_state = state_new(default_fname, time(NULL)); + + tor_assert(new_disk_state); + + new_state->version = new_disk_state->Version; + new_state->valid_until = new_disk_state->ValidUntil; + new_state->valid_after = new_disk_state->ValidAfter; + + /* Set our current phase according to the valid-after time in our disk + * state. The disk state we are parsing contains everything for the phase + * starting at valid_after so make sure our phase reflects that. */ + new_state->phase = get_sr_protocol_phase(new_state->valid_after); + + /* Parse the shared random values. */ + if (disk_state_parse_sr_values(new_state, new_disk_state) < 0) { + goto error; + } + /* Parse the commits. */ + if (disk_state_parse_commits(new_state, new_disk_state) < 0) { + goto error; + } + /* Great! This new state contains everything we had on disk. */ + return new_state; + + error: + state_free(new_state); + return NULL; +} + +/* From a valid commit object and an allocated config line, set the line's + * value to the state string representation of a commit. */ +static void +disk_state_put_commit_line(const sr_commit_t *commit, config_line_t *line) +{ + char *reveal_str = NULL; + + tor_assert(commit); + tor_assert(line); + + if (!tor_mem_is_zero(commit->encoded_reveal, + sizeof(commit->encoded_reveal))) { + /* Add extra whitespace so we can format the line correctly. */ + tor_asprintf(&reveal_str, " %s", commit->encoded_reveal); + } + tor_asprintf(&line->value, "%u %s %s %s%s", + SR_PROTO_VERSION, + crypto_digest_algorithm_get_name(commit->alg), + sr_commit_get_rsa_fpr(commit), + commit->encoded_commit, + reveal_str != NULL ? reveal_str : ""); + if (reveal_str != NULL) { + memwipe(reveal_str, 0, strlen(reveal_str)); + tor_free(reveal_str); + } +} + +/* From a valid srv object and an allocated config line, set the line's + * value to the state string representation of a shared random value. */ +static void +disk_state_put_srv_line(const sr_srv_t *srv, config_line_t *line) +{ + char encoded[SR_SRV_VALUE_BASE64_LEN + 1]; + + tor_assert(line); + + /* No SRV value thus don't add the line. This is possible since we might + * not have a current or previous SRV value in our state. */ + if (srv == NULL) { + return; + } + sr_srv_encode(encoded, sizeof(encoded), srv); + tor_asprintf(&line->value, "%" PRIu64 " %s", srv->num_reveals, encoded); +} + +/* Reset disk state that is free allocated memory and zeroed the object. */ +static void +disk_state_reset(void) +{ + /* Free allocated memory */ + config_free_lines(sr_disk_state->Commit); + config_free_lines(sr_disk_state->SharedRandValues); + config_free_lines(sr_disk_state->ExtraLines); + tor_free(sr_disk_state->TorVersion); + + /* Clean up the struct */ + memset(sr_disk_state, 0, sizeof(*sr_disk_state)); + + /* Reset it with useful data */ + sr_disk_state->magic_ = SR_DISK_STATE_MAGIC; + sr_disk_state->TorVersion = tor_strdup(get_version()); +} + +/* Update our disk state based on our global SR state. */ +static void +disk_state_update(void) +{ + config_line_t **next, *line; + + if (BUG(!sr_disk_state)) + return; + if (BUG(!sr_state)) + return; + + /* Reset current disk state. */ + disk_state_reset(); + + /* First, update elements that we don't need to do a construction. */ + sr_disk_state->Version = sr_state->version; + sr_disk_state->ValidUntil = sr_state->valid_until; + sr_disk_state->ValidAfter = sr_state->valid_after; + + /* Shared random values. */ + next = &sr_disk_state->SharedRandValues; + if (sr_state->previous_srv != NULL) { + *next = line = tor_malloc_zero(sizeof(config_line_t)); + line->key = tor_strdup(dstate_prev_srv_key); + disk_state_put_srv_line(sr_state->previous_srv, line); + /* Go to the next shared random value. */ + next = &(line->next); + } + if (sr_state->current_srv != NULL) { + *next = line = tor_malloc_zero(sizeof(*line)); + line->key = tor_strdup(dstate_cur_srv_key); + disk_state_put_srv_line(sr_state->current_srv, line); + } + + /* Parse the commits and construct config line(s). */ + next = &sr_disk_state->Commit; + DIGESTMAP_FOREACH(sr_state->commits, key, sr_commit_t *, commit) { + *next = line = tor_malloc_zero(sizeof(*line)); + line->key = tor_strdup(dstate_commit_key); + disk_state_put_commit_line(commit, line); + next = &(line->next); + } DIGESTMAP_FOREACH_END; +} + +/* Load state from disk and put it into our disk state. If the state passes + * validation, our global state will be updated with it. Return 0 on + * success. On error, -EINVAL is returned if the state on disk did contained + * something malformed or is unreadable. -ENOENT is returned indicating that + * the state file is either empty of non existing. */ +static int +disk_state_load_from_disk(void) +{ + int ret; + char *fname; + + fname = get_datadir_fname(default_fname); + ret = disk_state_load_from_disk_impl(fname); + tor_free(fname); + + return ret; +} + +/* Helper for disk_state_load_from_disk(). */ +STATIC int +disk_state_load_from_disk_impl(const char *fname) +{ + int ret; + char *content = NULL; + sr_state_t *parsed_state = NULL; + sr_disk_state_t *disk_state = NULL; + + /* Read content of file so we can parse it. */ + if ((content = read_file_to_str(fname, 0, NULL)) == NULL) { + log_warn(LD_FS, "SR: Unable to read SR state file %s", + escaped(fname)); + ret = -errno; + goto error; + } + + { + config_line_t *lines = NULL; + char *errmsg = NULL; + + /* Every error in this code path will return EINVAL. */ + ret = -EINVAL; + if (config_get_lines(content, &lines, 0) < 0) { + config_free_lines(lines); + goto error; + } + + disk_state = disk_state_new(time(NULL)); + config_assign(&state_format, disk_state, lines, 0, &errmsg); + config_free_lines(lines); + if (errmsg) { + log_warn(LD_DIR, "SR: Reading state error: %s", errmsg); + tor_free(errmsg); + goto error; + } + } + + /* So far so good, we've loaded our state file into our disk state. Let's + * validate it and then parse it. */ + if (disk_state_validate(disk_state) < 0) { + ret = -EINVAL; + goto error; + } + + parsed_state = disk_state_parse(disk_state); + if (parsed_state == NULL) { + ret = -EINVAL; + goto error; + } + state_set(parsed_state); + disk_state_set(disk_state); + tor_free(content); + log_info(LD_DIR, "SR: State loaded successfully from file %s", fname); + return 0; + + error: + disk_state_free(disk_state); + tor_free(content); + return ret; +} + +/* Save the disk state to disk but before that update it from the current + * state so we always have the latest. Return 0 on success else -1. */ +static int +disk_state_save_to_disk(void) +{ + int ret; + char *state, *content = NULL, *fname = NULL; + char tbuf[ISO_TIME_LEN + 1]; + time_t now = time(NULL); + + /* If we didn't have the opportunity to setup an internal disk state, + * don't bother saving something to disk. */ + if (sr_disk_state == NULL) { + ret = 0; + goto done; + } + + /* Make sure that our disk state is up to date with our memory state + * before saving it to disk. */ + disk_state_update(); + state = config_dump(&state_format, NULL, sr_disk_state, 0, 0); + format_local_iso_time(tbuf, now); + tor_asprintf(&content, + "# Tor shared random state file last generated on %s " + "local time\n" + "# Other times below are in UTC\n" + "# Please *do not* edit this file.\n\n%s", + tbuf, state); + tor_free(state); + fname = get_datadir_fname(default_fname); + if (write_str_to_file(fname, content, 0) < 0) { + log_warn(LD_FS, "SR: Unable to write SR state to file %s", fname); + ret = -1; + goto done; + } + ret = 0; + log_debug(LD_DIR, "SR: Saved state to file %s", fname); + + done: + tor_free(fname); + tor_free(content); + return ret; +} + +/* Reset our state to prepare for a new protocol run. Once this returns, all + * commits in the state will be removed and freed. */ +STATIC void +reset_state_for_new_protocol_run(time_t valid_after) +{ + if (BUG(!sr_state)) + return; + + /* Keep counters in track */ + sr_state->n_reveal_rounds = 0; + sr_state->n_commit_rounds = 0; + sr_state->n_protocol_runs++; + + /* Reset valid-until */ + sr_state->valid_until = get_state_valid_until_time(valid_after); + sr_state->valid_after = valid_after; + + /* We are in a new protocol run so cleanup commits. */ + sr_state_delete_commits(); +} + +/* This is the first round of the new protocol run starting at + * valid_after. Do the necessary housekeeping. */ +STATIC void +new_protocol_run(time_t valid_after) +{ + sr_commit_t *our_commitment = NULL; + + /* Only compute the srv at the end of the reveal phase. */ + if (sr_state->phase == SR_PHASE_REVEAL) { + /* We are about to compute a new shared random value that will be set in + * our state as the current value so rotate values. */ + state_rotate_srv(); + /* Compute the shared randomness value of the day. */ + sr_compute_srv(); + } + + /* Prepare for the new protocol run by reseting the state */ + reset_state_for_new_protocol_run(valid_after); + + /* Do some logging */ + log_info(LD_DIR, "SR: Protocol run #%" PRIu64 " starting!", + sr_state->n_protocol_runs); + + /* Generate fresh commitments for this protocol run */ + our_commitment = sr_generate_our_commit(valid_after, + get_my_v3_authority_cert()); + if (our_commitment) { + /* Add our commitment to our state. In case we are unable to create one + * (highly unlikely), we won't vote for this protocol run since our + * commitment won't be in our state. */ + sr_state_add_commit(our_commitment); + } +} + +/* Return 1 iff the next_phase is a phase transition from the current + * phase that is it's different. */ +STATIC int +is_phase_transition(sr_phase_t next_phase) +{ + return sr_state->phase != next_phase; +} + +/* Helper function: return a commit using the RSA fingerprint of the + * authority or NULL if no such commit is known. */ +static sr_commit_t * +state_query_get_commit(const char *rsa_fpr) +{ + tor_assert(rsa_fpr); + return digestmap_get(sr_state->commits, rsa_fpr); +} + +/* Helper function: This handles the GET state action using an + * obj_type and data needed for the action. */ +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) { + case SR_STATE_OBJ_COMMIT: + { + obj = state_query_get_commit(data); + break; + } + case SR_STATE_OBJ_COMMITS: + obj = sr_state->commits; + break; + case SR_STATE_OBJ_CURSRV: + obj = sr_state->current_srv; + break; + case SR_STATE_OBJ_PREVSRV: + obj = sr_state->previous_srv; + break; + case SR_STATE_OBJ_PHASE: + obj = &sr_state->phase; + break; + case SR_STATE_OBJ_VALID_AFTER: + default: + tor_assert(0); + } + return obj; +} + +/* 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) +{ ++ if (BUG(!sr_state)) ++ return; ++ + switch (obj_type) { + case SR_STATE_OBJ_COMMIT: + { + 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: - 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: - 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); + break; + /* It's not allowed to change the phase nor the full commitments map from + * the state. The phase is decided during a strict process post voting and + * the commits should be put individually. */ + case SR_STATE_OBJ_PHASE: + case SR_STATE_OBJ_COMMITS: + default: + tor_assert(0); + } +} + +/* Helper function: This handles the DEL_ALL state action using an + * obj_type and data needed for the action. */ +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: + { + /* We are in a new protocol run so cleanup commitments. */ + DIGESTMAP_FOREACH_MODIFY(sr_state->commits, key, sr_commit_t *, c) { + sr_commit_free(c); + MAP_DEL_CURRENT(key); + } DIGESTMAP_FOREACH_END; + break; + } + /* The following object are _NOT_ suppose to be removed. */ + case SR_STATE_OBJ_CURSRV: + case SR_STATE_OBJ_PREVSRV: + case SR_STATE_OBJ_PHASE: + case SR_STATE_OBJ_COMMITS: + case SR_STATE_OBJ_VALID_AFTER: + default: + tor_assert(0); + } +} + +/* Helper function: This handles the DEL state action using an + * obj_type and data needed for the action. */ +static void +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); + break; + case SR_STATE_OBJ_CURSRV: + tor_free(sr_state->current_srv); + break; + case SR_STATE_OBJ_COMMIT: + case SR_STATE_OBJ_COMMITS: + case SR_STATE_OBJ_PHASE: + case SR_STATE_OBJ_VALID_AFTER: + default: + tor_assert(0); + } +} + +/* Query state using an action for an object type obj_type. + * The data pointer needs to point to an object that the action needs + * to use and if anything is required to be returned, it is stored in + * out. + * + * This mechanism exists so we have one single point where we synchronized + * our memory state with our disk state for every actions that changes it. + * We then trigger a write on disk immediately. + * + * This should be the only entry point to our memory state. It's used by all + * our state accessors and should be in the future. */ +static void +state_query(sr_state_action_t action, sr_state_object_t obj_type, + void *data, void **out) +{ + switch (action) { + case SR_STATE_ACTION_GET: + *out = state_query_get_(obj_type, data); + break; + case SR_STATE_ACTION_PUT: + state_query_put_(obj_type, data); + break; + case SR_STATE_ACTION_DEL: + state_query_del_(obj_type, data); + break; + case SR_STATE_ACTION_DEL_ALL: + state_query_del_all_(obj_type); + break; + case SR_STATE_ACTION_SAVE: + /* Only trigger a disk state save. */ + break; + default: + tor_assert(0); + } + + /* If the action actually changes the state, immediately save it to disk. + * The following will sync the state -> disk state and then save it. */ + if (action != SR_STATE_ACTION_GET) { + disk_state_save_to_disk(); + } +} + +/* Delete the current SRV value from the state freeing it and the value is set + * to NULL meaning empty. */ - static void ++STATIC void +state_del_current_srv(void) +{ + state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_CURSRV, NULL, NULL); +} + +/* Delete the previous SRV value from the state freeing it and the value is + * set to NULL meaning empty. */ - static void ++STATIC void +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(sr_srv_dup(sr_state_get_current_srv())); ++ /* Free and NULL the current srv. */ + sr_state_set_current_srv(NULL); +} + +/* Set valid after time in the our state. */ +void +sr_state_set_valid_after(time_t valid_after) +{ + state_query(SR_STATE_ACTION_PUT, SR_STATE_OBJ_VALID_AFTER, + (void *) &valid_after, NULL); +} + +/* Return the phase we are currently in according to our state. */ +sr_phase_t +sr_state_get_phase(void) +{ + void *ptr; + state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_PHASE, NULL, &ptr); + 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) +{ + const sr_srv_t *srv; + state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_PREVSRV, NULL, + (void *) &srv); + return srv; +} + +/* Set the current SRV value from our state. Value CAN be NULL. The srv + * object ownership is transferred to the state object. */ +void +sr_state_set_previous_srv(const sr_srv_t *srv) +{ + state_query(SR_STATE_ACTION_PUT, SR_STATE_OBJ_PREVSRV, (void *) 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) +{ + const sr_srv_t *srv; + state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_CURSRV, NULL, + (void *) &srv); + return srv; +} + +/* Set the current SRV value from our state. Value CAN be NULL. The srv + * object ownership is transferred to the state object. */ +void +sr_state_set_current_srv(const sr_srv_t *srv) +{ + state_query(SR_STATE_ACTION_PUT, SR_STATE_OBJ_CURSRV, (void *) srv, + NULL); +} + +/* Clean all the SRVs in our state. */ +void +sr_state_clean_srvs(void) +{ + /* Remove SRVs from state. They will be set to NULL as "empty". */ + state_del_previous_srv(); + state_del_current_srv(); +} + +/* Return a pointer to the commits map from our state. CANNOT be NULL. */ +digestmap_t * +sr_state_get_commits(void) +{ + digestmap_t *commits; + state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_COMMITS, + NULL, (void *) &commits); + tor_assert(commits); + return commits; +} + +/* Update the current SR state as needed for the upcoming voting round at + * valid_after. */ +void +sr_state_update(time_t valid_after) +{ + sr_phase_t next_phase; + + if (BUG(!sr_state)) + return; + + /* Don't call this function twice in the same voting period. */ + if (valid_after <= sr_state->valid_after) { + log_info(LD_DIR, "SR: Asked to update state twice. Ignoring."); + return; + } + + /* Get phase of upcoming round. */ + next_phase = get_sr_protocol_phase(valid_after); + + /* If we are transitioning to a new protocol phase, prepare the stage. */ + if (is_phase_transition(next_phase)) { + if (next_phase == SR_PHASE_COMMIT) { + /* Going into commit phase means we are starting a new protocol run. */ + new_protocol_run(valid_after); + } + /* Set the new phase for this round */ + sr_state->phase = next_phase; + } else if (sr_state->phase == SR_PHASE_COMMIT && + digestmap_size(sr_state->commits) == 0) { + /* We are _NOT_ in a transition phase so if we are in the commit phase + * and have no commit, generate one. Chances are that we are booting up + * so let's have a commit in our state for the next voting period. */ + sr_commit_t *our_commit = + sr_generate_our_commit(valid_after, get_my_v3_authority_cert()); + if (our_commit) { + /* Add our commitment to our state. In case we are unable to create one + * (highly unlikely), we won't vote for this protocol run since our + * commitment won't be in our state. */ + sr_state_add_commit(our_commit); + } + } + + sr_state_set_valid_after(valid_after); + + /* Count the current round */ + if (sr_state->phase == SR_PHASE_COMMIT) { + /* invariant check: we've not entered reveal phase yet */ + if (BUG(sr_state->n_reveal_rounds != 0)) + return; + sr_state->n_commit_rounds++; + } else { + sr_state->n_reveal_rounds++; + } + + { /* Debugging. */ + char tbuf[ISO_TIME_LEN + 1]; + format_iso_time(tbuf, valid_after); + log_info(LD_DIR, "SR: State prepared for upcoming voting period (%s). " + "Upcoming phase is %s (counters: %d commit & %d reveal rounds).", + tbuf, get_phase_str(sr_state->phase), + sr_state->n_commit_rounds, sr_state->n_reveal_rounds); + } +} + +/* Return commit object from the given authority digest rsa_identity. + * Return NULL if not found. */ +sr_commit_t * +sr_state_get_commit(const char *rsa_identity) +{ + sr_commit_t *commit; + + tor_assert(rsa_identity); + + state_query(SR_STATE_ACTION_GET, SR_STATE_OBJ_COMMIT, + (void *) rsa_identity, (void *) &commit); + return commit; +} + +/* Add commit to the permanent state. The commit object ownership is + * transferred to the state so the caller MUST not free it. */ +void +sr_state_add_commit(sr_commit_t *commit) +{ + tor_assert(commit); + + /* Put the commit to the global state. */ + state_query(SR_STATE_ACTION_PUT, SR_STATE_OBJ_COMMIT, + (void *) commit, NULL); + + log_debug(LD_DIR, "SR: Commit from %s has been added to our state.", + sr_commit_get_rsa_fpr(commit)); +} + +/* Remove all commits from our state. */ +void +sr_state_delete_commits(void) +{ + state_query(SR_STATE_ACTION_DEL_ALL, SR_STATE_OBJ_COMMIT, NULL, NULL); +} + +/* Copy the reveal information from commit into saved_commit. + * This saved_commit MUST come from our current SR state. Once modified, + * the disk state is updated. */ +void +sr_state_copy_reveal_info(sr_commit_t *saved_commit, const sr_commit_t *commit) +{ + tor_assert(saved_commit); + tor_assert(commit); + + saved_commit->reveal_ts = commit->reveal_ts; + memcpy(saved_commit->random_number, commit->random_number, + sizeof(saved_commit->random_number)); + + strlcpy(saved_commit->encoded_reveal, commit->encoded_reveal, + sizeof(saved_commit->encoded_reveal)); + state_query(SR_STATE_ACTION_SAVE, 0, NULL, NULL); + log_debug(LD_DIR, "SR: Reveal value learned %s (for commit %s) from %s", + saved_commit->encoded_reveal, saved_commit->encoded_commit, + sr_commit_get_rsa_fpr(saved_commit)); +} + +/* Set the fresh SRV flag from our state. This doesn't need to trigger a + * disk state synchronization so we directly change the state. */ +void +sr_state_set_fresh_srv(void) +{ + sr_state->is_srv_fresh = 1; +} + +/* Unset the fresh SRV flag from our state. This doesn't need to trigger a + * disk state synchronization so we directly change the state. */ +void +sr_state_unset_fresh_srv(void) +{ + sr_state->is_srv_fresh = 0; +} + +/* Return the value of the fresh SRV flag. */ +unsigned int +sr_state_srv_is_fresh(void) +{ + return sr_state->is_srv_fresh; +} + +/* Cleanup and free our disk and memory state. */ +void +sr_state_free_all(void) +{ + state_free(sr_state); + disk_state_free(sr_disk_state); + /* Nullify our global state. */ + sr_state = NULL; + sr_disk_state = NULL; +} + +/* Save our current state in memory to disk. */ +void +sr_state_save(void) +{ + /* Query a SAVE action on our current state so it's synced and saved. */ + state_query(SR_STATE_ACTION_SAVE, 0, NULL, NULL); +} + +/* Return 1 iff the state has been initialized that is it exists in memory. + * Return 0 otherwise. */ +int +sr_state_is_initialized(void) +{ + return sr_state == NULL ? 0 : 1; +} + +/* Initialize the disk and memory state. + * + * If save_to_disk is set to 1, the state is immediately saved to disk after + * creation else it's not thus only kept in memory. + * If read_from_disk is set to 1, we try to load the state from the disk and + * if not found, a new state is created. + * + * Return 0 on success else a negative value on error. */ +int +sr_state_init(int save_to_disk, int read_from_disk) +{ + int ret = -ENOENT; + time_t now = time(NULL); + + /* We shouldn't have those assigned. */ + tor_assert(sr_disk_state == NULL); + tor_assert(sr_state == NULL); + + /* First, try to load the state from disk. */ + if (read_from_disk) { + ret = disk_state_load_from_disk(); + } + + if (ret < 0) { + switch (-ret) { + case EINVAL: + /* We have a state on disk but it contains something we couldn't parse + * or an invalid entry in the state file. Let's remove it since it's + * obviously unusable and replace it by an new fresh state below. */ + case ENOENT: + { + /* No state on disk so allocate our states for the first time. */ + sr_state_t *new_state = state_new(default_fname, now); + sr_disk_state_t *new_disk_state = disk_state_new(now); + state_set(new_state); + /* It's important to set our disk state pointer since the save call + * below uses it to synchronized it with our memory state. */ + disk_state_set(new_disk_state); + /* No entry, let's save our new state to disk. */ + if (save_to_disk && disk_state_save_to_disk() < 0) { + goto error; + } + break; + } + default: + /* Big problem. Not possible. */ + tor_assert(0); + } + } + /* We have a state in memory, let's make sure it's updated for the current + * and next voting round. */ + { + time_t valid_after = voting_schedule_get_next_valid_after_time(); + sr_state_update(valid_after); + } + return 0; + + error: + return -1; +} + +#ifdef TOR_UNIT_TESTS + +/* Set the current phase of the protocol. Used only by unit tests. */ +void +set_sr_phase(sr_phase_t phase) +{ + if (BUG(!sr_state)) + return; + sr_state->phase = phase; +} + +/* Get the SR state. Used only by unit tests */ +sr_state_t * +get_sr_state(void) +{ + return sr_state; +} + +#endif /* defined(TOR_UNIT_TESTS) */ diff --cc src/feature/dirauth/shared_random_state.h index 35626be3f,000000000..08f999f9d mode 100644,000000..100644 --- a/src/feature/dirauth/shared_random_state.h +++ b/src/feature/dirauth/shared_random_state.h @@@ -1,146 -1,0 +1,148 @@@ +/* Copyright (c) 2016-2019, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#ifndef TOR_SHARED_RANDOM_STATE_H +#define TOR_SHARED_RANDOM_STATE_H + +#include "feature/dirauth/shared_random.h" + +/* Action that can be performed on the state for any objects. */ +typedef enum { + SR_STATE_ACTION_GET = 1, + SR_STATE_ACTION_PUT = 2, + SR_STATE_ACTION_DEL = 3, + SR_STATE_ACTION_DEL_ALL = 4, + SR_STATE_ACTION_SAVE = 5, +} sr_state_action_t; + +/* Object in the state that can be queried through the state API. */ +typedef enum { + /* Will return a single commit using an authority identity key. */ + SR_STATE_OBJ_COMMIT, + /* Returns the entire list of commits from the state. */ + SR_STATE_OBJ_COMMITS, + /* Return the current SRV object pointer. */ + SR_STATE_OBJ_CURSRV, + /* Return the previous SRV object pointer. */ + SR_STATE_OBJ_PREVSRV, + /* Return the phase. */ + SR_STATE_OBJ_PHASE, + /* Get or Put the valid after time. */ + SR_STATE_OBJ_VALID_AFTER, +} sr_state_object_t; + +/* State of the protocol. It's also saved on disk in fname. This data + * structure MUST be synchronized at all time with the one on disk. */ +typedef struct sr_state_t { + /* Filename of the state file on disk. */ + char *fname; + /* Version of the protocol. */ + uint32_t version; + /* The valid-after of the voting period we have prepared the state for. */ + time_t valid_after; + /* Until when is this state valid? */ + time_t valid_until; + /* Protocol phase. */ + sr_phase_t phase; + + /* Number of runs completed. */ + uint64_t n_protocol_runs; + /* The number of commitment rounds we've performed in this protocol run. */ + unsigned int n_commit_rounds; + /* The number of reveal rounds we've performed in this protocol run. */ + unsigned int n_reveal_rounds; + + /* A map of all the received commitments for this protocol run. This is + * indexed by authority RSA identity digest. */ + digestmap_t *commits; + + /* Current and previous shared random value. */ + sr_srv_t *previous_srv; + sr_srv_t *current_srv; + + /* Indicate if the state contains an SRV that was _just_ generated. This is + * used during voting so that we know whether to use the super majority rule + * or not when deciding on keeping it for the consensus. It is _always_ set + * to 0 post consensus. + * + * EDGE CASE: if an authority computes a new SRV then immediately reboots + * and, once back up, votes for the current round, it won't know if the + * SRV is fresh or not ultimately making it _NOT_ use the super majority + * when deciding to put or not the SRV in the consensus. This is for now + * an acceptable very rare edge case. */ + unsigned int is_srv_fresh:1; +} sr_state_t; + +/* Persistent state of the protocol, as saved to disk. */ +typedef struct sr_disk_state_t { + uint32_t magic_; + /* Version of the protocol. */ + int Version; + /* Version of our running tor. */ + char *TorVersion; + /* Creation time of this state */ + time_t ValidAfter; + /* State valid until? */ + time_t ValidUntil; + /* All commits seen that are valid. */ + struct config_line_t *Commit; + /* Previous and current shared random value. */ + struct config_line_t *SharedRandValues; + /* Extra Lines for configuration we might not know. */ + struct config_line_t *ExtraLines; +} sr_disk_state_t; + +/* API */ + +/* Public methods: */ + +void sr_state_update(time_t valid_after); + +/* Private methods (only used by shared-random.c): */ + +void sr_state_set_valid_after(time_t valid_after); +sr_phase_t sr_state_get_phase(void); +const sr_srv_t *sr_state_get_previous_srv(void); +const sr_srv_t *sr_state_get_current_srv(void); +void sr_state_set_previous_srv(const sr_srv_t *srv); +void sr_state_set_current_srv(const sr_srv_t *srv); +void sr_state_clean_srvs(void); +digestmap_t *sr_state_get_commits(void); +sr_commit_t *sr_state_get_commit(const char *rsa_fpr); +void sr_state_add_commit(sr_commit_t *commit); +void sr_state_delete_commits(void); +void sr_state_copy_reveal_info(sr_commit_t *saved_commit, + const sr_commit_t *commit); +unsigned int sr_state_srv_is_fresh(void); +void sr_state_set_fresh_srv(void); +void sr_state_unset_fresh_srv(void); +int sr_state_init(int save_to_disk, int read_from_disk); +int sr_state_is_initialized(void); +void sr_state_save(void); +void sr_state_free_all(void); + +#ifdef SHARED_RANDOM_STATE_PRIVATE + +STATIC int disk_state_load_from_disk_impl(const char *fname); + +STATIC sr_phase_t get_sr_protocol_phase(time_t valid_after); + +STATIC time_t get_state_valid_until_time(time_t now); +STATIC const char *get_phase_str(sr_phase_t phase); +STATIC void reset_state_for_new_protocol_run(time_t valid_after); +STATIC void new_protocol_run(time_t valid_after); +STATIC void state_rotate_srv(void); +STATIC int is_phase_transition(sr_phase_t next_phase); + +#endif /* defined(SHARED_RANDOM_STATE_PRIVATE) */ + +#ifdef TOR_UNIT_TESTS + +STATIC void set_sr_phase(sr_phase_t phase); +STATIC sr_state_t *get_sr_state(void); ++STATIC void state_del_previous_srv(void); ++STATIC void state_del_current_srv(void); + +#endif /* defined(TOR_UNIT_TESTS) */ + +#endif /* !defined(TOR_SHARED_RANDOM_STATE_H) */

1 0

[tor/maint-0.4.0] Merge branch bug29706_029_refactor into bug29706_034_refactor
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit c7854933e9d8a6634beb7079e7c8af3266a362a8 Merge: 155b0f552 9eeff921a Author: teor <teor(a)torproject.org> Date: Tue Mar 12 11:31:52 2019 +1000 Merge branch bug29706_029_refactor into bug29706_034_refactor changes/bug29706_minimal | 4 + changes/bug29706_refactor | 4 + src/or/dirauth/shared_random.c | 8 +- src/or/dirauth/shared_random.h | 2 +- src/or/dirauth/shared_random_state.c | 60 +++++++-- src/or/dirauth/shared_random_state.h | 2 + src/test/test_shared_random.c | 245 ++++++++++++++++++++++++++++++++--- 7 files changed, 290 insertions(+), 35 deletions(-) diff --cc src/or/dirauth/shared_random_state.h index 60a326f86,cf027f2d3..d31983d18 --- a/src/or/dirauth/shared_random_state.h +++ b/src/or/dirauth/shared_random_state.h @@@ -140,8 -140,10 +140,10 @@@ STATIC int is_phase_transition(sr_phase STATIC void set_sr_phase(sr_phase_t phase); STATIC sr_state_t *get_sr_state(void); + STATIC void state_del_previous_srv(void); + STATIC void state_del_current_srv(void); -#endif /* TOR_UNIT_TESTS */ +#endif /* defined(TOR_UNIT_TESTS) */ -#endif /* TOR_SHARED_RANDOM_STATE_H */ +#endif /* !defined(TOR_SHARED_RANDOM_STATE_H) */ diff --cc src/test/test_shared_random.c index c04e4660c,c81b6ec2d..45f0e1db7 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@@ -1031,9 -944,9 +1039,9 @@@ test_utils_general(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_u64_op(dup_srv->num_reveals, OP_EQ, srv->num_reveals); tt_mem_op(dup_srv->value, OP_EQ, srv->value, sizeof(srv->value)); tor_free(srv); tor_free(dup_srv); @@@ -1078,25 -991,220 +1086,220 @@@ /* Testing get_phase_str(). */ { - tt_str_op(get_phase_str(SR_PHASE_REVEAL), ==, "reveal"); - tt_str_op(get_phase_str(SR_PHASE_COMMIT), ==, "commit"); + tt_str_op(get_phase_str(SR_PHASE_REVEAL), OP_EQ, "reveal"); + tt_str_op(get_phase_str(SR_PHASE_COMMIT), OP_EQ, "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); + tt_int_op(is_phase_transition(SR_PHASE_REVEAL), OP_EQ, 1); + tt_int_op(is_phase_transition(SR_PHASE_COMMIT), OP_EQ, 0); set_sr_phase(SR_PHASE_REVEAL); - tt_int_op(is_phase_transition(SR_PHASE_REVEAL), ==, 0); - tt_int_op(is_phase_transition(SR_PHASE_COMMIT), ==, 1); + tt_int_op(is_phase_transition(SR_PHASE_REVEAL), OP_EQ, 0); + tt_int_op(is_phase_transition(SR_PHASE_COMMIT), OP_EQ, 1); /* Junk. */ - tt_int_op(is_phase_transition(42), ==, 1); + tt_int_op(is_phase_transition(42), OP_EQ, 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 @@@ -1142,16 -1251,18 +1346,18 @@@ 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 = sr_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()); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); sr_state_clean_srvs(); + tor_free(cur); } /* New protocol run. */ @@@ -1173,13 -1284,14 +1379,14 @@@ * 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); + tt_int_op(digestmap_size(state->commits), OP_EQ, 1); + tt_int_op(state->n_reveal_rounds, OP_EQ, 0); + tt_int_op(state->n_commit_rounds, OP_EQ, 0); /* 46 here since we were at 45 just before. */ - tt_u64_op(state->n_protocol_runs, ==, 46); + tt_u64_op(state->n_protocol_runs, OP_EQ, 46); + tor_free(cur); } /* Cleanup of SRVs. */ @@@ -1190,7 -1302,8 +1397,8 @@@ } done: + tor_free(cur); - sr_state_free(); + sr_state_free_all(); } static void

1 0

[tor/master] test/sr: Free SRVs before replacing them in test_sr_setup_srv()
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit 9400da9b5e44bfce0684a3b36edb37465be514d6 Author: teor <teor(a)torproject.org> Date: Sat Mar 9 10:50:07 2019 +1000 test/sr: Free SRVs before replacing them in test_sr_setup_srv() Stop leaking parts of the shared random state in the shared-random unit tests. The previous fix in 29599 was incomplete. Fixes bug 29706; bugfix on 0.2.9.1-alpha. --- changes/bug29706_minimal | 4 ++++ src/or/shared_random_state.c | 4 ++-- src/or/shared_random_state.h | 2 ++ src/test/test_shared_random.c | 4 ++++ 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/changes/bug29706_minimal b/changes/bug29706_minimal new file mode 100644 index 000000000..9d4a43326 --- /dev/null +++ b/changes/bug29706_minimal @@ -0,0 +1,4 @@ + o Minor bugfixes (memory management, testing): + - Stop leaking parts of the shared random state in the shared-random unit + tests. The previous fix in 29599 was incomplete. + Fixes bug 29706; bugfix on 0.2.9.1-alpha. diff --git a/src/or/shared_random_state.c b/src/or/shared_random_state.c index 8438d4640..f27eccafc 100644 --- a/src/or/shared_random_state.c +++ b/src/or/shared_random_state.c @@ -1007,7 +1007,7 @@ state_query(sr_state_action_t action, sr_state_object_t obj_type, /* Delete the current SRV value from the state freeing it and the value is set * to NULL meaning empty. */ -static void +STATIC void state_del_current_srv(void) { state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_CURSRV, NULL, NULL); @@ -1015,7 +1015,7 @@ state_del_current_srv(void) /* Delete the previous SRV value from the state freeing it and the value is * set to NULL meaning empty. */ -static void +STATIC void state_del_previous_srv(void) { state_query(SR_STATE_ACTION_DEL, SR_STATE_OBJ_PREVSRV, NULL, NULL); diff --git a/src/or/shared_random_state.h b/src/or/shared_random_state.h index 43a7f1d28..cf027f2d3 100644 --- a/src/or/shared_random_state.h +++ b/src/or/shared_random_state.h @@ -140,6 +140,8 @@ STATIC int is_phase_transition(sr_phase_t next_phase); STATIC void set_sr_phase(sr_phase_t phase); STATIC sr_state_t *get_sr_state(void); +STATIC void state_del_previous_srv(void); +STATIC void state_del_current_srv(void); #endif /* TOR_UNIT_TESTS */ diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c index cebe772d9..0a3c2e119 100644 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@ -461,6 +461,8 @@ test_sr_setup_srv(int also_current) "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ", sizeof(srv->value)); + /* sr_state_set_previous_srv() does not free() the old previous srv. */ + state_del_previous_srv(); sr_state_set_previous_srv(srv); if (also_current) { @@ -470,6 +472,8 @@ test_sr_setup_srv(int also_current) "NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN", sizeof(srv->value)); + /* sr_state_set_previous_srv() does not free() the old current srv. */ + state_del_current_srv(); sr_state_set_current_srv(srv); } }

1 0

[tor/master] 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/master] 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/master] 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/master] 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

[tor/master] Merge branch bug29706_029_refactor into bug29706_034_refactor
by nickm＠torproject.org 12 Mar '19

12 Mar '19

commit c7854933e9d8a6634beb7079e7c8af3266a362a8 Merge: 155b0f552 9eeff921a Author: teor <teor(a)torproject.org> Date: Tue Mar 12 11:31:52 2019 +1000 Merge branch bug29706_029_refactor into bug29706_034_refactor changes/bug29706_minimal | 4 + changes/bug29706_refactor | 4 + src/or/dirauth/shared_random.c | 8 +- src/or/dirauth/shared_random.h | 2 +- src/or/dirauth/shared_random_state.c | 60 +++++++-- src/or/dirauth/shared_random_state.h | 2 + src/test/test_shared_random.c | 245 ++++++++++++++++++++++++++++++++--- 7 files changed, 290 insertions(+), 35 deletions(-) diff --cc src/or/dirauth/shared_random_state.h index 60a326f86,cf027f2d3..d31983d18 --- a/src/or/dirauth/shared_random_state.h +++ b/src/or/dirauth/shared_random_state.h @@@ -140,8 -140,10 +140,10 @@@ STATIC int is_phase_transition(sr_phase STATIC void set_sr_phase(sr_phase_t phase); STATIC sr_state_t *get_sr_state(void); + STATIC void state_del_previous_srv(void); + STATIC void state_del_current_srv(void); -#endif /* TOR_UNIT_TESTS */ +#endif /* defined(TOR_UNIT_TESTS) */ -#endif /* TOR_SHARED_RANDOM_STATE_H */ +#endif /* !defined(TOR_SHARED_RANDOM_STATE_H) */ diff --cc src/test/test_shared_random.c index c04e4660c,c81b6ec2d..45f0e1db7 --- a/src/test/test_shared_random.c +++ b/src/test/test_shared_random.c @@@ -1031,9 -944,9 +1039,9 @@@ test_utils_general(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_u64_op(dup_srv->num_reveals, OP_EQ, srv->num_reveals); tt_mem_op(dup_srv->value, OP_EQ, srv->value, sizeof(srv->value)); tor_free(srv); tor_free(dup_srv); @@@ -1078,25 -991,220 +1086,220 @@@ /* Testing get_phase_str(). */ { - tt_str_op(get_phase_str(SR_PHASE_REVEAL), ==, "reveal"); - tt_str_op(get_phase_str(SR_PHASE_COMMIT), ==, "commit"); + tt_str_op(get_phase_str(SR_PHASE_REVEAL), OP_EQ, "reveal"); + tt_str_op(get_phase_str(SR_PHASE_COMMIT), OP_EQ, "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); + tt_int_op(is_phase_transition(SR_PHASE_REVEAL), OP_EQ, 1); + tt_int_op(is_phase_transition(SR_PHASE_COMMIT), OP_EQ, 0); set_sr_phase(SR_PHASE_REVEAL); - tt_int_op(is_phase_transition(SR_PHASE_REVEAL), ==, 0); - tt_int_op(is_phase_transition(SR_PHASE_COMMIT), ==, 1); + tt_int_op(is_phase_transition(SR_PHASE_REVEAL), OP_EQ, 0); + tt_int_op(is_phase_transition(SR_PHASE_COMMIT), OP_EQ, 1); /* Junk. */ - tt_int_op(is_phase_transition(42), ==, 1); + tt_int_op(is_phase_transition(42), OP_EQ, 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 @@@ -1142,16 -1251,18 +1346,18 @@@ 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 = sr_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()); + tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL); sr_state_clean_srvs(); + tor_free(cur); } /* New protocol run. */ @@@ -1173,13 -1284,14 +1379,14 @@@ * 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); + tt_int_op(digestmap_size(state->commits), OP_EQ, 1); + tt_int_op(state->n_reveal_rounds, OP_EQ, 0); + tt_int_op(state->n_commit_rounds, OP_EQ, 0); /* 46 here since we were at 45 just before. */ - tt_u64_op(state->n_protocol_runs, ==, 46); + tt_u64_op(state->n_protocol_runs, OP_EQ, 46); + tor_free(cur); } /* Cleanup of SRVs. */ @@@ -1190,7 -1302,8 +1397,8 @@@ } done: + tor_free(cur); - sr_state_free(); + sr_state_free_all(); } static void

1 0

[tor/release-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