Avoid vulnerability CVE-2011-4576 : Disable SSL3?

According to http://openssl.org/news/secadv_20120104.txt , there is an information leakage vulnerability when making SSL3 connections with SSL_MODE_RELEASE_BUFFERS, where uninitialized memory can get leaked, up to 15 bytes at a time. The bug is fixed in openssl 1.0.0f and 0.9.8s.

(I'm told this was found by wanoskarnet, reported by asn to agl, who got it fixed in openssl.)

On Tor's side, the easiest fix is to just require TLS1 only, and not support SSL3 any more. But that could create problems with our cipher lists.

added component::core tor/tor priority::very high resolution::fixed status::closed tor-client type::defect labels

Of course, we should ship packages with updated openssls. I think that is a good fix no matter what Tor does.

(I'm told this was found by wanoskarnet, reported by asn to agl, who got it fixed in openssl.)

Ah, I misunderstood CVE-2011-4619 was the misattributed one.

Have a look at branch "bug4822_021" in my public repo.

I considered an approach where we would allow any handshake, but disallow any SSL3 ciphers so that the handshake would fail if the ssl3 handshake were actually tried. Problem was, openssl allows tls1 ciphers with the ssl3 handshake, so that wouldn't have worked. (Thanks to asn for testing that.)

This needs review and a changes file.

Trac:
Status: new to needs_review

nickm's bug4822_021 is a compile-time version check; we need to do a runtime version check.

This needs to be tested to determine whether a server which uses TLSv1_method can receive a connection from a client which uses SSLv23_method. (When I read the OpenSSL documentation for those, it sort of hinted that that would not work.)

Replying to rransom:

This needs to be tested to determine whether a server which uses TLSv1_method can receive a connection from a client which uses SSLv23_method. (When I read the OpenSSL documentation for those, it sort of hinted that that would not work.)

BZZZT! Wrong. The documentation (SSL_CTX_new(3ssl)) says quite explicitly that that will not work:

       TLSv1_method(void), TLSv1_server_method(void),
       TLSv1_client_method(void)
           A TLS/SSL connection established with these methods will only
           understand the TLSv1 protocol. A client will send out TLSv1 client
           hello messages and will indicate that it only understands TLSv1. A
           server will only understand TLSv1 client hello messages. This
           especially means, that it will not understand SSLv2 client hello
           messages which are widely used for compatibility reasons, see
           SSLv23_*_method(). It will also not understand SSLv3 client hello
           messages.

       SSLv23_method(void), SSLv23_server_method(void),
       SSLv23_client_method(void)
           A TLS/SSL connection established with these methods will understand
           the SSLv2, SSLv3, and TLSv1 protocol. A client will send out SSLv2
           client hello messages and will indicate that it also understands
           SSLv3 and TLSv1. A server will understand SSLv2, SSLv3, and TLSv1
           client hello messages. This is the best choice when compatibility
           is a concern.

Can we afford to throw away all relays run on RHEL/CentOS and other crap OSes whose OpenSSL packages lie about their versions?

Replying to rransom:

Replying to rransom:

This needs to be tested to determine whether a server which uses TLSv1_method can receive a connection from a client which uses SSLv23_method. (When I read the OpenSSL documentation for those, it sort of hinted that that would not work.)

BZZZT! Wrong. The documentation (SSL_CTX_new(3ssl)) says quite explicitly that that will not work:

Ugh. So that patch would break compatibility with existing tors. Yuck.

We could have clients use TLSv1_method, at least. That would be something. Servers would probably need some disgusting hackery of the info_cb variety.

Any other ideas?

Can we afford to throw away all relays run on RHEL/CentOS and other crap OSes whose OpenSSL packages lie about their versions?

Does anybody claim to have a later openssl version than they really have? I thought they all were claiming too early.or about right. Any documentation there?

(Using flyspray from phone is pretty hard, ow)

Maybe SSL_OP_NO_SSLv3? It seems to exist; haven't tested when it appeared.

Okay, please look at branch bug4822_021_v2 in my public repository. It takes the SSL_OP_NO_SSLv3 approach.

Replying to nickm_mobile:

Maybe SSL_OP_NO_SSLv3? It seems to exist; haven't tested when it appeared.

This appears to be sufficient to keep us from ever calling ssl3_enc (the function in OpenSSL which contains The Bug), because it ensures that we will never use an SSL 3 ‘method’.

Replying to nickm:

Okay, please look at branch bug4822_021_v2 in my public repository. It takes the SSL_OP_NO_SSLv3 approach.

The EVERYONE_HAS_AES change probably belongs in a separate commit, although I don't think leaving it in the same commit as the disable-SSL3 change will ever cause any trouble.

I still think we should use a runtime version check, not a compile-time version check -- people will build Tor against an old OpenSSL shared library and run it with a new one.

Other than that, looks good.

Replying to nickm_mobile:

We could have clients use TLSv1_method, at least. That would be something. Servers would probably need some disgusting hackery of the info_cb variety.

This would make Tor clients' ClientHello messages distinguishable from most other applications'. I don't think we're going to want to do that until SSL 2 and 3 are really dead, and I'm not convinced that that will ever happen.

Just pushed an updated version with a changes file.

Replying to rransom:

Replying to nickm:

Okay, please look at branch bug4822_021_v2 in my public repository. It takes the SSL_OP_NO_SSLv3 approach.

The EVERYONE_HAS_AES change probably belongs in a separate commit, although I don't think leaving it in the same commit as the disable-SSL3 change will ever cause any trouble.

Right; I'm inclined to leave it.

I still think we should use a runtime version check, not a compile-time version check -- people will build Tor against an old OpenSSL shared library and run it with a new one.

Hm. That case is actually safe; the problem would be if they built with a newer one and ran with an older one. Does that seem likely? And shouldn't a decent library system prevent this?

Replying to nickm:

Replying to rransom:

Replying to nickm:

Okay, please look at branch bug4822_021_v2 in my public repository. It takes the SSL_OP_NO_SSLv3 approach.

I still think we should use a runtime version check, not a compile-time version check -- people will build Tor against an old OpenSSL shared library and run it with a new one.

Hm. That case is actually safe; the problem would be if they built with a newer one and ran with an older one. Does that seem likely? And shouldn't a decent library system prevent this?

The dangerous case could happen with packages for one Linux distribution used on a different distribution (e.g. packages built on and for Ubuntu used on Mint, before Mint updates its OpenSSL packages). But if there is no reason to try to enable SSL 3 whenever it is safe to do so, we shouldn't make this change depend on OpenSSL's version at all.

Replying to rransom:

The dangerous case could happen with packages for one Linux distribution used on a different distribution (e.g. packages built on and for Ubuntu used on Mint, before Mint updates its OpenSSL packages).

Okay. Then I'd say, "do this whenever the runtime version looks bad or the compile-time version looks bad."

But if there is no reason to try to enable SSL 3 whenever it is safe to do so, we shouldn't make this change depend on OpenSSL's version at all.

I think we think it might help for profiling resistance. I don't want to make extra changes to our default SSL profile back to 0.2.1 and 0.2.2 as part of this ticket without significant further analysis. This is a "let's make sure that the SSL vulnerability doesn't bite our users" ticket, not a "and while we're here, let's throw out parts of our SSL profile that we think we can do without" ticket.

Replying to nickm:

Replying to rransom:

The dangerous case could happen with packages for one Linux distribution used on a different distribution (e.g. packages built on and for Ubuntu used on Mint, before Mint updates its OpenSSL packages).

Okay. Then I'd say, "do this whenever the runtime version looks bad or the compile-time version looks bad."

See my bug4822_021_v2 branch for fixups.

Replying to rransom:

Replying to nickm:

Replying to rransom:

The dangerous case could happen with packages for one Linux distribution used on a different distribution (e.g. packages built on and for Ubuntu used on Mint, before Mint updates its OpenSSL packages).

Okay. Then I'd say, "do this whenever the runtime version looks bad or the compile-time version looks bad."

See my bug4822_021_v2 branch for fixups.

nickm approves these fixups. (Currently, that branch points to commit e0795667f924dec650bc4f8c1be93787776c2408.)

Should we emit a log message when we disable SSLv3? If we don't, there will be no easy way to determine whether a particular Tor instance supports SSLv3.

Sounds like a reasonable thing to do.

Replying to rransom:

Should we emit a log message when we disable SSLv3?

I've pushed a commit to log at info level when disabling SSLv3 to my bug4822_021_v2 branch; the previous (nickm-approved) commits are on my bug4822_021_v2-nickm-approved branch.

For logging version numbers, prefer OPENSSL_VERSION_TEXT and SSLeay_version(SSLEAY_VERSION), I think.

And a better message would be IMO "Disabling SSLv3 because this OpenSSL version might otherwise be vulnerable to CVE-foo." In other words, make it clear that this is a problem that stems from the openssl version, and that disabling SSLv3 will solve the problem if it exists.

This could be at NOTICE, I think. But I'm not sure it has to be. Others can decide.

Replying to nickm:

For logging version numbers, prefer OPENSSL_VERSION_TEXT

OK.

and SSLeay_version(SSLEAY_VERSION), I think.

No.

char *SSLeay_version(t)
int t;
	{
	if (t == SSLEAY_VERSION)
		return("SSLeay 0.9.1a 06-Jul-1998");

That is impressively bogus.

And a better message would be IMO "Disabling SSLv3 because this OpenSSL version might otherwise be vulnerable to CVE-foo." In other words, make it clear that this is a problem that stems from the openssl version, and that disabling SSLv3 will solve the problem if it exists.

OK.

This could be at NOTICE, I think. But I'm not sure it has to be. Others can decide.

Currently we emit the log message whenever we disable SSLv3 for some TLS context, which we do at least once on every SIGHUP. Do you still think we should emit this at notice level? If so, should we try to emit it less often?

Fixups pushed to improve the log message text and include OPENSSL_VERSION_TEXT.

Replying to rransom:

Replying to rransom:

This needs to be tested to determine whether a server which uses TLSv1_method can receive a connection from a client which uses SSLv23_method. (When I read the OpenSSL documentation for those, it sort of hinted that that would not work.)

BZZZT! Wrong. The documentation (SSL_CTX_new(3ssl)) says quite explicitly that that will not work:

That chunk of documentation is out of date for clients using OpenSSL 1.0.0 or later. From the CHANGES file:

 Changes between 0.9.8n and 1.0.0  [29 Mar 2010]

  *) If no SSLv2 ciphers are used don't use an SSLv2 compatible client hello:
     this allows the use of compression and extensions. Change default cipher
     string to remove SSLv2 ciphersuites. This effectively avoids ancient SSLv2
     by default unless an application cipher string requests it.
     [Steve Henson]

But I expect that clients using SSLv23_method still won't be able to connect to servers using TLSv1_method.

Replying to rransom:

Replying to nickm:

For logging version numbers, prefer OPENSSL_VERSION_TEXT

OK.

and SSLeay_version(SSLEAY_VERSION), I think.

No.

char *SSLeay_version(t)
int t;
	{
	if (t == SSLEAY_VERSION)
		return("SSLeay 0.9.1a 06-Jul-1998");
}}}

