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)