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.
Every functions and objects that are used for hidden service descriptor
caches are moved to rendcache.{c|h}.
This commit does NOT change anything, just moving code around.
Fixes#16399
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
For now, rend_cache_entry_t has been moved from or.h to rendcache.h and
those files have been added to the build system.
In the next commit, these will contain hidden service descriptor cache ABI
and API for both client and directory side. The goal is to consolidate the
descriptor caches in one location to ease development, maintenance, review
and improve documentation for each cache behavior and algorithm.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
When --keygen is provided, we prompt for a passphrase when we make a
new master key; if it is nonempty, we store the secret key in a new
crypto_pwbox.
Also, if --keygen is provided and there *is* an encrypted master key,
we load it and prompt for a passphrase unconditionally.
We make a new signing key unconditionally when --keygen is provided.
We never overwrite a master key.
If crypto_early_init fails, a typo in a return value from tor_init
means that tor_main continues running, rather than returning
an error value.
Fixes bug 16360; bugfix on d3fb846d8c in 0.2.5.2-alpha,
introduced when implementing #4900.
Patch by "teor".
This reverts commit 9407040c59.
Small fix, "e->received" had to be removed since that variable doesn't exist
anymore.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
signing_key can be NULL in ed_key_init_from_file in routerkeys.c.
Discovered by clang 3.7 address sanitizer.
Fix on c03694938e, not in any released version of Tor.
Rend_add_service() frees its argument on failure; no need to free again.
Fixes bug 16228, bugfix on 0.2.7.1-alpha
Found by coverity; this is CID 1301387.
Also, have testing-level options to set the lifetimes and
expiration-tolerances of all key types, plus a non-testing-level
option to set the lifetime of any auto-generated signing key.
# The first commit's message is:
Regenerate ed25519 keys when they will expire soon.
Also, have testing-level options to set the lifetimes and
expiration-tolerances of all key types, plus a non-testing-level
option to set the lifetime of any auto-generated signing key.
# The 2nd commit message will be skipped:
# fixup! Regenerate ed25519 keys when they will expire soon.
This is a new collator type that follows proposal 220 for deciding
which identities to include. The rule is (approximately):
If a <ed,rsa> identity is listed by more than half of authorities,
include it. And include all <rsa> votes about that node as
matching.
Otherwise, if an <*,rsa> or <rsa> identity is listed by more than
half of the authorities, and no <ed,rsa> has been listed, include
it.
* Include ed25519 identities in votes
* Include "no ed25519 identity" in votes
* Include some commented-out code about identity voting. (This
will disappear.)
* Include some functions for identity voting (These will disappear.)
* Enforce uniqueness in ed25519 keys within a vote
Extrainfo documents are now ed-signed just as are router
descriptors, according to proposal 220. This patch also includes
some more tests for successful/failing parsing, and fixes a crash
bug in ed25519 descriptor parsing.
An earlier version of these tests was broken; now they're a nicer,
more robust, more black-box set of tests. The key is to have each
test check a handshake message that is wrong in _one_ way.
When there are annotations on a router descriptor, the
ed25519-identity element won't be at position 0 or 1; it will be at
router+1 or router-1.
This patch also adds a missing smartlist function to search a list for
an item with a particular pointer.
With this patch:
* Authorities load the key-pinning log at startup.
* Authorities open a key-pinning log for writing at startup.
* Authorities reject any router with an ed25519 key where they have
previously seen that ed25519 key with a different RSA key, or vice
versa.
* Authorities warn about, but *do not* reject, RSA-only descriptors
when the RSA key has previously gone along with an Ed25519 key.
(We should make this a 'reject' too, but we can't do that until we're
sure there's no legit reason to downgrade to 0.2.5.)
This module implements a key-pinning mechanism to ensure that it's
safe to use RSA keys as identitifers even as we migrate to Ed25519
keys. It remembers, for every Ed25519 key we've seen, what the
associated Ed25519 key is. This way, if we see a different Ed25519
key with that RSA key, we'll know that there's a mismatch.
We persist these entries to disk using a simple format, where each
line has a base64-encoded RSA SHA1 hash, then a base64-endoded
Ed25519 key. Empty lines, misformed lines, and lines beginning with
a # are ignored. Lines beginning with @ are reserved for future
extensions.
Routers now use TAP and ntor onion keys to sign their identity keys,
and put these signatures in their descriptors. That allows other
parties to be confident that the onion keys are indeed controlled by
the router that generated the descriptor.
Routers now use TAP and ntor onion keys to sign their identity keys,
and put these signatures in their descriptors. That allows other
parties to be confident that the onion keys are indeed controlled by
the router that generated the descriptor.
For prop220, we have a new ed25519 certificate type. This patch
implements the code to create, parse, and validate those, along with
code for routers to maintain their own sets of certificates and
keys. (Some parts of master identity key encryption are done, but
the implementation of that isn't finished)
As OpenSSL >= 1.0.0 is now required, ECDHE is now mandatory. The group
has to be validated at runtime, because of RedHat lawyers (P224 support
is entirely missing in the OpenSSL RPM, but P256 is present and is the
default).
Resolves ticket #16140.
When set, this limits the maximum number of simultaneous streams per
rendezvous circuit on the server side of a HS, with further RELAY_BEGIN
cells being silently ignored.
This can be modified via "HiddenServiceMaxStreamsCloseCircuit", which
if set will cause offending rendezvous circuits to be torn down instead.
Addresses part of #16052.
Ephemeral services will be listed in rend_services_list at the end of
rend_config_services, so it must check whether directory is non-NULL
before comparing.
This crash happens when reloading config on a tor with mixed configured
and ephemeral services.
Fixes bug #16060. Bugfix on 0.2.7.1-alpha.
For FAILED and RECEIVED action of the HS_DESC event, we now sends back the
descriptor ID at the end like specified in the control-spec section 4.1.25.
Fixes#15881
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
The length of auth_data from an INTRODUCE2 cell is checked when the
auth_type is recognized (1 or 2), but not for any other non-zero
auth_type. Later, auth_data is assumed to have at least
REND_DESC_COOKIE_LEN bytes, leading to a client-triggered out of bounds
read.
Fixed by checking auth_len before comparing the descriptor cookie
against known clients.
Fixes#15823; bugfix on 0.2.1.6-alpha.
"+HSPOST" and the related event changes allow the uploading of HS
descriptors via the control port, and more comprehensive event
monitoring of HS descriptor upload status.
When we have a new descriptor ID for an onion address request, change it in
the rend_data_t object and purge the old one from the last hid serv request
cache.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Stop using an onion address since it's not indexed with that anymore in the
last hid serv request cache. Instead use a base32 encoded descriptor ID
contained in the rend_data_t object.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Every callsite that use to allocate a rend_data_t object now use the
rend_data_client/service_create() function.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Ground works for fixing #15816. This adds the rend_data_create() function in
order to have a single place where we initialize that data structure.
Furthermore, an array of descriptor IDs is added (one per replica) so we can
keep a copy of the current id in the object. It will be used to purge the
last hid serv request cache using those descriptor IDs. When they change,
they will be replaced and the old ones will be purged from the cache.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Fixes#15850, part of #15801. Change file is added by this commit. The
original comment in the reverted commit is removed because right now we
*need* a DirPort until #15849 is implemented so no doubt nor confusion there
anymore.
This reverts commit 80bed1ac96.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
These commands allow for the creation and management of ephemeral
Onion ("Hidden") services that are either bound to the lifetime of
the originating control connection, or optionally the lifetime of
the tor instance.
Implements #6411.
The "longest possible policy" comment in
router_parse_addr_policy_item_from_string() used an example policy
that was actually shorter than the maximum length.
This comment was amended, and expanded to count the maximum number of
characters.
Comment change only.
Unit tests for the 10 valid combinations of set/NULL config options
DirAuthorities, AlternateBridgeAuthority, AlternateDirAuthority,
and FallbackDir.
Add assertion in consider_adding_dir_servers() for checks in
validate_dir_servers():
"You cannot set both DirAuthority and Alternate*Authority."
Only add the default fallback directories when the DirAuthorities,
AlternateDirAuthority, and FallbackDir directory config options
are set to their defaults.
The default fallback directory list is currently empty, this fix will
only change tor's behaviour when it has default fallback directories.
Fixes bug 15642; bugfix on 90f6071d8d in 0.2.4.7-alpha. Patch by "teor".
When self-testing reachability, use ExtendAllowPrivateAddresses
to determine if local/private addresses imply reachability.
The previous fix used TestingTorNetwork, which implies
ExtendAllowPrivateAddresses, but this excluded rare configs where
ExtendAllowPrivateAddresses is set but TestingTorNetwork is not.
Fixes bug 15771; bugfix on 0.2.6.1-alpha, bug #13924.
Patch by "teor", issue discovered by CJ Ess.
The HS_DESC_CONTENT event results in multiple line thus must be prefixed
with a "650+" and ending with "650 OK".
Reported-by: Damian Johnson <atagar@torproject.org>
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
The HS_DESC event was using rend_data_t from the dir connection to reply the
onion address and authentication type. With the new HSFETCH command, it's
now possible to fetch a descriptor only using the descriptor id thus
resulting in not having an onion address in any HS_DESC event.
This patch removes rend_query from the hs desc control functions and replace
it by an onion address string and an auth type.
On a successful fetch, the service id is taken from the fetched descriptor.
For that, an extra parameter is added to "store as a client" function that
contains the cache entry stored.
This will make the control event functions scale more easily over time if
other values not present in rend_data_t are needed since the rend_data from
the dir connection might not contained everything we need.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Big refactor of the HS client descriptor fetch functionnality. This allows
to fetch an HS descriptor using only a descriptor ID. Furthermore, it's also
possible to provide a list of HSDir(s) now that are used instead of the
automatically choosen one.
The approach taken was to add a descriptor_id field to the rend_data_t
structure so it can be used, if available, by the HS client. The onion
address field however has priority over it that is if both are set, the
onion address is used to fetch the descriptor.
A new public function is introduced called rend_client_fetch_v2_desc(...)
that does NOT lookup the client cache before fetching and can take a list of
HSDirs as a parameter.
The HSFETCH control command now uses this new function thus making it work
and final.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
As defined in section 4.1.26 in the control-spec.txt, this new event replies
the content of a successfully fetched HS descriptor. This also adds a unit
test for the controller event.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
This adds the command on the controller side that parses and validate
arguments but does nothing for now. The HS desriptor fetch must be
modularized a bit more before we can use the command.
See control-spec.txt section 3.26 for more information on this command.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Incidently, this fixes a bug where the maximum value was never used when
only using crypto_rand_int(). For instance this example below in
rendservice.c never gets to INTRO_POINT_LIFETIME_MAX_SECONDS.
int intro_point_lifetime_seconds =
INTRO_POINT_LIFETIME_MIN_SECONDS +
crypto_rand_int(INTRO_POINT_LIFETIME_MAX_SECONDS -
INTRO_POINT_LIFETIME_MIN_SECONDS);
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
An introduction point is currently rotated when the amount of INTRODUCE2
cells reached a fixed value of 16384. This makes it pretty easy for an
attacker to inflate that number and observe when the IP rotates which leaks
the popularity of the HS (amount of client that passed through the IP).
This commit makes it a random count between the current value of 16384 and
two times that.
Fixes#15745
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Background processes spawned by Tor now will have a valid stdin.
Pluggable transports can detect this behavior with the aformentioned
enviornment variable, and exit if stdin ever gets closed.
The rend-spec.txt document doesn't specify this extra newline. Furthermore,
this is the only descryptor type that contains one. Client and HSDir without
this patch still work perfectly since the HS descriptor parsing doesn't
expect a newline at the end.
Fixes#15296
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Remove src/or/or_sha1.i and src/common/common_sha1.i on `make clean` and remove
the temporary micro-revision file when its no longer needed.
Additional changes;
- show a message when generating the micro-revision file.
- add the temporary micro revision file to the list of files to be removed on
`make clean` just in case.
- fix indentation of the make rule to improve readability.
PTs expect the auth cookie to be available immedieately after launch,
leading to a race condition when PTs opt to cache the extorport cookie
once immediately after startup.
Fixes#15240.
Report errors if the notification fails; report success only if it
succeeds; and if we are not notifying systemd because we aren't
running with systemd, don't log at notice.
We already log whenever our state changes, e.g. whenever new directory
information arrives. This additional log_warn() will at best just add more
log messages, or worse, make the user wonder what she needs to fix.
(Changed after consultation with Yawning.)
Nothing ever uses the string when we're in "have minimum dir info"
state. The flow of the function is "check for problems, if you see a
problem write an explanation to dir_info_status and set res to 0". If
you get to the end of the function without any problems, then res =
1 and we're all ready to start making circuits.
(Changed after consultation with Yawning.)
Parse the file just before voting and apply its information to the
provided vote_routerstatus_t. This follows the same logic as when
dirauths parse bwauth files.
`dir_info_status` is used from main.c:directory_info_has_arrived() to
provide useful (INFO/NOTICE) level logging to users, and should always
be updated regardless of the rate limiting.
This adds the key "hs/client/desc/id/<ADDR>" to the GETINFO command used to
lookup the given onion address in the client hs descriptor cache.
If found, prints it formatted as specified in section 1.3 of rend-spec.txt.
Fixes#14845
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
The idea here is that a controller should be able to make Tor produce a
new relay descriptor on demand, without that descriptor actually being
uploaded to the dirauths (they would likely reject it anyway due to
freshness concerns).
Implements #14784.
Allow building a router descriptor without storing it to global state.
This is in preparation of a patch to export the created descriptors via
the control port.
The issue is that we use the cpuworker system with relays only, so if we
start up as a client and transition to being a relay later, we'll be
sad.
This fixes bug 14901; not in any released version of Tor.
They have been off-by-default since 0.2.5 and nobody has complained. :)
Also remove the buf_shrink() function, which hasn't done anything
since we first stopped using contiguous memory to store buffers.
Closes ticket 14848.
This fixes a bug where we decide to free the circuit because it isn't on
any workqueue anymore, and then the job finishes and the circuit gets
freed again.
Fixes bug #14815, not in any released version of Tor.
like might happen for Tails or Whonix users who start with a very wrong
hardware clock, use Tor to discover a more accurate time, and then
fix their clock.
Resolves part of ticket 8766.
(There are still some timers in various places that aren't addressed yet.)
In #14803, Damian noticed that his Tor sometimes segfaults. Roger noted
that his valgrind gave an invalid write of size one here. Whenever we
use FLEXIBLE_ARRAY_MEMBER, we have to make sure to actually malloc a
thing that's large enough.
Fixes bug #14803, not in any released version of Tor.
If the guard unreachable_since variable was set, the status "up" was
reported which is wrong. This adds the "down" status followed by the
unreachable_since time value.
Fixes#14184
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
David Goulet finds that when he runs a busy relay for a while with the
latest version of the git code, the number of onionskins handled
slowly dwindles to zero, with total_pending_tasks wedged at its
maximum value.
I conjecture this is because the total_pending_tasks variable isn't
decremented when we successfully cancel a job. Fixed that.
Fixes bug 14741; bugfix not on any released version of tor.
Ordinarily, get_options() can never return NULL, but with
test_status.c mocking, it can. So test for that case.
The best fix here would be to pass the options value to a
bridge_server_mode() function.
After connectivity problems, only try connecting to bridges which
are currently configured; don't mark bridges which we previously
used but are no longer configured. Fixes 14216. Reported by
and fix provided by arma.
It's now possible to use SocksPort or any other kind of port that can use a
Unix socket like so:
SocksPort unix:/foo/bar/unix.sock
Fixes#14451
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Introduces two new circuit status name-value parameters: SOCKS_USERNAME
and SOCKS_PASSWORD. Values are enclosing in quotes and unusual characters
are escaped.
Example:
650 CIRC 5 EXTENDED [...] SOCKS_USERNAME="my_username" SOCKS_PASSWORD="my_password"
Here is why:
1) v0 descriptors are deprecated since 0.2.2.1 and not suppose to be alive
in the network anymore. This function should only serve v2 version for now
as the default.
2) It should return different error code depending on what's the actual
error is. Right now, there is no distinction between a cache entry not found
and an invalid query.
3) This function should NOT test if the intro points are usable or not. This
adds some load on a function that should be "O(1)" and do one job.
Furthermore, multiple callsites actually already test that doing twice the
job...
4) While adding control event, it would be useful to be able to lookup a
cache entry without having it checking the intro points. There are also
places in the code that do want to lookup the cache entry without doing
that.
Fixes#14391
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Also, do a little light refactoring to move some variable declarations
around and make a few things const
Also fix an obnoxious bug on checking for the DONE stream end reason.
It's not a flag; it's a possible value or a variable that needs to be
masked.
Once a NACK is received on the intro circuit, tor tries an other usable one
by extending the current circuit to it. If no more intro points are usable,
now close the circuit. Also, it's reason is changed before closing it so we
don't report again an intro point failure and trigger an extra HS fetch.
Fixes#14224
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Once a NACK is received on the intro circuit, tor tries an other usable one
by extending the current circuit to it. If no more intro points are usable,
now close the circuit.
Fixes#14224
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
This fixes a bug where we'd fetch different replicas of the same
descriptor for a down hidden service over and over, until we got lucky
and fetched the same replica twice in a row.
Fixes bug 14219; bugfix on 0.2.0.10-alpha.
(Patch from Roger; commit message and changes file by Nick.)
We've started to hit the limit here. We introduced the limit in
0.1.2.5-alpha. This fixes bug 14261, but we should have a smarter way
to not actually do the behavior this permits. See #14267 for a ticket
about fixing that.
This incidentally makes unix SocksSocket support all the same options
as SocksPort.
This patch breaks 'SocksSocket 0'; next will restore it.
Resolves 14254.
Previously I used one queue per worker; now I use one queue for
everyone. The "broadcast" code is gone, replaced with an idempotent
'update' operation.
The solution I took is to not free a circuit with a pending
uncancellable work item, but rather to set its magic number to a
sentinel value. When we get a work item, we check whether the circuit
has that magic sentinel, and if so, we free it rather than processing
the reply.
To avoid having diffs turn out too big, I had replaced some unneeded
ifs and fors with if (1), so that the indentation would still work out
right. Now I might as well clean those up.
The trick here is to apply mapaddress first, and only then apply
automapping. Otherwise, the automap checks don't get done.
Fix for bug 7555; bugfix on all versions of Tor supporting both
MapAddress and AutoMap.
There were following problems:
- configure.ac wrongly checked for defined HAVE_SYSTEMD; this
wasn't working, so the watchdog code was not compiled in.
Replace library search with explicit version check
- sd_notify() watchdog call was unsetting NOTIFY_SOCKET from env;
this means only first "watchdog ping" was delivered, each
subsequent one did not have socket to be sent to and systemd
was killing service
- after those fixes, enable Watchdog in systemd unit with one
minute intervals
If running under systemd, send back information when reloading
configuration and gracefully shutting down. This gives administator
more information about current Tor daemon state.
If running under systemd, notify the supervisor about current PID
of Tor daemon. This makes systemd unit simpler and more robust:
it will do the right thing regardless of RunAsDaemon settings.
Check for a missing option value in parse_virtual_addr_network
before asserting on the NULL in tor_addr_parse_mask_ports.
This avoids crashing on torrc lines like Vi[rtualAddrNetworkIPv[4|6]]
when no value follows the option.
Bugfix on 0.2.3 (de4cc126cb on 24 November 2012), fixes#14142.
Drop the MIN_REND_INITIAL_POST_DELAY on a testing network to 5 seconds,
but keep the default at 30 seconds.
Reduces the hidden service bootstrap to 25 seconds from around 45 seconds.
Change the default src/test/test-network.sh delay to 25 seconds.
Closes ticket 13401.
TestingDirAuthVoteHSDir ensures that authorities vote the HSDir flag
for the listed relays regardless of uptime or ORPort connectivity.
Respects the value of VoteOnHidServDirectoriesV2.
Partial fix for bug 14067.
Fixes bug 11454, where we would keep around a superseded descriptor
if the descriptor replacing it wasn't at least a week later. Bugfix
on 0.2.1.8-alpha.
Fixes bug 11457, where a certificate with a publication time in the
future could make us discard existing (and subsequent!) certificates
with correct publication times. Bugfix on 0.2.0.3-alpha.
When I made time parsing more strict, I broke the
EntryGuardDownSince line, which relied on two concatenated ISO times
being parsed as a single time.
Fixes bug 14136. Bugfix on 7984fc1531. Bug not in any released
version of Tor.
Also, avoid crashing when we attempt to double-remove an edge
connection from the DNS resolver: just log a bug warning instead.
Fixes bug 14129. Bugfix on 0d20fee2fb, which was in 0.0.7rc1.
jowr found the bug. cypherpunks wrote the fix. I added the log
message and removed the assert.
"Maybe this time should be reduced, since we are considering
guard-related changes as quite important? It would be a pity to
settle on a guard node, then close the Tor client fast and lose that
information."
Closes 12485.
If we decide not to use a new guard because we want to retry older
guards, only close the locally-originating circuits passing through
that guard. Previously we would close all the circuits.
Fixes bug 9819; bugfix on 0.2.1.1-alpha. Reported by "skruffy".
Have clients and authorities both have new behavior, since the
fix for bug 11243 has gone in. But make clients still accept
accept old bogus HSDir descriptors, to avoid fingerprinting trickery.
Fixes bug 9286.
If we're not a relay, we ignore it.
If it's set to 1, we obey ExitPolicy.
If it's set to 0, we force ExitPolicy to 'reject *:*'
And if it's set to auto, then we warn the user if they're running an
exit, and tell them how they can stop running an exit if they didn't
mean to do that.
Fixes ticket 10067
We had a check to block these, but the patch we merged as a1c1fc72
broke this check by making them absolute on demand every time we
opened them. That's not so great though. Instead, we should make them
absolute on startup, and not let them change after that.
Fixes bug 13397; bugfix on 0.2.3.11-alpha.
This happened because we changed AutomapHostsSuffixes to replace "."
with "", since a suffix of "" means "match everything." But our
option handling code for CSV options likes to remove empty entries
when it re-parses stuff.
Instead, let "." remain ".", and treat it specially when we're
checking for a match.
Fixes bug 12509; bugfix on 0.2.0.1-alpha.
This allows hidden services to disable the anti-scanning feature
introduced in 0.2.6.2-alpha. With this option not set, a connection
to an unlisted port closes the circuit. With this option set, only
a RELAY_DONE cell is sent.
Closes ticket #14084.
Supposedly there are a decent number of applications that "support"
IPv6 and SOCKS5 using the FQDN address type. While said applications
should be using the IPv6 address type, allow the connection if
SafeSocks is not set.
Bug not in any released version.
Stop assuming that private addresses are local when checking
reachability in a TestingTorNetwork. Instead, when testing, assume
all OR connections are remote. (This is necessary due to many test
scenarios running all nodes on localhost.)
This assists in bootstrapping a testing Tor network.
Fixes bugs 13718 & 13924.
If the consensus does not contain Exits, Tor will only build internal
circuits. In this case, relevant statuses will contain the word "internal"
as indicated in the Tor control-spec.txt. When bootstrap completes,
Tor will be ready to handle an application requesting an internal
circuit to hidden services at ".onion" addresses.
If a future consensus contains Exits, exit circuits may become available.
Tor already notifies the user at "notice" level if they have no exits in
the consensus, and can therefor only build internal paths.
Consequential change from #13718.
Tor can now build circuits from a consensus with no exits.
But if it tries to build exit circuits, they fail and flood the logs.
The circuit types in the Exit Circuits list below will only be
built if the current consensus has exits. If it doesn't,
only the Internal Circuits will be built. (This can change
with each new consensus.)
Fixes bug #13814, causes fewer path failures due to #13817.
Exit Circuits:
Predicted Exit Circuits
User Traffic Circuits
Most AP Streams
Circuits Marked Exit
Build Timeout Circuits (with exits)
Internal Circuits:
Hidden Service Server Circuits
Hidden Service Client Circuits
Hidden Service AP Streams
Hidden Service Intro Point Streams
Circuits Marked Internal
Build Timeout Circuits (with no exits)
Other Circuits?
If the consensus has no exits (typical of a bootstrapping
test network), allow tor to build circuits once enough
descriptors have been downloaded.
When there are no exits, we always have "enough"
exit descriptors. (We treat the proportion of available
exit descriptors as 100%.)
This assists in bootstrapping a testing Tor network.
Fixes bug 13718.
Makes bug 13161's TestingDirAuthVoteExit non-essential.
(But still useful for speeding up a bootstrap.)
Add router_have_consensus_path() which reports whether
the consensus has exit paths, internal paths, or whether it
just doesn't know.
Used by #13718 and #13814.
count_usable_descriptors now uses named exit_only values:
USABLE_DESCRIPTOR_ALL
USABLE_DESCRIPTOR_EXIT_ONLY
Add debug logging code for descriptor counts.
This (hopefully) resolves nickm's request in bug 13718 to improve
argument readability in nodelist.c.
choose_good_entry_server() now excludes current entry
guards and their families, unless we're in a test network,
and excluding guards would exclude all nodes.
This typically occurs in incredibly small tor networks,
and those using TestingAuthVoteGuard *
This is an incomplete fix, but is no worse than the previous
behaviour, and only applies to minimal, testing tor networks
(so it's no less secure).
Discovered as part of #13718.
Make hidden service port scanning harder by sending back REASON_DONE which
does not disclose that it was in fact an exit policy issue. After that, kill
the circuit immediately to avoid more bad requests on it.
This means that everytime an hidden service exit policy does match, the user
(malicious or not) needs to build a new circuit.
Fixes#13667.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
When V3AuthVotingInterval is low, decrease the delay on the
If-Modified-Since header passed to directory servers.
This allows us to obtain consensuses promptly when the consensus
interval is very short.
This assists in bootstrapping a testing Tor network.
Fixes bugs 13718 & 13963.
Decrease minimum consensus interval to 10 seconds
when TestingTorNetwork is set. (Or 5 seconds for
the first consensus.)
Fix code that assumes larger interval values.
This assists in quickly bootstrapping a testing
Tor network.
Fixes bugs 13718 & 13823.
Stop requiring exits to have non-zero bandwithcapacity in a
TestingTorNetwork. Instead, when TestingMinExitFlagThreshold is 0,
ignore exit bandwidthcapacity.
This assists in bootstrapping a testing Tor network.
Fixes bugs 13718 & 13839.
Makes bug 13161's TestingDirAuthVoteExit non-essential.
Matthew's autoaddr code returned an undecorated address when trying to check
that the code didn't insert an undecorated one into the map.
This patch fixes this by actually storing the undecorated address in tmp
instead of buf as it was originally intended.
This patch is released under the same license as the original file as
long as the author iscredited.
Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@gentoo.org>
Document why we divide it by two.
Check for > 0 instead of nonzero for success, since that's what the
manpage says.
Allow watchdog timers greater than 1 second.
It work by notifying systemd on a regular basis. If
there is no notification, the daemon is restarted.
This requires a version newer than the 209 version
of systemd, as it is not supported before.
The original call to getsockopt to know the original address on transparently
proxyed sockets using REDIRECT in iptables failed with IPv6 addresses because
it assumed all sockets used IPv4.
This patch fixes this by using the appropriate options and adding the headers
containing the needed definitions for these.
This patch is released under the same license as the original file as
long as the author iscredited.
Signed-off-by: Francisco Blas Izquierdo Riera (klondike) <klondike@gentoo.org>
The address of an array in the middle of a structure will
always be non-NULL. clang recognises this and complains.
Disable the tautologous and redundant check to silence
this warning.
Fixes bug 14001.
The address of an array in the middle of a structure will
always be non-NULL. clang recognises this and complains.
Disable the tautologous and redundant check to silence
this warning.
A comment about an IPv6 address string incorrectly refers
to an IPv4 address format.
A log buffer is sized 10024 rather than 10240.
Fixes bug 14001.
The two statistics are:
1. number of RELAY cells observed on successfully established
rendezvous circuits; and
2. number of .onion addresses observed as hidden-service
directory.
Both statistics are accumulated over 24 hours, obfuscated by rounding
up to the next multiple of a given number and adding random noise,
and written to local file stats/hidserv-stats.
Notably, no statistics will be gathered on clients or services, but
only on relays.
In circuit_get_open_circ_or_launch(), for a rendezvous circuit,
rend_client_rendcirc_has_opened() but circuit_has_opened() is preferred here
since it will call the right function for a specific circuit purpose.
Furthermore, a controller event is triggered where the former did not.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
1) Set them to the values that (according to Rob) avoided performance
regressions. This means that the scheduler won't get much exercise
until we implement KIST or something like it.
2) Rename the options to end with a __, since I think they might be
going away, and nobody should mess with them.
3) Use the correct types for the option variables. MEMUNIT needs to be a
uint64_t; UINT needs to be (I know, I know!) an int.
4) Validate the values in options_validate(); do the switch in
options_act(). This way, setting the option to an invalid value on
a running Tor will get backed out.
We add a compression level argument to tor_zlib_new, and use it to
determine how much memory to allocate for the zlib object. We use the
existing level by default, but shift to smaller levels for small
requests when we have been over 3/4 of our memory usage in the past
half-hour.
Closes ticket 11791.
When closing parallel introduction points, the given reason (timeout)
was actually changed to "no reason" thus when the circuit purpose was
CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT, we were reporting an introduction
point failure and flagging it "unreachable". After three times, that
intro point gets removed from the rend cache object.
In the case of CIRCUIT_PURPOSE_C_INTRODUCING, the intro point was
flagged has "timed out" and thus not used until the connection to the HS
is closed where that flag gets reset.
This commit adds an internal circuit reason called
END_CIRC_REASON_IP_NOW_REDUNDANT which tells the closing circuit
mechanism to not report any intro point failure.
This has been observed while opening hundreds of connections to an HS on
different circuit for each connection. This fix makes this use case to
work like a charm.
Fixes#13698.
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
Instead, generate new keys, and overwrite the empty key files.
Adds FN_EMPTY to file_status_t and file_status.
Fixes bug 13111.
Related changes due to review of FN_FILE usage:
Stop generating a fresh .old RSA key file when the .old file is missing.
Avoid overwriting .old key files with empty key files.
Skip loading zero-length extra info store, router store, stats, state,
and key files.
We were only using it when smartlist_choose_node_by_bandwidth_weights
failed. But that function could only fail in the presence of
buggy/ancient authorities or in the absence of a consensus. Either
way, it's better to use sensible defaults and a nicer algorithm.
Now, if a router ever changes its microdescriptor, but the new
microdescriptor SHA256 hash has the same 160-bit prefix as the old
one, we treat it as a new microdescriptor when deciding whether to
copy status information.
(This function also is used to compare SHA1 digests of router
descriptors, but don't worry: the descriptor_digest field either holds
a SHA256 hash, or a SHA1 hash padded with 0 bytes.)
Silence clang warnings under --enable-expensive-hardening, including:
+ implicit truncation of 64 bit values to 32 bit;
+ const char assignment to self;
+ tautological compare; and
+ additional parentheses around equality tests. (gcc uses these to
silence assignment, so clang warns when they're present in an
equality test. But we need to use extra parentheses in macros to
isolate them from other code).
By now, support in the network is widespread and it's time to require
more modern crypto on all Tor instances, whether they're clients or
servers. By doing this early in 0.2.6, we can be sure that at some point
all clients will have reasonable support.
Ensure we securely wipe keys from memory after
crypto_digest_get_digest and init_curve25519_keypair_from_file
have finished using them.
Fixes bug 13477.
Also, refactor the way we handle failed handshakes so that this
warning doesn't propagate itself to "onion_skin_client_handshake
failed" and "circuit_finish_handshake failed" and
"connection_edge_process_relay_cell (at origin) failed."
Resolves warning from 9635.
1. The test that adds things to the cache needs to set the clock back so
that the descriptors it adds are valid.
2. We split ROUTER_NOT_NEW into ROUTER_TOO_OLD, so that we can
distinguish "already had it" from "rejected because of old published
date".
3. We make extrainfo_insert() return a was_router_added_t, and we
make its caller use it correctly. This is probably redundant with
the extrainfo_is_bogus flag.
One pain point in evolving the Tor design and implementing has been
adding code that makes clients reject directory documents that they
previously would have accepted, if those descriptors actually exist.
When this happened, the clients would get the document, reject it,
and then decide to try downloading it again, ad infinitum. This
problem becomes particularly obnoxious with authorities, since if
some authorities accept a descriptor that others don't, the ones
that don't accept it would go crazy trying to re-fetch it over and
over. (See for example ticket #9286.)
This patch tries to solve this problem by tracking, if a descriptor
isn't parseable, what its digest was, and whether it is invalid
because of some flaw that applies to the portion containing the
digest. (This excludes RSA signature problems: RSA signatures
aren't included in the digest. This means that a directory
authority can still put another directory authority into a loop by
mentioning a descriptor, and then serving that descriptor with an
invalid RSA signatures. But that would also make the misbehaving
directory authority get DoSed by the server it's attacking, so it's
not much of an issue.)
We already have a mechanism to mark something undownloadable with
downloadstatus_mark_impossible(); we use that here for
microdescriptors, extrainfos, and router descriptors.
Unit tests to follow in another patch.
Closes ticket #11243.
Bitwise check for the BRIDGE_DIRINFO flag, rather than checking for
equality.
Fixes a (potential) bug where directories offering BRIDGE_DIRINFO,
and some other flag (i.e. microdescriptors or extrainfo),
would be ignored when looking for bridge directories.
Final fix in series for bug 13163.
Document usage of the NO_DIRINFO and ALL_DIRINFO flags clearly in functions
which take them as arguments. Replace 0 with NO_DIRINFO in a function call
for clarity.
Seeks to prevent future issues like 13163.
Stop using the default authorities in networks which provide both
AlternateDirAuthority and AlternateBridgeAuthority.
This bug occurred due to an ambiguity around the use of NO_DIRINFO.
(Does it mean "any" or "none"?)
Partially fixes bug 13163.
Cases that now send errors:
* Malformed IP address (SOCKS5_GENERAL_ERROR)
* CONNECT/RESOLVE request with IP, when SafeSocks is set
(SOCKS5_NOT_ALLOWED)
* RESOLVE_PTR request with FQDN (SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED)
* Malformed FQDN (SOCKS5_GENERAL_ERROR)
* Unknown address type (SOCKS5_ADDRESS_TYPE_NOT_SUPPORTED)
Fixes bug 13314.
Add the TestingDirAuthVoteExit option, a list of nodes to vote Exit for,
regardless of their uptime, bandwidth, or exit policy.
TestingTorNetwork must be set for this option to have any effect.
Works around an issue where authorities would take up to 35 minutes to
give nodes the Exit flag in a test network, despite short consensus
intervals. Partially implements ticket 13161.
Otherwise, when we authority try to do a self-test because of
init-keys, if that self-test can't be launched for whatever reason and
so we close the channel immediately, we crash.
Yes, this a silly way for initialization to work.
In circuit_build_times_calculate_timeout() in circuitstats.c, avoid dividing
by zero in the pareto calculations.
If either the alpha or p parameters are 0, we would divide by zero, yielding
an infinite result; which would be clamped to INT32_MAX anyway. So rather
than dividing by zero, we just skip the offending calculation(s), and
use INT32_MAX for the result.
Division by zero traps under clang -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error.
Avoid 4 null pointer errors under clang shallow analysis (the default when
building under Xcode) by using tor_assert() to prove that the pointers
aren't null. Resolves issue 13284 via minor code refactoring.
Bugfix on ed8f020e205267e6270494634346ab68d830e1d8; bug not in any
released version of Tor. Found by Coverity; this is CID 1239290.
[Yes, I used this commit message before, in 58e813d0fc.
Turns out, that fix wasn't right, since I didn't look up a
screen. :P ]
Uses libscrypt when found; otherwise, we don't have scrypt and we
only support openpgp rfc2440 s2k hashing, or pbkdf2.
Includes documentation and unit tests; coverage around 95%. Remaining
uncovered code is sanity-checks that shouldn't be reachable fwict.
When DisableNetwork is set, do not launch pluggable transport plugins,
and if any are running already, terminate the existing instances.
Resolves ticket 13213.
Allow clients to use optimistic data when connecting to a hidden service,
which should cut out the initial round-trip for client-side programs
including Tor Browser.
(Now that Tor 0.2.2.x is obsolete, all hidden services should support
server-side optimistic data.)
See proposal 181 for details. Implements ticket 13211.
Clients are now willing to send optimistic circuit data (before they
receive a 'connected' cell) to relays of any version. We used to
only do it for relays running 0.2.3.1-alpha or later, but now all
relays are new enough.
Resolves ticket 13153.
Return an error when the second or later arguments of the
"setevents" controller command are invalid events. Previously we
would return success while silently skipping invalid events.
Fixes bug 13205; bugfix on 0.2.3.2-alpha. Reported by "fpxnns".
Stop modifying the value of our DirReqStatistics torrc option just
because we're not a bridge or relay. This bug was causing Tor
Browser users to write "DirReqStatistics 0" in their torrc files
as if they had chosen to change the config.
Fixes bug 4244; bugfix on 0.2.3.1-alpha.
Clients now send the correct address for their chosen rendezvous point
when trying to access a hidden service. They used to send the wrong
address, which would still work some of the time because they also
sent the identity digest of the rendezvous point, and if the hidden
service happened to try connecting to the rendezvous point from a relay
that already had a connection open to it, the relay would reuse that
connection. Now connections to hidden services should be more robust
and faster. Also, this bug meant that clients were leaking to the hidden
service whether they were on a little-endian (common) or big-endian (rare)
system, which for some users might have reduced their anonymity.
Fixes bug 13151; bugfix on 0.2.1.5-alpha.
"At this point in the code, msg has been set to a string
constant. But the tor code checks that msg is not NULL, and the
redundant NULL check confuses the analyser[...] To avoid this
spurious warning, the patch initialises msg to NULL."
Patch from teor. another part of 13157.
Tor Browser includes several ClientTransportPlugin lines in its
torrc-defaults file, leading every Tor Browser user who looks at her
logs to see these notices and wonder if they're dangerous.
Resolves bug 13124; bugfix on 0.2.5.3-alpha.
Technically, we're not allowed to take the address of a member can't
exist relative to the null pointer. That makes me wonder how any sane
compliant system implements the offsetof macro, but let's let sleeping
balrogs lie.
Fixes 13096; patch on 0.1.1.9-alpha; patch from "teor", who was using
clang -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error -ftrapv
(And replay them once we know our first real logs.)
This is an implementation for issue 6938. It solves the problem of
early log mesages not getting sent to log files, but not the issue of
early log messages not getting sent to controllers.
This implements the meat of #12899. This commit should simply remove the
parts of Tor dirauths used to check whether a relay was supposed to be
named or not, it doesn't yet convert to a new mechanism for
reject/invalid/baddir/badexiting relays.
Back in 078d6bcd, we added an event number 0x20, but we didn't make
the event_mask field big enough to compensate.
Patch by "teor". Fixes 13085; bugfix on 0.2.5.1-alpha.
This function never returns non-null, but its usage doesn't reflect
that. Let's make it explicit. This will be mostly overridden by later
commits, so no changes file here.
This is in preparation for a big patch series removing the entire Naming
system from Tor. In its wake, the approved-routers file is being
deprecated, and a replacement option to allow only pre-approved routers
is not being implemented.
This implements a feature from bug 13000. Instead of starting a bwauth
run with this wrong idea about their bw, relays should do the self-test
and then get measured.
When a tor relay starts up and has no historical information about its
bandwidth capability, it uploads a descriptor with a bw estimate of 0.
It then starts its bw selftest, but has to wait 20 minutes to upload the
next descriptor due to the MAX_BANDWIDTH_CHANGE_FREQ delay. This change
should mean that on average, relays start seeing meaningful traffic a
little quicker, since they will have a higher chance to appear in the
consensus with a nonzero bw.
Patch by Roger, changes file and comment by Sebastian.
We're calling mallocfn() and reallocfn() in the HT_GENERATE macro
with the result of a product. But that makes any sane analyzer
worry about overflow.
This patch keeps HT_GENERATE having its old semantics, since we
aren't the only project using ht.h. Instead, define a HT_GENERATE2
that takes a reallocarrayfn.
Most of these are in somewhat non-obvious code where it is probably
a good idea to initialize variables and add extra assertions anyway.
Closes 13036. Patches from "teor".
It's now a protocol-warn, since there's nothing relay operators can
do about a client that sends them a malformed create cell.
Resolves bug 12996; bugfix on 0.0.6rc1.
Previously, we had done this only in the connection_free() case, but
when we called connection_free_() directly from
connections_free_all(), we didn't free the connections.
The fix for bug 4647 accidentally removed our hack from bug 586 that
rewrote HashedControlPassword to __HashedControlSessionPassword when
it appears on the commandline (which allowed the user to set her own
HashedControlPassword in the torrc file while the controller generates
a fresh session password for each run).
Fixes bug 12948; bugfix on 0.2.5.1-alpha.
This modifies the format of the bridge networkstatus documents produced
by the BridgeAuth. The new format adds a `published` line to the header
of the file, before the `flag-thresholds` line. This is done once per
networkstatus file that is written. The timestamp in the `published`
line is the standard ISO 8601 format (with a space as the separator)
used throughout the rest of tor.
* FIXES#12951https://bugs.torproject.org/12951
Using the *_array() functions here confused coverity, and was actually
a bit longer than we needed. Now we just use macros for the repeated
bits, so that we can mention a file and a suffix-appended version in
one line.
We had some code to fix up the 'status' return value to -1 on error
if it wasn't set, but it was unreachable because our code was
correct. Tweak this by initializing status to -1, and then only
setting it to 0 on success. Also add a goto which was missing: its
absence was harmless.
[CID 718614, 718616]
(We allowed it previously, but produced an LD_BUG message when it
happened, which is not consistent
Also, remove inconsistent NULL checks before calling
rend_service_intro_free.
(Removing the check is for CID 718613)
Coverity doesn't like doing NULL checks on things that can't be
NULL; I like checking things where the logic for their not being
NULL is nontrivial. Let's compromise, and make it obvious that this
field can't be NULL.
[Coverity CID 202004]
Coverity thinks that when we do "double x = int1/int2;", we probably
meant "double x = ((double)int1) / int2;". In these cases, we
didn't.
[Coverity CID 1232089 and 1232090]
Two bugs here:
1) We didn't add EXTEND2/EXTENDED2 to relay_command_to_string().
2) relay_command_to_string() didn't log the value of unrecognized
commands.
Both fixed here.
This will fix the warning
"/src/or/config.c:6854:48: error: unused parameter 'group_readable'"
that I introduced while fixing 12864.
Bug not in any released version of Tor.
Implements proposal 215; closes ticket 10163.
Why? From proposal 215:
Consensus method 1 is no longer viable for the Tor network. It
doesn't result in a microdescriptor consensus, and omits other
fields that clients need in order to work well. Consensus methods
under 12 have security issues, since they let a single authority
set a consensus parameter.
...
For example, while Tor 0.2.4.x is under development, authorities
should really not be running anything before Tor 0.2.3.x. Tor
0.2.3.x has supported consensus method 13 since 0.2.3.21-rc, so
it's okay for 0.2.4.x to require 13 as the minimum method. We even
might go back to method 12, since the worst outcome of not using 13
would be some warnings in client logs. Consensus method 12 was a
security improvement, so we don't want to roll back before that.
When we merged the cookieauthfile creation logic in 33c3e60a37, we
accidentally took out this feature. Fixes bug 12864, bugfix on
0.2.5.1-alpha.
Also adds an ExtORPortCookieAuthFileGroupReadable, since there's no
reason not to.
I looked for other places where we set circ->n_chan early, and found
one in circuit_handle_first_hop() right before it calls
circuit_send_next_onion_skin(). If onion_skin_create() fails there,
then n_chan will still be set when circuit_send_next_onion_skin()
returns. We should probably fix that too.
When Tor starts with DisabledNetwork set, it would correctly
conclude that it shouldn't try making circuits, but it would
mistakenly cache this conclusion and continue believing it even
when DisableNetwork is set to 0. Fixes the bug introduced by the
fix for bug 11200; bugfix on 0.2.5.4-alpha.
Those used to be normal to receive on hidden service circuits due to bug
1038, but the buggy Tor versions are long gone from the network so we
can afford to resume watching for them. Resolves the rest of bug 1038;
bugfix on 0.2.1.19.
Roger spotted this on tor-dev in his comments on proposal 221.
(Actually, detect DESTROY vs everything else, since arma likes
network timeout indicating failure but not overload indicating failure.)
Authorities now assign the Guard flag to the fastest 25% of the
network (it used to be the fastest 50%). Also raise the consensus
weight that guarantees the Guard flag from 250 to 2000. For the
current network, this results in about 1100 guards, down from 2500.
This step paves the way for moving the number of entry guards
down to 1 (proposal 236) while still providing reasonable expected
performance for most users.
Implements ticket 12690.
Otherwise, it always seems as though our Exclude* options have
changed, since we're comparing modified to unmodified values.
Patch from qwerty1. Fixes bug 9801. Bugfix on 0.2.4.10-alpha, where
GeoIPExcludeUnknown was introduced.
Currently tor fails to build its test when enabled with bufferevents
because an #ifndef USE_BUFFEREVENTS hides bucket_millis_empty() and
friends. This is fine if we don't run tests, but if we do, we need
these functions in src/or/libtor-testing.a when linking src/test/test.
This patch moves the functions outside the #ifndef and exposes them.
See downstream bug:
https://bugs.gentoo.org/show_bug.cgi?id=510124
When we run into bug 8387 (if we run into it again), report when we
last called circuit_expire_old_circuits_clientside(). This will let
us know -- if my fix for 8387 doesn't work -- whether my diagnosis
was at least correct.
Suggested by Andrea.
We should only assign a relay the HSDir flag if it is currently
considered valid. We can accomplish this by only considering active
relays, and as a consequence of this we also exclude relays that are
currently hibernating. Fixes#12573
Long ago we supported systems where there was no support for
threads, or where the threading library was broken. We shouldn't
have do that any more: on every OS that matters, threads exist, and
the OS supports running threads across multiple CPUs.
This resolves tickets 9495 and 12439. It's a prerequisite to making
our workqueue code work better, since sensible workqueue
implementations don't split across multiple processes.
The variable was useless since it was only toggled off in disabled code.
If the 'exit_family' smartlist is empty, we don't consider exit family
anyway.
Check for consistency between the queued destroy cells and the marked
circuit IDs. Check for consistency in the count of queued destroy
cells in several ways. Check to see whether any of the marked circuit
IDs have somehow been marked longer than the channel has existed.
And add a comment about why conditions that would cause us to drop a
cell should get checked before actions that would cause us to send a
destroy cell.
Spotted by 'cypherpunks'.
And note that these issues have been present since 0.0.8pre1 (commit
0da256ef), where we added a "shutting down" state, and started
responding to all create cells with DESTROY when shutting down.
Conflicts:
src/or/channel.c
src/or/circuitlist.c
src/or/connection.c
Conflicts involved removal of next_circ_id and addition of
unusable-circid tracking.
The point of the "idle timeout" for connections is to kill the
connection a while after it has no more circuits. But using "last
added a non-padding cell" as a proxy for that is wrong, since if the
last circuit is closed from the other side of the connection, we
will not have sent anything on that connection since well before the
last circuit closed.
This is part of fixing 6799.
When applied to 0.2.5, it is also a fix for 12023.
Instead of killing an or_connection_t that has had no circuits for
the last 3 minutes, give every or_connection_t a randomized timeout,
so that an observer can't so easily infer from the connection close
time the time at which its last circuit closed.
Also, increase the base timeout for canonical connections from 3
minutes to 15 minutes.
Fix for ticket 6799.
When we find a stranded one-hop circuit, log whether it is dirty,
log information about any streams on it, and log information about
connections they might be linked to.
This code mis-handled the case where a circuit got the same circuit
ID in both directions. I found three instances of it in the
codebase, by grepping for [pn]_circ_id.
Because of the issue in command_process_relay_cell(), this would
have made roughly one circuit in a million completely nonfunctional.
Fixes bug 12195.
On some profiles of Andrea's from #11332, I found that a great deal
of time can still be attributed to functions called from
update_router_have_minimum_dir_info(). This is making our
digestmap, tor_memeq, and siphash functions take a much bigger
portion of runtime than they really should.
If we're calling update_router_have_minimum_dir_info() too often,
that's because we're calling router_dir_info_changed() too often.
And it looks like most of the callers of router_dir_info_changed()
are coming as tail-calls from router_set_status() as invoked by
channel_do_open_actions().
But we don't need to call router_dir_info_changed() so much! (I'm
not quite sure we need to call it from here at all, but...) Surely
we don't need to call it from router_set_status when the router's
status has not actually changed.
This patch makes us call router_dir_info_changed() from
router_set_status only when we are changing the router's status.
Fix for bug 12170. This is leftover from our fix back in 273ee3e81
in 0.1.2.1-alpha, where we started caching the value of
update_router_have_minimum_dir_info().
tor_memeq has started to show up on profiles, and this is one of the
most frequent callers of that function, appearing as it does on every
cell handled for entry or exit.
59f9097d5c introduced tor_memneq here;
it went into Tor 0.2.1.31. Fixes part of 12169.
Without this fix, when running with bridges, we would try fetching
directory info far too early, and have up to a 60 second delay if we
started with bridge descriptors available.
Fixes bug 11965. Fix on 0.2.3.6-alpha, arma thinks.
The old cache had problems:
* It needed to be manually preloaded. (It didn't remember any
address you didn't tell it to remember)
* It was AF_INET only.
* It looked at its cache even if the sandbox wasn't turned on.
* It couldn't remember errors.
* It had some memory management problems. (You can't use memcpy
to copy an addrinfo safely; it has pointers in.)
This patch fixes those issues, and moves to a hash table.
Fixes bug 11970; bugfix on 0.2.5.1-alpha.
The code was not disambiguating ClientTransportPlugin configured and
not used, and ClientTransportPlugin configured, but in a failed state.
The right thing to do is to undo moving the get_transport_by_addrport()
call back into get_proxy_addrport(), and remove and explicit check for
using a Bridge since by the time the check is made, if a Bridge is
being used, it is PT/proxy-less.
This change allows using Socks4Proxy, Socks5Proxy and HTTPSProxy with
ClientTransportPlugins via the TOR_PT_PROXY extension to the
pluggable transport specification.
This fixes bug #8402.
None of the things we might exec() can possibly run under the
sanbox, so rather than crash later, we have to refuse to accept the
configuration nice and early.
The longer-term solution is to have an exec() helper, but wow is
that risky.
fixes 12043; bugfix on 0.2.5.1-alpha
When we converted the horrible set of options that previously
controlled "use ORPort or DirPort? Anonymously or Non-anonymouly?" to
a single 'indirection' argument, we missed
directory_post_to_dirservers.
The problematic code was introduced in 5cbeb6080, which went into
0.2.4.3-alpha. This is a fix for bug 11469.
If somebody has configured a client to use a bridge without setting
an identity digest (not recommended), learn the identity digest from
whatever bridge descriptor we have downloaded or have in our cache.
When running with User set, we frequently try to look up our
information in the user database (e.g., /etc/passwd). The seccomp2
sandbox setup doesn't let us open /etc/passwd, and probably
shouldn't.
To fix this, we have a pair of wrappers for getpwnam and getpwuid.
When a real call to getpwnam or getpwuid fails, they fall back to a
cached value, if the uid/gid matches.
(Granting access to /etc/passwd isn't possible with the way we
handle opening files through the sandbox. It's not desirable either.)
On OpenBSD 5.4, time_t is a 32-bit integer. These instances contain
implicit treatment of long and time_t as comparable types, so explicitly
cast to time_t.
Clients should always believe that v3 directory authorities serve
extra-info documents, regardless of whether their server descriptor
contains a "caches-extra-info" line or not.
Fixes part of #11683.
on #9686, gmorehose reports that the 500 MB lower limit is too high
for raspberry pi users.
This is a backport of 647248729f to 0.2.4.
Note that in 0.2.4, the option is called MaxMemInCellQueues.
This was previously satisfied by using a temporary variable, but there
are three other instances in circuitlist.c that gcc is now bothered by,
so now introduce a CONST_TO_ORIGIN_CIRCUIT that takes a const
circuit_t instead.
When clearing a list of tokens, it's important to do token_clear()
on them first, or else any keys they contain will leak. This didn't
leak memory on any of the successful microdescriptor parsing paths,
but it does leak on some failing paths when the failure happens
during tokenization.
Fixes bug 11618; bugfix on 0.2.2.6-alpha.
If we can't detect the physical memory, the new default is 8 GB on
64-bit architectures, and 1 GB on 32-bit architectures.
If we *can* detect the physical memory, the new default is
CLAMP(256 MB, phys_mem * 0.75, MAX_DFLT)
where MAX_DFLT is 8 GB on 64-bit architectures and 2 GB on 32-bit
architectures.
You can still override the default by hand. The logic here is simply
trying to choose a lower default value on systems with less than 12 GB
of physical RAM.
Use a per-channel ratelim_t to control the rate at which we report
failures for each channel.
Explain why I picked N=32.
Never return a zero circID.
Thanks to Andrea and to cypherpunks.
Instead of taking the length of a buffer, we were taking the length of
a pointer, so that our debugging log would cover only the first
sizeof(void*) bytes of the client nonce.
If 'intro' is NULL in these functions, I'm pretty sure that the
error message must be set before we hit the end. But scan-build
doesn't notice that, and is worried that we'll do a null-pointer
dereference in the last-chance errormsg generation.
As it stands, it relies on the fact that onion_queue_entry_remove
will magically remove each onionskin from the right list. This
patch changes the logic to be more resilient to possible bugs in
onion_queue_entry_remove, and less confusing to static analysis tools.
scan-build doesn't realize that a request can't be timed at the end
unless it's timed at the start, and so it's not possible for us to
be subtracting start from end without start being set.
Nevertheless, let's not confuse it.
When get_proxy_addrport returned PROXY_NONE, it would leave
addr/port unset. This is inconsistent, and could (if we used the
function in a stupid way) lead to undefined behavior. Bugfix on
5b050a9b0, though I don't think it affects tor-as-it-is.
Throughout circuituse, when we log about a circuit, we log its
desired path length from build_state. scan-build is irrationally
concerned that build_state might be NULL.
In circuitmux_detach_all_circuits, we check whether an HT iterator
gives us NULL. That should be impossible for an HT iterator. But
our checking it has confused scan-build (justly) into thinking that
our later use of HT_NEXT_RMV might not be kosher. I'm taking the
coward's route here and strengthening the check. Bugfix on
fd31dd44. (Not a real bug though)
If we fail in circuit_get_by_rend_token_and_purpose because the
circuit has no rend_info, don't try to reference fiends from its
rend_info when logging an error. Bugfix on 8b9a2cb68, which is
going into Tor 0.2.5.4-alpha.
Fixes a possible root cause of 11553 by only making 64 attempts at
most to pick a circuitID. Previously, we would test every possible
circuit ID until we found one or ran out.
This algorithm succeeds probabilistically. As the comment says:
This potentially causes us to give up early if our circuit ID
space is nearly full. If we have N circuit IDs in use, then we
will reject a new circuit with probability (N / max_range) ^
MAX_CIRCID_ATTEMPTS. This means that in practice, a few percent
of our circuit ID capacity will go unused.
The alternative here, though, is to do a linear search over the
whole circuit ID space every time we extend a circuit, which is
not so great either.
This makes new vs old clients distinguishable, so we should try to
batch it with other patches that do that, like 11438.
This means that tor can run without needing to communicate with ioctls
to the firewall, and therefore doesn't need to run with privileges to
open the /dev/pf device node.
A new TransProxyType is added for this purpose, "pf-divert"; if the user
specifies this TransProxyType in their torrc, then the pf device node is
never opened and the connection destination is determined with getsockname
(as per pf(4)). The default behaviour (ie., when TransProxyType is "default"
when using the pf firewall) is still to assume that pf is configured with
rdr-to rules.
This isn't on by default; to get it, you need to set "TransProxyType
ipfw". (The original patch had automatic detection for whether
/dev/pf is present and openable, but that seems marginally fragile.)
Libevent uses an arc4random implementation (I know, I know) to
generate DNS transaction IDs and capitalization. But it liked to
initialize it either with opening /dev/urandom (which won't work
under the sandbox if it doesn't use the right pointer), or with
sysctl({CTL_KERN,KERN_RANDOM,RANDOM_UUIC}). To make _that_ work, we
were permitting sysctl unconditionally. That's not such a great
idea.
Instead, we try to initialize the libevent PRNG _before_ installing
the sandbox, and make sysctl always fail with EPERM under the
sandbox.
Appearently, the majority of the filenames we pass to
sandbox_cfg_allow() functions are "freeable right after". So, consider
_all_ of them safe-to-steal, and add a tor_strdup() in the few cases
that aren't.
(Maybe buggy; revise when I can test.)
(If we don't restrict rename, there's not much point in restricting
open, since an attacker could always use rename to make us open
whatever they want.)
A new set of unit test cases are provided, as well as introducing
an alternative paradigm and macros to support it. Primarily, each test
case is given its own namespace, in order to isolate tests from each
other. We do this by in the usual fashion, by appending module and
submodule names to our symbols. New macros assist by reducing friction
for this and other tasks, like overriding a function in the global
namespace with one in the current namespace, or declaring integer
variables to assist tracking how many times a mock has been called.
A set of tests for a small-scale module has been included in this
commit, in order to highlight how the paradigm can be used. This
suite gives 100% coverage to status.c in test execution.
My first implementation was broken, since it returned "whether there
is one bridge" rather than "how many bridges."
Also, the implementation for the n_options_out feature in
choose_random_entry_impl was completely broken due to a missing *.
When we successfully create a usable circuit after it previously
timed out for a certain amount of time, we should make sure that
our public IP address hasn't changed and update our descriptor.
In C, it's a bad idea to do this:
char *cp = array;
char *end = array + array_len;
/* .... */
if (cp + 3 >= end) { /* out of bounds */ }
because cp+3 might be more than one off the end of the array, and
you are only allowed to construct pointers to the array elements,
and to an element one past the end. Instead you have to say
if (cp - array + 3 >= array_len) { /* ... */ }
or something like that.
This patch fixes two of these: one in process_versions_cell
introduced in 0.2.0.10-alpha, and one in process_certs_cell
introduced in 0.2.3.6-alpha. These are both tracked under bug
10363. "bobnomnom" found and reported both. See also 10313.
In our code, this is likely to be a problem as we used it only if we
get a nasty allocator that makes allocations end close to (void*)-1.
But it's best not to have to worry about such things at all, so
let's just fix all of these we can find.
According to reports, most programs degrade somewhat gracefully on
getting no answer for an MX or a CERT for www.example.com, but many
flip out completely on a NOTIMPL error.
Also, treat a QTYPE_ALL query as just asking for an A record.
The real fix here is to implement proposal 219 or something like it.
Fixes bug 10268; bugfix on 0.2.0.1-alpha.
Based on a patch from "epoch".
Found by testing with chutney. The old behavior was "fail an
assertion", which obviously isn't optimal.
Bugfix on 8b9a2cb68b290e550695124d7ef0511225b451d5; bug not in any
released version.
Also, stop accepting the old kind of RESOLVED cells with no TTL
fields; they haven't been sent since 0.1.1.6-alpha.
This patch won't work without the fix to #10468 -- it will break
DNSPorts unless they set the proper ipv4/6 flags on entry_connection_t.
Otherwise, it could mung the thing that came over the net on windows,
which would defeat the purpose of recording the unparseable thing.
Fixes bug 11342; bugfix on 0.2.2.1-alpha.
This is a fix for 9963. I say this is a feature, but if it's a
bugfix, it's a bugfix on 0.2.4.18-rc.
Old behavior:
Mar 27 11:02:19.000 [notice] Bootstrapped 50%: Loading relay descriptors.
Mar 27 11:02:20.000 [notice] Bootstrapped 51%: Loading relay descriptors.
Mar 27 11:02:20.000 [notice] Bootstrapped 52%: Loading relay descriptors.
... [Many lines omitted] ...
Mar 27 11:02:29.000 [notice] Bootstrapped 78%: Loading relay descriptors.
Mar 27 11:02:33.000 [notice] We now have enough directory information to build circuits.
New behavior:
Mar 27 11:16:17.000 [notice] Bootstrapped 50%: Loading relay descriptors
Mar 27 11:16:19.000 [notice] Bootstrapped 55%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 60%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 65%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 70%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] Bootstrapped 75%: Loading relay descriptors
Mar 27 11:16:21.000 [notice] We now have enough directory information to build circuits.
Most of these are simple. The only nontrivial part is that our
pattern for using ENUM_BF was confusing doxygen by making declarations
that didn't look like declarations.
In circuitlist_free_all, we free all the circuits, removing them from
the map as we go, but we weren't actually freeing the placeholder
entries that we use to indicate pending DESTROY cells.
Fix for bug 11278; bugfix on the 7912 code that was merged in
0.2.5.1-alpha
There are still quite a few 0.2.3.2x relays running for x<5, and while I
agree they should upgrade, I don't think cutting them out of the network
is a net win on either side.
This contains the obvious implementation using the circuitmux data
structure. It also runs the old (slow) algorithm and compares
the results of the two to make sure that they're the same.
Needs review and testing.
This change prevents LD_BUG warnings and bootstrap failure messages
when we try to do directory fetches when starting with
DisableNetwork == 1, a consensus present, but no descriptors (or
insufficient descriptors) yet.
Fixes bug 11200 and bug 10405. It's a bugfix on 0.2.3.9-alpha.
Thanks to mcs for walking me through the repro instructions!
This is meant to be a better bug 9229 fix -- or at least, one more
in tune with the intent of the original code, which calls
router_retry_directory_downloads() only on the first bridge descriptor.
This prevents long stalls when we're starting with a state file but
with no bridge descriptors. Fixes bug 9229. I believe this bug has
been present since 0.2.0.3-alpha.
By default, after you've made a connection to port XYZ, we assume
you might still want to have an exit ready to connect to XYZ for one
hour. This patch lets you lower that interval.
Implements ticket 91
This should fixes some "hey, that function could have
__attribute__((noreturn))" warnings introduced by f96400d9.
Bug not in any released version of Tor.
We have ignored any ports listed here since 80365b989 (0.0.7rc1),
but we didn't warn the user that we were ignoring them. This patch
adds a warning if you put explicit ports in any of the options
{Socks,Dir}Policy or AuthDir{Reject,Invalid,BadDir,BadExit}. It
also adjusts the manpage to say that ports are ignored.
Fixes ticket 11108.
In a couple of places, to implement the OOM-circuit-killer defense
against sniper attacks, we have counters to remember the age of
cells or data chunks. These timers were based on wall clock time,
which can move backwards, thus giving roll-over results for our age
calculation. This commit creates a low-budget monotonic time, based
on ratcheting gettimeofday(), so that even in the event of a time
rollback, we don't do anything _really_ stupid.
A future version of Tor should update this function to do something
even less stupid here, like employ clock_gettime() or its kin.
Back in 5e762e6a5c, non-exit servers
stopped launching DNS requests for users. So there's no need for them
to see if their DNS answers are hijacked.
Patch from Matt Pagan. I think this is a 965 fix.
For a client using a SocksPort connection and IPv6, the connect reply
from tor daemon did not handle AF_INET6 thus sending back the wrong
payload to the client.
A changes file is provided and this fixes#10987
Signed-off-by: David Goulet <dgoulet@ev0ke.net>
When we have more than two return values, we should really be using
an enum rather than "-2 means this, -1 means that, 0 means this, and
1 or more means a number."
On busy servers, this function takes up something like 3-7% in
different profiles, and gets invoked every time we need to participate
as the midpoint in a hidden service.
So maybe walking through a linked list of all the circuits here wasn't
a good idea.
We log only one message, containing a complete list of what's
wrong. We log the complete list whenever any of the possible things
that could have gotten wrong gets worse.
Fix for #9870. Bugfix on 10480dff01, which we merged in
0.2.5.1-alpha.
This patch splits out some of the functions in OOM handling so that
it's easier to check them without involving the rest of Tor or
requiring that the circuits be "wired up".
It's increasingly apparent that we want to make sure we initialize our
PRNG nice and early, or else OpenSSL will do it for us. (OpenSSL
doesn't do _too_ bad a job, but it's nice to do it ourselves.)
We'll also need this for making sure we initialize the siphash key
before we do any hashes.
I've made an exception for cases where I'm sure that users can't
influence the inputs. This is likely to cause a slowdown somewhere,
but it's safer to siphash everything and *then* look for cases to
optimize.
This patch doesn't actually get us any _benefit_ from siphash yet,
since we don't really randomize the key at any point.
These options were added back in 0.1.2.5-alpha, but no longer make any
sense now that all directories support tunneled connections and
BEGIN_DIR cells. These options were on by default; now they are
always-on.
This is a fix for 10849, where TunnelDirConns 0 would break hidden
services -- and that bug arrived, I think, in 0.2.0.10-alpha.
(There is no longer meaningfully any such thing as a HS authority,
since we stopped uploading or downloading v0 hs descriptors in
0.2.2.1-alpha.)
Implements #10881, and part of #10841.
This patch removes an "if (chan)" that occurred at a place where
chan was definitely non-NULL. Having it there made some static
analysis tools conclude that we were up to shenanigans.
This resolves#9979.
Right now this accounts for about 1% of circuits over all, but if you
pick a guard that's running 0.2.3, it will be about 6% of the circuits
running through that guard.
Making sure that every circuit has at least one ntor link means that
we're getting plausibly good forward secrecy on every circuit.
This implements ticket 9777,
It's possible to set your ExitNodes to contains only exits that don't
have the Exit flag. If you do that, we'll decide that 0 of your exits
are working. Instead, in that case we should look at nodes which have
(or which might have) exit policies that don't reject everything.
Fix for bug 10543; bugfix on 0.2.4.10-alpha.
According to control spec, longname should not contain any spaces and is
consists only of identy_digest + nickname
added two functions:
* node_get_verbose_nickname_by_id()
* node_describe_longname_by_id()
If you want a slow shutdown, send SIGNAL SHUTDOWN.
(Why not just have the default be SIGNAL QUIT? Because this case
should only happen when an owning controller has crashed, and a
crashed controller won't be able to give the user any "tor is
shutting down" feedback, and so the user gets confused for a while.
See bug 10449 for more info)
I'm doing this because:
* User doesn't mean you're running as root, and running as root
doesn't mean you've set User.
* It's possible that the user has done some other
capability-based hack to retain the necessary privileges.
The remaining vestige is that we continue to publish the V2dir flag,
and that, for the controller, we continue to emit v2 directory
formats when requested.
Previously, we would sometimes decide in directory_get_from_hs_dir()
to connect to an excluded node, and then later in
directory_initiate_command_routerstatus_rend() notice that it was
excluded and strictnodes was set, and catch it as a stopgap.
Additionally, this patch preferentially tries to fetch from
non-excluded nodes even when StrictNodes is off.
Fix for bug #10722. Bugfix on 0.2.0.10-alpha (the v2 hidserv directory
system was introduced in e136f00ca). Reported by "mr-4".
If we don't, we can wind up with a wedged cpuworker, and write to it
for ages and ages.
Found by skruffy. This was a bug in 2dda97e8fd, a.k.a. svn
revision 402. It's been there since we have been using cpuworkers.
When I introduced the unusable_for_new_circuits flag in
62fb209d83, I had a spurious ! in the
circuit_stream_is_being_handled loop. This made us decide that
non-unusable circuits (that is, usable ones) were the ones to avoid,
and caused it to launch a bunch of extra circuits.
Fixes bug 10456; bugfix on 0.2.4.12-alpha.
When we wrote the directory request statistics code in August 2009, we
thought that these statistics were only relevant for bridges, and that
bridges should not report them. That's why we added a switch to discard
relevant observations made by bridges. This code was first released in
0.2.2.1-alpha.
In May 2012 we learned that we didn't fully disable directory request
statistics on bridges. Bridges did report directory request statistics,
but these statistics contained empty dirreq-v3-ips and dirreq-v3-reqs
lines. But the remaining dirreq-* lines have always been non-empty. (We
didn't notice for almost three years, because directory-request statistics
were disabled by default until 0.2.3.1-alpha, and all statistics have been
removed from bridge descriptors before publishing them on the metrics
website.)
Proposal 201, created in May 2012, suggests to add a new line called
bridge-v3-reqs that is similar to dirreq-v3-reqs, but that is published
only by bridges. This proposal is still open as of December 2013.
Since October 2012 we're using dirreq-v3-resp (not -reqs) lines in
combination with bridge-ips lines to estimate bridge user numbers; see
task 8462. This estimation method has superseded the older approach that
was only based on bridge-ips lines in November 2013. Using dirreq-v3-resp
and bridge-ips lines is a workaround. The cleaner approach would be to
use dirreq-v3-reqs instead.
This commit makes bridges report the same directory request statistics as
relays, including dirreq-v3-ips and dirreq-v3-reqs lines. It makes
proposal 201 obsolete.
In 0.2.3.8-alpha we attempted to "completely disable stats if we aren't
running as a relay", but instead disabled them only if we aren't running
as a server.
This commit leaves DirReqStatistics enabled on both relays and bridges,
and disables (Cell,Entry,ExitPort)Statistics on bridges.
The 'body' field of a microdesc_t holds a strdup()'d value if the
microdesc's saved_location field is SAVED_IN_JOURNAL or
SAVED_NOWHERE, and holds a pointer to the middle of an mmap if the
microdesc is SAVED_IN_CACHE. But we weren't setting that field
until a while after we parsed the microdescriptor, which left an
interval where microdesc_free() would try to free() the middle of
the mmap().
This patch also includes a regression test.
This is a fix for #10409; bugfix on 0.2.2.6-alpha.
The old behavior was that NULL matched only bridges without known
identities; the correct behavior is that NULL should match all
bridges (assuming that their addr:port matches).
We were checking whether a 8-bit length field had overflowed a
503-byte buffer. Unless somebody has found a way to store "504" in a
single byte, it seems unlikely.
Fix for 10313 and 9980. Based on a pach by Jared L Wong. First found
by David Fifield with STACK.
It's conceivable (but probably impossible given our code) that lseek
could return -1 on an error; when that happens, we don't want off to
become -1.
Fixes CID 1035124.
This was a mistake in the merge commit 7a2b30fe16. It
would have made the CellStatistics code give completely bogus
results. Bug not in any released Tor.
We had accidentially grown two fake ones: one for backtrace.c, and one
for sandbox.c. Let's do this properly instead.
Now, when we configure logs, we keep track of fds that should get told
about bad stuff happening from signal handlers. There's another entry
point for these that avoids using non-signal-handler-safe functions.
On platforms with the backtrace/backtrace_symbols_fd interface, Tor
can now dump stack traces on assertion failure. By default, I log
them to DataDir/stack_dump and to stderr.
Conflicts:
src/or/or.h
src/or/relay.c
Conflicts were simple to resolve. More fixes were needed for
compilation, including: reinstating the tv_to_msec function, and renaming
*_conn_cells to *_chan_cells.
In proposal 157, we added a cross-certification element for
directory authority certificates. We implemented it in
0.2.1.9-alpha. All Tor directory authorities now generate it.
Here, as planned, make it required, so that we can finally close
proposal 157.
The biggest change in the code is in the unit test data, where some
old hardcoded certs that we made long ago have become no longer
valid and now need to be replaced.
Previously, when we ran low on memory, we'd close whichever circuits
had the most queued cells. Now, we close those that have the
*oldest* queued cells, on the theory that those are most responsible
for us running low on memory, and that those are the least likely to
actually drain on their own if we wait a little longer.
Based on analysis from a forthcoming paper by Jansen, Tschorsch,
Johnson, and Scheuermann. Fixes bug 9093.
Roger spotted this on tor-dev in his comments on proposal 221.
We etect DESTROY vs everything else, since arma likes network
timeout indicating failure but not overload indicating failure.
Don't cast uint64_t * to const uint64_t * explicitly. The cast is always
safe, so C does it for us. Doing the cast explitictly can hide bugs if
the input is secretly the wrong type.
Suggested by Nick.
As a bridge authority, before we create our networkstatus document, we
should compute the thresholds needed for the various status flags
assigned to each bridge based on the status of all other bridges. We
then add these thresholds to the networkstatus document for easy access.
Fixes for #1117 and #9859.
Also fix a bug where if the guard we choose first doesn't answer, we
would try the second guard, but once we connected to the second guard
we would abandon it and retry the first one, slowing down bootstrapping.
The fix in both cases is to treat all our initially chosen guards as
acceptable to use.
Fixes bug 9946.
We had updated our "do we have enough microdescs to begin building
circuits?" logic most recently in 0.2.4.10-alpha (see bug 5956), but we
left the bootstrap status event logic at "how far through getting 1/4
of them are we?"
Fixes bug 9958; bugfix on 0.2.2.36, which is where they diverged (see
bug 5343).
The old code had logic to use a shorter path length if we didn't
have enough nodes. But we don't support 2-node networks anwyay.
Fix for #9926. I'm not calling this a bugfix on any particular
version, since a 2-node network would fail to work for you for a lot
of other reasons too, and it's not clear to me when that began, or if
2-node networks would ever have worked.
By calling circuit_n_chan_done() unconditionally on close, we were
closing pending connections that might not have been pending quite for
the connection we were closing. Fix for bug 9880.
Thanks to skruffy for finding this and explaining it patiently until
we understood.
this was causing directory authorities to send a time of 0 on all
connections they generated themselves, which means everybody reachability
test caused a time skew warning in the log for that relay.
(i didn't just revert, because the changes file has been modified by
other later commits.)
This isn't actually much of an issue, since only relays send
AUTHENTICATE cells, but while we're removing timestamps, we might as
well do this too.
Part of proposal 222. I didn't take the approach in the proposal of
using a time-based HMAC, since that was a bad-prng-mitigation hack
from SSL3, and in real life, if you don't have a good RNG, you're
hopeless as a Tor server.
For now, round down to the nearest 10 minutes. Later, eliminate entirely by
setting a consensus parameter.
(This rounding is safe because, in 0.2.2, where the timestamp mattered,
REND_REPLAY_TIME_INTERVAL was a nice generous 60 minutes.)
We were freeing these on exit, but when we added the dl_status_map
field to them in fddb814f, we forgot to arrange for it to be freed.
I've moved the cert_list_free() code into its own function, and added
an appropriate dsmap_free() call.
Fixes bug 9644; bugfix on 0.2.4.13-alpha.
The problem was that the server_identity_key_is_set() function could
return true under conditions where we don't really have an identity
key -- specifically, where we used to have one, but we stopped being a
server.
This is a fix for 6979; bugfix on 0.2.2.18-alpha where we added that
assertion to get_server_identity_key().
Whenever we had an non-option commandline arguments *and*
option-bearing commandline arguments on the commandline, we would save
only the latter across invocations of options_init_from_torrc, but
take their existence as license not to re-parse the former. Yuck!
Incidentally, this fix lets us throw away the backup_arg[gv] logic.
Fix for bug 9746; bugfix on d98dfb3746,
not in any released Tor. Found by Damian. Thanks, Damian!
This just goes to show: never cast a function pointer. Found while
testing new command line parse logic.
Bugfix on 1293835440, which implemented
6752: Not in any released tor.
Fall back to SOMAXCONN if INT_MAX doesn't work.
We'd like to do this because the actual maximum is overrideable by the
kernel, and the value in the header file might not be right at all.
All implementations I can find out about claim that this is supported.
Fix for 9716; bugfix on every Tor.
Now we explicitly check for overflow.
This approach seemed smarter than a cascade of "change int to unsigned
int and hope nothing breaks right before the release".
Nick, feel free to fix in a better way, maybe in master.
This would make us do testing circuits "even when cbt is disabled by
consensus, or when we're a directory authority, or when we've failed
to write cbt history to our state file lately." (Roger's words.)
This is a fix for 9671 and an improvement in our fix for 5049.
The original misbehavior was in 0.2.2.14-alpha; the incomplete
fix was in 0.2.3.17-beta.
(In practice they don't exist, but so long as we're making changes for
standards compliance...)
Also add several more unit tests for good and bad URL types.
There were only two functions outside of circuitstats that actually
wanted to know what was inside this. Making the structure itself
hidden should help isolation and prevent us from spaghettifying the
thing more.
The spec requires them to do so, and not doing so creates a situation
where they can't send-test because relays won't extend to them because
of the other part of bug 9546.
Fixes bug 9546; bugfix on 0.2.3.6-alpha.
The spec requires them to do so, and not doing so creates a situation
where they can't send-test because relays won't extend to them because
of the other part of bug 9546.
Fixes bug 9546; bugfix on 0.2.3.6-alpha.
(Backport to Tor 0.2.3)
Relays previously, when initiating a connection, would only send a
NETINFO after sending an AUTHENTICATE. But bridges, when receiving a
connection, would never send AUTH_CHALLENGE. So relays wouldn't
AUTHENTICATE, and wouldn't NETINFO, and then bridges would be
surprised to be receiving CREATE cells on a non-open circuit.
Fixes bug 9546.
Relays previously, when initiating a connection, would only send a
NETINFO after sending an AUTHENTICATE. But bridges, when receiving a
connection, would never send AUTH_CHALLENGE. So relays wouldn't
AUTHENTICATE, and wouldn't NETINFO, and then bridges would be
surprised to be receiving CREATE cells on a non-open circuit.
Fixes bug 9546.
Use the generic function for both the ControlPort cookie and the
ExtORPort cookie.
Also, place the global cookie variables in the heap so that we can
pass them around more easily as pointers.
Also also, fix the unit tests that broke by this change.
Conflicts:
src/or/config.h
src/or/ext_orport.c
memwipe some stack-allocated stuff
Add DOCDOC comments for state machines
Use memdup_nulterm as appropriate
Check for NULs in useraddr
Add a macro so that <= AUTH_MAX has a meaning.
- Don't leak if a transport proxy sends us a TRANSPORT command more
than once.
- Don't use smartlist_string_isin() in geoip_get_transport_history().
(pointed out by Nick)
- Use the 'join' argument of smartlist_join_strings() instead of
trying to write the separator on our own.
(pointed out by Nick)
- Document 'ext_or_transport' a bit better.
(pointed out by Nick)
- Be a bit more consistent with the types of the values of 'transport_counts'.
(pointed out by Nick)
Fortunately, later checks mean that uninitialized data can't get sent
to the network by this bug. Unfortunately, reading uninitialized heap
*can* (in some cases, with some allocators) cause a crash if you get
unlucky and go off the end of a page.
Found by asn. Bugfix on 0.2.4.1-alpha.
Both 'managed_proxy_list' and 'unconfigured_proxies_n' are global
src/or/transports.c variables that are not initialized properly when
unit tests are run.
When we moved channel_matches_target_addr_for_extend() into a separate
function, its sense was inverted from what one might expect, and we
didn't have a ! in one place where we should have.
Found by skruffy.
When we moved channel_matches_target_addr_for_extend() into a separate
function, its sense was inverted from what one might expect, and we
didn't have a ! in one place where we should have.
Found by skruffy.
I added this so I could write a unit test for ServerTransportOptions,
but it incidentally exercises the succeed-on-defaults case of
options_validate too.
Previously we would accept relative paths, but only if they contained a
slash somewhere (not at the end).
Otherwise we would silently not work. Closes: #9258. Bugfix on
0.2.3.16-alpha.
If you pass the --enable-coverage flag on the command line, we build
our testing binaries with appropriate options eo enable coverage
testing. We also build a "tor-cov" binary that has coverage enabled,
for integration tests.
On recent OSX versions, test coverage only works with clang, not gcc.
So we warn about that.
Also add a contrib/coverage script to actually run gcov with the
appropriate options to generate useful .gcov files. (Thanks to
automake, the .o files will not have the names that gcov expects to
find.)
Also, remove generated gcda and gcno files on clean.
We previously used FILENAME_PRIVATE identifiers mostly for
identifiers exposed only to the unit tests... but also for
identifiers exposed to the benchmarker, and sometimes for
identifiers exposed to a similar module, and occasionally for no
really good reason at all.
Now, we use FILENAME_PRIVATE identifiers for identifiers shared by
Tor and the unit tests. They should be defined static when we
aren't building the unit test, and globally visible otherwise. (The
STATIC macro will keep us honest here.)
For identifiers used only by the unit tests and never by Tor at all,
on the other hand, we wrap them in #ifdef TOR_UNIT_TESTS.
This is not the motivating use case for the split test/non-test
build system; it's just a test example to see how it works, and to
take a chance to clean up the code a little.
This is mainly a matter of automake trickery: we build each static
library in two versions now: one with the TOR_UNIT_TESTS macro
defined, and one without. When TOR_UNIT_TESTS is defined, we can
enable mocking and expose more functions. When it's not defined, we
can lock the binary down more.
The alternatives would be to have alternate build modes: a "testing
configuration" for building the libraries with test support, and a
"production configuration" for building them without. I don't favor
that approach, since I think it would mean more people runnning
binaries build for testing, or more people not running unit tests.
Fix a bug in the voting algorithm that could yield incorrect results
when a non-naming authority declared too many flags. Fixes bug 9200;
bugfix on 0.2.0.3-alpha.
Found by coverity scan.
This implements "algorithm 1" from my discussion of bug #9072: on OOM,
find the circuits with the longest queues, and kill them. It's also a
fix for #9063 -- without the side-effects of bug #9072.
The memory bounds aren't perfect here, and you need to be sure to
allow some slack for the rest of Tor's usage.
This isn't a perfect fix; the rest of the solutions I describe on
codeable.
In my #7912 fix, there wasn't any code to remove entries from the
(channel, circuit ID)->circuit map corresponding to queued but un-sent
DESTROYs.
Spotted by skruffy. Fixes bug 9082; bug not in any released Tor.
I added the code to pass a destroy cell to a queueing function rather
than writing it immediately, and the code to remember that we
shouldn't reuse the circuit id until the destroy is actually sent, and
the code to release the circuit id once the destroy has been sent...
and then I finished by hooking destroy_cell_queue into the rest of
Tor.
This is a reprise of the fix in bdff7e3299d78; 6905c1f6 reintroduced
that bug. Briefly: windows doesn't seem to like deleting a mapped
file. I tried adding the PROT_SHARED_DELETE flag to the createfile
all, but that didn't actually fix this issue. Fortunately, the unit
test I added in 4f4fc63fea should
prevent us from making this particular screw-up again.
This patch also tries to limit the crash potential of a failure to
write by a little bit, although it could do a better job of retaining
microdescriptor bodies.
Fix for bug 8822, bugfix on 0.2.4.12-alpha.
There's an assertion failure that can occur if a connection has
optimistic data waiting, and then the connect() call returns 0 on the
first attempt (rather than -1 and EINPROGRESS). That latter behavior
from connect() appears to be an (Open?)BSDism when dealing with remote
addresses in some cases. (At least, I've only seen it reported with
the BSDs under libevent, even when the address was 127.0.0.1. And
we've only seen this problem in Tor with OpenBSD.)
Fixes bug 9017; bugfix on 0.2.3.1-alpha, which first introduced
optimistic data. (Although you could also argue that the commented-out
connection_start_writing in 155c9b80 back in 2002 is the real source
of the issue.)
A new option TestingV3AuthVotingStartOffset is added which offsets the
starting time of the voting interval. This is possible only when
TestingTorNetwork is set.
This patch makes run_scheduled_events() check for new consensus
downloads every second when TestingTorNetwork, instead of every
minute. This should be fine, see #8532 for reasoning.
This patch also brings MIN_VOTE_SECONDS and MIN_DIST_SECONDS down from
20 to 2 seconds, unconditionally. This makes sanity checking of
misconfiguration slightly less sane.
Addresses #8532.
You can't use != to compare arbitary members of or_options_t.
(Also, generate a better error message to say which Testing* option
was set.)
Fix for bug 8992. Bugfix on b0d4ca49. Bug not in any released Tor.
- Rename n_read and n_written in origin_circuit_t to make it clear that
these are only used for CIRC_BW events.
- Extract new code in control_update_global_event_mask to new
clear_circ_bw_fields function.
- Avoid control_event_refill_global function with 13 arguments and
increase code reuse factor by moving more code from control.c to
connection.c.
- Avoid an unsafe uint32_t -> int cast.
- Add TestingEnableTbEmptyEvent option.
- Prepare functions for testing.
- Rename a few functions and improve documentation.
- Move cell_command_to_string from control.c to command.c.
- Use accessor for global_circuitlist instead of extern.
- Add a struct for cell statistics by command instead of six arrays.
- Split up control_event_circuit_cell_stats by using two helper functions.
- Add TestingEnableCellStatsEvent option.
- Prepare functions for testing.
- Rename a few variables and document a few things better.
- Move new ID= parameter in ORCONN event to end. Avoids possible trouble
from controllers that parse parameters by position, even though they
shouldn't.
Create new methods check_or_create_data_subdir() and
write_to_data_subdir() in config.c and use them throughout
rephist.c and geoip.c.
This should solve ticket #4282.
This is a fix for bug 8844, where eugenis correctly notes that there's
a sentinel value at the end of the list-of-freelists that's never
actually checked. It's a bug since the first version of the chunked
buffer code back in 0.2.0.16-alpha.
This would probably be a crash bug if it ever happens, but nobody's
ever reported something like this, so I'm unsure whether it can occur.
It would require write_to_buf, write_to_buf_zlib, read_to_buf, or
read_to_buf_tls to get an input size of more than 32K. Still, it's a
good idea to fix this kind of thing!
It appears that moria1 crashed because of one instance of this (the
one in router_counts_toward_thresholds). The other instance I fixed
won't actually have broken anything, but I think it's more clear this
way.
Fixes bug 8833; bugfix on 0.2.4.12-alpha.
We need to subtract both the current built circuits *and* the attempted
circuits from the attempt count during scaling, since *both* have already been
counted there.
It sure is a good thing we can run each test in its own process, or
else the amount of setup I needed to do to make this thing work
would have broken all the other tests.
Test mocking would have made this easier to write too.
Now we can compute the hash and signature of a dirobj before
concatenating the smartlist, and we don't need to play silly games
with sigbuf and realloc any more.
I believe this was introduced in 6bc071f765, which makes
this a fix on 0.2.0.10-alpha. But my code archeology has not extended
to actually testing that theory.
We were mixing bandwidth file entries (which are in kilobytes) with
router_get_advertised_bw() entries, which were in bytes.
Also, use router_get_advertised_bandwidth_capped() for credible_bandwidth.
Since 7536c40 only DNS results for real SOCKS requests are added to the cache,
but not DNS results for DNSPort queries or control connection RESOLVE queries.
Only cache additions would trigger ADDRMAP events on successful resolve.
Change it so that DNS results received after a RESOLVE command also generate
ADDRMAP events.
There was a bug in Tor prior to 0.2.4.10-alpha that allowed counts to
become invalid. Clipping the counts at startup allows us to rule out
log messages due to corruption from these prior Tor versions.
This was causing dirauths to emit flag weight validation warns if there
was a sufficiently large amount of badexit bandwidth to make a difference in
flag weight results.
It seems that some versions of clang that would prefer the
-Wswitch-enum compiler flag to warn about switch statements with
missing enum values, even if those switch statements have a
default.
Fixes bug 8598; bugfix on 0.2.4.10-alpha.
Found while investigating 8093, but probably not the cause of it,
since this bug would result in us sending too few SENDMEs, not in us
receiving SENDMEs unexpectedly.
Bugfix on the fix for 7889, which has appeared in 0.2.4.10-alpha, but
not yet in any released 0.2.3.x version.
It can never be NULL, since it's an array in bridge_line_t.
Introduced in 266f8cddd8. Found by coverity; this is CID 992691. Bug
not in any released Tor.
If we get a write error on a SOCKS connection, we can't send a
SOCKS reply, now can we?
This bug has been here since 36baf7219, where we added the "hey, I'm
closing an AP connection but I haven't finished the socks
handshake!" message. It's bug 8427.
Also, don't call the exit node 'reject *' unless our decision to pick
that node was based on a non-summarized version of that node's exit
policy.
rransom and arma came up with the ideas for this fix.
Fix for 7582; the summary-related part is a bugfix on 0.2.3.2-alpha.
When we're hibernating, the main reqason we can't bootstrap will
always be that we're hibernating: reporting anything else at severity
WARN is pointless.
Fixes part of 7302.
This bug affects hosts where time_t is unsigned, which AFAICT does
not include anything we currently support. (It _does_ include
OpenVMS, about a month of BSD4.2's history[1], and a lot of the 1970s.)
There are probably more bugs when time_t is unsigned. This one was
[1] http://mail-index.netbsd.org/tech-userlevel/1998/06/04/0000.html
This time, I'm checking whether our calculated offset matches our
real offset, in each case, as we go along. I don't think this is
the bug, but it can't hurt to check.
This should have been 2 bytes all along, since version numbers can
be 16 bits long. This isn't a live bug, since the call to
is_or_protocol_version_known in channel_tls_process_versions_cell
will reject any version number not in the range 1..4. Still, let's
fix this before we accidentally start supporting version 256.
Reported pseudonymously. Fixes bug 8062; bugfix on 0.2.0.10-alpha --
specifically, on commit 6fcda529, where during development I
increased the width of a version to 16 bits without changing the
type of link_proto.
Our ++ should have been += 2. This means that we'd accept version
numbers even when they started at an odd position.
This bug should be harmless in practice for so long as every version
number we allow begins with a 0 byte, but if we ever have a version
number starting with 1, 2, 3, or 4, there will be trouble here.
Fix for bug 8059, reported pseudonymously. Bugfix on 0.2.0.10-alpha
-- specifically, commit 6fcda529, where during development I
increased the width of a version to 16 bits without changing the
loop step.
I have no idea whether b0rken clients will DoS the network if the v2
authorities all turn this on or not. It's experimental. See #6783 for
a description of how to test it more or less safely, and please be
careful!
Now that circid_t is 4 bytes long, the default integer promotions will
leave it alone when sizeof(int) == 4, which will leave us formatting an
unsigned as an int. That's technically undefined behavior.
Fixes bug 8447 on bfffc1f0fc. Bug not
in any released Tor.
In a number of places, we decrement timestamp_dirty by
MaxCircuitDirtiness in order to mark a stream as "unusable for any
new connections.
This pattern sucks for a few reasons:
* It is nonobvious.
* It is error-prone: decrementing 0 can be a bad choice indeed.
* It really wants to have a function.
It can also introduce bugs if the system time jumps backwards, or if
MaxCircuitDirtiness is increased.
So in this patch, I add an unusable_for_new_conns flag to
origin_circuit_t, make it get checked everywhere it should (I looked
for things that tested timestamp_dirty), and add a new function to
frob it.
For now, the new function does still frob timestamp_dirty (after
checking for underflow and whatnot), in case I missed any cases that
should be checking unusable_for_new_conns.
Fixes bug 6174. We first used this pattern in 516ef41ac1,
which I think was in 0.0.2pre26 (but it could have been 0.0.2pre27).
Without this patch, there's no way to know what went wrong when we
fail to parse a torrc line entirely (that is, we can't turn it into
a K,V pair.) This patch introduces a new function that yields an
error message on failure, so we can at least tell the user what to
look for in their nonfunctional torrc.
(Actually, it's the same function as before with a new name:
parse_config_line_from_str is now a wrapper macro that the unit
tests use.)
Fixes bug 7950; fix on 0.2.0.16-alpha (58de695f90) which first
introduced the possibility of a torrc value not parsing correctly.
This patch moves the measured_bw field and the has_measured_bw field
into vote_routerstatus_t, since only votes have 'Measured=XX' set on
their weight line.
I also added a new bw_is_unmeasured flag to routerstatus_t to
represent the Unmeasured=1 flag on a w line. Previously, I was using
has_measured_bw for this, which was quite incorrect: has_measured_bw
means that the measured_bw field is set, and it's probably a mistake
to have it serve double duty as meaning that 'baandwidth' represents a
measured value.
While making this change,I also found a harmless but stupid bug in
dirserv_read_measured_bandwidths: It assumes that it's getting a
smartlist of routerstatus_t, when really it's getting a smartlist of
vote_routerstatus_t. C's struct layout rules mean that we could never
actually get an error because of that, but it's still quite incorrect.
I fixed that, and in the process needed to add two more sorting and
searching helpers.
Finally, I made the Unmeasured=1 flag get parsed. We don't use it for
anything yet, but someday we might.
This isn't complete yet -- the new 2286 unit test doesn't build.
Instead of capping whenever a router has fewer than 3 measurements,
we cap whenever a router has fewer than 3 measurements *AND* there
are at least 3 authorities publishing measured bandwidths.
We also generate bandwidth lines with a new "Unmeasured=1" flag,
meaning that we didn't have enough observations for a node to use
measured bandwidth values in the authority's input, whether we capped
it or not.
Stop marking every relay as having been down for one hour every
time we restart a directory authority. These artificial downtimes
were messing with our Stable and Guard flag calculations.
Fixes bug 8218 (introduced by the fix for 1035). Bugfix on 0.2.2.23-alpha.
Apparently something in the directory guard code made it possible
for the same node to get added as a guard over and over when there
were no actual running guard nodes.
Relays used to check every 10 to 60 seconds, as an accidental side effect
of calling directory_fetches_from_authorities() when considering doing
a directory fetch. The fix for bug 1992 removes that side effect. At the
same time, bridge relays never had the side effect, leading to confused
bridge operators who tried crazy tricks to get their bridges to notice
IP address changes (see ticket 1913).
The new behavior is to reinstate an every-60-seconds check for both
public relays and bridge relays, now that the side effect is gone.
For example, we were doing a resolve every time we think about doing a
directory fetch. Now we reuse the cached answer in some cases.
Fixes bugs 1992 (bugfix on 0.2.0.20-rc) and 2410 (bugfix on
0.1.2.2-alpha).
When we compute the estimated microseconds we need to handle our
pending onionskins, we could (in principle) overflow a uint32_t if
we ever had 4 million pending onionskins before we had any data
about how onionskins take. Nevertheless, let's compute it properly.
Fixes bug 8210; bugfix on 0.2.4.10. Found by coverity; this is CID
980651.
Coverity is worried that we're checking entry_conn in some cases,
but not in the case where we set entry_conn->pending_optimistic_data.
This commit should calm it down (CID 718623).
The refactoring in commit 471ab34032 wasn't complete enough: we
were checking the auth_len variable, but never actually setting it,
so it would never seem that authentication had been provided.
This commit also removes a bunch of unused variables from
rend_service_introduce, whose unusedness we hadn't noticed because
we were wiping them at the end of the function.
Fix for bug 8207; bugfix on 0.2.4.1-alpha.
It returns the method by which we decided our public IP address
(explicitly configured, resolved from explicit hostname, guessed from
interfaces, learned by gethostname).
Now we can provide more helpful log messages when a relay guesses its IP
address incorrectly (e.g. due to unexpected lines in /etc/hosts). Resolves
ticket 2267.
While we're at it, stop sending a stray "(null)" in some cases for the
server status "EXTERNAL_ADDRESS" controller event. Resolves bug 8200.
since router_parse_entry_from_string() already checks whether
!tor_inet_aton(router->address, &in)
(And no need to print address, since router_describe does that.)
- Document the key=value format.
- Constify equal_sign_pos.
- Pass some strings that are about to be logged to escape().
- Update documentation and fix some bugs in tor_escape_str_for_socks_arg().
- Use string_is_key_value() in parse_bridge_line().
- Parenthesize a forgotten #define
- Add some more comments.
- Add some more unit test cases.
This check isn't necessary (see comment on #7801), but it took at
least two smart people a little while to see why it wasn't necessary,
so let's have it in to make the code more readable.
We need a weak RNG in a couple of places where the strong RNG is
both needless and too slow. We had been using the weak RNG from our
platform's libc implementation, but that was problematic (because
many platforms have exceptionally horrible weak RNGs -- like, ones
that only return values between 0 and SHORT_MAX) and because we were
using it in a way that was wrong for LCG-based weak RNGs. (We were
counting on the low bits of the LCG output to be as random as the
high ones, which isn't true.)
This patch adds a separate type for a weak RNG, adds an LCG
implementation for it, and uses that exclusively where we had been
using the platform weak RNG.
If we're deciding on a node's bandwidth based on "Bandwidth="
declarations, clip it to "20" or to the maxunmeasuredbw parameter,
if it's voted on.
This adds a new consensus method.
This is "part A" of bug 2286
Authorities don't set is_possible_guard on node_t, so they were
never deciding that they could build enough paths. This is a quick
and dirty fix.
Bug not in any released version of Tor
These seem to have gotten conflicted out of existence while mike was
working on path bias stuff.
Thanks to sysrqb for collecting these in a handy patch.
Now we can specify to skip bridges that wouldn't be able to answer the
type of dir fetch we're launching.
It's still the responsibility of the rest of the code to prevent us from
launching a given dir fetch if we have no bridges that could handle it.
Now as we move into a future where most bridges can handle microdescs
we will generally find ourselves using them, rather than holding back
just because one of our bridges doesn't use them.
When we first implemented TLS, we assumed in conneciton_handle_write
that a TOR_TLS_WANT_WRITE from flush_buf_tls meant that nothing had
been written. But when we moved our buffers to a ring buffer
implementation back in 0.1.0.5-rc (!), we broke that invariant: it's
possible that some bytes have been written but nothing.
That's bad. It means that if we do a sequence of TLS writes that ends
with a WANTWRITE, we don't notice that we flushed any bytes, and we
don't (I think) decrement buckets.
Fixes bug 7708; bugfix on 0.1.0.5-rc
Also, deprecate the torrc options for the scaling values. It's unlikely anyone
but developers will ever tweak them, even if we provided a single ratio value.
This is meant to avoid conflict with the built-in log() function in
math.h. It resolves ticket 7599. First reported by dhill.
This was generated with the following perl script:
#!/usr/bin/perl -w -i -p
s/\blog\(LOG_(ERR|WARN|NOTICE|INFO|DEBUG)\s*,\s*/log_\L$1\(/g;
s/\blog\(/tor_log\(/g;
Instead of hardcoding the minimum fraction of possible paths to 0.6, we
take it from the user, and failing that from the consensus, and
failing that we fall back to 0.6.
Previously we did this based on the fraction of descriptors we
had. But really, we should be going based on what fraction of paths
we're able to build based on weighted bandwidth, since otherwise a
directory guard or two could make us behave quite oddly.
Implementation for feature 5956
If any circuits were opened during a scaling event, we were scaling attempts
and successes by different amounts. This leads to rounding error.
The fix is to record how many circuits are in a state that hasn't been fully
counted yet, and subtract that before scaling, and add it back afterwords.
Since they use RELAY_EARLY (which can be seen by all hops on the path),
it's not safe to say they actually count as a successful use.
There are also problems with trying to allow them to finish extending due to
the circuit purpose state machine logic. It is way less complicated (and
possibly more semantically coherent) to simply wait until we actually try to
do something with them before claiming we 'used' them.
Also, we shouldn't call timed out circuits 'used' either, for semantic
consistency.
An adversary could let the first stream request succeed (ie the resolve), but
then tag and timeout the remainder (via cell dropping), forcing them on new
circuits.
Rolling back the state will cause us to probe such circuits, which should lead
to probe failures in the event of such tagging due to either unrecognized
cells coming in while we wait for the probe, or the cipher state getting out
of sync in the case of dropped cells.
Path use bias measures how often we can actually succeed using the circuits we
actually try to use. It is a subset of path bias accounting, but it is
computed as a separate statistic because the rate of client circuit use may
vary depending on use case.
This is a minimal refactoring to expose the weighted bandwidth
calculations for each node so I can use them to see what fraction of
nodes, weighted by bandwidth, we have descriptors for.
This is ticket 7706, reported by "bugcatcher." The rationale here
is that if somebody says 'ExcludeNodes {tv}', then they probably
don't just want to block definitely Tuvaluan nodes: they also want
to block nodes that have unknown country, since for all they know
such nodes are also in Tuvalu.
This behavior is controlled by a new GeoIPExcludeUnknown autobool
option. With the default (auto) setting, we exclude ?? and A1 if
any country is excluded. If the option is 1, we add ?? and A1
unconditionally; if the option is 0, we never add them.
(Right now our geoip file doesn't actually seem to include A1: I'm
including it here in case it comes back.)
This feature only takes effect if you have a GeoIP file. Otherwise
you'd be excluding every node.
The implementation is pretty straightforward: parse_extended_hostname() is
modified to drop any leading components from an address like
'foo.aaaaaaaaaaaaaaaa.onion'.
This is an automatically generated commit, from the following perl script,
run with the options "-w -i -p".
s/smartlist_string_num_isin/smartlist_contains_int_as_string/g;
s/smartlist_string_isin((?:_case)?)/smartlist_contains_string$1/g;
s/smartlist_digest_isin/smartlist_contains_digest/g;
s/smartlist_isin/smartlist_contains/g;
s/digestset_isin/digestset_contains/g;
In 6fbdf635 we added a couple of statements like:
if (test) {
...
};
The extraneous semicolons there get flagged as worrisome empty
statements by the cparser library, so let's fix them.
Patch by Christian Grothoff; fixes bug 7115.
Otherwise, it's possible to create streams or circuits with these
bogus IDs, leading to orphaned circuits or streams, or to ones that
can cause bandwidth DOS problems.
Fixes bug 7889; bugfix on all released Tors.
In general, if we tried to use a circ for a stream, but then decided to place
that stream on a different circuit, we need to probe the original circuit
before deciding it was a "success".
We also need to do the same for cannibalized circuits that go unused.
This makes removing items from the middle of the queue into an O(1)
operation, which could prove important as we let onionqueues grow
longer.
Doing this actually makes the code slightly smaller, too.
The right way to set "MaxOnionsPending" was to adjust it until the
processing delay was appropriate. So instead, let's measure how long
it takes to process onionskins (sampling them once we have a big
number), and then limit the queue based on its expected time to
finish.
This change is extra-necessary for ntor, since there is no longer a
reasonable way to set MaxOnionsPending without knowing what mix of
onionskins you'll get.
This patch also reserves 1/3 of the onionskin spots for ntor
handshakes, on the theory that TAP handshakes shouldn't be allowed to
starve their speedier cousins. We can change this later if need be.
Resolves 7291.
The unit of work sent to a cpuworker is now a create_cell_t; its
response is now a created_cell_t. Several of the things that call or
get called by this chain of logic now take create_cell_t or
created_cell_t too.
Since all cpuworkers are forked or spawned by Tor, they don't need a
stable wire protocol, so we can just send structs. This saves us some
insanity, and helps p
As elsewhere, it makes sense when adding or extending a cell type to
actually make the code to parse it into a separate tested function.
This commit doesn't actually make anything use these new functions;
that's for a later commit.
The handshake_digest field was never meaningfully a digest *of* the
handshake, but rather is a digest *from* the handshake that we exapted
to prevent replays of ESTABLISH_INTRO cells. The ntor handshake will
generate it as more key material rather than taking it from any part
of the circuit handshake reply..
I'm going to want a generic "onionskin" type and set of wrappers, and
for that, it will be helpful to isolate the different circuit creation
handshakes. Now the original handshake is in onion_tap.[ch], the
CREATE_FAST handshake is in onion_fast.[ch], and onion.[ch] now
handles the onion queue.
This commit does nothing but move code and adjust header files.
Here we try to handle curve25519 onion keys from generating them,
loading and storing them, publishing them in our descriptors, putting
them in microdescriptors, and so on.
This commit is untested and probably buggy like whoa
This patch moves curve25519_keypair_t from src/or/onion_ntor.h to
src/common/crypto_curve25519.h, and adds new functions to generate,
load, and store keypairs.
The ntor handshake--described in proposal 216 and in a paper by
Goldberg, Stebila, and Ustaoglu--gets us much better performance than
our current approach.
Our old warn_nonlocal_client_ports() would give a bogus warning for
every nonlocal port every time it parsed any ports at all. So if it
parsed a nonlocal socksport, it would complain that it had a nonlocal
socksport...and then turn around and complain about the nonlocal
socksport again, calling it a nonlocal transport or nonlocal dnsport,
if it had any of those.
Fixes bug 7836; bugfix on 0.2.3.3-alpha.
mr-4 reports on #7799 that he was seeing it several times per second,
which suggests that things had gone very wrong.
This isn't a real fix, but it should make Tor usable till we can
figure out the real issue.
This implements the server-side of proposal 198 by detecting when
clients lack the magic list of ciphersuites that indicates that
they're lying faking some ciphers they don't really have. When
clients lack this list, we can choose any cipher that we'd actually
like. The newly allowed ciphersuites are, currently, "All ECDHE-RSA
ciphers that openssl supports, except for ECDHE-RSA-RC4".
The code to detect the cipher list relies on on (ab)use of
SSL_set_session_secret_cb.
It's important not to call choose_array_element_by_weight and then
pass its return value unchecked to smartlist_get : it is allowed to
return -1.
Fixes bug 7756; bugfix on 4e3d07a6 (not in any released Tor)
This is good enough to give P_success >= 999,999,999/1,000,000,000 so
long as the address space is less than 97.95 full. It'd be ridiculous
for that to happen for IPv6, and usome reasonable assumptions, it
would also be pretty silly for IPv4.
With an IPv6 virtual address map, we can basically hand out a new
IPv6 address for _every_ address we connect to. That'll be cool, and
will let us maybe get around prop205 issues.
This uses some fancy logic to try to make the code paths in the ipv4
and the ipv6 case as close as possible, and moves to randomly
generated addresses so we don't need to maintain those stupid counters
that will collide if Tor restarts but apps don't.
Also has some XXXX items to fix to make this useful. More design
needed.
This function gives us a single place to set reasonable default flags
for port_cfg_t entries, to avoid bugs like the one where we weren't
setting ipv4_traffic_ok to 1 on SocksPorts initialized in an older
way.
(This is part 2 of making DNS cache use enabled/disabled on a
per-client port basis. This implements the CacheIPv[46]DNS options,
but not the UseCachedIPv[46] ones.)
(This is part 1 of making DNS cache use enabled/disabled on a
per-client port basis. These options are shuffled around correctly,
but don't do anything yet.)
We want to be saying fast_mem{cmp,eq,neq} when we're doing a
comparison that's allowed to exit early, or tor_mem{cmp,eq,neq} when
we need a data-invariant timing. Direct use of memcmp tends to imply
that we haven't thought about the issue.
This has several advantages, including more resilience to ambient failure.
I still need to rename all the first_hop vars tho.. Saving that for a separate
commit.
Turns out there's more than one way to block a tagged circuit.
This seems to successfully handle all of the normal exit circuits. Hidden
services need additional tweaks, still.
This replaces the old FallbackConsensus notion, and should provide a
way -- assuming we pick reasonable nodes! -- to give clients
suggestions of placs to go to get their first consensus.
Now creating a dir_server_t and adding it are separate functions, and
there are frontend functions for adding a trusted dirserver and a
fallback dirserver.
We use trusted_dir_server_t for two pieces of functionality: a list of
all directory authorities, and a list of initial places to look for
a directory. With this patch we start to separate those two roles.
There is as of now no actual way to be a fallback directory without being
an authority.
