QDtls: delay protocol version verification
A weird behavior of the DTLS server example, when linked with 1.0.2, exposed that client code, requesting an invalid protocol (for example, SSLv3) can end-up with connection encrypted with DTLS 1.2 (which is not that bad, but totally surprising). When we check the protocol version early in setDtlsConfiguration() and find a wrong version, we leave our previous configuration intact and we will use it later during the handshake. This is wrong. So now we let our user set whatever wrong configuration they have and later fail in TLS initialization, saying - 'Unsupported protocol, DTLS was expected'. Auto-test was reduced - the follow-up patch will introduce a new 'invalidConfiguration' auto-test. Change-Id: I9be054c6112eea11b7801a1595aaf1d34329e1d2 Reviewed-by: Edward Welbourne <edward.welbourne@qt.io> Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
parent
26a6afd472
commit
ab731692e2
@ -333,19 +333,6 @@
|
||||
|
||||
QT_BEGIN_NAMESPACE
|
||||
|
||||
static bool isDtlsProtocol(QSsl::SslProtocol protocol)
|
||||
{
|
||||
switch (protocol) {
|
||||
case QSsl::DtlsV1_0:
|
||||
case QSsl::DtlsV1_0OrLater:
|
||||
case QSsl::DtlsV1_2:
|
||||
case QSsl::DtlsV1_2OrLater:
|
||||
return true;
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
QSslConfiguration QDtlsBasePrivate::configuration() const
|
||||
{
|
||||
auto copyPrivate = new QSslConfigurationPrivate(dtlsConfiguration);
|
||||
@ -368,7 +355,6 @@ void QDtlsBasePrivate::setConfiguration(const QSslConfiguration &configuration)
|
||||
dtlsConfiguration.caCertificates = configuration.caCertificates();
|
||||
dtlsConfiguration.peerVerifyDepth = configuration.peerVerifyDepth();
|
||||
dtlsConfiguration.peerVerifyMode = configuration.peerVerifyMode();
|
||||
Q_ASSERT(isDtlsProtocol(configuration.protocol()));
|
||||
dtlsConfiguration.protocol = configuration.protocol();
|
||||
dtlsConfiguration.sslOptions = configuration.d->sslOptions;
|
||||
dtlsConfiguration.sslSession = configuration.sessionTicket();
|
||||
@ -398,6 +384,19 @@ bool QDtlsBasePrivate::setCookieGeneratorParameters(QCryptographicHash::Algorith
|
||||
return true;
|
||||
}
|
||||
|
||||
bool QDtlsBasePrivate::isDtlsProtocol(QSsl::SslProtocol protocol)
|
||||
{
|
||||
switch (protocol) {
|
||||
case QSsl::DtlsV1_0:
|
||||
case QSsl::DtlsV1_0OrLater:
|
||||
case QSsl::DtlsV1_2:
|
||||
case QSsl::DtlsV1_2OrLater:
|
||||
return true;
|
||||
default:
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
static QString msgUnsupportedMulticastAddress()
|
||||
{
|
||||
return QDtls::tr("Multicast and broadcast addresses are not supported");
|
||||
@ -755,13 +754,8 @@ bool QDtls::setDtlsConfiguration(const QSslConfiguration &configuration)
|
||||
return false;
|
||||
}
|
||||
|
||||
if (isDtlsProtocol(configuration.protocol())) {
|
||||
d->setConfiguration(configuration);
|
||||
return true;
|
||||
}
|
||||
|
||||
d->setDtlsError(QDtlsError::InvalidInputParameters, tr("Unsupported protocol"));
|
||||
return false;
|
||||
d->setConfiguration(configuration);
|
||||
return true;
|
||||
}
|
||||
|
||||
/*!
|
||||
|
@ -721,7 +721,13 @@ bool DtlsState::initCtxAndConnection(QDtlsBasePrivate *dtlsBase)
|
||||
return false;
|
||||
}
|
||||
|
||||
// create a deep copy of our configuration
|
||||
if (!QDtlsBasePrivate::isDtlsProtocol(dtlsBase->dtlsConfiguration.protocol)) {
|
||||
dtlsBase->setDtlsError(QDtlsError::TlsInitializationError,
|
||||
QDtls::tr("Invalid protocol version, DTLS protocol expected"));
|
||||
return false;
|
||||
}
|
||||
|
||||
// Create a deep copy of our configuration
|
||||
auto configurationCopy = new QSslConfigurationPrivate(dtlsBase->dtlsConfiguration);
|
||||
configurationCopy->ref.store(0); // the QSslConfiguration constructor refs up
|
||||
|
||||
|
@ -96,6 +96,8 @@ public:
|
||||
bool setCookieGeneratorParameters(QCryptographicHash::Algorithm alg,
|
||||
const QByteArray &secret);
|
||||
|
||||
static bool isDtlsProtocol(QSsl::SslProtocol protocol);
|
||||
|
||||
QHostAddress remoteAddress;
|
||||
quint16 remotePort = 0;
|
||||
quint16 mtuHint = 0;
|
||||
|
@ -284,12 +284,6 @@ void tst_QDtls::configuration()
|
||||
QFETCH(const QSslSocket::SslMode, mode);
|
||||
QDtls dtls(mode);
|
||||
QCOMPARE(dtls.dtlsConfiguration(), config);
|
||||
// Default TLS (no 'D') configuration has a wrong protocol version:
|
||||
QCOMPARE(dtls.setDtlsConfiguration(QSslConfiguration::defaultConfiguration()), false);
|
||||
QCOMPARE(dtls.dtlsError(), QDtlsError::InvalidInputParameters);
|
||||
// The previous failure did not change our default configuration:
|
||||
QCOMPARE(dtls.dtlsConfiguration(), config);
|
||||
// Now set a valid (non-default) configuration:
|
||||
config.setProtocol(QSsl::DtlsV1_0OrLater);
|
||||
config.setDtlsCookieVerificationEnabled(false);
|
||||
QCOMPARE(config.dtlsCookieVerificationEnabled(), false);
|
||||
|
Loading…
x
Reference in New Issue
Block a user