David Goulet [Thu, 29 Aug 2019 14:46:24 +0000 (10:46 -0400)]
hs-v3: Make service pick the exact amount of intro points
When encoding introduction points, we were not checking if that intro points
had an established circuit.
When botting up, the service will pick, by default, 3 + 2 intro points and the
first 3 that establish, we use them and upload the descriptor.
However, the intro point is removed from the service descriptor list only when
the circuit has opened and we see that we have already enough intro points, it
is then removed.
But it is possible that the service establishes 3 intro points successfully
before the other(s) have even opened yet.
This lead to the service encoding extra intro points in the descriptor even
though the circuit is not opened or might never establish (#31561).
Fixes #31548
Signed-off-by: David Goulet <dgoulet@torproject.org>
Nick Mathewson [Tue, 1 Oct 2019 16:55:42 +0000 (12:55 -0400)]
Fix a crash bug in max_u16_in_sl()
The documentation for this function says that the smartlist can
contain NULLs, but the code only handled NULLs if they were at the
start of the list.
We didn't notice this for a long time, because when Tor is run
normally, the sequence of msg_id_t is densely packed, and so this
list (mapping msg_id_t to channel_id_t) contains no NULL elements.
We could only run into this bug:
* when Tor was running in embedded mode, and starting more than once.
* when Tor ran first with more pubsub messages enabled, and then
later with fewer.
* When the second run (the one with fewer enabled pubsub messages)
had at least some messages enabled, and those messages were not
the ones with numerically highest msg_id_t values.
Nick Mathewson [Tue, 1 Oct 2019 13:42:10 +0000 (09:42 -0400)]
Add comments to try to prevent recurrence of #31495.
There is a bad design choice in two of our configuration types,
where the empty string encodes a value that is not the same as the
default value. This design choice, plus an implementation mistake,
meant that config_dup() did not preserve the value of routerset_t,
and thereby caused bug #31495.
This comment-only patch documents the two types with the problem,
and suggests that implementors try to avoid it in the future.
teor [Tue, 1 Oct 2019 07:32:46 +0000 (17:32 +1000)]
Travis: Keep the macOS Rust job, but don't wait for it to finish
Since Rust on macOS is slow, don't wait for the macOS Rust job to finish.
Instead, split rust into slow rust (macOS) and fast rust (Linux). And
allow the build to finish before slow rust finishes.
Also make sure that we have:
* a Rust build on each platform,
* a Rust build with each compiler, and
* a check on all our Rust builds.
Finally, sort builds: allow fail last, macOS first, slowest first.
Travis: Add a macOS chutney job, but don't wait for it to finish
Since Travis macOS has IPv6 support (and Travis Linux does not), chutney
will now run its IPv6 networks as part of Travis CI.
But since chutney is slow, don't wait for the macOS chutney to finish.
(Travis have fixed the duplicate notification bug in fast_finish. So we
can use fast_finish and allow_failure to finish early. Unfortunately,
allow_failure also means we ignore failures in macOS chutney.)
Also make sure that we have:
* a compile on each platform, with each compiler,
* a check on each platform, and
* a check on each compiler.
Finally, sort builds: allow fail last, macOS first, slowest first.
Closes ticket 30860.
Closes ticket 31859 for 0.2.9.
test: Avoid a map_anon_nofork test failure on SunOS
This test failure happened due to a signed/unsigned integer
comparison.
This bug occurred on SunOS, it may also occur on other systems that
use signed char as the default. (And cast 1-byte integer constants
to an unsigned integer.)
Nick Mathewson [Thu, 26 Sep 2019 01:13:30 +0000 (21:13 -0400)]
annotate_ifdef_directives: clarify situation with newlines
Our line limit is 80 characters, assuming that there is a single
terminating newline character that counts towards the limit. On
Windows, this might go as high as 81 characters, if we count CRLF as
two characters.
Nick Mathewson [Wed, 18 Sep 2019 14:59:35 +0000 (10:59 -0400)]
annotate_ifdef_directives: remove some cases of double negation
This change should reduce the number of cases where we say
"/* !(!defined(foo)) */" .
This only does cases where we can use a regex to make sure that the
simplification is guaranteed to be correct. Full boolean
simplification would require this script to parse C, and nobody
wants that.
log: explain why it is safe to leave the log mutex initialized
The log mutex is dynamically initialized, guarded by log_mutex_initialized.
We don't want to destroy it, because after it is destroyed, we won't see
any more logs.
If tor is re-initialized, log_mutex_initialized will still be 1. So we
won't trigger any undefined behaviour by trying to re-initialize the
log mutex.
Part of 31736, but committed in this branch to avoid merge conflicts.
backtrace: avoid undefined behaviour on re-initialisation
cb_buf_mutex is statically initialised, so we can not destroy it when
we are shutting down the err subsystem. If we destroy it, and then
re-initialise tor, all our backtraces will fail.
Part of 31736, but committed in this branch to avoid merge conflicts.