base16_decodes() now returns the number of decoded bytes. It's interface
changes from returning a "int" to a "ssize_t". Every callsite now checks the
returned value.
Fixes#14013
Signed-off-by: David Goulet <dgoulet@torproject.org>
When deleting an ephemeral HS, we were only iterating on circuit with an
OPEN state. However, it could be possible that an intro point circuit didn't
reached the open state yet.
This commit makes it that we close the circuit regardless of its state
except if it was already marked for close.
Fixes#18604
Signed-off-by: David Goulet <dgoulet@torproject.org>
The FetchHidServDescriptors check was placed before the descriptor cache
lookup which made the option not working because it was never using the
cache in the first place.
Fixes#18704
Patched-by: twim
Signef-off-by: David Goulet <dgoulet@torproject.org>
When you divide an int by an int and get a fraction and _then_ cast
to double, coverity assumes that you meant to cast to a double
first.
In my fix for -Wfloat-conversion in 493499a339, I
did something like this that coverity didn't like.
Instead, I'm taking another approach here.
Fixes CID 1232089, I hope.
This is a big-ish patch, but it's very straightforward. Under this
clang warning, we're not actually allowed to have a global variable
without a previous extern declaration for it. The cases where we
violated this rule fall into three roughly equal groups:
* Stuff that should have been static.
* Stuff that was global but where the extern was local to some
other C file.
* Stuff that was only global when built for the unit tests, that
needed a conditional extern in the headers.
The first two were IMO genuine problems; the last is a wart of how
we build tests.
This warning triggers on silently promoting a float to a double. In
our code, it's just a sign that somebody used a float by mistake,
since we always prefer double.
This warning, IIUC, means that the compiler doesn't like it when it
sees a NULL check _after_ we've already dereferenced the
variable. In such cases, it considers itself free to eliminate the
NULL check.
There are a couple of tricky cases:
One was the case related to the fact that tor_addr_to_in6() can
return NULL if it gets a non-AF_INET6 address. The fix was to
create a variant which asserts on the address type, and never
returns NULL.
Previously, we used !directory_fetches_from_authorities() to predict
that we would tunnel connections. But the rules have changed
somewhat over the course of 0.2.8
So, back long ago, XXX012 meant, "before Tor 0.1.2 is released, we
had better revisit this comment and fix it!"
But we have a huge pile of such comments accumulated for a large
number of released versions! Not cool.
So, here's what I tried to do:
* 0.2.9 and 0.2.8 are retained, since those are not yet released.
* XXX+ or XXX++ or XXX++++ or whatever means, "This one looks
quite important!"
* The others, after one-by-one examination, are downgraded to
plain old XXX. Which doesn't mean they aren't a problem -- just
that they cannot possibly be a release-blocking problem.
There was a > that should have been an ==, and a missing !. These
together prevented us from issuing a warning in the case that a
nickname matched an Unnamed node only.
Fixes bug 19203; bugfix on 0.2.3.1-alpha.
Remove support for "GET /tor/bytes.txt" DirPort request, and
"GETINFO dir-usage" controller request, which were only available
via a compile-time option in Tor anyway.
Feature was added in 0.2.2.1-alpha. Resolves ticket 19035.
I introduced this bug when I moved signing_key_cert into
signed_descriptor_t. Bug not in any released Tor. Fixes bug 19175, and
another case of 19128.
Just like signed_descriptor_from_routerinfo(), routerlist_reparse_old()
copies the fields from one signed_descriptor_t to another, and then
clears the fields from the original that would have been double-freed by
freeing the original. But when I fixed the s_d_f_r() bug [#19128] in
50cbf22099, I missed the fact that the code was duplicated in
r_p_o().
Duplicated code strikes again!
For a longer-term solution here, I am not only adding the missing fix to
r_p_o(): I am also extracting the duplicated code into a new function.
Many thanks to toralf for patiently sending me stack traces until
one made sense.
Now that the field exists in signed_descriptor_t, we need to make
sure we free it when we free a signed_descriptor_t, and we need to
make sure that we don't free it when we convert a routerinfo_t to a
signed_descriptor_t.
But not in any released Tor. I found this while working on #19128.
One problem: I don't see how this could cause 19128.
With the fix for #17150, I added a duplicate certificate here. Here
I remove the original location in 0.2.8. (I wouldn't want to do
that in 027, due to the amount of authority-voting-related code
drift.)
Closes 19073.
This API change makes it so that routerinfo_incompatible...() no
longer takes a routerinfo_t, so that it's obvious that it should
only look at fields from the signed_descriptor_t.
This change should prevent a recurrence of #17150.
We need this field to be in signed_descriptor_t so that
routerinfo_incompatible_with_extrainfo can work correctly (#17150).
But I don't want to move it completely in this patch, since a great
deal of the code that messes with it has been in flux since 0.2.7,
when this ticket was opened. I should open another ticket about
removing the field from routerinfo_t and extrainfo_t later on.
This patch fixes no actual behavior.
The routerinfo we pass to routerinfo_incompatible_with_extrainfo is
the latest routerinfo for the relay. The signed_descriptor_t, on
the other hand, is the signed_descriptor_t that corresponds to the
extrainfo. That means we should be checking the digest256 match
with that signed_descriptor_t, not with the routerinfo.
Fixes bug 17150 (and 19017); bugfix on 0.2.7.2-alpha.
When parsing detached signature, we make sure that we use the length of the
digest algorithm instead of an hardcoded DIGEST256_LEN in order to avoid
comparing bytes out of bound with a smaller digest length such as SHA1.
Fixes#19066
Signed-off-by: David Goulet <dgoulet@torproject.org>
We know there are overflows in curve25519-donna-c32, so we'll have
to have that one be fwrapv.
Only apply the asan, ubsan, and trapv options to the code that does
not need to run in constant time. Those options introduce branches
to the code they instrument.
(These introduced branches should never actually be taken, so it
might _still_ be constant time after all, but branch predictors are
complicated enough that I'm not really confident here. Let's aim for
safety.)
Closes 17983.
Make directory authorities write the v3-status-votes file out
to disk earlier in the consensus process, so we have the votes
even if we abort the consensus process later on.
Resolves ticket 19036.
In dirserv_compute_performance_thresholds, we allocate arrays based
on the length of 'routers', a list of routerinfo_t, but loop over
the nodelist. The 'routers' list may be shorter when relays were
filtered by routers_make_ed_keys_unique, leading to an out-of-bounds
write on directory authorities.
This bug was originally introduced in 26e89742, but it doesn't look
possible to trigger until routers_make_ed_keys_unique was introduced
in 13a31e72.
Fixes bug 19032; bugfix on tor 0.2.8.2-alpha.
This was one of our longest functions, at 600 lines. It makes a nice
table-driven URL-based function instead.
The code is a bit ugly, it leave the indentation as it is in hopes of
making pending directory.c changes easier to merge. Later we can
clean up the indentation.
Also, remove unused mallinfo export code from directory.c
Closes ticket 16698
Previously, we were using the generic schedule for some downloads,
and the consensus schedule for others.
Resolves ticket 18816; fix on fddb814fe in 0.2.4.13-alpha.
We used to be locked in to the "tap" handshake length, and now we can
handle better handshakes like "ntor".
Resolves ticket 18998.
I checked that relay_send_command_from_edge() behaves fine when you
hand it a payload with length 0. Clients behave fine too, since current
clients remain strict about the required length in the rendezvous2 cells.
(Clients will want to become less strict once they have an alternate
format that they're willing to receive.)
we should avoid launching a consensus fetch if we don't want one,
but if we do end up with an extra one, we should let the other checks
take care of it.
We'll back off from the request in connection_ap_handshake_attach_circuit,
or cancel it in connection_dir_close_consensus_fetches, and those are the
only places we need to check.
Tor stores client authorization cookies in two slightly different forms.
The service's client_keys file has the standard base64-encoded cookie,
including two chars of padding. The hostname file and the client remove
the two padding chars, and store an auth type flag in the unused bits.
The distinction makes no sense. Refactor all decoding to use the same
function, which will accept either form, and use a helper function for
encoding the truncated format.
This improves client anonymity and avoids directory header tampering.
The extra load on the authorities should be offset by the fallback
directories feature.
This also simplifies the fixes to #18809.
Delete an unnecessary check for non-preferred IP versions.
Allows clients which can't reach any directories of their
preferred IP address version to get directory documents.
Patch on #17840 in 0.2.8.1-alpha.
After #17840 in 0.2.8.1-alpha, we incorrectly chose an IPv4
address for all DIRIND_ONEHOP directory connections,
even if the routerstatus didn't have an IPv4 address.
This likely affected bridge clients with IPv6 bridges.
Resolves#18921.
The problem is that "q" is always set on the first iteration even
if the question is not a supported question. This set of "q" is
not necessary, and will be handled after exiting the loop if there
if a supported q->type was found.
[Changes file by nickm]
lease enter the commit message for your changes. Lines starting
Decide to advertise begindir support in a similar way to how
we decide to advertise DirPort.
Fix up the associated descriptor-building unit tests.
Resolves#18616, bugfix on 0c8e042c30 in #12538 in 0.2.8.1-alpha.
Apparently somewhere along the line we decided that MIN might be
missing.
But we already defined it (if it was missing) in compat.h, which
everybody includes.
Closes ticket 18889.
When we connect to a hidden service as a client we may need three internal
circuits, one for the descriptor retrieval, introduction, and rendezvous.
Let's try to make sure we have them. Closes#13239.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Unlike tor_assert(), these macros don't abort the process. They're
good for checking conditions we want to warn about, but which don't
warrant a full crash.
This commit also changes the default implementation for
tor_fragile_assert() to tor_assert_nonfatal_unreached_once().
Closes ticket 18613.
When the directory authorities refuse a bad relay's descriptor,
encourage the relay operator to contact us. Many relay operators
won't notice this line in their logs, but it's a win if even a
few learn why we don't like what their relay was doing.
Resolves ticket 18760.
I didn't specify a contact mechanism (e.g. an email address), because
every time we've done that in the past, a few years later we noticed
that the code was pointing people to an obsolete contact address.
This changes simply renames them by removing "Testing" in front of them and
they do not require TestingTorNetwork to be enabled anymore.
Fixes#18481
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Only when we were actually flushing the cell stats to a controller
would we free them. Thus, they could stay in RAM even after the
circuit was freed (eg if we didn't have any controllers).
Fixes bug 18673; bugfix on 0.2.5.1-alpha.
They incorrectly summarized what the function was planning to do,
leading to wrong behavior like making an http request to an orport,
or making a begindir request to a dirport.
This change backs out some of the changes made in commit e72cbf7a, and
most of the changes made in commit ba6509e9.
This patch resolves bug 18625. There more changes I want to make
after this one, for code clarity.
This change allows us to simplify path selection for clients, and it
should have minimal effect in practice since >99% of Guards already have
the Stable flag. Implements ticket 18624.
Commit e72cbf7a4 introduced a change to directory_initiate_command_rend()
that made tor use the ORPort when making a directory request to the DirPort.
The primary consequence was that a relay couldn't selftest its DirPort thus
failing to work and join the network properly.
The main issue was we were always considering an anonymized connection to be
an OR connection which is not true.
Fixes#18623
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Regardless of the setting of ExtendAllowPrivateAddresses.
This fixes a bug with pluggable transports that ignore the
(potentially private) address in their bridge line.
Fixes bug 18517; bugfix on 23b088907f in tor-0.2.8.1-alpha.
When requesting extrainfo descriptors from a trusted directory
server, check whether it is an authority or a fallback directory
which supports extrainfo descriptors.
Fixes bug 18489; bugfix on 90f6071d8d in tor-0.2.4.7-alpha.
Reported by "atagar", patch by "teor".
Make it clearer that they are about outgoing connection attempts.
Specify the options involved where they were missing from one log
message.
Clarify a comment.
Downgrade logs and backtraces about IP versions to
info-level. Only log backtraces once each time tor runs.
Assists in diagnosing bug 18351; bugfix on c3cc8e16e in
tor-0.2.8.1-alpha.
Reported by "sysrqb" and "Christian", patch by "teor".
commit edeba3d4 removed a switch, but left the "break" lines in
from that switch. fortunately the resulting behavior was not wrong,
since there was an outer switch that it was ok to break from.
no actual changes here -- but the new indenting makes it clear
that the fixes in #18332 were not as good as they should have been.
the next commit will deal with that.
We've got to make sure that every single subsequent calculation in
dirserv_generate_networkstatus_vote_obj() are based on the list of
routerinfo_t *after* we've removed possible duplicates, not before.
Fortunately, none of the functions that were taking a routerlist_t
as an argument were actually using any fields other than this list
of routers.
Resolves issue 18318.DG3.
I had a half-built mechanism to track, during the voting process,
whether the Ed25519 value (or lack thereof) reflected a true
consensus among the authorities. But we never actually inserted this
field in the consensus.
The key idea here is that we first attempt to match up votes by pairs
of <Ed,RSA>, where <Ed> can be NULL if we're told that there is no
Ed key. If this succeeds, then we can treat all those votes as 'a
consensus for Ed'. And we can include all other votes with a
matching RSA key and no statement about Ed keys as being "also about
the same relay."
After that, we look for RSA keys we haven't actually found an entry
for yet, and see if there are enough votes for them, NOT considering
Ed keys. If there are, we match them as before, but we treat them
as "not a consensus about ed".
When we include an entry in a consensus, if it does not reflect a
consensus about ed keys, then we include a new NoEdConsensus flag on
it.
This is all only for consensus method 22 or later.
Also see corresponding dir-spec patch.
When generating a vote, and we have two routerinfos with the same ed
key, omit the one published earlier.
This was supposed to have been solved by key pinning, but when I
made key pinning optional, I didn't realize that this would jump up
and bite us. It is part of bug 18318, and the root cause of 17668.
If we're a server with no address configured, resolve_my_hostname
will need this. But not otherwise. And the preseeding itself can
consume a few seconds if like tails we have no resolvers.
Fixes bug 18548.
I didn't want to grant blanket permissions for chmod() and chown(),
so here's what I had to do:
* Grant open() on all parent directories of a unix socket
* Write code to allow chmod() and chown() on a given file only.
* Grant chmod() and chown() on the unix socket.
This is a part of a fix for 18253; bugfix on 0.2.8.1-alpha.
Alternatively, we could permit chmod/chown in the sandbox, but I
really don't like giving the sandbox permission to alter
permissions.
Launching 7 descriptor fetches makes a connection to each HSDir that is 6
and the seventh one fails to pick an HSDir because they are all being used
already so it was killing all pending connections at once.
Fixes#15937
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
When we made HidServDirectoryV2 always 1, we removed the situation
where a relay could choose not to be an HSDir. Now simplify the
rest of the code to reflect this decision.
(We have to remove two apparently unrelated free() calls in the unit
tests, since they used to free stuff that we created as a side effect
of calling router_get_my_routerinfo(), and now we no longer call that.)
This simplifies relay behavior, because the relay offers the hsdir
functionality independent of whether the directory authorities have
decided this relay is suitable for clients to use yet.
Implements ticket 18332.
The transproxy feature is only enabled when __FreeBSD__ is defined, and
only regular FreeBSD does that. Change this to __FreeBSD_kernel__ which
is defined on derivatives as well.
This enables the relevant options/validate__transproxy test on FreeBSD
derivatives.
Allow fallback directories which have been stable for 7 days
to work around #18050, which causes relays to submit descriptors
with 0 DirPorts when restarted. (Particularly during Tor version
upgrades.)
Ignore low fallback directory count in alpha builds.
Set the target count to 50.
This is in accordance with our usual policy against freelists,
now that working allocators are everywhere.
It should also make memarea.c's coverage higher.
I also doubt that this code ever helped performance.
Previously, I had left in some debugging code with /*XXX*/ after it,
which nobody noticed. Live and learn! Next time I will use /*XXX
DO NOT COMMIT*/ or something.
We need to define a new consensus method for this; consensus method
21 shouldn't actually be used.
Fixes bug 17702; bugfix on 0.2.7.2-alpha.
We already write out bootstrapping progress (see bug 9927) per new
microdesc batch. There's no need to do a full "I learned some more
directory information, but not enough to..." line each time too.
Now, when a user who has set EntryNodes finishes bootstrapping, Tor
automatically repopulates the guard set based on this new directory
information. Fixes bug 16825; bugfix on 0.2.3.1-alpha.
Originally it can overflow in some weird cases. Now it should no longer
be able to do so.
Additionally, limit main's timers to 30 days rather than to 38 years;
we don't actually want any 38-year timers.
Closes bug 17682.
They are no longer "all" digests, but only the "common" digests.
Part of 17795.
This is an automated patch I made with a couple of perl one-liners:
perl -i -pe 's/crypto_digest_all/crypto_common_digests/g;' src/*/*.[ch]
perl -i -pe 's/\bdigests_t\b/common_digests_t/g;' src/*/*.[ch]
If unix socket was configured as listener (such as a ControlSocket or a
SocksPort unix socket), and tor was started as root but not configured
to switch to another user, tor would segfault while trying to string
compare a NULL value. Fixes bug 18261; bugfix on 0.2.8.1-alpha. Patch
by weasel.
When ClientPreferIPv6ORPort is auto, bridges prefer the configured
bridge ORPort address. Otherwise, they use the value of the option.
Other clients prefer IPv4 ORPorts if ClientPreferIPv6ORPort is auto.
When ClientPreferIPv6DirPort is auto, all clients prefer IPv4 DirPorts.
Modify callers to correctly handle these new NULL returns:
* fix assert in onion_extend_cpath
* warn and discard circuit in circuit_get_open_circ_or_launch
* warn, discard circuit, and tell controller in handle_control_extendcircuit
Bridge clients ignore ClientUseIPv6, acting as if it is always 1.
This preserves existing behaviour.
Make ClientPreferIPv6OR/DirPort auto by default:
* Bridge clients prefer IPv6 by default.
* Other clients prefer IPv4 by default.
This preserves existing behaviour.
ClientUseIPv4 0 tells tor to avoid IPv4 client connections.
ClientPreferIPv6DirPort 1 tells tor to prefer IPv6 directory connections.
Refactor policy for IPv4/IPv6 preferences.
Fix a bug where node->ipv6_preferred could become stale if
ClientPreferIPv6ORPort was changed after the consensus was loaded.
Update documentation, existing code, add unit tests.
node_get_all_orports and router_get_all_orports incorrectly used or_port
with IPv6 addresses. They now use ipv6_orport.
Also refactor and remove duplicated code.
Avoid using a pronoun where it makes comments unclear.
Avoid using gender for things that don't have it.
Avoid assigning gender to people unnecessarily.
Allow fallback directories which have been stable for 30 days
to work around #18050, which causes relays to submit descriptors
with 0 DirPorts when restarted. (Particularly during Tor version
upgrades.)
Ignore low fallback directory count in alpha builds.
Otherwise, relays publish a descriptor with DirPort 0 when the DirPort
reachability test takes longer than the ORPort reachability test.
Closes bug #18050. Reported by "starlight", patch by "teor".
Bugfix on 0.1.0.1-rc, commit a1f1fa6ab on 27 Feb 2005.
* DIGEST_SHA3_[256,512] added as supported algorithms, which do
exactly what is said on the tin.
* test/bench now benchmarks all of the supported digest algorithms,
so it's possible to see just how slow SHA-3 is, though the message
sizes could probably use tweaking since this is very dependent on
the message size vs the SHA-3 rate.
This will give relay operators the ability of disabling the caching of
directory data. In general, this should not be necessary, but on some
lower-resource systems it may beneficial.
I believe that the final SMARTLIST_DEL_CURRENT was sometimes
double-removing items that had already been removed by
connection_mark_unattached_ap or
connection_ap_handshake_attach_circuit().
The fix here is to prevent iteration over the list that other
functions might be modifying.
According to the POSIX standard the option value is a pointer to void
and the option length a socklen_t. The Windows implementation makes the
option value be a pointer to character and the option length an int.
Casting the option value to a pointer to void conforms to the POSIX
standard while the implicit cast to a pointer to character conforms to
the Windows implementation.
The casts of the option length to the socklen_t data type conforms to
the POSIX standard. The socklen_t data type is actually an alias of an
int so it also conforms to the Windows implementation.
It is AP-specific, so that's where it belongs. This shouldn't have
caused a bug, but due to #17876, we were never actually calling
connection_edge_about_to_close from connection_ap_about_to_close,
causing bug #17874 (aka bug #17752).
When a relay does not have an open directory port but it has an
orport configured and is accepting client connections then it can
now service tunnelled directory requests, too. This was already true
of relays with an dirport configured.
We also conditionally stop advertising this functionality if the
relay is nearing its bandwidth usage limit - same as how dirport
advertisement is determined.
Partial implementation of prop 237, ticket 12538
Applies the 6c443e987d fix to router_pick_directory_server_impl.
6c443e987d applied to directory servers chosen from the consensus,
and was:
"Tweak the 9969 fix a little
If we have busy nodes and excluded nodes, then don't retry with the
excluded ones enabled. Instead, wait for the busy ones to be nonbusy."
These IPv6 addresses must be quoted, because : is the port separator,
and "acce" is a valid hex block.
Add unit tests for assumed actions in IPv6 policies.
"Tor has included a feature to fetch the initial consensus from nodes
other than the authorities for a while now. We just haven't shipped a
list of alternate locations for clients to go to yet.
Reasons why we might want to ship tor with a list of additional places
where clients can find the consensus is that it makes authority
reachability and BW less important.
We want them to have been around and using their current key, address,
and port for a while now (120 days), and have been running, a guard,
and a v2 directory mirror for most of that time."
Features:
* whitelist and blacklist for an opt-in/opt-out trial.
* excludes BadExits, tor versions that aren't recommended, and low
consensus weight directory mirrors.
* reduces the weighting of Exits to avoid overloading them.
* places limits on the weight of any one fallback.
* includes an IPv6 address and orport for each FallbackDir, as
implemented in #17327. (Tor won't bootstrap using IPv6 fallbacks
until #17840 is merged.)
* generated output includes timestamps & Onionoo URL for traceability.
* unit test ensures that we successfully load all included default
fallback directories.
Closes ticket #15775. Patch by "teor".
OnionOO script by "weasel", "teor", "gsathya", and "karsten".
* The option is now KeepBindCapabilities
* We now warn if the user specifically asked for KeepBindCapabilities
and we can't deliver.
* The unit tests are willing to start.
* Fewer unused-variable warnings.
* More documentation, fewer misspellings.
router_digest_is_fallback_dir returns 1 if the digest is in the
currently loaded list of fallback directories, and 0 otherwise.
This function is for future use.
Once tor is downloading a usable consensus, any other connection
attempts are not needed.
Choose a connection to keep, favouring:
* fallback directories over authorities,
* connections initiated earlier over later connections
Close all other connections downloading a consensus.
Prop210: Add attempt-based connection schedules
Existing tor schedules increment the schedule position on failure,
then retry the connection after the scheduled time.
To make multiple simultaneous connections, we need to increment the
schedule position when making each attempt, then retry a (potentially
simultaneous) connection after the scheduled time.
(Also change find_dl_schedule_and_len to find_dl_schedule, as it no
longer takes or returns len.)
Prop210: Add multiple simultaneous consensus downloads for clients
Make connections on TestingClientBootstrapConsensus*DownloadSchedule,
incrementing the schedule each time the client attempts to connect.
Check if the number of downloads is less than
TestingClientBootstrapConsensusMaxInProgressTries before trying any
more connections.
UseDefaultFallbackDirs enables any hard-coded fallback
directory mirrors. Default is 1, set it to 0 to disable fallbacks.
Implements ticket 17576.
Patch by "teor".
Update the code for IPv6 authorities and fallbacks for function
argument changes.
Update unit tests affected by the function argument changes in
the patch.
Add unit tests for authority and fallback:
* adding via a function
* line parsing
* adding default authorities
(Adding default fallbacks is unit tested in #15775.)
The internal memory allocation and history object counters of the
reputation code can be used to verify the correctness of (part of) the
code. Using these counters revealed an issue where the memory allocation
counter is not decreased when the bandwidth arrays are freed.
A new function ensures the memory allocation counter is decreased when a
bandwidth array is freed.
This commit also removes an unnecessary cast which was found while
working on the code.
* Since the variable is no longer modified, it should be called
'policy' instead of 'dest'. ("Dest" is short for
"destination".)
* Fixed the space issue that dgoulet found on the ticket.
* Fixed the comment a little. (We use the imperative for function
documentation.)
Some functions that use digest maps did not mention that the digests are
expected to have DIGEST_LEN bytes. This lead to buffer over-reads in the
past.
When an HS process an INTRODUCE2 cell, we didn't validate if the IP address
of the rendezvous point was a local address. If it's the case, we end up
wasting resources by trying to extend to a local address which fails since
we do not allow that in circuit_extend().
This commit now rejects a rendezvous point that has a local address once
seen at the hidden service side unless ExtendAllowPrivateAddresses is set.
Fixes#8976
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
There was a dead check when we made sure that an array member of a
struct was non-NULL. Tor has been doing this check since at least
0.2.3, maybe earlier.
Fixes bug 17781.
Previously we'd suppressed the mask-bits field in the output when
formatting a policy if it was >=32. But that should be a >=128 if
we're talking about IPv6.
Since we didn't put these in descriptors, this bug affects only log
messages and controller outputs.
Fix for bug 16056. The code in question was new in 0.2.0, but the
bug was introduced in 0.2.4 when we started supporting IPv6 exits.
port is in host order (addr is tor_addr_t, endianness is abstracted).
addr and port can be different to conn->addr and conn->port if
connecting via a proxy.
Consistently ignore multicast addresses when automatically
generating reject private exit policies.
Closes ticket 17763. Bug fix on 10a6390deb,
not in any released version of Tor. Patch by "teor".
The tor_cert_get_checkable_sig function uses the signing key included in
the certificate (if available) when a separate public key is not given.
When the signature is valid, the tor_cert_checksig function copies the
public key from the checkable structure to the public key field of the
certificate signing key.
In situations where the separate public key is not given but the
certificate includes a signing key, the source and destination pointers
in the copy operation are equal and invoke undefined behavior.
Undefined behaviour is avoided by ensuring both pointers are different.
These functions must really never fail; so have crypto_rand() assert
that it's working okay, and have crypto_seed_rng() demand that
callers check its return value. Also have crypto_seed_rng() check
RAND_status() before returning.
Stop ignoring ExitPolicyRejectPrivate in getinfo
exit-policy/reject-private. Fix a memory leak.
Set ExitPolicyRejectPrivate in the unit tests, and make a mock
function declaration static.
(If we take the branch above this assertion, than we *didn't* have a
v1 handshake. So if we don't take the branch, we did. So if we
reach this assertion, we must be running as a server, since clients
no longer attempt v1 handshakes.)
Fix for bug 17654; bugfix on 9d019a7db7.
Bug not in any released Tor.
Refuse connection requests to private OR addresses unless
ExtendAllowPrivateAddresses is set. Previously, tor would
connect, then refuse to send any cells to a private address.
Fixes bugs 17674 and 8976; bugfix on b7c172c9ec (28 Aug 2012)
Original bug 6710, released in 0.2.3.21-rc and an 0.2.2 maint
release.
Patch by "teor".
This migrates away from SHA1, and provides further hash flooding
protection on top of the randomised siphash implementation.
Add unit tests to make sure that different inputs don't have the
same hash.
The wrong list was used when looking up expired intro points in a rend
service object causing what we think could be reachability issues and
triggering a BUG log.
Fixes#16702
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
exit-policy/reject-private lists the reject rules added by
ExitPolicyRejectPrivate. This makes it easier for stem to
display exit policies.
Add unit tests for getinfo exit-policy/*.
Completes ticket #17183. Patch by "teor".
Modify policies_parse_exit_policy_reject_private so it also blocks
the addresses configured for OutboundBindAddressIPv4_ and
OutboundBindAddressIPv6_, and any publicly routable port addresses
on exit relays.
Add and update unit tests for these functions.
Move the code that rejects publicly routable exit relay addresses
to policies_parse_exit_policy_reject_private. Add
addr_policy_append_reject_addr_list and use it to reject interface
addresses.
This removes the duplicate reject checks on local_address and
ipv6_local_address, but duplicates will be removed by
exit_policy_remove_redundancies at the end of the function.
This also removes the info-level logging on rejected interface
addresses. Instead, log a debug-level message in
addr_policy_append_reject_addr.
This simplifies policies_parse_exit_policy_internal and prepares for
reporting these addresses over the control port in #17183.
Loading a on disk bridge descriptor causes a directory download to be
scheduled, which asserts due to the periodic events not being
initialized yet.
Fixes bug #17635, not in any released version of tor.
Now we only re-scan the list in the cases we did before: when we
have a new circuit that we should try attaching to, or when we have
added a new stream that we haven't tried to attach yet.
This is part of 17590.
Long ago we used to call connection_ap_handshake_attach_circuit()
only in a few places, since connection_ap_attach_pending() attaches
all the pending connections, and does so regularly. But this turned
out to have a performance problem: it would introduce a delay to
launching or connecting a stream.
We couldn't just call connection_ap_attach_pending() every time we
make a new connection, since it walks the whole connection list. So
we started calling connection_ap_attach_pending all over, instead!
But that's kind of ugly and messes up our callgraph.
So instead, we now have connection_ap_attach_pending() use a list
only of the pending connections, so we can call it much more
frequently. We have a separate function to scan the whole
connection array to see if we missed adding anything, and log a
warning if so.
Closes ticket #17590
Mark fallback directory mirrors as "too busy" when they return
a 503 response. Previously, the code just marked authorities as busy.
Unless clients set their own fallback directories, they will never see
this bug. (There are no default fallbacks yet.)
Fixes bug 17572; bugfix on 5c51b3f1f0 released in 0.2.4.7-alpha.
Patch by "teor".
When logging to syslog, allow a tag to be added to the syslog identity
("Tor"), i.e. the string prepended to every log message. The tag can be
configured by setting SyslogIdentityTag and defaults to none. Setting
it to "foo" will cause logs to be tagged as "Tor-foo". Closes: #17194.
Warn when the state file was last written in the future.
Tor doesn't know that consensuses have expired if the clock is in the past.
Patch by "teor". Implements ticket #17188.
BSD make takes spaces around = literally
and produces a "TESTING_TOR_BINARY "
variable with a trailing space, which leads
to test_keygen.sh failure.
Fixes 17154
When we find a conflict in the keypinning journal, treat the new
entry as superseding all old entries that overlap either of its
keys.
Also add a (not-yet-used) configuration option to disable keypinning
enforcement.
src/test/test_policy.c:
Merged calls to policies_parse_exit_policy by adding additional arguments.
fixup to remaining instance of ~EXIT_POLICY_IPV6_ENABLED.
Compacting logic test now produces previous list length of 4, corrected this.
src/config/torrc.sample.in:
src/config/torrc.minimal.in-staging:
Merged torrc modification dates in favour of latest.
Log an info-level message containing the reject line added to the
exit policy for each local IP address blocked by ExitPolicyRejectPrivate:
- Published IPv4 and IPv6 addresses
- Publicly routable IPv4 and IPv6 interface addresses
ExitPolicyRejectPrivate now rejects more local addresses by default:
* the relay's published IPv6 address (if any), and
* any publicly routable IPv4 or IPv6 addresses on any local interfaces.
This resolves a security issue for IPv6 Exits and multihomed Exits that
trust connections originating from localhost.
Resolves ticket 17027. Patch by "teor".
Patch on 42b8fb5a15 (11 Nov 2007), released in 0.2.0.11-alpha.
In previous versions of Tor, ExitPolicy accept6/reject6 * produced
policy entries for IPv4 and IPv6 wildcard addresses.
To reduce operator confusion, change accept6/reject6 * to only produce
an IPv6 wildcard address.
Resolves bug #16069.
Patch on 2eb7eafc9d and a96c0affcb (25 Oct 2012),
released in 0.2.4.7-alpha.
Tor now warns when ExitPolicy lines occur after accept/reject *:*
or variants. These lines are redundant, and were always ignored.
Partial fix for ticket 16069. Patch by "teor".
Patch on 2eb7eafc9d and a96c0affcb (25 Oct 2012),
released in 0.2.4.7-alpha.
When parsing torrc ExitPolicies, we now warn if:
* an IPv4 address is used on an accept6 or reject6 line. The line is
ignored, but the rest of the policy items in the list are used.
(accept/reject continue to allow both IPv4 and IPv6 addresses in torrcs.)
* a "private" address alias is used on an accept6 or reject6 line.
The line filters both IPv4 and IPv6 private addresses, disregarding
the 6 in accept6/reject6.
When parsing torrc ExitPolicies, we now issue an info-level message:
* when expanding an accept/reject * line to include both IPv4 and IPv6
wildcard addresses.
In each instance, usage advice is provided to avoid the message.
Partial fix for ticket 16069. Patch by "teor".
Patch on 2eb7eafc9d and a96c0affcb (25 Oct 2012),
released in 0.2.4.7-alpha.
routerset_parse now accepts IPv6 literal addresses.
Fix for ticket 17060. Patch by "teor".
Patch on 3ce6e2fba2 (24 Jul 2008), and related commits,
released in 0.2.1.3-alpha.
When validating a new descriptor against our rend cache failure, we were
added the failure entry to the new cache entry without duplicating. It was
then freed just after the validation ending up in a very bad memory state
that was making tor abort(). To fix this, a dup function has been added and
used just before adding the failure entry.
Fixes#17041
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
No functional changes, but since NoKeepAliveIsolateSOCKSAuth isn't
part of isoflag, it should be checked where all other similar options
are, and bypass the (no-op) masking at the end.
This controls the circuit dirtyness reset behavior added for Tor
Browser's user experience fix (#15482). Unlike previous iterations
of this patch, the tunable actually works, and is documented.
Performing lookups in both the client and service side descriptor
caches from the same rend_cache_lookup_entry() function increases the
risk of accidental API misuse.
I'm separating the lookup functions to keep the caches distinct.
Parameterize the rend_cache_clean() function to allow it clean
old rendezvous descriptors from the service-side cache as well as
the client descriptor cache.
Including the replica number in the HS_DESC CREATED event provides
more context to a control port client. The replica allows clients
to more easily identify each replicated descriptor from the
independantly output control events.
Entries in the service-side descriptor cache are now cleaned when
rend_cache_free_all() is called. The call to tor_free(intro_content)
in rend_cache_store_v2_desc_as_service() is moved to prevent a
potential double-free when a service has a descriptor with a newer
timestamp already in it's service-side descriptor cache.
Adds an Enum which represents the different types of rendezvous
descriptor caches. This argument is passed in each call to
rend_cache_lookup_entry() to specify lookup in the client-side or
service-side descriptor caches.
Adds a control command to fetch a local service descriptor from the
service descriptor cache. The local service descriptor cache is
referenced by the onion address of the service.
This control command is documented in the control spec.
When this is set, and Tor is running as a relay, it will not
generate or load its secret identity key. You can manage the secret
identity key with --keygen. Implements ticket 16944.
In a nutshell, since a circuit can not exit at its entry point, it's very
easy for an attacker to find the hidden service guard if only one EntryNodes
is specified since for that guard, the HS will refuse to build a rendezvous
circuit to it.
For now, the best solution is to stop tor to allow a single EntryNodes for
an hidden service.
Fixes#14917
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Only applies to connections with SOCKS auth set, so that non-web Tor
activity is not affected.
Simpler version of Nick's patch because the randomness worried me, and I'm not
otherwise sure why we want a max here.
In validate_recommended_package_line, at this point in the function,
n_entries is always >= 1. Coverity doesn't like us checking it for
0.
CID 1268063.
Removes a check to PublishHidServDescriptors in
rend_consider_services_upload(). This allows descriptors to be
generated and stored in the local cache when PublishHidServDescriptor = 0.
Keep the PublishHidServDescriptors option check in
rend_consider_descriptor_republication(). We will never need to republish
a descriptor if we are not publishing descriptors to the HSDirs.
Service descriptors are now generated regardless of the the
PublishHidServDescriptors option. The generated descriptors are stored
in the service descriptor cache.
The PublishHidServDescriptors = 1 option now prevents descriptor
publication to the HSDirs rather than descriptor generation.
Deindent a block of code inside the PublishHidServDescriptors option
check in upload_service_descriptor(). Stylistic commit to make the
subsequent reworking of this code cleaner.
The HS_DESC CREATED event should be emmited when a new service descriptor
is generated for a local rendevous service. This event is documented
in the control spec.
This commit resolves ticket #16291.
Adds a service descriptor cache which is indexed by service ID. This
descriptor cache is used to store service descriptors generated by a
local rendevous service.
The service-side cach can be queried by calling rend_cache_lookup_entry()
with the 'service' argument set to 1.
We used to use this when we had some controllers that would accept
long names and some that wouldn't. But it's been obsolete for a
while, and it's time to strip it out of the code.
Previously we'd put these strings right on the controllers'
outbufs. But this could cause some trouble, for these reasons:
1) Calling the network stack directly here would make a huge portion
of our networking code (from which so much of the rest of Tor is
reachable) reachable from everything that potentially generated
controller events.
2) Since _some_ events (EVENT_ERR for instance) would cause us to
call connection_flush(), every control_event_* function would
appear to be able to reach even _more_ of the network stack in
our cllgraph.
3) Every time we generated an event, we'd have to walk the whole
connection list, which isn't exactly fast.
This is an attempt to break down the "blob" described in
http://archives.seul.org/tor/dev/Mar-2015/msg00197.html -- the set of
functions from which nearly all the other functions in Tor are
reachable.
Closes ticket 16695.
