This patch ensures that we always lowercase the BridgeDistribution from
torrc in descriptors before submitting it.
See: https://bugs.torproject.org/32753
Conflicts:
doc/tor.1.txt
src/app/config/config.c
src/app/config/or_options_st.h
src/core/mainloop/connection.h
Between 042 and 043, the dirauth options were modularized so this merge commit
address this by moving the AuthDirRejectUncompressedRequests to the module
along with a series of accessors.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Authorities were never sending back 503 error code because by design they
should be able to always answer directory requests regardless of bandwidth
capacity.
However, that recently backfired because of a large number of requests from
unknown source using the DirPort that are _not_ getting their 503 code which
overloaded the DirPort leading to the authority to be unable to answer to its
fellow authorities.
This is not a complete solution to the problem but it will help ease off the
load on the authority side by sending back 503 codes *unless* the connection
is from a known relay or an authority.
Fixes#33029
Signed-off-by: David Goulet <dgoulet@torproject.org>
This controls the previous feature added that makes dirauth send back a 503
error code on non relay connections if under bandwidth pressure.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The configured, within the torrc or hardcoded, directory authorities addresses
are now added to the nodelist address set.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We separate v4 and v6 because we often use an IPv4 address represented with
a uint32_t instead of a tor_addr_t.
This will be used to also add the trusted directory addresses taken from the
configuration.
The trusted directories from the consensus are already added to the address
set from their descriptor.
Signed-off-by: David Goulet <dgoulet@torproject.org>
That function is only used to test the global bucket write limit for a
directory connection.
It should _not_ be used for anything else since that function looks to see if
we are a directory authority.
Rename it to something more meaningful. No change in behavior at this commit,
only renaming.
Part of #33029
Signed-off-by: David Goulet <dgoulet@torproject.org>
clang-format sometimes thinks that "#name" should be written as
"# name" if it appears at the start of a line. Using () appears
to suppress this, while confusing Coccinelle.
Doing this makes our macro usage a little clear IMO, and also avoids
having to use an unadorned "new" within a macro. (Clang-format
seems to think that "new" means we're doing C++, and so it generates
some output that checkSpace.pl doesn't care for.)
These belong in util_bug.h (and they already are there).
Their presence made clang-format misindent these functions in a way
that checkSpace.pl dislikes.
Without the hs_init(), the caches are not initialized and the previous commit
was causing an assert due to the missing caches.
But regardless of that, those tests were not initializing but were calling
hs_free_all().
Signed-off-by: David Goulet <dgoulet@torproject.org>
When the ONION_CLIENT_AUTH_REMOVE command is given to tor, now also remove the
descriptor associated with the client authorization credentials.
Fixes#33148
Signed-off-by: David Goulet <dgoulet@torproject.org>
The "once" flag makes tor_bug_occurred_() say that future instances
of the warning will be suppressed -- but that's not something that
BUG() does. Fixes bug 33095; bugfix on 0.4.1.1-alpha.
When we added the ACTIVE and DORMANT virtual signals, we taught the
signal command to handle them, but we didn't teach SIGNAL event to
report them.
To solve this problem and prevent it from recurring, this patch
revises the implementation of control_event_signal() to use the same
signal_table that handle_control_signal() uses. This way, the two
controller commands can't become out of sync.
Fixes bug 33104; bugfix on 0.4.0.1-alpha.
Fixes bug 32673; bugfix on 0.3.0.4-alpha. We introduced these
checks in ee5471f9aa to help diagnose 21369, but we used "-1"
when "TOR_TLS_ERROR_MISC" would have been correct. Found by opara.
I don't think that this is actually getting triggered in the wild,
but if it were, it could cause nasty behavior: spurious
WANTREAD/WANTWRITE returns have a way of turning into CPU-eating
busy-loops.
We introduced these BUG() checks in b0ddaac074 to prevent a
recurrence of bug 23690. But there's a report of the BUG() message
getting triggered and filling up the disk. Let's change it to
IF_BUG_ONCE().
Fixes bug 33093; bugfix on 0.3.2.2-alpha.
Simplify ip_address_changed() by removing redundant checks
(all exits are relays, and need to rebuild their descriptor when their
IP addresses change).
Rewrite the function comment, and rename the argument, so the function
is easier to understand and use.
Closes 33091.
Now we use the exact same INTRO2 decrypt logic regardless of whether the
service is an OB instance or not.
The new get_subcredential_for_handling_intro2_cell() function is responsible
for loading the right subcredentials in either case.
This approach saves us a pair of curve25519 operations for every
subcredential but the first. It is not yet constant-time.
I've noted a few places where IMO we should refactor the code so
that the complete list of subcredentials is passed in earlier.
At this commit, the service reads the config file and parse it to finally set
the service config object with the options.
Part of #32709
Signed-off-by: David Goulet <dgoulet@torproject.org>
The hs_parse_address() can not be used without an options_t object existing
since on error it uses the escaped_safe_str() that looks at the options.
This new function won't log and returns an error message in case of failure
that can then be used to log.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Make Tor clients tell dual-stack exits that they prefer IPv6
connections.
This change is equivalent to setting the PreferIPv6 flag on
SOCKSPorts (and most other listener ports). Tor Browser has
been setting this flag for some time, and we want to remove a
client distinguisher at exits.
Also update the man page, putting all the IP flags in their
non-default forms.
Closes ticket 32637.
When we removed obsolete consensus methods in 32695, we also
removed ROUTERSTATUS_FORMAT_NO_CONSENSUS_METHOD, so we need to
remove it from the routerstatus format tests.
Part of 20218.
* ticket 32695 removed networkstatus_consensus_has_ipv6(),
keep that change in master.
* ticket 20218 modifies the function name and comment for
routerstatus_has_visibly_changed(), keep that change
in ticket20218_rebased_squashed.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
routerstatus_has_changed routerstatus_has_visibly_changed
It was generated with --no-verify, since it introduces a wide line.
I'll fix it in a subsequent commit.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
directory_must_use_begindir dirclient_must_use_begindir \
directory_fetches_from_authorities dirclient_fetches_from_authorities \
directory_fetches_dir_info_early dirclient_fetches_dir_info_early \
directory_fetches_dir_info_later dirclient_fetches_dir_info_later \
directory_too_idle_to_fetch_descriptors dirclient_too_idle_to_fetch_descriptors
To make Tor still work, we define a minimal dircache_stub.c file
that defines the entry points to the module that can actually be
seen by the compiler when we're building with dircache and relay
disabled.
This function had some XXX comments indicating (correctly) that it
was not actually used by the dirserver code, and that only the
controller still used it.
We already check if there are invalid values in
check_bridge_distribution_setting() and reject the value if that is the
case. We can therefore only have strings of [A-Z] | [a-z] | [0-9] | '-'
| '_' here which is according to the directory specification.
See: https://bugs.torproject.org/32753
Fix dirauth and relay module include.am add_c_file.py
"control line not preceded by a blank line" errors.
Also remove a duplicate ADD_C_FILE: SOURCES in the relay module.
Obviously correct fixes to already-reviewed code.
This patch makes sure we lowercase the value of BridgeDistribution
before we add it to the descriptor as `bridge-distribution-request`.
See: https://bugs.torproject.org/32753
This patch makes sure that we accept values such as "nOne", "None", and
"AnY" as valid values for BridgeDistribution. We later ensure to
lower-case the values before they are forwarded to the BridgeDB.
See: https://bugs.torproject.org/32753
When the BridgeDistribution value was added in commit
ebab521525 the check for whether the value
contains '\n' or '\r' was added as a requirement for whether or not the
value should be escaped.
This patch removes this check and makes sure we call `escaped()` on the
value every time before we add it to a descriptor.
See: https://bugs.torproject.org/32753
When we made these functions exist unconditionally (as macros on
non-windows platforms), we started to get a dead-code warning on
Coverity. We now use a macro to tell coverity not to worry about
this particular dead-code instance.
The POSSIBLE(e) macro evaluates to the value of (e), but does so in
a way that a static analyzer will not conclude that (e) is
impossible. We can use this when we expect our regular compilers to
eliminate deadcode, but we don't want coverity to complain about it.
Part of a fix for 32960.
The variable lenght cells are handled directly by
channel_tls_handle_var_cell() from an OR connection reading its inbuf. The
channel var cell handler (agnostic) was never used.
Closes#32892
Signed-off-by: David Goulet <dgoulet@torproject.org>
We hit this assertion with bug 32868, but I'm stymied figuring out
how we wound up with a routerstatus like this. This patch is a
diagnostic to attempt to figure out what is going on, and to avoid a
crash in the meantime.
Now that these tests are skipped on windows _and_ android, we should
name the macro that skips them after the functionality that the
tests depend on.
Closes last part of 32825.
When looking up an entry in the rend_cache, stop asserting that it exists but
rather confirm it exists and if not, return that no entry was found.
The reason for that is because the hs_circ_cleanup_on_free() function (which
can end up looking at the rend_cache) can be called from the
circuit_free_all() function that is called _after_ the rend cache is cleaned
up in tor_free_all().
We could fix the free all ordering but then it will just hide a future bug.
Instead, handle a missing rend_cache as a valid use case as in while we are in
the cleanup process.
As Tor becomes more modular, it is getting more and more difficult to ensure
subsystem callstack ordering thus this fix aims at making the HSv2 subsystem
more robust at being called while tor is pretty much in any kind of state.
Fixes#32847.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We never used them very much, and although they had potential to
clarify some of our tests, they also made some of the logic harder
for people to follow. Clang-format can't make head or tail of them,
so the time has come to say goodbye to them.
This is an automated commit, generated by:
perl -i -pe 'next if /define/; s/((?:ENABLE|DISABLE)_GCC_WARNING)\(([A-Za-z0-9_\-]+)\)/$1(\"-W$2\")/' src/*/*/*.[ch] src/*/*.[ch]
These test cases are validated differently by some
programming languages, because those languages have
incorrect UTF-8 implementations.
We want to make sure that tor validates them correctly.
Closes ticket 32845.
Allow calls to dup() which was introduced in commit a22fbab986.
From a security perspective, I don't think this should impact the
security of the sandbox significantly. As far as I can tell, there
is nothing an adversary can do with a duplicated FD that can't be
done with the original.
* there is no ~ or HOME in Android
* UIDs and GIDs in Android represent apps, not users, and Android apps
cannot ever change UIDs
This should be replaced by whatever @nmathewson wants to name this stuff:
https://github.com/torproject/tor/pull/1436
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
EXPOSE_CLEAN_BACKTRACE BACKTRACE_PRIVATE \
TOR_CHANNEL_INTERNAL_ CHANNEL_OBJECT_PRIVATE \
CHANNEL_PRIVATE_ CHANNEL_FILE_PRIVATE \
EXPOSE_ROUTERDESC_TOKEN_TABLE ROUTERDESC_TOKEN_TABLE_PRIVATE \
SCHEDULER_PRIVATE_ SCHEDULER_PRIVATE
Some ".c" files define *_PRIVATE macros, but those macros are
not used in any header file. Delete them.
These changes were created using the "make autostyle" from
32522, and then split into commits.
We stopped looking at this option in 85cf6dcba3, back when we
implemented the minimal pieces of prop275. Since then, we've had
code to validate and adjust this option, and to give it a different
value in testing networks, but the option hasn't actually done
anything.
We can safely mark it as OBSOLETE, since doing so does not make any
old configuration get rejected.
Closes ticket 32807.
I've chosen the "AuthDirMaxServersPerAddr" option here for
simplicity, since it is used literally nowhere else besides the dirauth
module. Once we have all the infrastructure in place for this, we
can move more options into this structure.
Like "obsolete" variables, these variables produce a warning when
you try to set them, but the warning says that the relevant module
doesn't have support.
The confdecl macros now have a CONF_CONTEXT that you can define to
make all the modules in a given table disabled.
These modules are only built when the selected modules are disabled.
The provide stub implementations of the subsystem blocks. Later,
other stub implementations could move here.
Having real subsystem blocks here will let us handle disabled
configuration options better.
There is no /tmp or mkdtemp on Android, there is /data/local/tmp for
root and the shell user. So this fakes mkdtemp. Also, FYI, tor might
not like the default perms of /data/local/tmp, e.g. 0770.
https://trac.torproject.org/projects/tor/ticket/32172
With v3, the "pending_final_cpath" of a circuit is always NULL which means
that for v3, established client rendezvous circuit waiting for the intro point
to ACK, will always end up timing out quickly.
This can increase the delays to which you connect to a service since in order
to succeed, the rendezvous circuit needs to fully established
(CIRCUIT_PURPOSE_C_REND_JOINED) within the cutoff of the introduction circuit
as well which is these days around 2-3 seconds.
Fixes#32021
Signed-off-by: David Goulet <dgoulet@torproject.org>
Simplify handle_control_getinfo() by using the new reply lines
abstraction. Previously, this function explicitly checked for whether
it should generate a MidReplyLine, a DataReplyLine, or an
EndReplyLine. control_write_reply_lines() now abstracts this check.
Part of #30984.
In handle_control_getconf(), use the new control reply line
abstraction to simplify output generation. Previously, this function
explicitly checked for whether it should generate a MidReplyLine or an
EndReplyLine. control_write_reply_lines() now abstracts this check.
Part of #30984.
