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.
This fixes bug 10402, where the rdrand engine would use the rdrand
instruction, not as an additional entropy source, but as a replacement
for the entire userspace PRNG. That's obviously stupid: even if you
don't think that RDRAND is a likely security risk, the right response
to an alleged new alleged entropy source is never to throw away all
previously used entropy sources.
Thanks to coderman and rl1987 for diagnosing and tracking this down.
Incidentally, this business here where I make crypto_rand mockable:
this is exactly the kind of thing that would make me never want to
include test-support stuff in production builds.
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.
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.
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.
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;
Previously, we only used the strong OS entropy source as part of
seeding OpenSSL's RNG. But with curve25519, we'll have occasion to
want to generate some keys using extremely-good entopy, as well as the
means to do so. So let's!
This patch refactors the OS-entropy wrapper into its own
crypto_strongest_rand() function, and makes our new
curve25519_secret_key_generate function try it as appropriate.
This is a customizable extract-and-expand HMAC-KDF for deriving keys.
It derives from RFC5869, which derives its rationale from Krawczyk,
H., "Cryptographic Extraction and Key Derivation: The HKDF Scheme",
Proceedings of CRYPTO 2010, 2010, <http://eprint.iacr.org/2010/264>.
I'm also renaming the existing KDF, now that Tor has two of them.
This is the key derivation scheme specified in ntor.
There are also unit tests.
Apparently some compilers like to eliminate memset() operations on
data that's about to go out-of-scope. I've gone with the safest
possible replacement, which might be a bit slow. I don't think this
is critical path in any way that will affect performance, but if it
is, we can work on that in 0.2.4.
Fixes bug 7352.
Now that crypto_pk_cmp_keys might return the result of tor_memcmp, there
is no guarantee that it will only return -1, 0, or 1. (It currently does
only return -1, 0, or 1, but that's a lucky accident due to details of the
current implementation of tor_memcmp and the particular input given to it.)
Fortunately, none of crypto_pk_cmp_keys's callers rely on this behaviour,
so changing its documentation is sufficient.
097 hasn't seen a new version since 2007; we can drop support too.
This lets us remove our built-in sha256 implementation, and some
checks for old bugs.
We already do this for libevent; let's do it for openssl too.
For now, I'm making it always a warn, since this has caused some
problems in the past. Later, we can see about making it less severe.
Also, try to resolve some doxygen issues. First, define a magic
"This is doxygen!" macro so that we take the correct branch in
various #if/#else/#endifs in order to get the right documentation.
Second, add in a few grouping @{ and @} entries in order to get some
variables and fields to get grouped together.
This tells the windows headers to give us definitions that didn't
exist before XP -- like the ones that we need for IPv6 support.
See bug #5861. We didn't run into this issue with mingw, since
mingw doesn't respect _WIN32_WINNT as well as it should for some of
its definitions.
Fixes coverity CID 508: coverity scan doesn't like checking a
variable for non-NULL after it has been definitely dereferenced.
This should take us back down to zero coverity issues.
Calling crypto_cipher_free(NULL) is always safe, since (by
convention) all of our xyz_free() functions treat xyz_free(NULL) as
a no-op.
Flagged by coverity scan; fixes CID 508 and 509.
This commit is completely mechanical; I used this perl script to make it:
#!/usr/bin/perl -w -i.bak -p
if (/^\s*\#/) {
s/MS_WINDOWS/_WIN32/g;
s/\bWIN32\b/_WIN32/g;
}
To solve bug 4779, we want to avoid OpenSSL 1.0.0's counter mode.
But Fedora (and maybe others) lie about the actual OpenSSL version,
so we can't trust the header to tell us if it's safe.
Instead, let's do a run-time test to see whether it's safe, and if
not, use our built-in version.
fermenthor contributed a pretty essential fixup to this patch. Thanks!
MAX_DNS_LABEL_SIZE was only defined for old versions of openssl, which
broke the build. Spotted by xiando. Fixes bug 4413; not in any released
version.
The thing that's limited to 63 bytes is a "label", not a hostname.
Docment input constraints and behavior on bogus inputs.
Generally it's better to check for overflow-like conditions before
than after. In this case, it's not a true overflow, so we're okay,
but let's be consistent.
pedantic less->fewer in the documentation
Fixes bug 4413; bugfix on xxxx.
Hostname components cannot be larger than 63 characters.
This simple check makes certain randlen cannot overflow rand_bytes_len.
If a relay is dormant at startup, it will call init_keys before
crypto_set_tls_dh_prime. This is bad. Let's make it not so bad, because
someday it *will* happen again.
Instead of only writing the dynamic DH prime modulus to a file, write
the whole DH parameters set for forward compatibility. At the moment
we only accept '2' as the group generator.
The DH parameters gets stored in base64-ed DER format to the
'dynamic_dh_params' file.
George Kadianakis notes that if you give crypto_rand_int() a value
above INT_MAX, it can return a negative number, which is not what
the documentation would imply.
The simple solution is to assert that the input is in [1,INT_MAX+1].
If in the future we need a random-value function that can return
values up to UINT_MAX, we can add one.
Fixes bug 3306; bugfix on 0.2.2pre14.
When we added the check for key size, we required that the keys be
128 bytes. But RSA_size (which defers to BN_num_bytes) will return
128 for keys of length 1017..1024. This patch adds a new
crypto_pk_num_bits() that returns the actual number of significant
bits in the modulus, and uses that to enforce key sizes.
Also, credit the original bug3318 in the changes file.
Rename crypto_pk_check_key_public_exponent to crypto_pk_public_exponent_ok:
it's nice to name predicates s.t. you can tell how to interpret true
and false.
Conflicts in various places, mainly node-related. Resolved them in
favor of HEAD, with copying of tor_mem* operations from bug3122_memcmp_022.
src/common/Makefile.am
src/or/circuitlist.c
src/or/connection_edge.c
src/or/directory.c
src/or/microdesc.c
src/or/networkstatus.c
src/or/router.c
src/or/routerlist.c
src/test/test_util.c
Conflicts throughout. All resolved in favor of taking HEAD and
adding tor_mem* or fast_mem* ops as appropriate.
src/common/Makefile.am
src/or/circuitbuild.c
src/or/directory.c
src/or/dirserv.c
src/or/dirvote.c
src/or/networkstatus.c
src/or/rendclient.c
src/or/rendservice.c
src/or/router.c
src/or/routerlist.c
src/or/routerparse.c
src/or/test.c
Here I looked at the results of the automated conversion and cleaned
them up as follows:
If there was a tor_memcmp or tor_memeq that was in fact "safe"[*] I
changed it to a fast_memcmp or fast_memeq.
Otherwise if there was a tor_memcmp that could turn into a
tor_memneq or tor_memeq, I converted it.
This wants close attention.
[*] I'm erring on the side of caution here, and leaving some things
as tor_memcmp that could in my opinion use the data-dependent
fast_memcmp variant.
Our regular DH parameters that we use for circuit and rendezvous
crypto are unchanged. This is yet another small step on the path of
protocol fingerprinting resistance.
(Backport from 0.2.2's 5ed73e3807)
Our regular DH parameters that we use for circuit and rendezvous
crypto are unchanged. This is yet another small step on the path of
protocol fingerprinting resistance.
Our public key functions assumed that they were always writing into a
large enough buffer. In one case, they weren't.
(Incorporates fixes from sebastian)
It's all too easy in C to convert an unsigned value to a signed one,
which will (on all modern computers) give you a huge signed value. If
you have a size_t value of size greater than SSIZE_T_MAX, that is way
likelier to be an underflow than it is to be an actual request for
more than 2gb of memory in one go. (There's nothing in Tor that
should be trying to allocate >2gb chunks.)
In a2bb0bf we started using a separate client identity key. When we are
in "public server mode" (that means not a bridge) we will use the same
key. Reusing the key without doing the proper refcounting leads to a
segfault on cleanup during shutdown. Fix that.
Also introduce an assert that triggers if our refcount falls below 0.
That should never happen.
This should make us conflict less with system files named "log.h".
Yes, we shouldn't have been conflicting with those anyway, but some
people's compilers act very oddly.
The actual change was done with one "git mv", by editing
Makefile.am, and running
find . -name '*.[ch]' | xargs perl -i -pe 'if (/^#include.*\Wlog.h/) {s/log.h/torlog.h/; }'
This should never happen unless openssl is buggy or some of our
assumptions are deeply wrong, but one of those might have been the
cause of the not-yet-reproducible bug 1209. If it ever happens again,
let's get some info we can use.
Some *_free functions threw asserts when passed NULL. Now all of them
accept NULL as input and perform no action when called that way.
This gains us consistence for our free functions, and allows some
code simplifications where an explicit null check is no longer necessary.
In 5e4d53d535 we made it so that
crypto_cipher_set_key cannot fail. The call will now
always succeed, to returning a boolean for success/failure makes
no sense.
In C, the code "char x[10]; if (x) {...}" always takes the true branch of
the if statement. Coverity notices this now.
In some cases, we were testing arrays to make sure that an operation
we wanted to do would suceed. Those cases are now always-true.
In some cases, we were testing arrays to see if something was _set_.
Those caes are now tests for strlen(s), or tests for
!tor_mem_is_zero(d,len).
See task 1114. The most plausible explanation for someone sending us weak
DH keys is that they experiment with their Tor code or implement a new Tor
client. Usually, we don't care about such events, especially not on warn
level. If we really care about someone not following the Tor protocol, we
can set ProtocolWarnings to 1.
This patch introduces a new type called document_signature_t to represent the
signature of a consensus document. Now, each consensus document can have up
to one document signature per voter per digest algorithm. Also, each
detached-signatures document can have up to one signature per <voter,
algorithm, flavor>.
Apparently the Android developers dumped OpenSSL's support for hardware
acceleration in order to save some memory, so you can't build programs using
engines on Android.
[Patch revised by nickm]
This shouldn't be necessary, but apparently the Android cross-compiler
doesn't respect -I as well as it should. (-I is supposed to add to the
*front* of the search path. Android's gcc wrapper apparently likes to add to
the end. This is broken, but we need to work around it.)
(This would be everywhere running OpenSSL 0.9.7x and earlier, including
all current Macintosh users.)
The code is based on Tom St Denis's LibTomCrypt implementation,
modified to be way less general and use Tor's existing facilities. I
picked this one because it was pretty fast and pretty free, and
because Python uses it too.
crypto_global_init gets called. Also have it be crypto_global_init
that calls crypto_seed_rng, so we are not dependent on OpenSSL's
RAND_poll in these fiddly cases.
Should fix bug 907. Bugfix on 0.0.9pre6. Backport candidate.
svn:r18210
The subversion $Id$ fields made every commit force a rebuild of
whatever file got committed. They were not actually useful for
telling the version of Tor files in the wild.
svn:r17867
