From 8bd3f76a8ea3d8751c0305cfcaaa3592d2ffa9b2 Mon Sep 17 00:00:00 2001 From: Mikkel Krautz Date: Sat, 26 Sep 2015 22:19:20 +0200 Subject: [PATCH] Murmur: add support for EDH cipher suites, and for specifying Diffie-Hellman parmeters. This change allows server admins to specify Diffie-Hellman parameters for Murmur to use. This is done using the sslDHParams option in the config file. Diffie-Hellman parameters can also be set on a per-server basis using the sslDHParams option. Note: the functionality implemented in this change requires the QSslDiffieHellmanParameters class in Qt, which has not yet landed upstream in the Qt 5 'dev' branch. This means that the functionality discussed in this change will, for now, only work in binaries provided by the Mumble project, or binaries that are built using our build environments, and not binaries that link against any released versions of Qt at present. This change modifies the default TLS cipher suite string to add EDH+aRSA+AESGCM, DHE-RSA-AES256-SHA and DHE-RSA-AES128-SHA. This yields the following ciphers, in TLS/RFC notation: TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 TLS_DHE_RSA_WITH_AES_256_CBC_SHA TLS_DHE_RSA_WITH_AES_128_CBC_SHA This change also allows Murmur servers to provide forward secrecy to older clients, such as our own pre-built binaries before 1.2.9. It also provides forward secrecy for users that use Mumble 1.2.x versions on Linux distros, and other Unix-like systems. This is because Mumble 1.2.x on Unix-like systems builds against Qt 4, which limits the connection to TLS 1.0. Before this change, Murmur was not able to negotiate an ephemeral Diffie-Hellman key exchange for those clients. This is now possible. --- INSTALL | 6 ++++ scripts/murmur.ini | 9 +++++- src/SSL.cpp | 2 +- src/murmur/Cert.cpp | 73 ++++++++++++++++++++++++++++++++++++++++++- src/murmur/Meta.cpp | 58 ++++++++++++++++++++++++++++++++-- src/murmur/Meta.h | 1 + src/murmur/Server.cpp | 6 ++++ src/murmur/Server.h | 6 ++++ src/murmur/main.cpp | 1 + src/murmur/murmur.pro | 16 ++++++++++ 10 files changed, 172 insertions(+), 6 deletions(-) diff --git a/INSTALL b/INSTALL index 3fc8490b5..5ecca1cb2 100644 --- a/INSTALL +++ b/INSTALL @@ -160,3 +160,9 @@ CONFIG+=no-gkey (Mumble, Win32) time dependencies, and requires Logitech Gaming Software to be installed to have any effect at runtime. + +CONFIG+=no-qssldiffiehellmanparameters + Don't build in support for custom Diffie-Hellman + parameters, even if the QSslDiffieHellmanParameters + class is available in the version of Qt you are + building against. diff --git a/scripts/murmur.ini b/scripts/murmur.ini index b2c3fff59..02c3c38d2 100644 --- a/scripts/murmur.ini +++ b/scripts/murmur.ini @@ -160,6 +160,13 @@ users=100 ;sslCert= ;sslKey= +; The sslDHParams option allows you to specify a PEM-encoded file with +; Diffie-Hellman parameters, which will be used as the default Diffie- +; Hellman parameters for all virtual servers. +; If a file is not specified, each Murmur virtual server will auto-generate +; its own unique set of 2048-bit Diffie-Hellman parameters on first launch. +;sslDHParams= + ; The sslCiphers option chooses the cipher suites to make available for use ; in SSL/TLS. This option is server-wide, and cannot be set on a ; per-virtual-server basis. @@ -176,7 +183,7 @@ users=100 ; Note: Changing this option may impact the backwards compatibility of your ; Murmur server, and can remove the ability for older Mumble clients to be able ; to connect to it. -;sslCiphers=EECDH+AESGCM:AES256-SHA:AES128-SHA +;sslCiphers=EECDH+AESGCM:EDH+aRSA+AESGCM:DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:AES256-SHA:AES128-SHA ; If Murmur is started as root, which user should it switch to? ; This option is ignored if Murmur isn't started with root privileges. diff --git a/src/SSL.cpp b/src/SSL.cpp index 03573515c..d18db6222 100644 --- a/src/SSL.cpp +++ b/src/SSL.cpp @@ -35,7 +35,7 @@ #include "Version.h" QString MumbleSSL::defaultOpenSSLCipherString() { - return QLatin1String("EECDH+AESGCM:AES256-SHA:AES128-SHA"); + return QLatin1String("EECDH+AESGCM:EDH+aRSA+AESGCM:DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:AES256-SHA:AES128-SHA"); } QList MumbleSSL::ciphersFromOpenSSLCipherString(QString cipherString) { diff --git a/src/murmur/Cert.cpp b/src/murmur/Cert.cpp index cf08ebfb1..99111807f 100644 --- a/src/murmur/Cert.cpp +++ b/src/murmur/Cert.cpp @@ -49,6 +49,15 @@ static int add_ext(X509 * crt, int nid, char *value) { return 1; } +// dh_progress is a status callback for DH_generate_parameterss_ex. +// We use it to run the event loop while generating DH params, in +// order to keep the Murmur GUI on Windows responsive during the +// process. +static int dh_progress(int, int, BN_GENCB *) { + qApp->processEvents(); + return 1; +} + bool Server::isKeyForCert(const QSslKey &key, const QSslCertificate &cert) { if (key.isNull() || cert.isNull() || (key.type() != QSsl::PrivateKey)) return false; @@ -85,11 +94,12 @@ bool Server::isKeyForCert(const QSslKey &key, const QSslCertificate &cert) { } void Server::initializeCert() { - QByteArray crt, key, pass; + QByteArray crt, key, pass, dhparams; crt = getConf("certificate", QString()).toByteArray(); key = getConf("key", QString()).toByteArray(); pass = getConf("passphrase", QByteArray()).toByteArray(); + dhparams = getConf("sslDHParams", Meta::mp.qbaDHParams).toByteArray(); QList ql; @@ -116,6 +126,21 @@ void Server::initializeCert() { qlCA = ql; } +#if defined(USE_QSSLDIFFIEHELLMANPARAMETERS) + if (! dhparams.isEmpty()) { + QSslDiffieHellmanParameters qdhp = QSslDiffieHellmanParameters(dhparams); + if (qdhp.isValid()) { + qsdhpDHParams = qdhp; + } else { + log(QString::fromLatin1("Unable to use specified Diffie-Hellman parameters (sslDHParams): %1").arg(qdhp.errorString())); + } + } +#else + if (! dhparams.isEmpty()) { + log("Diffie-Hellman parameters (sslDHParams) were specified, but will not be used. This version of Murmur does not support Diffie-Hellman parameters."); + } +#endif + QString issuer; #if QT_VERSION >= 0x050000 QStringList issuerNames = qscCert.issuerInfo(QSslCertificate::CommonName); @@ -190,6 +215,52 @@ void Server::initializeCert() { setConf("key", qskKey.toPem()); } } + +#if defined(USE_QSSLDIFFIEHELLMANPARAMETERS) + if (qsdhpDHParams.isEmpty()) { + log("Generating new server 2048-bit Diffie-Hellman parameters. This could take a while..."); + + DH *dh = DH_new(); + if (dh == NULL) { + qFatal("DH_new failed: unable to generate Diffie-Hellman parameters for virtual server"); + } + + // Generate DH params. + // We register a status callback in order to update the UI + // for Murmur on Windows. We don't show the actual status, + // but we do it to keep Murmur on Windows responsive while + // generating the parameters. + BN_GENCB cb; + memset(&cb, 0, sizeof(BN_GENCB)); + BN_GENCB_set(&cb, dh_progress, NULL); + if (DH_generate_parameters_ex(dh, 2048, 2, &cb) == 0) { + qFatal("DH_generate_parameters_ex failed: unable to generate Diffie-Hellman parameters for virtual server"); + } + + BIO *mem = BIO_new(BIO_s_mem()); + if (PEM_write_bio_DHparams(mem, dh) == 0) { + qFatal("PEM_write_bio_DHparams failed: unable to write generated Diffie-Hellman parameters to memory"); + } + + char *pem = NULL; + long len = BIO_get_mem_data(mem, &pem); + if (len <= 0) { + qFatal("BIO_get_mem_data returned an empty or invalid buffer"); + } + + QByteArray pemdh(pem, len); + QSslDiffieHellmanParameters qdhp(pemdh); + if (!qdhp.isValid()) { + qFatal("QSslDiffieHellmanParameters: unable to import generated Diffie-HellmanParameters: %s", qdhp.errorString()); + } + + qsdhpDHParams = qdhp; + setConf("sslDHParams", pemdh); + + BIO_free(mem); + DH_free(dh); + } +#endif } const QString Server::getDigest() const { diff --git a/src/murmur/Meta.cpp b/src/murmur/Meta.cpp index 94e00d983..780f3e8fe 100644 --- a/src/murmur/Meta.cpp +++ b/src/murmur/Meta.cpp @@ -40,6 +40,10 @@ #include "Version.h" #include "SSL.h" +#if defined(USE_QSSLDIFFIEHELLMANPARAMETERS) +# include +#endif + MetaParams Meta::mp; #ifdef Q_OS_WIN @@ -377,6 +381,7 @@ void MetaParams::read(QString fname) { QString qsSSLCert = qsSettings->value("sslCert").toString(); QString qsSSLKey = qsSettings->value("sslKey").toString(); QString qsSSLCA = qsSettings->value("sslCA").toString(); + QString qsSSLDHParams = qsSettings->value("sslDHParams").toString(); qbaPassPhrase = qsSettings->value("sslPassPhrase").toByteArray(); @@ -396,7 +401,7 @@ void MetaParams::read(QString fname) { } } - QByteArray crt, key; + QByteArray crt, key, dhparams; if (! qsSSLCert.isEmpty()) { QFile pem(qsSSLCert); @@ -452,6 +457,31 @@ void MetaParams::read(QString fname) { } } +#if defined(USE_QSSLDIFFIEHELLMANPARAMETERS) + if (! qsSSLDHParams.isEmpty()) { + QFile pem(qsSSLDHParams); + if (pem.open(QIODevice::ReadOnly)) { + dhparams = pem.readAll(); + pem.close(); + } else { + qCritical("Failed to read %s", qPrintable(qsSSLDHParams)); + } + } + + if (! dhparams.isEmpty()) { + QSslDiffieHellmanParameters qdhp = QSslDiffieHellmanParameters(dhparams); + if (qdhp.isValid()) { + qbaDHParams = dhparams; + } else { + qFatal("Unable to use specified Diffie-Hellman parameters: %s", qPrintable(qdhp.errorString())); + } + } +#else + if (! qsSSLDHParams.isEmpty()) { + qFatal("This version of Murmur does not support Diffie-Hellman parameters (sslDHParams). Murmur will not start unless you remove the option from your murmur.ini file."); + } +#endif + if (! QSslSocket::supportsSsl()) { qFatal("Qt without SSL Support"); } @@ -462,10 +492,31 @@ void MetaParams::read(QString fname) { qFatal("Invalid sslCiphers option. Either the cipher string is invalid or none of the ciphers are available: \"%s\"", qPrintable(qsCiphers)); } - QSslSocket::setDefaultCiphers(ciphers); + // If the version of Qt we're building against doesn't support + // QSslDiffieHellmanParameters, then we must filter out Diffie- + // Hellman cipher suites in order to guarantee that we do not + // use Qt's default Diffie-Hellman parameters. + QList filtered; +#if !defined(USE_QSSLDIFFIEHELLMANPARAMETERS) + foreach (QSslCipher c, ciphers) { + if (c.keyExchangeMethod() == QLatin1String("DH")) { + continue; + } + filtered << c; + } + if (ciphers.size() != filtered.size()) { + qWarning("Warning: all cipher suites in sslCiphers using Diffie-Hellman key exchange " + "have been removed. Qt %s does not support custom Diffie-Hellman parameters.", + qVersion()); + } +#else + filtered = ciphers; +#endif + + QSslSocket::setDefaultCiphers(filtered); QStringList pref; - foreach (QSslCipher c, ciphers) { + foreach (QSslCipher c, filtered) { pref << c.name(); } qWarning("Meta: TLS cipher preference is \"%s\"", qPrintable(pref.join(QLatin1String(":")))); @@ -510,6 +561,7 @@ void MetaParams::read(QString fname) { qmConfig.insert(QLatin1String("opusthreshold"), QString::number(iOpusThreshold)); qmConfig.insert(QLatin1String("channelnestinglimit"), QString::number(iChannelNestingLimit)); qmConfig.insert(QLatin1String("sslCiphers"), qsCiphers); + qmConfig.insert(QLatin1String("sslDHParams"), QString::fromLatin1(qbaDHParams.constData())); } Meta::Meta() { diff --git a/src/murmur/Meta.h b/src/murmur/Meta.h index 884e86038..5eb3791df 100644 --- a/src/murmur/Meta.h +++ b/src/murmur/Meta.h @@ -114,6 +114,7 @@ public: QSslCertificate qscCert; QSslKey qskKey; + QByteArray qbaDHParams; QByteArray qbaPassPhrase; QString qsCiphers; diff --git a/src/murmur/Server.cpp b/src/murmur/Server.cpp index 43f1c7a04..8ae835b93 100644 --- a/src/murmur/Server.cpp +++ b/src/murmur/Server.cpp @@ -1173,6 +1173,12 @@ void Server::newClient() { sock->addCaCertificate(qscCert); sock->addCaCertificates(qlCA); +#if defined(USE_QSSLDIFFIEHELLMANPARAMETERS) + QSslConfiguration cfg = sock->sslConfiguration(); + cfg.setDiffieHellmanParameters(qsdhpDHParams); + sock->setSslConfiguration(cfg); +#endif + if (qqIds.isEmpty()) { log(QString("Session ID pool (%1) empty, rejecting connection").arg(iMaxUsers)); sock->disconnectFromHost(); diff --git a/src/murmur/Server.h b/src/murmur/Server.h index 19e26706e..d437d68e1 100644 --- a/src/murmur/Server.h +++ b/src/murmur/Server.h @@ -49,6 +49,9 @@ #include #include #include +#if defined(USE_QSSLDIFFIEHELLMANPARAMETERS) +# include +#endif #ifdef Q_OS_WIN #include #endif @@ -168,6 +171,9 @@ class Server : public QThread { QList qlCA; QSslCertificate qscCert; QSslKey qskKey; +#if defined(USE_QSSLDIFFIEHELLMANPARAMETERS) + QSslDiffieHellmanParameters qsdhpDHParams; +#endif Timer tUptime; diff --git a/src/murmur/main.cpp b/src/murmur/main.cpp index 5c5d291fb..d9f7b4b58 100644 --- a/src/murmur/main.cpp +++ b/src/murmur/main.cpp @@ -419,6 +419,7 @@ int main(int argc, char **argv) { ServerDB::setConf(sid, "key"); ServerDB::setConf(sid, "certificate"); ServerDB::setConf(sid, "passphrase"); + ServerDB::setConf(sid, "sslDHParams"); } } diff --git a/src/murmur/murmur.pro b/src/murmur/murmur.pro index 79480e762..dda41f164 100644 --- a/src/murmur/murmur.pro +++ b/src/murmur/murmur.pro @@ -168,4 +168,20 @@ bonjour { } } +# Check for QSslDiffieHellmanParameters availability, and define +# USE_QSSLDIFFIEHELLMANPARAMETERS preprocessor if available. +# +# Can be disabled with no-qssldiffiehellmanparameters. +!CONFIG(no-qssldiffiehellmanparameters):exists($$[QT_INSTALL_HEADERS]/QtNetwork/QSslDiffieHellmanParameters) { + # ...but only if we're inside a Mumble build environment for now. + # If someone decides to put a Mumble snapshot into a distro, this + # could break the build in the future, with newer versions of Qt, + # if the API of QSslDiffieHellmanParameters changes when it is + # upstreamed. + MUMBLE_PREFIX=$$(MUMBLE_PREFIX) + !isEmpty(MUMBLE_PREFIX) { + DEFINES += USE_QSSLDIFFIEHELLMANPARAMETERS + } +} + include(../../symbols.pri)