The old implementation had duplicated code in a bunch of places, and
it interspersed spool-management with resource management. The new
implementation should make it easier to add new resource types and
maintain the spooling code.
Closing ticket 21651.
When calculating max sampled size, Tor would only count the number of
bridges in torrc, without considering that our state file might already
have sampled bridges in it. This caused problems when people swap
bridges, since the following error would trigger:
[warn] Not expanding the guard sample any further; just hit the
maximum sample threshold of 1
This patch changes the way we decide when to check for whether it's time
to rotate and/or expiry our onion keys. Due to proposal #274 we can now
have the keys rotate at different frequencies than before and we thus
do the check once an hour when our Tor daemon is running in server mode.
This should allow us to quickly notice if the network consensus
parameter have changed while we are running instead of having to wait
until the current parameters timeout value have passed.
See: See: https://bugs.torproject.org/21641
This patch adds a new timer that is executed when it is time to expire
our current set of old onion keys. Because of proposal #274 this can no
longer be assumed to be at the same time we rotate our onion keys since
they will be updated less frequently.
See: https://bugs.torproject.org/21641
This patch adds an API to get the current grace period, in days, defined
as the consensus parameter "onion-key-grace-period-days".
As per proposal #274 the values for "onion-key-grace-period-days" is a
default value of 7 days, a minimum value of 1 day, and a maximum value
defined by other consensus parameter "onion-key-rotation-days" also
defined in days.
See: https://bugs.torproject.org/21641
This patch turns `MIN_ONION_KEY_LIFETIME` into a new function
`get_onion_key_lifetime()` which gets its value from a network consensus
parameter named "onion-key-rotation-days". This allows us to tune the
value at a later point in time with no code modifications.
We also bump the default onion key lifetime from 7 to 28 days as per
proposal #274.
See: https://bugs.torproject.org/21641
This patch fixes a regression described in bug #21757 that first
appeared after commit 6e78ede73f which was an attempt to fix bug #21654.
When switching from buffered I/O to direct file descriptor I/O our
output strings from get_string_from_pipe() might contain newline
characters (\n). In this patch we modify tor_get_lines_from_handle() to
ensure that the function splits the newly read string at the newline
character and thus might return multiple lines from a single call to
get_string_from_pipe().
Additionally, we add a test case to test_util_string_from_pipe() to
ensure that get_string_from_pipe() correctly returns multiple lines in a
single call.
See: https://bugs.torproject.org/21757
See: https://bugs.torproject.org/21654
We could use one of these for holding "junk" descriptors and
unparseable things -- but we'll _need_ it for having cached
consensuses and diffs between them.
There was a frequent block of code that did "find the next router
line, see if we've hit the end of the list, get the ID hash from the
line, and enforce well-ordering." Per Ahf's review, I'm extracting
it to its own function.
Previously, we operated on smartlists of NUL-terminated strings,
which required us to copy both inputs to produce the NUL-terminated
strings. Then we copied parts of _those_ inputs to produce an
output smartlist of NUL-terminated strings. And finally, we
concatenated everything into a final resulting string.
This implementation, instead, uses a pointer-and-extent pattern to
represent each line as a pointer into the original inputs and a
length. These line objects are then added by reference into the
output. No actual bytes are copied from the original strings until
we finally concatenate the final result together.
Bookkeeping structures and newly allocated strings (like ed
commands) are allocated inside a memarea, to avoid needless mallocs
or complicated should-I-free-this-or-not bookkeeping.
In my measurements, this improves CPU performance by something like
18%. The memory savings should be much, much higher.
This takes two fuzzers: one which generates a diff and makes sure it
works, and one which applies a diff.
So far, they won't crash, but there's a bug in my
string-manipulation code someplace that I'm having to work around,
related to the case where you have a blank line at the end of a
file, or where you diff a file with itself.
Also, add very strict split/join functions, and totally forbid
nonempty files that end with somethig besides a newline. This
change is necessary to ensure that diff/apply are actually reliable
inverse operations.
The 2-line diff changs is needed to make the unit tests actually
test the cases that they thought they were testing.
The bogus free was found while testing those cases
(This commit was extracted by nickm based on the final outcome of
the project, taking only the changes in the files touched by this
commit from the consdiff_rebased branch. The directory-system
changes are going to get worked on separately.)
Windows doesn't let you check the socket error for a socket with
WSAGetLastError() and getsockopt(SO_ERROR). But
getsockopt(SO_ERROR) clears the error on the socket, so you can't
call it more than once per error.
When we introduced recv_ni to help drain alert sockets, back in
0.2.6.3-alpha, we had the failure path for recv_ni call getsockopt()
twice, though: once to check for EINTR and one to check for EAGAIN.
Of course, we never got the eagain, so we treated it as an error,
and warned about: "No error".
The fix here is to have these functions return -errno on failure.
Fixes bug 21540; bugfix on 0.2.6.3-alpha.
The 64-bit load and store code was generating pretty bad output with
my compiler, so I extracted the code from csiphash and used that instead.
Close ticket 21737
So we require that SMARTLIST_FOREACH_END() have the name of the loop
variable in it. But right now the only enforcement for that is to
clear the variable at the end of the loop, which is really not
sufficient: I spent 45 minutes earlier today debugging an issue
where I had said:
SMARTLIST_FOREACH_BEGIN(spool, spooled_resource_t *, spooled) {
...
} SMARTLIST_FOREACH_END(spool);
This patch makes it so that ONLY loop variables can be used, by
referring to the _sl_idx variable.
Also, relaxed the checks of encrypted_data_length_is_valid() since now
only one encrypted section has padding requirements and we don't
actually care to check that all the padding is there.
Consider starting code review from function encode_superencrypted_data().
- Refactor our HS desc crypto funcs to be able to differentiate between
the superencrypted layer and the encrypted layer so that different
crypto constants and padding is used in each layer.
- Introduce some string constants.
- Add some comments.
As part of the work for proposal #274 we are going to remove the need
for MIN_ONION_KEY_LIFETIME and turn it into a dynamic value defined by a
consensus parameter.
See: https://bugs.torproject.org/21641
The bridges+ipv6-min integration test has a client with bridges:
Bridge 127.0.0.1:5003
Bridge [::1]:5003
which got stuck in guard_selection_have_enough_dir_info_to_build_circuits()
because it couldn't find the descriptor of both bridges.
Specifically, the guard_has_descriptor() function could not find the
node_t of the [::1] bridge, because the [::1] bridge had no identity
digest assigned to it.
After further examination, it seems that during fetching the descriptor
for our bridges, we used the CERTS cell to fill the identity digest of
127.0.0.1:5003 properly. However, when we received a CERTS cell from
[::1]:5003 we actually ignored its identity digest because the
learned_router_identity() function was using
get_configured_bridge_by_addr_port_digest() which was returning the
127.0.0.1 bridge instead of the [::1] bridge (because it prioritizes
digest matching over addrport matching).
The fix replaces get_configured_bridge_by_addr_port_digest() with the
recent get_configured_bridge_by_exact_addr_port_digest() function. It
also relaxes the constraints of the
get_configured_bridge_by_exact_addr_port_digest() function by making it
return bridges whose identity digest is not yet known.
By using the _exact_() function, learned_router_identity() actually
fills in the identity digest of the [::1] bridge, which then allows
guard_has_descriptor() to find the right node_t and verify that the
descriptor is there.
FWIW, in the bridges+ipv6-min test both 127.0.0.1 and [::1] bridges
correspond to the same node_t, which I guess makes sense given that it's
actually the same underlying bridge.
This patch removes the `tor_fgets()` wrapper around `fgets(3)` since it
is no longer needed. The function was created due to inconsistency
between the returned values of `fgets(3)` on different versions of Unix
when using `fgets(3)` on non-blocking file descriptors, but with the
recent changes in bug #21654 we switch from unbuffered to direct I/O on
non-blocking file descriptors in our utility module.
We continue to use `fgets(3)` directly in the geoip and dirserv module
since this usage is considered safe.
This patch also removes the test-case that was created to detect
differences in the implementation of `fgets(3)` as well as the changes
file since these changes was not included in any releases yet.
See: https://bugs.torproject.org/21654
This patch changes a number of read loops in the util module to use
less-than comparison instead of not-equal-to comparison. We do this in
the case that we have a bug elsewhere that might cause `numread` to
become larger than `count` and thus become an infinite loop.
This patch removes the buffered I/O stream usage in process_handle_t and
its related utility functions. This simplifies the code and avoids racy
code where we used buffered I/O on non-blocking file descriptors.
See: https://bugs.torproject.org/21654
This patch modifies `tor_read_all_handle()` to use read(2) instead of
fgets(3) when reading the stdout from the child process. This should
eliminate the race condition that can be triggered in the 'slow/util/*'
tests on slower machines running OpenBSD, FreeBSD and HardenedBSD.
See: https://bugs.torproject.org/21654
Make hidden services with 8 to 10 introduction points check for failed
circuits immediately after startup. Previously, they would wait for 5
minutes before performing their first checks.
Fixes bug 21594; bugfix on commit 190aac0eab in Tor 0.2.3.9-alpha.
Reported by alecmuffett.
This change is the only one necessary to allow future versions of
the microdescriptor consensus to replace every 'published' date with
e.g. 2038-01-01 00:00:00; this will save 50-75% in compressed
microdescriptor diff size, which is quite significant.
This commit is a minimal change for 0.2.9; future series will
reduce the use of the 'published' date even more.
Implements part of ticket 21642; implements part of proposal 275.
Previously, they would stop checking when they exceeded their intro point
creation limit.
Fixes bug 21596; bugfix on commit d67bf8b2f2 in Tor 0.2.7.2-alpha.
Reported by alecmuffett.
Previously, they would stop checking when they exceeded their intro point
creation limit.
Fixes bug 21596; bugfix on commit d67bf8b2f2 in Tor 0.2.7.2-alpha.
Reported by alecmuffett.
This patch resets `buf` in test_util_fgets_eagain() after each succesful
ivocation to avoid stray artifacts left in the buffer by erroneous
tor_fgets() calls.
This patch adds the `tor_fgets()` function to our compatibility layer.
`tor_fgets()` adds an additional check for whether the error-bit have
been enabled for the given file stream, if that is the case and `errno`
is set to `EAGAIN` we make sure that we always return NULL.
Unfortunately `fgets(3)` behaves differently on different versions of
the C library.
See: https://bugs.torproject.org/21416
See: https://bugs.torproject.org/20988
In that chutney test, the bridge client is configured to connect to
the same bridge at 127.0.0.1:5003 _and_ at [::1]:5003, with no
change in transports.
That meant, I think, that the descriptor is only assigned to the
first bridge when it arrives, and never the second.
- Make sure we check at least two guards for descriptor before making
circuits. We typically use the first primary guard for circuits, but
it can also happen that we use the second primary guard (e.g. if we
pick our first primary guard as an exit), so we should make sure we
have descriptors for both of them.
- Remove BUG() from the guard_has_descriptor() check since we now know
that this can happen in rare but legitimate situations as well, and we
should just move to the next guard in that case.
(But use bash if it's available.)
This is a workaround until we remove bash-specific code in 19699.
Fixes bug 21581; bugfix on 21562, not in any released version of tor.
Previously I'd made a bad assumption in the implementation of
prop271 in 0.3.0.1-alpha: I'd assumed that there couldn't be two
guards with the same identity. That's true for non-bridges, but in
the bridge case, we allow two bridges to have the same ID if they
have different addr:port combinations -- in order to have the same
bridge ID running multiple PTs.
Fortunately, this assumption wasn't deeply ingrained: we stop
enforcing the "one guard per ID" rule in the bridge case, and
instead enforce "one guard per <id,addr,port>".
We also needed to tweak our implementation of
get_bridge_info_for_guard, since it made the same incorrect
assumption.
Fixes bug 21027; bugfix on 0.3.0.1-alpha.
This feature makes it possible to turn off memory sentinels (like
those used for safety in buffers.c and memarea.c) when fuzzing, so
that we can catch bugs that they would otherwise prevent.
Since 0.2.4.11-alpha (in 0196647970) we've tried to randomize
the start time to up to some time in the past. But unfortunately we
allowed the start time to be in the future as well, which isn't
really legit.
The new behavior lets the start time be be up to
MAX(cert_lifetime-2days, 0) in the past, but never in the future.
Fixes bug 21420; bugfix on 0.2.4.11-alpha.
Teor thinks that this connection_dirserv_add_dir_bytes_to_outbuf()
might be the problem, if the "remaining" calculation underflows. So
I'm adding a couple of checks there, and improving the casts.
When encoding a legacy ESTABLISH_INTRO cell, we were using the sizeof() on a
pointer instead of using the real size of the destination buffer leading to an
overflow passing an enormous value to the signing digest function.
Fortunately, that value was only used to make sure the destination buffer
length was big enough for the key size and in this case it always was because
of the overflow.
Fixes#21553
Signed-off-by: David Goulet <dgoulet@torproject.org>
strto* and _atoi64 accept +, -, and various whitespace before numeric
characters. And permitted whitespace is different between POSIX and Windows.
Fixes bug 21507 and part of 21508; bugfix on 0.0.8pre1.
This patch makes us store the number of sent and received RELAY_DATA
cells used for directory connections. We log the numbers after we have
received an EOF in connection_dir_client_reached_eof() from the
directory server.
Instead of returning 404 error code, this led to a NULL pointer being used and
thus a crash of tor.
Fixes#21471
Signed-off-by: David Goulet <dgoulet@torproject.org>
Fixes bug 20894; bugfix on 0.2.0.16-alpha.
We already applied a workaround for this as 20834, so no need to
freak out (unless you didn't apply 20384 yet).
This should be "impossible" without making a SHA1 collision, but
let's not keep the assumption that SHA1 collisions are super-hard.
This prevents another case related to 21278. There should be no
behavioral change unless -ftrapv is on.
I think this one probably can't underflow, since the input ranges
are small. But let's not tempt fate.
This patch also replaces the "cmp" functions here with just "eq"
functions, since nothing actually checked for anything besides 0 and
nonzero.
Related to 21278.
Fix for TROVE-2017-001 and bug 21278.
(Note: Instead of handling signed ints "correctly", we keep the old
behavior, except for the part where we would crash with -ftrapv.)
This is a purely cosmetic patch that changes RELAY_BEGINDIR in various
comments to RELAY_BEGIN_DIR, which should make it easier to grep for the
symbols.
According to 21116, it seems to be needed for Wheezy Raspbian build. Also,
manpage of socket(2) does confirm that this errno value should be catched as
well in case of no support from the OS of IPv4 or/and IPv6.
Fixes#21116
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch ensures that we log the size of the inbuf when a directory
client have reached EOF on the connection.
See: https://bugs.torproject.org/21206
This patch makes the log-statements in `connection_dir_client_reached_eof`
more explicit by writing "body size" instead of just "size" which could
be confused as being the size of the entire response, which would
include HTTP status-line and headers.
See: https://bugs.torproject.org/21206
This reverts commit 5446cb8d3d.
The underlying revert was done in 0.2.6, since we aren't backporting
seccomp2 loosening fixes to 0.2.6. But the fix (for 17354) already
went out in 0.2.7.4-rc, so we shouldn't revert it in 0.2.7.
This patch adds a debug log statement when sending a request to a
directory server. The information logged includes: the payload size (if
available), the total size of the request, the address and port of the
directory server, and the purpose of the directory connection.
See: https://bugs.torproject.org/21206
maint-0.2.7-redux is an attempt to try to re-create a plausible
maint-0.2.7 branch. I've started from the tor-0.2.7.6, and then I
merged maint-0.2.6 into the branch.
This has produced 2 conflicts: one related to the
rendcommon->rendcache move, and one to the authority refactoring.
If a hostname is supplied to tor-resolve which is too long, it will be
silently truncated, resulting in a different hostname lookup:
$ tor-resolve $(python -c 'print("google.com" + "m" * 256)')
If tor-resolve uses SOCKS5, the length is stored in an unsigned char,
which overflows in this case and leads to the hostname "google.com".
As this one is a valid hostname, it returns an address instead of giving
an error due to the invalid supplied hostname.
Check size argument to memwipe() for underflow.
Closes bug #18089. Reported by "gk", patch by "teor".
Bugfix on 0.2.3.25 and 0.2.4.6-alpha (#7352),
commit 49dd5ef3 on 7 Nov 2012.
The length of auth_data from an INTRODUCE2 cell is checked when the
auth_type is recognized (1 or 2), but not for any other non-zero
auth_type. Later, auth_data is assumed to have at least
REND_DESC_COOKIE_LEN bytes, leading to a client-triggered out of bounds
read.
Fixed by checking auth_len before comparing the descriptor cookie
against known clients.
Fixes#15823; bugfix on 0.2.1.6-alpha.
Bug 21242 occurred because we asserted that extend_info_from_node()
had succeeded...even though we already had the code to handle such a
failure. We fixed that in 93b39c5162.
But there were four other cases in our code where we called
extend_info_from_node() and either tor_assert()ed that it returned
non-NULL, or [in one case] silently assumed that it returned
non-NULL. That's not such a great idea. This patch makes those
cases check for a bug of this kind instead.
Fixes bug 21372; bugfix on 0.2.3.1-alpha when
extend_info_from_node() was introduced.
Once a second, we go over all services and consider the validity of the intro
points. Now, also try to remove expiring nodes that have no more circuit
associated to them. This is possible if we moved an intro point object
previously to that list and the circuit actually timed out or was closed by
the introduction point itself.
Signed-off-by: David Goulet <dgoulet@torproject.org>
In rend_service_intro_has_opened(), this is subject to a possible underflow
because of how the if() casts the results. In the case where the expiring
nodes list length is bigger than the number of IP circuits, we end up in the
following situation where the result will be cast to an unsigned int. For
instance, "5 - 6" is actually a BIG number.
Ultimately leading to closing IP circuits in a non stop loop.
Partially fixes#21302.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously the dirserv_orconn_tls_done() function would skip routers
when they advertised an ed25519 key but didn't present it during the
link handshake. But that covers all versions between 0.2.7.2-alpha
and 0.2.9.x inclusive!
Fixes bug 21107; bugfix on 0.3.0.1-alpha.
Because we don't allow client functionalities in non anonymous mode,
recommending Tor2web is a bad idea.
If a user wants to use Tor2web as a client (losing all anonymity), it should
run a second tor, not use it with a single onion service tor.
Fixes#21294.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch adds checks for expected log messages for failure cases of
different ill-formed ESTABLISH_INTRO cell's.
See: https://bugs.torproject.org/21266
In rend_consider_services_intro_points(), we had a possible interger underflow
which could lead to creating a very large number of intro points. We had a
safe guard against that *except* if the expiring_nodes list was not empty
which is realistic thing.
This commit removes the check on the expiring nodes length being zero. It's
not because we have an empty list of expiring nodes that we don't want to open
new IPs. Prior to this check, we remove invalid IP nodes from the main list of
a service so it should be the only thing to look at when deciding if we need
to create new IP(s) or not.
Partially fixes#21302.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Determining if OpenSSL structures are opaque now uses an autoconf check
instead of comparing the version number. Some definitions have been
moved to their own check as assumptions which were true for OpenSSL
with opaque structures did not hold for LibreSSL. Closes ticket 21359.
This disregards anything smaller than an IPv6 /64, and rejects ports that
are rejected on an IPv6 /16 or larger.
Adjust existing unit tests, and add more to cover exceptional cases.
No IPv4 behaviour changes.
Fixes bug 21357
This interim fix results in too many IPv6 rejections.
No behaviour change for IPv4 counts, except for overflow fixes that
would require 4 billion redundant 0.0.0.0/0 policy entries to trigger.
Part of 21357
Stop modifying the value of our torrc option HiddenServiceStatistics just
because we're not a bridge or relay. This bug was causing Tor Browser users to
write "HiddenServiceStatistics 0" in their torrc files as if they had chosen
to change the config.
Fixes#21150
Signed-off-by: David Goulet <dgoulet@torproject.org>
Since we can call this function more than once before we update all
the confirmed_idx fields, we can't rely on all the relays having an
accurate confirmed_idx.
Fixes bug 21129; bugfix on 0.3.0.1-alpha
We need to call it before nt_service_parse_options(), since
nt_service_parse_options() can call back into nt_service_main(),
which calls do_main_loop().
Fixes bug 21356; bugfix on 0.2.9.1-alpha.
In addition to not wanting to build circuits until we can see most
of the paths in the network, and in addition to not wanting to build
circuits until we have a consensus ... we shouldn't build circuits
till all of our (in-use) primary guards have descriptors that we can
use for them.
This is another bug 21242 fix.
Actually, it's _fine_ to use a descriptorless guard for fetching
directory info -- we just shouldn't use it when building circuits.
Fortunately, we already have a "usage" flag that we can use here.
Partial fix for bug 21242.
This relates to the 21242 fix -- entry_guard_pick_for_circuit()
should never yield nodes without descriptors when the node is going
to be used for traffic, since we won't be able to extend through
them.
This assertion triggered in the (error) case where we got a result
from guards_choose_guard() without a descriptor. That's not
supposed to be possible, but it's not worth crashing over.
I broke "GETCONF *Port" in 20956, when I made SocksPort a
subordinate option of the virtual option SocksPortLines, so that I
could make SocksPort and __SocksPort provide qthe same
functionality. The problem was that you can't pass a subordinate
option to GETCONF.
So, this patch fixes that by letting you fetch subordinate options.
It won't always be meaningful to consider these options
out-of-context, but that can be the controller-user's
responsibility to check.
Closes ticket 21300.
If there are no ephemeral or detached onion services, then
"GETINFO onions/current" or "GETINFO onions/detached" should
return an empty list instead of an error
If tor_mmap_file is called with a file which is larger than SIZE_MAX,
only a small part of the file will be memory-mapped due to integer
truncation.
This can only realistically happen on 32 bit architectures with large
file support.
If a hostname is supplied to tor-resolve which is too long, it will be
silently truncated, resulting in a different hostname lookup:
$ tor-resolve $(python -c 'print("google.com" + "m" * 256)')
If tor-resolve uses SOCKS5, the length is stored in an unsigned char,
which overflows in this case and leads to the hostname "google.com".
As this one is a valid hostname, it returns an address instead of giving
an error due to the invalid supplied hostname.
When marking for close a circuit, the reason value, a integer, was assigned to
a uint16_t converting any negative reasons (internal) to the wrong value. On
the HS side, this was causing the client to flag introduction points to be
unreachable as the internal reason was wrongfully converted to a positive
16bit value leading to flag 2 out of 3 intro points to be unreachable.
Fixes#20307 and partially fixes#21056
Signed-off-by: David Goulet <dgoulet@torproject.org>
- Also remove LCOV marks from blocks of code that can be reachable by tests
if we mock relay_send_command_from_edge().
Signed-off-by: David Goulet <dgoulet@torproject.org>
With the previous commit, we validate the circuit _before_ calling
rend_mid_introduce() which handles the INTRODUCE1 payload.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Adds a better semantic and it also follows the same interface for the
INTRODUCE1 API which is circuit_is_suitable_for_introduce1().
Signed-off-by: David Goulet <dgoulet@torproject.org>
That way, when we are parsing the options and LearnCircuitBuildTimeout is set
to 0, we don't assert trying to get the options list with get_options().
Fixes#21062
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch refactors duplicated code, to check if a given router
supports fetching the extra-info document, into a common macro called
SKIP_MISSING_TRUSTED_EXTRAINFO.
This patch generalizes the two functions
router_is_already_dir_fetching_rs and router_is_already_dir_fetching_ds
into a single function, router_is_already_dir_fetching_, by lifting the
passing of the IPv4 & IPv6 addresses and the directory port number to
the caller.
So far, the TTLs for both A and AAAA records were not initialised,
resulting in exit relays sending back the value 60 to Tor clients. This
also impacts exit relays' DNS cache -- the expiry time for all domains
is set to 60.
This fixes <https://bugs.torproject.org/19025>.
The server-side clipping now clamps to one of two values, both
for what to report, and how long to cache.
Additionally, we move some defines to dns.h, and give them better
names.
An operator couldn't set the number of introduction point below the default
value which is 3. With this commit, from 0 to the hardcoded maximum is now
allowed.
Closes#21033
Signed-off-by: David Goulet <dgoulet@torproject.org>
Our config code is checking correctly at DataDirectoryGroupReadable but then
when we initialize the keys, we ignored that option ending up at setting back
the DataDirectory to 0700 instead of 0750. Patch by "redfish".
Fixes#19953
Signed-off-by: David Goulet <dgoulet@torproject.org>
In the past, when we exhausted all guards in our sampled set, we just
waited there till we mark a guard for retry again (usually takes 10 mins
for a primary guard, 1 hour for a non-primary guard). This patch marks
all guards as maybe-reachable when we exhaust all guards (this can
happen when network is down for some time).
Let A = UseBridges
Let B = ClientUseIPv4
Then firewall_is_fascist_impl expands and simplifies to:
B || (!(A || ...) && A)
B || (!A && ... && A)
B || 0
B
The microdesc consensus does not contain any IPv6 addresses.
When a client has a microdesc consensus but no microdescriptor, make it
use the hard-coded IPv6 address for the node (if available).
(Hard-coded addresses can come from authorities, fallback directories,
or configured bridges.)
If there is no hard-coded address, log a BUG message, and fail the
connection attempt. (All existing code checks for a hard-coded address
before choosing a node address.)
Fixes 20996, fix on b167e82 from 19608 in 0.2.8.5-alpha.
It is no longer possible for the IPv6 preference options to differ from the
IPv6 usage: preferring IPv6 implies possibly using IPv6.
Also remove the corresponding unit test warning message checks.
(But keep the unit tests themselves - they now run without warnings.)
In order to help an HS operator knowing if the application configured behind
it is not working properly, add a log at warning level for the connection
refused or timeout case. This log will only be printed if a client connection
fails and is rate limited.
Closes#21019
Signed-off-by: David Goulet <dgoulet@torproject.org>
In 8a0ea3ee43 we added a
temp_service_list local variable to rend_config_services, but we
didn't add a corresponding "free" for it to all of the exit paths.
Fixes bug 20987; bugfix on 0.3.0.1-alpha.
This commit adds 3 unit tests which validates a wrong signature length, a
wrong authentication key length and a wrong MAC in the cell.
Closes#20992
Signed-off-by: David Goulet <dgoulet@torproject.org>
Add the "sr/current" and "sr/previous" keys for the GETINFO command in order
to get through the control port the shared random values from the consensus.
Closes#19925
Signed-off-by: David Goulet <dgoulet@torproject.org>
Some DNS NXDOMAIN hijackers hijack truly ridiculous domains, like
"invalid-stuff!!" or "1.2.3.4.5". This would provoke unit test
failures where we used addresses like that to force
tor_addr_lookup() to fail. The fix, for testing, is to mock
tor_addr_lookup() with a variant that always fails when it gets
a name with a !.
Fixes bugs 20862 and 20863.
These relays need to be contacted over their ORPorts using a begindir
connection, and relays try not to use begindir connections.
Fixes bug 20711; bugfix on 0.2.8.2-alpha.
Because <unset> makes more sense than AAAAAAAAAAAAAAAAAAA...
(I have indeed verified that ed25519_fmt() is only used for
logging. This patch also clarifies the intention that ed25519_fmt()
is only for logging.
Closes ticket 21037.
We switched these to be "if (1) " a while back, so we could keep
the indentation and avoid merge conflicts. But it's nice to clean
up from time to time.
Previously we were marking directory guards up in
..._process_inbuf(), but that's wrong: we call that function on
close as well as on success. Instead, we're marking the dirguard up
only after we parse the HTTP headers. Closes 20974.
The abort handler masks the exit status of the backtrace generator by
capturing the abort signal from the backtrace handler and exiting with
zero. Because the output of the backtrace generator is meant to be piped
to `bt_test.py`, its exit status is unimportant and is currently
ignored.
The abort handler calls `exit(3)` which is not asynchronous-signal-safe
and calling it in this context is undefined behavior [0].
Closes ticket 21026.
[0] https://www.securecoding.cert.org/confluence/x/34At
When marking for close a circuit, the reason value, a integer, was assigned to
a uint16_t converting any negative reasons (internal) to the wrong value. On
the HS side, this was causing the client to flag introduction points to be
unreachable as the internal reason was wrongfully converted to a positive
16bit value leading to flag 2 out of 3 intro points to be unreachable.
Fixes#20307 and partially fixes#21056
Signed-off-by: David Goulet <dgoulet@torproject.org>
First, this commit moves the code used to prune the service list when
reloading Tor (HUP signal for instance) to a function from
rend_config_services().
Second, fix bug #21054, improve the code by using the newly added
circuit_get_next_service_intro_circ() function instead of poking at the global
list directly and add _many_ more comments.
Fixes#21054.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This came up on #21035, where somebody tried to build on a linux
system with kernel headers including CLOCK_MONOTONIC_COARSE, then
run on a kernel that didn't support it.
I've adopted a belt-and-suspenders approach here: we detect failures
at initialization time, and we also detect (loudly) failures later on.
Fixes bug 21035; bugfix on 0.2.9.1-alpha when we started using
monotonic time.
This helps protect against bugs where any part of a buf_t's memory
is passed to a function that expects a NUL-terminated input.
It also closes TROVE-2016-10-001 (aka bug 20384).
Replace the 81 remaining fallbacks of the 100 originally introduced
in Tor 0.2.8.3-alpha in March 2016, with a list of 177 fallbacks
(123 new, 54 existing, 27 removed) generated in December 2016.
Resolves ticket 20170.
In get_token(), we could read one byte past the end of the
region. This is only a big problem in the case where the region
itself is (a) potentially hostile, and (b) not explicitly
nul-terminated.
This patch fixes the underlying bug, and also makes sure that the
one remaining case of not-NUL-terminated potentially hostile data
gets NUL-terminated.
Fix for bug 21018, TROVE-2016-12-002, and CVE-2016-1254
They broke stem, and breaking application compatibility is usually a
bad idea.
This reverts commit 6e10130e18,
commit 78a13df158, and
commit 62f52a888a.
We might re-apply this later, if all the downstream tools can handle
it, and it turns out to be useful for some reason.
I got confused when I saw my Tor saying it was opening a file
that doesn't exist. It turns out it isn't opening it, it's just
calling open() on it and then moving on when it's not there.
Since both the client and service will use that data structure to store the
descriptor decoded data, only the public keys are common to both.
Fixes#20572.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The "sig_len" fields was moved below the "end_sig_fields" in the trunnel
specification so when signing the cell content, the function generating such a
cell needed to be adjust.
Closes#20991
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, we had NumEntryGuards kind of hardwired to 1. Now we
have the code (but not the configuarability) to choose randomly from
among the first N primary guards that would work, where N defaults
to 1.
Part of 20831 support for making NumEntryGuards work again.
Since we already had a separate function for getting the universe of
possible guards, all we had to do was tweak it to handle very the
GS_TYPE_RESTRICTED case.
asn found while testing that this function can be reached with
GUARD_STATE_COMPLETE circuits; I believe this happens when
cannibalization occurs.
The added complexity of handling one more state made it reasonable
to turn the main logic here into a switch statement.
Letting the maximum sample size grow proportionally to the number of
guards defeats its purpose to a certain extent. Noted by asn during
code review.
Fixes bug 20920; bug not in any released (or merged) version of Tor.
- Correctly maintain the previous guard selection in choose_guard_selection().
- Print bridge identifier instead of nothing in entry_guard_describe()._
If a complete circuit C2 doesn't obey the restrictions of C1, then
C2 cannot block C1.
The patch here is a little big-ish, since we can no longer look
through all the complete circuits and all the waiting circuits on a
single pass: we have to find the best waiting circuit first.
This is an important thing I hadn't considered when writing prop271:
sometimes you have to restrict what guard you use for a particular
circuit. Most frequently, that would be because you plan to use a
certain node as your exit, and so you can't choose that for your
guard.
This change means that the upgrade-waiting-circuits algorithm needs
a slight tweak too: circuit A cannot block circuit B from upgrading
if circuit B needs to follow a restriction that circuit A does not
follow.
I had been asking myself, "hey, doesn't the new code need to look at
this "info" parameter? The old code did!" But it turns out that the
old code hasn't, since 05f7336624.
So instead of "support this!" the comment now says "we can remove
this!"
George pointed out that (-1,0,1) for (never usable, maybe usable
later, usable right now) was a pretty rotten convention that made
the code harder to read.
Here we handle most (all?) of the remaining tasks, and fix some
bugs, in the prop271 bridge implementation.
* We record bridge identities as we learn them.
* We only call deprecated functions from bridges.c when the
deprecated guard algorithm is in use.
* We update any_bridge_descriptors_known() and
num_bridges_usable() to work correctly with the new backend
code. (Previously, they called into the guard selection logic.
* We update bridge directory fetches to work with the new
guard code.
* We remove some erroneous assertions where we assumed that we'd
never load a guard that wasn't for the current selection.
Also, we fix a couple of typos.
Still missing is functionality for picking bridges when we don't
know a descriptor for them yet, and functionality for learning a
bridge ID.
Everything else remains (basically) the same. Neat!
This includes:
* making bridge_info_t exposed but opaque
* allowing guards where we don't know an identity
* making it possible to learn the identity of a guard
* creating a guard that lacks a node_t
* remembering a guard's address and port.
* Looking up a guard by address and port.
* Only enforcing the rule that we need a live consensus to update
the "listed" status for guards when we are not using bridges.
This is safe, because no entry_guard_t ever outlives its
guard_selection_t.
I want this because now that multiple guard selections can be active
during one tor session, we should make sure that any information we
register about guards is with respect to the selection that they came
from.
Currently, this code doesn't actually have the contexts behave
differently, (except for the legacy context), but it does switch
back and forth between them nicely.
If a guard becomes primary as a result of confirming it, consider
the circuit through that guard as a primary circuit.
Also, note open questions on behavior when confirming nonprimary guards
Some of these will get torrc options to override them too; this
is just the mechanical conversion.
Also, add documentation for a couple of undocumented (but now used)
parameters.
Act I.
" But that I am forbid
To tell the secrets of my prison-house,
I could a tale unfold..."
Here's the bug: sometimes, rend_cache/store_v2_desc_as_client would
say:
"Dec 15 08:31:26.147 [warn] rend_cache_store_v2_desc_as_client():
Bug: Couldn't decode base32 [scrubbed] for descriptor id. (on Tor
0.3.0.0-alpha-dev 4098bfa260)"
When we merged ade5005853 back in 0.2.8.1-alpha, we added that
test: it mangles the hidden service ID for a hidden service, and
ensures that when the descriptor ID doesn't match the descriptor's
key, we don't store the descriptor.
How did it mangle the descriptor ID? By doing
desc_id_base32[0]++;
So, if the hidden service ID started with z or 7, we'd wind up with an
invalid base32 string, and get the warning. And if it started with
any other character, we wouldn't.
That there is part 1 of the bug: in 2/32 cases, we'd get a BUG
warning. But we wouldn't display it, since warnings weren't shown
from the unit tests.
Act II.
"Our indiscretion sometime serves us well,
When our deep plots do pall"
Part two: in 0.2.9.3-alpha, for part of #19999, we turned on BUG
warnings in the unit tests, so that we'd actually start seeing them.
At this point we also began to consider each BUG warning that made
it through the unit tests to be an actual bug. So before this
point, we wouldn't actually notice anything happening in those 2/32
cases.
So, at this point it was a nice random _visible_ bug.
Act III.
"Our thoughts are ours, their ends none of our own"
In acbb60cd63, which was part of my prop220 work, I
changed how RSA key generation worked in the unit tests. While
previously we'd use pre-made RSA keys in some cases, this change
made us use a set of pregenerated RSA keys for _all_ 1024 or 2048
keys, and to return them in a rotation when Tor tried to generate a
key.
And now we had the heisenbug: anything that affected the number of
pregenerated keys that we had yielded before reaching
rend_cache/store_v2_desc_as_client would make us return a different
key, which would give us a different base32 ID, which would make the
bug occur, or not. So as we added or removed test cases, the bug
might or might not happen.
So yeah. Don't mangle a base32 ID like that. Do it this way instead.
The new HS circuitmap API replaces old public functions as follows:
circuit_clear_rend_token -> hs_circuitmap_remove_circuit
circuit_get_rendezvous -> hs_circuitmap_get_rend_circ
circuit_get_intro_point -> hs_circuitmap_get_intro_circ_v2
circuit_set_rendezvous_cookie -> hs_circuitmap_register_rend_circ
circuit_set_intro_point_digest -> hs_circuitmap_register_intro_circ_v2
This commit also removes the old rendinfo code that is now unused.
It also fixes the broken rendinfo unittests.
The HS circuitmap is a hash table that maps introduction and rendezvous
tokens to specific circuits such that given a token it's easy to find
the corresponding circuit. It supports rend circuits and v2/v3 intro
circuits.
It will be used by the prop224 ESTABLISH_INTRO code to register and
lookup v3 introduction circuits.
The next commit after this removes the old code and fixes the unittests.
Please consult both commits while reviewing functionality differences
between the old and new code. Let me know if you want this rebased
differently :)
WRT architectural differences, this commit removes the rendinfo pointer
from or_circuit_t. It then adds an hs_token_t pointer and a hashtable
node for the HS circuitmap. IIUC, this adds another pointer to the
weight of or_circuit_t. Let me know if you don't like this, or if you
have suggestions on improving it.
Back when Roger had do do most of our testing on the moria host, we
needed a higher limit for the number of relays running on a single
IP address when that limit was shared with an authority. Nowadays,
the idea is pretty obsolete.
Also remove the router_addr_is_trusted_dir() function, which served
no other purpose.
Closes ticket 20960.
In c35fad2bde, merged in
0.2.4.7-alpha, we removed the code to parse v1 directory
objects. When we did so, we removed everything that could set the
CST_CHECK_AUTHORITY flag for check_signature_token().
So in this code, we remove the flag itself, the code to handle the
flag, and a function that only existed to handle the flag.
Circuit object wasn't freed correctly. Also, the cpath build state object
needed to be zeroed else we were freeing garbage pointers.
Closes#20936
Signed-off-by: David Goulet <dgoulet@torproject.org>
The signed_descriptor_move() was not releasing memory inside the destination
object before overwriting it with the source object. This commit adds a reset
function that free that memory inside a signed descriptor object and zero it.
Closes#20715.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This was breaking the build on debian precise, since it thought that
using a 'const int' to dimension an array made that array
variable-size, and made us not get protection.
Bug not in any released version of Tor.
I will insist that this one wasn't my fault.
"Variables won't. Constants aren't." -- Osborn's Law
If a node can prove its Ed25519 identity, don't consider connections
to it canonical unless they match both identities.
Includes link handshake changes needed to avoid crashing with bug
warnings, since the tests now reach more parts of the code.
Closes ticket 20355
(Only run the connection_or_group_set_badness_() function on groups
of channels that have the same RSA and Ed25519 identities.)
There's a possible opportunity here where we might want to set a
channel to "bad" if it has no ed25519 identity and some other
channel has some. Also there's an opportunity to add a warning if
we ever have an Ed mismatch on open connections with the same RSA
ID.
This function has never gotten testing for the case where an
identity had been set, and then got set to something else. Rather
than make it handle those cases, we forbid them.
If there is some horrible bug in our ed25519 link authentication
code that causes us to label every single ed25519-having node as
non-running, we'll be glad we had this. Otherwise we can remove it
later.
This patch makes two absolutely critical changes:
- If an ed25519 identity is not as expected when creating a channel,
we call that channel unsuccessful and close it.
- When a client creating a channel or an extend cell for a circuit, we
only include the ed25519 identity if we believe that the node on
the other side supports ed25519 link authentication (from
#15055). Otherwise we will insist on nodes without the right
link protocol authenticating themselves.
- When deciding to extend to another relay, we only upgrade the
extend to extend by ed25519 ID when we know the ed25519 ID _and_
we know that the other side can authenticate.
This patch also tells directory servers, when probing nodes, to
try to check their ed25519 identities too (if they can authenticate
by ed25519 identity).
Also, handle the case where we connect by RSA Id, and learn the
ED25519 ID for the node in doing so.
I need to be able to turn on Ed25519 support in client generation
of extend cells so I can test it, but leave it off-by-default until
enough clients support it for us to turn it on for a bunch at once.
This is part of #15056 / prop#220.
- forbid extending to the previous hop by Ed25519 ID.
- If we know the Ed25519 ID for the next hop and the client doesn't,
insist on the one from the consensus.
Right now, there's only a mechanism to look for a channel where the
RSA ID matches *and* the ED ID matches. We can add a separate map
later if we want.
They added clock_gettime(), but with tv_nsec as a long, whereas
tv_usec is a __darwin_suseconds_t (a.k.a. 'int'). Now, why would
they do that? Are they preparing for a world where there are more
than 2 billion nanoseconds per second? Are they planning for having
int be less than 32 bits again? Or are they just not paying
attention to the Darwin API?
Also, they forgot to mark clock_gettime() as Sierra-only, so even
if we fixed the issue here, we'd still be stick with portability
breakage like we were for 0.2.9.
So, just disable clock_gettime() on apple.
Tor 0.2.9 has a broader range of fixes and workarounds here, but for
0.2.8, we're just going to maintain the existing behavior.
(The alternative would be to backport both
1eba088054 and
16fcbd21c9 , but the latter is kind of
a subtle kludge in the configure.ac script, and I'm not a fan of
backporting that kind of thing.)
(OpenSSL 1.1 makes EVP_CIPHER_CTX opaque, _and_ adds acceleration
for counter mode on more architectures. So it won't work if we try
the older approach, and it might help if we try the newer one.)
Fixes bug 20588.
This resolves two issues:
* the checks in rend_add_services were only being performed when adding
the service, and not when the service was validated,
(this meant that duplicate checks were not being performed, and some SETCONF
commands appeared to succeed when they actually failed), and
* if one service failed while services were being added, then the service
list would be left in an inconsistent state (tor dies when this happens,
but the code is cleaner now).
Fixes#20860.
When computing old Tor protocol line version in protover, we were looking at
0.2.7.5 twice instead of the specific case for 0.2.9.1-alpha.
Fixes#20810
Signed-off-by: David Goulet <dgoulet@torproject.org>
Coverity doesn't like it when there are paths to the end of the
function where something doesn't get freed, even when those paths
are only reachable on unit test failure.
Fixes CID 1372899 and CID 1372900. Bug not in any released Tor.
newconn->address is strdup'ed twice when new_type == CONN_TYPE_AP
and conn->socket_family == AF_UNIX. Whilst here, juggle code to
make sure newconn->port is assigned from an initialised value in
the above case.
Instead, refuse to start tor until the misconfigurations have been corrected.
Fixes bug 20559; bugfix on multiple commits in 0.2.7.1-alpha and earlier.
Instead, refuse to start tor if any hidden service key has been used in
a different hidden service anonymity mode.
Fixes bug 20638; bugfix on 17178 in 0.2.9.3-alpha; reported by ahf.
The original single onion service poisoning code checked poisoning state
in options_validate, and poisoned in options_act. This was problematic,
because the global array of hidden services had not been populated in
options_validate (and there were ordrering issues with hidden service
directory creation).
This patch fixes this issue in rend_service_check_dir_and_add, which:
* creates the directory, or checks permissions on an existing directory, then
* checks the poisoning state of the directory, then
* poisons the directory.
When validating, only the permissions checks and the poisoning state checks
are perfomed (the directory is not modified).
To do this, it makes sense to treat legacy guards as a separate
guard_selection_t *, and handle them separately. This also means we
add support here for having multiple guard selections.
Note that we don't persist pathbias information yet; that will take
some refactoring.
This patch doesn't cover every case; omitted cases are marked with
"XXXX prop271", as usual. It leaves both the old interface and the
new interface for guard status notification, since they don't
actually work in the same way: the new API wants to be told when a
circuit has failed or succeeded, whereas the old API wants to know
when a channel has failed or succeeded.
I ran into some trouble with directory guard stuff, since when we
pick the directory guard, we don't actually have a circuit to
associate it with. I solved that by allowing guard states to be
associated with directory connections, not just circuits.
I expect we'll be ripping this out somewhere in 0.3.0, but let's
keep it around for a little while in case it turns out to be the
only way to avert disaster?
This state corresponds to the WAITING_FOR_BETTER_GUARD state; it's
for circuits that are 100% constructed, but which we won't use until
we are sure that we wouldn't use circuits with a better guard.
When a nonprimary guard's circuit is complete, we don't call it
actually usable until we are pretty sure that every better guard
is indeed not going to give us a working circuit.
Here we add a little bit of state to origin circuits, and set up
the necessary functions for the circuit code to call in order to
find guards, use guards, and decide when circuits can be used.
There's also an incomplete function for the hard part of the
circuit-maintenance code, where we figure out whether any waiting
guards are ready to become usable.
(This patch finally uses the handle.c code to make safe handles to
entry_guard_t objects, so that we are allowed to free an
entry_guard_t without checking whether any origin_circuit_t is
holding a reference to it.)
This code handles:
* Maintaining the sampled set, the filtered set, and the
usable_filtered set.
* Maintaining the confirmed and primary guard lists.
* Picking guards for circuits, and updating guard state when
circuit state changes.
Additionally, I've done code structure movement: even more constants
and structures from entrynodes.c have become ENTRYNODES_PRIVATE
fields of entrynodes.h.
I've also included a bunch of documentation and a bunch of unit
tests. Coverage on the new code is pretty high.
I've noted important things to resolve before this branch is done
with the /XXXX.*prop271/ regex.
These are taken from the proposal, and defined there. Some of them
should turn into consensus parameters.
Also, remove some dead code that was there to make compilation work,
and use ATTR_UNUSED like a normal person.
The previous commit, in moving a bunch of functions to bridges.c,
broke compilation because bridges.c required two entry points to
entrynodes.c it didn't have.
This patch is just:
* Code movement
* Adding headers here and there as needed
* Adding a bridges_free_all() with a call to it.
It breaks compilation, since the bridge code needed to make exactly
2 calls into entrynodes.c internals. I'll fix those in the next
commit.
The encoding code is very straightforward. The decoding code is a
bit tricky, but clean-ish. The sampling code is untested and
probably needs more work.
This was a relatively mechanical change. First, I added an accessor
function for the pathbias-state field of a guard. Then I did a
search-and-replace in circpathbias.c to replace "guard->pb." with
"pb->". Finally, I made sure that "pb" was declared whenever it was
needed.
The entry_guard_t structure should really be opaque, so that we
can change its contents and have the rest of Tor not care.
This commit makes it "mostly opaque" -- circpathbias.c can still see
inside it. (I'm making circpathbias.c exempt since it's the only
part of Tor outside of entrynodes.c that made serious use of
entry_guard_t internals.)
This affects clients with FetchUselessDescriptors 1.
It might also cause subtle bugs on directory mirrors and authorities,
causing them to consider all full descriptors as failed or old.
Improve the messages logged when Tor wants or needs to load the master ed25519 identity key so the user is explicitly informed when further action is required or not. Fixes ticket #20650.
(We only create HS directories if we are acting on the config.)
Log a BUG warning if the directories aren't present immediately before they
are used, then fail.
For relays that don't know their own address, avoid attempting
a local hostname resolve for each descriptor we download. Also cut
down on the number of "Success: chose address 'x.x.x.x'" log lines.
Fixes bugs 20423 and 20610; bugfix on 0.2.8.1-alpha.
Single onion services and Tor2web deliberately create long-term one-hop
circuits to their intro and rend points, respectively.
These log messages are intended to diagnose issue 8387, which relates to
circuits hanging around forever for no reason.
Fixes bug 20613; bugfix on 0.2.9.1-alpha. Reported by "pastly".
It's not okay to use the same varargs list twice, and apparently
some windows build environments produce code here that would leave
tor_asprintf() broken. Fix for bug 20560; bugfix on 0.2.2.11-alpha
when tor_asprintf() was introduced.
This field indicates if the service is a Single Onion Service if present in
the descriptor.
Closes#19642
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because as Teor puts it: "[Resetting on 503] is exactly what we
don't want when relays are busy - imagine clients doing an automatic
reset every time they DoS a relay..."
Fixes bug 20593.
The test was broken and skipped because the hardcoded cross certificate didn't
include the dynamically generated signing key generated by the test. The only
way we could have fixed that is extracting the signing key from the hardcoded
string and put it in the descriptor object or dynamically generate the cross
certificate.
In the end, all this was kind of pointless as we already test the decoding of
multiple introduction points elsewhere and we don't gain anything with that
specific test thus the removal.
Fixes#20570
Signed-off-by: David Goulet <dgoulet@torproject.org>
As of #19899, we decided to allow any relay understanding the onion service
version 3 protocol to be able to use it. The service and client will be the
one controlled by a consensus parameter (different one for both of them) but
if you are a relay and you can understand a protocol, basically you should use
the feature.
Closes#19899
Signed-off-by: David Goulet <dgoulet@torproject.org>
It's only safe to remove the failure limit (per 20536) if we are in
fact waiting a bit longer each time we try to download.
Fixes bug 20534; bugfix on 0.2.9.1-alpha.
If a consensus expires while we are waiting for certificates to download,
stop waiting for certificates.
If we stop waiting for certificates less than a minute after we started
downloading them, do not consider the certificate download failure a
separate failure.
Fixes bug 20533; bugfix on commit e0204f21 in 0.2.0.9-alpha.
Relays do not deliberately launch multiple attempts, so the impact of this
bug should be minimal. This fix also defends against bugs like #20499.
Bugfix on 0.2.8.1-alpha.
(OpenSSL 1.1 makes EVP_CIPHER_CTX opaque, _and_ adds acceleration
for counter mode on more architectures. So it won't work if we try
the older approach, and it might help if we try the newer one.)
Fixes bug 20588.
Note that the "signed key" in the signing key certificate is the
signing key. The "signing key" in the signing key certificate is
the key that signs the certificate -- that is, the blinded key.
This parameter controls if onion services version 3 (first version of prop224)
is enabled or not. If disabled, the tor daemon will not support the protocol
for all components such as relay, directory, service and client. If the
parameter is not found, it's enabled by default.
Closes#19899
Signed-off-by: David Goulet <dgoulet@torproject.org>
Signed-off-by: George Kadianakis <desnacked@riseup.net>
This implements the proposal 224 directory descriptor cache store and lookup
functionalities. Furthermore, it merges the OOM call for the HSDir cache with
current protocol v2 and the new upcoming v3.
Add hs_cache.{c|h} with store/lookup API.
Closes#18572
Signed-off-by: David Goulet <dgoulet@torproject.org>
Signed-off-by: George Kadianakis <desnacked@riseup.net>
Add hs_descriptor.{c|h} with the needed ABI to represent a descriptor and
needed component.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Signed-off-by: George Kadianakis <desnacked@riseup.net>
Also add a trunnel definition for link_specifier_list
Signed-off-by: John Brooks <special@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>
Signed-off-by: George Kadianakis <desnacked@riseup.net>
In order to implement proposal 224, we need the data structure rend_data_t to
be able to accomodate versionning that is the current version of hidden
service (2) and the new version (3) and future version.
For that, we implement a series of accessors and a downcast function to get
the v2 data structure. rend_data_t becomes a top level generic place holder.
The entire rend_data_t API has been moved to hs_common.{c|h} in order to
seperate code that is shared from between HS versions and unshared code (in
rendcommon.c).
Closes#19024
Signed-off-by: David Goulet <dgoulet@torproject.org>
Signed-off-by: George Kadianakis <desnacked@riseup.net>
One is fixed by disabling the -Wredundant-decls warnings around
openssl headers here, because of the old double-declaration of
SSL_get_selected_srtp_profile().
One is fixed by including compat.h before or.h so that we get the
winsock2.h include before the windows.h include.
In our code to write public keys to a string, for some unfathomable
reason since 253f0f160e, we would allocate a memory BIO, then
set the NOCLOSE flag on it, extract its memory buffer, and free it.
Then a little while later we'd free the memory buffer with
BUF_MEM_free().
As of openssl 1.1 this doesn't work any more, since there is now a
BIO_BUF_MEM structure that wraps the BUF_MEM structure. This
BIO_BUF_MEM doesn't get freed in our code.
So, we had a memory leak!
Is this an openssl bug? Maybe. But our code was already pretty
silly. Why mess around with the NOCLOSE flag here when we can just
keep the BIO object around until we don't need the buffer any more?
Fixes bug 20553; bugfix on 0.0.2pre8
This function is allowed to return NULL if the certified key isn't
RSA. But in a couple of places we were treating this as a bug or
internal error, and in one other place we weren't checking for it at
all!
Caught by Isis during code review for #15055. The serious bug was
only on the 15055 branch, thank goodness.
All supported Tors (0.2.4+) require versions of openssl that can
handle this.
Now that our link certificates are RSA2048, this might actually help
vs fingerprinting a little.
This was a stopgap method, designed on the theory that some routers
might support it before they could support Ed25519. But it looks
like everybody who supports RFC5705 will also have an Ed25519 key,
so there's not a lot of reason to have this even supported.
This patch moves the pregenerated RSA key logic into a new
testing_rsakeys.c.
Also, it adds support for RSA2048, since the link handshake tests
want that.
Also, it includes pregenerated keys, rather than trying to actually
generate the keys at startup, since generating even a small handful
of RSA2048 keys makes for an annoying delay.
This code stores the ed certs as appropriate, and tries to check
them. The Ed25519 result is not yet used, and (because of its
behavior) this will break RSA authenticate cells. That will get
fixed as we go, however.
This should implement 19157, but it needs tests, and it needs
to get wired in.
In particular, these functions are the ones that set the identity of
a given connection or channel, and/or confirm that we have learned
said IDs.
There's a lot of stub code here: we don't actually need to use the
new keys till we start looking up connections/channels by Ed25519
IDs. Still, we want to start passing the Ed25519 IDs in now, so it
makes sense to add these stubs as part of 15055.
The impact here isn't too bad. First, the only affected certs that
expire after 32-bit signed time overflows in Y2038. Second, it could
only make it seem that a non-expired cert is expired: it could never
make it seem that an expired cert was still live.
Fixes bug 20027; bugfix on 0.2.7.2-alpha.
Also, adjust signing approach to more closely match the signing
scheme in the proposal.
(The format doesn't quite match the format in the proposal, since
RSA signatures aren't fixed-length.)
Closes 19020.
See proposal 244. This feature lets us stop looking at the internals
of SSL objects, *and* should let us port better to more SSL libraries,
if they have RFC5705 support.
Preparatory for #19156
We no longer generate certs cells by pasting the certs together one
by one. Instead we use trunnel to generate them.
Preliminary work for 19155 (send CERTS cell with ed certs)
passthrough_test_setup doesn't pass through arguments if the argument
is equal to 0 or TT_SKIP. Instead, it fails or skips the test.
Assert on this, so we don't accidentally fail or skip tests.
Fixes bug 19969; bugfix on b1d56fc58. We can fix this some more in
later Tors, but for now, this is probably the simplest fix possible.
This is a belt-and-suspenders fix, where the earlier fix ("Ask
event_base_loop to finish when we add a pending stream") aims to respond
to new streams as soon as they arrive, and this one aims to make sure
that we definitely respond to all of the streams.
ome policies are default-reject, some default-accept. But
policy_is_reject_star() assumed they were all default_reject. Fix
that!
Also, document that policy_is_reject_star() treats a NULL policy as
empty. This allows us to simplify the checks in
parse_reachable_addresses() by quite a bit.
Fxes bug 20306; bugfix on 0.2.8.2-alpha.
(Also, refactor the code to create a hidden service directory into a
separate funcion, so we don't have to duplicate it.)
Fixes bug 20484; bugfix on 0.2.9.3-alpha.
This simplifies the function: if we have an ntor key, use ntor/EXTEND2,
otherwise, use TAP/EXTEND.
Bugfix on commit 10aa913 from 19163 in 0.2.9.3-alpha.
I had replaced a comment implying that a set of ifs was meant to be
exhaustive with an actual check for exhaustiveness. It turns out,
they were exhaustive, but not in the way I had assumed. :(
Bug introduced in f3e158edf7, not in any released Tor.
Use the following coccinelle script to change uses of
smartlist_add(sl, tor_strdup(str)) to
smartlist_add_strdup(sl, string) (coccinelle script from nickm
via bug 20048):
@@
expression a;
expression b;
@@
- smartlist_add
+ smartlist_add_strdup
(a,
- tor_strdup(
b
- )
)
The tor_fragile_assert() bug has existed here since c8a5e2d588
in tor-0.2.1.7-alpha forever, but tor_fragile_assert() was mostly a
no-op until 0.2.9.1-alpha.
Fixes bug 19869.
When we refactored purpose_needs_anonymity(), we made it so _all_
bridge requests required anonymity. But that missed the case
that we are allowed to ask a bridge for its own descriptor.
With this patch, we consider the resource, and allow "authority.z"
("your own descriptor, compressed") for a bridge's server descriptor
to be non-anonymous.
Fix for bug 20410; bug not in any released Tor.
I believe that this should never trigger, but if it does, it
suggests that there was a gap between is_sensitive_dir_purpose and
purpose_needs_anonymity that we need to fill. Related to 20077.
This helps protect against bugs where any part of a buf_t's memory
is passed to a function that expects a NUL-terminated input.
It also closes TROVE-2016-10-001 (aka bug 20384).
This commit adds or improves the module-level documenation for:
buffers.c circuitstats.c command.c connection_edge.c control.c
cpuworker.c crypto_curve25519.c crypto_curve25519.h
crypto_ed25519.c crypto_format.c dircollate.c dirserv.c dns.c
dns_structs.h fp_pair.c geoip.c hibernate.c keypin.c ntmain.c
onion.c onion_fast.c onion_ntor.c onion_tap.c periodic.c
protover.c protover.h reasons.c rephist.c replaycache.c
routerlist.c routerparse.c routerset.c statefile.c status.c
tor_main.c workqueue.c
In particular, I've tried to explain (for each documented module)
what each module does, what's in it, what the big idea is, why it
belongs in Tor, and who calls it. In a few cases, I've added TODO
notes about refactoring opportunities.
I've also renamed an argument, and fixed a few DOCDOC comments.
(I've done this instead of changing the semantics of
router_compare_to_my_exit_policy, because dns.c uses
router_compare_to_my_exit_policy too, in a slightly weird way.)
Some compilers apparently noticed that p2len was allowed to be equal
to msg, and so maybe we would be doing memset(prompt2, ' ', 0), and
decided that we probably meant to do memset(prompt2, 0, 0x20);
instead.
Stupid compilers, doing optimization before this kind of warning!
My fix is to just fill the entire prompt2 buffer with spaces,
because it's harmless.
Bugfix on e59f0d4cb9, not in any released Tor.
(Specifically, carriage return after a quoted value in a config
line. Fixes bug 19167; bugfix on 0.2.0.16-alpha when we introduced
support for quoted values. Unit tests, changes file, and this
parenthetical by nickm.)
This is a kludge to deal with the fact that `tor_addr_t` doesn't contain
`sun_path`. This currently ONLY happens when circuit isolation is being
checked, for an isolation mode that is force disabled anyway, so the
kludge is "ugly but adequate", but realistically, making `tor_addr_t`
and the AF_UNIX SocksPort code do the right thing is probably the better
option.
Closes ticket 20303.
The LIBRESSL_VERSION_NUMBER check is needed because if our openssl
is really libressl, it will have an openssl version number we can't
really believe.
Previously, we would reject even rendezvous connections to IPv6
addresses when IPv6Exit was false. But that doesn't make sense; we
don't count that as "exit"ing. I've corrected the logic and tried
to make it a lottle more clear.
Fixes bug 18357; this code has been wrong since 9016d9e829 in
0.2.4.7-alpha.
When deleting unsuitable addresses in get_interface_address6_list(), to
avoid reordering IPv6 interface addresses and keep the order returned by
the OS, use SMARTLIST_DEL_CURRENT_KEEPORDER() instead of
SMARTLIST_DEL_CURRENT().
This issue was reported by René Mayrhofer.
[Closes ticket 20163; changes file written by teor. This paragraph
added by nickm]
We removed that feature in 0.2.4.2-alpha, but some comments seem to
have lingered.
I didn't add a changes/ file since this is just internal code cleanup.
(like clients do) rather than old-style router descriptors. Now bridges
will blend in with clients in terms of the circuits they build.
Fixes bug 6769; bugfix on 0.2.3.2-alpha.
Clients that use bridges were ignoring their cached microdesc-flavor
consensus files, because they only thought they should use the microdesc
flavor once they had a known-working bridge that could offer microdescs,
and at first boot no bridges are known-working.
This bug caused bridge-using clients to download a new microdesc consensus
on each startup.
Fixes bug 20269; bugfix on 0.2.3.12-alpha.
The client addr is essentially meaningless in this context (yes, it is
possible to explicitly `bind()` AF_LOCAL client side sockets to a path,
but no one does it, and there are better ways to grant that sort of
feature if people want it like using `SO_PASSCRED`).
As before, we check server protocols whenever server_mode(options)
is true and we check client protocols whenever server_mode(options)
is false.
Additionally, we now _also_ check client protocols whenever any
client port is set.
(Technically, we could just remove extend2 cell checking entirely,
since all Tor versions on our network are required to have it, but
let's keep this around as an example of How To Do It.)
(Despite the increased size of the consensus, this should have
approximately zero effect on the compressed consensus size, since
the "proto" line should be completely implied by the "v" line.)
[This is a brute-force method that potentially uses way too much
RAM. Need to rethink this a little. Right now you can DOS an
authority by saying "Foo=1-4294967295".]
Sierra provides clock_gettime(), but not pthread_condattr_setclock.
So we had better lot try to use CLOCK_MONOTONIC as our source for
time when waiting, since we ccan never actually tell the condition
that we mean CLOCK_MONOTONIC.
This isn't a tor bug yet, since we never actually pass a timeout to
tor_cond_wait() outside of the unit tests.
Not telling the cmux would sometimes cause an assertion failure in
relay.c when we tried to get an active circuit and found an "active"
circuit with no cells.
Additionally, replace that assert with a test and a log message.
Fix for bug 20203. This is actually probably a bugfix on
0.2.8.1-alpha, specifically my code in 8b4e5b7ee9 where I
made circuit_mark_for_close_() do less in order to simplify our call
graph. Thanks to "cypherpunks" for help diagnosing.
Our use of the (mockable) tor_close_socket() in the util/socket_..
tests confused coverity, which could no longer tell that we were
actually closing the sockets.
It's a macro that calls down to a function whose behavior has been
getting progresively more complicated.... but we named it as if it
were a variable. That's not so smart. So, replace it with a
function call to a function that was just doing "return
current_consensus".
Fixes bug 20176.
Commit 41cc1f612b introduced a "dns_request"
configuration value which wasn't set to 1 for an entry connection on the
DNSPort leading to a refusal to resolve the given hostname.
This commit set the dns_request flag by default for every entry connection
made to the DNSPort.
Fixes#20109
Signed-off-by: David Goulet <dgoulet@torproject.org>
For a brief moment in networkstatus_set_current_consensus(), the old
consensus has been freed, but the node_t objects still have dead
pointers to the routerstatus_t objects within it. During that
interval, we absolutely must not do anything that would cause Tor to
look at those dangling pointers.
Unfortunately, calling the (badly labeled!) current_consensus macro
or anything else that calls into we_use_microdescriptors_for_circuits(),
can make us look at the nodelist.
The fix is to make sure we identify the main consensus flavor
_outside_ the danger zone, and to make the danger zone much much
smaller.
Fixes bug 20103. This bug has been implicitly present for AGES; we
just got lucky for a very long time. It became a crash bug in
0.2.8.2-alpha when we merged 35bbf2e4a4 to make
find_dl_schedule start looking at the consensus, and 4460feaf28
which made node_get_all_orports less (accidentally) tolerant of
nodes with a valid ri pointer but dangling rs pointer.
Previously, the IV and key were stored in the structure, even though
they mostly weren't needed. The only purpose they had was to
support a seldom-used API where you could pass NULL when creating
a cipher in order to get a random key/IV, and then pull that key/IV
back out.
This saves 32 bytes per AES instance, and makes it easier to support
different key lengths.
* Check consistency between the two single onion torrc options
* Use the more relevant option each time we check for single onion mode
* Clarify log messages
* Clarify comments
* Otherwise, no behaviour change
Tor checks that the flag matches the configured onion service anonymity.
Tor refuses to create unflagged onion service using ADD_ONION, if they
would be non-anonymous. The error is:
512 Tor is in non-anonymous onion mode
Similarly, if the NonAnonymous flag is present, and Tor has the default
anonymous onion config:
512 Tor is in anonymous onion mode
Parse the value to UseEntryNodes_option, then set UseEntryNodes before
validating options.
This way, Authorities, Tor2web, and Single Onion Services don't write
spurious "UseEntryNodes 0" lines to their configs. Document the fact that
these tor configurations ignore UseEntryNodes in the manual page.
Also reorder options validation so we modify UseEntryNodes first, then
check its value against EntryNodes.
And silence a warning about disabled UseEntryNodes for hidden services
when we're actually in non-anonymous single onion service mode.
We log this message every time we validate tor's options.
There's no need to log a duplicate in main() as well.
(It is impossible to run main() without validating our options.)
The changes in #19973 fixed ReachableAddresses being applied
too broadly, but they also broke Tor2web (somewhat unintentional)
compatibility with ReachableAddresses.
This patch restores that functionality, which makes intro and
rend point selection is consistent between Tor2web and Single Onion
Services.
Add experimental OnionServiceSingleHopMode and
OnionServiceNonAnonymousMode options. When both are set to 1, every
hidden service on a tor instance becomes a non-anonymous Single Onion
Service. Single Onions make one-hop (direct) connections to their
introduction and renzedvous points. One-hop circuits make Single Onion
servers easily locatable, but clients remain location-anonymous.
This is compatible with the existing hidden service implementation, and
works on the current tor network without any changes to older relays or
clients.
Implements proposal #260, completes ticket #17178. Patch by teor & asn.
squash! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Implement Prop #260: Single Onion Services
Redesign single onion service poisoning.
When in OnionServiceSingleHopMode, each hidden service key is poisoned
(marked as non-anonymous) on creation by creating a poison file in the
hidden service directory.
Existing keys are considered non-anonymous if this file exists, and
anonymous if it does not.
Tor refuses to launch in OnionServiceSingleHopMode if any existing keys
are anonymous. Similarly, it refuses to launch in anonymous client mode
if any existing keys are non-anonymous.
Rewrite the unit tests to match and be more comprehensive.
Adds a bonus unit test for rend_service_load_all_keys().
(We check consensus method when deciding whether to assume a node is
valid. No need to check the consensus method for Running, since
we will never see a method before 13.)
Closes ticket 20001
g
The other test vectors are pretty complete, and get full coverage, I
believe.
This one test vector accounted for half the time spent in
test-slow. "Now that's slow!"
We have a mock for our RSA key generation function, so we now wire
it to pk_generate(). This covers all the cases that were not using
pk_generate() before -- all ~93 of them.
Previously, you needed to store the previous log severity in a local
variable, and it wasn't clear if you were allowed to call these
functions more than once.
The functions it warns about are:
assert, memcmp, strcat, strcpy, sprintf, malloc, free, realloc,
strdup, strndup, calloc.
Also, fix a few lingering instances of these in the code. Use other
conventions to indicate _intended_ use of assert and
malloc/realloc/etc.
We should consider them bugs. If they are happening intentionally,
we should use the log_test_helpers code to capture and suppress
them. But having them off-by-default has potential to cause
programming errors.
Previously setup_capture_of_logs would prevent log messages from
going to the console entirely. That's a problem, since sometimes
log messages are bugs! Now setup_capture_of_logs() acts sensibly.
If you really do need to keep a message from going to the console
entirely, there is setup_full_capture_of_logs(). But only use that
if you're prepared to make sure that there are no extraneous
messages generated at all.
Users can't run an anonymous client and non-anonymous single
onion service at the same time. We need to know whether we have
any client ports or sockets open to do this check.
When determining whether a client port (SOCKS, Trans, NATD, DNS)
is set, count unix sockets when counting client listeners. This
has no user-visible behaviour change, because these options are
set once and never read in the current tor codebase.
Don't count sockets when setting ControlPort_set, that's what
ControlSocket is for. (This will be reviewed in #19665.)
Don't count sockets when counting server listeners, because the code
that uses these options expects to count externally-visible ports.
(And it would change the behaviour of Tor.)
Copying the integer 42 in a char buffer has a different representation
depending on the endianess of the system thus that unit test was failing on
big endian system.
This commit introduces a python script, like the one we have for SRV, that
computes a COMMIT/REVEAL from scratch so we can use it as a test vector for
our encoding unit tests.
With this, we use a random value of bytes instead of a number fixing the
endianess issue and making the whole test case more solid with an external
tool that builds the COMMIT and REVEAL according to the spec.
Fixes#19977
Signed-off-by: David Goulet <dgoulet@torproject.org>
Check NoOnionTraffic before attaching a stream.
NoOnionTraffic refuses connections to all onion hostnames,
but permits non-onion hostnames and IP addresses.
Check NoDNSRequest, NoIPv4Traffic, and NoIPv6Traffic before
attaching a stream.
NoDNSRequest refuses connections to all non-onion hostnames,
but permits IP addresses.
NoIPv4Traffic refuses connections to IPv4 addresses, but resolves
hostnames.
NoIPv6Traffic refuses connections to IPv6 addresses, but resolves
hostnames.
Combined, they refuse all non-onion hostnames and IP addresses.
OnionTrafficOnly is equivalent to NoDNSRequest, NoIPv4Traffic,
and NoIPv6Traffic.
Add unit tests for parsing and checking option validity.
Add documentation for each flag to the man page.
Add changes file for all of #18693.
Parsing only: the flags do not change client behaviour (yet!)
Rely on onion_populate_cpath to check that we're only using
TAP for the rare hidden service cases.
Check and log if handshakes only support TAP when they should support
ntor.
When a client connects to an intro point not in the client's consensus,
or a hidden service connects to a rend point not in the hidden service's
consensus, we are stuck with using TAP, because there is no ntor link
specifier.
This bug had existed since 0.2.4.7-alpha, but now that we have
FallbackDirs by default, it actually matters.
Fixes bug 19947; bugfix on 0.2.4.7-alpha or maybe 0.2.8.1-alpha.
Rubiate wrote the patch; teor wrote the changes file.
Longer and more explicit log message so we don't confuse users with behind NAT with working configurations and state that public IP addresses only should be provided with "Address", won't work with internal addresses.
OpenBSD removes this function, and now that Tor requires Libevent 2,
we should also support the OpenBSD Libevent 2.
Fixes bug 19904; bugfix on 0.2.5.4-alpha.
These functions were there so that we could abstract the differences
between evbuffer and buf_t. But with the bufferevent removal, this
no longer serves a purpose.
