We want to support parsing a cell with unknown status code so we are forward
compatible.
Part of #30454
Signed-off-by: David Goulet <dgoulet@torproject.org>
Like the previous commit about the INTRODUCE_ACK status code, change all auth
key type to use the one defined in the trunnel file.
Standardize the use of these auth type to a common ABI.
Part of #30454
Signed-off-by: David Goulet <dgoulet@torproject.org>
This enum was the exact same as hs_intro_ack_status_t that was removed at the
previous commit. It was used client side when parsing the INTRODUCE_ACK cell.
Now, the entire code dealing with the INTRODUCE_ACK cell (both sending and
receiving) have been modified to all use the same ABI defined in the trunnel
introduce1 file.
Finally, the client will default to the normal behavior when receiving an
unknown NACK status code which is to note down that we've failed and re-extend
to the next intro point. This way, unknown status code won't trigger a
different behavior client side.
Part of #30454.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Remove the hs_intro_ack_status_t enum and move the value into trunnel. Only
use these values from now on in the intro point code.
Interestingly enough, the client side also re-define these values in hs_cell.h
with the hs_cell_introd_ack_status_t enum. Next commit will fix that and force
to use the trunnel ABI.
Part of #30454
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously we purged it in 1-hour increments -- but one-hour is the
maximum TTL for the cache! Now we do it in 25%-TTL increments.
Fixes bug 29617; bugfix on 0.3.5.1-alpha.
Fortunately, in 0.3.5.1-alpha we improved logging for various
failure cases involved with onion service client auth.
Unfortunately, for this one, we freed the file right before logging
its name.
Fortunately, tor_free() sets its pointer to NULL, so we didn't have
a use-after-free bug.
Unfortunately, passing NULL to %s is not defined.
Fortunately, GCC 9.1.1 caught the issue!
Unfortunately, nobody has actually tried building Tor with GCC 9.1.1
before. Or if they had, they didn't report the warning.
Fixes bug 30475; bugfix on 0.3.5.1-alpha.
The INTRODUCE1 trunnel definition file doesn't support that value so it can
not be used else it leads to an assert on the intro point side if ever tried.
Fortunately, it was impossible to reach that code path.
Part of #30454
Signed-off-by: David Goulet <dgoulet@torproject.org>
In 0.3.4 and later, these functions are declared in rephist.h:
STATIC uint64_t find_largest_max(bw_array_t *b);
STATIC void commit_max(bw_array_t *b);
STATIC void advance_obs(bw_array_t *b);
But in 0.2.9, they are declared in rephist.c and test_relay.c.
So compilers fail with a "must use 'struct' tag" error.
We add the missing struct typedef in test_relay.c, to match the
declarations in rephist.c.
(Merge commit 813019cc57 moves these functions into rephist.h instead.)
Fixes bug 30184; not in any released version of Tor.
When releasing OpenSSL patch-level maintenance updates,
we do not want to rebuild binaries using it.
And since they guarantee ABI stability, we do not have to.
Without this patch, warning messages were produced
that confused users:
https://bugzilla.opensuse.org/show_bug.cgi?id=1129411
Fixes bug 30190; bugfix on 0.2.4.2-alpha commit 7607ad2bec
Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.de>
In 0.3.4 and later, we declare write_array as:
extern struct bw_array_t *write_array;
...
typedef struct bw_array_t bw_array_t;
But in 0.2.9, we declare write_array as:
typedef struct bw_array_t bw_array_t;
extern bw_array_t *write_array;
And then again in rephist.c:
typedef struct bw_array_t bw_array_t;
So some compilers fail with a duplicate declaration error.
We backport 684b396ce5, which removes the duplicate declaration.
And this commit deals with the undeclared type error.
Backports a single line from merge commit 813019cc57.
Fixes bug 30184; not in any released version of Tor.
Previously, our use of abort() would break anywhere that we didn't
include stdlib.h. This was especially troublesome in case where
tor_assert_nonfatal() was used with ALL_BUGS_ARE_FATAL, since that
one seldom gets tested.
As an alternative, we could have just made this header include
stdlib.h. But that seems bloaty.
Fixes bug 30189; bugfix on 0.3.4.1-alpha.
The function compat_getdelim_ is used for tor_getline if tor is compiled
on a system that lacks getline and getdelim. These systems should be
very rare, considering that getdelim is POSIX.
If this system is further a 32 bit architecture, it is possible to
trigger a double free with huge files.
If bufsiz has been already increased to 2 GB, the next chunk would
be 4 GB in size, which wraps around to 0 due to 32 bit limitations.
A realloc(*buf, 0) could be imagined as "free(*buf); return malloc(0);"
which therefore could return NULL. The code in question considers
that an error, but will keep the value of *buf pointing to already
freed memory.
The caller of tor_getline() would free the pointer again, therefore
leading to a double free.
This code can only be triggered in dirserv_read_measured_bandwidths
with a huge measured bandwith list file on a system that actually
allows to reach 2 GB of space through realloc.
It is not possible to trigger this on Linux with glibc or other major
*BSD systems even on unit tests, because these systems cannot reach
so much memory due to memory fragmentation.
This patch is effectively based on the penetration test report of
cure53 for curl available at https://cure53.de/pentest-report_curl.pdf
and explained under section "CRL-01-007 Double-free in aprintf() via
unsafe size_t multiplication (Medium)".
If the concatenation of connection buffer and the buffer of linked
connection exceeds INT_MAX bytes, then buf_move_to_buf returns -1 as an
error value.
This value is currently casted to size_t (variable n_read) and will
erroneously lead to an increasement of variable "max_to_read".
This in turn can be used to call connection_buf_read_from_socket to
store more data inside the buffer than expected and clogging the
connection buffer.
If the linked connection buffer was able to overflow INT_MAX, the call
of buf_move_to_buf would have previously internally triggered an integer
overflow, corrupting the state of the connection buffer.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Many buffer functions have a hard limit of INT_MAX for datalen, but
this limitation is not enforced in all functions:
- buf_move_all may exceed that limit with too many chunks
- buf_move_to_buf exceeds that limit with invalid buf_flushlen argument
- buf_new_with_data may exceed that limit (unit tests only)
This patch adds some annotations in some buf_pos_t functions to
guarantee that no out of boundary access could occur even if another
function lacks safe guards against datalen overflows.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
If the concatenation of connection buffer and the buffer of linked
connection exceeds INT_MAX bytes, then buf_move_to_buf returns -1 as an
error value.
This value is currently casted to size_t (variable n_read) and will
erroneously lead to an increasement of variable "max_to_read".
This in turn can be used to call connection_buf_read_from_socket to
store more data inside the buffer than expected and clogging the
connection buffer.
If the linked connection buffer was able to overflow INT_MAX, the call
of buf_move_to_buf would have previously internally triggered an integer
overflow, corrupting the state of the connection buffer.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Many buffer functions have a hard limit of INT_MAX for datalen, but
this limitation is not enforced in all functions:
- buf_move_all may exceed that limit with too many chunks
- buf_move_to_buf exceeds that limit with invalid buf_flushlen argument
- buf_new_with_data may exceed that limit (unit tests only)
This patch adds some annotations in some buf_pos_t functions to
guarantee that no out of boundary access could occur even if another
function lacks safe guards against datalen overflows.
[This is a backport of the submitted patch to 0.2.9, where the
buf_move_to_buf and buf_new_with_data functions did not exist.]
Fixes bug 29922; bugfix on 0.2.9.3-alpha when we tried to capture
all these warnings. No need to backport any farther than 0.3.5,
though -- these warnings don't cause test failures before then.
This one was tricky to find because apparently it only happened on
_some_ windows builds.
In current NSS versions, these ciphersuites don't work with
SSL_ExportKeyingMaterial(), which was causing relays to fail when
they tried to negotiate the v3 link protocol authentication.
Fixes bug 29241; bugfix on 0.4.0.1-alpha.
When classifying a client's selection of TLS ciphers, if the client
ciphers are not yet available, do not cache the result. Previously,
we had cached the unavailability of the cipher list and never looked
again, which in turn led us to assume that the client only supported
the ancient V1 link protocol. This, in turn, was causing Stem
integration tests to stall in some cases. Fixes bug 30021; bugfix
on 0.2.4.8-alpha.
David Goulet
Nick Mathewson
Karsten Loesing
teor
Bernhard M. Wiedemann
Tobias Stoeckmann
rl1987