This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
router_skip_orport_reachability_check router_should_skip_orport_reachability_check \
router_skip_dirport_reachability_check router_should_skip_dirport_reachability_check \
router_connect_assume_or_reachable client_or_conn_should_skip_reachable_address_check \
router_connect_assume_dir_reachable client_dir_conn_should_skip_reachable_address_check
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.
GCC added an implicit-fallthrough warning a while back, where it
would complain if you had a nontrivial "case:" block that didn't end
with break, return, or something like that. Clang recently added
the same thing.
GCC, however, would let you annotate a fall-through as intended by
any of various magic "/* fall through */" comments. Clang, however,
only seems to like "__attribute__((fallthrough))". Fortunately, GCC
accepts that too.
A previous commit in this branch defined a FALLTHROUGH macro to do
the right thing if GNUC is defined; here we replace all of our "fall
through" comments with uses of that macro.
This is an automated commit, made with the following perl one-liner:
#!/usr/bin/perl -i -p
s#/\* *falls? ?thr.*?\*/#FALLTHROUGH;#i;
(In order to avoid conflicts, I'm applying this script separately to
each maint branch. This is the 0.4.3 version.)
GCC added an implicit-fallthrough warning a while back, where it
would complain if you had a nontrivial "case:" block that didn't end
with break, return, or something like that. Clang recently added
the same thing.
GCC, however, would let you annotate a fall-through as intended by
any of various magic "/* fall through */" comments. Clang, however,
only seems to like "__attribute__((fallthrough))". Fortunately, GCC
accepts that too.
A previous commit in this branch defined a FALLTHROUGH macro to do
the right thing if GNUC is defined; here we replace all of our "fall
through" comments with uses of that macro.
This is an automated commit, made with the following perl one-liner:
#!/usr/bin/perl -i -p
s#/\* *falls? ?thr.*?\*/#FALLTHROUGH;#i;
(In order to avoid conflicts, I'm applying this script separately to
each maint branch. This is the 0.4.2 version.)
GCC added an implicit-fallthrough warning a while back, where it
would complain if you had a nontrivial "case:" block that didn't end
with break, return, or something like that. Clang recently added
the same thing.
GCC, however, would let you annotate a fall-through as intended by
any of various magic "/* fall through */" comments. Clang, however,
only seems to like "__attribute__((fallthrough))". Fortunately, GCC
accepts that too.
A previous commit in this branch defined a FALLTHROUGH macro to do
the right thing if GNUC is defined; here we replace all of our "fall
through" comments with uses of that macro.
This is an automated commit, made with the following perl one-liner:
#!/usr/bin/perl -i -p
s#/\* *falls? ?thr.*?\*/#FALLTHROUGH;#i;
(In order to avoid conflicts, I'm applying this script separately to
each maint branch. This is the 0.4.1 version.)
GCC added an implicit-fallthrough warning a while back, where it
would complain if you had a nontrivial "case:" block that didn't end
with break, return, or something like that. Clang recently added
the same thing.
GCC, however, would let you annotate a fall-through as intended by
any of various magic "/* fall through */" comments. Clang, however,
only seems to like "__attribute__((fallthrough))". Fortunately, GCC
accepts that too.
A previous commit in this branch defined a FALLTHROUGH macro to do
the right thing if GNUC is defined; here we replace all of our "fall
through" comments with uses of that macro.
This is an automated commit, made with the following perl one-liner:
#!/usr/bin/perl -i -p
s#/\* *falls? ?thr.*?\*/#FALLTHROUGH;#i;
Move a series of function from config.c into that new file which is related to
address resolving.
Part of #33789
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit moves router_pick_published_address() and the related helper
functions into the new file.
The log_addr_has_changed() function has been made public in router.h so we can
use it in relay_resolve_addr.c.
This is a refactoring as part of Sponsor 55. Only code movement at this
commit.
Part of #33789
Signed-off-by: David Goulet <dgoulet@torproject.org>
Do not try to stuff "HS_DESC_DECODE_GENERIC_ERROR" (-1) into a
socks5_reply_status_t (enum). It doesn't actually make sense, and
isn't one of our documented extensions.
(This can only happen on a nonfatal assertion that we haven't seen,
so it probably isn't happening in practice.)
Fixes another case of bug 34077; bugfix on 0.4.3.1-alpha.
When a relay starts testing reachability, log its IPv6 ORPort.
The existing code logs the IPv4 ORPort and DirPort.
The IPv4 ORPort is required. The other ports are only logged if they
are present.
Part of 33222.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
check_whether_orport_reachable router_skip_orport_reachability_check \
check_whether_dirport_reachable router_skip_dirport_reachability_check
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.
Part of 33222.
Change some function names to distinguish between:
* client first hop reachability (ReachableAddresses)
* relay port reachability self-tests
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
router_skip_or_reachability router_connect_assume_or_reachable \
router_skip_dir_reachability router_connect_assume_dir_reachable
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.
Part of 33222.
Split "can extend over IPv6" and "has advertised IPv6 ORPort" into
separate functions. They currently have the same result, but this may
change in 33818 with ExtendAllowIPv6Addresses.
Part of 33817.
Refactor circuit_open_connection_for_extend(), splitting out the IP
address choice code into a new function.
Adds unit tests. No behaviour changes in tor.
Part of 33817.
Allow extend cells with IPv6-only link specifiers.
Warn and fail if both IPv4 and IPv6 are invalid.
Also warn if the IPv4 or IPv6 addresses are unexpectedly internal,
but continue with the valid address.
Part of 33817.
The client auth protocol allows attacker-controlled x25519 private keys being
passed around, which allows an attacker to potentially trigger the all-zeroes
assert for client_auth_sk in hs_descriptor.c:decrypt_descriptor_cookie().
We fixed that by making sure that an all-zeroes client auth key will not be
used.
There are no guidelines for validating x25519 private keys, and the assert was
there as a sanity check for code flow issues (we don't want to enter that
function with an unitialized key if client auth is being used). To avoid such
crashes in the future, we also changed the assert to a BUG-and-err.
circuit_extend() may be called when a client receives an extend cell,
even if the relay module is disabled.
Log a protocol warning when the relay module is disabled.
Part of 33633.
This is to allow a visual feedback in the logs for operators setting up Onion
Balance so they can confirm they properly configured the instances.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The ob_subcreds array was not copied after SIGHUP, and that left the
post-SIGHUP service with a NULL ob_subcreds pointer (until the next descriptor
gets build where we regenerate ob_subcreds in hs_ob_refresh_keys()).
Fixes bug #33762; not in any released tor version.
This patch doesn't actually use the results of the parsed object to
create the service configuration: subsequent patches will do
that. This patch just introduces the necessary configuration tables
and starts using them to validate the configuration.
As of this writing, this patch breaks tests. I'll likely fix that
in a rebase later on: the current error messages for failures to
parse options are a regression, so I've opened #33640 for that.
Return early when there is no suitable IPv6 ORPort.
Show the address and port on error, using a convenience function.
Code simplification and refactoring.
Cleanup after 32588.
When IPv6 ORPorts are set to "auto", tor relays and bridges would
advertise an incorrect port in their descriptor.
This may be a low-severity memory safety issue, because the published
port number may be derived from uninitialised or out-of-bounds memory
reads.
Fixes bug 32588; bugfix on 0.2.3.9-alpha.
Previously we just ignored this option, which would leave it unset,
and cause an assertion failure later on when running with the User
option.
Fixes bug 33668; bugfix on 0.4.3.1-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>
Given that ed25519 public key validity checks are usually not needed
and (so far) they are only necessary for onion addesses in the Tor
protocol, we decided to fix this specific bug instance without
modifying the rest of the codebase (see below for other fix
approaches).
In our minimal fix we check that the pubkey in
hs_service_add_ephemeral() is valid and error out otherwise.
When a service can not upload its descriptor(s), we have no logs on why. This
adds logging for each possible reason for each descriptors.
That logging is emitted every second so it is rate limited for each reason and
per descriptor.
Closes#33400
Signed-off-by: David Goulet <dgoulet@torproject.org>
In this case, when we're looking for the voting interval, we should
try looking at the _latest_ consensus if we have one. When we're
looking for the start of the current voting period, we can use our
existing fallback logic without complaint, since the voting interval
code will already have given us a reasonable voting interval, and we
want to have a round starting time based on the current time.
This is not as clean a patch as I would like: see the comment on
ASSUME_AUTHORITY_SCHEDULING. This issue here is that the unit tests
sometimes assume that we are going to be looking at the dirauth
options and behaving like a dirauth, but without setting the options
to turn is into one.
This isn't an issue for actually running Tor, as far as I can tell
with chutney.
Most of this function was about recreating a voting schedule on
demand if it didn't exist yet or was not up-to-date. I've made that
into its own function.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
voting_schedule_recalculate_timing dirauth_sched_recalculate_timing \
voting_schedule_get_start_of_next_interval voting_sched_get_start_of_interval_after \
voting_schedule_get_next_valid_after_time dirauth_sched_get_next_valid_after_time
Apparently it is only used by the unit tests: tor doesn't want it at
all.
I've opened a new ticket (33383) to we if we should remove this
whole feature.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
MIN_DNS_TTL_AT_EXIT MIN_DNS_TTL \
MAX_DNS_TTL_AT_EXIT MAX_DNS_TTL \
dns_clip_ttl clip_dns_ttl
If we receive via 'post' a vote from a dir auth after the
fetch_missing_votes cutoff, that means we didn't get it by the time we
begin the "fetching missing votes from everybody else" phase, which means
it is very likely to cause a consensus split if we count it. Instead,
we reject it.
But we still allow votes that we fetch ourselves after that cutoff.
This is a demo branch for making progress on #4631.
I've been running it on moria1 and it catches and handles real buggy
behavior from directory authorities, e.g.
Jan 28 15:59:50.804 [warn] Rejecting vote from 199.58.81.140 received at 2020-01-28 20:59:50; our cutoff for received votes is 2020-01-28 20:52:30
Jan 28 15:59:50.805 [warn] Rejected vote from 199.58.81.140 ("Vote received too late, would be dangerous to count it").
Jan 29 01:52:52.667 [warn] Rejecting vote from 204.13.164.118 received at 2020-01-29 06:52:52; our cutoff for received votes is 2020-01-29 06:52:30
Jan 29 01:52:52.669 [warn] Rejected vote from 204.13.164.118 ("Vote received too late, would be dangerous to count it").
Jan 29 04:53:26.323 [warn] Rejecting vote from 204.13.164.118 received at 2020-01-29 09:53:26; our cutoff for received votes is 2020-01-29 09:52:30
Jan 29 04:53:26.326 [warn] Rejected vote from 204.13.164.118 ("Vote received too late, would be dangerous to count it").
Add doxygen comments to the new recommended and required subprotocol
version strings.
Add a warning to the required protocol documentation, because requiring
the wrong protocols can break the tor network. Also reference
proposal 303: When and how to remove support for protocol versions.
Part of 33285.
Move the recommended and required protocol version lists into the
private section of the dirvote header, and turn them into macros.
Preparation for 33285.
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>
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.)
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>
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.
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>
* 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
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.
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>
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.
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.
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.
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.
All of these files contain "*.h", except for:
* src/app/config/.may_include
* src/test/.may_include
which also contain "*.inc".
This change prevents includes of "*.c" files, and other
unusually named files.
Part of 32609.
- Remove key_dir which is useless.
- Kill an indentation layer.
We want to make it cleaner and slimmer so that we can reuse parts of it in the
REMOVE command for removing the right client auth file.
Because the function that parses client auth credentials saved on
disk (parse_auth_file_content()) is not future compatible, there is no way to
add support for storing the nickname on the disk. Hence, nicknames cannot
persist after Tor restart making them pretty much useless.
In the future we can introduce nicknames by adding a new file format for client
auth credentials, but this was not deemed worth doing at this stage.
- See hs_client_register_auth_credentials() for the entry point.
- Also set the permanent flag for credentials we read from the filesystem.
- Also add some missing documentation.
Since the removal of ip->circuit_established, this function does litterally
nothing so clean it up.
Part of #32020
Signed-off-by: David Goulet <dgoulet@torproject.org>
By centralizing the circuit cleanup type that is: on close, free and
repurpose, some actions on the circuit can not happen for a certain cleanup
type or for all types.
This passes a cleanup type so the HS subsystem (v2 and v3) can take actions
based on the type of cleanup.
For instance, there is slow code that we do not run on a circuit close but
rather only on free.
Part of #32020
Signed-off-by: David Goulet <dgoulet@torproject.org>
Report back to the v3 subsystem any introduction point client circuit failure
so they can be noted down in the failure cache.
Fixes#32020
Signed-off-by: David Goulet <dgoulet@torproject.org>
Old and messy code path. Structure it in a more pleasant and readable way. No
behavior change with this refactor.
Part of #32020
Signed-off-by: David Goulet <dgoulet@torproject.org>
The FP_ identifiers referred to fingerprints, but they also applied
to address ranges. The router_status_t name invited confusion with
routerstasus_t. Fixes ticket 29826.
This is an automated commit, generated by this command:
./scripts/maint/rename_c_identifier.py \
router_status_t rtr_flags_t \
FP_INVALID RTR_INVALID \
FP_BADEXIT RTR_BADEXIT \
FP_REJECT RTR_REJECT
Refactor to decomplexify circuit_about_to_free() and finally have one single
entry point into the HS subsystems (v2 and v3) for when a circuit is freed.
With this, hs_circ_cleanup() becomes the one and only entry point when a
circuit is freed which then routes to the right subsystem version for any
actions to be taken.
This moves a big chunk of code from circuituse.c to rendclient.c. No behavior
change. Next commit will refactor it to reduce our technical debt.
Part of #32020
Signed-off-by: David Goulet <dgoulet@torproject.org>
We now keep descriptor that we can't decode due to missing client
authorization in the cache.
This new function is used when new client authorization are added and to tell
the client cache to retry decoding.
Part of #30382
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit extract most of the code that dirclient.c had to handle the end of
a descriptor directory requests (fetch). It is moved into hs_client.c in order
to have one single point of entry and the rest is fully handled by the HS
subsystem.
As part of #30382, depending on how the descriptor ended up stored (decoded or
not), different SOCKS error code can be returned.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This will allow us to callback into the HS subsytem depending on the decoding
status and return an extended SOCKS5 error code depending on the decoding
issue.
This is how we'll be able to tell the SocksPort connection if we are missing
or have bad client authorization for a service.
Part of #30382
Signed-off-by: David Goulet <dgoulet@torproject.org>
We now keep the descriptor in the cache, obviously not decoded, if it can't be
decrypted for which we believe client authorization is missing or unusable
(bad).
This way, it can be used later once the client authorization are added or
updated.
Part of #30382
Signed-off-by: David Goulet <dgoulet@torproject.org>
Using a standard ending here will let other tools that expect
markdown understand our output here.
This commit was automatically generated with:
for fn in $(find src -name '*.dox'); do \
git mv "$fn" "${fn%.dox}.md"; \
done
