When sending the INTRODUCE1 cell, we acquire the needed data for the cell but
if the RP node_t has invalid data, we'll fail the send and completely kill the
SOCKS connection.
Instead, close the rendezvous circuit and return a transient error meaning
that Tor can recover by selecting a new rendezvous point. We'll also do the
same when we are unable to encode the INTRODUCE1 cell for which at that point,
we'll simply take another shot at a new rendezvous point.
Fixes#27774
Signed-off-by: David Goulet <dgoulet@torproject.org>
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.
This was missed in be583a34a3.
When we close a socket via tor_tls_free(), we previously had no way
for our socket accounting logic to learn about it. This meant that
the socket accounting code would think we had run out of sockets,
and freak out.
Fixes bug 27795; bugfix on 0.3.5.1-alpha.
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>
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>
Instead, have it call a mockable function. We don't want
crypto_strongest_rand() to be mockable, since doing so creates a
type error when we call it from ed25519-donna, which we do not build
in a test mode.
Fixes bug 27728; bugfix on 0.3.5.1-alpha
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.
Implements ticket 27738.
This argument was added to match an older idea for the C api, but we
decided not to do it that way in C.
Fixes bug 27741; bugfix on 0.3.3.6 / TROVE-2018-005 fix.
Since the default cache directory is the same as the default data
directory, we don't want the default CacheDirectoryGroupReadable
value (0) to override an explicitly set "DataDirectoryGroupReadable
1".
To fix this, I'm making CacheDirectoryGroupReadable into an
autobool, and having the default (auto) value mean "Use the value of
DataDirectoryGroupReadable if the directories are the same, and 0
otherwise."
Fixes bug 26913; bugfix on 0.3.3.1-alpha when the CacheDirectory
option was introduced.
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>
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.
.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.
Closes ticket 27206. Bugfix on e6625113c9.
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.
Fixes bug 27708; bugfix on 0.3.3.1-alpha.
All node_get_all_orports() does is allocate and return a smartlist
with at most two tor_addr_port_t members that match ORPort's of
node configuration. This is harmful for memory efficiency, as it
allocates the same stuff every time it is called. However,
node_is_a_configured_bridge() does not need to call it, as it
already has all the information to check if there is configured
bridge for a given node.
The new code is arranged in a way that hopefully makes each succeeding
linear search through bridge_list less likely.
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.
Existing cached directory information can cause misleadingly high
bootstrap percentages. To improve user experience, defer reporting of
directory information progress until at least one connection has
succeeded to a relay or bridge.
Closes ticket 27169.
There are three reasons we use a cached_dir_t to hold a consensus:
1. to serve that consensus to a client
2. to apply a consensus diff to an existing consensus
3. to send the consensus to a controller.
But case 1 is dircache-only. Case 2 and case 3 both fall back to
networkstatus_read_cached_consensus(). So there's no reason for us
to store this as a client. Avoiding this saves about 23% of our RAM
usage, according to our experiments last month.
This is, semantically, a partial revert of e5c608e535.
Fixes bug 27247; bugfix on 0.3.0.1-alpha.
As of August 29, 2018, Appveyor images come with gcc 8.2.0 by
default. 64-bit Windows executables compiled with gcc 8.2.0 and
tor's --enable-gcc-hardening crash.
Fixes bug 27460; bugfix on 0.3.4.1-alpha.
It is not supported, and always fails. Some compilers warn about the
function pointer cast on 64-bit Windows.
Fixes bug 27461; bugfix on 0.2.2.23-alpha.
gcc 8 warns that extend_info_t.nickname might be truncated by strncpy().
But it doesn't know that nickname can either contain a hex id, or a
nicknames. hex ids are only used for general and HSDir circuits.
Fixes bug 27463; bugfix on 0.1.1.2-alpha.
GetProcAddress() returns FARPROC, which is (long long int(*)()) on
64-bit Windows:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683212(v=vs.85).aspx
But GetAdaptersAddresses() is (long unsigned int(*)()), on both 32-bit
and 64-bit Windows:
https://docs.microsoft.com/en-us/windows/desktop/api/iphlpapi/nf-iphlpapi-getadaptersaddresses
So gcc 8 issues a spurious "incompatible function pointer" warning
about the cast to GetAdaptersAddresses_fn_t.
Silence this warning by casting to a void function pointer, before
the cast to GetAdaptersAddresses_fn_t.
This issue is already fixed by 26481 in 0.3.5 and later, by removing
the lookup and cast.
Fixes bug 27465; bugfix on 0.2.3.11-alpha.
We allow their CONNECTEDs, RESOLVEDs, ENDs, SENDMEs, and DATA cells to not
count as dropped until the windows are empty, or we get an END.
This commit does not change behavior. It only changes CIRC_BW event field
values.
No behaviour change.
A previous fix to chutney removed v3 onion services from the
mixed+hs-v23 network, so seeing "mixed+hs-v23" in tests is
confusing.
Fixes bug 27345; bugfix on 0.3.2.1-alpha.
LinkAuth method 1 is the one where we pull the TLS master secrets
out of the OpenSSL data structures and authenticate them with
RSA. Right now we list method 1 as required for clients and relays.
That's a problem, since we can't reasonably support it with NSS. So
let's remove it as a requirement and a recommendation.
As for method 3: I'd like to recommend it it, but that would make
0.2.9 start warning. Let's not do that till at least some time
after 0.3.5 (the next LTS) is stable.
Closes ticket 27286
