commit b9f849bdee20f36264fe061498536e0d3a8616a6 Author: George Kadianakis desnacked@riseup.net Date: Tue Aug 29 16:02:01 2017 +0300

prop224: Clear list of prev hsdirs before we upload all descs.

This fixes a serious bug in our hsdir set change logic:

We used to add nodes in the list of previous hsdirs everytime we uploaded to a new hsdir and we only cleared the list when we built a new descriptor. This means that our prev_hsdirs list could end up with 7 hsdirs, if for some reason we ended up uploading our desc to 7 hsdirs before rebuilding our descriptor (e.g. this can happen if the set of hsdirs changed).

After our previous hdsir set had 7 nodes, then our old algorithm would always think that the set has changed since it was comparing a smartlist with 7 elements against a smartlist with 6 elements.

This commit fixes this bug, by clearning the prev_hsdirs list before we upload to all hsdirs. This makes sure that our prev_hsdirs list always contains the latest hsdirs! --- src/or/hs_service.c | 4 ++++ src/test/test_hs_common.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/or/hs_service.c b/src/or/hs_service.c index 0fba0d7f9..218d49ace 100644 --- a/src/or/hs_service.c +++ b/src/or/hs_service.c @@ -2316,6 +2316,10 @@ upload_descriptor_to_all(const hs_service_t *service, hs_get_responsible_hsdirs(&desc->blinded_kp.pubkey, desc->time_period_num, for_next_period, 0, responsible_dirs);

+ /** Clear list of previous hsdirs since we are about to upload to a new + * list. Let's keep it up to date. */ + service_desc_clear_previous_hsdirs(desc); + /* For each responsible HSDir we have, initiate an upload command. */ SMARTLIST_FOREACH_BEGIN(responsible_dirs, const routerstatus_t *, hsdir_rs) { diff --git a/src/test/test_hs_common.c b/src/test/test_hs_common.c index 6bb03f39d..998089295 100644 --- a/src/test/test_hs_common.c +++ b/src/test/test_hs_common.c @@ -613,10 +613,10 @@ test_desc_reupload_logic(void *arg) tt_int_op(service_desc_hsdirs_changed(service, desc), OP_EQ, 1); tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6);

- /* Now order another upload and see how we end up with 7 hsdirs */ + /* Now order another upload and see that we keep having 6 prev hsdirs */ upload_descriptor_to_all(service, desc, 0); /* Check that previous hsdirs were populated */ - tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 7); + tt_int_op(smartlist_len(desc->previous_hsdirs), OP_EQ, 6);

/* Now restore the HSDir hash ring to its original state by swapping back aaron for nora */