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.
