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.
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.
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.
The server cipher list is (thanks to #11513) chosen systematically to
put the best choices for Tor first. The client cipher list is chosen
to resemble a browser. So let's set SSL_OP_CIPHER_SERVER_PREFERENCE
to have the servers pick according to their own preference order.
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.
Back in 175b2678, we allowed servers to recognize clients who are
telling them the truth about their ciphersuites, and select the best
cipher from on that list. This implemented the server side of proposal
198.
In bugs 11492, 11498, and 11499, cypherpunks found a bunch of mistakes
and omissions and typos in the UNRESTRICTED_SERVER_CIPHER_LIST we had.
In #11513, I found a couple more.
Rather than try to hand-edit this list, I wrote a short python script
to generate our ciphersuite preferences from the openssl headers.
The new rules are:
* Require forward secrecy.
* Require RSA (since our servers only configure RSA keys)
* Require AES or 3DES. (This means, reject RC4, DES, SEED, CAMELLIA,
and NULL.)
* No export ciphersuites.
Then:
* Prefer AES to 3DES.
* If both suites have the same cipher, prefer ECDHE to DHE.
* If both suites have the same DHE group type, prefer GCM to CBC.
* If both suites have the same cipher mode, prefer SHA384 to SHA256
to SHA1.
* If both suites have the same digest, prefer AES256 to AES128.
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.
The previous commit from piet would have backed out some of proposal
198 and made servers built without the V2 handshake not use the
unrestricted cipher list from prop198.
Bug not in any released Tor.
According to the manpage, bridges use P256 for conformity and relays
use P224 for speed. But skruffy points out that we've gotten it
backwards in the code.
In this patch, we make the default P256 for everybody.
Fixes bug 9780; bugfix on 0.2.4.8-alpha.
To fix#6033, we disabled TLS 1.1 and 1.2. Eventually, OpenSSL fixed
the bug behind #6033.
I've considered alternate implementations that do more testing to see
if there's secretly an OpenSSL 1.0.1c or something that secretly has a
backport of the OpenSSL 1.0.1e fix, and decided against it on the
grounds of complexity.
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 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;
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.
We already use this classification for deciding whether (as a server)
to do a v2/v3 handshake, and we're about to start using it for
deciding whether we can use good ciphersuites too.
This is less easy than you might think; we can't just look at the
client ciphers list, since openssl doesn't remember client ciphers if
it doesn't know about them. So we have to keep a list of the "v2"
ciphers, with the ones we don't know about removed.
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.
OpenSSL 1.0.0 added an implementation of TLS session tickets, a
"feature" that let session resumption occur without server-side state
by giving clients an encrypted "ticket" that the client could present
later to get the session going again with the same keys as before.
OpenSSL was giving the keys to decrypt these tickets the lifetime of
the SSL contexts, which would have been terrible for PFS if we had
long-lived SSL contexts. Fortunately, we don't. Still, it's pretty
bad. We should also drop these, since our use of the extension stands
out with our non-use of session cacheing.
Found by nextgens. Bugfix on all versions of Tor when built with
openssl 1.0.0 or later. Fixes bug 7139.
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.
This is a feature removal: we no longer fake any ciphersuite other
than the not-really-standard SSL_RSA_FIPS_WITH_3DES_EDE_CBC_SHA
(0xfeff). This change will let servers rely on our actually
supporting what we claim to support, and thereby let Tor migrate to
better TLS ciphersuites.
As a drawback, Tor instances that use old openssl versions and
openssl builds with ciphers disabled will no longer give the
"firefox" cipher list.
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.
It appears that when OpenSSL negotiates a 1.1 or 1.2 connection, and it
decides to renegotiate, the client will send a record with version "1.0"
rather than with the current TLS version. This would cause the
connection to fail whenever both sides had OpenSSL 1.0.1, and the v2 Tor
handshake was in use.
As a workaround, disable TLS 1.1 and TLS 1.2. When a later version of
OpenSSL is released, we can make this conditional on running a fixed
version of OpenSSL.
Alternatively, we could disable TLS 1.1 and TLS 1.2 only on the client
side. But doing it this way for now means that we not only fix TLS with
patched clients; we also fix TLS when the server has this patch and the
client does not. That could be important to keep the network running
well.
Fixes bug 6033.
We've been only treating SW_SERVER_HELLO_A as meaning that an SSL
handshake was happening. But that's not right: if the initial
attempt to write a ServerHello fails, we would get a callback in
state SW_SERVER_HELLO_B instead.
(That's "instead" and not "in addition": any failed attempt to write
the hello will fail and cause the info callback not to get written.)
Fix for bug 4592; bugfix on 0.2.0.13-alpha.
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.
If the client uses a v2 cipherlist on the renegotiation handshake,
it looks as if they could fail to get a good cert chain from the
server, since they server would re-disable certificate chaining.
This patch makes it so the code that make the server side of the
first v2 handshake special can get called only once.
Fix for 4591; bugfix on 0.2.0.20-rc.
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;
}
- Rename tor_tls_got_server_hello() to tor_tls_got_client_hello().
- Replaced some aggressive asserts with LD_BUG logging.
They were the innocent "I believe I understand how these callbacks
work, and this assert proves it" type of callbacks, and not the "If
this statement is not true, computer is exploding." type of
callbacks.
- Added a changes file.
* Make tor_tls_context_new internal to tortls.c, and return the new
tor_tls_context_t from it.
* Add a public tor_tls_context_init wrapper function to replace it.
Conflicts:
src/or/main.c
src/or/router.c
SSL_read(), SSL_write() and SSL_do_handshake() can always progress the
SSL protocol instead of their normal operation, this means that we
must be checking for needless renegotiations after they return.
Introduce tor_tls_got_excess_renegotiations() which makes the
tls->server_handshake_count > 2
check for us, and use it in tor_tls_read() and tor_tls_write().
Cases that should not be handled:
* SSL_do_handshake() is only called by tor_tls_renegotiate() which is a
client-only function.
* The SSL_read() in tor_tls_shutdown() does not need to be handled,
since SSL_shutdown() will be called if SSL_read() returns an error.
Since we check for naughty renegotiations using
tor_tls_t.server_handshake_count we don't need that semi-broken
function (at least till there is a way to disable rfc5746
renegotiations too).
Switch 'server_handshake_count' from a uint8_t to 2 unsigned int bits.
Since we won't ever be doing more than 3 handshakes, we don't need the
extra space.
Toggle tor_tls_t.got_renegotiate based on the server_handshake_count.
Also assert that when we've done two handshakes as a server (the initial
SSL handshake, and the renegotiation handshake) we've just
renegotiated.
Finally, in tor_tls_read() return an error if we see more than 2
handshakes.
The renegotiation callback was called only when the first Application
Data arrived, instead of when the renegotiation took place.
This happened because SSL_read() returns -1 and sets the error to
SSL_ERROR_WANT_READ when a renegotiation happens instead of reading
data [0].
I also added a commented out aggressive assert that I won't enable yet
because I don't feel I understand SSL_ERROR_WANT_READ enough.
[0]: Look at documentation of SSL_read(), SSL_get_error() and
SSL_CTX_set_mode() (SSL_MODE_AUTO_RETRY section).
Introduce tor_tls_state_changed_callback(), which handles every SSL
state change.
The new function tor_tls_got_server_hello() is called every time we
send a ServerHello during a v2 handshake, and plays the role of the
previous tor_tls_server_info_callback() function.
Right now we can take the digests only of an RSA key, and only expect to
take the digests of an RSA key. The old tor_cert_get_id_digests() would
return a good set of digests for an RSA key, and an all-zero one for a
non-RSA key. This behavior is too error-prone: it carries the risk that
we will someday check two non-RSA keys for equality and conclude that
they must be equal because they both have the same (zero) "digest".
Instead, let's have tor_cert_get_id_digests() return NULL for keys we
can't handle, and make its callers explicitly test for NULL.
