Nick Mathewson [Wed, 14 Oct 2020 15:28:37 +0000 (11:28 -0400)]
Implement proposal 318: Limit protovers to 0..63
In brief: we go through a lot of gymnastics to handle huge protover
numbers, but after years of development we're not even close to 10
for any of our current versions. We also have a convenient
workaround available in case we ever run out of protocols: if (for
example) we someday need Link=64, we can just add Link2=0 or
something.
This patch is a minimal patch to change tor's behavior; it doesn't
take advantage of the new restrictions.
Nick Mathewson [Wed, 12 Aug 2020 16:36:54 +0000 (12:36 -0400)]
Parallelize src/test/test into chunks.
First, we introduce a flag to teach src/test/test to split its work
into chunks. Then we replace our invocation of src/test/test in our
"make check" target with a set of 8 scripts that invoke the first
8th of the tests, the second 8th, and so on.
This change makes our "make -kj4 check" target in our hardened
gitlab build more than twice as fast, since src/test/test was taking
the longest to finish.
David Goulet [Thu, 17 Sep 2020 16:55:40 +0000 (12:55 -0400)]
test: Increment rend cache allocation before freeing
The rend_cache/entry_free was missing the rend cache allocation increment
before freeing the object.
Without it, it had an underflow bug:
Sep 17 08:40:13.845 [warn] rend_cache_decrement_allocation(): Bug: Underflow
in rend_cache_decrement_allocation (on Tor 0.4.5.0-alpha-dev 7eef9ced61e72b1d)
Fixes #40125
Signed-off-by: David Goulet <dgoulet@torproject.org>
Nick Mathewson [Mon, 3 Aug 2020 15:25:37 +0000 (11:25 -0400)]
Remove channel_is_canonical_is_reliable()
This function once served to let circuits continue to be built over
version-1 link connections. But such connections are long-obsolete,
and it's time to remove this check.
Nick Mathewson [Thu, 30 Jul 2020 18:24:25 +0000 (14:24 -0400)]
Fix a bug in buf_move_all() when the input buffer is empty.
We found this in #40076, after we started using buf_move_all() in
more places. Fixes bug #40076; bugfix on 0.3.3.1-alpha. As far as
I know, the crash only affects master, but I think this warrants a
backport, "just in case".
Nick Mathewson [Wed, 22 Jul 2020 19:20:41 +0000 (15:20 -0400)]
Treat all extorport connections with un-set addresses as remote
Without this fix, if an PT forgets to send a USERADDR command, that
results in a connection getting treated as local for the purposes of
rate-limiting.
If the PT _does_ use USERADDR, we still believe it.
Nick Mathewson [Wed, 22 Jul 2020 18:45:03 +0000 (14:45 -0400)]
Adjust the rules for warning about too many connections.
Previously we tolerated up to 1.5 connections for every relay we
were connected to, and didn't warn if we had fewer than 5
connections total.
Now we tolerate up to 1.5 connections per relay, and up to 4
connections per authority, and we don't warn at all when we have
fewer than 25 connections total.
Fixes bug 33880, which seems to have been provoked by our #17592
change in 0.3.5.
David Goulet [Tue, 7 Jul 2020 13:20:28 +0000 (09:20 -0400)]
CI: Fix Appveyor printf format error
For some reasons, Appveyor started to use the stdio printf format for 64 bit
values (PRIu64, ...). Mingw doesn't like that so force it to use the Windows
specific macros by setting D__USE_MINGW_ANSI_STDIO=0.
Alexander Færøy [Sat, 16 May 2020 19:18:56 +0000 (19:18 +0000)]
Use ((x + 7) >> 3) instead of (x >> 3) when converting from bits to bytes.
This patch changes our bits-to-bytes conversion logic in the NSS
implementation of `tor_tls_cert_matches_key()` from using (x >> 3) to
((x + 7) >> 3) since DER bit-strings are allowed to contain a number of
bits that is not a multiple of 8.
Additionally, we add a comment on why we cannot use the
`DER_ConvertBitString()` macro from NSS, as we would potentially apply
the bits-to-bytes conversion logic twice, which would lead to an
insignificant amount of bytes being compared in
`SECITEM_ItemsAreEqual()` and thus turn the logic into being a
prefix match instead of a full match.
The `DER_ConvertBitString()` macro is defined in NSS as:
/*
** Macro to convert der decoded bit string into a decoded octet
** string. All it needs to do is fiddle with the length code.
*/
#define DER_ConvertBitString(item) \
{ \
(item)->len = ((item)->len + 7) >> 3; \
}
Alexander Færøy [Sat, 16 May 2020 15:34:37 +0000 (15:34 +0000)]
Add constness to length variables in `tor_tls_cert_matches_key`.
We add constness to `peer_info_orig_len` and `cert_info_orig_len` in
`tor_tls_cert_matches_key` to ensure that we don't accidentally alter
the variables.
Alexander Færøy [Tue, 31 Mar 2020 02:33:54 +0000 (02:33 +0000)]
Fix out-of-bound memory read in `tor_tls_cert_matches_key()` for NSS.
This patch fixes an out-of-bound memory read in
`tor_tls_cert_matches_key()` when Tor is compiled to use Mozilla's NSS
instead of OpenSSL.
The NSS library stores some length fields in bits instead of bytes, but
the comparison function found in `SECITEM_ItemsAreEqual()` needs the
length to be encoded in bytes. This means that for a 140-byte,
DER-encoded, SubjectPublicKeyInfo struct (with a 1024-bit RSA public key
in it), we would ask `SECITEM_ItemsAreEqual()` to compare the first 1120
bytes instead of 140 (140bytes * 8bits = 1120bits).
This patch fixes the issue by converting from bits to bytes before
calling `SECITEM_ItemsAreEqual()` and convert the `len`-fields back to
bits before we leave the function.
Alexander Færøy [Tue, 31 Mar 2020 02:28:12 +0000 (02:28 +0000)]
Run `tor_tls_cert_matches_key()` Test Suite with both OpenSSL and NSS.
This patch lifts the `tor_tls_cert_matches_key()` tests out of the
OpenSSL specific TLS test suite and moves it into the generic TLS test
suite that is executed for both OpenSSL and NSS.
This patch is largely a code movement, but we had to rewrite parts of
the test to avoid using OpenSSL specific data-types (such as `X509 *`)
and replace it with the generic Tor abstraction type
(`tor_x509_cert_impl_t *`).
Nick Mathewson [Tue, 30 Jun 2020 15:54:13 +0000 (11:54 -0400)]
Downgrade "Bug: No entry found in extrainfo map" message.
This is not actually a bug! It can happen for a bunch of reasons,
which all boil down to "trying to add an extrainfo for which we no
longer have the corresponding routerinfo".
Roger Dingledine [Sat, 30 May 2020 05:54:22 +0000 (01:54 -0400)]
Preemptive circs should work with UseEntryGuards 0
Resume being willing to use preemptively-built circuits when
UseEntryGuards is set to 0. We accidentally disabled this feature with
that config setting (in our fix for #24469), leading to slower load times.
Nick Mathewson [Wed, 6 May 2020 14:45:48 +0000 (10:45 -0400)]
Use __attribute__((fallthrough)) rather than magic GCC comments.
GCC added an implicit-fallthrough warning a while back, where it
would complain if you had a nontrivial "case:" block that didn't end
with break, return, or something like that. Clang recently added
the same thing.
GCC, however, would let you annotate a fall-through as intended by
any of various magic "/* fall through */" comments. Clang, however,
only seems to like "__attribute__((fallthrough))". Fortunately, GCC
accepts that too.
A previous commit in this branch defined a FALLTHROUGH macro to do
the right thing if GNUC is defined; here we replace all of our "fall
through" comments with uses of that macro.
This is an automated commit, made with the following perl one-liner:
Nick Mathewson [Wed, 6 May 2020 14:24:21 +0000 (10:24 -0400)]
Add a fallthrough macro.
This macro defers to __attribute__((fallthrough)) on GCC (and
clang). Previously we had been using GCC's magic /* fallthrough */
comments, but clang very sensibly doesn't accept those.
Since not all compiler recognize it, we only define it when our
configure script detects that it works.
teor [Thu, 19 Mar 2020 07:35:49 +0000 (17:35 +1000)]
Appveyor: Copy required DLLs to test and app
Copy required DLLs to test and app, before running tor's tests.
This ensures that tor.exe and test*.exe use the correct version of each
DLL. This fix is not required, but we hope it will avoid DLL search
issues in future.
Nick Mathewson [Thu, 19 Mar 2020 19:25:11 +0000 (15:25 -0400)]
Add a TOR_SKIP_TESTCASES environment variable for suppressing tests.
For example, "TOR_SKIP_TESTCASES=crypto/.. ./src/test/test" will run
the tests and suppress all the "crypto/" tests. You could get the
same effect by running "./src/test/test :crypto/..", but that can be
harder to arrange from CI.
Given that ed25519 public key validity checks are usually not needed
and (so far) they are only necessary for onion addesses in the Tor
protocol, we decided to fix this specific bug instance without
modifying the rest of the codebase (see below for other fix
approaches).
In our minimal fix we check that the pubkey in
hs_service_add_ephemeral() is valid and error out otherwise.