Skip to content

Commit

Permalink
Merge PR #3174: SSL: register OpenSSL threading callbacks when we can…
Browse files Browse the repository at this point in the history
…'t access Qt's OpenSSL.
  • Loading branch information
mkrautz committed Jul 17, 2017
2 parents 80916fd + f6fb4d8 commit 5b82a7a
Show file tree
Hide file tree
Showing 19 changed files with 325 additions and 16 deletions.
26 changes: 23 additions & 3 deletions src/SSL.cpp
Expand Up @@ -6,17 +6,37 @@
#include "murmur_pch.h"

#include "SSL.h"
#include "SSLLocks.h"

#include "Version.h"

void MumbleSSL::initialize() {
// Let Qt initialize its copy of OpenSSL, if it's different than
// Mumble's. (Initialization is a side-effect of calling
// QSslSocket::supportsSsl()).
QSslSocket::supportsSsl();

// Initialize our copy of OpenSSL.
SSL_library_init(); // Safe to discard return value, per OpenSSL man pages.
SSL_load_error_strings();

// Let Qt initialize its copy of OpenSSL, if it's different than
// Mumble's.
QSslSocket::supportsSsl();
// Determine if a locking callback has not been set.
// This should be the case if there are multiple copies
// of OpensSSL in the address space. This is mostly due
// to Qt dynamically loading OpenSSL when it is not
// configured with -openssl-linked.
//
// If we detect that no locking callback is configured, we
// have to set it up ourselves to allow multi-threaded use
// of OpenSSL.
void *lockcb = reinterpret_cast<void *>(CRYPTO_get_locking_callback());
if (lockcb == NULL) {
SSLLocks::initialize();
}
}

void MumbleSSL::destroy() {
SSLLocks::destroy();
}

