fuzzball-muck / fuzzball

Ongoing development of the Fuzzball MUCK server software and associated functionality.
Other
46 stars 26 forks source link

FB7 does not support MODP-based Diffie-Hellman #197

Closed ghost closed 6 years ago

ghost commented 6 years ago

Testing has revealed that FB7 supports ECDH key agreement, but not traditional modulo-based DH key agreement. While ECDH is superior in many aspects, it's entirely possible (perhaps even likely) that some TLS v1.0-only clients support DH while lacking support for ECDH. Enabling this support might go a long way toward eventually being able to disable the (now deprecated/insecure) RSA encryption-based cipher modes.

Tested using tf5.0b8 on CentOS 7.

Configured a test MUCK using the following SSL preference: aRSA+EECDH+AESGCM:aRSA+EDH+AESGCM:aRSA+EECDH+AES+SHA384:aRSA+EECDH+AES+SHA256:aRSA+EDH+AES+SHA384:aRSA+EDH+AES+SHA256:aRSA+EECDH+AES+SHA1:aRSA+EDH+AES+SHA1:aRSA+AESGCM:aRSA+AES+SHA384:aRSA+AES+SHA256:aRSA+AES+SHA1:!SRP:!PSK

$ openssl ciphers 'aRSA+EECDH+AESGCM:aRSA+EDH+AESGCM:aRSA+EECDH+AES+SHA384:aRSA+EECDH+AES+SHA256:aRSA+EDH+AES+SHA384:aRSA+EDH+AES+SHA256:aRSA+EECDH+AES+SHA1:aRSA+EDH+AES+SHA1:aRSA+AESGCM:aRSA+AES+SHA384:aRSA+AES+SHA256:aRSA+AES+SHA1:!SRP:!PSK' | tr ':' '\n'

ECDHE-RSA-AES256-GCM-SHA384 ECDHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM-SHA384 DHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-SHA384 ECDHE-RSA-AES128-SHA256 DHE-RSA-AES256-SHA256 DHE-RSA-AES128-SHA256 ECDHE-RSA-AES256-SHA ECDHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-AES128-SHA AES256-GCM-SHA384 AES128-GCM-SHA256 AES256-SHA256 AES128-SHA256 AES256-SHA AES128-SHA

Attempting to connect to the test MUCK succeeds using the ECDHE-RSA-AES256-GCM-SHA384 cipher suite.

Appending !EECDH to the end of the cipher preferences string, restarting the MUCK, and reattempting the connection succeeds using the AES256-GCM-SHA384 cipher suite, which is the highest-preference (obsolete) RSA encryption mode specified in the cipher preferences.

Performing the same tests against a web server causes tf to connect to the web server using the DHE-RSA-AES256-GCM-SHA384 cipher suite, which is the legacy-but-still-secure MODP-based Diffie-Hellman key agreement mode.

ghost commented 6 years ago

Tested using tf5.0b8 on CentOS 7.

I also use tinyfugue as my client, so I'm familiar with the fact that it honestly doesn't do so well with TLS, despite the fact that it CAN do them. I'd firmly recommend, when testing TLS stuff, to instead be using OpenBSD's netcat, which can do TLS connections as so: nc -c https://man.openbsd.org/nc It's also available in many GNU/Linux distributions as a package, and also available in the portable release of libressl. It, unlike tinyfugue, verifies cert signatures, among other things. You can use the -T flag to modify how strict it is. In any case, tinyfugue shouldn't be used for TLS testing even though it's a nice client for actual MUCKing. For example, since HereLieMonsters has a self-signed certificate: nc -c -T noverify muck.hereliemonsters.org 7777 connects. Using -cv instead of -c lets you see details about the TLS negotiation when you connect. Many more details than tinyfugue tells you.

ghost commented 6 years ago

(Edited to mitigate pre-caffeine surliness.)

I'm familiar with the fact that it honestly doesn't do so well with TLS

TinyFugue does just fine with TLS (up to and including TLS v1.2) when linked against OpenSSL. When you forcibly relink it to Some Other TLS library by way of a hacked-together-and-relatively-unmaintained shim library (Debian, we're looking at you, here!), no, it doesn't do TLS well at all.

I'd firmly recommend, when testing TLS stuff, to instead be using OpenBSD's netcat...

The same argument could be made for using openssl's s_client functionality. Anything that verifies which cipher suite is negotiated with the other end is suitable for purpose. TinyFugue just happens to be a common MUCK client that is also very verbose about what cipher suite it used to connect.

It, unlike tinyfugue, verifies cert signatures, among other things...

The issue is about FB7 not supporting MODP Diffie-Hellman. I'm unaware of any MUCK client that actually does any certificate verification (which is an entirely separate point of annoyance for me, by the way), so that capability isn't really relevant to troubleshooting this issue.

In any case, tinyfugue shouldn't be used for TLS testing

As long as it connects, negotiates a cipher suite, and reports which one it used, it'll work for testing this particular issue. In general, however, I would agree with you.

Many more details than tinyfugue tells you.

For this particular issue, only one detail is actually helpful: which cipher suite was negotiated with the server? For anything else, you're right, I'd probably want the extra details, but this one is fairly simple.

charlesreiss commented 6 years ago

The underlying cause is probably a lack of call to SSL_CTX_set_tmp_dh or SSL_CTX_set_tmp_dh_auto. I think I previously avoided doing this because I was concerned about old Java clients which would negotiate a DHE ciphersuite but then reject modp groups larger than 1024 bits, which we can probably stop caring about anymore.

ghost commented 6 years ago

Think we could do an Apache-style solution and allow for the administrator to specify a PEM-format DH parameter group in the cert file? That would allow the admin to determine whether or not they want to support old clients (by specifying a 1024-bit MODP group, or something larger otherwise), and default to something sane (the standard 2048-bit group, perhaps) if one isn't given?

Edit: A standard/preset DH group is a bad idea as I think about it further. Perhaps fbmuck can make a call to OpenSSL's DH parameter group generation routine with sane assumptions if one does not already exist and generate a parameter set to be used going forward, stored in a file dh.pem alongside server.pem in the data directory?

charlesreiss commented 6 years ago

With OpenSSL 1.1+, we probably should just call SSL_CTX_set_tmp_dh_auto SSL_CTX_set_dh_auto, on the assumption that the OpenSSL maintainers will do the right thing.

For prior versions, a parameter file generated in the build process would probably be easiest.

wyld-sw commented 6 years ago

I added the following to reconfigure_ssl:

#if defined(SSL_CTX_set_dh_auto)
    if (ctx->config->dheparams == -1)
        SSL_CTX_set_dh_auto(new_ssl_ctx, 1);
    else if (ctx->config->dheparams == 1024)
        SSL_CTX_set_dh_auto(new_ssl_ctx, 2);
#endif

Seems to work. Let me know if it's actually completely broken, or if there's a better way. I'll close this issue after a few days if we determine it's the right solution.