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.
KIST works by computing how much should be allowed to write to the kernel for
a given socket, and then it writes that amount to the outbuf.
The problem is that it could be possible that the outbuf already has lots of
data in it from a previous scheduling round (because the kernel is full/busy
and Tor was not able to flush the outbuf yet). KIST ignores that the outbuf
has been filling (is above its "highwater") and writes more anyway. The end
result is that the outbuf length would exceed INT_MAX, hence causing an
assertion error and a corresponding "Bug()" message to get printed to the
logs.
This commit makes it for KIST to take into account the outbuf length when
computing the available space.
Bug found and patch by Rob Jansen.
Closes#29168. TROVE-2019-001.
Signed-off-by: David Goulet <dgoulet@torproject.org>
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.
Bug not in any released Tor.
This test fails in some environments; since the code isn't used in
0.4.0, let's disable it for now.
Band-aid solution for #29534; bug not in any released Tor.
malloc_options needs to be declared extern (and declaring it extern
means we need to initialize it separately)
Fixes bug 29145; bugfix on 0.2.9.3-alpha
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
Also:
* delete some obsolete code that was #if 0
* improve cleanup on failure
* make the dir_format tests more consistent with each other
* construct the descriptors using smartlist chunks
This refactor is incomplete, because removing the remaining duplicate
code would be time-consuming.
Part of 29017 and 29018.
Remove router_update_info_send_unencrypted(), and move its code into the
relevant functions.
Then, re-use an options pointer.
Preparation for testing 29017 and 20918.
Remove some tiny static functions called by router_build_fresh_descriptor(),
and move their code into more relevant functions.
Then, give router_update_{router,extra}info_descriptor_body identical layouts.
Preparation for testing 29017 and 20918.
Make sure that these static functions aren't passed NULL.
If they are, log a BUG() warning, and return an error.
Preparation for testing 29017 and 20918.
Split the body of router_build_fresh_descriptor() into static functions,
by inserting function prologues and epilogues between existing sections.
Write a new body for router_build_fresh_descriptor() that calls the new
static functions.
Initial refactor with no changes to the body of the old
router_build_fresh_descriptor(), except for the split.
Preparation for testing 29017 and 20918.
When ExtraInfoStatistics is 0, stop including bandwidth usage statistics,
GeoIPFile hashes, ServerTransportPlugin lines, and bridge statistics
by country in extra-info documents.
Fixes bug 29018; bugfix on 0.2.4.1-alpha (and earlier versions).
This module is currently implemented to use the same technique as
libottery (later used by the bsds' arc4random replacement), using
AES-CTR-256 as its underlying stream cipher. It's backtracking-
resistant immediately after each call, and prediction-resistant
after a while.
Here's how it works:
We generate psuedorandom bytes using AES-CTR-256. We generate BUFLEN bytes
at a time. When we do this, we keep the first SEED_LEN bytes as the key
and the IV for our next invocation of AES_CTR, and yield the remaining
BUFLEN - SEED_LEN bytes to the user as they invoke the PRNG. As we yield
bytes to the user, we clear them from the buffer.
Every RESEED_AFTER times we refill the buffer, we mix in an additional
SEED_LEN bytes from our strong PRNG into the seed.
If the user ever asks for a huge number of bytes at once, we pull SEED_LEN
bytes from the PRNG and use them with our stream cipher to fill the user's
request.
Because the test is adding entries to the "rend_cache" directly, the
rend_cache_increment_allocation() was never called which made the
rend_cache_clean() call trigger that underflow warning:
rend_cache/clean: [forking] Nov 29 09:55:04.024 [warn] rend_cache_decrement_allocation(): Bug: Underflow in rend_cache_decrement_allocation (on Tor 0.4.0.0-alpha-dev 2240fe63feb9a8cf)
The test is still good and valid.
Fixes#28660
Signed-off-by: David Goulet <dgoulet@torproject.org>
Using an anonymous mmap() is a good way to get pages that we can set
kernel-level flags on, like minherit() or madvise() or mlock().
We're going to use that so that we can make uninheritable locked
pages to store PRNG data.
Rewrite service_intro_point_new() to take a node_t. Since
node_get_link_specifier_smartlist() supports IPv6 link specifiers,
this refactor adds IPv6 addresses to onion service descriptors.
Part of 23576, implements 26992.
Because the test is adding entries to the "rend_cache" directly, the
rend_cache_increment_allocation() was never called which made the
rend_cache_clean() call trigger that underflow warning:
rend_cache/clean: [forking] Nov 29 09:55:04.024 [warn] rend_cache_decrement_allocation(): Bug: Underflow in rend_cache_decrement_allocation (on Tor 0.4.0.0-alpha-dev 2240fe63feb9a8cf)
The test is still good and valid.
Fixes#28660
Signed-off-by: David Goulet <dgoulet@torproject.org>
Also, when we log about a failure from base32_decode(), we now
say that the length is wrong or that the characters were invalid:
previously we would just say that there were invalid characters.
Follow-up on 28913 work.
The code checked for sysctl being available and HW_PHYSMEM being
defined, but HW_USERMEM was actually being used with sysctl instead
of HW_PHYSMEM.
The case for OpenBSD, etc. use HW_PHYSMEM64 (which is obviously a
64-bit variant of HW_PHYSMEM) and the case for OSX uses HW_MEMSIZE
(which appears to be a 64-bit variant of HW_PHYSMEM).
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
We log these messages at INFO level, except when we are reading a
private key from a file, in which case we log at WARN.
This fixes a regression from when we re-wrote our PEM code to be
generic between nss and openssl.
Fixes bug 29042, bugfix on 0.3.5.1-alpha.
When cleaning up after an error in process_unix_exec, the stdin
pipe was being double closed instead of closing both the stdin
and stdout pipes. This occurred in two places.
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
NOTE: This commit breaks the build, because there was a mistake in an
earlier change of exactly the sort that this is meant to detect! I'm
leaving it broken for illustration.
Test exactly what the geometric sampler returns, because that's what
the downstream callers of it are going to use.
While here, also assert that the geometric sampler returns a positive
integer. (Our geometric distribution is the one suported on {1, 2,
3, ...} that returns the number of trials before the first success,
not the one supported on {0, 1, 2, ...} that returns the number of
failures before the first success.)
In file included from ./src/core/or/or_circuit_st.h:12:0,
from src/core/or/circuitlist.c:112:
./src/core/or/circuit_st.h:15:39: error: redefinition of typedef ‘circpad_machine_spec_t’
./src/core/or/circuitpadding.h:572:3: note: previous declaration of ‘circpad_machine_spec_t’ was here
./src/core/or/circuit_st.h:16:40: error: redefinition of typedef ‘circpad_machine_state_t’
./src/core/or/circuitpadding.h:517:3: note: previous declaration of ‘circpad_machine_state_t’ was here
In file included from src/core/or/connection_edge.c:70:0:
./src/core/or/circuitpadding.h:16:26: error: redefinition of typedef ‘circuit_t’
./src/core/or/or.h:930:26: note: previous declaration of ‘circuit_t’ was here
./src/core/or/circuitpadding.h:17:33: error: redefinition of typedef ‘origin_circuit_t’
./src/core/or/or.h:931:33: note: previous declaration of ‘origin_circuit_t’ was here
./src/core/or/circuitpadding.h:18:23: error: redefinition of typedef ‘cell_t’
./src/core/or/or.h:628:23: note: previous declaration of ‘cell_t’ was here
typedef doesn't work for forward declarations, but plain struct
without a typedef wrapper does (and unlike the _t type aliases makes
it clearer for everyone whether you're talking about the struct or
the pointer).
Stop logging "Tried to establish rendezvous on non-OR circuit..." as
a warning. Instead, log it as a protocol warning, because there is
nothing that relay operators can do to fix it.
Fixes bug 29029; bugfix on 0.2.5.7-rc.
Prior to this commit, the testsuite was failing on OpenBSD. After
this commit the testsuite runs fine on OpenBSD.
It was previously decided to test for the OpenBSD macro (rather than
__OpenBSD__, etc.) because OpenBSD forks seem to have the former
macro defined. sys/param.h must be included for the OpenBSD macro
definition; however, many files tested for the OpenBSD macro without
having this header included.
This commit includes sys/param.h in the files where the OpenBSD macro
is used (and sys/param.h is not already included), and it also
changes some instances of the __OpenBSD__ macro to OpenBSD.
See commit 27df23abb6 which changed
everything to use OpenBSD instead of __OpenBSD__ or OPENBSD. See
also tickets #6982 and #20980 (the latter ticket is where it was
decided to use the OpenBSD macro).
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
In get_local_listener used by tor_ersatz_socketpair, the address
family used when binding the IPv6 socket was AF_INET instead of
AF_INET6.
Fixes bug 28995; bugfix on 0.3.5.1-alpha.
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
Reported on tor-dev by Gisle Vanem. Bug not in any released Tor
(The suggested patch used _MSC_VER, but that's not how we do stuff
with autoconf. With autoconf, you detect the feature you want,
rather than trying to list all the systems that do or do not have
it.)
