In the spirit of reducing technical debt. Move code that marks a channel as a
client into its own function and document it properly.
No behavior change, only code movement.
Related to #40022
Signed-off-by: David Goulet <dgoulet@torproject.org>
In order to process a NETINFO cell, the OR connection needs to go through a
series of validation else we don't process the cell.
Move those into its own function in and improve documentation.
This is an attempt at reducing technical debt of the rather large and
complicated channel_tls_process_netinfo_cell() function.
Related to #40022
Signed-off-by: David Goulet <dgoulet@torproject.org>
Now that we have src/ext/ext.md (since b0a716dfb0), we
don't want to have src/ext excluded in its entirety.
Like a smart person, when I added src/ext/ext.md, I edited Doxyfile,
forgetting that it is generated from Doxyfile.in. :/
This should fix travis builds.
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.
Fixes#40026
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; \
}
Thanks to Taylor Yu for spotting this problem.
This patch is part of the fix for TROVE-2020-001.
See: https://bugs.torproject.org/33119
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.
This patch is part of the fix for TROVE-2020-001.
See: https://bugs.torproject.org/33119
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.
This patch is part of the fix for TROVE-2020-001.
See: https://bugs.torproject.org/33119
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 *`).
This patch is part of the fix for TROVE-2020-001.
See: https://bugs.torproject.org/33119
If at least one service is configured as a version 2, a log warning is emitted
once and only once.
Closes#40003
Signed-off-by: David Goulet <dgoulet@torproject.org>
We introduce TOR_EXTRA_PRE_COMMIT_CHECKS environment variable to run the
pre-commit hook. The pre-push git hook will set it in order to run all
pre-commit checks.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We don't need to log that we're about to look for a channel for a
given extend_info_t, since we're either going to log that we're
launching one (at info), or that we're using an existing one (at
debug).
In practice, there will be at most one ipv4 address and ipv6 address
for now, but this code is designed to not care which address is
which until forced to do so.
This patch does not yet actually create extend_info_t objects with
multiple addresses.
Closes#34069.
The find_my_address() function now prioritize the local interface over the
local hostname when guessing the IP address.
See proposal 312, section 3.2.1, general case:
https://gitweb.torproject.org/torspec.git/tree/proposals/312-relay-auto-ipv6-addr.txt#n359
The entire unit tests had to be refactored to make this possible. Instead of
hot patching it, it has been rewritten to cover all possible cases and the
test interface has been changed to accomodate both IPv4 and IPv6 in order for
them to be tested identically.
Closes#33238
Signed-off-by: David Goulet <dgoulet@torproject.org>
Now instead of saying "DONE, DONE" or "MISC, MISC" or "TLS_ERROR,
TLS_ERROR", we can finally give a nice sensible "TLS_ERROR,
wrong version number" which should help debug a great deal.
Closes ticket 32622.
