From 0065b55da42b8c6ee0095264b5275fb708887c9d Mon Sep 17 00:00:00 2001 From: Daniel Molkentin Date: Fri, 2 May 2014 00:10:21 +0200 Subject: Ignore expired certificate during certificate validation OpenSSL has a bug when validating a chain with two certificates. If a certificate exists twice (which is a valid use case for renewed CAs), and the first one it hits is expired (which depends on the order on data structure internal to OpenSSL), it will fail to validate the chain. This is only a bandaid fix, which trades improved chain validation for error reporting accuracy. However given that reissuing of CA certs is a real problem that is only getting worse, this fix is needed. See also: https://www.openssl.org/docs/ssl/SSL_CTX_load_verify_locations.html#WARNINGS [ChangeLog][QtNetwork][QSslSocket] Added a workaround to an OpenSSL problem that may cause errors when the trust store contains two certificates of the issuing CA, one of which is expired. Task-number: QTBUG-38896 Change-Id: I8f17972ac94555648098624e470fff0eff2e7940 Reviewed-by: Richard J. Moore Reviewed-by: Frederik Gladhorn --- src/network/ssl/qsslcontext.cpp | 23 ++++++++++++----------- src/network/ssl/qsslsocket_openssl.cpp | 24 ++++++++++++------------ 2 files changed, 24 insertions(+), 23 deletions(-) (limited to 'src/network') diff --git a/src/network/ssl/qsslcontext.cpp b/src/network/ssl/qsslcontext.cpp index 9c68218062..f5e5352d5e 100644 --- a/src/network/ssl/qsslcontext.cpp +++ b/src/network/ssl/qsslcontext.cpp @@ -214,22 +214,23 @@ init_context: } // Add all our CAs to this store. - QList expiredCerts; foreach (const QSslCertificate &caCertificate, sslContext->sslConfiguration.caCertificates()) { - // add expired certs later, so that the - // valid ones are used before the expired ones - if (caCertificate.expiryDate() < QDateTime::currentDateTime()) { - expiredCerts.append(caCertificate); - } else { + // From https://www.openssl.org/docs/ssl/SSL_CTX_load_verify_locations.html: + // + // If several CA certificates matching the name, key identifier, and + // serial number condition are available, only the first one will be + // examined. This may lead to unexpected results if the same CA + // certificate is available with different expiration dates. If a + // ``certificate expired'' verification error occurs, no other + // certificate will be searched. Make sure to not have expired + // certificates mixed with valid ones. + // + // See also: QSslSocketBackendPrivate::verify() + if (caCertificate.expiryDate() >= QDateTime::currentDateTime()) { q_X509_STORE_add_cert(sslContext->ctx->cert_store, (X509 *)caCertificate.handle()); } } - // now add the expired certs - foreach (const QSslCertificate &caCertificate, expiredCerts) { - q_X509_STORE_add_cert(sslContext->ctx->cert_store, reinterpret_cast(caCertificate.handle())); - } - if (QSslSocketPrivate::s_loadRootCertsOnDemand && allowRootCertOnDemandLoading) { // tell OpenSSL the directories where to look up the root certs on demand QList unixDirs = QSslSocketPrivate::unixRootCertDirectories(); diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp index 1b3928fdfb..45e2fabfe9 100644 --- a/src/network/ssl/qsslsocket_openssl.cpp +++ b/src/network/ssl/qsslsocket_openssl.cpp @@ -1598,23 +1598,23 @@ QList QSslSocketBackendPrivate::verify(QList certifi setDefaultCaCertificates(defaultCaCertificates() + systemCaCertificates()); } - QList expiredCerts; - foreach (const QSslCertificate &caCertificate, QSslSocket::defaultCaCertificates()) { - // add expired certs later, so that the - // valid ones are used before the expired ones - if (caCertificate.expiryDate() < QDateTime::currentDateTime()) { - expiredCerts.append(caCertificate); - } else { + // From https://www.openssl.org/docs/ssl/SSL_CTX_load_verify_locations.html: + // + // If several CA certificates matching the name, key identifier, and + // serial number condition are available, only the first one will be + // examined. This may lead to unexpected results if the same CA + // certificate is available with different expiration dates. If a + // ``certificate expired'' verification error occurs, no other + // certificate will be searched. Make sure to not have expired + // certificates mixed with valid ones. + // + // See also: QSslContext::fromConfiguration() + if (caCertificate.expiryDate() >= QDateTime::currentDateTime()) { q_X509_STORE_add_cert(certStore, reinterpret_cast(caCertificate.handle())); } } - // now add the expired certs - foreach (const QSslCertificate &caCertificate, expiredCerts) { - q_X509_STORE_add_cert(certStore, reinterpret_cast(caCertificate.handle())); - } - QMutexLocker sslErrorListMutexLocker(&_q_sslErrorList()->mutex); // Register a custom callback to get all verification errors. -- cgit v1.2.3 From c8de2a8b5f5d0b9b3bc1d8ed8d3027ac40b00ee3 Mon Sep 17 00:00:00 2001 From: Kai Koehne Date: Thu, 8 May 2014 09:14:44 +0200 Subject: Fix MSVC warnings in qspdyprotocolhandler Fix warnings about 'truncation of constant value': qspdyprotocolhandler.cpp(583) : warning C4309: '=' : truncation of constant value qspdyprotocolhandler.cpp(656) : warning C4309: '=' : truncation of constant value qspdyprotocolhandler.cpp(659) : warning C4309: '=' : truncation of constant value Change-Id: I3c32b9f47c06da9b50f5c94871a2ee455b3a5cb6 Reviewed-by: Peter Hartmann --- src/network/access/qspdyprotocolhandler.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/network') diff --git a/src/network/access/qspdyprotocolhandler.cpp b/src/network/access/qspdyprotocolhandler.cpp index 6d22ebeb35..32a804e9f4 100644 --- a/src/network/access/qspdyprotocolhandler.cpp +++ b/src/network/access/qspdyprotocolhandler.cpp @@ -598,7 +598,7 @@ void QSpdyProtocolHandler::sendControlFrame(FrameType type, { // frame type and stream ID char header[8]; - header[0] = 0x80; // leftmost bit == 1 -> is a control frame + header[0] = 0x80u; // leftmost bit == 1 -> is a control frame header[1] = 0x03; // 3 bit == version 3 header[2] = 0; switch (type) { @@ -671,10 +671,10 @@ void QSpdyProtocolHandler::sendSYN_STREAM(HttpMessagePair messagePair, prioAndSlot[0] = 0x00; // == prio 0 (highest) break; case QHttpNetworkRequest::NormalPriority: - prioAndSlot[0] = 0x80; // == prio 4 + prioAndSlot[0] = 0x80u; // == prio 4 break; case QHttpNetworkRequest::LowPriority: - prioAndSlot[0] = 0xe0; // == prio 7 (lowest) + prioAndSlot[0] = 0xe0u; // == prio 7 (lowest) break; } prioAndSlot[1] = 0x00; // slot in client certificates (not supported currently) -- cgit v1.2.3