Added tests for
1. commit_max
2. advance_obs
3. add_obs
4. rep_hist_fill_bandwidth_history
5. rep_hist_get_bandwidth_lines
(Lightly edited by nickm to use new names from bwhist refactoring.)
Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
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".
This function tried to modify an array in place, but did it in a
pretty confusing and complicated way. I've revised it to follow a
much more straightforward approach.
Fixes bug #40065.
Fix crash introduced in #40020. On startup, tor calls
check_private_dir on the data and key directories. This function
uses open instead of opendir on the received directory. Data and
key directoryes are only opened here, so the seccomp rule added
should be for open instead of opendir, despite the fact that they
are directories.
On an IPv6 reachability failure test, if the address was configured, don't
publish the descriptor and log warn. If the address was auto discovered, still
publish the descriptor.
Closes#33247.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Enum allows us to easily compare what is being returned but also better
semantic to the code.
Related #33247
Signed-off-by: David Goulet <dgoulet@torproject.org>
Now support IPv4 _and_ IPv6.
This also cleans up nicely the function that was moving IPv4 addresses from
uint32_t to tor_addr_t.
Fixes#40058
Signed-off-by: David Goulet <dgoulet@torproject.org>
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.
Closes ticket 33747.
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.
In routerconf_find_ipv6_or_ap(), we check if the returned ORPort is internal
but not for listening. This means that IPv6 [::] is considered internal.
Thus, we can't use it, we have to look directly at the configured address and
port and if they are valid, we do consider that we have a valid IPv6 ORPort
and that we can thus extend in IPv6.
Related #33246
Signed-off-by: David Goulet <dgoulet@torproject.org>
Now that tor automatically binds to IPv4 _and_ IPv6, in order to avoid
breaking configurations, we sanitize the parsed lists for duplicate ORPorts.
It is possible to happen because we still allow this configuration;
ORPort 9888
ORPort [4242::1]:9888
Meaning that the first ORPort value will bind to 0.0.0.0:9888 _and_ [::]:9888
which would lead to an error when attempting to bind on [4242::1]:9888.
However, that configuration is accepted today and thus we must not break it.
To remedy, we now sanitize the parsed list and prioritize an ORPort that has
an explicit address over the global one.
A warning is emitted if such configuration pattern is found. This is only for
the ORPort.
Related to #33246
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit makes it that if the ORPort is set with a single port, it will
bind to both global listen IPv4 and IPv6 addresses.
To pin an "ORPort <PORT>" to be IPv4 or IPv6, the IPv4Only/IPv6Only flags are
honored thus this will _only_ bind on IPv6 for that port value:
ORPort 9050 IPv6Only
Results in: [::]:9050
ORPort 9051 IPv4Only
Results in: [0.0.0.0]:9051
Attempting to configure an explicit IPv4 address with IPv6Only flag is an
error and vice versa.
Closes#33246
Signed-off-by: David Goulet <dgoulet@torproject.org>
These now (or_port and dir_port) now have "find" names, since they
look at the portcfg first, then at the actual ports from the
listeners.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
router_get_advertised_or_port routerconf_find_or_port \
router_get_advertised_ipv6_or_ap routerconf_find_ipv6_or_ap \
router_has_advertised_ipv6_orport routerconf_has_ipv6_orport \
router_get_advertised_dir_port routerconf_find_dir_port
Rationale: these don't actually give the first advertised
address/port, but instead give us the first such port that we are
_configured_ to advertise. Putting them in a portconf_ namespace
therefore makes sense.
Similarly, there are no other functions that get the first
configured advertised addr/port, so the "by_type_af()" part is needless.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
get_first_advertised_addr_by_type_af portconf_get_first_advertised_addr \
get_first_advertised_port_by_type_af portconf_get_first_advertised_port
This option controls if a tor relay will attempt address auto discovery and
thus ultimately publish an IPv6 ORPort in the descriptor.
Behavior is from proposal 312 section 3.2.6.
Closes#33245
Signed-off-by: David Goulet <dgoulet@torproject.org>
The ORPort can be IPv4Only which means that even if we can auto discover an
IPv6 address, we should not publish it because it would have an ORPort of 0.
Fixes#40054
Signed-off-by: David Goulet <dgoulet@torproject.org>
The need for casting negative syscall arguments depends on the
glibc version. This affects the rules for the openat syscall which
uses the constant AT_FDCWD that is defined as a negative number.
This commit adds logic to only apply the cast when necessary, on
glibc versions from 2.27 onwards.
Different versions of glibc use either open or openat for the
opendir function. This commit adds logic to use the correct rule
for each glibc version, namely:
- Until 2.14 open is used
- From 2.15 to to 2.21 openat is used
- From 2.22 to 2.26 open is used
- From 2.27 onwards openat is used
The need for casting negative syscall arguments depends on the
glibc version. This affects the rules for the openat syscall which
uses the constant AT_FDCWD that is defined as a negative number.
This commit adds logic to only apply the cast when necessary, on
glibc versions from 2.27 onwards.
Dirauth code use the warn log severity when calling find_my_address() which
made it that every time we would find an address, it would log a warning.
These are not needed below info level and thus set them to info level. An IP
change is set to notice by default.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Instead of a boolean saying "cache_only" add the concept of flags so we add
semantic through out the code and allow ourselves to have more options in the
future.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Tell the relay find address interface to only use the cache so we don't
trigger an address resolve everytime the "GETINFO address" is called.
Related #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previous development introduced the error of using 0/1 for a boolean
parameter. Fix that everywhere
Related #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
Remove use of router_pick_published_address() and use
relay_find_addr_to_publish instead.
Related to #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
At the moment, this command only returns the IPv4. Do so by using the new
relay_find_addr_to_publish().
New commands to return IPv4 and IPv6 will be done with the work in tor#40039.
Related to #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
Use the new relay_has_address_set() interface when deciding if we need to
fetch directory information from an authority as a relay.
If no IPv4 address is found, we'll proceed with a fetch so we can learn our
address in the HTTP header or NETINFO cell that a trusted authority will send
us back.
Related to #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
Tor periodic events have moved to a role base model where relays have specific
events. One of those is to rebuild the descriptor and that is ran every
minute.
This removes the call to router_rebuild_descriptor() from
router_get_my_routerinfo_with_err() because that is the only code path that
can call for a rebuild every second.
Instead, when we mark the descriptor as dirty, immediately reschedule the
descriptor check periodic event so it can be rebuilt that way instead of
randomly when router_get_my_routerinfo_with_err() is called.
Related to #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
When a relay builds a new descriptor, use the new relay_find_addr_to_publish()
interface to find the address to publish per family.
This commit also make the check for address consistency to also work for a
configured IPv6 for which before it was IPv4 only.
Related to #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
In case the transport has no usable address configured (likely 0.0.0.0 or
[::]), attempt to find the IPv4 and on failure, fallback to the IPv6. If none
are found, a log error is emitted and the transport is skiped.
Related to #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
In order for a relay to find which address to publish in its descriptor,
router_pick_published_address() is used. However, that function only supports
AF_INET and uses the directory server suggested address discovery mechanism.
This new function uses a new interface so that the caller can request an
address family and get the tor_addr_t object. Furthermore, it drops the use of
directory servers address discovery (tor#33244) and instead uses the new
suggested cache that is populated at the moment from data in the NETINFO cell
coming from the directory authorities.
At this commit, function is unused.
Related to #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
Commit b14b1f2b1d was a mistake.
In case an Address statement is missing for the wanted family but another one
exists for another family, simply continue the address discovery.
It is not a mistake to be missing an Address statement for a family because
the address could simply be discovered by the next methods. Not all address
family requires a specific Address statement.
However, we do bail if we couldn't find any valid address for the requested
family _and_ a resolve failed meaning we had a hostname but couldn't resolve
it. In that case, we can't know if that hostname would have been for v4 or v6
thus we can't continue the address discovery properly.
Couple unit tests case were removed to match this reality.
Related #40025
Signed-off-by: David Goulet <dgoulet@torproject.org>
Instead of replacing connection_t.{addr,port} with a canonical
orport, and tracking the truth in real_addr, we now leave
connection_t.addr alone, and put the canonical address in
canonical_orport.
Closes#40042Closes#33898
(This is not fully general yet: we only pick randomly among
_supported_ addresses, and each extendinfo contains at most one IPv4
address and at most one IPv6 address, no matter what the extend cell
had.)
This change will help dual-stack relays do IPv6 reachability tests,
in theory, by having them sometimes do IPv4 connections and
sometimes do ipv6 connections.
Closes ticket 33220.
This fixes CID 1465291, which was a complaint that we never actually
checked the return value of this function. It turns out that this
function was failing, and it didn't matter, because it wasn't
necessary for this test.
This is an automated commit made with a python script.
After running the automated script, I had to hand-revert the cases where it
made the conversion functions call themselves.
Additionally, I had to edit a variable declaration in control_bootstrap.c so
that the result of a const cast could be put in a const field.
Only log the 'real_addr' when it is set to something.
Only log the ID when it is set.
When scrubbing the address, don't include a canonical address.
(There should never be a canonical address for a connection with no
ID set.)
Now that we've clarified that these functions only need to describe
the peer in a human-readable way, we can have them delegate to
connection_describe_peer().
This mode was only used in one place, and it caused a dangerous
mingling of functionality. The method is supposed to _describe_ the
peer's address, not give its actual address. We already had a
function to get the actual address.
* We no longer call this an optional method
* We document that it returns the real address, not a canonical one.
* We have it try harder if the real address hasn't been set yet.
Tests %include with files and folders, modifying and reloading
the config file with sandbox enabled and reponse of SAVECONF and
getinfo config-can-saveconf control commmands.
This patch ensures that we strip "\r" characters on both Windows as well
as Unix when we read text files. This should prevent the issue where
some Tor state files have been moved from a Windows machine, and thus
contains CRLF line ending, to a Unix machine where only \n is needed.
We add a test-case to ensure that we handle this properly on all our
platforms.
See: https://bugs.torproject.org/tpo/core/tor/33781
Pass the IPv4 before the IPv6 like all our other interfaces.
Changes unreleased code related to #40043.
Closes#40045
Signed-off-by: David Goulet <dgoulet@torproject.org>
This changes a LOT of code but in the end, behavior is the same.
Unfortunately, many functions had to be changed to accomodate but in majority
of cases, to become simpler.
Functions are also removed specifically those that were there to convert an
IPv4 as a host format to a tor_addr_t. Those are not needed anymore.
The IPv4 address field has been standardized to "ipv4_addr", the ORPort to
"ipv4_orport" (currently IPv6 uses ipv6_orport) and DirPort to "ipv4_dirport".
This is related to Sponsor 55 work that adds IPv6 support for relays and this
work is needed in order to have a common interface between IPv4 and IPv6.
Closes#40043.
Signed-off-by: David Goulet <dgoulet@torproject.org>
These fields have a complicated history, some slightly complicated
behavior, and some definitely inadequate documentation. Before we
go fixing them up, let's document how they work now.
Fix on unreleased code.
Logical || in the BUG() made it that it would always trigger the BUG().
Fixes#40034
Signed-off-by: David Goulet <dgoulet@torproject.org>
Fix on unreleased code.
The relay_new_address_suggestion() is called when a NETINFO cell is received
thus not only for relay or bridges.
Remove the BUG() that made sure only in server mode we could handle the
suggested address.
Fixes#40032
Signed-off-by: David Goulet <dgoulet@torproject.org>
We've done a lot to improve our internal APIs since we wrote this
code, and it shows. We can just use a buf_t to build up the
bandwidth lines, and save a bunch of stack fiddling.
Additionally, we can use a function to format a single line, and
thereby get rid of the cheezy pattern that does
for (i=0;i<n;++i) {
switch (i) {
...
}
...
}
Unclear but that somehow failed on Windows once (?) according to ticket #33768
but we are not seeing that failure.
Nevertheless, add a simple unit test.
Closes#33768
Signed-off-by: David Goulet <dgoulet@torproject.org>
This will make sure that we always properly initialize the cache by the exact
size all the time.
Related to #40022
Signed-off-by: David Goulet <dgoulet@torproject.org>
When receiving an introduction NACK, the client either decides to close or
re-extend the circuit to another intro point.
In order to do this, the service descriptor needs to exists but it is possible
that it gets removed from the cache between the establishement of the
introduction circuit and the reception of the (N)ACK.
For that reason, the BUG(desc == NULL) is removed because it is a possible
normal use case. Tor recovers gracefully already.
Fixes#34087
Signed-off-by: David Goulet <dgoulet@torproject.org>
It now uses the 'goto err' pattern, instead of the fatal_unreached()
pattern. The latter pattern is usually used when there is a loop, but there is
no loop in this function so it can be simplified easily.
