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.
Make it easier to unit test TestingDirAuthVote{Exit,Guard,HSDir}
by refactoring the code which sets flags based on them into a
new function dirserv_set_routerstatus_testing.
"option to prevent guard,exit,hsdir flag assignment"
"A node will never receive the corresponding flag unless
that node is specified in the
TestingDirAuthVote{Exit,Guard,HSDir} list, regardless of
its uptime, bandwidth, exit policy, or DirPort".
Patch modified by "teor": VoteOnHidServDirectoriesV2
is now obsolete, so TestingDirAuthVoteHSDir always
votes on HSDirs.
Closes ticket 14882. Patch by "robgjansen".
Commit message and changes file by "teor"
with quotes from "robgjansen".
Fix an error in the manual page and comments for
TestingDirAuthVoteHSDir, which suggested that a
HSDir required "ORPort connectivity". While this is true,
it is in no way unique to the HSDir flag. Of all the flags,
only HSDirs need a DirPort configured in order for the
authorities to assign that particular flag.
Fixed as part of 14882. Patch by "teor".
Bugfix on 0.2.6.3 (f9d57473e1 on 10 January 2015).
* FIXES#16823: https://bugs.torproject.org/16823
If an OP were to send a CREATE_FAST cell to an OR, and that
CREATE_FAST cell had unparseable key material, then tor_free() would
be called on the create cell twice. This fix removes the second
(conditional on the key material being bad) call to tor_free(), so
that now the create cell is always freed once, regardless of the status of
the key material.
(This isn't actually a double-free bug, since tor_free() sets its
input to NULL, and has no effect when called with input NULL.)
Instead of having it call update_all_descriptor_downloads and
update_networkstatus_downloads directly, we can have it cause them to
get rescheduled and called from run_scheduled_events.
Closes ticket 16789.
When fetching a descriptor, we know test every introduction points in it
against our rend failure cache to know if we keep it or not. For this to
work, now everytime an introduction points is discareded (ex: receiving a
NACK), we note it down in our introduction cache.
See rendcache.c for a detailed explanation of the cache's behavior.
Fixes#16389
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
When we removed Running/Valid checks from Fast and Stable in 8712, I
removed them from HSDir too, which apparently wasn't a good idea.
Reverts part of a65e835800. Fixes bug 16524. Bugfix
on 0.2.7.2-alpha.
microdesc_free_() called get_microdesc_cache(), which had the fun
side-effect of potentially reloading the whole cache from disk.
Replace it with a variant that doesn't.
The base64 and base32 functions used to be in crypto.c;
crypto_format.h had no header; some general-purpose functions were in
crypto_curve25519.c.
This patch makes a {crypto,util}_format.[ch], and puts more functions
there. Small modules are beautiful!
The control port was using set_max_file_descriptors() with a limit set to 0
to query the number of maximum socket Tor can use. With the recent changes
to that function, a check was introduced to make sure a user can not set a
value below the amount we reserved for non socket.
This commit adds get_max_sockets() that returns the value of max_sockets so
we can stop using that "setter" function to get the current value.
Finally, the dead code is removed that is the code that checked for limit
equal to 0. From now on, set_max_file_descriptors() should never be used
with a limit set to 0 for a valid use case.
Fixes#16697
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
1) We already require C99.
2) This allows us to support MSVC again (thanks to Gisle Vanem for
this part)
3) This change allows us to dump some rotten old compatibility code
from log.c
Make sure that signing certs are signed by the right identity key,
to prevent a recurrence of #16530. Also make sure that the master
identity key we find on disk matches the one we have in RAM, if we
have one.
This is for #16581.
When there is a signing key and the certificate lists a key, make
sure that the certificate lists the same signing key.
When there are public key and secret key stored in separate files,
make sure they match.
Use the right file name when we load an encrypted secret key and
then find a problem with it.
This is part of 16581.
When cleaning up extra circuits that we've opened for performance reason, we
need to count all the introduction circuit and not only the established ones
else we can end up with too many introduction points.
This also adds the check for expiring nodes when serving an INTRODUCE cell
since it's possible old clients are still using them before we have time to
close them.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
To upload a HS descriptor, this commits makes it that we wait for all
introduction point to be fully established.
Else, the HS ends up uploading a descriptor that may contain intro points
that are not yet "valid" meaning not yet established or proven to work. It
could also trigger three uploads for the *same* descriptor if every intro
points takes more than 30 seconds to establish because of desc_is_dirty
being set at each intro established.
To achieve that, n_intro_points_established varialbe is added to the
rend_service_t object that is incremented when we established introduction
point and decremented when we remove a valid intro point from our list.
The condition to upload a descriptor also changes to test if all intro
points are ready by making sure we have equal or more wanted intro points
that are ready.
The desc_id_dirty flag is kept to be able to still use the
RendInitialPostPeriod option.
This partially fixes#13483.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
There is a case where if the introduction circuit fails but the node is
still in the consensus, we clean up the intro point and choose an other one.
This commit fixes that by trying to reuse the existing intro point with a
maximum value of retry.
A retry_nodes list is added to rend_services_introduce() and when we remove
an invalid intro points that fits the use case mentionned before, we add the
node to the retry list instead of removing it. Then, we retry on them before
creating new ones.
This means that the requirement to remove an intro point changes from "if no
intro circuit" to "if no intro circuit then if no node OR we've reached our
maximum circuit creation count".
For now, the maximum retries is set to 3 which it completely arbitrary. It
should also at some point be tied to the work done on detecting if our
network is down or not.
Fixes#8239
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
The reasoning for refactoring this function is that removing the
introduction point adaptative algorithm (#4862) ended up changing quite a
bit rend_services_introduce(). Also, to fix some open issues (#8239, #8864
and #13483), this work had to be done.
First, this removes time_expiring variable in an intro point object and
INTRO_POINT_EXPIRATION_GRACE_PERIOD trickery and use an expiring_nodes list
where intro nodes that should expire are moved to that list and cleaned up
only once the new descriptor is successfully uploaded. The previous scheme
was adding complexity and arbitrary timing to when we expire an intro point.
We keep the intro points until we are sure that the new descriptor is
uploaded and thus ready to be used by clients. For this,
rend_service_desc_has_uploaded() is added to notify the HS subsystem that
the descriptor has been successfully uploaded. The purpose of this function
is to cleanup the expiring nodes and circuits if any.
Secondly, this adds the remove_invalid_intro_points() function in order to
split up rend_services_introduce() a bit with an extra modification to it
that fixes#8864. We do NOT close the circuit nor delete the intro point if
the circuit is still alive but the node was removed from the consensus. Due
to possible information leak, we let the circuit and intro point object
expire instead.
Finally, the whole code flow is simplified and large amount of documentation
has been added to mostly explain the why of things in there.
Fixes#8864
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
This is a way to specify the amount of introduction points an hidden service
can have. Maximum value is 10 and the default is 3.
Fixes#4862
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
When we ran out of intro points for a hidden service (which could
happen on a newnym), we would change the connection's state back to
"waiting for hidden service descriptor." But this would make an
assertion fail if we went on to call circuit_get_open_circ_or_launch
again.
This fixes bug 16013; I believe the bug was introduced in
38be533c69, where we made it possible for
circuit_get_open_circ_or_launch() to change the connection's state.
When I fixed#11243, I made it so we would take the digest of a
descriptor before tokenizing it, so we could desist from download
attempts if parsing failed. But when I did that, I didn't remove an
assertion that the descriptor began with "onion-key". Usually, this
was enforced by "find_start_of_next_microdescriptor", but when
find_start_of_next_microdescriptor returned NULL, the assertion was
triggered.
Fixes bug 16400. Thanks to torkeln for reporting and
cypherpunks_backup for diagnosing and writing the first fix here.
