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>
This commit is contained in:
Timur Pocheptsov 2018-07-12 16:31:20 +02:00
parent 9ce4006565
commit 8e1e275f8f
2 changed files with 60 additions and 11 deletions

View File

@ -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, &currentState);
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;
}
}

View File

@ -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)
};