Nick Mathewson [Thu, 25 Oct 2018 13:06:13 +0000 (09:06 -0400)]
Fix possible UB in an end-of-string check in get_next_token().
Remember, you can't check to see if there are N bytes left in a
buffer by doing (buf + N < end), since the buf + N computation might
take you off the end of the buffer and result in undefined behavior.
David Goulet [Thu, 20 Sep 2018 12:50:27 +0000 (08:50 -0400)]
node: Make node_supports_v3_rendezvous_point() also check for the key
It is not enough to look at protover for v3 rendezvous support but also we
need to make sure that the curve25519 onion key is present or in other words
that the descriptor has been fetched and does contain it.
Fixes #27797.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Occasionally, key pinning doesn't catch a relay that shares an ed25519
ID with another relay. Log the identity fingerprints and the shared ed25519 ID when this happens, instead of making a BUG() warning.
David Goulet [Wed, 17 Oct 2018 16:25:12 +0000 (12:25 -0400)]
conn: Stop writing when our write bandwidth limist is exhausted
Commit 488e2b00bf881b97bcc8e4bbe304845ff1d79a03 introduced an issue, most
likely introduced by a bad copy paste, that made us stop reading on the
connection if our write bandwidth limit was reached.
The problem is that because "read_blocked_on_bw" was never set, the connection
was never reenabled for reading.
This is most likely the cause of #27813 where bytes were accumulating in the
kernel TCP bufers because tor was not doing reads. Only relays with
RelayBandwidthRate would suffer from this but affecting all relays connecting
to them. And using that tor option is recommended and best practice so many
many relays have it enabled.
David Goulet [Wed, 26 Sep 2018 12:36:08 +0000 (08:36 -0400)]
mainloop: Set client role if ControlPort is set
It turns out that if _only_ the ControlPort is set and nothing else, tor would
simply not bootstrap and thus not start properly. Commit 67a41b63063370c2
removed that requirement for tor to be considered a "client".
Unfortunately, this made the mainloop enable basically nothing if only the
ControlPort is set in the torrc.
This commit now makes it that we also consider the ControlPort when deciding
if we are a Client or not. It does not revert 67a41b63063370c2 meaning
options_any_client_port_set() stays the same, not looking at the control port.
Fixes #27849.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Nick Mathewson [Wed, 10 Oct 2018 13:44:36 +0000 (09:44 -0400)]
Install only one of the two mingw architectures in appveyor
Closes ticket 27765
This commit also changes the appveyor file to a unified line ending
style, since the old one was pretty confused. Use "-b" to see the
real differences.
rust/protover: return C-allocated string in protover_all_supported()
The result of CString::into_raw() is not safe to free
with free() except under finicky and fragile circumstances
that we definitely don't meet right now.
David Goulet [Wed, 19 Sep 2018 14:56:15 +0000 (10:56 -0400)]
hs-v3: Close all SOCKS request on descriptor failure
Client side, when a descriptor is finally fetched and stored in the cache, we
then go over all pending SOCKS request for that descriptor. If it turns out
that the intro points are unusable, we close the first SOCKS request but not
the others for the same .onion.
This commit makes it that we'll close all SOCKS requests so we don't let
hanging the other ones.
It also fixes another bug which is having a SOCKS connection in RENDDESC_WAIT
state but with a descriptor in the cache. At some point, tor will expire the
intro failure cache which will make that descriptor usable again. When
retrying all SOCKS connection (retry_all_socks_conn_waiting_for_desc()), we
won't end up in the code path where we have already the descriptor for a
pending request causing a BUG().
Bottom line is that we should never have pending requests (waiting for a
descriptor) with that descriptor in the cache (even if unusable).
Fixees #27410.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Use the Travis Homebrew addon to install packages on macOS. The package
list is the same, but the Homebrew addon does not do a `brew update` by
default.
This makes builds faster, at the cost of using slightly older packages.
David Goulet [Fri, 14 Sep 2018 14:56:34 +0000 (10:56 -0400)]
hs-v3: Don't BUG() on directory permission check failure
In hs_config.c, we do validate the permission of the hidden service directory
but we do not try to create it. So, in the event that the directory doesn't
exists, we end up in the loading key code path which checks for the
permission and possibly creates the directory. On failure, don't BUG() since
there is a perfectly valid use case for that function to fail.
Fixes #27335
Signed-off-by: David Goulet <dgoulet@torproject.org>
Nick Mathewson [Fri, 14 Sep 2018 15:39:37 +0000 (11:39 -0400)]
Revise our assertion and bug macros to work with -Wparentheses
On GCC and Clang, there's a feature to warn you about bad
conditionals like "if (a = b)", which should be "if (a == b)".
However, they don't warn you if there are extra parentheses around
"a = b".
Unfortunately, the tor_assert() macro and all of its kin have been
passing their inputs through stuff like PREDICT_UNLIKELY(expr) or
PREDICT_UNLIKELY(!(expr)), both of which expand to stuff with more
parentheses around "expr", thus suppressing these warnings.
To fix this, this patch introduces new macros that do not wrap
expr. They're only used when GCC or Clang is enabled (both define
__GNUC__), since they require GCC's "({statement expression})"
syntax extension. They're only used when we're building the
unit-test variant of the object files, since they suppress the
branch-prediction hints.
I've confirmed that tor_assert(), tor_assert_nonfatal(),
tor_assert_nonfatal_once(), BUG(), and IF_BUG_ONCE() all now give
compiler warnings when their argument is an assignment expression.
Fixes bug 27709.
Bugfix on 0.0.6, where we first introduced the "tor_assert()" macro.
cypherpunks [Thu, 9 Aug 2018 21:26:10 +0000 (21:26 +0000)]
rust/protover: use .and_not_in() instead of .retain() in all_supported()
.retain() would allocating a Vec of billions of integers and check them
one at a time to separate the supported versions from the unsupported.
This leads to a memory DoS.
Nick Mathewson [Fri, 14 Sep 2018 14:34:45 +0000 (10:34 -0400)]
If options_act() fails, restore the previous value of global_options
Before 0.3.3.1-alpha, we would exit() in this case immediately. But
now that we leave tor_main() more conventionally, we need to make
sure we restore things so as not to cause a double free.
Nick Mathewson [Fri, 14 Sep 2018 12:36:33 +0000 (08:36 -0400)]
Avoid integer overflow on fast 32-bit millisecond conversion.
Multiply-then-divide is more accurate, but it runs into trouble when
our input is above INT32_MAX/numerator. So when our value is too
large, do divide-then-multiply instead.
Nick Mathewson [Fri, 14 Sep 2018 12:30:24 +0000 (08:30 -0400)]
Use a slightly more accurate formula for OSX 32-bit msec conversion
We use an optimized but less accurate formula for converting coarse
time differences to milliseconds on 32-bit OSX platforms, so that we
can avoid 64-bit division.
The old numbers were off by 0.4%. The new numbers are off by .006%.
This should make the unit tests a bit cleaner, and our tolerances a
bit closer.
Nick Mathewson [Wed, 12 Sep 2018 12:57:18 +0000 (08:57 -0400)]
Check waitpid return value and exit status in tinytest.c
It's possible for a unit test to report success via its pipe, but to
fail as it tries to clean up and exit. Notably, this happens on a
leak sanitizer failure.
Fixes bug 27658; bugfix on 0.2.2.4-alpha when tinytest was
introduced.
projects / thirdparty / tor.git / log
