Libevent uses an arc4random implementation (I know, I know) to
generate DNS transaction IDs and capitalization. But it liked to
initialize it either with opening /dev/urandom (which won't work
under the sandbox if it doesn't use the right pointer), or with
sysctl({CTL_KERN,KERN_RANDOM,RANDOM_UUIC}). To make _that_ work, we
were permitting sysctl unconditionally. That's not such a great
idea.
Instead, we try to initialize the libevent PRNG _before_ installing
the sandbox, and make sysctl always fail with EPERM under the
sandbox.
Appearently, the majority of the filenames we pass to
sandbox_cfg_allow() functions are "freeable right after". So, consider
_all_ of them safe-to-steal, and add a tor_strdup() in the few cases
that aren't.
(Maybe buggy; revise when I can test.)
(If we don't restrict rename, there's not much point in restricting
open, since an attacker could always use rename to make us open
whatever they want.)
A new set of unit test cases are provided, as well as introducing
an alternative paradigm and macros to support it. Primarily, each test
case is given its own namespace, in order to isolate tests from each
other. We do this by in the usual fashion, by appending module and
submodule names to our symbols. New macros assist by reducing friction
for this and other tasks, like overriding a function in the global
namespace with one in the current namespace, or declaring integer
variables to assist tracking how many times a mock has been called.
A set of tests for a small-scale module has been included in this
commit, in order to highlight how the paradigm can be used. This
suite gives 100% coverage to status.c in test execution.
My first implementation was broken, since it returned "whether there
is one bridge" rather than "how many bridges."
Also, the implementation for the n_options_out feature in
choose_random_entry_impl was completely broken due to a missing *.
When we successfully create a usable circuit after it previously
timed out for a certain amount of time, we should make sure that
our public IP address hasn't changed and update our descriptor.
In C, it's a bad idea to do this:
char *cp = array;
char *end = array + array_len;
/* .... */
if (cp + 3 >= end) { /* out of bounds */ }
because cp+3 might be more than one off the end of the array, and
you are only allowed to construct pointers to the array elements,
and to an element one past the end. Instead you have to say
if (cp - array + 3 >= array_len) { /* ... */ }
or something like that.
This patch fixes two of these: one in process_versions_cell
introduced in 0.2.0.10-alpha, and one in process_certs_cell
introduced in 0.2.3.6-alpha. These are both tracked under bug
10363. "bobnomnom" found and reported both. See also 10313.
In our code, this is likely to be a problem as we used it only if we
get a nasty allocator that makes allocations end close to (void*)-1.
But it's best not to have to worry about such things at all, so
let's just fix all of these we can find.
According to reports, most programs degrade somewhat gracefully on
getting no answer for an MX or a CERT for www.example.com, but many
flip out completely on a NOTIMPL error.
Also, treat a QTYPE_ALL query as just asking for an A record.
The real fix here is to implement proposal 219 or something like it.
Fixes bug 10268; bugfix on 0.2.0.1-alpha.
Based on a patch from "epoch".
Found by testing with chutney. The old behavior was "fail an
assertion", which obviously isn't optimal.
Bugfix on 8b9a2cb68b290e550695124d7ef0511225b451d5; bug not in any
released version.
Also, stop accepting the old kind of RESOLVED cells with no TTL
fields; they haven't been sent since 0.1.1.6-alpha.
This patch won't work without the fix to #10468 -- it will break
DNSPorts unless they set the proper ipv4/6 flags on entry_connection_t.
Otherwise, it could mung the thing that came over the net on windows,
which would defeat the purpose of recording the unparseable thing.
Fixes bug 11342; bugfix on 0.2.2.1-alpha.
This is a fix for 9963. I say this is a feature, but if it's a
bugfix, it's a bugfix on 0.2.4.18-rc.
Old behavior:
Mar 27 11:02:19.000 [notice] Bootstrapped 50%: Loading relay descriptors.
Mar 27 11:02:20.000 [notice] Bootstrapped 51%: Loading relay descriptors.
Mar 27 11:02:20.000 [notice] Bootstrapped 52%: Loading relay descriptors.
... [Many lines omitted] ...
Mar 27 11:02:29.000 [notice] Bootstrapped 78%: Loading relay descriptors.
Mar 27 11:02:33.000 [notice] We now have enough directory information to build circuits.
New behavior:
Mar 27 11:16:17.000 [notice] Bootstrapped 50%: Loading relay descriptors
Mar 27 11:16:19.000 [notice] Bootstrapped 55%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 60%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 65%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 70%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 75%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] We now have enough directory information to build circuits.
Most of these are simple. The only nontrivial part is that our
pattern for using ENUM_BF was confusing doxygen by making declarations
that didn't look like declarations.
In circuitlist_free_all, we free all the circuits, removing them from
the map as we go, but we weren't actually freeing the placeholder
entries that we use to indicate pending DESTROY cells.
Fix for bug 11278; bugfix on the 7912 code that was merged in
0.2.5.1-alpha
There are still quite a few 0.2.3.2x relays running for x<5, and while I
agree they should upgrade, I don't think cutting them out of the network
is a net win on either side.
This contains the obvious implementation using the circuitmux data
structure. It also runs the old (slow) algorithm and compares
the results of the two to make sure that they're the same.
Needs review and testing.
This change prevents LD_BUG warnings and bootstrap failure messages
when we try to do directory fetches when starting with
DisableNetwork == 1, a consensus present, but no descriptors (or
insufficient descriptors) yet.
Fixes bug 11200 and bug 10405. It's a bugfix on 0.2.3.9-alpha.
Thanks to mcs for walking me through the repro instructions!
This is meant to be a better bug 9229 fix -- or at least, one more
in tune with the intent of the original code, which calls
router_retry_directory_downloads() only on the first bridge descriptor.
This prevents long stalls when we're starting with a state file but
with no bridge descriptors. Fixes bug 9229. I believe this bug has
been present since 0.2.0.3-alpha.
By default, after you've made a connection to port XYZ, we assume
you might still want to have an exit ready to connect to XYZ for one
hour. This patch lets you lower that interval.
Implements ticket 91
This should fixes some "hey, that function could have
__attribute__((noreturn))" warnings introduced by f96400d9.
Bug not in any released version of Tor.
We have ignored any ports listed here since 80365b989 (0.0.7rc1),
but we didn't warn the user that we were ignoring them. This patch
adds a warning if you put explicit ports in any of the options
{Socks,Dir}Policy or AuthDir{Reject,Invalid,BadDir,BadExit}. It
also adjusts the manpage to say that ports are ignored.
Fixes ticket 11108.
In a couple of places, to implement the OOM-circuit-killer defense
against sniper attacks, we have counters to remember the age of
cells or data chunks. These timers were based on wall clock time,
which can move backwards, thus giving roll-over results for our age
calculation. This commit creates a low-budget monotonic time, based
on ratcheting gettimeofday(), so that even in the event of a time
rollback, we don't do anything _really_ stupid.
A future version of Tor should update this function to do something
even less stupid here, like employ clock_gettime() or its kin.
Back in 5e762e6a5c, non-exit servers
stopped launching DNS requests for users. So there's no need for them
to see if their DNS answers are hijacked.
Patch from Matt Pagan. I think this is a 965 fix.
For a client using a SocksPort connection and IPv6, the connect reply
from tor daemon did not handle AF_INET6 thus sending back the wrong
payload to the client.
A changes file is provided and this fixes#10987
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
When we have more than two return values, we should really be using
an enum rather than "-2 means this, -1 means that, 0 means this, and
1 or more means a number."
On busy servers, this function takes up something like 3-7% in
different profiles, and gets invoked every time we need to participate
as the midpoint in a hidden service.
So maybe walking through a linked list of all the circuits here wasn't
a good idea.
We log only one message, containing a complete list of what's
wrong. We log the complete list whenever any of the possible things
that could have gotten wrong gets worse.
Fix for #9870. Bugfix on 10480dff01, which we merged in
0.2.5.1-alpha.
This patch splits out some of the functions in OOM handling so that
it's easier to check them without involving the rest of Tor or
requiring that the circuits be "wired up".
It's increasingly apparent that we want to make sure we initialize our
PRNG nice and early, or else OpenSSL will do it for us. (OpenSSL
doesn't do _too_ bad a job, but it's nice to do it ourselves.)
We'll also need this for making sure we initialize the siphash key
before we do any hashes.
I've made an exception for cases where I'm sure that users can't
influence the inputs. This is likely to cause a slowdown somewhere,
but it's safer to siphash everything and *then* look for cases to
optimize.
This patch doesn't actually get us any _benefit_ from siphash yet,
since we don't really randomize the key at any point.
These options were added back in 0.1.2.5-alpha, but no longer make any
sense now that all directories support tunneled connections and
BEGIN_DIR cells. These options were on by default; now they are
always-on.
This is a fix for 10849, where TunnelDirConns 0 would break hidden
services -- and that bug arrived, I think, in 0.2.0.10-alpha.
(There is no longer meaningfully any such thing as a HS authority,
since we stopped uploading or downloading v0 hs descriptors in
0.2.2.1-alpha.)
Implements #10881, and part of #10841.
This patch removes an "if (chan)" that occurred at a place where
chan was definitely non-NULL. Having it there made some static
analysis tools conclude that we were up to shenanigans.
This resolves#9979.
Right now this accounts for about 1% of circuits over all, but if you
pick a guard that's running 0.2.3, it will be about 6% of the circuits
running through that guard.
Making sure that every circuit has at least one ntor link means that
we're getting plausibly good forward secrecy on every circuit.
This implements ticket 9777,
It's possible to set your ExitNodes to contains only exits that don't
have the Exit flag. If you do that, we'll decide that 0 of your exits
are working. Instead, in that case we should look at nodes which have
(or which might have) exit policies that don't reject everything.
Fix for bug 10543; bugfix on 0.2.4.10-alpha.
According to control spec, longname should not contain any spaces and is
consists only of identy_digest + nickname
added two functions:
* node_get_verbose_nickname_by_id()
* node_describe_longname_by_id()
