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.
Fixes bug 13295; bugfix on 0.2.5.3-alpha.
The alternative here is to call crypto_global_init() from tor-resolve,
but let's avoid linking openssl into tor-resolve for as long as we
can.
When a spawned process forks, fails, then exits very quickly, (this
typically occurs when exec fails), there is a race condition between the
SIGCHLD handler updating the process_handle's fields, and checking the
process status in those fields. The update can occur before or after the
spawn tests check the process status.
We check whether the process is running or not running (rather than just
checking if it is running) to avoid this issue.
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.
This helps us avoid undefined behavior. It's based on a patch from teor,
except that I wrote a perl script to regenerate the patch:
#!/usr/bin/perl -p -w -i
BEGIN { %vartypes = (); }
if (/^[{}]/) {
%vartypes = ();
}
if (/^ *crypto_int(\d+) +([a-zA-Z_][_a-zA-Z0-9]*)/) {
$vartypes{$2} = $1;
} elsif (/^ *(?:signed +)char +([a-zA-Z_][_a-zA-Z0-9]*)/) {
$vartypes{$1} = '8';
}
# This fixes at most one shift per line. But that's all the code does.
if (/([a-zA-Z_][a-zA-Z_0-9]*) *<< *(\d+)/) {
$v = $1;
if (exists $vartypes{$v}) {
s/$v *<< *(\d+)/SHL$vartypes{$v}($v,$1)/;
}
}
# remove extra parenthesis
s/\(SHL64\((.*)\)\)/SHL64\($1\)/;
s/\(SHL32\((.*)\)\)/SHL32\($1\)/;
s/\(SHL8\((.*)\)\)/SHL8\($1\)/;
There are some loops of the form
for (i=1;i<1;++i) ...
And of course, if the loop index is initialized to 1, it will never
be less than 1, and the loop body will never be executed. This
upsets coverity.
Patch fixes CID 1221543 and 1221542
This bug shouldn't be reachable so long as secret_to_key_len and
secret_to_key_make_specifier stay in sync, but we might screw up
someday.
Found by coverity; this is CID 1241500
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 ]
When size_t is the most memory you can have, make sure that things
referring to real parts of memory are size_t, not uint64_t or off_t.
But not on any released Tor.
Also, use it to generate test vectors, and add those test vectors
to test_crypto.c
This is based on ed25519.py from the ed25519 webpage; the kludgy hacks
are my own.
This implementation allows somebody to add a blinding factor to a
secret key, and a corresponding blinding factor to the public key.
Robert Ransom came up with this idea, I believe. Nick Hopper proved a
scheme like this secure. The bugs are my own.
For proposal 228, we need to cross-certify our identity with our
curve25519 key, so that we can prove at descriptor-generation time
that we own that key. But how can we sign something with a key that
is only for doing Diffie-Hellman? By converting it to the
corresponding ed25519 point.
See the ALL-CAPS warning in the documentation. According to djb
(IIUC), it is safe to use these keys in the ways that ntor and prop228
are using them, but it might not be safe if we start providing crazy
oracle access.
(Unit tests included. What kind of a monster do you take me for?)
This is another case where DJB likes sticking the whole signature
prepended to the message, and I don't think that's the hottest idea.
The unit tests still pass.
This reduces the likelihood that I have made any exploitable errors
in the encoding/decoding.
This commit also imports the trunnel runtime source into Tor.
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.
Since address.c is the first file to get compiled, let's have it use
a little judicious c99 in order to catch broken compilers that
somehow make it past our autoconf tests.
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.
These wrappers went into place when the default type for our unit
test functions changed from "void fn(void)" to "void fn(void *arg)".
To generate this patch, I did the same hokey-pokey as before with
replacing all operators used as macro arguments, then I ran a
coccinelle script, then I ran perl script to fix up everything that
used legacy_test_helper, then I manually removed the
legacy_test_helper functions, then I ran a final perl script to put
the operators back how they were.
==============================
#!/usr/bin/perl -w -i -p
s/==,/_X_EQ_,/g;
s/!=,/_X_NE_,/g;
s/<,/_X_LT_,/g;
s/>,/_X_GT_,/g;
s/>=,/_X_GEQ_,/g;
s/<=,/_X_LEQ_,/g;
--------------------
@@
identifier func =~ "test_.*$";
statement S, S2;
@@
static void func (
-void
+void *arg
)
{
... when != S2
+(void) arg;
S
...
}
--------------------
#!/usr/bin/perl -w -i -p
s/, *legacy_test_helper, *([^,]+), *\&legacy_setup, *([^\}]+) *}/, $2, $1, NULL, NULL }/g;
--------------------
#!/usr/bin/perl -w -i -p
s/_X_NEQ_/!=/g;
s/_X_NE_/!=/g;
s/_X_EQ_/==/g;
s/_X_GT_/>/g;
s/_X_LT_/</g;
s/_X_GEQ_/>=/g;
s/_X_LEQ_/<=/g;
--------------------
"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.
"The NULL pointer warnings on the return value of
tor_addr_to_in6_addr32() are incorrect. But clang can't work this
out itself due to limited analysis depth. To teach the analyser that
the return value is safe to dereference, I applied tor_assert to the
return value."
Patch from teor. 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 fixes bug 13102 (not on any released Tor) where using the
standard SSIZE_MAX name broke mingw64, and we didn't realize.
I did this with
perl -i -pe 's/SIZE_T_MAX/SIZE_MAX/' src/*/*.[ch] src/*/*/*.[ch]
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.
Otherwise, when we're out of input *and* finalizing, we might report
TOR_ZLIB_OK erroneously and not finalize the buffer.
(I don't believe this can happen in practice, with our code today:
write_to_buf_zlib ensures that we are never trying to write into a
completely empty buffer, and zlib says "Z_OK" if you give it even
one byte to write into.)
Fixes bug 11824; bugfix on 0.1.1.23 (06e09cdd47).
torrc.minimal is now the one that should change as infrequently as
possible. To schedule an change to go into it eventually, make your
change to torrc.minimal.in-sample.
torrc.sample is now the volatile one: we can change it to our hearts'
content.
Closes ticket #11144
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".
This commit attempts to satisfy nickm's comment on check_private_dir() permissions:
https://trac.torproject.org/projects/tor/ticket/11291#comment:12
"""check_private_dir() ensures that the directory has bits 0700 if CPD_CHECK_MODE_ONLY is not set. Shouldn't it also ensure that the directory has bits 0050 if CPD_CHECK_MODE_ONLY is not set, and CPD_GROUP_READ is set?"""
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.
Any error when acquiring the CryptoAPI context should get treated as
bad. Also, this one can't happen for the arguments we're giving.
Fixes bug 10816; bugfix on 0.0.2pre26.
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 way, we don't get locking failures when we hit an assertion in
the unit tests. Also, we might find out about unit test bugs from
folks who can't do gdb.
We might use libsodium or ed25519-donna later on, but for now, let's
see whether this is fast enough. We should use it in all cases when
performance doesn't matter.
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]
Previously, we had documented it to return -1 or 0, when in fact
lseek returns -1 or the new position in the file.
This is harmless, since we were only checking for negative values
when we used tor_fd_seekend.
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.)
This function is supposed to construct a list of all the ciphers in
the "v2 link protocol cipher list" that are supported by Tor's
openssl. It does this by invoking ssl23_get_cipher_by_char on each
two-byte ciphersuite ID to see which ones give a match. But when
ssl23_get_cipher_by_char cannot find a match for a two-byte SSL3/TLS
ciphersuite ID, it checks to see whether it has a match for a
three-byte SSL2 ciphersuite ID. This was causing a read off the end
of the 'cipherid' array.
This was probably harmless in practice, but we shouldn't be having
any uninitialized reads.
(Using ssl23_get_cipher_by_char in this way is a kludge, but then
again the entire existence of the v2 link protocol is kind of a
kludge. Once Tor 0.2.2 clients are all gone, we can drop this code
entirely.)
Found by starlight. Fix on 0.2.4.8-alpha. Fixes bug 12227.
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.
Found because LibreSSL has OPENSSL_NO_COMP always-on, but this
conflicts with the way that _we_ turn off compression. Patch from
dhill, who attributes it to "OpenBSD". Fixes bug 12602; bugfix on
0.2.1.1-alpha, which introduced this turn-compression-off code.
The extra \ is harmless, but mildly unpleasant.
Fixes 12392; bugfix on 0.2.2.25-alpha where we started using
GetTempDir(). Based on a patch by Gisle Vanem.
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.
On a non-blocking pipe fgets sets EAGAIN when it encounters partial lines. No
error is set on full lines or EOF. EOF is reached when the writing end of the
pipe is closed. Partial lines and full lines are both returned by fgets, EOF
results in NULL.
Mention of this behaviour can be found in #1903 and #2045.
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.
The fix for bug 8746 added a hashtable instance that never actually
invoked HT_FIND. This caused a warning, since we didn't mark HT_FIND
as okay-not-to-use.
Try killing a running process; try noticing that a process has
exited without checking its output; verify that waitpid_cb (when
present) is set to NULL when you would expect it to be.
