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 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.
These tests don't actually exercise the authority IPv6 ORPort
self-add feature in 32822, but they do improve coverage of the
related config code.
Part of 32822.
Authorities currently add themselves to the trusted dir servers list,
but if they have an IPv6 ORPort, they leave it out.
This commit makes authorities add their own IPv6 ORPort to the trusted
dir servers list.
Closes ticket 32822.
The exposed interface is "stats/" thus make the unit tests clear that it is
testing that specific GETINFO command.
Signed-off-by: David Goulet <dgoulet@torproject.org>
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.
