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.)
In theory it would be better to detect this bug in advance, but this
approach is much simpler, and therefore safer to backport.
This closes tor issue 28973.
This project introduces the prob_distr.c subsystem which implements all the
probability distributions that WTF-PAD needs. It also adds unittests for all of
them.
Code and tests courtesy of Riastradh.
Co-authored-by: Taylor R Campbell <campbell+tor@mumble.net>
Co-authored-by: Mike Perry <mikeperry-git@torproject.org>
Hope is this will make it easier to test on the live tor network.
Does not need to be merged if we don't want to, but will come in handy
for researchers.
Co-authored-by: George Kadianakis <desnacked@riseup.net>
This implements all of the event handling, state machines, and padding
decisions for circuit padding.
I recommend reviewing this after you look at the call-in points into it from
the rest of Tor.
Co-authored-by: George Kadianakis <desnacked@riseup.net>
These callbacks allow the padding state machines to react to various types of
sent and received relay cells.
Co-authored-by: George Kadianakis <desnacked@riseup.net>
These event callbacks allow circuit padding to decide when to attempt to
launch and negotiate new padding machines, and when to tear old ones down.
Co-authored-by: George Kadianakis <desnacked@riseup.net>
This is a good code review start point, to get an overview of the interfaces
and types used in circuit padding.
Co-authored-by: George Kadianakis <desnacked@riseup.net>
We need this for padding negotiation so that we can have later machine
revisions supercede earlier ones.
Co-authored-by: George Kadianakis <desnacked@riseup.net>
signed_descriptor_digest has a length of DIGEST_LEN but the memset
used to fill it used DIGEST256_LEN.
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
Redefine the set of bootstrap phases to allow display of finer-grained
progress in the early connection stages of connecting to a relay.
This includes adding intermediate phases for proxy and PT connections.
Also add a separate new phase to indicate obtaining enough directory
info to build circuits so we can report that independently of actually
initiating an ORCONN to build the first application circuit.
Previously, we would claim to be connecting to a relay when we had
merely finished obtaining directory info.
Part of ticket 27167.
Replace a few invocations of control_event_bootstrap() with calls from
the bootstrap tracker subsystem. This mostly leaves behavior
unchanged. The actual behavior changes come in the next commit.
Part of ticket 27167.
Linked connections aren't woken up by libevent due to I/O but rather
artificially so we can, by chunks, empty the spooled object(s).
Commit 5719dfb48f (in 0.3.4.1-alpha) made it
that the schedule_active_linked_connections_event would be only called once at
startup but this is wrong because then we would never go through again the
active linked connections.
Fortunately, everytime a new linked connection is created, the event is
activated and thus we would go through the active list again. On a busy relay,
this issue is mitigated by that but on a slower relays or bridge, a connection
could get stuck for a while until a new directory information request would
show up.
Fixes#28717, #28912
Add a tracker for bootstrap progress, tracking events related to
origin circuit and ORCONN states. This uses the ocirc_event and
orconn_event publish-subscribe subsystems.
Part of ticket 27167.
Add a publish-subscribe subsystem to publish messages about changes to
origin circuits.
Functions in circuitbuild.c and circuitlist.c publish messages to this
subsystem.
Move circuit event constants out of control.h so that subscribers
don't have to include all of control.h to take actions based on
messages they receive.
Part of ticket 27167.
Add a publish-subscribe subsystem to publish messages about changes to
OR connections.
connection_or_change_state() in connection_or.c and
control_event_or_conn_event() in control.c publish messages to this
subsystem via helper functions.
Move state constants from connection_or.h to orconn_state.h so that
subscribers don't have to include all of connection_or.h to take
actions based on changes in OR connection state. Move event constants
from control.h for similar reasons.
Part of ticket 27167.
connection_or_change_state() saved an old_state to pass to
channel_tls_handle_state_change_on_orconn(), which promptly cast it to
void. Remove this unused variable and parameter.
This patch changes the CancelIoEx() example code to use CancelIo(),
which is available for older versions of Windows too. I still think the
kernel handles this nicely by sending broken pipes if either side
closes the pipe while I/O operations are pending.
See: https://bugs.torproject.org/28179
Handle `ERROR_BROKEN_PIPE` from ReadFileEx() and WriteFileEx() in
process_win32_stdin_write_done() and
process_win32_handle_read_completion() instead of in the early handler.
This most importantmly makes sure that `reached_eof` is set to true when
these errors appears.
See: https://bugs.torproject.org/28179
This patch adds some missing calls to set `reached_eof` of our handles
when various error conditions happens or when we close our handle (which
happens at `process_terminate()`.
See: https://bugs.torproject.org/28179
This patch adds some additional error checking after calls to
ReadFileEx() and WriteFileEx(). I have not managed to get this code to
reach the branch where `error_code` is NOT `ERROR_SUCCESS`, but MSDN
says one should check for this condition so we do so just to be safe.
See: https://bugs.torproject.org/28179
This patch makes us delay checking for whether we have an exit code
value (via GetExitCodeProcess()) until both stdout and stderr have been
closed by the operating system either by the process itself or by
process cleanup after termination.
See: https://bugs.torproject.org/28179
This patch adds support for the new STATUS message that PT's can emit
from their standard out. The STATUS message uses the `config_line_t` K/V
format that was recently added in Tor.
See: https://bugs.torproject.org/28846
This patch changes the LOG pluggable transport message to use the recent
K/V parser that landed in Tor. This allows PT's to specify the log
severity level as well as the message. A mapping between the PT log
severity levels and Tor's log serverity level is provided.
See: https://bugs.torproject.org/28846
Previously we had decoded the asn.1 to get a public key, and then
discarded the asn.1 so that we had to re-encode the key to store it
in the onion_pkey field of a microdesc_t or routerinfo_t.
Now we can just do a tor_memdup() instead, which should be loads
faster.
Previously, we would decode the PEM wrapper for keys twice: once in
get_next_token, and once later in PEM decode. Now we just do all of
the wrapper and base64 stuff in get_next_token, and store the
base64-decoded part in the token object for keys and non-keys alike.
This change should speed up parsing slightly by letting us skip a
bunch of stuff in crypto_pk_read_*from_string(), including the tag
detection parts of pem_decode(), and an extra key allocation and
deallocation pair.
Retaining the base64-decoded part in the token object will allow us
to speed up our microdesc parsing, since it is the asn1 portion that
we actually want to retain.
This patch makes sure that we terminate the event loop from the event
loop timer instead of directly in the process' exit handler. This allows
us to run the event loop an additional time to ensure that the SleepEx()
call on Windows is called and the data from stdout/stderr is delivered
to us.
Additionally we ensure that we don't try to read or write data from a
Unix process that have been terminated in the main loop, since its file
descriptors are closed at that time.
See: https://bugs.torproject.org/28179
This patch changes the API of the Windows backend of the Process
subsystem to allow the dormant interface to disable the Process event
timer.
See: https://bugs.torproject.org/28179
This patch changes our EVENT_TRANSPORT_LOG event to be EVENT_PT_LOG. The
new message includes the path to the PT executable instead of the
transport name, since one PT binary can include multiple transport they
sometimes might need to log messages that are not specific to a given
transport.
See: https://bugs.torproject.org/28179
This patch changes our process_t's exit_callback to return a boolean
value. If the returned value is true, the process subsystem will call
process_free() on the given process_t.
See: https://bugs.torproject.org/28179
This patch changes the slow process_t tests to use
run_main_loop_until_done() instead of do_main_loop() since
do_main_loop() initializes a lot of subsystem callbacks that we don't
need to run in our tests.
See: https://bugs.torproject.org/28179
This patch disables fork()'ing of the slow process tests. This fixes the
tests on the MacOS and other kqueue() based platforms.
Without this patch the main loop exits eearly with EBADF as error.
See: https://bugs.torproject.org/28179
This patch moves the remaining code from subprocess.{h,c} to more
appropriate places in the process.c and process_win32.c module.
We also delete the now empty subprocess module files.
See: https://bugs.torproject.org/28179
This patch adds test cases for process_t which uses Tor's main loop.
This allows us to test that the callbacks are actually invoked by the
main loop when we expect them.
See: https://bugs.torproject.org/28179
This patch adds support for the "LOG" protocol message from a pluggable
transport. This allows pluggable transport developers to relay log
messages from their binary to Tor, which will both emit them as log
messages from the Tor process itself, but also pass them on via the
control port.
See: https://bugs.torproject.org/28180
See: https://bugs.torproject.org/28181
See: https://bugs.torproject.org/28182
This patch makes the managed proxy subsystem use the process_t data
structure such that we can get events from the PT process while Tor is
running and not just when the PT process is being configured.
See: https://bugs.torproject.org/28179
This patch adds a new function that allows us to reset the environment
of a given process_t with a list of key/value pairs.
See: https://bugs.torproject.org/28179
This patch makes sure that we call process_notify_event_exit() after we
have done any modifications we need to do to the state of a process_t.
This allows application developers to call process_free() in the
exit_callback of the process.
See: https://bugs.torproject.org/28179
This patch adds support for getting the unique process identifier from a
given process_t. This patch implements both support for both the Unix
and Microsoft Windows backend.
See: https://bugs.torproject.org/28179
This patch adds support for Microsoft Windows in the Process subsystem.
Libevent does not support mixing different types of handles (sockets,
named pipes, etc.) on Windows in its core event loop code. This have
historically meant that Tor have avoided attaching any non-networking
handles to the event loop. This patch uses a slightly different approach
to roughly support the same features for the Process subsystem as we do
with the Unix backend.
In this patch we use Windows Extended I/O functions (ReadFileEx() and
WriteFileEx()) which executes asynchronously in the background and
executes a completion routine when the scheduled read or write operation
have completed. This is much different from the Unix backend where the
operating system signals to us whenever a file descriptor is "ready" to
either being read from or written to.
To make the Windows operating system execute the completion routines of
ReadFileEx() and WriteFileEx() we must get the Tor process into what
Microsoft calls an "alertable" state. To do this we execute SleepEx()
with a zero millisecond sleep time from a main loop timer that ticks
once a second. This moves the process into the "alertable" state and
when we return from the zero millisecond timeout all the outstanding I/O
completion routines will be called and we can schedule the next reads
and writes.
The timer loop is also responsible for detecting whether our child
processes have terminated since the last timer tick.
See: https://bugs.torproject.org/28179
This patch adds the Unix backend for the Process subsystem. The Unix
backend attaches file descriptors from the child process's standard in,
out and error to Tor's libevent based main loop using traditional Unix
pipes. We use the already available `waitpid` module to get events
whenever the child process terminates.
See: https://bugs.torproject.org/28179
This patch adds a new Process subsystem for running external programs in
the background of Tor. The design is focused around a new type named
`process_t` which have an API that allows the developer to easily write
code that interacts with the given child process. These interactions
includes:
- Easy API for writing output to the child process's standard input
handle.
- Receive callbacks whenever the child has output on either its standard
output or standard error handles.
- Receive callback when the child process terminates.
We also support two different "protocols" for handling output from the
child process. The default protocol is the "line" protocol where the
process output callbacks will be invoked only when there is complete
lines (either "\r\n" or "\n" terminated). We also support the "raw"
protocol where the read callbacks will get whatever the operating system
delivered to us in a single read operation.
This patch does not include any operating system backends, but the Unix
and Windows backends will be included in separate commits.
See: https://bugs.torproject.org/28179
The strcmp_len() function was somewhat misconceived, since we're
only using it to test whether a length+extent string is equal to a
NUL-terminated string or not. By simplifying it and making it
inlined, we should be able to make it a little faster.
(It *does* show up in profiles.)
Closes ticket 28856.
The old implementation did some funky out-of-order lexing, and
tended to parse every port twice if the %d-%d pattern didn't match.
Closes ticket 28853.
I believe we originally added this for "just in case" safety, but it
isn't actually needed -- we never copy uninitialized stack here.
What's more, this one memset is showing up on our startup profiles,
so we ought to remove it.
Closes ticket 28852.
The point of this function is to make sure that the ed25519-based
implementation of curve25519_basepoint() actually works when we
start tor, and use the regular fallback implementation if it
doesn't. But it accounts for 9% of our startup time in the case
when we have directory information, and I think it's safe to make
the test shorter. After all, it has yet to find any actual bugs in
curved25519_scalarmult_basepoint_donna() on any platforms.
Closes ticket 28838.
When the clock jumps, and we have a record of last user activity,
adjust that record. This way if I'm inactive for 10 minutes and
then the laptop is sleeping for an hour, I'll still count as having
been inactive for 10 minutes.
Previously, we treat every jump as if it were activity, which is
ridiculous, and would prevent a Tor instance with a jumpy clock from
ever going dormant.
encoding and decoding.
There are bunches of places where we don't want to invest in a full
fuzzer, but we would like to make sure that some string operation
can handle all its possible inputs. This fuzzer uses the first byte
of its input to decide what to do with the rest of the input. Right
now, all the possibilities are decoding a string, and seeing whether
it is decodeable. If it is, we try to re-encode it and do the whole
thing again, to make sure we get the same result.
This turned up a lot of bugs in the key-value parser, and I think it
will help in other cases too.
Closes ticket 28808.
Add the bootstrap tag name to the log messages, so people
troubleshooting connection problems can look up a symbol instead of a
number. Closes ticket 28731.
Merge Phoul's two lists into teor's list.
Replace the 150 fallbacks originally introduced in Tor 0.3.3.1-alpha in
January 2018 (of which ~115 were still functional), with a list of
157 fallbacks (92 new, 65 existing, 85 removed) generated in
December 2018.
Closes ticket 24803.
Replace the 150 fallbacks originally introduced in Tor 0.3.3.1-alpha in
January 2018 (of which ~115 were still functional), with a list of
148 fallbacks (89 new, 59 existing, 91 removed) generated in
December 2018.
Closes ticket 24803.
When retrying all SOCKS connection because new directory information just
arrived, do not BUG() if a connection in state AP_CONN_STATE_RENDDESC_WAIT is
found to have a usable descriptor.
There is a rare case when this can happen as detailed in #28669 so the right
thing to do is put that connection back in circuit wait state so the
descriptor can be retried.
Fixes#28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
This helper function marks an entry connection as pending for a circuit and
changes its state to AP_CONN_STATE_CIRCUIT_WAIT. The timestamps are set to
now() so it can be considered as new.
No behaviour change, this helper function will be used in next commit.
Part of #28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
Use the helper function connection_ap_mark_as_waiting_for_renddesc()
introduced in previous commit everywhere in the code where an AP connection
state is transitionned to AP_CONN_STATE_RENDDESC_WAIT.
Part of #28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
The transition for a connection to either become or go back in
AP_CONN_STATE_RENDDESC_WAIT state must make sure that the entry connection is
_not_ in the waiting for circuit list.
This commit implements the helper function
connection_ap_mark_as_waiting_for_renddesc() that removes the entry connection
from the pending list and then change its state. This code pattern is used in
many places in the code where next commit will remove this code duplication to
use this new helper function.
Part of #28669
Signed-off-by: David Goulet <dgoulet@torproject.org>
Rather than initializing the "Dormant" status to "off" and the "last
activity" count to "now", initialize them based on our state file:
stay dormant if we were dormant, or remember the amount of time
we've spent inactive.
Additionally, use it to test that is_staledesc is set correctly.
Eventually we'll want to test all the other flags, but I'm aiming
for only adding coverage on the changed code here.
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>
This patch adds two new functions: buf_flush_to_pipe() and
buf_read_from_pipe(), which makes use of our new buf_flush_to_fd() and
buf_read_from_fd() functions.
See: https://bugs.torproject.org/28179
This patch refactors buf_read_from_socket() into buf_read_from_fd(), and
creates a specialized function for buf_read_from_socket(), which uses
buf_read_from_fd().
See: https://bugs.torproject.org/28179
This patch refactors buf_flush_to_socket() into buf_flush_to_fd() and
creates a specialization function for buf_flush_to_socket() that makes
use of buf_flush_to_fd().
See: https://bugs.torproject.org/28179
The DormantClientTimeout option controls how long Tor will wait before
going dormant. It also provides a way to disable the feature by setting
DormantClientTimeout to e.g. "50 years".
The DormantTimeoutDisabledByIdleStreams option controls whether open but
inactive streams count as "client activity". To implement it, I had to
make it so that reading or writing on a client stream *always* counts as
activity.
Closes ticket 28429.
Specifically, if the consensus is older than the (estimted or
measured) release date for this version of tor, we assume that the
required versions may have changed in between that consensus and
this release.
Implements ticket 27735 and proposal 297.
This patch has routers use the same canonicalization logic as
authorities when encoding their family lists. Additionally, they
now warn if any router in their list is given by nickname, since
that's error-prone.
This patch also adds some long-overdue tests for family formatting.
Prop298 says that family entries should be formatted with
$hexids in uppercase, nicknames in lower case, $hexid~names
truncated, and everything sorted lexically. These changes implement
that ordering for nodefamily.c.
We don't _strictly speaking_ need to nodefamily.c formatting use
this for prop298 microdesc generation, but it seems silly to have
two separate canonicalization algorithms.
After we clear the protover map for getting full, we need to
re-create it, since we are about to use it.
This is a bugfix for bug 28558. It is a bugfix for the code from
ticket 27225, which is not in any released Tor. Found by Google
OSS-Fuzz, as issue 11475.
To succesful compile tor-print-ed-signing-cert.exe on Windows we
sometimes need to include the @TOR_LIB_GDI@ library.
See: https://bugs.torproject.org/28485
This representation is meant to save memory in microdescriptors --
we can't use it in routerinfo_t yet, since those families need to be
encoded losslessly for directory voting to work.
This representation saves memory in three ways:
1. It uses only one allocation per family. (The old way used a
smartlist (2 allocs) plus one strdup per entry.)
2. It stores identity digests in binary, not hex.
3. It keeps families in a canonical format, memoizes, and
reference-counts them.
Part of #27359.
This event makes us become dormant if we have seen no activity in a
long time.
Note that being any kind of a server, or running an onion service,
always counts as being active.
Note that right now, just having an open stream that Tor
did not open on its own (for a directory request) counts as "being
active", so if you have an idle ssh connection, that will keep Tor
from becoming dormant.
Many of the features here should become configurable; I'd like
feedback on which.
This is part of 28422, so we don't have to call
consider_hibernation() once per second when we're dormant.
This commit does not remove delayed shutdown from hibernate.c: it
uses it as a backup shutdown mechanism, in case the regular shutdown
timer mechanism fails for some reason.
The previous "ALL" role was the OR of a bunch of other roles,
which is a mistake: it's better if "ALL" means "all".
The "NET_PARTICIPANT" role refers to the anything that is actively
building circuits, downloading directory information, and
participating in the Tor network. For now, it is set to
!net_is_disabled(), but we're going to use it to implement a new
"extra dormant mode".
Closes ticket 28336.
Correctly identify Windows 8.1, Windows 10, and Windows Server 2008
and later from their NT versions.
On recent Windows versions, the GetVersionEx() function may report
an earlier Windows version than the running OS. To avoid user
confusion, add "[or later]" to Tor's version string on affected
versions of Windows.
Remove Windows versions that were never supported by the
GetVersionEx() function.
Stop duplicating the latest Windows version in get_uname().
Fixes bug 28096; bugfix on 0.2.2.34; reported by Keifer Bly.
In conn_close_if_marked(), we can decide to keep a connection open that still
has data to flush on the wire if it is being rate limited on the write side.
However, in this process, we were also looking at the read() side which can
still have token in its bucket and thus not stop the reading. This lead to a
BUG() introduced in 0.3.4.1-alpha that was expecting the read side to be
closed due to the rate limit but which only applies on the write side.
This commit removes any bandwidth check on the read side and simply stop the
read side on the connection regardless of the bucket state. If we keep the
connection open to flush it out before close, we should not read anything.
Fixes#27750
Signed-off-by: David Goulet <dgoulet@torproject.org>
Apparently some freebsd compilers can't tell that 'c' will never
be used uninitialized.
Fixes bug 28413; bugfix on 0.2.9.3-alpha when we added support for
longer AES keys to this function.
We don't use this syscall, but openssl apparently does.
(This syscall puts a socket into a half-closed state. Don't worry:
It doesn't shut down the system or anything.)
Fixes bug 28183; bugfix on 0.2.5.1-alpha where the sandbox was
introduced.
Apparently, even though the manpage says it returns an int, it
can return a long instead and cause a warning.
Bug not in any released Tor. Part of #28399
Our tests showed that this function is responsible for a huge number
of our malloc/free() calls. It's a prime candidate for being
memoized.
Closes ticket 27225.
Resume refusing to start with relative file paths and RunAsDaemon
set (regression from the fix for bug 22731).
Fixes bug 28298; bugfix on 0.3.3.1-alpha.
If tor terminates due to SIGNAL HALT before test_rebind.py calls
tor_process.terminate(), an OSError 3 (no such process) is thrown.
Fixes part of bug 27968 on 0.3.5.1-alpha.
Remember, you can't check to see if there are N bytes left in a
buffer by doing (buf + N < end), since the buf + N computation might
take you off the end of the buffer and result in undefined behavior.
Fixes 28202; bugfix on 0.2.0.3-alpha.
It is not enough to look at protover for v3 rendezvous support but also we
need to make sure that the curve25519 onion key is present or in other words
that the descriptor has been fetched and does contain it.
Fixes#27797.
Signed-off-by: David Goulet <dgoulet@torproject.org>
With the new refresh_service_descriptor() function we had both
refresh_service_descriptor() and update_service_descriptor() which is basically
the same thing.
This commit renames update_service_descriptor() to
update_service_descriptor_intro_points() to make it clear it's not a generic
refresh and it's only about intro points.
Commit changes no code.
Treat backtrace test failures as expected on NetBSD, OpenBSD, and
macOS/Darwin, until we solve bug 17808.
(FreeBSD failures have been treated as expected since 18204 in 0.2.8.)
Fixes bug 27948; bugfix on 0.2.5.2-alpha.
Before this commit, we would create the descriptor signing key certificate
when first building the descriptor.
In some extreme cases, it lead to the expiry of the certificate which triggers
a BUG() when encoding the descriptor before uploading.
Ticket #27838 details a possible scenario in which this can happen. It is an
edge case where tor losts internet connectivity, notices it and closes all
circuits. When it came back up, the HS subsystem noticed that it had no
introduction circuits, created them and tried to upload the descriptor.
However, in the meantime, if tor did lack a live consensus because it is
currently seeking to download one, we would consider that we don't need to
rotate the descriptors leading to using the expired signing key certificate.
That being said, this commit does a bit more to make this process cleaner.
There are a series of things that we need to "refresh" before uploading a
descriptor: signing key cert, intro points and revision counter.
A refresh function is added to deal with all mutable descriptor fields. It in
turn simplified a bit the code surrounding the creation of the plaintext data.
We keep creating the cert when building the descriptor in order to accomodate
the unit tests. However, it is replaced every single time the descriptor is
uploaded.
Fixes#27838
Signed-off-by: David Goulet <dgoulet@torproject.org>
When storing a descriptor in the client cache, if we are about to replace an
existing descriptor, make sure to close every introduction circuits of the old
descriptor so we don't have leftovers lying around.
Ticket 27471 describes a situation where tor is sending an INTRODUCE1 cell on
an introduction circuit for which it doesn't have a matching intro point
object (taken from the descriptor).
The main theory is that, after a new descriptor showed up, the introduction
points changed which led to selecting an introduction circuit not used by the
service anymore thus for which we are unable to find the corresponding
introduction point within the descriptor we just fetched.
Closes#27471.
Signed-off-by: David Goulet <dgoulet@torproject.org>
It won't be used if there are no authorized client configured. We do that so
we can easily support the addition of a client with a HUP signal which allow
us to avoid more complex code path to generate that cookie if we have at least
one client auth and we had none before.
Fixes#27995
Signed-off-by: David Goulet <dgoulet@torproject.org>
Both client and service had their own code for this. Consolidate into one
place so we avoid duplication.
Closes#27549
Signed-off-by: David Goulet <dgoulet@torproject.org>
Occasionally, key pinning doesn't catch a relay that shares an ed25519
ID with another relay. Log the identity fingerprints and the shared
ed25519 ID when this happens, instead of making a BUG() warning.
Fixes bug 27800; bugfix on 0.3.2.1-alpha.
Commit 488e2b00bf introduced an issue, most
likely introduced by a bad copy paste, that made us stop reading on the
connection if our write bandwidth limit was reached.
The problem is that because "read_blocked_on_bw" was never set, the connection
was never reenabled for reading.
This is most likely the cause of #27813 where bytes were accumulating in the
kernel TCP bufers because tor was not doing reads. Only relays with
RelayBandwidthRate would suffer from this but affecting all relays connecting
to them. And using that tor option is recommended and best practice so many
many relays have it enabled.
Fixes#28089.
It turns out that if _only_ the ControlPort is set and nothing else, tor would
simply not bootstrap and thus not start properly. Commit 67a41b6306
removed that requirement for tor to be considered a "client".
Unfortunately, this made the mainloop enable basically nothing if only the
ControlPort is set in the torrc.
This commit now makes it that we also consider the ControlPort when deciding
if we are a Client or not. It does not revert 67a41b6306 meaning
options_any_client_port_set() stays the same, not looking at the control port.
Fixes#27849.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Nothing should ever look at them on failure, but in some cases,
the unit tests don't check for failure, and then GCC-LTO freaks out.
Fixes part of 27772.
These confused GCC LTO, which thought they might be used
uninitialized. I'm pretty sure that as long as 'res' indicates
success, they will always be set to something, but let's unconfuse
the compiler in any case.
OpenSolaris apparently doesn't have timeradd(), so we added a
replacement, but we weren't including it here after the big
refactoring in 0.3.5.1-alpha.
Fixes bug 27963; bugfix on 0.3.5.1-alpha.
It looks to be the case that Rust's standard allocator, jemalloc, is
incompatible with sanitizers. The incompatibility, for whatever reason,
seems to cause segfaults at runtime when jemalloc is linked with
sanitizers.
Without actually trying to figure out what's going on here this commit
instead takes the hammer of "let's remove jemalloc when testing". The
`tor_allocate` crate now by default switches to the system allocator
(eventually this will want to be the tor allocator). Most crates then
link to `tor_allocate` ot pick this up, but the `smartlist` crate had to
manually switch to the system allocator in testing and the `external`
crate had to be sure to link to `tor_allocate`.
The final gotcha here is that this patch also switches to
unconditionally passing `--target` to Cargo. For weird and arcane
reasons passing `--target` with the host target of the compiler (which
Cargo otherwise uses as the default) is different than not passing
`--target` at all. This ensure that our custom `RUSTFLAGS` with
sanitizer options doesn't make its way into build scripts, just the
final testing artifacts.
This is no longer necessary with upstream rust-lang/rust changes as well
as some local tweaks. Namely:
* The `-fsanitize=address`-style options are now passed via `-C
link-args` through `RUSTFLAGS`. This obviates the need for the shell
script.
* The `-C default-linker-libraries`, disabling `-nodefaultlibs`, is
passed through `RUSTFLAGS`, which is necessary to ensure that
`-fsanitize=address` links correctly.
* The `-C linker` option is passed to ensure we're using the same C
compiler as normal C code, although it has a bit of hackery to only
get the `gcc` out of `gcc -std=c99`
Allowing this didn't do any actual harm, since there aren't any
shared structures or leakable objects here. Still, it's bad style
and might cause trouble in the future.
Closes ticket 27856.
Various places in our code try to activate these events or check
their status, so we should make sure they're initialized as early as
possible. Fixes bug 27861; bugfix on 0.3.5.1-alpha.
When freeing a configuration object from confparse.c in
dump_config(), we need to call the appropriate higher-level free
function (like or_options_free()) and not just config_free().
This only happens with options (since they're the one where
options_validate allocates extra stuff) and only when running
--dump-config with something other than minimal (since
OPTIONS_DUMP_MINIMAL doesn't hit this code).
Fixes bug 27893; bugfix on 0.3.2.1-alpha.
It differs from the rest of the rephist code in that it's actually
necessary for Tor to operate, so it should probably go somewhere
else. I'm not sure where yet, so I'll leave it in the same
directory, but give it its own file.
Since this is completely core functionality, I'm putting it in
core/mainloop, even though it depends on feature/hibernate. We'll
have to sort that out in the future.
If a tor client gets a descriptor that it can't decrypt, chances are that the
onion requires client authorization.
If a tor client is configured with client authorization for an onion but
decryption fails, it means that the configured keys aren't working anymore.
In both cases, we'll log notice the former and log warn the latter and the
rest of the decryption errors are now at info level.
Two logs statement have been removed because it was redundant and printing the
fetched descriptor in the logs when 80% of it is encrypted wat not helping.
Fixes#27550
Signed-off-by: David Goulet <dgoulet@torproject.org>
The main.c code is responsible for initialization and shutdown;
the mainloop.c code is responsible for running the main loop of Tor.
Splitting the "generic event loop" part of mainloop.c from the
event-loop-specific part is not done as part of this patch.
The parts for handling cell formats should be in src/core/or.
The parts for handling onionskin queues should be in src/core/or.
Only the crypto wrapper belongs in src/core/crypto.
When sending the INTRODUCE1 cell, we acquire the needed data for the cell but
if the RP node_t has invalid data, we'll fail the send and completely kill the
SOCKS connection.
Instead, close the rendezvous circuit and return a transient error meaning
that Tor can recover by selecting a new rendezvous point. We'll also do the
same when we are unable to encode the INTRODUCE1 cell for which at that point,
we'll simply take another shot at a new rendezvous point.
Fixes#27774
Signed-off-by: David Goulet <dgoulet@torproject.org>
The result of CString::into_raw() is not safe to free
with free() except under finicky and fragile circumstances
that we definitely don't meet right now.
This was missed in be583a34a3.
When we close a socket via tor_tls_free(), we previously had no way
for our socket accounting logic to learn about it. This meant that
the socket accounting code would think we had run out of sockets,
and freak out.
Fixes bug 27795; bugfix on 0.3.5.1-alpha.
In dirauth:
* bwauth.c reads and uses bandwidth files
* guardfraction.c reads and uses the guardfraction file
* reachability.c tests relay reachability
* recommend_pkg.c handles the recommended-packages lines.
* recv_descs.c handles fingerprint files and processing incoming
routerinfos that relays upload to us
* voteflag.c computes flag thresholds and sets those thresholds on
routerstatuses when computing votes
In control:
* fmt_serverstatus.c generates the ancient "v1 server status"
format that controllers expect.
In nodelist:
* routerstatus_fmt.c formats routerstatus entries for a consensus,
a vote, or for the controller.
Client side, when a descriptor is finally fetched and stored in the cache, we
then go over all pending SOCKS request for that descriptor. If it turns out
that the intro points are unusable, we close the first SOCKS request but not
the others for the same .onion.
This commit makes it that we'll close all SOCKS requests so we don't let
hanging the other ones.
It also fixes another bug which is having a SOCKS connection in RENDDESC_WAIT
state but with a descriptor in the cache. At some point, tor will expire the
intro failure cache which will make that descriptor usable again. When
retrying all SOCKS connection (retry_all_socks_conn_waiting_for_desc()), we
won't end up in the code path where we have already the descriptor for a
pending request causing a BUG().
Bottom line is that we should never have pending requests (waiting for a
descriptor) with that descriptor in the cache (even if unusable).
Fixees #27410.
Signed-off-by: David Goulet <dgoulet@torproject.org>
It is not enough to look at protover for v3 rendezvous support but also we
need to make sure that the curve25519 onion key is present or in other words
that the descriptor has been fetched and does contain it.
Fixes#27797.
Signed-off-by: David Goulet <dgoulet@torproject.org>
There are now separate modules for:
* the list of router descriptors
* the list of authorities and fallbacks
* managing authority certificates
* selecting random nodes
That unit test makes sure we don't have pending SOCK request if the descriptor
turns out to be unusable.
Part of #27410.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Client side, when a descriptor is finally fetched and stored in the cache, we
then go over all pending SOCKS request for that descriptor. If it turns out
that the intro points are unusable, we close the first SOCKS request but not
the others for the same .onion.
This commit makes it that we'll close all SOCKS requests so we don't let
hanging the other ones.
It also fixes another bug which is having a SOCKS connection in RENDDESC_WAIT
state but with a descriptor in the cache. At some point, tor will expire the
intro failure cache which will make that descriptor usable again. When
retrying all SOCKS connection (retry_all_socks_conn_waiting_for_desc()), we
won't end up in the code path where we have already the descriptor for a
pending request causing a BUG().
Bottom line is that we should never have pending requests (waiting for a
descriptor) with that descriptor in the cache (even if unusable).
Fixees #27410.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The trunnel functions are written under the assumption that their
allocators can fail, so GCC LTO thinks they might return NULL. In
point of fact, they're using tor_malloc() and friends, which can't
fail, but GCC won't necessarily figure that out.
Fixes part of #27772.
Instead, have it call a mockable function. We don't want
crypto_strongest_rand() to be mockable, since doing so creates a
type error when we call it from ed25519-donna, which we do not build
in a test mode.
Fixes bug 27728; bugfix on 0.3.5.1-alpha
This argument was added to match an older idea for the C api, but we
decided not to do it that way in C.
Fixes bug 27741; bugfix on 0.3.3.6 / TROVE-2018-005 fix.
Since the default cache directory is the same as the default data
directory, we don't want the default CacheDirectoryGroupReadable
value (0) to override an explicitly set "DataDirectoryGroupReadable
1".
To fix this, I'm making CacheDirectoryGroupReadable into an
autobool, and having the default (auto) value mean "Use the value of
DataDirectoryGroupReadable if the directories are the same, and 0
otherwise."
Fixes bug 26913; bugfix on 0.3.3.1-alpha when the CacheDirectory
option was introduced.
This shouldn't be a user-visible change: nobody has a 16 MB RSA
key that they're trying to use with Tor.
I'm doing this to fix CID 1439330 / ticket 27730, where coverity
complains (on 64-bit) that we are making a comparison that is never
true.
This patch moves the logic that adds the proxy headers to an earlier
point in the exit connection lifetime, which ensures that the
application data cannot be written to the outbuf before the proxy header
is added.
See: https://bugs.torproject.org/4700
This patch changes HiddenServiceExportCircuitID so instead of being a
boolean it takes a string, which is the protocol. Currently only the
'haproxy' protocol is defined.
See: https://bugs.torproject.org/4700
Without this patch we would encode the IPv6 address' last part as
::ffffffff instead of ::ffff:ffff when the GID is UINT32_MAX.
See: https://bugs.torproject.org/4700
In hs_config.c, we do validate the permission of the hidden service directory
but we do not try to create it. So, in the event that the directory doesn't
exists, we end up in the loading key code path which checks for the
permission and possibly creates the directory. On failure, don't BUG() since
there is a perfectly valid use case for that function to fail.
Fixes#27335
Signed-off-by: David Goulet <dgoulet@torproject.org>
This is harder than with OpenSSL, since OpenSSL counts the bytes on
its own and NSS doesn't. To fix this, we need to define a new
PRFileDesc layer that has its own byte-counting support.
Closes ticket 27289.
On GCC and Clang, there's a feature to warn you about bad
conditionals like "if (a = b)", which should be "if (a == b)".
However, they don't warn you if there are extra parentheses around
"a = b".
Unfortunately, the tor_assert() macro and all of its kin have been
passing their inputs through stuff like PREDICT_UNLIKELY(expr) or
PREDICT_UNLIKELY(!(expr)), both of which expand to stuff with more
parentheses around "expr", thus suppressing these warnings.
To fix this, this patch introduces new macros that do not wrap
expr. They're only used when GCC or Clang is enabled (both define
__GNUC__), since they require GCC's "({statement expression})"
syntax extension. They're only used when we're building the
unit-test variant of the object files, since they suppress the
branch-prediction hints.
I've confirmed that tor_assert(), tor_assert_nonfatal(),
tor_assert_nonfatal_once(), BUG(), and IF_BUG_ONCE() all now give
compiler warnings when their argument is an assignment expression.
Fixes bug 27709.
Bugfix on 0.0.6, where we first introduced the "tor_assert()" macro.
.retain() would allocating a Vec of billions of integers and check them
one at a time to separate the supported versions from the unsupported.
This leads to a memory DoS.
Closes ticket 27206. Bugfix on e6625113c9.
Before 0.3.3.1-alpha, we would exit() in this case immediately. But
now that we leave tor_main() more conventionally, we need to make
sure we restore things so as not to cause a double free.
Fixes bug 27708; bugfix on 0.3.3.1-alpha.
Since we use a 32-bit approximation for millisecond conversion here,
we can't expect so much precision.
Fixes part of bug 27139; bugfix on 0.3.4.1-alpha.
Multiply-then-divide is more accurate, but it runs into trouble when
our input is above INT32_MAX/numerator. So when our value is too
large, do divide-then-multiply instead.
Fixes part of bug 27139; bugfix on 0.3.4.1-alpha.
We use an optimized but less accurate formula for converting coarse
time differences to milliseconds on 32-bit OSX platforms, so that we
can avoid 64-bit division.
The old numbers were off by 0.4%. The new numbers are off by .006%.
This should make the unit tests a bit cleaner, and our tolerances a
bit closer.
All node_get_all_orports() does is allocate and return a smartlist
with at most two tor_addr_port_t members that match ORPort's of
node configuration. This is harmful for memory efficiency, as it
allocates the same stuff every time it is called. However,
node_is_a_configured_bridge() does not need to call it, as it
already has all the information to check if there is configured
bridge for a given node.
The new code is arranged in a way that hopefully makes each succeeding
linear search through bridge_list less likely.
We determine that a cell was dropped by inspecting CIRC_BW fields. If we did
not update the delivered or overhead fields after processing the cell, the
cell was dropped/not processed.
Also emit CIRC_BW events for cases where we decide to close the circuit in
this function, so vanguards can print messages about dropped cells in those
cases, too.
This function tells the underlying TLS object that it shouldn't
close the fd on exit. Mostly, we hope not to have to use it, since
the NSS implementation is kludgey, but it should allow us to fix
It's possible for a unit test to report success via its pipe, but to
fail as it tries to clean up and exit. Notably, this happens on a
leak sanitizer failure.
Fixes bug 27658; bugfix on 0.2.2.4-alpha when tinytest was
introduced.
This commit makes it that the authorized clients in the descriptor are in
random order instead of ordered by how they were read on disk.
Fixes#27545
Signed-off-by: David Goulet <dgoulet@torproject.org>
Existing cached directory information can cause misleadingly high
bootstrap percentages. To improve user experience, defer reporting of
directory information progress until at least one connection has
succeeded to a relay or bridge.
Closes ticket 27169.
If a tor client gets a descriptor that it can't decrypt, chances are that the
onion requires client authorization.
If a tor client is configured with client authorization for an onion but
decryption fails, it means that the configured keys aren't working anymore.
In both cases, we'll log notice the former and log warn the latter and the
rest of the decryption errors are now at info level.
Two logs statement have been removed because it was redundant and printing the
fetched descriptor in the logs when 80% of it is encrypted wat not helping.
Fixes#27550
Signed-off-by: David Goulet <dgoulet@torproject.org>
Track bootstrap phase (enumerated by bootstrap_status_t) independently
from the bootstrap progress (which can represent intermediate
progress). This allows control_event_bootstrap_problem() to avoid
doing a linear search through the bootstrap progress space to find the
current bootstrap phase.
Move the mostly-invariant part of control_event_boostrap() into a
helper control_event_bootstrap_core(). The helper doesn't modify any
state beyond doing logging and control port notifications.
Simplify control_event_bootstrap() by making it return void again. It
is currently a fairly complicated function, and it's made more
complicated by returning an int to signal whether it logged at NOTICE
or INFO.
The callers conditionally log messages at level NOTICE based on this
return value. Change the callers to unconditionally log their verbose
human-readable messages at level INFO to keep NOTICE logs less
cluttered.
This partially reverts the changes of #14950.
One HSv3 unit test used "tor_memeq()" without checking the return value. This
commit changes that to use "tt_mem_op()" to actually make the test validate
something :).
Signed-off-by: David Goulet <dgoulet@torproject.org>
>>>> CID 1439133: Null pointer dereferences (REVERSE_INULL)
>>>> Null-checking "fields" suggests that it may be null, but it
>>>> has already been dereferenced on all paths leading to the check.
>>>> CID 1439132: Null pointer dereferences (REVERSE_INULL)
>>>> Null-checking "fields" suggests that it may be null, but it
>>>> has already been dereferenced on all paths leading to the check.
This is an attempt to work around what I think may be a bug in
OSS-Fuzz, which thinks that uninitialized data might be passed to
the curve25519 functions.
There are three reasons we use a cached_dir_t to hold a consensus:
1. to serve that consensus to a client
2. to apply a consensus diff to an existing consensus
3. to send the consensus to a controller.
But case 1 is dircache-only. Case 2 and case 3 both fall back to
networkstatus_read_cached_consensus(). So there's no reason for us
to store this as a client. Avoiding this saves about 23% of our RAM
usage, according to our experiments last month.
This is, semantically, a partial revert of e5c608e535.
Fixes bug 27247; bugfix on 0.3.0.1-alpha.
We already had fallback code for "dir/status-vote/current/consensus"
to read from disk if we didn't have a cached_dir_t available. But
there's a function in networkstatus_t that does it for us, so let's
do that.
Return a newly allocated fake client authorization object instead of taking
the object as a parameter.
Signed-off-by: David Goulet <dgoulet@torproject.org>
When reloading tor, check if our the configured client authorization have
changed from what we previously had. If so, republish the updated descriptor.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, the validation by decoding a created descriptor was disabled
because the interface had to be entirely changed and not implemented at the
time.
This commit re-enabled it because it is now implemented.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Parse the client authorization section from the descriptor, use the client
private key to decrypt the auth clients, and then use the descriptor cookie to
decrypt the descriptor.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit refactors the existing decryption code to make it compatible with
a new logic for when the client authorization is enabled.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because this secret data building logic is not only used by the descriptor
encoding process but also by the descriptor decoding, refactor the function to
take both steps into account.
Signed-off-by: David Goulet <dgoulet@torproject.org>
The new ClientOnionAuthDir option is introduced which is where tor looks to
find the HS v3 client authorization files containing the client private key
material.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Previously, we encrypted the descriptor without the descriptor cookie. This
commit, when the client auth is enabled, the descriptor cookie is always used.
I also removed the code that is used to generate fake auth clients because it
will not be used anymore.
Signed-off-by: David Goulet <dgoulet@torproject.org>
This commit tests that the descriptor building result, when the client
authorization is enabled, includes everything that is needed.
Signed-off-by: David Goulet <dgoulet@torproject.org>
We need to generate all the related keys when building the descriptor, so that
we can encrypt the descriptor.
Signed-off-by: David Goulet <dgoulet@torproject.org>
It is not supported, and always fails. Some compilers warn about the
function pointer cast on 64-bit Windows.
Fixes bug 27461; bugfix on 0.2.2.23-alpha.
gcc 8 warns that extend_info_t.nickname might be truncated by strncpy().
But it doesn't know that nickname can either contain a hex id, or a
nicknames. hex ids are only used for general and HSDir circuits.
Fixes bug 27463; bugfix on 0.1.1.2-alpha.
GetProcAddress() returns FARPROC, which is (long long int(*)()) on
64-bit Windows:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683212(v=vs.85).aspx
But GetAdaptersAddresses() is (long unsigned int(*)()), on both 32-bit
and 64-bit Windows:
https://docs.microsoft.com/en-us/windows/desktop/api/iphlpapi/nf-iphlpapi-getadaptersaddresses
So gcc 8 issues a spurious "incompatible function pointer" warning
about the cast to GetAdaptersAddresses_fn_t.
Silence this warning by casting to a void function pointer, before
the cast to GetAdaptersAddresses_fn_t.
This issue is already fixed by 26481 in 0.3.5 and later, by removing
the lookup and cast.
Fixes bug 27465; bugfix on 0.2.3.11-alpha.
This reverts commit b5fddbd241.
The commit here was supposed to be a solution for #27451 (fd
management with NSS), but instead it caused an assertion failure.
Fixes bug 27500; but not in any released Tor.
On new glibc versions, there's an explicit_bzero(). With openssl,
there's openssl_memwipe().
When no other approach works, use memwipe() and a memory barrier.
This function was a wrapper around RSA_check_key() in openssl, which
checks for invalid RSA private keys (like those where p or q are
composite, or where d is not the inverse of e, or where n != p*q).
We don't need a function like this in NSS, since unlike OpenSSL, NSS
won't let you import a bogus private key.
I've renamed the function and changed its return type to make it
more reasonable, and added a unit test for trying to read a key
where n != p*q.
This function was supposed to implement a half-duplex mode for our
TLS connections. However, nothing in Tor actually uses it (besides
some unit tests), and the implementation looks really questionable
to me. It's probably best to remove it. We can add a tested one
later if we need one in the future.
The OpenSSL "RSA" object is currently 408 bytes compares to the ASN.1 encoding
which is 140 for a 1024 RSA key.
We save 268 bytes per descriptor (routerinfo_t) *and* microdescriptor
(microdesc_t). Scaling this to 6000 relays, and considering client usually
only have microdescriptors, we save 1.608 MB of RAM which is considerable for
mobile client.
This commit makes it that we keep the RSA onion public key (used for TAP
handshake) in ASN.1 format instead of an OpenSSL RSA object.
Changes is done in both routerinfo_t and microdesc_t.
Closes#27246
Signed-off-by: David Goulet <dgoulet@torproject.org>
TRUNCATED cells were ignored while in path bias. Now they are obeyed, and
cause us to tear down the circuit. The actual impact is minimal, since we
would just wait around for a probe that would never arrive before.
This commit changes client behavior.
We allow their CONNECTEDs, RESOLVEDs, ENDs, SENDMEs, and DATA cells to not
count as dropped until the windows are empty, or we get an END.
This commit does not change behavior. It only changes CIRC_BW event field
values.
By removing Tor2Web, there is no way a client can be non anonymous so we
remove that function and the callsites.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Because we just removed Tor2web support, the need_specific_rp is not needed
anymore when cannibalizing a circuit.
Signed-off-by: David Goulet <dgoulet@torproject.org>
No behaviour change.
A previous fix to chutney removed v3 onion services from the
mixed+hs-v23 network, so seeing "mixed+hs-v23" in tests is
confusing.
Fixes bug 27345; bugfix on 0.3.2.1-alpha.
LinkAuth method 1 is the one where we pull the TLS master secrets
out of the OpenSSL data structures and authenticate them with
RSA. Right now we list method 1 as required for clients and relays.
That's a problem, since we can't reasonably support it with NSS. So
let's remove it as a requirement and a recommendation.
As for method 3: I'd like to recommend it it, but that would make
0.2.9 start warning. Let's not do that till at least some time
after 0.3.5 (the next LTS) is stable.
Closes ticket 27286
Instead, count exits as usable if they have the exit flag, and
present if they also have a non-reject exit policy.
Requiring a threshold of usable descriptors avoids directories trickling
exit descriptors to clients to discover their ExitNodes settings.
Part of 27236.
Previously, Tor would only check the exit flag. In small networks, Tor
could bootstrap once it received a consensus with exits, without fetching
the new descriptors for those exits.
After bootstrap, Tor delays descriptor fetches, leading to failures in
fast networks like chutney.
Fixes 27236; bugfix on 0.2.6.3-alpha.
In order to switch the default HS version from 2 to 3, we need tor to be smart
and be able to decide on the version by trying to load the service keys during
configuration validation.
Part of #27215
Signed-off-by: David Goulet <dgoulet@torproject.org>
Part of #27215, we need to call the ed_key_init_from_file function during
option_validate() which is before the global_options variable is set.
This commit make ed_key_init_from_file() stop using get_options() and instead
now has a or_options_t parameter.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Bug description: For each descriptor, its revision counter is the OPE
ciphertext of the number of seconds since the start time of its SRV value.
This bug caused us to confuse the SRV start time in the middle of the lifetime
of a descriptor in some edge-cases, which caused descriptor rejects.
Bug cause: The bug occurs when we fetch a 23:00 consensus after
midnight (e.g. at 00:08 when not all dirauths have fetched the latest 00:00
consensus). In that case, the voting schedule (which was used for SRV start
time calculation) would return a valid-after past-midnight, whereas our
consensus would be pre-midnight, and that would confuse the SRV start time
computation which is used by HS revision counters (because we would reset the
start time of SRV, without rotating descriptors).
Bug fix: We now use our local consensus time to calculate the SRV start time,
instead of the voting schedule. The voting schedule does not work as originally
envisioned in this case, because it was created for voting by dirauths and not
for scheduling stuff on clients.
We used to link both libraries at once, but now that I'm working on
TLS, there's nothing left to keep OpenSSL around for when NSS is
enabled.
Note that this patch causes a couple of places that still assumed
OpenSSL to be disabled when NSS is enabled
- tor-gencert
- pbkdf2
Also, add a stubbed-out nss version of the modules. The tests won't
pass with NSS yet since the NSS modules don't do anything.
This is a good patch to read with --color-moved.
This cleans up a lot of junk from crypto_rsa_openssl, and will
save us duplicated code in crypto_rsa_nss (when it exists).
(Actually, it already exists, but I am going to use git rebase so
that this commit precedes the creation of crypto_rsa_nss.)
Unlike the old test, this test no will no longer mess around with
the forbidden internals of any openssl data structures.
Additionally, it verifies several other behaviors of
tor_tls_cert_matches_key() that we had wanted to verify, such as
the possibility of the certificate's key not matching.
Fixes bug 27226; bugfix on 0.2.5.1-alpha.
This allows us to mock our own tor_tls_get_peer_certificate()
function in order to test ..cert_matches_key(), which will in turn
allow us to simplify test_tortls_cert_matches_key() considerably.
Prep work for the fix for 27226.
Until https://github.com/rust-lang/rust/issues/52652 is fixed,
unwinding on panic is potentially unsound in a mixed C/Rust codebase.
The codebase is supposed to be panic-free already, but just to be safe.
This started mattering at commit d1820c1516.
Fixes#27199; bugfix on tor-0.3.3.1-alpha.
It's impossible for spaces to get here, since spaces are used as
separators between individual protocol entries higher up.
And it shouldn't ignore whitespace that isn't a literal space
character, because that would differ from the C implementation.
These were added in 9925d2e687.
Fixes#27177. Bugfix on 0.3.3.5-rc.
It was parsing "1-2-3" as if it were 1-2, ignoring the 2nd hyphen
and everything after.
Introduced in d1820c1516.
Fixes#27164; bugfix on 0.3.3.1-alpha.
The function takes an already validated utf-8 string, and
it never checks if the version numbers are an empty string.
That parse error happens later.
Fix on 701c2b69f5
These are the 12 stable and documented configuration options,
set to their default values.
use_small_heuristics is only stabilized in rustfmt 0.9, so maintain
support for 0.8.x for now by commenting it out.
comment_width is unstable and did nothing, since wrap_comments defaults
to false.
Default values gotten from `rustfmt --print-config default rustfmt.toml`.
e7932fa9c2/Configurations.md
Replace master .travis.yml with 034 .travis.yml.
All the changes in master have been backported to the
034 .travis.yml already.
Replace master src/test/test_rust.sh with 034
src/test/test_rust.sh, which was backported from
master. One 033/034-specific commit needs to be
reverted.
Replace 034 .travis.yml with 033 .travis.yml.
Subsequent commits will restore 034 functionality.
Replace 034 src/test/test_rust.sh with 033
src/test/test_rust.sh, which was backported from
master.
Replace 033 .travis.yml with 032 .travis.yml.
Subsequent commits will restore 033 functionality.
src/rust/tor_util/include.am is deleted in 033.
Subsequent commits will apply 032 changes to
src/rust/tor_rust/include.am.
Replace 033 src/test/test_rust.sh with 032
src/test/test_rust.sh, which was backported from
master.
At the same time, sternly warn any person thinking about relying on
any particular format too strictly. If you do this, and your
program breaks, it is your bug, not mine.
I hope that the debian clang maintainers will look at debian bug
903709 soon. But until they do, this should keep our users and our
CI happy on sid with clang.
Closes ticket 26779.
The seccomp rule for the openat syscall checks for the AT_FDCWD
constant. Because this constant is usually a negative value, a
cast to unsigned int is necessary to make sure it does not get
converted to uint64_t used by seccomp.
More info on:
https://github.com/seccomp/libseccomp/issues/69#issuecomment-273805980
When we fixed 25939 in f7633c1fca, we
introduced a call to rescan_periodic_events() from inside the onion
service logic. But this meant that we could rescan the event list --
thereby running event callbacks! -- from inside the hidden service code.
This could cause us to run some of our event callbacks from an
inconsistent state, if we were in the middle of changing options.
A related bug (#25761) prevented us from rescanning our periodic
events as appropriate, but when we fixed THAT one, this bug reared
its ugly head.
The fix here is that "enabling" an event should cause us to run it
from the event loop, but not immediately from the point where we
enable it.
Fixes bug 27003; bugfix on 0.3.4.1-alpha.
We need this so that the tor_api user can specify some arguments,
while the tor_api implementation adds others.
This implementation detail should not be visible to tor_api users.
This change also makes tor_ersatz_socketpair() follow the same
interface as socketpair() rather than tor_socketpair(), so it now
needs to be wrapped in the same code as socketpair() does.
This is comparatively straightforward too, except for a couple of
twists:
* For as long as we're building with two crypto libraries, we
want to seed _both_ their RNGs, and use _both_ their RNGs to
improve the output of crypto_strongest_rand()
* The NSS prng will sometimes refuse to generate huge outputs.
When it does, we stretch the output with SHAKE. We only need
this for the tests.
We would usually call it through tor_cleanup(), but in some code
paths, we wouldn't. These paths would break restart-in-process,
since leaving fields uncleared would cause assertion failures on
restart.
Fixes bug 26948; bugfix on 0.3.3.1-alpha
Conditionalize the pragma that temporarily disables
-Wunused-const-variable. Some versions of gcc don't support it. We
need to do this because of an apparent bug in some libzstd headers.
Fixes bug 26785; bugfix on 0.3.2.11.
Instead, log a protocol warning when single onion services or
Tor2web clients fail to authenticate direct connections to relays.
Fixes bug 26924; bugfix on 0.2.9.1-alpha.
Stop putting ed25519 link specifiers in v3 onion service descriptors,
when the intro point doesn't support ed25519 link authentication.
Fixes bug 26627; bugfix on 0.3.2.4-alpha.
Stop sending ed25519 link specifiers in v3 onion service introduce
cells, when the rendezvous point doesn't support ed25519 link
authentication.
Fixes bug 26627; bugfix on 0.3.2.4-alpha.
Our previous definition implied that code would never keep running
if a BUG occurred (which it does), and that BUG(x) might be true
even if x was false (which it can't be).
Closes ticket 26890. Bugfix on 0.3.1.4-alpha.
This is another attempt to fix 1437668. The assertion here should
be safe, since the rules of networkstatus_get_param() keep the value
it returns in range.
The following bug was causing many issues for this branch in chutney:
In sr_state_get_start_time_of_current_protocol_run() we were using the
consensus valid-after to calculate beginning_of_current_round, but we were
using time(NULL) to calculate the current_round slot. This was causing time
sync issues when the consensus valid-after and time(NULL) were disagreeing on
what the current round is. Our fix is to use the consensus valid-after in both
places.
This also means that we are not using 'now' (aka time(NULL)) anymore in that
function, and hence we can remove that argument from the function (and its
callers). I'll do this in the next commit so that we keep things separated.
Furthermore, we fix a unittest that broke.
We only build a descriptor once, and we just re-encode it (and change its intro
points if needed) before uploading.
Hence we should set the revision counter before uploading, not during building.
The OPE cipher is tied to the current blinded key which is tied to the current
time period. Hence create the OPE cipher structure when we create a new
descriptor (and build its blinded key).
Now that the rev counter depends on the local time, we need to be more careful
in the unittests. Some unittests were breaking because they were using
consensus values from 1985, but they were not updating the local time
appropriately. That was causing the OPE module to complain that it was trying
to encrypt insanely large values.
To do so for a given descriptor, we use the "seconds since the SR protocol run"
started, for the SRV that is relevant to this descriptor. This is guaranteed to
be a positive value (since we need an SRV to be able to build a descriptor),
and it's also guaranteed to be a small value (since SRVs stop being listed on a
consensus after 48 hours).
We cannot use the "seconds since the time period started", because for the next
descriptor we use the next time period, so the timestamp would end up negative.
See [SERVICEUPLOAD] from rend-spec-v3.txt for more details.
To do so, we have to introduce a new `is_current` argument to a bunch of
functions, because to use "seconds since the SR protocol run" we need to know
if we are building the current or the next descriptor, since we use a different
SRV for each descriptor.
This is meant for use when encrypting the current time within the
period in order to get a monotonically increasing revision counter
without actually revealing our view of the time.
This scheme is far from the most state-of-the-art: don't use it for
anything else without careful analysis by somebody much smarter than
I am.
See ticket #25552 for some rationale for this logic.
If an authority is not configured with a V3BandwidthsFile, this line
SHOULD NOT appear in its vote.
If an authority is configured with a V3BandwidthsFile, but parsing
fails, this line SHOULD appear in its vote, but without any headers.
Part of 3723, implements the spec in 26799.
also add tests for bw_file_headers.
Headers are all that is found before a correct relay line or
the terminator.
Tests include:
* a empty bandwidth file
* a bandwidth file with only timestamp
* a bandwidth file with v1.0.0 headers
* a bandwidth file with v1.0.0 headers and relay lines
* a bandwidth file with v1.1.0 headers and v1.0.0 relay lines
* a bandwidth file with v1.0.0 headers, malformed relay lines and
relay lines
* a bandwidth file with v1.0.0 headers, malformed relay lines,
relay lines and malformed relay lines
* a bandwidth file with v1.1.0 headers without terminator
* a bandwidth file with v1.1.0 headers with terminator
* a bandwidth file with v1.1.0 headers without terminator and
relay lines
* a bandwidth file with v1.1.0 headers with terminator and relay
lines
* a bandwidth file with v1.1.0 headers without terminator, bad
relay lines and relay lines
* a bandwidth file with v1.1.0 headers with terminator, bad relay
lines and relay lines
If bandwidth file terminator is found, set end of headers flag
and do not store the line.
If it is not, parse a relay line and check whether it is a header
line.
* add bwlist_headers argument to dirserv_read_measured_bandwidth
in order to store all the headers found when parsing the file
* add bwlist_headers to networkstatus_t in order to store the
the headers found by the previous function
* include the bandwidth headers as string in vote documents
* add test to check that dirserv_read_measured_bandwidth generates
the bwlist_headers
Coverity rightly complains that early in the function we're checking
whether username is NULL, and later we're passing it unconditionally
to strlen().
Fixes CID 1437967. Bug not in any released Tor.
We need this in our unit tests, since otherwise NSS will notice
we've forked and start cussing us out.
I suspect we'll need a different hack for daemonizing, but this
should be enough for tinytest to work.
This patch adds two assertions in get_net_param_from_list() to ensure
that the `res` value is correctly within the range of the output domain.
Hopefully fixes Coverity CID #1415721, #1415722, and #1415723.
See: https://bugs.torproject.org/26780
The "Bifroest" bridge authority has been retired; the new bridge authority
is "Serge", and it is operated by George from the TorBSD project.
Closes ticket 26771.
These are now part of crypto_init.c. The openssl-only parts now
live in crypto_openssl_mgt.c.
I recommend reviewing this patch with -b and --color-moved.
We have to check for ERR_load_KDF_strings() here, since that's the
only one that's actually a function rather than a macro.
Fixes compilation with LibreSSL. Fixes bug 26712; bug not in
any released Tor.
That place is git-revision.c; git-revision.c now lives in lib/log.
Also fix the compilation rules so that all object files that need
micro-revision.i depend on it.
Fun fact: these files used to be called log.[ch] until we ran into
conflicts with systems having a log.h file. But now that we always
include "lib/log/log.h", we should be fine.
This function has a nasty API, since whether or not it invokes the
resolver depends on whether one of its arguments is NULL. That's a
good way for accidents to happen.
This patch incidentally makes tor-resolve support socks hosts on
IPv6.
These are now combined into an inaddr.[ch], since their purpose is
to implement functions for struct in_addr and struct in6_addr.
The definitions for in6_addr and its allies are now in a separate
header, inaddr_st.h.
Closes ticket 26532.
This commit won't build yet -- it just puts everything in a slightly
more logical place.
The reasoning here is that "src/core" will hold the stuff that every (or
nearly every) tor instance will need in order to do onion routing.
Other features (including some necessary ones) will live in
"src/feature". The "src/app" directory will hold the stuff needed
to have Tor be an application you can actually run.
This commit DOES NOT refactor the former contents of src/or into a
logical set of acyclic libraries, or change any code at all. That
will have to come in the future.
We will continue to move things around and split them in the future,
but I hope this lays a reasonable groundwork for doing so.
This is a very gentle commit that just lays the groundwork in the
build system: it puts the include files to build libtor-app.a into
src/core, and to build the tor executable into src/app. The
executable is now "src/app/tor".
This is temporary, until src/or is split.
Putting this in containers would be another logical alternative,
except that addresses depend on containers, and we don't like
cycles.
When on a private network a single directory authority is explicitly
configured, allow that DA's instace of tor to pick itself as its own
DA, by having router_pick_dirserver_generic() set the PDS_ALLOW_SELF
flag when the list of potential sources contains only one element.
This would subsequently allow router_pick_trusteddirserver_impl() to
calculate is 'requreother' condition as 'false', enabling it to pick
itself as a valid DA candidate.
Fixes#25928
Signed-off-by: Gabriel Somlo <gsomlo@gmail.com>
Recent Python3 versions seem to require this on Windows.
Fixes bug 26535; bug introduced in f4be34f70d, which
was apparently intended itself as a Python3 workaround.
When we do redefine them, use inline functions instead of #define.
This fixes a latent code problem in our redefinition of these
functions, which was exposed by our refactoring: Previously, we
would #define strcasecmp after string.h was included, so nothing bad
would happen. But when we refactored, we would sometimes #define it
first, which was a problem on mingw, whose headers contain
(approximately):
inline int strcasecmp (const char *a, const char *b)
{ return _stricmp(a,b); }
Our define turned this into:
inline int _stricmp(const char *a, const char *b)
{ return _stricmp(a,b); }
And GCC would correctly infer that this function would loop forever,
rather than actually comparing anything. This caused bug 26594.
Fixes bug 26594; bug not in any released version of Tor.
This code was in compat_threads, since it was _used_ for efficiently
notifying the main libevent thread from another thread. But in
spite of its usage, it's fundamentally a part of the network code.
The "conffile" module knows about includes and filesystem access,
whereas confline doesn't. This will make it possible to put these
functions into libraries without introducing a cycle.
For HSv3, the HSADDRESS= wasn't properly parsed for the HSPOST command. It now
correctly use it and furthermore sends back a "200 OK" in case the command is
successful for a v3 descriptor.
Fixes#26523
Signed-off-by: David Goulet <dgoulet@torproject.org>
The smartlist_core library now contains only the parts of smartlists
that are needed for the logging library. This resolves the
circularity between "container" and "log".
The "containers" library still uses the logging code, and has the
higher-level smartlist functions.
Fixes bug 26480; bug appeared when we re-enabled the geoip tests on
windows. Bug originally introduced by our fix to 25787; bug not in
any released Tor.
This patch fixes a memory leak in disk_state_parse_commits() where if
commit is NULL, we continue the internal loop, but without ever freeing
the args variable.
See: Coverity CID 1437441.
This patch fixes a memory leak in frac_nodes_with_descriptors() where
we might return without free'ing the bandwidths variable.
See: Coverity CID 1437451.
This patch fixes a memory leak in new_establish_intro_cell() that could
happen if a test assertion fails and the *cell_out value isn't properly
free'd.
See: Coverity CID 1437445
This patch fixes a memory leak in decode_link_specifiers() where the
hs_spec variable might leak if the default label is taken in the
switch/case expression.
See: Coverity CID 1437437.
This patches fixes a memory leak in client_likes_consensus() where if
consensus_cache_entry_get_voter_id_digests() would fail we would return
without having free'd the voters list.
See: Coverity CID 1437447
This patch fixes a memory leak in hs_helper_build_hs_desc_impl() where
if a test assertion would fail we would leak the storage that `desc`
points to.
See: Coverity CID 1437448
This patch fixes a memory leak in pick_hsdir_v3() where we might return
early, but forgot to free the responsible_hsdirs variable. We solve this
by not allocating storage for responsible_hsdirs until it's actually
needed.
See: Coverity CID 1437449
This patch fixes a potential memory leak in test_hs_auth_cookies() if a
test-case fails and we goto the done label where no memory clean up is
done.
See: Coverity CID 1437453
This patch fixes a potential memory leak in
hs_helper_build_intro_point() where a `goto done` is called before the
`intro_point` variable have been assigned to the value of the `ip`
variable.
See: Coverity CID 1437460
See: Coverity CID 1437456
This patch:
- introduces an fdio module for low-level fd functions that don't
need to log.
- moves the responsibility for opening files outside of torlog.c,
so it won't need to call tor_open_cloexec.
The locking code gets its own module, since it's more fundamental
than the higher-level locking code.
Extracting the logging code was the whole point here. :)
Out-of-tree builds could fail to run the rust tests if built in
offline mode. cargo expects CARGO_HOME to point to the .cargo
directory, not the directory containing .cargo.
Fixes bug 26455; bug not in any released tor.
Includes configuration files to enforce these rules on lib and
common. Of course, "common" *is* a modularity violation right now,
so these rules aren't as strict as I would like them to be.
I am calling the crypto library "crypt_ops", since I want
higher-level crypto things to be separated from lower-level ones.
This library will hold only the low-level ones, once we have it
refactored.
When I wrote the first one of these, it needed the path of the geoip
file. But that doesn't translate well in at least two cases:
- Mingw, where the compile-time path is /c/foo/bar and the
run-time path is c:\foo\bar.
- Various CI weirdnesses, where we cross-compile a test binary,
then copy it into limbo and expect it to work.
Together, these problems precluded these tests running on windows.
So, instead let's just generate some minimal files ourselves, and
test against them.
Fixes bug 25787
Fix a memory leak where directory authorities would leak a chunk of
memory for every router descriptor every time they considered voting.
This bug was taking down directory authorities in the live network due
to out-of-memory issues.
Fixes bug 26435; bugfix on 0.3.3.6.
We'd like to feature gate code that calls C from Rust, as a workaround
to several linker issues when running `cargo test` (#25386), and we
can't feature gate anything out of test code if `cargo test` is called
with `--all-features`.
* FIXES#26400: https://bugs.torproject.org/26400
Previously we had code like this for bad things happening from
signal handlers, but it makes sense to use the same logic to handle
cases when something is happening at a level too low for log.c to be
involved.
My raw_assert*() stuff now uses this code.
We had accumulated a bunch of cruft here. Now let's only include
src and src/ext. (exception: src/trunnel is autogenerated code, and
need to include src/trunnel.)
This commit will break the build hard. The next commit will fix it.
The doctests for src/rust/crypto don't compile for multiple reasons,
including some missing exports and incorrect identifier paths. Fixes
bug 26415; bugfix on 0.3.4.1-alpha.
We need this trick because some of our Rust tests depend on our C
code, which in turn depend on other native libraries, which thereby
pulls a whole mess of our build system into "cargo test".
To solve this, we add a build script (build.rs) to set most of the
options that we want based on the contents of config.rust. Some
options can't be set, and need to go to the linker directly: we use
a linker replacement (link_rust.sh) for these. Both config.rust and
link_rust.sh are generated by autoconf for us.
This patch on its own should enough to make the crypto test build,
but not necessarily enough to make it pass.
After the big or.h refactoring, one single unit test file was missing two
headers for node_t and microdesc_t.
Signed-off-by: David Goulet <dgoulet@torproject.org>
Due to linker issues (#25386) when testing Rust code which calls C,
all tests which touch FFI code should now be feature-gated behind the
"test-c-from-rust" flag. To run this test code, cargo must be called
with `cargo test --features="test-c-from-rust"`.
* FIXES#26398: https://bugs.torproject.org/26398
Also make sure that we're actually running the test from within the right
cwd, like we do when we're building. This seems necessary to avoid
an error when running offline.
Amusingly, it appears that we had this bug before: we just weren't
noticing it, because of bug 26258.
Some versions of GCC complain that the bfn_mock_node_get_by_id
function might return NULL, but we're assuming that it won't.
(We're assuming it won't return NULL because we know in the tests
that we're passing it valid IDs.)
To make GCC happy, tt_assert() that each node_t is set before using
it.
Fixes a second case of bug26269; bugfix on 0.3.0.1-alpha.
Here is how this changes the HSv3 client-side and service-side:
For service side we already required live consensus to upload descriptors (see
9e900d1db7) so we should never get there without
a live consensus.
For the client-side we now require a live consensus to attempt to connect to
HS. While this changes the client behavior in principle, it doesn't really
change it, because we always required live consensus to set HSDir indices, so
before this patch a client with no live consensus would try to compute
responsible HSDirs without any HSDir indices and bug out. This makes the client
behavior more consistent, by requiring a live consensus (and hence a
semi-synced clock) for the client to connect to an HS entirely.
The alternative would have been to allow setting HSDir indices with a non-live
consensus, but this would cause the various problems outlined by commit
b89d2fa1db.
There are a few reasons that relays might be uploading desciptors
without saying X-Desc-Gen-Reason:
1. They are running an old version of our software, before 0.3.2.stable.
2. They are not running our software, but they are claiming they
are.
3. They are uploading through a proxy that strips X-Desc-Gen-Reason.
4. They somehow had a bug in their software.
According to the 25686 data, 1 is the most common reason. This
ticket is an attempt to diagnose case 4, or prove that case 4
doesn't actually happen.
With the work on #25500 (reducing CPU client usage), the HS service main loop
callback is enabled as soon as the HS service map changes which happens when
registering a new service.
Unfortunately, for an ephemeral service, we were building the onion address
*after* the registration leading to the "service->onion_address` to be an
empty string.
This broke the "HS_DESC CREATED" event which had no onion address in it. And
also, we were logging an empty onion address for that service.
Fixes#25939
Signed-off-by: David Goulet <dgoulet@torproject.org>
* REFACTORS `UnvalidatedProtoEntry::from_str` to place the bulk of the
splitting/parsing logic in to a new
`UnvalidatedProtoEntry::parse_protocol_and_version_str()` method (so that
both `from_str()` and `from_str_any_len()` can call it.)
* ADD a new `UnvalidatedProtoEntry::from_str_any_len()` method in order to
maintain compatibility with consensus methods older than 29.
* ADD a limit on the number of characters in a protocol name.
* FIXES part of #25517: https://bugs.torproject.org/25517
In protover.c, the `expand_protocol_list()` function expands a `smartlist_t` of
`proto_entry_t`s to their protocol name concatenated with each version number.
For example, given a `proto_entry_t` like so:
proto_entry_t *proto = tor_malloc(sizeof(proto_entry_t));
proto_range_t *range = tor_malloc_zero(sizeof(proto_range_t));
proto->name = tor_strdup("DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa");
proto->ranges = smartlist_new();
range->low = 1;
range->high = 65536;
smartlist_add(proto->ranges, range);
(Where `[19KB]` is roughly 19KB of `"a"` bytes.) This would expand in
`expand_protocol_list()` to a `smartlist_t` containing 65536 copies of the
string, e.g.:
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=1"
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=2"
[…]
"DoSaaaaaaaaaaaaaaaaaaaaaa[19KB]aaa=65535"
Thus constituting a potential resource exhaustion attack.
The Rust implementation is not subject to this attack, because it instead
expands the above string into a `HashMap<String, HashSet<u32>` prior to #24031,
and a `HashMap<UnvalidatedProtocol, ProtoSet>` after). Neither Rust version is
subject to this attack, because it only stores the `String` once per protocol.
(Although a related, but apparently of too minor impact to be usable, DoS bug
has been fixed in #24031. [0])
[0]: https://bugs.torproject.org/24031
* ADDS hard limit on protocol name lengths in protover.c and checks in
parse_single_entry() and expand_protocol_list().
* ADDS tests to ensure the bug is caught.
* FIXES#25517: https://bugs.torproject.org/25517
* REFACTORS `UnvalidatedProtoEntry::from_str` to place the bulk of the
splitting/parsing logic in to a new
`UnvalidatedProtoEntry::parse_protocol_and_version_str()` method (so that
both `from_str()` and `from_str_any_len()` can call it.)
* ADD a new `UnvalidatedProtoEntry::from_str_any_len()` method in order to
maintain compatibility with consensus methods older than 29.
* ADD a limit on the number of characters in a protocol name.
* FIXES part of #25517: https://bugs.torproject.org/25517
