From 0667a5af8df8f372f6482254ca24c66268ed02f8 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 30 Mar 2020 23:17:39 +0300 Subject: hs-v3: Don't crash after SIGHUP in Onionbalance backend mode. The ob_subcreds array was not copied after SIGHUP, and that left the post-SIGHUP service with a NULL ob_subcreds pointer (until the next descriptor gets build where we regenerate ob_subcreds in hs_ob_refresh_keys()). Fixes bug #33762; not in any released tor version. --- src/feature/hs/hs_service.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/feature/hs/hs_service.c') diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 3a2beb766f..367f8ca2b3 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -890,10 +890,18 @@ move_hs_state(hs_service_t *src_service, hs_service_t *dst_service) if (dst->replay_cache_rend_cookie != NULL) { replaycache_free(dst->replay_cache_rend_cookie); } + dst->replay_cache_rend_cookie = src->replay_cache_rend_cookie; + src->replay_cache_rend_cookie = NULL; /* steal pointer reference */ + dst->next_rotation_time = src->next_rotation_time; - src->replay_cache_rend_cookie = NULL; /* steal pointer reference */ + if (src_service->ob_subcreds) { + dst_service->ob_subcreds = src_service->ob_subcreds; + dst_service->n_ob_subcreds = src_service->n_ob_subcreds; + + src_service->ob_subcreds = NULL; /* steal pointer reference */ + } } /** Register services that are in the staging list. Once this function returns, -- cgit v1.2.3-54-g00ecf From 8fda94f944f00f6f436604038ce135ab70f4feb4 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Mon, 30 Mar 2020 23:24:33 +0300 Subject: hs-v3: Move ob_subcreds to hs_service_state_t. It's more natural there since it's runtime state. --- src/feature/hs/hs_circuit.c | 6 +++--- src/feature/hs/hs_ob.c | 8 ++++---- src/feature/hs/hs_service.c | 12 ++++++------ src/feature/hs/hs_service.h | 16 ++++++++-------- src/test/test_hs_service.c | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) (limited to 'src/feature/hs/hs_service.c') diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c index fdd226ba79..dc13c7045e 100644 --- a/src/feature/hs/hs_circuit.c +++ b/src/feature/hs/hs_circuit.c @@ -984,13 +984,13 @@ get_subcredential_for_handling_intro2_cell(const hs_service_t *service, /* This should not happen since we should have made onionbalance * subcredentials when we created our descriptors. */ - if (BUG(!service->ob_subcreds)) { + if (BUG(!service->state.ob_subcreds)) { return -1; } /* We are an onionbalance instance: */ - data->n_subcredentials = service->n_ob_subcreds; - data->subcredentials = service->ob_subcreds; + data->n_subcredentials = service->state.n_ob_subcreds; + data->subcredentials = service->state.ob_subcreds; return 0; } diff --git a/src/feature/hs/hs_ob.c b/src/feature/hs/hs_ob.c index f135ecd3f4..a6a5cec26f 100644 --- a/src/feature/hs/hs_ob.c +++ b/src/feature/hs/hs_ob.c @@ -392,12 +392,12 @@ hs_ob_refresh_keys(hs_service_t *service) } /* Delete old subcredentials if any */ - if (service->ob_subcreds) { - tor_free(service->ob_subcreds); + if (service->state.ob_subcreds) { + tor_free(service->state.ob_subcreds); } - service->ob_subcreds = ob_subcreds; - service->n_ob_subcreds = num_subcreds; + service->state.ob_subcreds = ob_subcreds; + service->state.n_ob_subcreds = num_subcreds; } /** Free any memory allocated by the onionblance subsystem. */ diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 367f8ca2b3..ed4d1af145 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -896,11 +896,11 @@ move_hs_state(hs_service_t *src_service, hs_service_t *dst_service) dst->next_rotation_time = src->next_rotation_time; - if (src_service->ob_subcreds) { - dst_service->ob_subcreds = src_service->ob_subcreds; - dst_service->n_ob_subcreds = src_service->n_ob_subcreds; + if (src->ob_subcreds) { + dst->ob_subcreds = src->ob_subcreds; + dst->n_ob_subcreds = src->n_ob_subcreds; - src_service->ob_subcreds = NULL; /* steal pointer reference */ + src->ob_subcreds = NULL; /* steal pointer reference */ } } @@ -4162,8 +4162,8 @@ hs_service_free_(hs_service_t *service) } /* Free onionbalance subcredentials (if any) */ - if (service->ob_subcreds) { - tor_free(service->ob_subcreds); + if (service->state.ob_subcreds) { + tor_free(service->state.ob_subcreds); } /* Wipe service keys. */ diff --git a/src/feature/hs/hs_service.h b/src/feature/hs/hs_service.h index 3fe14878ed..0f6a2c2358 100644 --- a/src/feature/hs/hs_service.h +++ b/src/feature/hs/hs_service.h @@ -279,6 +279,14 @@ typedef struct hs_service_state_t { /** When is the next time we should rotate our descriptors. This is has to be * done at the start time of the next SRV protocol run. */ time_t next_rotation_time; + + /* If this is an onionbalance instance, this is an array of subcredentials + * that should be used when decrypting an INTRO2 cell. If this is not an + * onionbalance instance, this is NULL. + * See [ONIONBALANCE] section in rend-spec-v3.txt for more details . */ + hs_subcredential_t *ob_subcreds; + /* Number of OB subcredentials */ + size_t n_ob_subcreds; } hs_service_state_t; /** Representation of a service running on this tor instance. */ @@ -304,14 +312,6 @@ typedef struct hs_service_t { hs_service_descriptor_t *desc_current; /** Next descriptor. */ hs_service_descriptor_t *desc_next; - - /* If this is an onionbalance instance, this is an array of subcredentials - * that should be used when decrypting an INTRO2 cell. If this is not an - * onionbalance instance, this is NULL. - * See [ONIONBALANCE] section in rend-spec-v3.txt for more details . */ - hs_subcredential_t *ob_subcreds; - /* Number of OB subcredentials */ - size_t n_ob_subcreds; } hs_service_t; /** For the service global hash map, we define a specific type for it which diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c index 9966bd108d..80383baff8 100644 --- a/src/test/test_hs_service.c +++ b/src/test/test_hs_service.c @@ -2520,7 +2520,7 @@ test_intro2_handling(void *arg) /* Start cleaning up X */ replaycache_free(x_service.state.replay_cache_rend_cookie); smartlist_free(x_service.config.ob_master_pubkeys); - tor_free(x_service.ob_subcreds); + tor_free(x_service.state.ob_subcreds); service_descriptor_free(x_service.desc_current); service_descriptor_free(x_service.desc_next); service_intro_point_free(x_ip); -- cgit v1.2.3-54-g00ecf