commit d18a167ff38799ea5cd846dd80acccab6404952a Author: Neel Chauhan neel@neelc.org Date: Tue Sep 19 16:08:24 2017 -0400

sr: Switch from tor_assert() to BUG()

Closes #19566

Signed-off-by: David Goulet dgoulet@torproject.org --- changes/ticket19566 | 6 ++++++ src/feature/dirauth/shared_random.c | 3 ++- src/feature/dirauth/shared_random_state.c | 18 ++++++++++++------ 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/changes/ticket19566 b/changes/ticket19566 new file mode 100644 index 000000000..bf7071e66 --- /dev/null +++ b/changes/ticket19566 @@ -0,0 +1,6 @@ + o Code simplification and refactoring (shared random, dirauth): + - Change many tor_assert() to use BUG() instead. The idea is to not crash + a dirauth but rather scream loudly with a stacktrace and let it continue + run. The shared random subsystem is very resilient and if anything wrong + happens with it, at worst a non coherent value will be put in the vote + and discarded by the other authorities. Closes ticket 19566. diff --git a/src/feature/dirauth/shared_random.c b/src/feature/dirauth/shared_random.c index db4f9d328..b027d9e37 100644 --- a/src/feature/dirauth/shared_random.c +++ b/src/feature/dirauth/shared_random.c @@ -949,7 +949,8 @@ sr_compute_srv(void) /* 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. */ - tor_assert(sr_state_get_phase() == SR_PHASE_REVEAL); + if (BUG(sr_state_get_phase() != SR_PHASE_REVEAL)) + return; state_commits = sr_state_get_commits();

commits = smartlist_new(); diff --git a/src/feature/dirauth/shared_random_state.c b/src/feature/dirauth/shared_random_state.c index 38c7fd76d..7ae4a5dc8 100644 --- a/src/feature/dirauth/shared_random_state.c +++ b/src/feature/dirauth/shared_random_state.c @@ -594,8 +594,10 @@ disk_state_update(void) { config_line_t **next, *line;

- tor_assert(sr_disk_state); - tor_assert(sr_state); + if (BUG(!sr_disk_state)) + return; + if (BUG(!sr_state)) + return;

/* Reset current disk state. */ disk_state_reset(); @@ -759,7 +761,8 @@ disk_state_save_to_disk(void) STATIC void reset_state_for_new_protocol_run(time_t valid_after) { - tor_assert(sr_state); + if (BUG(!sr_state)) + return;

/* Keep counters in track */ sr_state->n_reveal_rounds = 0; @@ -1091,7 +1094,8 @@ sr_state_update(time_t valid_after) { sr_phase_t next_phase;

- tor_assert(sr_state); + if (BUG(!sr_state)) + return;

/* Don't call this function twice in the same voting period. */ if (valid_after <= sr_state->valid_after) { @@ -1130,7 +1134,8 @@ sr_state_update(time_t valid_after) /* Count the current round */ if (sr_state->phase == SR_PHASE_COMMIT) { /* invariant check: we've not entered reveal phase yet */ - tor_assert(sr_state->n_reveal_rounds == 0); + if (BUG(sr_state->n_reveal_rounds != 0)) + return; sr_state->n_commit_rounds++; } else { sr_state->n_reveal_rounds++; @@ -1320,7 +1325,8 @@ sr_state_init(int save_to_disk, int read_from_disk) void set_sr_phase(sr_phase_t phase) { - tor_assert(sr_state); + if (BUG(!sr_state)) + return; sr_state->phase = phase; }