By doing so, it is renamed to voting_schedule_recalculate_timing(). This
required a lot of changes to include voting_schedule.h everywhere that this
function was used.
This effectively now makes voting_schedule.{c|h} not include dirauth/dirvote.h
for that symbol and thus no dependency on the dirauth module anymore.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This is a pretty big commit but it only moves these files to src/or/dirauth:
dircollate.c dirvote.c shared_random.c shared_random_state.c
dircollate.h dirvote.h shared_random.h shared_random_state.h
Then many files are modified to change the include line for those header files
that have moved into a new directory.
Without using --disable-module-dirauth, everything builds fine. When using the
flag to disable the module, tor doesn't build due to linking errors. This will
be addressed in the next commit(s).
No code behavior change.
Signed-off-by: David Goulet <dgoulet@torproject.org>
* ADD new /src/common/crypto_rand.[ch] module.
* ADD new /src/common/crypto_util.[ch] module (contains the memwipe()
function, since all crypto_* modules need this).
* FIXES part of #24658: https://bugs.torproject.org/24658
Additionally, this change extracts the functions that created and
freed these elements.
These structures had common "forward&reverse stream&digest"
elements, but they were initialized and freed through cpath objects,
and different parts of the code depended on them. Now all that code
is extacted, and kept in relay_crypto.c
This should avoid most intermittent test failures on developer and CI machines,
but there could (and probably should) be a more elegant solution.
Also, this test was testing that the IP was created and its expiration time was
set to a time greater than or equal to `now+INTRO_POINT_LIFETIME_MIN_SECONDS+5`:
/* Time to expire MUST also be in that range. We add 5 seconds because
* there could be a gap between setting now and the time taken in
* service_intro_point_new. On ARM, it can be surprisingly slow... */
tt_u64_op(ip->time_to_expire, OP_GE,
now + INTRO_POINT_LIFETIME_MIN_SECONDS + 5);
However, this appears to be a typo, since, according to the comment above it,
adding five seconds was done because the IP creation can be slow on some
systems. But the five seconds is added to the *minimum* time we're comparing
against, and so it actually functions to make this test *more* likely to fail on
slower systems. (It should either subtract five seconds, or instead add it to
time_to_expire.)
* FIXES#25450: https://bugs.torproject.org/25450
When we stopped looking at the "protocols" variable directly, we
broke the hs_service/build_update_descriptors test, since it didn't
actually update any of the flags.
The fix here is to call summarize_protover_flags() from that test,
and to expose summarize_protover_flags() as "STATIC" from
routerparse.c.
Using tt_assert in these helpers was implying to scan-build that our
'new' functions might be returning NULL, which in turn would make it
warn about null-pointer use.
Also demote a log message that can occur under natural causes
(if the circuit subsystem is missing descriptors/consensus etc.).
The HS subsystem will naturally retry to connect to intro points,
so no need to make that log user-facing.
Because of the latest changes on when we rotate, longer lifetime of
descriptors and no more overlap period, the tests needed to be improved to
test more functionnalities.
Signed-off-by: David Goulet <dgoulet@torproject.org>
First, this fixes#23372.
Second, the consensus timings for the build descriptor have been changed to
the current test can pass. More extensive tests of descriptor rotation are
coming in a commit near you because the rotation and time period logic has
been changed.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because of #23387, we've realized that there is one scenario that makes
the client unable to reach the service because of a desynch in the time
period used. The scenario is as follows:
+------------------------------------------------------------------+
| |
| 00:00 12:00 00:00 12:00 00:00 12:00 |
| SRV#1 TP#1 SRV#2 TP#2 SRV#3 TP#3 |
| |
| $==========|-----------$===========|-----------$===========| |
| ^ ^ |
| C S |
+------------------------------------------------------------------+
In this scenario the HS has a newer consensus than the client, and the
HS just moved to the next TP but the client is still stuck on the old
one. However, the service is not in any sort of overlap mode so it
doesn't cover the old TP anymore, so the client is unable to fetch a
descriptor.
We've decided to solve this by extending the concept of overlap period
to be permanent so that the service always publishes two descriptors and
aims to cover clients with both older and newer consensuses. See the
spec patch in #23387 for more details.
Use the valid_after time from the consensus to get the time period number else
we might get out of sync with the overlap period that uses valid_after.
Make it an optional feature since some functions require passing a
specific time (like hs_get_start_time_of_next_time_period()).
Signed-off-by: David Goulet <dgoulet@torproject.org>
We refactor the descriptor reupload logic to be similar to the v2 logic
where we update a global 'consider_republishing_rend_descriptors' flag
and then we use that to check for hash ring changes during the global
hidden service callbacks.
This fixes bugs where we would inspect the hash ring immediately as we
receive new dirinfo (e.g. consensus) but before running the hidden
service housekeeping events. That was leaving us in an inconsistent
state wrt hsdir indices and causing bugs all around.
We can't trigger a valid upload because it would require us to MOCK a long
list of functions ultimately not really testing the upload because we aren't
on a running network.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Conflicts:
src/test/test_hs_service.c
- Fix various ssize_t/size_t confusions in the tests.
- Fix a weird memset argument:
"bad_memset: Argument -16 in memset loses precision in
memset(&desc_two->blinded_kp.pubkey.pubkey, -16, 32UL)."
- Fix check_after_deref instance in check_state_line_for_service_rev_counter():
"check_after_deref: Null-checking items suggests that it may be null,
but it has already been dereferenced on all paths leading to the
check."