That is impressively bogus.

Weird; what openssl version are you looking at? Mine have {{{ crypto/cversion.c: if (t == SSLEAY_VERSION) crypto/cversion.c- return OPENSSL_VERSION_TEXT;

Replying to nickm:

Replying to rransom:

Replying to nickm:

For logging version numbers, prefer OPENSSL_VERSION_TEXT

OK.

and SSLeay_version(SSLEAY_VERSION), I think.

No.

char *SSLeay_version(t)
int t;
	{
	if (t == SSLEAY_VERSION)
		return("SSLeay 0.9.1a 06-Jul-1998");

That is impressively bogus.

Weird; what openssl version are you looking at? Mine have {{{ crypto/cversion.c: if (t == SSLEAY_VERSION) crypto/cversion.c- return OPENSSL_VERSION_TEXT; }}}

http://repo.or.cz/w/mirror-openssl.git/blob/OpenSSL_1_0_0-stable:/crypto/cversion.c

That's so weird that I want to blame it on the git conversion or something.

Replying to nickm:

That's so weird that I want to blame it on the git conversion or something.

I've pushed a fixup commit to include SSLeay_version(SSLEAY_VERSION) in the log message. Hopefully you're right, and that won't embarrass us.

Now all squashed in bug4822_021_v2_squashed

Merged after review. Thanks, all!

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

Trac:
Resolution: fixed to N/A
Status: closed to reopened

<wanoskarnet> Tor client which uses SSLv23_method must work with a server
which uses TLSv1_method. You missed "SSL_OP_NO_SSLv2" while disscuss #4822.

I don't understand the comment. We don't actually use TLSv1_method server-side, as far as I understand, and we're not planning to, since it would make us reject non-TLS1 handshakes?

Also, what did we miss about SSL_OP_NO_SSLv2? We already set that option.

This is what wanoskarnet said, before the comment that arma pasted in comment:33.

< wanoskarnet> "Tell OpenSSL to only use TLS1. This would actually break compatibility with clients that are configured to use SSLv23_method()". it is wrong statement. SSLv23 client
               sends ProtocolVersion that indicates understanding TLSv1 so server well understand it. Docs means SSLv3 clients that never sends ProtocolVersion == TLSv1 only SSLv3, not
               a SSLv23 clients.
< wanoskarnet> Tor never used SSLv2 compatiblity so client hello exactly SSLv3.1(TLSv1) looking like.
< wanoskarnet> Tor client which uses SSLv23_method can work with a server which uses TLSv1_method. You missed "SSL_OP_NO_SSLv2" while disscuss #4822.

Okay, so if I understand correctly, wanoskarnet is saying that our reading of the TLSv1_method() documentation and the SSLv23_method() documentation is wrong: that a TLSv1_method() client can connect perfectly well to a SSLv23_method() server, and vice versa.

I'm attaching a quick&dirty test program to demonstrate this (using some code from libevent and some from the openssl docs).

This doesn't mean that we need any changes in the code, except for fixing the comment to be correct. I'll do that after I attach the demo code.

Trac:
Resolution: N/A to fixed
Status: reopened to closed

Trac:
demo.sh

Trac:
demoprog.c

tortls.c misidentifies the vulnerability as CVE-2011-4657 while this is CVE-2011-4576 (twice in comments, once in logging).

Trac:
Username: fermenthor

Replying to fermenthor:

tortls.c misidentifies the vulnerability as CVE-2011-4657 while this is CVE-2011-4576 (twice in comments, once in logging).

Moved to #5066 (moved).

Trac:
Keywords: N/A deleted, tor-client added

Trac:
Component: Tor Client to Tor

Milestone Tor: 0.2.1.x-final deleted

Trac:
Milestone: Tor: 0.2.1.x-final to N/A

closed

moved to tpo/core/tor#4822 (closed)