The major changes are to re-order some ciphers, to drop the ECDH suites
(note: *not* ECDHE: ECDHE is still there), to kill off some made-up
stuff (like the SSL_RSA_FIPS_WITH_3DES_EDE_CBC_SHA suite), to drop
some of the DSS suites... *and* to enable the ECDHE+GCM ciphersuites.
This change is autogenerated by get_mozilla_ciphers.py from
Firefox 28 and OpenSSL 1.0.1g.
Resolves ticket 11438.
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.
ubsan doesn't like us to do (1u<<32) when 32 is wider than
unsigned. Fortunately, we already special-case
addr_mask_get_bits(0), so we can just change the loop bounds.
In digestmap_set/get benchmarks, doing unaligned access on x86
doesn't save more than a percent or so in the fast case. In the
slow case (where we cross a cache line), it could be pretty
expensive. It also makes ubsan unhappy.
This make clang's memory sanitizer happier that we aren't reading
off the end of a char[1]. We hadn't replaced the char[1] with a
char[FLEXIBLE_ARRAY_MEMBER] before because we were doing a union
trick to force alignment. Now we use __attribute__(aligned) where
available, and we do the union trick elsewhere.
Most of this patch is just replacing accesses to (x)->u.mem with
(x)->U_MEM, where U_MEM is defined as "u.mem" or "mem" depending on
our implementation.
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.
There was one "missing prototype" warning because the test function
wasn't static, and one "unused parameter" warning about the "data"
parameter.
Also, I added a couple of tests to make sure that the "make_null"
addresses really were the addresses we expected, by formatting them
as strings.
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.
See 1d2179bc90 in master for details.
"""
Fall back to registered country if necessary.
When extracting geoip and geoip6 files from MaxMind's GeoLite2 Country
database, we only look at country->iso_code which is the two-character ISO
3166-1 country code of the country where MaxMind believes the end user is
located.
But if MaxMind thinks a range belongs to anonymous proxies, they don't put
anything there. Hence, we omit those ranges and resolve them all to '??'.
That's not what we want.
What we should do is first try country->iso_code, and if there's no such
key, try registered_country->iso_code which is the country in which the
ISP has registered the IP address.
In short: let's fill all A1 entries with what ARIN et. al think.
"""
When extracting geoip and geoip6 files from MaxMind's GeoLite2 Country
database, we only look at country->iso_code which is the two-character ISO
3166-1 country code of the country where MaxMind believes the end user is
located.
But if MaxMind thinks a range belongs to anonymous proxies, they don't put
anything there. Hence, we omit those ranges and resolve them all to '??'.
That's not what we want.
What we should do is first try country->iso_code, and if there's no such
key, try registered_country->iso_code which is the country in which the
ISP has registered the IP address.
In short: let's fill all A1 entries with what ARIN et. al think.
If the cert turns out to be invalid or if wget is otherwise unable to
verify it, it's going to return an error and not download the file for us.
Spotted by nickm.
It's possible for two threads to hit assertion failures at the same
time. If that happens, let's keep them from stomping on the same
cb_buf field.
Fixes bug 11048; bugfix on 0.2.5.2-alpha. Reported by "cypherpunks".
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>
Since the first stat call is made for it to deliberately fail, and we
reference st.st_mode without st having valid data, st.st_mode can contain
garbage and cause chmod to fail with EINVAL. We rerun stat and ensure it
succeeded.
Also make use of tt_abort_perror, to properly convey failure reasons to
the user.
clang 3.4 introduced a new by-default warning about unused static
functions, which we triggered heavily for the hashtable and map function
generating macros. We can use __attribute__ ((unused)) (thanks nickm for
the suggestion :-) ) to silence these warnings.
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.
The latest GeoLite2 database includes a pointer from 2001::/32 to the root
node of the IPv4 address space in the tree. We need to exclude this whole
address space from geoip6, similar to how we exclude IPv4-mapped IPv6
addresses and the 6to4 mapping subnet.
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.
If you had a resolv.conf file with a nameserver line containing no
nameserver IP, we would crash. That's not terrible, but it's not
desirable.
Fixes bug 8788; bugfix on 0.1.1.23. Libevent already has this fix.
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.
siphash is a hash function designed for producing hard-to-predict
64-bit outputs from short inputs and a 128-bit key. It's chosen for
security and speed.
See https://131002.net/siphash/ for more information on siphash.
Source: https://github.com/majek/csiphash/
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.
Apparently fedora currently has ECDH but not P224. This isn't a huge
deal, since we no longer use OpenSSL's P224 ever (see #9780 and
72c1e5acfe). But we shouldn't have segfaulting benchmarks really.
Fixes bug 10835; bugfix on 0.2.4.8-alpha.
This time, we use a pthread_attr to make sure that if pthread_create
succeeds, the thread is successfully detached.
This probably isn't the big thing going on with 4345, since it'd be
a bit weird for pthread_detach to be failing. But it's worth
getting it right.
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()
My OSX laptop rightly gave a warning because of sticking strlen() into
an int, but once I took a closer look... it appears that the strlen()
was part of a needlessly verbose implementation for tor_strdup().
While I was there, I fixed the usage of tor_free() in test_hs.c: It
checks for NULL, and it zeros its argument. So instead of
if (foo) {
tor_free(foo);
foo = NULL;
}
we should just say
tor_free(foo);
If you want a slow shutdown, send SIGNAL SHUTDOWN.
(Why not just have the default be SIGNAL QUIT? Because this case
should only happen when an owning controller has crashed, and a
crashed controller won't be able to give the user any "tor is
shutting down" feedback, and so the user gets confused for a while.
See bug 10449 for more info)
Improvement on f308adf838, where we made the ntor
unit tests run everywhere... so long as a python curve25519 module
was installed. Now the unit tests don't require that module.
I'm doing this because:
* User doesn't mean you're running as root, and running as root
doesn't mean you've set User.
* It's possible that the user has done some other
capability-based hack to retain the necessary privileges.
The remaining vestige is that we continue to publish the V2dir flag,
and that, for the controller, we continue to emit v2 directory
formats when requested.
Previously, we would sometimes decide in directory_get_from_hs_dir()
to connect to an excluded node, and then later in
directory_initiate_command_routerstatus_rend() notice that it was
excluded and strictnodes was set, and catch it as a stopgap.
Additionally, this patch preferentially tries to fetch from
non-excluded nodes even when StrictNodes is off.
Fix for bug #10722. Bugfix on 0.2.0.10-alpha (the v2 hidserv directory
system was introduced in e136f00ca). Reported by "mr-4".
If we don't, we can wind up with a wedged cpuworker, and write to it
for ages and ages.
Found by skruffy. This was a bug in 2dda97e8fd, a.k.a. svn
revision 402. It's been there since we have been using cpuworkers.
When I introduced the unusable_for_new_circuits flag in
62fb209d83, I had a spurious ! in the
circuit_stream_is_being_handled loop. This made us decide that
non-unusable circuits (that is, usable ones) were the ones to avoid,
and caused it to launch a bunch of extra circuits.
Fixes bug 10456; bugfix on 0.2.4.12-alpha.
When we wrote the directory request statistics code in August 2009, we
thought that these statistics were only relevant for bridges, and that
bridges should not report them. That's why we added a switch to discard
relevant observations made by bridges. This code was first released in
0.2.2.1-alpha.
In May 2012 we learned that we didn't fully disable directory request
statistics on bridges. Bridges did report directory request statistics,
but these statistics contained empty dirreq-v3-ips and dirreq-v3-reqs
lines. But the remaining dirreq-* lines have always been non-empty. (We
didn't notice for almost three years, because directory-request statistics
were disabled by default until 0.2.3.1-alpha, and all statistics have been
removed from bridge descriptors before publishing them on the metrics
website.)
Proposal 201, created in May 2012, suggests to add a new line called
bridge-v3-reqs that is similar to dirreq-v3-reqs, but that is published
only by bridges. This proposal is still open as of December 2013.
Since October 2012 we're using dirreq-v3-resp (not -reqs) lines in
combination with bridge-ips lines to estimate bridge user numbers; see
task 8462. This estimation method has superseded the older approach that
was only based on bridge-ips lines in November 2013. Using dirreq-v3-resp
and bridge-ips lines is a workaround. The cleaner approach would be to
use dirreq-v3-reqs instead.
This commit makes bridges report the same directory request statistics as
relays, including dirreq-v3-ips and dirreq-v3-reqs lines. It makes
proposal 201 obsolete.
In 0.2.3.8-alpha we attempted to "completely disable stats if we aren't
running as a relay", but instead disabled them only if we aren't running
as a server.
This commit leaves DirReqStatistics enabled on both relays and bridges,
and disables (Cell,Entry,ExitPort)Statistics on bridges.
This fixes bug 10402, where the rdrand engine would use the rdrand
instruction, not as an additional entropy source, but as a replacement
for the entire userspace PRNG. That's obviously stupid: even if you
don't think that RDRAND is a likely security risk, the right response
to an alleged new alleged entropy source is never to throw away all
previously used entropy sources.
Thanks to coderman and rl1987 for diagnosing and tracking this down.
The 'body' field of a microdesc_t holds a strdup()'d value if the
microdesc's saved_location field is SAVED_IN_JOURNAL or
SAVED_NOWHERE, and holds a pointer to the middle of an mmap if the
microdesc is SAVED_IN_CACHE. But we weren't setting that field
until a while after we parsed the microdescriptor, which left an
interval where microdesc_free() would try to free() the middle of
the mmap().
This patch also includes a regression test.
This is a fix for #10409; bugfix on 0.2.2.6-alpha.
The old behavior was that NULL matched only bridges without known
identities; the correct behavior is that NULL should match all
bridges (assuming that their addr:port matches).
We were checking whether a 8-bit length field had overflowed a
503-byte buffer. Unless somebody has found a way to store "504" in a
single byte, it seems unlikely.
Fix for 10313 and 9980. Based on a pach by Jared L Wong. First found
by David Fifield with STACK.
This flag prevents the creation of a console window popup on Windows. We
need it for pluggable transport executables--otherwise you get blank
console windows when you launch the 3.x browser bundle with transports
enabled.
http://msdn.microsoft.com/en-us/library/ms684863.aspx#CREATE_NO_WINDOW
The browser bundles that used Vidalia used to set this flag when
launching tor itself; it was apparently inherited by the pluggable
transports launched by tor. In the 3.x bundles, tor is launched by some
JavaScript code, which doesn't have the ability to set CREATE_NO_WINDOW.
tor itself is now being compiled with the -mwindows option, so that it
is a GUI application, not a console application, and doesn't show a
console window in any case. This workaround doesn't work for pluggable
transports, because they need to be able to write control messages to
stdout.
https://trac.torproject.org/projects/tor/ticket/9444#comment:30
The previous commit from piet would have backed out some of proposal
198 and made servers built without the V2 handshake not use the
unrestricted cipher list from prop198.
Bug not in any released Tor.
It's conceivable (but probably impossible given our code) that lseek
could return -1 on an error; when that happens, we don't want off to
become -1.
Fixes CID 1035124.
This was a mistake in the merge commit 7a2b30fe16. It
would have made the CellStatistics code give completely bogus
results. Bug not in any released Tor.
We had accidentially grown two fake ones: one for backtrace.c, and one
for sandbox.c. Let's do this properly instead.
Now, when we configure logs, we keep track of fds that should get told
about bad stuff happening from signal handlers. There's another entry
point for these that avoids using non-signal-handler-safe functions.
On platforms with the backtrace/backtrace_symbols_fd interface, Tor
can now dump stack traces on assertion failure. By default, I log
them to DataDir/stack_dump and to stderr.
Conflicts:
src/or/or.h
src/or/relay.c
Conflicts were simple to resolve. More fixes were needed for
compilation, including: reinstating the tv_to_msec function, and renaming
*_conn_cells to *_chan_cells.
In proposal 157, we added a cross-certification element for
directory authority certificates. We implemented it in
0.2.1.9-alpha. All Tor directory authorities now generate it.
Here, as planned, make it required, so that we can finally close
proposal 157.
The biggest change in the code is in the unit test data, where some
old hardcoded certs that we made long ago have become no longer
valid and now need to be replaced.
If openssl was old, Tor would add a warning about its version in
between saying "no torrc found, using reasonable defaults" and
"configuration was valid".
Previously, when we ran low on memory, we'd close whichever circuits
had the most queued cells. Now, we close those that have the
*oldest* queued cells, on the theory that those are most responsible
for us running low on memory, and that those are the least likely to
actually drain on their own if we wait a little longer.
Based on analysis from a forthcoming paper by Jansen, Tschorsch,
Johnson, and Scheuermann. Fixes bug 9093.
Roger spotted this on tor-dev in his comments on proposal 221.
We etect DESTROY vs everything else, since arma likes network
timeout indicating failure but not overload indicating failure.
Don't cast uint64_t * to const uint64_t * explicitly. The cast is always
safe, so C does it for us. Doing the cast explitictly can hide bugs if
the input is secretly the wrong type.
Suggested by Nick.
I was using the assertIn() function on unit tests, which Python 2.7
introduced. But we'd like to be able to run our unit tests on Pythons
from older operating systems.
As a bridge authority, before we create our networkstatus document, we
should compute the thresholds needed for the various status flags
assigned to each bridge based on the status of all other bridges. We
then add these thresholds to the networkstatus document for easy access.
Fixes for #1117 and #9859.
Also fix a bug where if the guard we choose first doesn't answer, we
would try the second guard, but once we connected to the second guard
we would abandon it and retry the first one, slowing down bootstrapping.
The fix in both cases is to treat all our initially chosen guards as
acceptable to use.
Fixes bug 9946.
We had updated our "do we have enough microdescs to begin building
circuits?" logic most recently in 0.2.4.10-alpha (see bug 5956), but we
left the bootstrap status event logic at "how far through getting 1/4
of them are we?"
Fixes bug 9958; bugfix on 0.2.2.36, which is where they diverged (see
bug 5343).
This reverts the torrc.sample.in changes from commit
66a04a6ac3.
We're going to not make this change in 0.2.4, since changing
torrc.sample.in makes all the debian users do some pointless
busywork. see tor-dev discusion of 9 Oct 2013.
Explicitly include bridges, and note that we archive and publish all
descriptors.
(We are not yet publishing ContactInfo lines contained in bridge
descriptors, but maybe we'll want to do that soon, so let's err on the
side of caution here.)
Related to #9854.
According to the manpage, bridges use P256 for conformity and relays
use P224 for speed. But skruffy points out that we've gotten it
backwards in the code.
In this patch, we make the default P256 for everybody.
Fixes bug 9780; bugfix on 0.2.4.8-alpha.
The old code had logic to use a shorter path length if we didn't
have enough nodes. But we don't support 2-node networks anwyay.
Fix for #9926. I'm not calling this a bugfix on any particular
version, since a 2-node network would fail to work for you for a lot
of other reasons too, and it's not clear to me when that began, or if
2-node networks would ever have worked.
This is probably not an exploitable bug, since you would need to have
errno be a large negative value in the unix pluggable-transport launcher
case. Still, best avoided.
Fixes bug 9928; bugfix on 0.2.3.18-rc.
By calling circuit_n_chan_done() unconditionally on close, we were
closing pending connections that might not have been pending quite for
the connection we were closing. Fix for bug 9880.
Thanks to skruffy for finding this and explaining it patiently until
we understood.
To fix#6033, we disabled TLS 1.1 and 1.2. Eventually, OpenSSL fixed
the bug behind #6033.
I've considered alternate implementations that do more testing to see
if there's secretly an OpenSSL 1.0.1c or something that secretly has a
backport of the OpenSSL 1.0.1e fix, and decided against it on the
grounds of complexity.
this was causing directory authorities to send a time of 0 on all
connections they generated themselves, which means everybody reachability
test caused a time skew warning in the log for that relay.
(i didn't just revert, because the changes file has been modified by
other later commits.)
This isn't actually much of an issue, since only relays send
AUTHENTICATE cells, but while we're removing timestamps, we might as
well do this too.
Part of proposal 222. I didn't take the approach in the proposal of
using a time-based HMAC, since that was a bad-prng-mitigation hack
from SSL3, and in real life, if you don't have a good RNG, you're
hopeless as a Tor server.
For now, round down to the nearest 10 minutes. Later, eliminate entirely by
setting a consensus parameter.
(This rounding is safe because, in 0.2.2, where the timestamp mattered,
REND_REPLAY_TIME_INTERVAL was a nice generous 60 minutes.)
We were freeing these on exit, but when we added the dl_status_map
field to them in fddb814f, we forgot to arrange for it to be freed.
I've moved the cert_list_free() code into its own function, and added
an appropriate dsmap_free() call.
Fixes bug 9644; bugfix on 0.2.4.13-alpha.
The problem was that the server_identity_key_is_set() function could
return true under conditions where we don't really have an identity
key -- specifically, where we used to have one, but we stopped being a
server.
This is a fix for 6979; bugfix on 0.2.2.18-alpha where we added that
assertion to get_server_identity_key().
tor_malloc returns void *; in C, it is not necessary to cast a
void* to another pointer type before assigning it.
tor_malloc fails with an error rather than returning NULL; it's not
necessary to check its output. (In one case, doing so annoyed Coverity.)
Whenever we had an non-option commandline arguments *and*
option-bearing commandline arguments on the commandline, we would save
only the latter across invocations of options_init_from_torrc, but
take their existence as license not to re-parse the former. Yuck!
Incidentally, this fix lets us throw away the backup_arg[gv] logic.
Fix for bug 9746; bugfix on d98dfb3746,
not in any released Tor. Found by Damian. Thanks, Damian!
This just goes to show: never cast a function pointer. Found while
testing new command line parse logic.
Bugfix on 1293835440, which implemented
6752: Not in any released tor.
Fall back to SOMAXCONN if INT_MAX doesn't work.
We'd like to do this because the actual maximum is overrideable by the
kernel, and the value in the header file might not be right at all.
All implementations I can find out about claim that this is supported.
Fix for 9716; bugfix on every Tor.
SCMP_CMP(a,b,c) leaves the fourth field of the structure undefined,
giving a missing-initializer error. All of our uses are
three-argument, so I'm overriding the default.
Now we explicitly check for overflow.
This approach seemed smarter than a cascade of "change int to unsigned
int and hope nothing breaks right before the release".
Nick, feel free to fix in a better way, maybe in master.
This would make us do testing circuits "even when cbt is disabled by
consensus, or when we're a directory authority, or when we've failed
to write cbt history to our state file lately." (Roger's words.)
This is a fix for 9671 and an improvement in our fix for 5049.
The original misbehavior was in 0.2.2.14-alpha; the incomplete
fix was in 0.2.3.17-beta.
(In practice they don't exist, but so long as we're making changes for
standards compliance...)
Also add several more unit tests for good and bad URL types.
There were only two functions outside of circuitstats that actually
wanted to know what was inside this. Making the structure itself
hidden should help isolation and prevent us from spaghettifying the
thing more.
The spec requires them to do so, and not doing so creates a situation
where they can't send-test because relays won't extend to them because
of the other part of bug 9546.
Fixes bug 9546; bugfix on 0.2.3.6-alpha.
The spec requires them to do so, and not doing so creates a situation
where they can't send-test because relays won't extend to them because
of the other part of bug 9546.
Fixes bug 9546; bugfix on 0.2.3.6-alpha.
(Backport to Tor 0.2.3)
Relays previously, when initiating a connection, would only send a
NETINFO after sending an AUTHENTICATE. But bridges, when receiving a
connection, would never send AUTH_CHALLENGE. So relays wouldn't
AUTHENTICATE, and wouldn't NETINFO, and then bridges would be
surprised to be receiving CREATE cells on a non-open circuit.
Fixes bug 9546.
Relays previously, when initiating a connection, would only send a
NETINFO after sending an AUTHENTICATE. But bridges, when receiving a
connection, would never send AUTH_CHALLENGE. So relays wouldn't
AUTHENTICATE, and wouldn't NETINFO, and then bridges would be
surprised to be receiving CREATE cells on a non-open circuit.
Fixes bug 9546.
Use the generic function for both the ControlPort cookie and the
ExtORPort cookie.
Also, place the global cookie variables in the heap so that we can
pass them around more easily as pointers.
Also also, fix the unit tests that broke by this change.
Conflicts:
src/or/config.h
src/or/ext_orport.c
Incidentally, this business here where I make crypto_rand mockable:
this is exactly the kind of thing that would make me never want to
include test-support stuff in production builds.
No other changes were made here. Keeping everything in
src/test/test.c was a legacy of back when we had all our unit tests in
one big file.
Doing this now because I'm adding an ext_or_command test.
memwipe some stack-allocated stuff
Add DOCDOC comments for state machines
Use memdup_nulterm as appropriate
Check for NULs in useraddr
Add a macro so that <= AUTH_MAX has a meaning.
- Don't leak if a transport proxy sends us a TRANSPORT command more
than once.
- Don't use smartlist_string_isin() in geoip_get_transport_history().
(pointed out by Nick)
- Use the 'join' argument of smartlist_join_strings() instead of
trying to write the separator on our own.
(pointed out by Nick)
- Document 'ext_or_transport' a bit better.
(pointed out by Nick)
- Be a bit more consistent with the types of the values of 'transport_counts'.
(pointed out by Nick)
Fortunately, later checks mean that uninitialized data can't get sent
to the network by this bug. Unfortunately, reading uninitialized heap
*can* (in some cases, with some allocators) cause a crash if you get
unlucky and go off the end of a page.
Found by asn. Bugfix on 0.2.4.1-alpha.
Both 'managed_proxy_list' and 'unconfigured_proxies_n' are global
src/or/transports.c variables that are not initialized properly when
unit tests are run.
When we moved channel_matches_target_addr_for_extend() into a separate
function, its sense was inverted from what one might expect, and we
didn't have a ! in one place where we should have.
Found by skruffy.
When we moved channel_matches_target_addr_for_extend() into a separate
function, its sense was inverted from what one might expect, and we
didn't have a ! in one place where we should have.
Found by skruffy.
I added this so I could write a unit test for ServerTransportOptions,
but it incidentally exercises the succeed-on-defaults case of
options_validate too.
This way, we don't have to use snprintf, which is not guaranteed to
be signal-safe.
(Technically speaking, strlen() and strlcpy() are not guaranteed to
be signal-safe by the POSIX standard. But I claim that they are on
every platform that supports libseccomp2, which is what matters
here.)
Better tests for upper bounds, and for failing cases.
Also, change the function's interface to take a buffer length rather
than a maximum length, and then NUL-terminate: functions that don't
NUL-terminate are trouble waiting to happen.
The only thing that used format_helper_exit_status on win32 was the
unit tests. This caused an error when we tried to leave a static
format_helper_exit_status lying around in a production object file.
The easiest solution is to admit that this way of dealing with process
exit status is Unix-only.
Previously we would accept relative paths, but only if they contained a
slash somewhere (not at the end).
Otherwise we would silently not work. Closes: #9258. Bugfix on
0.2.3.16-alpha.
This is not the most beautiful possible implementation (it requires
decorating mockable functions with ugly macros), but it actually
works, and is portable across multiple compilers and architectures.
If you pass the --enable-coverage flag on the command line, we build
our testing binaries with appropriate options eo enable coverage
testing. We also build a "tor-cov" binary that has coverage enabled,
for integration tests.
On recent OSX versions, test coverage only works with clang, not gcc.
So we warn about that.
Also add a contrib/coverage script to actually run gcov with the
appropriate options to generate useful .gcov files. (Thanks to
automake, the .o files will not have the names that gcov expects to
find.)
Also, remove generated gcda and gcno files on clean.
We previously used FILENAME_PRIVATE identifiers mostly for
identifiers exposed only to the unit tests... but also for
identifiers exposed to the benchmarker, and sometimes for
identifiers exposed to a similar module, and occasionally for no
really good reason at all.
Now, we use FILENAME_PRIVATE identifiers for identifiers shared by
Tor and the unit tests. They should be defined static when we
aren't building the unit test, and globally visible otherwise. (The
STATIC macro will keep us honest here.)
For identifiers used only by the unit tests and never by Tor at all,
on the other hand, we wrap them in #ifdef TOR_UNIT_TESTS.
This is not the motivating use case for the split test/non-test
build system; it's just a test example to see how it works, and to
take a chance to clean up the code a little.
This is mainly a matter of automake trickery: we build each static
library in two versions now: one with the TOR_UNIT_TESTS macro
defined, and one without. When TOR_UNIT_TESTS is defined, we can
enable mocking and expose more functions. When it's not defined, we
can lock the binary down more.
The alternatives would be to have alternate build modes: a "testing
configuration" for building the libraries with test support, and a
"production configuration" for building them without. I don't favor
that approach, since I think it would mean more people runnning
binaries build for testing, or more people not running unit tests.
Fix a bug in the voting algorithm that could yield incorrect results
when a non-naming authority declared too many flags. Fixes bug 9200;
bugfix on 0.2.0.3-alpha.
Found by coverity scan.
This implements "algorithm 1" from my discussion of bug #9072: on OOM,
find the circuits with the longest queues, and kill them. It's also a
fix for #9063 -- without the side-effects of bug #9072.
The memory bounds aren't perfect here, and you need to be sure to
allow some slack for the rest of Tor's usage.
This isn't a perfect fix; the rest of the solutions I describe on
codeable.
In my #7912 fix, there wasn't any code to remove entries from the
(channel, circuit ID)->circuit map corresponding to queued but un-sent
DESTROYs.
Spotted by skruffy. Fixes bug 9082; bug not in any released Tor.
I added the code to pass a destroy cell to a queueing function rather
than writing it immediately, and the code to remember that we
shouldn't reuse the circuit id until the destroy is actually sent, and
the code to release the circuit id once the destroy has been sent...
and then I finished by hooking destroy_cell_queue into the rest of
Tor.
This is a reprise of the fix in bdff7e3299d78; 6905c1f6 reintroduced
that bug. Briefly: windows doesn't seem to like deleting a mapped
file. I tried adding the PROT_SHARED_DELETE flag to the createfile
all, but that didn't actually fix this issue. Fortunately, the unit
test I added in 4f4fc63fea should
prevent us from making this particular screw-up again.
This patch also tries to limit the crash potential of a failure to
write by a little bit, although it could do a better job of retaining
microdescriptor bodies.
Fix for bug 8822, bugfix on 0.2.4.12-alpha.
This reverts commit 884a0e269c.
I'm reverting this because it doesn't actually make the problem go
away. It appears that instead we need to do unmap-then-replace.
A comment by rransom on #8795 taken together with a comment by doorss
recorded on #2077 suggest that *every* attempt to replace the md cache
will fail on Vista/Win7 if we don't have the FILE_SHARE_DELETE flag
passed to CreateFile, and if we try to replace the file ourselves
before unmapping it. I'm adding the FILE_SHARE_DELETE, since that's
this simplest fix. Broken indexers (the favored #2077 hypothesis)
could still cause trouble here, but at least this patch should make us
stop stepping on our own feet.
Likely fix for #2077 and its numerous duplicates. Bugfix on
0.2.2.6-alpha, which first had a microdescriptor cache that would get
replaced before remapping it.
Is it possible that *every* attempt to replace the microdesc cache on
windows 7 is going to fail because of our lack of FILE_SHARE_DELETE
while opening the file? If so, this test will catch #2077 and let us
know when it's fixed.
There's an assertion failure that can occur if a connection has
optimistic data waiting, and then the connect() call returns 0 on the
first attempt (rather than -1 and EINPROGRESS). That latter behavior
from connect() appears to be an (Open?)BSDism when dealing with remote
addresses in some cases. (At least, I've only seen it reported with
the BSDs under libevent, even when the address was 127.0.0.1. And
we've only seen this problem in Tor with OpenBSD.)
Fixes bug 9017; bugfix on 0.2.3.1-alpha, which first introduced
optimistic data. (Although you could also argue that the commented-out
connection_start_writing in 155c9b80 back in 2002 is the real source
of the issue.)
A new option TestingV3AuthVotingStartOffset is added which offsets the
starting time of the voting interval. This is possible only when
TestingTorNetwork is set.
This patch makes run_scheduled_events() check for new consensus
downloads every second when TestingTorNetwork, instead of every
minute. This should be fine, see #8532 for reasoning.
This patch also brings MIN_VOTE_SECONDS and MIN_DIST_SECONDS down from
20 to 2 seconds, unconditionally. This makes sanity checking of
misconfiguration slightly less sane.
Addresses #8532.
You can't use != to compare arbitary members of or_options_t.
(Also, generate a better error message to say which Testing* option
was set.)
Fix for bug 8992. Bugfix on b0d4ca49. Bug not in any released Tor.
- Rename n_read and n_written in origin_circuit_t to make it clear that
these are only used for CIRC_BW events.
- Extract new code in control_update_global_event_mask to new
clear_circ_bw_fields function.
- Avoid control_event_refill_global function with 13 arguments and
increase code reuse factor by moving more code from control.c to
connection.c.
- Avoid an unsafe uint32_t -> int cast.
- Add TestingEnableTbEmptyEvent option.
- Prepare functions for testing.
- Rename a few functions and improve documentation.
- Move cell_command_to_string from control.c to command.c.
- Use accessor for global_circuitlist instead of extern.
- Add a struct for cell statistics by command instead of six arrays.
- Split up control_event_circuit_cell_stats by using two helper functions.
- Add TestingEnableCellStatsEvent option.
- Prepare functions for testing.
- Rename a few variables and document a few things better.
- Move new ID= parameter in ORCONN event to end. Avoids possible trouble
from controllers that parse parameters by position, even though they
shouldn't.
Create new methods check_or_create_data_subdir() and
write_to_data_subdir() in config.c and use them throughout
rephist.c and geoip.c.
This should solve ticket #4282.
This is a fix for bug 8844, where eugenis correctly notes that there's
a sentinel value at the end of the list-of-freelists that's never
actually checked. It's a bug since the first version of the chunked
buffer code back in 0.2.0.16-alpha.
This would probably be a crash bug if it ever happens, but nobody's
ever reported something like this, so I'm unsure whether it can occur.
It would require write_to_buf, write_to_buf_zlib, read_to_buf, or
read_to_buf_tls to get an input size of more than 32K. Still, it's a
good idea to fix this kind of thing!
It appears that moria1 crashed because of one instance of this (the
one in router_counts_toward_thresholds). The other instance I fixed
won't actually have broken anything, but I think it's more clear this
way.
Fixes bug 8833; bugfix on 0.2.4.12-alpha.
We need to subtract both the current built circuits *and* the attempted
circuits from the attempt count during scaling, since *both* have already been
counted there.
It sure is a good thing we can run each test in its own process, or
else the amount of setup I needed to do to make this thing work
would have broken all the other tests.
Test mocking would have made this easier to write too.
Now we can compute the hash and signature of a dirobj before
concatenating the smartlist, and we don't need to play silly games
with sigbuf and realloc any more.
I believe this was introduced in 6bc071f765, which makes
this a fix on 0.2.0.10-alpha. But my code archeology has not extended
to actually testing that theory.
We were mixing bandwidth file entries (which are in kilobytes) with
router_get_advertised_bw() entries, which were in bytes.
Also, use router_get_advertised_bandwidth_capped() for credible_bandwidth.
Since 7536c40 only DNS results for real SOCKS requests are added to the cache,
but not DNS results for DNSPort queries or control connection RESOLVE queries.
Only cache additions would trigger ADDRMAP events on successful resolve.
Change it so that DNS results received after a RESOLVE command also generate
ADDRMAP events.
There was a bug in Tor prior to 0.2.4.10-alpha that allowed counts to
become invalid. Clipping the counts at startup allows us to rule out
log messages due to corruption from these prior Tor versions.
This was causing dirauths to emit flag weight validation warns if there
was a sufficiently large amount of badexit bandwidth to make a difference in
flag weight results.
It seems that some versions of clang that would prefer the
-Wswitch-enum compiler flag to warn about switch statements with
missing enum values, even if those switch statements have a
default.
Fixes bug 8598; bugfix on 0.2.4.10-alpha.
Found while investigating 8093, but probably not the cause of it,
since this bug would result in us sending too few SENDMEs, not in us
receiving SENDMEs unexpectedly.
Bugfix on the fix for 7889, which has appeared in 0.2.4.10-alpha, but
not yet in any released 0.2.3.x version.
It can never be NULL, since it's an array in bridge_line_t.
Introduced in 266f8cddd8. Found by coverity; this is CID 992691. Bug
not in any released Tor.
It was previously --Test in the help output and --test-commandline in
the getopt call. The man page already had --test.
(Originally by David, who resolved the tie in favor of "--test"; I
chose --test-commandline" instead so that nothing that depended
on it could break. -Nick)
Change nesting from [(]) to [()]. Formerly it made it look to me at
first glance that "internal port" was optional.
[Trivial change; fixes#7767 --nickm]
If we get a write error on a SOCKS connection, we can't send a
SOCKS reply, now can we?
This bug has been here since 36baf7219, where we added the "hey, I'm
closing an AP connection but I haven't finished the socks
handshake!" message. It's bug 8427.
Also, don't call the exit node 'reject *' unless our decision to pick
that node was based on a non-summarized version of that node's exit
policy.
rransom and arma came up with the ideas for this fix.
Fix for 7582; the summary-related part is a bugfix on 0.2.3.2-alpha.
When we're hibernating, the main reqason we can't bootstrap will
always be that we're hibernating: reporting anything else at severity
WARN is pointless.
Fixes part of 7302.
This bug affects hosts where time_t is unsigned, which AFAICT does
not include anything we currently support. (It _does_ include
OpenVMS, about a month of BSD4.2's history[1], and a lot of the 1970s.)
There are probably more bugs when time_t is unsigned. This one was
[1] http://mail-index.netbsd.org/tech-userlevel/1998/06/04/0000.html
This time, I'm checking whether our calculated offset matches our
real offset, in each case, as we go along. I don't think this is
the bug, but it can't hurt to check.
This should have been 2 bytes all along, since version numbers can
be 16 bits long. This isn't a live bug, since the call to
is_or_protocol_version_known in channel_tls_process_versions_cell
will reject any version number not in the range 1..4. Still, let's
fix this before we accidentally start supporting version 256.
Reported pseudonymously. Fixes bug 8062; bugfix on 0.2.0.10-alpha --
specifically, on commit 6fcda529, where during development I
increased the width of a version to 16 bits without changing the
type of link_proto.
Our ++ should have been += 2. This means that we'd accept version
numbers even when they started at an odd position.
This bug should be harmless in practice for so long as every version
number we allow begins with a 0 byte, but if we ever have a version
number starting with 1, 2, 3, or 4, there will be trouble here.
Fix for bug 8059, reported pseudonymously. Bugfix on 0.2.0.10-alpha
-- specifically, commit 6fcda529, where during development I
increased the width of a version to 16 bits without changing the
loop step.
I have no idea whether b0rken clients will DoS the network if the v2
authorities all turn this on or not. It's experimental. See #6783 for
a description of how to test it more or less safely, and please be
careful!
Now that circid_t is 4 bytes long, the default integer promotions will
leave it alone when sizeof(int) == 4, which will leave us formatting an
unsigned as an int. That's technically undefined behavior.
Fixes bug 8447 on bfffc1f0fc. Bug not
in any released Tor.
In a number of places, we decrement timestamp_dirty by
MaxCircuitDirtiness in order to mark a stream as "unusable for any
new connections.
This pattern sucks for a few reasons:
* It is nonobvious.
* It is error-prone: decrementing 0 can be a bad choice indeed.
* It really wants to have a function.
It can also introduce bugs if the system time jumps backwards, or if
MaxCircuitDirtiness is increased.
So in this patch, I add an unusable_for_new_conns flag to
origin_circuit_t, make it get checked everywhere it should (I looked
for things that tested timestamp_dirty), and add a new function to
frob it.
For now, the new function does still frob timestamp_dirty (after
checking for underflow and whatnot), in case I missed any cases that
should be checking unusable_for_new_conns.
Fixes bug 6174. We first used this pattern in 516ef41ac1,
which I think was in 0.0.2pre26 (but it could have been 0.0.2pre27).
Without this patch, there's no way to know what went wrong when we
fail to parse a torrc line entirely (that is, we can't turn it into
a K,V pair.) This patch introduces a new function that yields an
error message on failure, so we can at least tell the user what to
look for in their nonfunctional torrc.
(Actually, it's the same function as before with a new name:
parse_config_line_from_str is now a wrapper macro that the unit
tests use.)
Fixes bug 7950; fix on 0.2.0.16-alpha (58de695f90) which first
introduced the possibility of a torrc value not parsing correctly.
This patch moves the measured_bw field and the has_measured_bw field
into vote_routerstatus_t, since only votes have 'Measured=XX' set on
their weight line.
I also added a new bw_is_unmeasured flag to routerstatus_t to
represent the Unmeasured=1 flag on a w line. Previously, I was using
has_measured_bw for this, which was quite incorrect: has_measured_bw
means that the measured_bw field is set, and it's probably a mistake
to have it serve double duty as meaning that 'baandwidth' represents a
measured value.
While making this change,I also found a harmless but stupid bug in
dirserv_read_measured_bandwidths: It assumes that it's getting a
smartlist of routerstatus_t, when really it's getting a smartlist of
vote_routerstatus_t. C's struct layout rules mean that we could never
actually get an error because of that, but it's still quite incorrect.
I fixed that, and in the process needed to add two more sorting and
searching helpers.
Finally, I made the Unmeasured=1 flag get parsed. We don't use it for
anything yet, but someday we might.
This isn't complete yet -- the new 2286 unit test doesn't build.
Instead of capping whenever a router has fewer than 3 measurements,
we cap whenever a router has fewer than 3 measurements *AND* there
are at least 3 authorities publishing measured bandwidths.
We also generate bandwidth lines with a new "Unmeasured=1" flag,
meaning that we didn't have enough observations for a node to use
measured bandwidth values in the authority's input, whether we capped
it or not.
There are two ways to use sysconf to ask about the number of
CPUs. When we're on a VM, we would sometimes get it wrong by asking
for the number of total CPUs (say, 64) when we should have been asking
for the number of CPUs online (say, 1 or 2).
Fix for bug 8002.
Stop marking every relay as having been down for one hour every
time we restart a directory authority. These artificial downtimes
were messing with our Stable and Guard flag calculations.
Fixes bug 8218 (introduced by the fix for 1035). Bugfix on 0.2.2.23-alpha.
Apparently something in the directory guard code made it possible
for the same node to get added as a guard over and over when there
were no actual running guard nodes.
Relays used to check every 10 to 60 seconds, as an accidental side effect
of calling directory_fetches_from_authorities() when considering doing
a directory fetch. The fix for bug 1992 removes that side effect. At the
same time, bridge relays never had the side effect, leading to confused
bridge operators who tried crazy tricks to get their bridges to notice
IP address changes (see ticket 1913).
The new behavior is to reinstate an every-60-seconds check for both
public relays and bridge relays, now that the side effect is gone.
For example, we were doing a resolve every time we think about doing a
directory fetch. Now we reuse the cached answer in some cases.
Fixes bugs 1992 (bugfix on 0.2.0.20-rc) and 2410 (bugfix on
0.1.2.2-alpha).
When we compute the estimated microseconds we need to handle our
pending onionskins, we could (in principle) overflow a uint32_t if
we ever had 4 million pending onionskins before we had any data
about how onionskins take. Nevertheless, let's compute it properly.
Fixes bug 8210; bugfix on 0.2.4.10. Found by coverity; this is CID
980651.
If geoip_format_bridge_stats() returned NULL when it should have
returned a string, we would have tried to deref NULL, and died. Not
a big deal in the unit tests, but still worth fixing.
Found by coverity; This is CID 743384.
Coverity is worried that we're checking entry_conn in some cases,
but not in the case where we set entry_conn->pending_optimistic_data.
This commit should calm it down (CID 718623).
The refactoring in commit 471ab34032 wasn't complete enough: we
were checking the auth_len variable, but never actually setting it,
so it would never seem that authentication had been provided.
This commit also removes a bunch of unused variables from
rend_service_introduce, whose unusedness we hadn't noticed because
we were wiping them at the end of the function.
Fix for bug 8207; bugfix on 0.2.4.1-alpha.
This shouldn't actually matter, since tor-resolve will return soon
after this function exits, but it's nice to be warning-free
Found by coverity, fixes CID 718633
It returns the method by which we decided our public IP address
(explicitly configured, resolved from explicit hostname, guessed from
interfaces, learned by gethostname).
Now we can provide more helpful log messages when a relay guesses its IP
address incorrectly (e.g. due to unexpected lines in /etc/hosts). Resolves
ticket 2267.
While we're at it, stop sending a stray "(null)" in some cases for the
server status "EXTERNAL_ADDRESS" controller event. Resolves bug 8200.
since router_parse_entry_from_string() already checks whether
!tor_inet_aton(router->address, &in)
(And no need to print address, since router_describe does that.)
- Document the key=value format.
- Constify equal_sign_pos.
- Pass some strings that are about to be logged to escape().
- Update documentation and fix some bugs in tor_escape_str_for_socks_arg().
- Use string_is_key_value() in parse_bridge_line().
- Parenthesize a forgotten #define
- Add some more comments.
- Add some more unit test cases.
This check isn't necessary (see comment on #7801), but it took at
least two smart people a little while to see why it wasn't necessary,
so let's have it in to make the code more readable.
We need a weak RNG in a couple of places where the strong RNG is
both needless and too slow. We had been using the weak RNG from our
platform's libc implementation, but that was problematic (because
many platforms have exceptionally horrible weak RNGs -- like, ones
that only return values between 0 and SHORT_MAX) and because we were
using it in a way that was wrong for LCG-based weak RNGs. (We were
counting on the low bits of the LCG output to be as random as the
high ones, which isn't true.)
This patch adds a separate type for a weak RNG, adds an LCG
implementation for it, and uses that exclusively where we had been
using the platform weak RNG.
Right now, all our curve25519 backends ignore the high bit of the
public key. But possibly, others could treat the high bit of the
public key as encoding out-of-bounds values, or as something to be
preserved. This could be used to distinguish clients with different
backends, at the cost of killing a circuit.
As a workaround, let's just clear the high bit of each public key
indiscriminately before we use it. Fix for bug 8121, reported by
rransom. Bugfix on 0.2.4.8-alpha.
If we're deciding on a node's bandwidth based on "Bandwidth="
declarations, clip it to "20" or to the maxunmeasuredbw parameter,
if it's voted on.
This adds a new consensus method.
This is "part A" of bug 2286
Authorities don't set is_possible_guard on node_t, so they were
never deciding that they could build enough paths. This is a quick
and dirty fix.
Bug not in any released version of Tor
These seem to have gotten conflicted out of existence while mike was
working on path bias stuff.
Thanks to sysrqb for collecting these in a handy patch.
The fix is to move the two functions to format/parse base64
curve25519 public keys into a new "crypto_format.c" file. I could
have put them in crypto.c, but that's a big file worth splitting
anyway.
Fixes bug 8153; bugfix on 0.2.4.8-alpha where I did the fix for 7869.
Now we can specify to skip bridges that wouldn't be able to answer the
type of dir fetch we're launching.
It's still the responsibility of the rest of the code to prevent us from
launching a given dir fetch if we have no bridges that could handle it.
Now as we move into a future where most bridges can handle microdescs
we will generally find ourselves using them, rather than holding back
just because one of our bridges doesn't use them.
When we first implemented TLS, we assumed in conneciton_handle_write
that a TOR_TLS_WANT_WRITE from flush_buf_tls meant that nothing had
been written. But when we moved our buffers to a ring buffer
implementation back in 0.1.0.5-rc (!), we broke that invariant: it's
possible that some bytes have been written but nothing.
That's bad. It means that if we do a sequence of TLS writes that ends
with a WANTWRITE, we don't notice that we flushed any bytes, and we
don't (I think) decrement buckets.
Fixes bug 7708; bugfix on 0.1.0.5-rc
Also, deprecate the torrc options for the scaling values. It's unlikely anyone
but developers will ever tweak them, even if we provided a single ratio value.
This is meant to avoid conflict with the built-in log() function in
math.h. It resolves ticket 7599. First reported by dhill.
This was generated with the following perl script:
#!/usr/bin/perl -w -i -p
s/\blog\(LOG_(ERR|WARN|NOTICE|INFO|DEBUG)\s*,\s*/log_\L$1\(/g;
s/\blog\(/tor_log\(/g;
Instead of hardcoding the minimum fraction of possible paths to 0.6, we
take it from the user, and failing that from the consensus, and
failing that we fall back to 0.6.
Previously we did this based on the fraction of descriptors we
had. But really, we should be going based on what fraction of paths
we're able to build based on weighted bandwidth, since otherwise a
directory guard or two could make us behave quite oddly.
Implementation for feature 5956
This is allowed by the C statndard, which permits you to represent
doubles any way you like, but in practice we have some code that
assumes that memset() clears doubles in structs. Noticed as part of
7802 review; see 8081 for more info.
When we implemented #5823 and removed v2 directory request info, we
never actually changed the unit tests not to expect it.
Fixes bug 8084; bug not in any released version of Tor.
It looks like there was a compilation error for 6826 on some
platforms. Removing even more now-uncallable code to handle detecting
libevent versions before 1.3e.
Fixes bug 8012; bug not in any released Tor.
If any circuits were opened during a scaling event, we were scaling attempts
and successes by different amounts. This leads to rounding error.
The fix is to record how many circuits are in a state that hasn't been fully
counted yet, and subtract that before scaling, and add it back afterwords.
Since they use RELAY_EARLY (which can be seen by all hops on the path),
it's not safe to say they actually count as a successful use.
There are also problems with trying to allow them to finish extending due to
the circuit purpose state machine logic. It is way less complicated (and
possibly more semantically coherent) to simply wait until we actually try to
do something with them before claiming we 'used' them.
Also, we shouldn't call timed out circuits 'used' either, for semantic
consistency.
An adversary could let the first stream request succeed (ie the resolve), but
then tag and timeout the remainder (via cell dropping), forcing them on new
circuits.
Rolling back the state will cause us to probe such circuits, which should lead
to probe failures in the event of such tagging due to either unrecognized
cells coming in while we wait for the probe, or the cipher state getting out
of sync in the case of dropped cells.
Path use bias measures how often we can actually succeed using the circuits we
actually try to use. It is a subset of path bias accounting, but it is
computed as a separate statistic because the rate of client circuit use may
vary depending on use case.
This is a minimal refactoring to expose the weighted bandwidth
calculations for each node so I can use them to see what fraction of
nodes, weighted by bandwidth, we have descriptors for.
This is ticket 7706, reported by "bugcatcher." The rationale here
is that if somebody says 'ExcludeNodes {tv}', then they probably
don't just want to block definitely Tuvaluan nodes: they also want
to block nodes that have unknown country, since for all they know
such nodes are also in Tuvalu.
This behavior is controlled by a new GeoIPExcludeUnknown autobool
option. With the default (auto) setting, we exclude ?? and A1 if
any country is excluded. If the option is 1, we add ?? and A1
unconditionally; if the option is 0, we never add them.
(Right now our geoip file doesn't actually seem to include A1: I'm
including it here in case it comes back.)
This feature only takes effect if you have a GeoIP file. Otherwise
you'd be excluding every node.
This won't actually break them any worse than they were broken before:
it just removes a set of warnings that nobody was actually seeing, I
hope.
Closes 6826
The implementation is pretty straightforward: parse_extended_hostname() is
modified to drop any leading components from an address like
'foo.aaaaaaaaaaaaaaaa.onion'.
This is an automatically generated commit, from the following perl script,
run with the options "-w -i -p".
s/smartlist_string_num_isin/smartlist_contains_int_as_string/g;
s/smartlist_string_isin((?:_case)?)/smartlist_contains_string$1/g;
s/smartlist_digest_isin/smartlist_contains_digest/g;
s/smartlist_isin/smartlist_contains/g;
s/digestset_isin/digestset_contains/g;
In 6fbdf635 we added a couple of statements like:
if (test) {
...
};
The extraneous semicolons there get flagged as worrisome empty
statements by the cparser library, so let's fix them.
Patch by Christian Grothoff; fixes bug 7115.
Otherwise, it's possible to create streams or circuits with these
bogus IDs, leading to orphaned circuits or streams, or to ones that
can cause bandwidth DOS problems.
Fixes bug 7889; bugfix on all released Tors.
In general, if we tried to use a circ for a stream, but then decided to place
that stream on a different circuit, we need to probe the original circuit
before deciding it was a "success".
We also need to do the same for cannibalized circuits that go unused.
This makes removing items from the middle of the queue into an O(1)
operation, which could prove important as we let onionqueues grow
longer.
Doing this actually makes the code slightly smaller, too.
The right way to set "MaxOnionsPending" was to adjust it until the
processing delay was appropriate. So instead, let's measure how long
it takes to process onionskins (sampling them once we have a big
number), and then limit the queue based on its expected time to
finish.
This change is extra-necessary for ntor, since there is no longer a
reasonable way to set MaxOnionsPending without knowing what mix of
onionskins you'll get.
This patch also reserves 1/3 of the onionskin spots for ntor
handshakes, on the theory that TAP handshakes shouldn't be allowed to
starve their speedier cousins. We can change this later if need be.
Resolves 7291.
The unit of work sent to a cpuworker is now a create_cell_t; its
response is now a created_cell_t. Several of the things that call or
get called by this chain of logic now take create_cell_t or
created_cell_t too.
Since all cpuworkers are forked or spawned by Tor, they don't need a
stable wire protocol, so we can just send structs. This saves us some
insanity, and helps p
As elsewhere, it makes sense when adding or extending a cell type to
actually make the code to parse it into a separate tested function.
This commit doesn't actually make anything use these new functions;
that's for a later commit.
The handshake_digest field was never meaningfully a digest *of* the
handshake, but rather is a digest *from* the handshake that we exapted
to prevent replays of ESTABLISH_INTRO cells. The ntor handshake will
generate it as more key material rather than taking it from any part
of the circuit handshake reply..
I'm going to want a generic "onionskin" type and set of wrappers, and
for that, it will be helpful to isolate the different circuit creation
handshakes. Now the original handshake is in onion_tap.[ch], the
CREATE_FAST handshake is in onion_fast.[ch], and onion.[ch] now
handles the onion queue.
This commit does nothing but move code and adjust header files.
Here we try to handle curve25519 onion keys from generating them,
loading and storing them, publishing them in our descriptors, putting
them in microdescriptors, and so on.
This commit is untested and probably buggy like whoa
This patch moves curve25519_keypair_t from src/or/onion_ntor.h to
src/common/crypto_curve25519.h, and adds new functions to generate,
load, and store keypairs.
Previously, we only used the strong OS entropy source as part of
seeding OpenSSL's RNG. But with curve25519, we'll have occasion to
want to generate some keys using extremely-good entopy, as well as the
means to do so. So let's!
This patch refactors the OS-entropy wrapper into its own
crypto_strongest_rand() function, and makes our new
curve25519_secret_key_generate function try it as appropriate.
The ntor handshake--described in proposal 216 and in a paper by
Goldberg, Stebila, and Ustaoglu--gets us much better performance than
our current approach.
We want to use donna-c64 when we have a GCC with support for
64x64->uint128_t multiplying. If not, we want to use libnacl if we
can, unless it's giving us the unsafe "ref" implementation. And if
that isn't going to work, we'd like to use the
portable-and-safe-but-slow 32-bit "donna" implementation.
We might need more library searching for the correct libnacl,
especially once the next libnacl release is out -- it's likely to have
bunches of better curve25519 implementations.
I also define a set of curve25519 wrapper functions, though it really
shouldn't be necessary.
We should eventually make the -donna*.c files get build with
-fomit-frame-pointer, since that can make a difference.
There was one place in curve25519-donna-c64 that was relying on
unaligned access and relying on little-endian values. This patch
fixes that.
I've sent Adam a pull request.
Our old warn_nonlocal_client_ports() would give a bogus warning for
every nonlocal port every time it parsed any ports at all. So if it
parsed a nonlocal socksport, it would complain that it had a nonlocal
socksport...and then turn around and complain about the nonlocal
socksport again, calling it a nonlocal transport or nonlocal dnsport,
if it had any of those.
Fixes bug 7836; bugfix on 0.2.3.3-alpha.
mr-4 reports on #7799 that he was seeing it several times per second,
which suggests that things had gone very wrong.
This isn't a real fix, but it should make Tor usable till we can
figure out the real issue.
This implements the server-side of proposal 198 by detecting when
clients lack the magic list of ciphersuites that indicates that
they're lying faking some ciphers they don't really have. When
clients lack this list, we can choose any cipher that we'd actually
like. The newly allowed ciphersuites are, currently, "All ECDHE-RSA
ciphers that openssl supports, except for ECDHE-RSA-RC4".
The code to detect the cipher list relies on on (ab)use of
SSL_set_session_secret_cb.
We already use this classification for deciding whether (as a server)
to do a v2/v3 handshake, and we're about to start using it for
deciding whether we can use good ciphersuites too.
This is less easy than you might think; we can't just look at the
client ciphers list, since openssl doesn't remember client ciphers if
it doesn't know about them. So we have to keep a list of the "v2"
ciphers, with the ones we don't know about removed.
It's important not to call choose_array_element_by_weight and then
pass its return value unchecked to smartlist_get : it is allowed to
return -1.
Fixes bug 7756; bugfix on 4e3d07a6 (not in any released Tor)
This is good enough to give P_success >= 999,999,999/1,000,000,000 so
long as the address space is less than 97.95 full. It'd be ridiculous
for that to happen for IPv6, and usome reasonable assumptions, it
would also be pretty silly for IPv4.
With an IPv6 virtual address map, we can basically hand out a new
IPv6 address for _every_ address we connect to. That'll be cool, and
will let us maybe get around prop205 issues.
This uses some fancy logic to try to make the code paths in the ipv4
and the ipv6 case as close as possible, and moves to randomly
generated addresses so we don't need to maintain those stupid counters
that will collide if Tor restarts but apps don't.
Also has some XXXX items to fix to make this useful. More design
needed.
This function gives us a single place to set reasonable default flags
for port_cfg_t entries, to avoid bugs like the one where we weren't
setting ipv4_traffic_ok to 1 on SocksPorts initialized in an older
way.
(This is part 2 of making DNS cache use enabled/disabled on a
per-client port basis. This implements the CacheIPv[46]DNS options,
but not the UseCachedIPv[46] ones.)
(This is part 1 of making DNS cache use enabled/disabled on a
per-client port basis. These options are shuffled around correctly,
but don't do anything yet.)
We want to be saying fast_mem{cmp,eq,neq} when we're doing a
comparison that's allowed to exit early, or tor_mem{cmp,eq,neq} when
we need a data-invariant timing. Direct use of memcmp tends to imply
that we haven't thought about the issue.
This has several advantages, including more resilience to ambient failure.
I still need to rename all the first_hop vars tho.. Saving that for a separate
commit.
Turns out there's more than one way to block a tagged circuit.
This seems to successfully handle all of the normal exit circuits. Hidden
services need additional tweaks, still.
This replaces the old FallbackConsensus notion, and should provide a
way -- assuming we pick reasonable nodes! -- to give clients
suggestions of placs to go to get their first consensus.
Now creating a dir_server_t and adding it are separate functions, and
there are frontend functions for adding a trusted dirserver and a
fallback dirserver.
We use trusted_dir_server_t for two pieces of functionality: a list of
all directory authorities, and a list of initial places to look for
a directory. With this patch we start to separate those two roles.
There is as of now no actual way to be a fallback directory without being
an authority.
This is a customizable extract-and-expand HMAC-KDF for deriving keys.
It derives from RFC5869, which derives its rationale from Krawczyk,
H., "Cryptographic Extraction and Key Derivation: The HKDF Scheme",
Proceedings of CRYPTO 2010, 2010, <http://eprint.iacr.org/2010/264>.
I'm also renaming the existing KDF, now that Tor has two of them.
This is the key derivation scheme specified in ntor.
There are also unit tests.
This one is necessary for sending BEGIN cells with sane flags when
self-testing a directory port. All real entry connections were
getting their ipv{4,6}_traffic_ok flags set from their listeners, and
for begindir entry connections we didn't care, but for directory
self-testing, we had a problem.
Fixes at least one more case of 7493; if there are more lingering
cases of 7493, this might fix them too.
Bug not in any released version of Tor.
Looks like when i was writing the code to set the ipv4_traffic flag on
port_cfg_t, I missed some cases, such as the one where the port was
set from its default value.
Fix for 7493. Bug not in any released Tor.
Previously, I was freaking out about passing an unspec address to
dns_found_answer() on an error, since I was using the address type to
determine whether the error was an error on an ipv4 address lookup or
on an ipv6 address lookup. But now dns_found_answer() has a separate
orig_query_type argument to tell what kind of query it is, so there's
no need to freak out.
* If there's an IPv4 and an IPv6 address, return both in the resolved
cell.
* Treat all resolve requests as permitting IPv6, since by the spec they're
allowed to, and by the code that won't break anything.
IPv4-only exits have an implicit "reject [::]/0", which was making
policy_is_reject_star() return 1 for them, making us refuse to do
hostname lookups.
This fix chanes policy_is_reject_star() to ask about which family we meant.
The code previously detected wildcarding and replaced wildcarded
answers with DNS_STATUS_FAILED_PERMANENT. But that status variable
was no longer used! Remove the status variable, and instead change
the value of 'result' in evdns_callback.
Thank goodness for compiler warnings. In this case,
unused-but-set-variable.
Thanks to Linus for finding this one.
Now, every cached_resolve_t can remember an IPv4 result *and* an IPv6
result. As a light protection against timing-based distinguishers for
IPv6 users (and against complexity!), every forward request generates
an IPv4 *and* an IPv6 request, assuming that we're an IPv6 exit. Once
we have answers or errors for both, we act accordingly.
This patch additionally makes some useful refactorings in the dns.c
code, though there is quite a bit more of useful refactoring that could
be done.
Additionally, have a new interface for the argument passed to the
evdns_callback function. Previously, it was just the original address
we were resolving. But it turns out that, on error, evdns doesn't
tell you the type of the query, so on a failure we didn't know whether
IPv4 or IPv6 queries were failing.
The new convention is to have the first byte of that argument include
the query type. I've refactored the code a bit to make that simpler.
This makes it so we can handle getting an IPv6 in the 3 different
formats we specified it for in RESOLVED cells,
END_STREAM_REASON_EXITPOLICY cells, and CONNECTED cells.
We don't cache IPv6 addresses yet, since proposal 205 isn't
implemented.
There's a refactored function for parsing connected cells; it has unit
tests.
These options are for telling the SOCKSPort that it should allow or
not allow connections to IPv4/IPv6 addresses.
These aren't implemented yet; this is just the code to read the
options and get them into the entrey_connection_t.
Also, count ipv6 timeouts vs others. If we have too many ipv6
requests time out, then we could be degrading performance because of a
broken DNS server that ignores AAAA requests. Other cases in which
we never learn an AAAA address aren't so bad, since they don't slow
A (ipv4) answers down very much.
This is a relatively simple set of changes: we mostly need to
remove a few "but not for IPv6" changes. We also needed to tweak
the handling of DNS code to generate RESOLVED cells that could get
an IPv6 answer in return.
Now, "accept *:80" means "accept all addresses on port 80", and not
just IPv4. For just v4, say "accept *4:80"; for just v6 say "accept
*6:80".
We can parse these policies from torrc just fine, and we should be
successfully keeping them out of descriptors for now.
We also now include appropriate IPv6 addresses in "reject private:*"
By default, "*" means "All IPv4 addresses" with
tor_addr_parse_mask_ports, so I won't break anything. But if the new
EXTENDED_STAR flag is provided, then * means "any address", *4 means
"any IPv4 address" (that is, 0.0.0.0/0), and "*6" means "any IPv6
address" (that is, [::]/0).
This is going to let us have a syntax for specifying exit policies in
torrc that won't drive people mad.
Also, add a bunch of unit tests for tor_addr_parse_mask_ports to test
these new features, and to increase coverage.
We'd like these functions to be circuit-relative so that we can
implement a per-circuit DNS cache and per-circuit DNS cache rules for
proposal 205 or its successors. I'm doing this now, as a part of the
IPv6 exits code, since there are about to be a few more instances
of code using this.
This is the simplest possible workaround: make it safe to call
circuit_cell_queue_clear() on a non-attached circuit, and make it
safe-but-a-LD_BUG-warning to call update_circuit_on_cmux() on a
non-attached circuit.
LocalWords: unstage src Untracked
Apparently some compilers like to eliminate memset() operations on
data that's about to go out-of-scope. I've gone with the safest
possible replacement, which might be a bit slow. I don't think this
is critical path in any way that will affect performance, but if it
is, we can work on that in 0.2.4.
Fixes bug 7352.
There is probably no code that can write the 2 bytes at the end of the
packed_cell_t when the cell is only a 512-byte cell, but let's not get
overconfident there.
Instead of warning about low ports that are advertised, we should have
been warning about low ports that we're listening on. Bug 7285, fix
on 0.2.3.9-alpha.
That's not where I'd want to put a $, but apparently the other
foo/id/<identity> things allow it, as does an arguably valid
interpretation of control-spec.txt. So let's be consistent.
Fix for a piece of bug 7059.
This is part of what's needed to build without warnings on mingw64:
it was warning about the cast from void* to long that happened in
the places we were using test_{n,}eq on pointers.
The alternative here would have been to broaden tt_int_op to accept
a long long or an intptr_t, but that's less correct (since pointers
aren't integers), and would hurt the portability of tinytest a
little.
Fixes part of 7260.
