This test works by having two post-loop events activate one another
in a tight loop. If the "post-loop" mechanism didn't work, this
would be enough to starve all other events.
A linked connection_t is one that gets its I/O, not from the
network, but from another connection_t. When such a connection has
something to write, we want the corresponding connection to run its
read callback ... but not immediately, to avoid infinite recursion
and/or event loop starvation.
Previously we handled this case by activating the read events
outside the event loop. Now we use the "postloop event" logic.
This lets us simplify do_main_loop_once() a little.
We've been labeling some events as happening "outside the event
loop", to avoid Libevent starvation. This patch provides a cleaner
mechanism to avoid that starvation.
For background, the problem here is that Libevent only scans for new
events once it has run all its active callbacks. So if the
callbacks keep activating new callbacks, they could potentially
starve Libevent indefinitely and keep it from ever checking for
timed, socket, or signal events.
To solve this, we add the ability to label some events as
"post-loop". The rule for a "post-loop" event is that any events
_it_ activates can only be run after libevent has re-scanned for new
events at least once.
This differs from our previous token bucket abstraction in a few
ways:
1) It is an abstraction, and not a collection of fields.
2) It is meant to be used with monotonic timestamps, which should
produce better results than calling gettimeofday over and over.
In d1874b4339, we adjusted this check so that we insist on
using routerinfos for bridges. That's almost correct... but if we
have a bridge that is also a regular relay, then we should use
insist on its routerinfo when connecting to it as a bridge
(directly), and be willing to use its microdescriptor when
connecting to it elsewhere in our circuits.
This bug is a likely cause of some (all?) of the (exit_ei == NULL)
failures we've been seeing.
Fixes bug 25691; bugfix on 0.3.3.4-alpha
When size_t is 32 bits, the unit tests can't fit anything more than
4GB-1 into a size_t.
Additionally, tt_int_op() uses "long" -- we need tt_u64_op() to
safely test uint64_t values for equality.
Bug caused by tests for #24782 fix; not in any released Tor.
This patch changes the algorithm of compute_real_max_mem_in_queues() to
use 0.4 * RAM iff the system has more than or equal to 8 GB of RAM, but
will continue to use the old value of 0.75 * RAM if the system have less
than * GB of RAM available.
This patch also adds tests for compute_real_max_mem_in_queues().
See: https://bugs.torproject.org/24782
This patch makes get_total_system_memory mockable, which allows us to
alter the return value of the function in tests.
See: https://bugs.torproject.org/24782
This one happens if for some reason you start with DirPort enabled
but server mode turned off entirely.
Fixes a case of bug 23693; bugfix on 0.3.1.1-alpha.
This roughly doubles our test coverage of the bridges.c module.
* ADD new testing module, .../src/test/test_bridges.c.
* CHANGE a few function declarations from `static` to `STATIC`.
* CHANGE one function in transports.c, transport_get_by_name(), to be
mockable.
* CLOSES#25425: https://bugs.torproject.org/25425
This patch lifts the list of default directory authorities from config.c
into their own auth_dirs.inc file, which is then included in config.c
using the C preprocessor.
Patch by beastr0.
See: https://bugs.torproject.org/24854
* ADD new /src/common/crypto_rand.[ch] module.
* ADD new /src/common/crypto_util.[ch] module (contains the memwipe()
function, since all crypto_* modules need this).
* FIXES part of #24658: https://bugs.torproject.org/24658
This module doesn't actually need to mock the libevent mainloop at
all: it can just use the regular mainloop that the test environment
sets up.
Part of ticket 23750.
This change makes cpuworker and test_workqueue no longer need to
include event2/event.h. Now workqueue.c needs to include it, but
that is at least somewhat logical here.
We are going to stop recommending 0.2.5 so there is no reason to keep the
undef statement anymore.
Fixes#20522.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Both in geoip_note_client_seen() and options_need_geoip_info(), switch from
accessing the options directly to using the should_record_bridge_info() helper
function.
Fixes#25290
Signed-off-by: David Goulet <dgoulet@torproject.org>
Next commit is addressing the circuit queue cell limit so cleanup before doing
anything else.
Part of #25226
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied
in Rust to the maximum number of version (total, for all subprotocols).
Whereas in C, it was being applied to the number of subprotocols that were
allowed. This changes the Rust to match C's behaviour.
The behaviours still do not match, unsurprisingly, but now we know where a
primary difference is: the Rust is validating version ranges more than the C,
so in the C it's possible to call protover_all_supported on a ridiculous
version range like "Sleen=0-4294967294" because the C uses
MAX_PROTOCOLS_TO_EXPAND to count the number of *subprotocols* whereas the Rust
uses it to count the total number of *versions* of all subprotocols.
There's now no difference in these tests w.r.t. the C or Rust: both
fail miserably (well, Rust fails with nice descriptive errors, and C
gives you a traceback, because, well, C).
The DoS potential is slightly higher in C now due to some differences to the
Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs.
Also, the comment about "failing at the splitting stage" in Rust wasn't true,
since when we split, we ignore empty chunks (e.g. "1--1" parses into
"(1,None),(None,1)" and "None" can't be parsed into an integer).
Finally, the comment about "Rust seems to experience an internal error" is only
true in debug mode, where u32s are bounds-checked at runtime. In release mode,
code expressing the equivalent of this test will error with
`Err(ProtoverError::Unparseable)` because 4294967295 is too large.
Previously, if "Link=1-5" was supported, and you asked protover_all_supported()
(or protover::all_supported() in Rust) if it supported "Link=3-999", the C
version would return "Link=3-999" and the Rust would return "Link=6-999". These
both behave the same now, i.e. both return "Link=6-999".
During code review and discussion with Chelsea Komlo, she pointed out
that protover::compute_for_old_tor() was a public function whose
return type was `&'static CStr`. We both agree that C-like parts of
APIs should:
1. not be exposed publicly (to other Rust crates),
2. only be called in the appropriate FFI code,
3. not expose types which are meant for FFI code (e.g. `*mut char`,
`CString`, `*const c_int`, etc.) to the pure-Rust code of other
crates.
4. FFI code (e.g. things in `ffi.rs` modules) should _never_ be called
from pure-Rust, not even from other modules in its own crate
(i.e. do not call `protover::ffi::*` from anywhere in
`protover::protoset::*`, etc).
With that in mind, this commit makes the following changes:
* CHANGE `protover::compute_for_old_tor()` to be
visible only at the `pub(crate)` level.
* RENAME `protover::compute_for_old_tor()` to
`protover::compute_for_old_tor_cstr()` to reflect the last change.
* ADD a new `protover::compute_for_old_tor()` function wrapper which
is public and intended for other Rust code to use, which returns a
`&str`.
It was changed to take borrows instead of taking ownership.
* REFACTOR `protover::ffi::protover_is_supported_here()` to use changed method
signature on `protover::is_supported_here()`.
This includes a subtle difference in behaviour to the previous Rust
implementation, where, for each vote that we're computing over, if a single one
fails to parse, we skip it. This now matches the current behaviour in the C
implementation.
* REFACTOR `protover::ffi::protover_compute_vote()` to use
new types and methods.
This includes a subtle difference in behaviour, as in 4258f1e18, where we return
(matching the C impl's return behaviour) earlier than before if parsing failed,
saving us computation in parsing the versions into a
protover::protoset::ProtoSet.
* REFACTOR `protover::ffi::protover_list_supports_protocol_or_later()` to use
new types and methods.
This includes a subtle difference in behaviour, as in 4258f1e18, where we return
(matching the C impl's return behaviour) earlier than before if parsing failed,
saving us computation in parsing the versions into a
protover::protoset::ProtoSet.
* REFACTOR `protover::ffi::protover_list_supports_protocol()` to use new types
and methods.
This includes differences in behaviour to before, which should now more closely
match the C version:
- If parsing a protover `char*` from C, and the string is not parseable, this
function will return 1 early, which matches the C behaviour when protocols
are unparseable. Previously, we would parse it and its version numbers
simultaneously, i.e. there was no fail early option, causing us to spend more
time unnecessarily parsing versions.
* REFACTOR `protover::ffi::protover_all_supported()` to use new types and
methods.
Previously, the rust implementation of protover considered an empty string to be
a valid ProtoEntry, while the C version did not (it must have a "=" character).
Other differences include that unknown protocols must now be parsed as
`protover::UnknownProtocol`s, and hence their entries as
`protover::UnvalidatedProtoEntry`s, whereas before (nearly) all protoentries
could be parsed regardless of how erroneous they might be considered by the C
version.
My apologies for this somewhat messy and difficult to read commit, if any part
is frustrating to the reviewer, please feel free to ask me to split this into
smaller changes (possibly hard to do, since so much changed), or ask me to
comment on a specific line/change and clarify how/when the behaviours differ.
The tests here should more closely match the behaviours exhibited by the C
implementation, but I do not yet personally guarantee they match precisely.
* REFACTOR unittests in protover::protover.
* ADD new integration tests for previously untested behaviour.
* FIXES part of #24031: https://bugs.torproject.org/24031.
This changes `protover::is_supported_here()` to be aware of new datatypes
(e.g. don't call `.0` on things which are no longer tuple structs) and also
changes the method signature to take borrows, making it faster, threadable, and
easier to read (i.e. the caller can know from reading the function signature
that the function won't mutate values passed into it).
* CHANGE the `protover::is_supported_here()` function to take borrows.
* REFACTOR the `protover::is_supported_here()` function to be aware of new
datatypes.
* FIXES part of #24031: https://bugs.torproject.org/24031
This adds a new type for votes upon `protover::ProtoEntry`s (technically, on
`protover::UnvalidatedProtoEntry`s, because the C code does not validate based
upon currently known protocols when voting, in order to maintain
future-compatibility), and converts several functions which would have operated
on this datatype into methods for ease-of-use and readability.
This also fixes a behavioural differentce to the C version of
protover_compute_vote(). The C version of protover_compute_vote() calls
expand_protocol_list() which checks if there would be too many subprotocols *or*
expanded individual version numbers, i.e. more than MAX_PROTOCOLS_TO_EXPAND, and
does this *per vote* (but only in compute_vote(), everywhere else in the C seems
to only care about the number of subprotocols, not the number of individual
versions). We need to match its behaviour in Rust and ensure we're not allowing
more than it would to get the votes to match.
* ADD new `protover::ProtoverVote` datatype.
* REMOVE the `protover::compute_vote()` function and refactor it into an
equivalent-in-behaviour albeit more memory-efficient voting algorithm based
on the new underlying `protover::protoset::ProtoSet` datatype, as
`ProtoverVote::compute()`.
* REMOVE the `protover::write_vote_to_string()` function, since this
functionality is now generated by the impl_to_string_for_proto_entry!() macro
for both `ProtoEntry` and `UnvalidatedProtoEntry` (the latter of which is the
correct type to return from a voting protocol instance, since the entity
voting may not know of all protocols being voted upon or known about by other
voting parties).
* FIXES part of #24031: https://bugs.torproject.org/24031
rust: Fix a difference in compute_vote() behaviour to C version.
This implements conversions from either a ProtoEntry or an UnvalidatedProtoEntry
into a String, for use in replacing such functions as
`protover::write_vote_to_string()`.
* ADD macro for implementing ToString trait for ProtoEntry and
UnvalidatedProtoEntry.
* FIXES part of #24031: https://bugs.torproject.org/24031
This adds a new protover::UnvalidatedProtoEntry type, which is the
UnknownProtocol variant of a ProtoEntry, and refactors several functions which
should operate on this type into methods.
This also fixes what was previously another difference to the C implementation:
if you asked the C version of protovet_compute_vote() to compute a single vote
containing "Fribble=", it would return NULL. However, the Rust version would
return "Fribble=" since it didn't check if the versions were empty before
constructing the string of differences. ("Fribble=" is technically a valid
protover string.) This is now fixed, and the Rust version in that case will,
analogous to (although safer than) C returning a NULL, return None.
* REMOVE internal `contains_only_supported_protocols()` function.
* REMOVE `all_supported()` function and refactor it into
`UnvalidatedProtoEntry::all_supported()`.
* REMOVE `parse_protocols_from_string_with_no_validation()` and
refactor it into the more rusty implementation of
`impl FromStr for UnvalidatedProtoEntry`.
* REMOVE `protover_string_supports_protocol()` and refactor it into
`UnvalidatedProtoEntry::supports_protocol()`.
* REMOVE `protover_string_supports_protocol_or_later()` and refactor
it into `UnvalidatedProtoEntry::supports_protocol_or_later()`.
* FIXES part of #24031: https://bugs.torproject.org/24031
rust: Fix another C/Rust different in compute_vote().
This fixes the unittest from the prior commit by checking if the versions are
empty before adding a protocol to a vote.
This replaces the `protover::SupportedProtocols` (why would you have a type just
for things which are supported?) with a new, more generic type,
`protover::ProtoEntry`, which utilises the new, more memory-efficient datatype
in protover::protoset.
* REMOVE `get_supported_protocols()` and `SupportedProtocols::tor_supported()`
(since they were never used separately) and collapse their functionality into
a single `ProtoEntry::supported()` method.
* REMOVE `SupportedProtocols::from_proto_entries()` and reimplement its
functionality as the more rusty `impl FromStr for ProtoEntry`.
* REMOVE `get_proto_and_vers()` function and refactor it into the more rusty
`impl FromStr for ProtoEntry`.
* FIXES part of #24031: https://bugs.torproject.org/24031
* ADD new type, protover::UnknownProtocol, so that we have greater type safety
and our protover functionality which works with unsanitised protocol names is
more clearly demarcated.
* REFACTOR protover::Proto, renaming it protover::Protocol to mirror the new
protover::UnknownProtocol type name.
* ADD a utility conversion of `impl From<Protocol> for UnknownProtocol` so that
we can easily with known protocols and unknown protocols simultaneously
(e.g. doing comparisons, checking their version numbers), while not allowing
UnknownProtocols to be accidentally used in functions which should only take
Protocols.
* FIXES part of #24031: https://bugs.torproject.org/24031
* ADD new protover::protoset module.
* ADD new protover::protoset::ProtoSet class for holding protover versions.
* REMOVE protover::Versions type implementation and its method
`from_version_string()`, and instead implement this behaviour in a more
rust-like manner as `impl FromStr for ProtoSet`.
* MOVE the `find_range()` utility function from protover::protover to
protover::protoset since it's only used internally in the
implementation of ProtoSet.
* REMOVE the `contract_protocol_list()` function from protover::protover and
instead refactor it (reusing nearly the entire thing, with minor superficial,
i.e. non-behavioural, changes) into a more rusty
`impl ToString for ProtoSet`.
* REMOVE the `expand_version_range()` function from protover::protover and
instead refactor it into a more rusty implementation of
`impl Into<Vec<Version>> for ProtoSet` using the new error types in
protover::errors.
* FIXES part of #24031: https://bugs.torproject.org/24031.
This will allow us to do actual error handling intra-crate in a more
rusty manner, e.g. propogating errors in match statements, conversion
between error types, logging messages, etc.
* FIXES part of #24031: https://bugs.torproject.org/24031.
Previously, the limit for MAX_PROTOCOLS_TO_EXPAND was actually being applied
in Rust to the maximum number of version (total, for all subprotocols).
Whereas in C, it was being applied to the number of subprotocols that were
allowed. This changes the Rust to match C's behaviour.
The behaviours still do not match, unsurprisingly, but now we know where a
primary difference is: the Rust is validating version ranges more than the C,
so in the C it's possible to call protover_all_supported on a ridiculous
version range like "Sleen=0-4294967294" because the C uses
MAX_PROTOCOLS_TO_EXPAND to count the number of *subprotocols* whereas the Rust
uses it to count the total number of *versions* of all subprotocols.
There's now no difference in these tests w.r.t. the C or Rust: both
fail miserably (well, Rust fails with nice descriptive errors, and C
gives you a traceback, because, well, C).
The DoS potential is slightly higher in C now due to some differences to the
Rust code, see the C_RUST_DIFFERS tags in src/rust/protover/tests/protover.rs.
Also, the comment about "failing at the splitting stage" in Rust wasn't true,
since when we split, we ignore empty chunks (e.g. "1--1" parses into
"(1,None),(None,1)" and "None" can't be parsed into an integer).
Finally, the comment about "Rust seems to experience an internal error" is only
true in debug mode, where u32s are bounds-checked at runtime. In release mode,
code expressing the equivalent of this test will error with
`Err(ProtoverError::Unparseable)` because 4294967295 is too large.
Previously, if "Link=1-5" was supported, and you asked protover_all_supported()
(or protover::all_supported() in Rust) if it supported "Link=3-999", the C
version would return "Link=3-999" and the Rust would return "Link=6-999". These
both behave the same now, i.e. both return "Link=6-999".
During code review and discussion with Chelsea Komlo, she pointed out
that protover::compute_for_old_tor() was a public function whose
return type was `&'static CStr`. We both agree that C-like parts of
APIs should:
1. not be exposed publicly (to other Rust crates),
2. only be called in the appropriate FFI code,
3. not expose types which are meant for FFI code (e.g. `*mut char`,
`CString`, `*const c_int`, etc.) to the pure-Rust code of other
crates.
4. FFI code (e.g. things in `ffi.rs` modules) should _never_ be called
from pure-Rust, not even from other modules in its own crate
(i.e. do not call `protover::ffi::*` from anywhere in
`protover::protoset::*`, etc).
With that in mind, this commit makes the following changes:
* CHANGE `protover::compute_for_old_tor()` to be
visible only at the `pub(crate)` level.
* RENAME `protover::compute_for_old_tor()` to
`protover::compute_for_old_tor_cstr()` to reflect the last change.
* ADD a new `protover::compute_for_old_tor()` function wrapper which
is public and intended for other Rust code to use, which returns a
`&str`.
It was changed to take borrows instead of taking ownership.
* REFACTOR `protover::ffi::protover_is_supported_here()` to use changed method
signature on `protover::is_supported_here()`.
This includes a subtle difference in behaviour to the previous Rust
implementation, where, for each vote that we're computing over, if a single one
fails to parse, we skip it. This now matches the current behaviour in the C
implementation.
* REFACTOR `protover::ffi::protover_compute_vote()` to use
new types and methods.
This includes a subtle difference in behaviour, as in 4258f1e18, where we return
(matching the C impl's return behaviour) earlier than before if parsing failed,
saving us computation in parsing the versions into a
protover::protoset::ProtoSet.
* REFACTOR `protover::ffi::protover_list_supports_protocol_or_later()` to use
new types and methods.
This includes a subtle difference in behaviour, as in 4258f1e18, where we return
(matching the C impl's return behaviour) earlier than before if parsing failed,
saving us computation in parsing the versions into a
protover::protoset::ProtoSet.
* REFACTOR `protover::ffi::protover_list_supports_protocol()` to use new types
and methods.
This includes differences in behaviour to before, which should now more closely
match the C version:
- If parsing a protover `char*` from C, and the string is not parseable, this
function will return 1 early, which matches the C behaviour when protocols
are unparseable. Previously, we would parse it and its version numbers
simultaneously, i.e. there was no fail early option, causing us to spend more
time unnecessarily parsing versions.
* REFACTOR `protover::ffi::protover_all_supported()` to use new types and
methods.
Previously, the rust implementation of protover considered an empty string to be
a valid ProtoEntry, while the C version did not (it must have a "=" character).
Other differences include that unknown protocols must now be parsed as
`protover::UnknownProtocol`s, and hence their entries as
`protover::UnvalidatedProtoEntry`s, whereas before (nearly) all protoentries
could be parsed regardless of how erroneous they might be considered by the C
version.
My apologies for this somewhat messy and difficult to read commit, if any part
is frustrating to the reviewer, please feel free to ask me to split this into
smaller changes (possibly hard to do, since so much changed), or ask me to
comment on a specific line/change and clarify how/when the behaviours differ.
The tests here should more closely match the behaviours exhibited by the C
implementation, but I do not yet personally guarantee they match precisely.
* REFACTOR unittests in protover::protover.
* ADD new integration tests for previously untested behaviour.
* FIXES part of #24031: https://bugs.torproject.org/24031.
This changes `protover::is_supported_here()` to be aware of new datatypes
(e.g. don't call `.0` on things which are no longer tuple structs) and also
changes the method signature to take borrows, making it faster, threadable, and
easier to read (i.e. the caller can know from reading the function signature
that the function won't mutate values passed into it).
* CHANGE the `protover::is_supported_here()` function to take borrows.
* REFACTOR the `protover::is_supported_here()` function to be aware of new
datatypes.
* FIXES part of #24031: https://bugs.torproject.org/24031
This adds a new type for votes upon `protover::ProtoEntry`s (technically, on
`protover::UnvalidatedProtoEntry`s, because the C code does not validate based
upon currently known protocols when voting, in order to maintain
future-compatibility), and converts several functions which would have operated
on this datatype into methods for ease-of-use and readability.
This also fixes a behavioural differentce to the C version of
protover_compute_vote(). The C version of protover_compute_vote() calls
expand_protocol_list() which checks if there would be too many subprotocols *or*
expanded individual version numbers, i.e. more than MAX_PROTOCOLS_TO_EXPAND, and
does this *per vote* (but only in compute_vote(), everywhere else in the C seems
to only care about the number of subprotocols, not the number of individual
versions). We need to match its behaviour in Rust and ensure we're not allowing
more than it would to get the votes to match.
* ADD new `protover::ProtoverVote` datatype.
* REMOVE the `protover::compute_vote()` function and refactor it into an
equivalent-in-behaviour albeit more memory-efficient voting algorithm based
on the new underlying `protover::protoset::ProtoSet` datatype, as
`ProtoverVote::compute()`.
* REMOVE the `protover::write_vote_to_string()` function, since this
functionality is now generated by the impl_to_string_for_proto_entry!() macro
for both `ProtoEntry` and `UnvalidatedProtoEntry` (the latter of which is the
correct type to return from a voting protocol instance, since the entity
voting may not know of all protocols being voted upon or known about by other
voting parties).
* FIXES part of #24031: https://bugs.torproject.org/24031
rust: Fix a difference in compute_vote() behaviour to C version.
This implements conversions from either a ProtoEntry or an UnvalidatedProtoEntry
into a String, for use in replacing such functions as
`protover::write_vote_to_string()`.
* ADD macro for implementing ToString trait for ProtoEntry and
UnvalidatedProtoEntry.
* FIXES part of #24031: https://bugs.torproject.org/24031
This adds a new protover::UnvalidatedProtoEntry type, which is the
UnknownProtocol variant of a ProtoEntry, and refactors several functions which
should operate on this type into methods.
This also fixes what was previously another difference to the C implementation:
if you asked the C version of protovet_compute_vote() to compute a single vote
containing "Fribble=", it would return NULL. However, the Rust version would
return "Fribble=" since it didn't check if the versions were empty before
constructing the string of differences. ("Fribble=" is technically a valid
protover string.) This is now fixed, and the Rust version in that case will,
analogous to (although safer than) C returning a NULL, return None.
* REMOVE internal `contains_only_supported_protocols()` function.
* REMOVE `all_supported()` function and refactor it into
`UnvalidatedProtoEntry::all_supported()`.
* REMOVE `parse_protocols_from_string_with_no_validation()` and
refactor it into the more rusty implementation of
`impl FromStr for UnvalidatedProtoEntry`.
* REMOVE `protover_string_supports_protocol()` and refactor it into
`UnvalidatedProtoEntry::supports_protocol()`.
* REMOVE `protover_string_supports_protocol_or_later()` and refactor
it into `UnvalidatedProtoEntry::supports_protocol_or_later()`.
* FIXES part of #24031: https://bugs.torproject.org/24031
rust: Fix another C/Rust different in compute_vote().
This fixes the unittest from the prior commit by checking if the versions are
empty before adding a protocol to a vote.
This replaces the `protover::SupportedProtocols` (why would you have a type just
for things which are supported?) with a new, more generic type,
`protover::ProtoEntry`, which utilises the new, more memory-efficient datatype
in protover::protoset.
* REMOVE `get_supported_protocols()` and `SupportedProtocols::tor_supported()`
(since they were never used separately) and collapse their functionality into
a single `ProtoEntry::supported()` method.
* REMOVE `SupportedProtocols::from_proto_entries()` and reimplement its
functionality as the more rusty `impl FromStr for ProtoEntry`.
* REMOVE `get_proto_and_vers()` function and refactor it into the more rusty
`impl FromStr for ProtoEntry`.
* FIXES part of #24031: https://bugs.torproject.org/24031
* ADD new type, protover::UnknownProtocol, so that we have greater type safety
and our protover functionality which works with unsanitised protocol names is
more clearly demarcated.
* REFACTOR protover::Proto, renaming it protover::Protocol to mirror the new
protover::UnknownProtocol type name.
* ADD a utility conversion of `impl From<Protocol> for UnknownProtocol` so that
we can easily with known protocols and unknown protocols simultaneously
(e.g. doing comparisons, checking their version numbers), while not allowing
UnknownProtocols to be accidentally used in functions which should only take
Protocols.
* FIXES part of #24031: https://bugs.torproject.org/24031
* ADD new protover::protoset module.
* ADD new protover::protoset::ProtoSet class for holding protover versions.
* REMOVE protover::Versions type implementation and its method
`from_version_string()`, and instead implement this behaviour in a more
rust-like manner as `impl FromStr for ProtoSet`.
* MOVE the `find_range()` utility function from protover::protover to
protover::protoset since it's only used internally in the
implementation of ProtoSet.
* REMOVE the `contract_protocol_list()` function from protover::protover and
instead refactor it (reusing nearly the entire thing, with minor superficial,
i.e. non-behavioural, changes) into a more rusty
`impl ToString for ProtoSet`.
* REMOVE the `expand_version_range()` function from protover::protover and
instead refactor it into a more rusty implementation of
`impl Into<Vec<Version>> for ProtoSet` using the new error types in
protover::errors.
* FIXES part of #24031: https://bugs.torproject.org/24031.
This will allow us to do actual error handling intra-crate in a more
rusty manner, e.g. propogating errors in match statements, conversion
between error types, logging messages, etc.
* FIXES part of #24031: https://bugs.torproject.org/24031.
When a relay is collecting internal statistics about how many
create cell requests it has seen of each type, accurately count the
requests from relays that temporarily fall out of the consensus.
(To be extra conservative, we were already ignoring requests from clients
in our counts, and we continue ignoring them here.)
Fixes bug 24910; bugfix on 0.2.4.17-rc.
Directory authorities no longer vote in favor of the Guard flag
for relays that don't advertise directory support.
Starting in Tor 0.3.0.1-alpha, Tor clients have been avoiding using
such relays in the Guard position, leading to increasingly broken load
balancing for the 5%-or-so of Guards that don't advertise directory
support.
Fixes bug 22310; bugfix on 0.3.0.6.
Add a missing lock acquisition around access to queued_control_events
in control_free_all(). Use the reassign-and-unlock strategy as in
queued_events_flush_all(). Fixes bug 25675. Coverity found this bug,
but only after we recently added an access to
flush_queued_event_pending.
The earlier checks in this function should ensure that components is
always nonempty. But in case somebody messes with them in the
future, let's add an extra check to make sure we aren't crashing.
Coverity found a null pointer reference in nodelist_add_microdesc().
This is almost certainly impossible assuming that the routerstatus_t
returned by router_get_consensus_status_by_descriptor_digest() always
corresponds to an entry in the nodelist. Fixes bug 25629.
Coverity found a null pointer reference in nodelist_add_microdesc().
This is almost certainly impossible assuming that the routerstatus_t
returned by router_get_consensus_status_by_descriptor_digest() always
corresponds to an entry in the nodelist. Fixes bug 25629.
Coverity found a null pointer reference in nodelist_add_microdesc().
This is almost certainly impossible assuming that the routerstatus_t
returned by router_get_consensus_status_by_descriptor_digest() always
corresponds to an entry in the nodelist. Fixes bug 25629.
If we failed to connect at the TCP level to a relay, note it down and refuse
to connect again for another 60 seconds.
Fixes#24767
Signed-off-by: David Goulet <dgoulet@torproject.org>
This update is needed to make it consistent with the behavior of
node_awaiting_ipv6(), which doesn't believe in the addresses from
routerinfos unless it actually plans to use those routerinfos.
Fixes bug 25213; bugfix on b66b62fb75 in 0.3.3.1-alpha,
which tightened up the definition of node_awaiting_ipv6().
There was a nonfatal assertion in pathbias_should_count that would
trigger if onehop_tunnel was set, but the desired_path_length was
greater than 1. This patch fixes that. Fixes bug 24903; bugfix on
0.2.5.2-alpha.
These tests handle incoming and outgoing cells on a three-hop
circuit, and make sure that the crypto works end-to-end. They don't
yet test spec conformance, leaky-pipe, or various error cases.
Make sure we actually only report client channel to the geoip cache instead of
looking if it is a known relay. Looking if it is a known relay can be
unreliable because they come and go from the consensus.
Fixes#24904
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because of #25306 for which we are unable to reproduce nor understand how it
is possible, this commit removes the asserts() and BUG() on the missing
descriptors instead when rotating them.
This allows us to log more data on error but also to let tor recover
gracefully instead of dying.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This patch adds some additional logging to circuits_handle_oom() to give
us more information about which specific compression backend that is
using a certain amount of memory.
See: https://bugs.torproject.org/25372
These declarations need to exist unconditionally, but they were
trapped inside an "#else /* !(defined(HAVE_SYSLOG_H)) */" block.
Fixes a travis regression caused by 23881; bug not in any released tor.
Additionally, this change extracts the functions that created and
freed these elements.
These structures had common "forward&reverse stream&digest"
elements, but they were initialized and freed through cpath objects,
and different parts of the code depended on them. Now all that code
is extacted, and kept in relay_crypto.c
This should help us improve modularity, and should also make it
easier for people to experiment with other relay crypto strategies
down the road.
This commit is pure function movement.
This function is used upon receiving a cell, and only handles the
decrypting part. The encryption part is currently handled inside
circuit_package_relay_cell.
This should avoid most intermittent test failures on developer and CI machines,
but there could (and probably should) be a more elegant solution.
Also, this test was testing that the IP was created and its expiration time was
set to a time greater than or equal to `now+INTRO_POINT_LIFETIME_MIN_SECONDS+5`:
/* Time to expire MUST also be in that range. We add 5 seconds because
* there could be a gap between setting now and the time taken in
* service_intro_point_new. On ARM, it can be surprisingly slow... */
tt_u64_op(ip->time_to_expire, OP_GE,
now + INTRO_POINT_LIFETIME_MIN_SECONDS + 5);
However, this appears to be a typo, since, according to the comment above it,
adding five seconds was done because the IP creation can be slow on some
systems. But the five seconds is added to the *minimum* time we're comparing
against, and so it actually functions to make this test *more* likely to fail on
slower systems. (It should either subtract five seconds, or instead add it to
time_to_expire.)
* FIXES#25450: https://bugs.torproject.org/25450
These were meant to demonstrate old behavior, or old rust behavior.
One of them _should_ work in Rust, but won't because of
implementation details. We'll fix that up later.
The C code and the rust code had different separate integer overflow
bugs here. That suggests that we're better off just forbidding this
pathological case.
Also, add tests for expected behavior on receiving a bad protocol
list in a consensus.
Fixes another part of 25249.
I've refactored these to be a separate function, to avoid tricky
merge conflicts.
Some of these are disabled with "XXXX" comments; they should get
fixed moving forward.
This one can only be exploited if you can generate a correctly
signed consensus, so it's not as bad as 25074.
Fixes bug 25251; also tracked as TROVE-2018-004.
In some cases we had checked for it, but in others we had not. One
of these cases could have been used to remotely cause
denial-of-service against directory authorities while they attempted
to vote.
Fixes TROVE-2018-001.
* ADD includes for "torint.h" and "container.h" to crypto_digest.h.
* ADD includes for "crypto_digest.h" to a couple places in which
crypto_digest_t was then missing.
* FIXES part of #24658: https://bugs.torproject.org/24658#comment:30
Folks have found two in the past week or so; we may as well fix the
others.
Found with:
\#!/usr/bin/python3
import re
def findMulti(fname):
includes = set()
with open(fname) as f:
for line in f:
m = re.match(r'^\s*#\s*include\s+["<](\S+)[>"]', line)
if m:
inc = m.group(1)
if inc in includes:
print("{}: {}".format(fname, inc))
includes.add(m.group(1))
import sys
for fname in sys.argv[1:]:
findMulti(fname)
We moved the crypto_pk_obselete_* functions into crypto_rsa.[ch] because they fit
better with the RSA module.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
since all it does is produce false positives
this commit should get merged into 0.2.9 and 0.3.0 *and* 0.3.1, even
though the code in the previous commit is already present in 0.3.1. sorry
for the mess.
[Cherry-picked]
since all it does is produce false positives
this commit should get merged into 0.2.9 and 0.3.0 *and* 0.3.1, even
though the code in the previous commit is already present in 0.3.1. sorry
for the mess.
This commit takes a piece of commit af8cadf3a9 and a piece of commit
46fe353f25, with the goal of making channel_is_client() be based on what
sort of connection handshake the other side used, rather than seeing
whether the other side ever sent a create_fast cell to us.
We moved the crypto_pk_* digest functions into crypto_rsa.[ch] because they fit
better with the RSA module.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Also correct MAX_VERSIONS_TO_EXPAND to match the C.
NOTE that this patch leads to incorrect behavior: the C code allows
huge ranges; it just doesn't allow votes on them (currently). For
full compatibility, we'll need to make the rust code store ranges as
ranges natively, possibly using something like the range_map crate.
Still, this patch is smaller than a "proper" fix.
Fixes TROVE-2018-003.
Since 0.2.4, tor uses EWMA circuit policy to prioritize. The previous
algorithm, round-robin, hasn't been used since then but was still used as a
fallback.
Now that EWMA is mandatory, remove that code entirely and enforce a cmux
policy to be set.
This is part of a circuitmux cleanup to improve performance and reduce
complexity in the code. We'll be able to address future optimization with this
work.
Closes#25268
Signed-off-by: David Goulet <dgoulet@torproject.org>
The reason to do so is because these functions haven't been used in years so
since 0.2.4, every callsite is NOP.
In future commits, we'll remove the round robin circuit policy which is mostly
validated within those function.
This simplifies the code greatly and remove dead code for which we never had a
configure option in the first place nor an easy way to use them in production.
Part of #25268
Signed-off-by: David Goulet <dgoulet@torproject.org>
It is rename to something more meaningful that explains what it does exactly
which is sets the EWMA options (currently only one exists). The new name is
cmux_ewma_set_options().
Also, remove a public function from circuitmux_ewma.h that is only used in the
C file. Make it static inline as well.
Signed-off-by: David Goulet <dgoulet@torproject.org>
To achieve this, a default value for the CircuitPriorityHalflife option was
needed. We still look in the options and then the consensus but in case no
value can be found, the default CircuitPriorityHalflifeMsec=30000 is used. It
it the value we've been using since 0.2.4.4-alpha.
This means that EWMA, our only policy, can not be disabled anymore fallbacking
to the round robin algorithm. Unneeded code to control that is removed in this
commit.
Part of #25268
Signed-off-by: David Goulet <dgoulet@torproject.org>
We had this safeguard around dos_init() but not when the consensus changes
which can modify consensus parameters and possibly enable the DoS mitigation
even if tor wasn't a public relay.
Fixes#25223
Signed-off-by: David Goulet <dgoulet@torproject.org>
Explicitly inform the operator of the rejected relay to set a valid email
address in the ContactInfo field and contact bad-relays@ mailing list.
Fixes#25170
Signed-off-by: David Goulet <dgoulet@torproject.org>
We don't expect this to come up very much, but we may as well make
sure that the value isn't predictable (as we do for the other
addresses) in case the issue ever comes up.
Spotted by teor.
This patch lowers the log-level from warning to info in the cases where
we are going to attempt another method as entropy source to hopefully
make the user feel less concerned.
See: https://bugs.torproject.org/25120
* ADD a new macro, tor_util::string::cstr!() which takes Rust strings,
concatenates them together, appends a NUL byte, and converts it into a
std::ffi::CStr for handing to C.
This is to avoid positively identifying Exit relays if tor client connection
comes from them that is reentering the network.
One thing to note is that this is done only in the DoS subsystem but we'll
still add it to the geoip cache as a "client" seen. This is done that way so
to avoid as much as possible changing the current behavior of the geoip client
cache since this is being backported.
Closes#25193
Signed-off-by: David Goulet <dgoulet@torproject.org>
Rationale: this helps for performance only, but we don't actually
have any reason to think that the checks here are
performance-critical. Let's not normalize the use of unsafe {}.
Explicitly inform the operator of the rejected relay to set a valid email
address in the ContactInfo field and contact bad-relays@ mailing list.
Fixes#25170
Signed-off-by: David Goulet <dgoulet@torproject.org>
* FIXES#25127: https://bugs.torproject.org/25127
* ADDS a new module to the Rust tor_util crate for small utilities
for working with static strings between languages.
* CHANGES the return type of protover_compute_for_old_tor to point to
immutable data.
* CHANGES the code from the previous commit to use the new static
string utilities.
At this commit, the SocksSocketsGroupWritable option is renamed to
UnixSocksGroupWritable. A deprecated warning is triggered if the old option is
used and tor will use it properly.
Fixes#24343
Signed-off-by: David Goulet <dgoulet@torproject.org>
On slow system, 1 msec between one read and the other was too tight. For
instance, it failed on armel with a 4msec gap:
https://buildd.debian.org/status/package.php?p=tor&suite=experimental
Increase to 10 msec for now to address slow system. It is important that we
keep this OP_LE test in so we make sure the msec/usec/nsec read aren't
desynchronized by huge gaps. We'll adjust again if we ever encounter a system
that goes slower than 10 msec between calls.
Fixes#25113
Signed-off-by: David Goulet <dgoulet@torproject.org>
Remove a series of connection counters that were only used when dumping the
rephist statistics with SIGUSR1 signal.
This reduces the or_history_t structure size.
Closes#25163
Signed-off-by: David Goulet <dgoulet@torproject.org>
This removes the code that tracks the extend attemps a client makes. We don't
use it and it was only used to provide statistics on a SIGUSR1 from the
rephist dump stats function.
Part of #25163
Signed-off-by: David Goulet <dgoulet@torproject.org>
Since we're making it so that unstable zstd apis can be disabled,
we need to test them. I do this by adding a variant setup/cleanup
function for the tests, and teaching it about a fake compression
method called "x-zstd:nostatic".
Services can keep rendezvous circuits for a while so don't log them if tor is
a single onion service.
Fixes#25116
Signed-off-by: David Goulet <dgoulet@torproject.org>
The HT_FOREACH() is insanely heavy on the CPU and this is part of the fast
path so make it return the nice memory size counter we added in
4d812e29b9.
Fixes#25148
Signed-off-by: David Goulet <dgoulet@torproject.org>
Included crypto_digest.[ch] into include.am in order to solve a compiling
issue. Also EOF line in crypto_digest.c added.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
Included crypto_digest.h in some files in order to solve xof+digest module
dependency issues. Removed crypto.h where it isn't needed anymore.
Follows #24658.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
