This is in response to a question about why we don't always log
orport self-tests as reachability tests.
I'm not 100% convinced that bandwidth self-tests are still useful,
but that's an issue for another day. :)
This function is about learning if a given address is local to us as in the
resolved address as a relay.
Closes#40009
Signed-off-by: David Goulet <dgoulet@torproject.org>
New name reflects that the function is only used to compare router addresses
in order to learn if they are in the same network.
The network check is /16 and /32 respectively for IPv4 and IPv6.
Related to #40009
Signed-off-by: David Goulet <dgoulet@torproject.org>
If at least one Address line is given but invalid, we should not attempt to
guess our address.
This commit sends back the "bail" signal so find_my_address() can return an
error if the requested family doesn't exists but still an Address line is
found which is likely another family.
Fixed in #33235
Related to #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
When going over all Address lines from the configuration, continue to attempt
resolving other lines if one fails.
Before that, we would bail right away and never noticed the other Address
lines.
Fixed in #33235
Related to #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
In get_address_from_config(), we would attempt to resolve an Address line that
is not from the requested family but that line could be a valid address from
another family (v4 vs v6).
This makes it that we don't attempt to resolve a valid address from another
family.
Found with unit test config/find_my_address_mixed.
Fixed in #33235
Related to #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
This unit tests validates the use of IPv4 _and_ IPv6 together as in multiple
option Address lines both addresses and hostnames.
Closes#33235
Signed-off-by: David Goulet <dgoulet@torproject.org>
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".
Fixes#16016; bugfix on 0.2.6.3-alpha.
Previously we had two chains of logic for reachability tests: one
for launching them, and one for telling the user that we had
launched them. Now, we simply have the launch code inform the user:
this way, we can't get out of sync.
Closes ticket 34137.
If our TLS connection fails for a "misc" reason, we don't need to
say that the reason is "misc" -- we can at least localize it to
the TLS module.
Part of a fix for #32622.
Previously, we would only update this field when the error happened
during a read.
This will improves our reporting for our bootstrap status, and help
to address #32622. The problem is not completely solved by this
patch, however: too many errors are still lumped into "MISC".
AssumeReachable should only be about whether a relay thinks that it
is reachable itself. But previously, we've had it also turn off
reachability checking of _other_ relays on authorities.
(Technically, reachability tests were still run, but we would ignore
the results.)
With this patch, there is a new AuthDirTestReachability option
(default 1) that controls whether authorities run reachability
tests.
Making this change allows us to have test networks where authorities
set AssumeReachable without disabling their reachability testing
entirely.
Closes ticket #34445.
These parameters do not suppress checks, but they tell relays that
it's okay to publish even when those checks fail.
I have chosen lowercase hyphenated names, since these seem to be
more common in networkstatus params.
Closes#33224 and part of #34064.
This was supposed to happen in #40012, but the command line was wrong.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
router_should_skip_orport_reachability_check router_all_orports_seem_reachable
Instead of a complex if/else block, use a table of functions that have the
same interface and each of them attempt to find the address one after the
other.
Pointed out by nickm's during review.
Signed-off-by: David Goulet <dgoulet@torproject.org>
By doing this, a memory leak was found with "hostname_used" that could have
been overwritten by another function.
This commit changes that by making it a NULL string instead.
Found by nickm's review.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The last resolved address cache uses an index that is mapped to an address
family (AF_INET and AF_INET6).
This commit adds a conversion function from af to index and change the code to
use that all the time only.
In the process, this commit fixes a bug that the last resolved address
accessors were using the af value insted of the index.
Spotted by nickm during review
Signed-off-by: David Goulet <dgoulet@torproject.org>
Replace it by find_my_address() everywhere. This changes many parts of the
code that uses it to use a tor_addr_t instead of a plain uint32_t for IPv4.
Many changes to the unit test to also use the new interface.
Part #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
Series of changes:
1. Rename function to reflect the namespace of the file.
2. Use the new last resolved cache instead of the unused
last_resolved_addr_v4 (which is also removed in this commit).
3. Make the entire code base use the new resolved_addr_is_local() function.
You will notice that this function uses /24 to differentiate subnets where the
rest of tor uses /16 (including documentation of EnforceDistinctSubnets).
Ticket #40009 has been opened for that.
But that the moment, the function keeps looking at /24.
Part of #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
Series of things done in this commit:
1. Rename the functions to better reflect the namespace of the file.
2. Make both reset and get function to operate on the last_resolved_addrs
cache that is per family.
3. Make the get function to take a tor_addr_t.
4. Change all callsite to use the new convention.
Part of #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
In order to transition smoothly, maek resolve_my_address_v4() call the new
fancy find_my_address() with AF_INET.
Next commits should remove the use of resolve_my_address_v4() accross the code
to use find_my_address().
This commit is so the unit tests would be more easily fixed and port to the
new find_my_address() internals.
Part of #33233.
Signed-off-by: David Goulet <dgoulet@torproject.org>
resolve_my_address() was beyond repair in terms of refactoring. Way too
complex and doing too many things.
This commit implements find_my_address() which in theory does the same as
resolve_my_address() but in a more clean, concise and modern way using the
tor_addr_t interface and for multiple address family.
The caller needs to pass the address family (IPv4 or IPv6) which this
interface supports. For both, a last resolved cache is used as well.
Implements #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
These identifiers are confusing and unreadable. I think these
replacements should be better. Closes ticket #40012.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
router_should_skip_orport_reachability_check_family router_orport_seems_reachable \
router_should_skip_dirport_reachability_check router_dirport_seems_reachable \
router_should_skip_dirport_reachability_check router_all_orports_seem_reachable
The buf_read_from_tls() function was designed to read up to a certain number
of bytes a TLS socket using read_to_chunk_tls() which boils down to SSL_read()
(with OpenSSL, common case).
However, at the end of the loop, the returned number of bytes from
read_to_chunk_tls() was treated like the syscall read() for which if less
bytes than the total asked are returned, it signals EOF.
But, with SSL_read(), it returns up to a TLS record which can be less than
what was asked. The assumption that it was EOF was wrong which made the while
loop exiting before it was able to consume all requested bytes (at_most
parameter).
The general use case that Tor sees is that it will ask the network layer to
give it at most 16KB (that is roughly 32 cells) but because of KIST scheduler,
the highest possible TLS record we currently observe is 4096 bytes (4KB or 8
cells). Thus the loop would at best always return 8 cells even though much
more could be on the TLS socket. See ticket #40006 for more details.
Fixes#40006
Signed-off-by: David Goulet <dgoulet@torproject.org>
Since "skip orport check" is the "and" of v4_ok and v6_ok, we can
just compute v4_ok and v6_ok once, to clarify that we don't enter
this block of code if they're both true.
With prop312, we want to support IPv4 and IPv6 thus multiple Address statement
(up to 2) will be accepted.
For this, "Address" option becomes a LINELIST so we can properly process the
IPv4 or/and IPv6.
Part of #33233
Signed-off-by: David Goulet <dgoulet@torproject.org>
I've managed to keep this change mainly contained to our
self-testing module. The changes here are:
* There are two different variables for tracking "is our orport
reachable".
* We have a new function that says whether we can skip a single
family's orport reachability test; the old function for this now
tells whether we can skip _all_ orport reachability testing.
(The name, router_should_skip_orport_reachability_test, is not
so good. I will rename it later if I can think of a good
replacement.)
* The function that launches orport reachability tests now only
launches the ones that haven't completed.
* The function that notes that we're reachable on an ORPort now
takes a family.
* Various log messages are cleaned up.
Since Tor 0.2.9 has been unsupported since January, 0.3.5 is the
oldest supported version, and its features constitute the oldest
recommended feature-set.
This patch updates these recommendations:
DirCache=2
Support for consensus diffs.
New in 0.3.1.1-alpha.
HSDir=2
Support for v3 onion service descriptors.
New in 0.3.0.4-alpha.
HSIntro=4
Support for Ed25519 intropoint authentication keys.
New in 0.3.0-4-alpha.
HSRend=2
Support for rendezvous cells longer than 20 bytes.
New in 0.2.9.4-alpha.
Link=5
Link padding and link padding negotiation.
New in 0.3.3.2-alpha.
LinkAuth=3
Ed25519 link authentication.
New in 0.3.0.1-alpha.
This tracking of the instantiation count should eliminate race conditions due
to starting and stopping machines rapidly. Now, we should no longer obey
STOP commands for previous machines.
This field area was memset to 0 in old versions, which the code treats as
"match any machine instance", for backward compatibility without a protover
bump.
M_SYSCALL is used to report information about a sandbox violation,
but when we don't have a definition for it, it still makes sense to
compile.
Closes ticket 34382.
Per ticket #32888 this should address logging "the Address torrc
option", "and whether it is an IP address, or a DNS name"; or the
detected "local hostname", "and whether it is an IP address, or a DNS
name". Some of these details already seem to be logged, so just add
what's missing.
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.
Fixes bug 34303; bugfix on 0.3.3.2-alpha.
Add an inline helper function that indicates if the cached object contains a
decrypted descriptor or not.
The descriptor object is NULL if tor is unable to decrypt it (lacking client
authorization) and some actions need to be done only when we have a decrypted
object.
This improves code semantic.
Fixes#33458
Signed-off-by: David Goulet <dgoulet@torproject.org>
The warning was:
11:23:10 ../tor/src/feature/hs/hs_service.c: In function 'log_cant_upload_desc':
11:23:10 ../tor/src/feature/hs/hs_service.c:3118:3: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
See #34254 for more info.
I guess this means that gcc assigned an unsigned type to the
`log_desc_upload_reason_t` enum and it warned if we compared it against 0...
For now I think it's simpler to remove that check instead of turning the enum
to a signed type, or trying to hack it some other way.
From what it seems, enum is up to the compiler on whether it's signed/unsigned:
https://stackoverflow.com/questions/159034/are-c-enums-signed-or-unsigned
Use the node check function to check that there are enough nodes to
select a circuit path.
Adds these checks, which are implied by other code:
* supports EXTEND2 cells,
* does not allow single-hop exits,
Adds these extra checks:
* has a general-purpose routerinfo,
* if it is a direct connection, check reachable addresses.
These checks reduce the node count, but they will never under-count
nodes.
Bridge nodes aren't handled correctly, we'll fix that in the next
commit.
Part of 34200.
And check that the correct flags are passed when choosing exits.
Adds the following checks for exits:
* must support EXTEND2 cells,
* must have an ntor circuit crypto key,
* can't require the guard flag,
* can't be a direct connection.
All these checks are already implied by other code.
Part of 34200.
Instead, call out to a helper function, repeating the call if needed.
Avoids duplicating exclusions for:
* the current relay's family, and
* any exclusions specified by the caller.
Part of 34200.
And delete a loop that is now empty. This change should improve tor's
performance, because we no longer iterate through the nodelist twice for
every node in every circuit path.
Part of 34200.
Make Rust protocol version support checks consistent with the
undocumented error behaviour of the corresponding C code.
Fixes bug 34251; bugfix on 0.3.3.5-rc.
Declare support for the onion service introduction point denial of
service extensions, when building tor with Rust.
Fixes bug 34248; bugfix on 0.4.2.1-alpha.
Add tests for relays sending IPv6 extend cells in
circuit_send_next_onion_skin().
Clients also use this code, check that they can only extend over IPv4
(for now).
Part of 33222.
