QSslContext - do a little cleanup (OpenSSL)

1. Remove a useless forward declaration of a non-existing class.
2. Simplify the cipher filtering.
3. A missing private key (when local cert is present) found
   by the Qt, not OpenSSL, so no need in asking OpenSSL for errors
   in  queue.
3. Fix a potential double-free (for opaque keys).
4. read/write BIOs normally owned by SSL object, but if
   we fail to allocate any of them, we return early,
   potentially failing to free the one that was allocated.

Change-Id: Ifb52fbc9fd1a38f101bd7ff02e79b82d6eb7e5b0
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
This commit is contained in:
Timur Pocheptsov 2020-08-19 11:10:11 +02:00
parent de1d5f6a94
commit eb7d1cf098
3 changed files with 11 additions and 11 deletions

View File

@ -437,16 +437,13 @@ init_context:
auto filterCiphers = [](const QList<QSslCipher> &ciphers, bool selectTls13)
{
QByteArray cipherString;
bool first = true;
for (const QSslCipher &cipher : qAsConst(ciphers)) {
for (const QSslCipher &cipher : ciphers) {
const bool isTls13Cipher = cipher.protocol() == QSsl::TlsV1_3 || cipher.protocol() == QSsl::TlsV1_3OrLater;
if (selectTls13 != isTls13Cipher)
continue;
if (first)
first = false;
else
if (cipherString.size())
cipherString.append(':');
cipherString.append(cipher.name().toLatin1());
}
@ -530,7 +527,7 @@ init_context:
if (!sslContext->sslConfiguration.localCertificate().isNull()) {
// Require a private key as well.
if (sslContext->sslConfiguration.privateKey().isNull()) {
sslContext->errorStr = QSslSocket::tr("Cannot provide a certificate with no key, %1").arg(QSslSocketBackendPrivate::getErrorsFromOpenSsl());
sslContext->errorStr = QSslSocket::tr("Cannot provide a certificate with no key");
sslContext->errorCode = QSslError::UnspecifiedError;
return;
}
@ -559,14 +556,15 @@ init_context:
q_EVP_PKEY_set1_EC_KEY(sslContext->pkey, reinterpret_cast<EC_KEY *>(configuration.d->privateKey.handle()));
#endif
}
auto pkey = sslContext->pkey;
if (configuration.d->privateKey.algorithm() == QSsl::Opaque)
sslContext->pkey = nullptr; // Don't free the private key, it belongs to QSslKey
if (!q_SSL_CTX_use_PrivateKey(sslContext->ctx, sslContext->pkey)) {
if (!q_SSL_CTX_use_PrivateKey(sslContext->ctx, pkey)) {
sslContext->errorStr = QSslSocket::tr("Error loading private key, %1").arg(QSslSocketBackendPrivate::getErrorsFromOpenSsl());
sslContext->errorCode = QSslError::UnspecifiedError;
return;
}
if (configuration.d->privateKey.algorithm() == QSsl::Opaque)
sslContext->pkey = nullptr; // Don't free the private key, it belongs to QSslKey
// Check if the certificate matches the private key.
if (!q_SSL_CTX_check_private_key(sslContext->ctx)) {

View File

@ -63,8 +63,6 @@ QT_BEGIN_NAMESPACE
#ifndef QT_NO_SSL
class QSslContextPrivate;
class QSslContext
{
public:

View File

@ -747,6 +747,10 @@ bool QSslSocketBackendPrivate::initSslContext()
if (!readBio || !writeBio) {
setErrorAndEmit(QAbstractSocket::SslInternalError,
QSslSocket::tr("Error creating SSL session: %1").arg(getErrorsFromOpenSsl()));
if (readBio)
q_BIO_free(readBio);
if (writeBio)
q_BIO_free(writeBio);
return false;
}