In 9c132a5f9e we replaced "buf" with a pointer and replaced
one instance of snprintf with asprintf -- but there was still one
snprintf left over, being crashy.
Fixes bug 29967; bug not in any released Tor. This is CID 1444262.
This can't actually result in a null pointer dereference, since
pub_excl and sub_excl are only set when the corresponding smartlists
are nonempty. But coverity isn't smart enough to figure that out,
and we shouldn't really be depending on it.
Bug 29938; CID 1444257. Bug not in any released Tor.
Having the numbers in those messages makes some of the unit test
unstable, by causing them to depend on the initialization order of
the naming objects.
Based on patches and review comments by Riastradh and Catalyst.
Co-authored-by: Taylor R Campbell <campbell+tor@mumble.net>
Co-authored-by: Taylor Yu <catalyst@torproject.org>
When a directory authority is using a bandwidth file to obtain the
bandwidth values that will be included in the next vote, serve this
bandwidth file at /tor/status-vote/next/bandwidth.z.
Let's use the same function exit point for BUG() codepath that we're using
for every other exit condition. That way, we're not forgetting to clean up
the memarea.
Previously, I had used integers encoded as pointers. This
introduced a flaw: NULL represented both the integer zero, and the
absence of a setting. This in turn made the checks in
cfg_msg_set_{type,chan}() not actually check for an altered value if
the previous value had been set to zero.
Also, I had previously kept a pointer to a dispatch_fypefns_t rather
than making a copy of it. This meant that if the dispatch_typefns_t
were changed between defining the typefns and creating the
dispatcher, we'd get the modified version.
Found while investigating coverage in pubsub_add_{pub,sub}_()
This is necessary to get the number of includes in main.c back under
control. (In the future, we could just use the subsystem manager for
this kind of stuff.)
We want the DISPATCH_ADD_PUB() macro to count as making a
DECLARE_PUBLISH() invocation "used", so let's try a new approach
that preserves that idea. The old one apparently did not work for
some versions of osx clang.
This code tries to prevent a large number of possible errors by
enforcing different restrictions on the messages that different
modules publish and subscribe to.
Some of these rules are probably too strict, and some too lax: we
should feel free to change them as needed as we move forward and
learn more.
This "publish/subscribe" layer sits on top of lib/dispatch, and
tries to provide more type-safety and cross-checking for the
lower-level layer.
Even with this commit, we're still not done: more checking will come
in the next commit, and a set of usability/typesafety macros will
come after.
This module implements a way to send messages from one module to
another, with associated data types. It does not yet do anything to
ensure that messages are correct, that types match, or that other
forms of consistency are preserved.
We already do this in our log_debug() macro, but there are times
when we'd like to avoid allocating or precomputing something that we
are only going to log if debugging is on.
Previously, or_connection_t did not record whether or not the
connection uses a pluggable transport. Instead, it stored the
underlying proxy protocol of the pluggable transport in
proxy_type. This made bootstrap reporting treat pluggable transport
connections as plain proxy connections.
Store a separate bit indicating whether a pluggable transport is in
use, and decode this during bootstrap reporting.
Fixes bug 28925; bugfix on 0.4.0.1-alpha.
When NULL is given to lpApplicationName we enable Windows' "magical"
path interpretation logic, which makes Tor 0.4.x behave in the same way
as previous Tor versions did when it comes to executing binaries in
different system paths.
For more information about this have a look at the CreateProcessA()
documentation on MSDN -- especially the string interpretation example is
useful to understand this issue.
This bug was introduced in commit bfb94dd2ca.
See: https://bugs.torproject.org/29874
The name of circpad_machine_state_t was very confusing since it was conflicting
with circpad_state_t and circpad_circuit_state_t.
Right now here is the current meaning of these structs:
circpad_state_t -> A state of the state machine.
circpad_machine_runtime_t -> The current mutable runtime info of the state machine.
circpad_circuit_state_t -> Circuit conditions based on which we should apply a machine to the circuit
so that the relays that would be "excluded" from the bandwidth
file because of something failed can be included to diagnose what
failed, without still including these relays in the bandwidth
authorities vote.
Closes#29806.
This is something we should think about harder, but we probably want dormant
mode to be more powerful than padding in case a client has been inactive for a
day or so. After all, there are probably no circuits open at this point and
dormant mode will not allow the client to open more circuits.
Furthermore, padding should not block dormant mode from being activated, since
dormant mode relies on SocksPort activity, and circuit padding does not mess
with that.
They are simply not used apart from assigning a pointer and asserting on the
pointer depending on the cell direction.
Closes#29196.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Allow connections to single onion services to remain idle without being
disconnected.
Relays acting as rendezvous points for single onion services were
mistakenly closing idle established rendezvous circuits after 60 seconds,
thinking that they are unused directory-fetching circuits that had served
their purpose.
Fixes bug 29665; bugfix on 0.2.1.26.
They were causing the following warnings in circuitpadding/circuitpadding_sample_distribution:
src/lib/math/prob_distr.c:1311:17: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/lib/math/prob_distr.c:1311:17 in
src/lib/math/prob_distr.c:1219:49: runtime error: division by zero
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/lib/math/prob_distr.c:1219:49 in
because the distributions were called with erroneous parameters (e.g. geometric
distribution with p=0).
We now defined these test probability distributions with more realistic
parameters.
As far as the circuitpadding_sample_distribution() test is concerned, it
doesn't matter if the distributions return values outside of [0,10] since we
already restrict the values into that interval using min=0 and max=10 (and RTT
estimate is disabled).
The previous commits introduced link_specifier_dup(), which is
implemented using trunnel's opaque interfaces. So we can now
remove hs_desc_link_specifier_dup().
Cleanup after bug 22781.
Cleanup some bugs discovered during 23576:
* stop copying the first 20 characters of a 40-character hex string
to a binary fingerprint
* stop putting IPv6 addresses in a variable called "ipv4"
* explain why we do a duplicate tt_int_op() to deliberately fail and
print a value
Fixes bug 29243; bugfix on 0.3.2.1-alpha.
The previous commits for 23576 confused hs_desc_link_specifier_t
and link_specifier_t. Removing hs_desc_link_specifier_t fixes this
confusion.
Fixes bug 22781; bugfix on 0.3.2.1-alpha.
Check if the new pointer is the same as the old one: if it is, it's
probably a bug:
* the caller may have confused current and previous, or
* they may have forgotten to sr_srv_dup().
Putting NULL multiple times is allowed.
Part of 29706.
Refactor the shared random state's memory management so that it actually
takes ownership of the shared random value pointers.
Fixes bug 29706; bugfix on 0.2.9.1-alpha.
Stop leaking parts of the shared random state in the shared-random unit
tests. The previous fix in 29599 was incomplete.
Fixes bug 29706; bugfix on 0.2.9.1-alpha.
This commit also explicitly set the value of the PRT enum so we can match/pin
the C enum values to the Rust one in protover/ffi.rs.
Fixes#29631
Signed-off-by: David Goulet <dgoulet@torproject.org>
There's an incorrect comment in compat_time.c that suggests we call
FreeLibrary() before we're done using the library's functions.
See 29642 for background.
Closes ticket 29643.
* Move out code that depends on NSS to crypto_digest_nss.c
* Move out code that depends on OpenSSL to crypto_digest_openssl.c
* Keep the general code that is not specific to any of the above in
crypto_digest.c
Prior to #23100, we were not counting HS circuit build times in our
calculation of the timeout. This could lead to a condition where our timeout
was set too low, based on non HS circuit build times, and then we would
abandon all HS circuits, storing no valid timeouts in the histogram.
This commit avoids the assert.
When "auto" was used for the port number for a listening socket, the
message logged after opening the socket would incorrectly say port 0
instead of the actual port used.
Fixes bug 29144; bugfix on 0.3.5.1-alpha
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
This patch fixes a crash bug (assertion failure) in the PT subsystem
that could get triggered if the user cancels bootstrap via the UI in
TorBrowser. This would cause Tor to call `managed_proxy_destroy()` which
called `process_free()` after it had called `process_terminate()`. This
leads to a crash when the various process callbacks returns with data
after the `process_t` have been freed using `process_free()`.
We solve this issue by ensuring that everywhere we call
`process_terminate()` we make sure to detach the `managed_proxy_t` from
the `process_t` (by calling `process_set_data(process, NULL)`) and avoid
calling `process_free()` at all in the transports code. Instead we just
call `process_terminate()` and let the process exit callback in
`managed_proxy_exit_callback()` handle the `process_free()` call by
returning true to the process subsystem.
See: https://bugs.torproject.org/29562
Fixes bug 29530, where the LOG_ERR messages were occurring when
we had no configured network, and so we were failing the unit tests
because of the recently-merged #28668.
Commit message edited by teor:
We backported 28668 and released it in 0.3.5.8.
This commit backports 29530 to 0.3.5.
Fixes bug 29530 in Tor 0.3.5.8.
When IPv4Only (IPv6Only) was used but the address could not be
interpreted as a IPv4 (IPv6) address, the error message referred
to the wrong IP version.
This also fixes up the error-checking logic so it's more precise
about what's being checked.
Fixes bug 13221; bugfix on 0.2.3.9-alpha
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
This test was previously written to use the contents of the system
headers to decide whether INHERIT_NONE or INHERIT_ZERO was going to
work. But that won't work across different environments, such as
(for example) when the kernel doesn't match the headers. Instead,
we add a testing-only feature to the code to track which of these
options actually worked, and verify that it behaved as we expected.
Closes ticket 29541; bugfix not on any released version of Tor.
