diff options
author | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2018-07-12 16:31:20 +0200 |
---|---|---|
committer | Timur Pocheptsov <timur.pocheptsov@qt.io> | 2018-07-27 11:53:06 +0000 |
commit | 8e1e275f8f45fab9d035e86c9caba1f03db43373 (patch) | |
tree | e30f72b0a049745f8108fff0934d23cc48e83879 /src/network/ssl | |
parent | 9ce4006565c4fd4420f01e4c2d767b67b89698da (diff) |
SecureTransport - implement renegotiations
After a handshake was completed, TLS socket is in 'connectionEncrypted' state.
So on a read notification, in 'transmit', we call 'SSLRead' to read supposedly
encrypted application data or TLS internal messages. In case SSLRead finds either
ClientHello or HelloRequest from a server, it attempts in a rather sneaky manner
to renegotiate. And as it happens here and there with SecureTransport, SSLRead
fails and the work is only half-done, since we have kSSLSessionOptionBreakOnServerAuth
and kSSLSessionOptionBreakOnCertRequested options set to 'true'. We end up with
completely unexpected errors like errSSLClientCertRequested or errSSLPeerAuthCompleted
(yes, this is so normal and totally expected for 'SSLRead' function to verify
certificates and WRITE messages, no need to document this at all!).
If SecureTransport is sneaky, so can be us:
- in a read callback SecureTransport is probing the type of record
and we can notice a sudden session state change - it goes from
kSSLConnected (which is set upon handshake completion) to
kSSLHandshake (which means a (re)handshake is ongoing);
- if this is the case - we lie to SecureTransport about the amount
of data available (0 bytes), set 'renegotiating' to 'true', return
errSSLWouldBlock;
- in 'transmit', if SSLRead returns errSSLWouldBlock and 'renegotiating'
was set, we call 'startHandshake' until isHandshakeComplete() == true
or some error encountered.
[ChangeLog][QtNetwork][QSslSocket] Implement renegotiation for SecureTransport backend
Task-number: QTBUG-69420
Change-Id: Iaab1336aa3abf3f6ac94b358f3142d2738a18ee9
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Diffstat (limited to 'src/network/ssl')
-rw-r--r-- | src/network/ssl/qsslsocket_mac.cpp | 64 | ||||
-rw-r--r-- | src/network/ssl/qsslsocket_mac_p.h | 7 |
2 files changed, 60 insertions, 11 deletions
diff --git a/src/network/ssl/qsslsocket_mac.cpp b/src/network/ssl/qsslsocket_mac.cpp index 406748d9f5..8aa01deee7 100644 --- a/src/network/ssl/qsslsocket_mac.cpp +++ b/src/network/ssl/qsslsocket_mac.cpp @@ -48,6 +48,7 @@ #include <QtCore/qmessageauthenticationcode.h> #include <QtCore/qoperatingsystemversion.h> +#include <QtCore/qscopedvaluerollback.h> #include <QtCore/qcryptographichash.h> #include <QtCore/qsystemdetection.h> #include <QtCore/qdatastream.h> @@ -231,12 +232,35 @@ static const uint8_t dhparam[] = "\x90\x0b\x35\x64\xff\xd9\xe3\xac\xf2\xf2\xeb\x3a\x63\x02\x01\x02"; #endif -static OSStatus _q_SSLRead(QTcpSocket *plainSocket, char *data, size_t *dataLength) +OSStatus QSslSocketBackendPrivate::ReadCallback(QSslSocketBackendPrivate *socket, + char *data, size_t *dataLength) { - Q_ASSERT(plainSocket); + Q_ASSERT(socket); Q_ASSERT(data); Q_ASSERT(dataLength); + QTcpSocket *plainSocket = socket->plainSocket; + Q_ASSERT(plainSocket); + + if (socket->isHandshakeComplete()) { + // Check if it's a renegotiation attempt, when the handshake is complete, the + // session state is 'kSSLConnected': + SSLSessionState currentState = kSSLConnected; + const OSStatus result = SSLGetSessionState(socket->context, ¤tState); + if (result != noErr) { + *dataLength = 0; + return result; + } + + if (currentState == kSSLHandshake) { + // Renegotiation detected, don't allow read more yet - 'transmit' + // will notice this and will call 'startHandshake': + *dataLength = 0; + socket->renegotiating = true; + return errSSLWouldBlock; + } + } + const qint64 bytes = plainSocket->read(data, *dataLength); #ifdef QSSLSOCKET_DEBUG qCDebug(lcSsl) << plainSocket << "read" << bytes; @@ -252,12 +276,16 @@ static OSStatus _q_SSLRead(QTcpSocket *plainSocket, char *data, size_t *dataLeng return err; } -static OSStatus _q_SSLWrite(QTcpSocket *plainSocket, const char *data, size_t *dataLength) +OSStatus QSslSocketBackendPrivate::WriteCallback(QSslSocketBackendPrivate *socket, + const char *data, size_t *dataLength) { - Q_ASSERT(plainSocket); + Q_ASSERT(socket); Q_ASSERT(data); Q_ASSERT(dataLength); + QTcpSocket *plainSocket = socket->plainSocket; + Q_ASSERT(plainSocket); + const qint64 bytes = plainSocket->write(data, *dataLength); #ifdef QSSLSOCKET_DEBUG qCDebug(lcSsl) << plainSocket << "write" << bytes; @@ -407,7 +435,9 @@ void QSslSocketBackendPrivate::continueHandshake() } #endif // QT_DARWIN_PLATFORM_SDK_EQUAL_OR_ABOVE - emit q->encrypted(); + if (!renegotiating) + emit q->encrypted(); + if (autoStartHandshake && pendingClose) { pendingClose = false; q->disconnectFromHost(); @@ -505,10 +535,10 @@ void QSslSocketBackendPrivate::transmit() if (!context || shutdown) return; - if (!connectionEncrypted) + if (!isHandshakeComplete()) startHandshake(); - if (connectionEncrypted && !writeBuffer.isEmpty()) { + if (isHandshakeComplete() && !writeBuffer.isEmpty()) { qint64 totalBytesWritten = 0; while (writeBuffer.nextDataBlockSize() > 0 && context) { const size_t nextDataBlockSize = writeBuffer.nextDataBlockSize(); @@ -543,7 +573,7 @@ void QSslSocketBackendPrivate::transmit() } } - if (connectionEncrypted) { + if (isHandshakeComplete()) { QVarLengthArray<char, 4096> data; while (context && (!readBufferMaxSize || buffer.size() < readBufferMaxSize)) { size_t readBytes = 0; @@ -563,6 +593,11 @@ void QSslSocketBackendPrivate::transmit() break; } + if (err == errSSLWouldBlock && renegotiating) { + startHandshake(); + break; + } + if (readBytes) { buffer.append(data.constData(), readBytes); if (readyReadEmittedPointer) @@ -845,8 +880,9 @@ bool QSslSocketBackendPrivate::initSslContext() return false; } - const OSStatus err = SSLSetIOFuncs(context, reinterpret_cast<SSLReadFunc>(&_q_SSLRead), - reinterpret_cast<SSLWriteFunc>(&_q_SSLWrite)); + const OSStatus err = SSLSetIOFuncs(context, + reinterpret_cast<SSLReadFunc>(&QSslSocketBackendPrivate::ReadCallback), + reinterpret_cast<SSLWriteFunc>(&QSslSocketBackendPrivate::WriteCallback)); if (err != errSecSuccess) { destroySslContext(); setErrorAndEmit(QAbstractSocket::SslInternalError, @@ -854,7 +890,7 @@ bool QSslSocketBackendPrivate::initSslContext() return false; } - SSLSetConnection(context, plainSocket); + SSLSetConnection(context, this); if (mode == QSslSocket::SslServerMode && !configuration.localCertificateChain.isEmpty()) { @@ -1406,6 +1442,7 @@ bool QSslSocketBackendPrivate::startHandshake() // Failure means a real error (invalid certificate, no private key, etc). if (!setSessionCertificate(errorDescription, errorCode)) { setErrorAndEmit(errorCode, errorDescription); + renegotiating = false; return false; } else { // We try to resume a handshake, even if have no @@ -1420,6 +1457,7 @@ bool QSslSocketBackendPrivate::startHandshake() return startHandshake(); } + renegotiating = false; setErrorAndEmit(QAbstractSocket::SslHandshakeFailedError, QStringLiteral("SSLHandshake failed: %1").arg(err)); plainSocket->disconnectFromHost(); @@ -1429,6 +1467,7 @@ bool QSslSocketBackendPrivate::startHandshake() // Connection aborted during handshake phase. if (q->state() != QAbstractSocket::ConnectedState) { qCDebug(lcSsl) << "connection aborted"; + renegotiating = false; return false; } @@ -1437,13 +1476,16 @@ bool QSslSocketBackendPrivate::startHandshake() if (!verifySessionProtocol()) { setErrorAndEmit(QAbstractSocket::SslHandshakeFailedError, QStringLiteral("Protocol version mismatch")); plainSocket->disconnectFromHost(); + renegotiating = false; return false; } if (verifyPeerTrust()) { continueHandshake(); + renegotiating = false; return true; } else { + renegotiating = false; return false; } } diff --git a/src/network/ssl/qsslsocket_mac_p.h b/src/network/ssl/qsslsocket_mac_p.h index 34e30ebb16..e37171e56a 100644 --- a/src/network/ssl/qsslsocket_mac_p.h +++ b/src/network/ssl/qsslsocket_mac_p.h @@ -120,7 +120,14 @@ private: bool checkSslErrors(); bool startHandshake(); + bool isHandshakeComplete() const {return connectionEncrypted && !renegotiating;} + + // IO callbacks: + static OSStatus ReadCallback(QSslSocketBackendPrivate *socket, char *data, size_t *dataLength); + static OSStatus WriteCallback(QSslSocketBackendPrivate *plainSocket, const char *data, size_t *dataLength); + QSecureTransportContext context; + bool renegotiating = false; Q_DISABLE_COPY(QSslSocketBackendPrivate) }; |