QString MumbleSSL::defaultOpenSSLCipherString() {
Expand Down
1 change: 1 addition & 0 deletions src/SSL.h
Expand Up @@ -13,6 +13,7 @@
class MumbleSSL {
public:
static void initialize();
static void destroy();
static QString defaultOpenSSLCipherString();
static QList<QSslCipher> ciphersFromOpenSSLCipherString(QString cipherString);
static void addSystemCA();
Expand Down
85 changes: 85 additions & 0 deletions src/SSLLocks.cpp
@@ -0,0 +1,85 @@
// Copyright 2005-2017 The Mumble Developers. All rights reserved.
// Use of this source code is governed by a BSD-style license
// that can be found in the LICENSE file at the root of the
// Mumble source tree or at <https://www.mumble.info/LICENSE>.

#include "murmur_pch.h"

#include "SSLLocks.h"

static QMutex **locks = NULL;

void locking_callback(int mode, int type, const char *, int) {
if (mode & CRYPTO_LOCK) {
locks[type]->lock();
} else {
locks[type]->unlock();
}
}

unsigned long id_callback() {
// OpenSSL's thread ID callback must return an unsigned long.
// It's therefore important that we make sure the value from
// Qt's QThread::currentThreadId actually makes sense to return
// in this case.
//
// On Windows, the return type is a DWORD. This is excellent,
// because on an LLP64 like 64-bit Windows, unsigned long is
// only 32-bits wide, just like DWORD.
//
// On macOS and embedded Linux, it's void *. And when Qt targets
// X11, it's unsigned long.
//
// So, in all cases, it's safe to use unsigned long.
//
// The trouble is, since the return type of QThread::currentThreadId()
// difers between platforms, we have to be clever about how we convert
// that type to an unsigned long. The way we do it here is to first convert
// it to an uintptr_t, since both pointer types and integer types will
// convert to that without the compiler complaining. Then, we convert
// *that* to an unsigned long. (Note that for LLP64 platforms, such as
// 64-bit Windows, this conversion is from a 64-bit type to a 32-bit type)
uintptr_t val = reinterpret_cast<uintptr_t>(QThread::currentThreadId());
return static_cast<unsigned long>(val);
}

void SSLLocks::initialize() {
int nlocks = CRYPTO_num_locks();

locks = reinterpret_cast<QMutex **>(calloc(nlocks, sizeof(QMutex *)));
if (locks == NULL) {
qFatal("SSLLocks: unable to allocate locks array");

// This initializer is called early during program
// execution, and the message handler that causes
// qFatal to terminate execution might not be registered
// yet. So, do exit(1) just in case.
exit(1);
}

for (int i = 0; i < nlocks; i++) {
locks[i] = new QMutex;
}

CRYPTO_set_locking_callback(locking_callback);
CRYPTO_set_id_callback(id_callback);
}

void SSLLocks::destroy() {
// If SSLLocks was never initialized, or has been destroyed already,
// don't try to do it again.
if (locks == NULL) {
return;
}

CRYPTO_set_locking_callback(NULL);
CRYPTO_set_id_callback(NULL);

int nlocks = CRYPTO_num_locks();
for (int i = 0; i < nlocks; i++) {
delete locks[i];
}

free(locks);
locks = NULL;
}
15 changes: 15 additions & 0 deletions src/SSLLocks.h
@@ -0,0 +1,15 @@
// Copyright 2005-2017 The Mumble Developers. All rights reserved.
// Use of this source code is governed by a BSD-style license
// that can be found in the LICENSE file at the root of the
// Mumble source tree or at <https://www.mumble.info/LICENSE>.

#ifndef MUMBLE_SSLLOCKS_H_
#define MUMBLE_SSLLOCKS_H_

class SSLLocks {
public:
static void initialize();
static void destroy();
};

#endif
4 changes: 2 additions & 2 deletions src/mumble.pri
Expand Up @@ -14,8 +14,8 @@ CONFIG += qt thread debug_and_release warn_on
DEFINES *= MUMBLE_VERSION_STRING=$$VERSION
INCLUDEPATH += $$PWD . ../mumble_proto
VPATH += $$PWD
HEADERS *= ACL.h Channel.h CryptState.h Connection.h Group.h HTMLFilter.h User.h Net.h OSInfo.h Timer.h SSL.h Version.h SSLCipherInfo.h SSLCipherInfoTable.h licenses.h License.h LogEmitter.h CryptographicHash.h CryptographicRandom.h PasswordGenerator.h ByteSwap.h HostAddress.cpp Ban.h EnvUtils.h UnresolvedServerAddress.h ServerAddress.h ServerResolver.h ServerResolverRecord.h SelfSignedCertificate.h
SOURCES *= ACL.cpp Group.cpp Channel.cpp Connection.cpp HTMLFilter.cpp User.cpp Timer.cpp CryptState.cpp OSInfo.cpp SSL.cpp Version.cpp SSLCipherInfo.cpp License.cpp LogEmitter.cpp CryptographicHash.cpp CryptographicRandom.cpp PasswordGenerator.cpp HostAddress.cpp Ban.cpp EnvUtils.cpp UnresolvedServerAddress.cpp ServerAddress.cpp ServerResolver_qt5.cpp ServerResolverRecord.cpp SelfSignedCertificate.cpp
HEADERS *= ACL.h Channel.h CryptState.h Connection.h Group.h HTMLFilter.h User.h Net.h OSInfo.h Timer.h SSL.h Version.h SSLCipherInfo.h SSLCipherInfoTable.h licenses.h License.h LogEmitter.h CryptographicHash.h CryptographicRandom.h PasswordGenerator.h ByteSwap.h HostAddress.cpp Ban.h EnvUtils.h UnresolvedServerAddress.h ServerAddress.h ServerResolver.h ServerResolverRecord.h SelfSignedCertificate.h SSLLocks.h
SOURCES *= ACL.cpp Group.cpp Channel.cpp Connection.cpp HTMLFilter.cpp User.cpp Timer.cpp CryptState.cpp OSInfo.cpp SSL.cpp Version.cpp SSLCipherInfo.cpp License.cpp LogEmitter.cpp CryptographicHash.cpp CryptographicRandom.cpp PasswordGenerator.cpp HostAddress.cpp Ban.cpp EnvUtils.cpp UnresolvedServerAddress.cpp ServerAddress.cpp ServerResolver_qt5.cpp ServerResolverRecord.cpp SelfSignedCertificate.cpp SSLLocks.cpp
LIBS *= -lmumble_proto

equals(QT_MAJOR_VERSION, 4) {
Expand Down
3 changes: 3 additions & 0 deletions src/mumble/main.cpp
Expand Up @@ -600,6 +600,9 @@ int main(int argc, char **argv) {
// correctly.
userLockFile.release();
#endif

// Tear down OpenSSL state.
MumbleSSL::destroy();

// At this point termination of our process is immenent. We can safely
// launch another version of Mumble. The reason we do an actual
Expand Down
11 changes: 11 additions & 0 deletions src/tests/TestCrypt/TestCrypt.cpp
Expand Up @@ -9,19 +9,30 @@
#include <QtCore>
#include <QtTest>

#include "SSL.h"
#include "Timer.h"
#include "CryptState.h"

class TestCrypt : public QObject {
Q_OBJECT
private slots:
void initTestCase();
void cleanupTestCase();
void testvectors();
void authcrypt();
void ivrecovery();
void reverserecovery();
void tamper();
};

void TestCrypt::initTestCase() {
MumbleSSL::initialize();
}

void TestCrypt::cleanupTestCase() {
MumbleSSL::destroy();
}

void TestCrypt::reverserecovery() {
CryptState enc, dec;
enc.genKey();
Expand Down
4 changes: 2 additions & 2 deletions src/tests/TestCrypt/TestCrypt.pro
Expand Up @@ -8,5 +8,5 @@ include(../test.pri)
QT *= network

TARGET = TestCrypt
HEADERS = Timer.h CryptState.h
SOURCES = TestCrypt.cpp CryptState.cpp Timer.cpp
HEADERS = SSL.h SSLLocks.h Timer.h CryptState.h
SOURCES = SSL.cpp SSLLocks.cpp TestCrypt.cpp CryptState.cpp Timer.cpp
13 changes: 13 additions & 0 deletions src/tests/TestCryptographicHash/TestCryptographicHash.cpp
Expand Up @@ -7,11 +7,16 @@
#include <QtTest>
#include <QLatin1String>

#include "SSL.h"

#include "CryptographicHash.h"

class TestCryptographicHash : public QObject {
Q_OBJECT
private slots:
void initTestCase();
void cleanupTestCase();

void sha1_data();
void sha1();

Expand All @@ -23,6 +28,14 @@ class TestCryptographicHash : public QObject {
void addDataAfterResult();
};

void TestCryptographicHash::initTestCase() {
MumbleSSL::initialize();
}

void TestCryptographicHash::cleanupTestCase() {
MumbleSSL::destroy();
}

/// normalizeHash removes all whitespace from the hex-encoded hash string.
static QString normalizeHash(QString str) {
str.replace(QLatin1String(" "), QLatin1String(""));
Expand Down
6 changes: 4 additions & 2 deletions src/tests/TestCryptographicHash/TestCryptographicHash.pro
Expand Up @@ -5,6 +5,8 @@

include(../test.pri)

QT += network

TARGET = TestCryptographicHash
SOURCES = TestCryptographicHash.cpp CryptographicHash.cpp
HEADERS = CryptographicHash.h
SOURCES = SSL.cpp SSLLocks.cpp TestCryptographicHash.cpp CryptographicHash.cpp
HEADERS = SSL.h SSLLocks.h CryptographicHash.h
12 changes: 12 additions & 0 deletions src/tests/TestCryptographicRandom/TestCryptographicRandom.cpp
Expand Up @@ -6,6 +6,8 @@
#include <QtCore>
#include <QtTest>

#include "SSL.h"

#include "CryptographicRandom.h"

#include <stdint.h>
Expand All @@ -15,11 +17,21 @@
class TestCryptographicRandom : public QObject {
Q_OBJECT
private slots:
void initTestCase();
void cleanupTestCase();
void fillBuffer();
void uint32();
void uniform();
};

void TestCryptographicRandom::initTestCase() {
MumbleSSL::initialize();
}

void TestCryptographicRandom::cleanupTestCase() {
MumbleSSL::destroy();
}

// Verify the entropy of the data returned by the random source
// by zlib compressing it and ensuring the compressed size is at
// least 99% of the size of the input data.
Expand Down
6 changes: 4 additions & 2 deletions src/tests/TestCryptographicRandom/TestCryptographicRandom.pro
Expand Up @@ -5,9 +5,11 @@

include(../test.pri)

QT += network

TARGET = TestCryptographicRandom
SOURCES = TestCryptographicRandom.cpp CryptographicRandom.cpp arc4random_uniform.cpp
HEADERS = CryptographicHash.h
SOURCES = SSL.cpp SSLLocks.cpp TestCryptographicRandom.cpp CryptographicRandom.cpp arc4random_uniform.cpp
HEADERS = SSL.h SSLLocks.h CryptographicHash.h

VPATH *= ../../../3rdparty/arc4random-src
INCLUDEPATH *= ../../../3rdparty/arc4random-src
12 changes: 12 additions & 0 deletions src/tests/TestPasswordGenerator/TestPasswordGenerator.cpp
Expand Up @@ -6,6 +6,8 @@
#include <QtCore>
#include <QtTest>

#include "SSL.h"

#include "PasswordGenerator.h"

// Get the password alphabet from PasswordGenerator.
Expand All @@ -14,9 +16,19 @@ extern QVector<QChar> mumble_password_generator_alphabet();
class TestPasswordGenerator : public QObject {
Q_OBJECT
private slots:
void initTestCase();
void cleanupTestCase();
void random();
};

void TestPasswordGenerator::initTestCase() {
MumbleSSL::initialize();
}

void TestPasswordGenerator::cleanupTestCase() {
MumbleSSL::destroy();
}

void TestPasswordGenerator::random() {
QVector<QChar> alphabet = mumble_password_generator_alphabet();
for (int i = 0; i < 100; i++) {
Expand Down
6 changes: 4 additions & 2 deletions src/tests/TestPasswordGenerator/TestPasswordGenerator.pro
Expand Up @@ -5,9 +5,11 @@

include(../test.pri)

QT += network

TARGET = TestPasswordGenerator
SOURCES = TestPasswordGenerator.cpp PasswordGenerator.cpp CryptographicRandom.cpp arc4random_uniform.cpp
HEADERS = PasswordGenerator.h CryptographicHash.h
SOURCES = SSL.cpp SSLLocks.cpp TestPasswordGenerator.cpp PasswordGenerator.cpp CryptographicRandom.cpp arc4random_uniform.cpp
HEADERS = SSL.h SSLLocks.h PasswordGenerator.h CryptographicHash.h

VPATH *= ../../../3rdparty/arc4random-src
INCLUDEPATH *= ../../../3rdparty/arc4random-src

0 comments on commit 5b82a7a

Please sign in to comment.