Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[qtnetwork] Replace SSL verification error locks #11

Open
wants to merge 1 commit into
base: mer-5.6
Choose a base branch
from
Open

[qtnetwork] Replace SSL verification error locks #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 41 additions & 23 deletions src/network/ssl/qsslsocket_openssl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ bool QSslSocketPrivate::s_loadRootCertsOnDemand = false;

#if OPENSSL_VERSION_NUMBER >= 0x10001000L
int QSslSocketBackendPrivate::s_indexForSSLExtraData = -1;
int QSslSocketBackendPrivate::s_indexForSSLErrorExtraData = -1;
int QSslSocketBackendPrivate::s_indexForX509StoreErrorExtraData = -1;
#endif

/* \internal
Expand Down Expand Up @@ -257,21 +259,10 @@ QSslCipher QSslSocketBackendPrivate::QSslCipher_from_SSL_CIPHER(SSL_CIPHER *ciph
}
return ciph;
}

// ### This list is shared between all threads, and protected by a
// mutex. Investigate using thread local storage instead.
struct QSslErrorList
{
QMutex mutex;
QList<QPair<int, int> > errors;
};
Q_GLOBAL_STATIC(QSslErrorList, _q_sslErrorList)

int q_X509Callback(int ok, X509_STORE_CTX *ctx)
{
if (!ok) {
// Store the error and at which depth the error was detected.
_q_sslErrorList()->errors << qMakePair<int, int>(q_X509_STORE_CTX_get_error(ctx), q_X509_STORE_CTX_get_error_depth(ctx));
#ifdef QSSLSOCKET_DEBUG
qCDebug(lcSsl) << "verification error: dumping bad certificate";
qCDebug(lcSsl) << QSslCertificatePrivate::QSslCertificate_from_X509(q_X509_STORE_CTX_get_current_cert(ctx)).toPem();
Expand All @@ -292,6 +283,28 @@ int q_X509Callback(int ok, X509_STORE_CTX *ctx)
qCDebug(lcSsl) << "Valid:" << cert.effectiveDate() << '-' << cert.expiryDate();
}
#endif
QList<QPair<int, int>> *errorList = nullptr;
// Check if the error list is stored as exdata in the X509 store
if (X509_STORE *store = q_X509_STORE_CTX_get0_store(ctx)) {
errorList = static_cast<QList<QPair<int, int>> *>(q_X509_STORE_get_ex_data(store, QSslSocketBackendPrivate::s_indexForX509StoreErrorExtraData));
}

// If not found, check if it is stored as exdata in the SSL object
if (errorList == nullptr) {
if (SSL *ssl = static_cast<SSL *>(q_X509_STORE_CTX_get_ex_data(ctx, q_SSL_get_ex_data_X509_STORE_CTX_idx()))) {
errorList = static_cast<QList<QPair<int, int>> *>(q_SSL_get_ex_data(ssl, QSslSocketBackendPrivate::s_indexForSSLErrorExtraData));
} else {
qCWarning(lcSsl, "No SSL attached to X509_STORE_CTX");
}
}

if (errorList == nullptr) {
qCWarning(lcSsl, "Neither the X509_STORE_CTX or SSL struct contain an error list. This should not be happening.");
return 0;
} else {
auto errorPair = QPair<int, int>(q_X509_STORE_CTX_get_error(ctx), q_X509_STORE_CTX_get_error_depth(ctx));
errorList->append(errorPair);
}
}
// Always return OK to allow verification to continue. We're handle the
// errors gracefully after collecting all errors, after verification has
Expand Down Expand Up @@ -484,8 +497,11 @@ bool QSslSocketPrivate::ensureLibraryLoaded()
q_OpenSSL_add_all_algorithms();

#if OPENSSL_VERSION_NUMBER >= 0x10001000L
if (q_SSLeay() >= 0x10001000L)
if (q_SSLeay() >= 0x10001000L) {
QSslSocketBackendPrivate::s_indexForSSLExtraData = q_SSL_get_ex_new_index(0L, NULL, NULL, NULL, NULL);
QSslSocketBackendPrivate::s_indexForSSLErrorExtraData = q_SSL_get_ex_new_index(0L, nullptr, nullptr, nullptr, nullptr);
QSslSocketBackendPrivate::s_indexForX509StoreErrorExtraData = q_CRYPTO_get_ex_new_index(CRYPTO_EX_INDEX_X509_STORE, 0l, nullptr, nullptr, nullptr, nullptr);
}
#endif

// Initialize OpenSSL's random seed.
Expand Down Expand Up @@ -1096,11 +1112,11 @@ bool QSslSocketBackendPrivate::startHandshake()

// Check if the connection has been established. Get all errors from the
// verification stage.
_q_sslErrorList()->mutex.lock();
_q_sslErrorList()->errors.clear();
QList<QPair<int, int>> lastErrors;
q_SSL_set_ex_data(ssl, s_indexForSSLErrorExtraData, &lastErrors);
int result = (mode == QSslSocket::SslClientMode) ? q_SSL_connect(ssl) : q_SSL_accept(ssl);
q_SSL_set_ex_data(ssl, s_indexForSSLErrorExtraData, nullptr); // clear exdata to prevent invalid memory access

const QList<QPair<int, int> > &lastErrors = _q_sslErrorList()->errors;
if (!lastErrors.isEmpty())
storePeerCertificates();
for (int i = 0; i < lastErrors.size(); ++i) {
Expand All @@ -1112,7 +1128,6 @@ bool QSslSocketBackendPrivate::startHandshake()
}

errorList << lastErrors;
_q_sslErrorList()->mutex.unlock();

// Connection aborted during handshake phase.
if (q->state() != QAbstractSocket::ConnectedState)
Expand Down Expand Up @@ -1692,7 +1707,14 @@ QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &
}
}

QMutexLocker sslErrorListMutexLocker(&_q_sslErrorList()->mutex);
QList<QPair<int, int>> lastErrors;
#if OPENSSL_VERSION_NUMBER >= 0x10100000
if (!q_X509_STORE_set_ex_data(certStore, s_indexForX509StoreErrorExtraData, &lastErrors)) {
qCWarning(lcSsl, "Cannot attach exdata to the certificate store");
errors << QSslError(QSslError::UnspecifiedError);
q_X509_STORE_free(certStore);
}
#endif

// Register a custom callback to get all verification errors.
X509_STORE_set_verify_cb_func(certStore, q_X509Callback);
Expand Down Expand Up @@ -1749,10 +1771,6 @@ QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &
#endif

// Now process the errors
const QList<QPair<int, int> > errorList = _q_sslErrorList()->errors;
_q_sslErrorList()->errors.clear();

sslErrorListMutexLocker.unlock();

// Translate the errors
if (QSslCertificatePrivate::isBlacklisted(certificateChain[0])) {
Expand All @@ -1768,10 +1786,10 @@ QList<QSslError> QSslSocketBackendPrivate::verify(const QList<QSslCertificate> &
}

// Translate errors from the error list into QSslErrors.
const int numErrors = errorList.size();
const int numErrors = lastErrors.size();
errors.reserve(errors.size() + numErrors);
for (int i = 0; i < numErrors; ++i) {
const QPair<int, int> &errorAndDepth = errorList.at(i);
const QPair<int, int> &errorAndDepth = lastErrors.at(i);
int err = errorAndDepth.first;
int depth = errorAndDepth.second;
errors << _q_OpenSSL_to_QSslError(err, certificateChain.value(depth));
Expand Down
4 changes: 4 additions & 0 deletions src/network/ssl/qsslsocket_openssl_p.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ class QSslSocketBackendPrivate : public QSslSocketPrivate
#if OPENSSL_VERSION_NUMBER >= 0x10001000L
static int s_indexForSSLExtraData; // index used in SSL_get_ex_data to get the matching QSslSocketBackendPrivate
#endif
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
static int s_indexForSSLErrorExtraData; // user data index used for attaching a list of errors to a SSL struct
static int s_indexForX509StoreErrorExtraData; // user data index used for attaching a list of errors to a X509_STORE
#endif

// Platform specific functions
void startClientEncryption() Q_DECL_OVERRIDE;
Expand Down
24 changes: 24 additions & 0 deletions src/network/ssl/qsslsocket_openssl_symbols.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ DEFINEFUNC(int, EC_GROUP_get_degree, const EC_GROUP* g, g, return 0, return)
DEFINEFUNC(int, CRYPTO_num_locks, DUMMYARG, DUMMYARG, return 0, return)
DEFINEFUNC(void, CRYPTO_set_locking_callback, void (*a)(int, int, const char *, int), a, return, DUMMYARG)
DEFINEFUNC(void, CRYPTO_set_id_callback, unsigned long (*a)(), a, return, DUMMYARG)
DEFINEFUNC3(int, CRYPTO_set_ex_data, CRYPTO_EX_DATA * r, r, int idx, idx, void * arg, arg, return 0, return);
DEFINEFUNC2(void *, CRYPTO_get_ex_data, CRYPTO_EX_DATA * r, r, int idx, idx, return nullptr, return);
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
DEFINEFUNC3(void, CRYPTO_free, void *str, str, const char *file, file, int line, line, return, DUMMYARG)
#else
Expand Down Expand Up @@ -341,6 +343,7 @@ DEFINEFUNC(SSL_SESSION*, SSL_get_session, const SSL *ssl, ssl, return 0, return)
DEFINEFUNC5(int, SSL_get_ex_new_index, long argl, argl, void *argp, argp, CRYPTO_EX_new *new_func, new_func, CRYPTO_EX_dup *dup_func, dup_func, CRYPTO_EX_free *free_func, free_func, return -1, return)
DEFINEFUNC3(int, SSL_set_ex_data, SSL *ssl, ssl, int idx, idx, void *arg, arg, return 0, return)
DEFINEFUNC2(void *, SSL_get_ex_data, const SSL *ssl, ssl, int idx, idx, return NULL, return)
DEFINEFUNC(int, SSL_get_ex_data_X509_STORE_CTX_idx, void, DUMMYARG, return -1, return)
#endif
#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_PSK)
DEFINEFUNC2(void, SSL_set_psk_client_callback, SSL* ssl, ssl, q_psk_client_callback_t callback, callback, return, DUMMYARG)
Expand Down Expand Up @@ -423,14 +426,25 @@ DEFINEFUNC(EVP_PKEY *, X509_PUBKEY_get, X509_PUBKEY *a, a, return 0, return)
DEFINEFUNC(void, X509_STORE_free, X509_STORE *a, a, return, DUMMYARG)
DEFINEFUNC(X509_STORE *, X509_STORE_new, DUMMYARG, DUMMYARG, return 0, return)
DEFINEFUNC2(int, X509_STORE_add_cert, X509_STORE *a, a, X509 *b, b, return 0, return)
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
DEFINEFUNC3(int, X509_STORE_set_ex_data, X509_STORE *d, d, int idx, idx, void *data, data, return -1, return)
DEFINEFUNC2(void *, X509_STORE_get_ex_data, X509_STORE *d, d, int idx, idx, return nullptr, return)
#endif
DEFINEFUNC(void, X509_STORE_CTX_free, X509_STORE_CTX *a, a, return, DUMMYARG)
DEFINEFUNC4(int, X509_STORE_CTX_init, X509_STORE_CTX *a, a, X509_STORE *b, b, X509 *c, c, STACK_OF(X509) *d, d, return -1, return)
DEFINEFUNC2(int, X509_STORE_CTX_set_purpose, X509_STORE_CTX *a, a, int b, b, return -1, return)
DEFINEFUNC(int, X509_STORE_CTX_get_error, X509_STORE_CTX *a, a, return -1, return)
DEFINEFUNC(int, X509_STORE_CTX_get_error_depth, X509_STORE_CTX *a, a, return -1, return)
DEFINEFUNC(X509 *, X509_STORE_CTX_get_current_cert, X509_STORE_CTX *a, a, return 0, return)
DEFINEFUNC(STACK_OF(X509) *, X509_STORE_CTX_get_chain, X509_STORE_CTX *a, a, return 0, return)
DEFINEFUNC(X509_STORE *, X509_STORE_CTX_get0_store, X509_STORE_CTX *ctx, ctx, return nullptr, return)
DEFINEFUNC(X509_STORE_CTX *, X509_STORE_CTX_new, DUMMYARG, DUMMYARG, return 0, return)

#if OPENSSL_VERSION_NUMBER >= 0x10100000L
DEFINEFUNC3(int, X509_STORE_CTX_set_ex_data, X509_STORE_CTX *d, d, int idx, idx, void *data, data, return -1, return)
DEFINEFUNC2(void *, X509_STORE_CTX_get_ex_data, X509_STORE_CTX *d, d, int idx, idx, return nullptr, return)
#endif

#ifdef SSLEAY_MACROS
DEFINEFUNC2(int, i2d_DSAPrivateKey, const DSA *a, a, unsigned char **b, b, return -1, return)
DEFINEFUNC2(int, i2d_RSAPrivateKey, const RSA *a, a, unsigned char **b, b, return -1, return)
Expand Down Expand Up @@ -975,6 +989,7 @@ bool q_resolveOpenSslSymbols()
RESOLVEFUNC(SSL_get_ex_new_index)
RESOLVEFUNC(SSL_set_ex_data)
RESOLVEFUNC(SSL_get_ex_data)
RESOLVEFUNC(SSL_get_ex_data_X509_STORE_CTX_idx)
#endif
#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_PSK)
RESOLVEFUNC(SSL_set_psk_client_callback)
Expand Down Expand Up @@ -1012,6 +1027,10 @@ bool q_resolveOpenSslSymbols()
RESOLVEFUNC(X509_STORE_free)
RESOLVEFUNC(X509_STORE_new)
RESOLVEFUNC(X509_STORE_add_cert)
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
RESOLVEFUNC(X509_STORE_get_ex_data)
RESOLVEFUNC(X509_STORE_set_ex_data)
#endif
RESOLVEFUNC(X509_STORE_CTX_free)
RESOLVEFUNC(X509_STORE_CTX_init)
RESOLVEFUNC(X509_STORE_CTX_new)
Expand All @@ -1020,6 +1039,11 @@ bool q_resolveOpenSslSymbols()
RESOLVEFUNC(X509_STORE_CTX_get_error_depth)
RESOLVEFUNC(X509_STORE_CTX_get_current_cert)
RESOLVEFUNC(X509_STORE_CTX_get_chain)
RESOLVEFUNC(X509_STORE_CTX_get0_store)
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
RESOLVEFUNC(X509_STORE_CTX_get_ex_data)
RESOLVEFUNC(X509_STORE_CTX_set_ex_data)
#endif
RESOLVEFUNC(X509_cmp)
#ifndef SSLEAY_MACROS
RESOLVEFUNC(X509_dup)
Expand Down
18 changes: 18 additions & 0 deletions src/network/ssl/qsslsocket_openssl_symbols_p.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,12 @@ int q_EC_GROUP_get_degree(const EC_GROUP* g);
int q_CRYPTO_num_locks();
void q_CRYPTO_set_locking_callback(void (*a)(int, int, const char *, int));
void q_CRYPTO_set_id_callback(unsigned long (*a)());
int q_CRYPTO_set_ex_data(CRYPTO_EX_DATA *r, int idx, void *arg);
void *q_CRYPTO_get_ex_data(CRYPTO_EX_DATA *r, int idx);
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
int q_CRYPTO_get_ex_new_index(int class_index, long arg1, void *argp,
CRYPTO_EX_new *new_func, CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
void q_CRYPTO_free(void *a, const char *b, int c);
#else
void q_CRYPTO_free(void *a);
Expand Down Expand Up @@ -380,6 +385,7 @@ SSL_SESSION *q_SSL_get_session(const SSL *ssl);
int q_SSL_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func, CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);
int q_SSL_set_ex_data(SSL *ssl, int idx, void *arg);
void *q_SSL_get_ex_data(const SSL *ssl, int idx);
int q_SSL_get_ex_data_X509_STORE_CTX_idx();
#endif
#if OPENSSL_VERSION_NUMBER >= 0x10001000L && !defined(OPENSSL_NO_PSK)
typedef unsigned int (*q_psk_client_callback_t)(SSL *ssl, const char *hint, char *identity, unsigned int max_identity_len, unsigned char *psk, unsigned int max_psk_len);
Expand Down Expand Up @@ -467,6 +473,12 @@ EVP_PKEY *q_X509_PUBKEY_get(X509_PUBKEY *a);
void q_X509_STORE_free(X509_STORE *store);
X509_STORE *q_X509_STORE_new();
int q_X509_STORE_add_cert(X509_STORE *ctx, X509 *x);

#if OPENSSL_VERSION_NUMBER >= 0x10100000L
int q_X509_STORE_set_ex_data(X509_STORE *d, int idx, void *data);
void *q_X509_STORE_get_ex_data(X509_STORE *d, int idx);
#endif

void q_X509_STORE_CTX_free(X509_STORE_CTX *storeCtx);
int q_X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store,
X509 *x509, STACK_OF(X509) *chain);
Expand All @@ -476,6 +488,12 @@ int q_X509_STORE_CTX_get_error(X509_STORE_CTX *ctx);
int q_X509_STORE_CTX_get_error_depth(X509_STORE_CTX *ctx);
X509 *q_X509_STORE_CTX_get_current_cert(X509_STORE_CTX *ctx);
STACK_OF(X509) *q_X509_STORE_CTX_get_chain(X509_STORE_CTX *ctx);
X509_STORE *q_X509_STORE_CTX_get0_store(X509_STORE_CTX *ctx);

#if OPENSSL_VERSION_NUMBER >= 0x10100000L
int q_X509_STORE_CTX_set_ex_data(X509_STORE_CTX *d, int idx, void *data);
void *q_X509_STORE_CTX_get_ex_data(X509_STORE_CTX *d, int idx);
#endif

// Diffie-Hellman support
DH *q_DH_new();
Expand Down