This validation was only done if DisableNetwork was off because we would use
the global list of transports/bridges and DisableNetwork would not populate
it.
This was a problem for any user using DisableNetwork which includes Tor
Browser and thus leading to the Bug() warning.
Without a more in depth refactoring, we can't do this validation without the
global list.
The previous commit makes it that any connection to a bridge without a
transport won't happen thus we keep the security feature of not connecting to
a bridge without its corresponding transport.
Related to #40106
Signed-off-by: David Goulet <dgoulet@torproject.org>
Don't pick the bridge as the guard or launch descriptor fetch if no transport
is found.
Fixes#40106
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch limits the number of items in the consensus diff cache to 64
on the Windows platform. Hopefully, this will allow us to investigate a
smarter fix while avoiding the situation reported in tor#24857 where
Windows relay operators report Tor using 100% CPU.
See: tor#24857
When selecting the first advertised port, we always prefer the one with an
explicit address.
Closes#40246
Signed-off-by: David Goulet <dgoulet@torproject.org>
We used to actually discard ORPorts that were the same port and same family
but they could have different address.
Instead, we need to keep all different ORPorts so we can bind a listener on
each of them. We will publish only one of these in our descriptor though.
Related to #40246
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch removes a call to `tor_assert_nonfatal()` if
`extend_info_from_node()` returns NULL. This is unnecessary as we
already handle the case where `info` is NULL in the next `if (!info) {
... }` block in the code.
See: tor#32666.
This reverts commit d07f17f676.
We don't want to consider an entire routable IPv6 network as sybil if more
than 2 relays happen to be on it. For path selection it is very important but
not for selecting relays in the consensus.
Fixes#40243
We can end up trying to find our address from an authority while we don't have
yet its descriptor.
In this case, don't BUG() and just come back later.
Closes#40231
Signed-off-by: David Goulet <dgoulet@torproject.org>
In case building the descriptor would fail, we could still flag that we did in
fact publish the descriptors leading to no more attempt at publishing it which
in turn makes the relay silent for some hours and not try to rebuild the
descriptor later.
This has been spotted with #40231 because the operator used a localhost
address for the ORPort and "AssumeReachable 1" leading to this code path where
the descriptor failed to build but all conditions to "can I publish" were met.
Related to #40231
Signed-off-by: David Goulet <dgoulet@torproject.org>
This one should work on GCC _and_ on Clang. The previous version
made Clang happier by not having unreachable "fallthrough"
statements, but made GCC sad because GCC didn't think that the
unconditional failures were really unconditional, and therefore
_wanted_ a FALLTHROUGH.
This patch adds a FALLTHROUGH_UNLESS_ALL_BUGS_ARE_FATAL macro that
seems to please both GCC and Clang in this case: ordinarily it is a
FALLTHROUGH, but when ALL_BUGS_ARE_FATAL is defined, it's an
abort().
Fixes bug 40241 again. Bugfix on earlier fix for 40241, which was
merged into maint-0.3.5 and forward, and released in 0.4.5.3-rc.
Our original code for parsing these parameters out of our list of
parameters pre-dated us having the
dirvote_get_intermediate_param_value() function... and it was buggy.
Specifically, it would reject any " ... K=V ..." value
where there were additional unconverted characters after the V, and
use the default value instead,
We haven't run into this yet because we've never voted for
bwweightscale to be anything besides the default 10000, or
maxunmeasuredbw to be anything besides the default 20.
This requires a new consensus method because it is a change in how
consensuses are computed.
Fixes bug 19011; bugfix on 0.2.2.10-alpha.
Some days before this commit, the network experienced a DDoS on the directory
authorities that prevented them to generate a consensus for more than 5 hours
straight.
That in turn entirely disabled onion service v3, client and service side, due
to the subsystem requiring a live consensus to function properly.
We know require a reasonably live consensus which means that the HSv3
subsystem will to its job for using the best consensus tor can find. If the
entire network is using an old consensus, than this should be alright.
If the service happens to use a live consensus while a client is not, it
should still work because the client will use the current SRV it sees which
might be the previous SRV for the service for which it still publish
descriptors for.
If the service is using an old one and somehow can't get a new one while
clients are on a new one, then reachability issues might arise. However, this
is a situation we already have at the moment since the service will simply not
work if it doesn't have a live consensus while a client has one.
Fixes#40237
Signed-off-by: David Goulet <dgoulet@torproject.org>
Thanks to proposal 315 / ticket #30132, more fields are now
required in these documents. But ancient Tors that try to upload
obsolete documents were causing the authorities to log warnings
about missing fields, and to do so very spammily.
We now detect the missing fields before tokenizing, and log at
debug. This is a bit of ugliness, but it's probably a safer choice
than making _all_ unparseable-desc warnings into debug-level logs.
I'm looking at identity-ed25519 in extrainfos and proto in
routerdescs because they were (I believe) the latest-added fields in
Tor's history: any Tor that lacks them will also lack the other
newly required fields.
Fixes bug #40238; bugfix on 0.4.5.1-alpha.
We're getting "fallback annotation annotation in unreachable code"
warnings when we build with ALL_BUGS_ARE_FATAL. This patch fixes
that.
Fixes bug 40241. Bugfix on 0.3.5.4-alpha.
If we get an address suggestion from a directory authority and we have no
address configured or discovered, log it at notice level so the operator can
learn what address will be used by Tor.
Fixes#40201
Signed-off-by: David Goulet <dgoulet@torproject.org>
The previous parser only considered stats files _starting_ with the
timestamp tag, not stats files having the timestamp tag in a later
position. While this applies to all current stats files, a future
stats file might look differently. Better to fix the function now than
be surprised in another 9 years from now.
This commit also adds a test case for such future stats, and it fixes
stats file paths in newly added unit tests.
It turns out that 9 years ago, we stopped appending data into stats file and
rather overwrite everytime we have new stats (see commit
a6a127c833)
The load_stats_file() function was still thinking that we could have the same
line many times in the file which turns out to be false since 9 years ago.
However, that did not cause problem until IPv6 connection stats came along
which introduced a new line in conn-stats: "ipv6-conn-bi-direct ...".
Before, that file contained a single line starting with the tag
"conn-bi-direct". That very tag appears also in the IPv6 tag (see above) so
the load_stats_file() function would consider that the IPv6 line as the last
tag to be appeneded to the file and fail to report the line above (for IPv4).
It would actually truncate the IPv6 line and report it (removing the "ipv6-"
part).
In other words, "conn-bi-direct" was not reported and instead
"ipv6-conn-bi-direct" was used without the "ipv6-" part.
This commit refactors the entire function so that now it looks for a
"timestamp tag" to validate and then if everything is fine, returns the entire
content of the file. The refactor simplifies the function, adds logging in
case of failures and modernize it in terms of coding standard.
Unit tests are also added that makes sure the loaded content matches the
entire file if timestamp validation passes.
Fixes#40226
Signed-off-by: David Goulet <dgoulet@torproject.org>
When making sure we have a Bridge line with a ClientTransportPlugin, we
now check in the managed proxy list and so we can catch any missing
ClientTransportPlugin for a Bridge line.
Fixes#40106
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, our code would send back an error if the socks5 request
parser said anything but DONE. But there are other non-error cases,
like TRUNCATED: we shouldn't send back errors for them.
This patch lowers the responsibility for setting the error message
into the parsing code, since the actual type of the error message
will depend on what problem was encountered.
Fixes bug 40190; bugfix on 0.3.5.1-alpha.
Adds a more user-friendly error message when the configuration is
reloaded and a new %include is added that makes its unglobbing
access files/folders not allowed by the seccomp sandbox.
Previously, "--list-fingerprint --quiet" was an error. Now, the
handler for optional arguments to "--list-fingerprint" can tell that
"--quiet" is a flag, not an argument.
This only affects flags that take an _optional_ argument, so you can
still put your torrc file in a location starting with "-".
Closes#40223.
The "-static" compile flag was set globally which means that all autoconf test
were attempting to be built statically and lead to failures of detecting
OpenSSL libraries and others.
This commit adds this flag only to the "tor" binary build.
There is also a fix on where to find libevent.a since it is using libtool, it
is in .libs/.
At this commit, there are still warnings being emitted that informs the user
that the built binary must still be linked dynamically with glibc.
Fixes#40111
Signed-off-by: David Goulet <dgoulet@torproject.org>
The Python code is such a nice addition to the documentation and the C
code for better understanding of onion v3 address generation. Straight
to the point and easy to understand.
Unfortunately it did not work with my distribution's Python version. I
have adjusted the code to support Python 3.8 (tested with 3.8.6) and
to still be compatible with Python 2.
Currently Tor fails with the following error:
src/test/test_stats.c: In function ‘test_rephist_v3_onions’:
src/test/test_stats.c:527:22: error: overflow in implicit constant conversion [-Werror=overflow]
update_approx_time(10101010101);
This patch changes the constant passed to update_approx_time() to avoid
the overflow in the implicit conversion.
See: tor#40199
The rest of rephist.c is doing the same kind of unsigned casting. For example
see rep_hist_format_buffer_stats() and rep_hist_format_exit_stats().
The previous switch to %ld made Appveyor fail:
https://ci.appveyor.com/project/torproject/tor/builds/36118502
When sending the stream level SENDME, it is possible the cirucit was marked
for close or any other failures that can occur. These events can occur
naturally.
Fixes#40142
Signed-off-by: David Goulet <dgoulet@torproject.org>
The function in charge of removing duplicate ORPorts from our configured ports
was skipping all non ORPorts port but only for the outer loop thus resulting
in comparing an ORPort with a non-ORPort which lead to problems.
For example, tor configured with the following would fail:
ORPort auto
DirPort auto
Both end up being the same configuration except that one is a OR listener and
one is a Dir listener. Thus because of the missing check in the inner loop,
they looked exactly the same and thus one is removed.
Fixes#40195
Signed-off-by: David Goulet <dgoulet@torproject.org>
First, this commit moves the launch_dummy_circuit_as_needed() function into
relay_find_addr.c and renames it to relay_addr_learn_from_dirauth(). This is
an attempt to centralize anything relate with address discovery in the right
module.
Second, when building a descriptor and we fail to discover our address,
immediately launch a dummy circuit to an authority in an attempt to learn our
descriptor.
It is still only done every 20 minutes even though the descriptor build is
done every minute. We ought to avoid load on the authority and if we can't
learn in the first place our address from them, chances are more things are
wrong.
Related to #40071
Signed-off-by: David Goulet <dgoulet@torproject.org>
Regularly, tor looks if its IP has changed. It does the entire auto discovery
process again. However, it is possible that it does not find anything.
Instead of thinking the IP changed to an unknown address, look at our cache
and see if that value has changed.
The reason for this is because if tor gets its address as a suggestion from a
directory authority, it is because the auto discovery failed and thus that
address should be consider for the IP change check.
Related to #40071
Signed-off-by: David Goulet <dgoulet@torproject.org>
Tor now can learn its address from a NETINFO cell coming from an authority.
Thus, instead from launching a dummy descriptor fetch to learn the address
from the directory response (unauthenticated), we simply now launch a one-hop
testing circuit.
Related to #40071
Signed-off-by: David Goulet <dgoulet@torproject.org>
This changes the behaviour of `tor --version` in such a way.
```console
src/app/tor --version
Tor version 0.4.5.1-alpha-dev (git-46ccde66a97d7985).
Tor is running on Linux with Libevent 2.1.12-stable, OpenSSL 1.1.1h, Zlib 1.2.11, Liblzma 5.2.4, Libzstd 1.4.5 and Glibc 2.31 as libc.
Tor compiled with GCC version 10.2.0
```
Fixes#32102
Commit c3a0f75796 added this feature for ORPort
that we ignore any port that is not the family of our default address when
parsing the port. So if port_parse_config() was called with an IPv4 default
address, all IPv6 address would be ignored.
That makes sense for ORPort since we call twice port_parse_config() for
0.0.0.0 and [::] but for the rest of the ports, it is not good since a
perfectly valid configuration can be:
SocksPort 9050
SocksPort [::1]:9050
Any non-ORPort only binds by default to an IPv4 except the ORPort that binds
to both IPv4 and IPv6 by default.
The fix here is to always parse all ports within port_parse_config() and then,
specifically for ORPort, remove the duplicates or superseding ones. The
warning is only emitted when a port supersedes another.
A unit tests is added to make sure SocksPort of different family always exists
together.
Fixes#40183
Signed-off-by: David Goulet <dgoulet@torproject.org>
There were three separate places where we were hitting a sandbox Bug
warning before we actually exited.
Fixes#40094; bugfix on 0.3.1.1-alpha when %includes were introduced.
Typos found with codespell.
Please keep in mind that this should have impact on actual code
and must be carefully evaluated:
src/core/or/lttng_circuit.inc
- ctf_enum_value("CONTROLER", CIRCUIT_PURPOSE_CONTROLLER)
+ ctf_enum_value("CONTROLLER", CIRCUIT_PURPOSE_CONTROLLER)
We already did this, but we did it by accident, which is pretty
risky: if we hadn't, then our code would have treated extra data in
the inbuf as having been transmitted as TLS-authenticated data.
Closes ticket 40017; Found by opara.
This way, if we guess wrong about whether the library has it,
we don't conflict with the library's headers.
Fixes#40181; bug not in any released version.
Previously, hashlib.shake_256 was a class (if present); now it can
also be a function. This change invalidated our old
compatibility/workaround code, and made one of our tests fail.
Fixes bug 40179; bugfix on 0.3.1.6-rc when the workaround code was
added.
In the past, only authorities and clients had to use that function because of
the SRV subsystem. However, because of its use in rep_hist_hs_stats_init() it
will now also be used by relays when bootstrapping without a consensus. Make it
do something sensible.
Another approach (instead of using magic values) would be to wait
initialization of HSv3 stats until we get a consensus but that seems messy to
schedule.
Another approach would be to make dirauth_sched_get_configured_interval() also
work for relays (particularly when TestingNetwork is enabled), but that also
seems a good amount of work.
The "engines.h" header has lots of stuff; the "opensslv.h" header
has the version number, which is all we actually need here.
We need to do this because we're about to change this header to
conditionally define OPENSSL_SUPPRESS_DEPRECATED, and it would be
too late to do so if we'd already included "engines.h".
The loop in the earlier patch would invoke undefined behavior in two
ways: First, it would check whether it was looking at a space before
it checked whether the pointer was in-range. Second, it would let a
pointer reach a position _before_ the start of a string, which is
not allowed.
I've removed the assertion about empty messages: empty messages can
be their own warning IMO.
I've also added tests for this formatting code, to make sure it
actually works.
The "METRICS_PREFIX" was not expanded but rather used as a litteral. Fix that
by just removing the define and using "tor_" directly.
Reviewed-by: Alexander Færøy <ahf@torproject.org>
Signed-off-by: David Goulet <dgoulet@torproject.org>
The total number of rendezvous circuit created and the number of established
ones which is a gauge that decreases to keep an updated counter.
Related to #40063
Signed-off-by: David Goulet <dgoulet@torproject.org>
At this commit, a new service registers to the module and a store is created.
It also remove itself from the metrics module if it goes away.
In order to hook into the metrics subsystem, this commit attaches the HS
subsystem into the subsystem global list so its get_metrics() call can be
accessible.
HS initialization is still _not_ done through the subsys module as it is
likely require much more testing.
Related to #40063
Signed-off-by: David Goulet <dgoulet@torproject.org>
First I began with a set of candidates:
The client's _required_ list starts with all the protocols
supported by every release in 0.2.9-stable through current
master.
The client's _required_ list starts with all the protocols
supported by every release in 0.3.5-stable through current
master.
Everybody's _recommended_ list starts with all the protocols
supported by every release in 0.3.5-stable through current master.
Then I removed the protocol versions that we do not actually want to
require or recommend:
DirCache=1 (deprecated)
HSDir=1, HSIntro=1-3, HSRend=1 (deprecated)
(All HS* protocol requirements for clients)
Link=1-3 (deprecated)
LinkAuth=1 (obsolete)
Relay=1 (obsolete)
Make it possible to specify multiple ConsensusParams torrc
lines.
Now directory authority operators can for example put the
main ConsensusParams config in one torrc file and then add to it
from a different torrc file.
Closes ticket 40164.
When calling `circpad_send_padding_cell_for_callback()`, the flag
`is_padding_timer_scheduled` was not resetted to 0 which caused an issue
in the circpad simulator.
This commit fixes this problem.
Fixes#32671.
Cons=1 is the old format of consensuses, without ed25519 keys. It
is no longer required or recommended.
Cons=2 is the new format of consensuses, with ed25519 keys. It
is now required.
(Similarly for Desc=1,2 and Microdesc=1,2)
No supported client or relay versions should be affected by this
change, since these versions are supported by clients and relays
running 0.2.9 and later. It will only take effect once enough
authorities vote for it.
Closes ticket 40162.
The "protos" field is mandatory, and so are Ed25519 signatures.
Also, remove formats_rsa (the version without Ed25519), since
RSA-only relays are no longer supported.
Had to replace these since we're updating the list of required fields.
These are taken from a chutney instance. Shockingly, this is enough
to make the test_dir_handle_get.c tests pass again.
These tests used a hardcoded vote with a hardcoded digest. That
vote didn't have any pr lines, and so it is now invalid. I've
adjusted the testing code so that it mocks the signature-checking,
so that we can more easily change the contents of the vote.
This was fairly simple: just had to replace the examples with ones
that had ntor keys. There were a couple of example chutney
routerinfos that I had to replace too.
I also removed tests for producing microdescs via consensus methods
that we don't support any longer.
In brief: we go through a lot of gymnastics to handle huge protover
numbers, but after years of development we're not even close to 10
for any of our current versions. We also have a convenient
workaround available in case we ever run out of protocols: if (for
example) we someday need Link=64, we can just add Link2=0 or
something.
This patch is a minimal patch to change tor's behavior; it doesn't
take advantage of the new restrictions.
Implements #40133 and proposal 318.
Tor has a feature to preserve unrecognized state file entries in
order to maintain forward compatibility. But this feature, along
with some unused code that we never actually removed, led to us
keeping items that were of no use to the user, other than at worst
to preserve ancient information about them.
This commit adds a feature to remove obsolete entries when we load
the file.
Closes ticket 40137.
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>
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.
This patch adds support for exposing the environment variables
`TOR_PT_OUTBOUND_BIND_ADDRESS_V4` and `TOR_PT_OUTBOUND_BIND_ADDRESS_V6` to
Pluggable Transport proccesses. These two values will contain the IPv4
and IPv6 address that the user have specified in torrc that they wish
the PT to use for all outgoing IP packets.
It is important to note here that it is up to the indvidual Pluggable
Transport if they are willing to honor these values or ignore them
completely.
One can test this feature using the following dummy PT written in POSIX
shell script:
#!/bin/sh
echo "LOG SEVERITY=warning MESSAGE=\"Value for IPv4: ${TOR_PT_OUTBOUND_BIND_ADDRESS_V4}\""
echo "LOG SEVERITY=warning MESSAGE=\"Value for IPv6: ${TOR_PT_OUTBOUND_BIND_ADDRESS_V6}\""
while true ; do
sleep 1
done
with the following entries in your torrc:
OutboundBindAddressPT 203.0.113.4
OutboundBindAddress 203.0.113.5
OutboundBindAddressPT 2001:db8::4
OutboundBindAddress 2001:db8::5
See: https://bugs.torproject.org/5304
This patch moves the logic for handling outbound addresses in torrc to
before we handle pluggable transports. Since we need access to the
values in OutboundBindAddress and friends for #5304 we have to parse
these values before we spawn any PT's.
This commit is code movement only.
See: https://bugs.torproject.org/5304
This patch renames the enumeration value in `outbound_addr_t` from
`OUTBOUND_ADDR_EXIT_AND_OR` to `OUTBOUND_ADDR_ANY` since with the
arrival of `OUTBOUND_ADDR_PT` it no longer makes sense to call the
fallback value for "Exit and OR". Instead we rename it to "any".
See: https://bugs.torproject.org/5304
This patch adds a new option to torrc: `OutboundBindAddressPT`. This
option works in the same way as `OutboundBindAddressOR` and
`OutboundBindAddressExit` in that it allows the user to specify which
outbound IP address the user wants the PT to make its connections from.
There is one difference though in that OutboundBindAddressPT will only
be a suggestion for the PT to use since Tor cannot enforce whether or
not the PT actually uses this option for anything.
See: https://bugs.torproject.org/5304
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.
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.
