Style:
- We end our types with _t.
- Use 'static' to declare functions that only exist in a single
module.
Correctness:
- Many tt_...() macros can invoke "goto done;" -- we need to make
sure that all the variables that could get freed are initialized
before any "goto done" is hit, or else we might free an
uninitialized variable.
[This is a squashed patch for ticket 7193, based on taking a "git
diff" for the original branch, then applying it with "git apply
-3". I earlier attempted to squash the branch with "git rebase",
but there were too many conflicts. --nickm]
The option `--enable-all-bugs-are-fatal` when used with
`--disable-module-relay` caused GCC to warn about functions that
it thought should be labeled noreturn.
I've tried a couple of approaches, but disabling the warning on
these functions seems to be the best approach.
Fixed#40129; bugfix on 0.4.4.1-alpha.
We set this flag if we've launched the connection in order to
satisfy an origin circuit, or when we decide the connection _would_
satisfy an origin circuit. These are the only or_connections we
want to consider for bootstrapping: other or_connections are opened
because of client EXTEND requests, and they may succeed or fail
because of the clients' confusion or misconfiguration.
Closes#25061.
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
7eef9ced61)
Fixes#40125
Signed-off-by: David Goulet <dgoulet@torproject.org>
Coverity's first complaint was that we didn't check the return
values from chmod. That's easily fixed.
Coverity's second complaint was that there were code paths where we pass
NULL to chmod. For example, if this line failed, we'd "goto done",
and then pass NULL to chmod.
tt_ptr_op(dirname, OP_NE, NULL);
Closes#40103. Bug not in any released Tor.
Our code was using [01] as for the key type of signed->link certs,
which was incorrect. The value should be [03], to indicate that the
value as the SHA256 of an x.509 cert.
Fortunately, nothing cares about this value, so there shouldn't be
compatibility issues.
Fixes bug 40124; bugfix on 0.2.7.2-alpha.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
tor_cert_create tor_cert_create_ed25519
It was generated with --no-verify, so it probably breaks some commit hooks.
The commiter should be sure to fix them up in a subsequent commit.
This patch disables the glob() support in the path library if glob() is
unavailable at build-time. This currently happens with the Android NDK
used for Tor Browser.
See: https://bugs.torproject.org/tpo/core/tor/40114
tl;dr We were not counting cells flying from the client to the service, but we
were counting cells flying from the service to the client.
When a rendezvous cell arrives from the client to the RP, the RP forwards it to
the service.
For this to happen, the cell first passes through command_process_relay_cell()
which normally does the statistics counting. However because the `rend_circ`
circuit was not flagged with `circuit_carries_hs_traffic_stats` in
rend_mid_rendezvous(), the cell is not counted there.
Then the cell goes to circuit_receive_relay_cell() which has a special code
block based on `rend_splice` specifically for rendezvous cells, and the cell
gets directly passed to `rend_circ` via a direct call to
circuit_receive_relay_cell(). The cell never passes through
command_process_relay_cell() ever again and hence is never counted by our
rephist module.
The fix here is to flag the `rend_circ` circuit with
`circuit_carries_hs_traffic_stats` so that the cell is counted as soon as it
hits command_process_relay_cell().
Furthermore we avoid double-counting cells since the special code block of
circuit_receive_relay_cell() makes us count rendezvous cells only as they enter
the RP and not as they exit it.
Fixes#40117.
When accessing the last_resolved_address cache we always need to convert the
AF family value to an index value else we are out of bound and thus
overflowing if we write to it.
This fix is on code that has not been released.
GeKo reported the following libasan crash using Tor Browser alpha with tor
0.4.5.0-alpha-dev (3c884bc9e0):
==4240==ERROR: AddressSanitizer: global-buffer-overflow on address
0x55888490e388 at pc 0x5588842cc216 bp 0x7ffc8c421b00 sp 0x7ffc8c421af8
READ of size 2 at 0x55888490e388 thread T0
#0 0x5588842cc215 in tor_addr_compare_masked
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x5a6215)
#1 0x558884203210 in is_local_to_resolve_addr
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x4dd210)
#2 0x558883f7e252 in channel_tls_connect
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x258252)
#3 0x558883f87ff7 in channel_connect_for_circuit
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x261ff7)
#4 0x558883f8bc90 in circuit_handle_first_hop
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x265c90)
#5 0x558883f8c891 in circuit_establish_circuit
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x266891)
#6 0x558883fc3bbc in circuit_launch_by_extend_info
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x29dbbc)
#7 0x558883fc5900
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x29f900)
#8 0x558883fc6988 in connection_ap_handshake_attach_circuit
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x2a0988)
#9 0x558883fd0d3f in connection_ap_attach_pending
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x2aad3f)
#10 0x7f4d50110885 (TorBrowser/Tor/libevent-2.1.so.7+0x22885)
#11 0x7f4d501110de in event_base_loop
(TorBrowser/Tor/libevent-2.1.so.7+0x230de)
#12 0x558883f69b3c in do_main_loop
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x243b3c)
#13 0x558883f3f70c in tor_run_main
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x21970c)
#14 0x558883f3c2f7 in tor_main
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x2162f7)
#15 0x558883f3531b in main
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x20f31b)
#16 0x7f4d4f76acc9 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x26cc9)
#17 0x558883f3ba00
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x215a00)
0x55888490e388 is located 24 bytes to the left of global variable
'state_mgr' defined in 'src/app/config/statefile.c:184:22'
(0x55888490e3a0) of size 8
0x55888490e388 is located 32 bytes to the right of global variable
'global_state' defined in 'src/app/config/statefile.c:204:20'
(0x55888490e360) of size 8
SUMMARY: AddressSanitizer: global-buffer-overflow
(/home/thomas/Arbeit/Tor/tor-browser-build/tor-browser_en-US/Browser/TorBrowser/Tor/tor+0x5a6215)
in tor_addr_compare_masked
Shadow bytes around the buggy address:
0x0ab190919c20: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
0x0ab190919c30: 04 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 03 f9 f9 f9
0x0ab190919c40: f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
0x0ab190919c50: 00 00 00 04 f9 f9 f9 f9 00 00 00 00 00 00 00 04
0x0ab190919c60: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
=>0x0ab190919c70: f9[f9]f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
0x0ab190919c80: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0ab190919c90: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0ab190919ca0: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
0x0ab190919cb0: 01 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
0x0ab190919cc0: 00 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==4240==ABORTING
Signed-off-by: David Goulet <dgoulet@torproject.org>
Opening a new listener connection can fail in many ways like a bind()
permission denied on a low port for instance.
And thus, we should expect to handle an error when creating a new one instead
of assert() on it.
To hit the removed assert:
ORPort 80
KeepBindCapabilities 0
Start tor. Then edit torrc:
ORPort <some-IP>:80
HUP tor and the assert is hit.
Fixes#40073
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, we would treat *any* incoming circuit on a non-local
channel as meaning that our ORPort was reachable. With this patch,
we make sure that the address that the peer _says_ we have is the
same as the one we're trying to advertise right now.
Closes 20165. Bugfix on 4f5192b280 in 0.1.0.1-rc, when
reachability self-tests were first introduced.
Instead of looking at the "Address" option alone, instead check if we have an
address in our cache (that is discovered by tor). If not, then it tells us
that tor does not have an address to work with so we can then ask a directory
authority for a suggestion.
Related #2178
Signed-off-by: David Goulet <dgoulet@torproject.org>
Turns out that the HS DoS defenses parameters were overwritten by the
consensus parameters everytime a new consensus would arrive.
This means that a service operator can still enable the defenses but as soon
as the intro point relay would get a new consensus, they would be overwritten.
And at this commit, the network is entirely disabling DoS defenses.
Fix this by introducing an "explicit" flag that indicate if the
ESTABLISH_INTRO cell DoS extension set those parameters or not. If set, avoid
using the consenus at once.
We are not bumping the protover HSIntro value for this because 0.4.2.x series
is EOL in 1 month and thus 0.4.3.x would be the only series with this bug. We
are confident that a backport and then upgrade path to the latest 0.4.4.x
stable coming up soon is enough to mitigate this problem in the coming months.
It avoids the upgrade path on the service side by keeping the requirement for
protover HSIntro=5.
Fixes#40109
Signed-off-by: David Goulet <dgoulet@torproject.org>
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.
Closes 40098.
(Unlike approaches in earlier versions of Tor, this one logs if and
only if we are launching a test, and we haven't said that we're
testing this port.)
Fixes bug 40068; bug not in any released version.
Closes#40068.
We used to have a single boolean, "FascistFirewall". Ages ago, in
tickets #17840 and #9067, we added an improved "ReachableAddresses"
mechanism. It's time to rename related identifiers in the code for
consistency. This closes#18106.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
fascist_firewall_allows_address reachable_addr_allows \
fascist_firewall_use_ipv6 reachable_addr_use_ipv6 \
fascist_firewall_prefer_ipv6_impl reachable_addr_prefer_ipv6_impl \
fascist_firewall_prefer_ipv6_orport reachable_addr_prefer_ipv6_orport \
fascist_firewall_prefer_ipv6_dirport reachable_addr_prefer_ipv6_dirport \
fascist_firewall_allows_address_addr reachable_addr_allows_addr \
fascist_firewall_allows_address_ap reachable_addr_allows_ap \
fascist_firewall_allows_base reachable_addr_allows_base \
fascist_firewall_allows_ri_impl reachable_addr_allows_ri_impl \
fascist_firewall_allows_rs_impl reachable_addr_allows_rs_impl \
fascist_firewall_allows_rs reachable_addr_allows_rs \
fascist_firewall_allows_md_impl reachable_addr_allows_md_impl \
fascist_firewall_allows_node reachable_addr_allows_node \
fascist_firewall_allows_dir_server reachable_addr_allows_dir_server \
fascist_firewall_choose_address_impl reachable_addr_choose_impl \
fascist_firewall_choose_address reachable_addr_choose \
fascist_firewall_choose_address_base reachable_addr_choose_base \
fascist_firewall_choose_address_rs reachable_addr_choose_from_rs \
fascist_firewall_choose_address_ls reachable_addr_choose_from_ls \
fascist_firewall_choose_address_node reachable_addr_choose_from_node \
fascist_firewall_choose_address_dir_server reachable_addr_choose_from_dir_server
Previous message would say "N messages in the last T seconds", but
would give an inaccurate number for N.
We now give an accurate number, rounded up to the nearest 60 seconds.
Closes#19431.
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.
Closes#40081.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
protover_contains_long_protocol_names protover_list_is_invalid
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