Peter Palfrader [Mon, 11 Jul 2016 07:37:01 +0000 (09:37 +0200)]
Add (SOCK_DGRAM, IPPROTO_UDP) sockets to the sandboxing whitelist
If we did not find a non-private IPaddress by iterating over interfaces,
we would try to get one via
get_interface_address6_via_udp_socket_hack(). This opens a datagram
socket with IPPROTO_UDP. Previously all our datagram sockets (via
libevent) used IPPROTO_IP, so we did not have that in the sandboxing
whitelist. Add (SOCK_DGRAM, IPPROTO_UDP) sockets to the sandboxing
whitelist. Fixes bug 19660.
When checking if a routerstatus is reachable, don't check the node
This fixes #19608, allowing IPv6-only clients to use
microdescriptors, while preserving the ability of bridge clients
to have some IPv4 bridges and some IPv6 bridges.
Run asciidoc in UTC timezone for build reproducibility.
asciidoc adds a timestamp at the end of a generated HTML file.
This timestamp is based on the date of the file but it can change
depending on the TZ environment variable.
Peter Palfrader [Sun, 3 Jul 2016 16:03:26 +0000 (18:03 +0200)]
sandboxing: allow writing to stats/hidserv-stats
Our sandboxing code would not allow us to write to stats/hidserv-stats,
causing tor to abort while trying to write stats. This was previously
masked by bug#19556.
Peter Palfrader [Sun, 3 Jul 2016 15:47:45 +0000 (17:47 +0200)]
sandboxing: allow open() of stats dir
When sandboxing is enabled, we could not write any stats to disk.
check_or_create_data_subdir("stats"), which prepares the private stats
directory, calls check_private_dir(), which also opens and not just stats() the
directory. Therefore, we need to also allow open() for the stats dir in our
sandboxing setup.
Comment-out fallbacks in a way the stem fallback parser understands
If we manually remove fallbacks in C by adding '/*' and '*/' on separate
lines, stem still parses them as being present, because it only looks at
the start of a line.
Add a comment to this effect in the generated source code.
Remove and blacklist 4 fallbacks which are unsuitable
Remove a fallback that changed its fingerprint after it was listed
This happened after to a software update:
https://lists.torproject.org/pipermail/tor-relays/2016-June/009473.html
Remove a fallback that changed IPv4 address
Remove two fallbacks that were slow to deliver consensuses,
we can't guarantee they'll be fast in future.
Blacklist all these fallbacks until operators confirm they're stable.
Operators have sent emails asking to have their relays added or
removed from the fallback list. Since none of the blacklisted
relays are in the hard-coded falback list, it does not need to
be changed.
Yawning Angel [Tue, 14 Jun 2016 06:14:28 +0000 (06:14 +0000)]
Bug 19406: OpenSSL made RSA and DH opaque in 1.1.0.
There's accessors to get at things, but it ends up being rather
cumbersome. The only place where behavior should change is that the
code will fail instead of attempting to generate a new DH key if our
internal sanity check fails.
Like the previous commit, this probably breaks snapshots prior to pre5.
Yawning Angel [Tue, 14 Jun 2016 04:40:36 +0000 (04:40 +0000)]
Bug 19406: OpenSSL changed the Thread API in 1.1.0 again.
Instead of `ERR_remove_thread_state()` having a modified prototype, it
now has the old prototype and a deprecation annotation. Since it's
pointless to add extra complexity just to remain compatible with an old
OpenSSL development snapshot, update the code to work with 1.1.0pre5
and later.
Nick Mathewson [Thu, 2 Jun 2016 14:40:39 +0000 (10:40 -0400)]
Use directory_must_use_begindir to predict we'll surely use begindir
Previously, we used !directory_fetches_from_authorities() to predict
that we would tunnel connections. But the rules have changed
somewhat over the course of 0.2.8
Nick Mathewson [Mon, 30 May 2016 16:03:03 +0000 (12:03 -0400)]
Fix a warning on unnamed nodes in node_get_by_nickname().
There was a > that should have been an ==, and a missing !. These
together prevented us from issuing a warning in the case that a
nickname matched an Unnamed node only.
Nick Mathewson [Wed, 25 May 2016 15:52:52 +0000 (11:52 -0400)]
Fix a double-free bug in routerlist_reparse_old
I introduced this bug when I moved signing_key_cert into
signed_descriptor_t. Bug not in any released Tor. Fixes bug 19175, and
another case of 19128.
Just like signed_descriptor_from_routerinfo(), routerlist_reparse_old()
copies the fields from one signed_descriptor_t to another, and then
clears the fields from the original that would have been double-freed by
freeing the original. But when I fixed the s_d_f_r() bug [#19128] in 50cbf220994c7cec593, I missed the fact that the code was duplicated in
r_p_o().
Duplicated code strikes again!
For a longer-term solution here, I am not only adding the missing fix to
r_p_o(): I am also extracting the duplicated code into a new function.
Many thanks to toralf for patiently sending me stack traces until
one made sense.
Nick Mathewson [Fri, 20 May 2016 17:58:52 +0000 (13:58 -0400)]
Fix a dangling pointer issue in our RSA keygen code
If OpenSSL fails to generate an RSA key, do not retain a dangling
pointer to the previous (uninitialized) key value. The impact here
should be limited to a difficult-to-trigger crash, if OpenSSL is
running an engine that makes key generation failures possible, or if
OpenSSL runs out of memory. Fixes bug 19152; bugfix on
0.2.1.10-alpha. Found by Yuan Jochen Kang, Suman Jana, and Baishakhi
Ray.
This is potentially scary stuff, so let me walk through my analysis.
I think this is a bug, and a backport candidate, but not remotely
triggerable in any useful way.
Observation 1a:
Looking over the OpenSSL code here, the only way we can really fail in
the non-engine case is if malloc() fails. But if malloc() is failing,
then tor_malloc() calls should be tor_asserting -- the only way that an
attacker could do an exploit here would be to figure out some way to
make malloc() fail when openssl does it, but work whenever Tor does it.
(Also ordinary malloc() doesn't fail on platforms like Linux that
overcommit.)
Observation 1b:
Although engines are _allowed_ to fail in extra ways, I can't find much
evidence online that they actually _do_ fail in practice. More evidence
would be nice, though.
Observation 2:
We don't call crypto_pk_generate*() all that often, and we don't do it
in response to external inputs. The only way to get it to happen
remotely would be by causing a hidden service to build new introduction
points.
Observation 3a:
So, let's assume that both of the above observations are wrong, and the
attacker can make us generate a crypto_pk_env_t with a dangling pointer
in its 'key' field, and not immediately crash.
This dangling pointer will point to what used to be an RSA structure,
with the fields all set to NULL. Actually using this RSA structure,
before the memory is reused for anything else, will cause a crash.
In nearly every function where we call crypto_pk_generate*(), we quickly
use the RSA key pointer -- either to sign something, or to encode the
key, or to free the key. The only exception is when we generate an
intro key in rend_consider_services_intro_points(). In that case, we
don't actually use the key until the intro circuit is opened -- at which
point we encode it, and use it to sign an introduction request.
So in order to exploit this bug to do anything besides crash Tor, the
attacker needs to make sure that by the time the introduction circuit
completes, either:
* the e, d, and n BNs look valid, and at least one of the other BNs is
still NULL.
OR
* all 8 of the BNs must look valid.
To look like a valid BN, *they* all need to have their 'top' index plus
their 'd' pointer indicate an addressable region in memory.
So actually getting useful data of of this, rather than a crash, is
going to be pretty damn hard. You'd have to force an introduction point
to be created (or wait for one to be created), and force that particular
crypto_pk_generate*() to fail, and then arrange for the memory that the
RSA points to to in turn point to 3...8 valid BNs, all by the time the
introduction circuit completes.
Naturally, the signature won't check as valid [*], so the intro point
will reject the ESTABLISH_INTRO cell. So you need to _be_ the
introduction point, or you don't actually see this information.
[*] Okay, so if you could somehow make the 'rsa' pointer point to a
different valid RSA key, then you'd get a valid signature of an
ESTABLISH_INTRO cell using a key that was supposed to be used for
something else ... but nothing else looks like that, so you can't use
that signature elsewhere.
Observation 3b:
Your best bet as an attacker would be to make the dangling RSA pointer
actually contain a fake method, with a fake RSA_private_encrypt
function that actually pointed to code you wanted to execute. You'd
still need to transit 3 or 4 pointers deep though in order to make that
work.
Conclusion:
By 1, you probably can't trigger this without Tor crashing from OOM.
By 2, you probably can't trigger this reliably.
By 3, even if I'm wrong about 1 and 2, you have to jump through a pretty
big array of hoops in order to get any kind of data leak or code
execution.
So I'm calling it a bug, but not a security hole. Still worth
patching.
Nick Mathewson [Thu, 19 May 2016 14:52:27 +0000 (10:52 -0400)]
Fix a pointer arithmetic bug in memarea_alloc()
Fortunately, the arithmetic cannot actually overflow, so long as we
*always* check for the size of potentially hostile input before
copying it. I think we do, though. We do check each line against
MAX_LINE_LENGTH, and each object name or object against
MAX_UNPARSED_OBJECT_SIZE, both of which are 128k. So to get this
overflow, we need to have our memarea allocated way way too high up
in RAM, which most allocators won't actually do.
Bugfix on 0.2.1.1-alpha, where memarea was introduced.