From d82d2f67161ed12d94edcc79914ec74df4cd40d3 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 2 Mar 2017 17:41:06 +0100 Subject: QSslSocket: fix connection to a international domain name RFC6125 section 6.4.2 specify we need to convert the IDN to ascii before comparison. Note that we don't need to toLower anymore because toAce takes care of it. Section 7.2 recommands that we dod not attempt to check for wildcard character embedded within the A-labels or U-labels of an internationalized domain name. So we reject names that contiains a '*' but starts with 'xn--'. Change-Id: Ib0830520a1f82bbf9fd11818718277a479527ee3 Reviewed-by: Timur Pocheptsov --- src/network/ssl/qsslsocket.cpp | 25 ++++++++++++++++----- src/network/ssl/qsslsocket_p.h | 3 ++- .../ssl/qsslsocket/certs/xn--schufele-2za.crt | 11 +++++++++ .../auto/network/ssl/qsslsocket/tst_qsslsocket.cpp | 26 ++++++++++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 tests/auto/network/ssl/qsslsocket/certs/xn--schufele-2za.crt diff --git a/src/network/ssl/qsslsocket.cpp b/src/network/ssl/qsslsocket.cpp index 2fc779b257..166907780b 100644 --- a/src/network/ssl/qsslsocket.cpp +++ b/src/network/ssl/qsslsocket.cpp @@ -326,6 +326,7 @@ #include #include #include +#include #include #include #include @@ -2673,31 +2674,35 @@ QSharedPointer QSslSocketPrivate::sslContext(QSslSocket *socket) bool QSslSocketPrivate::isMatchingHostname(const QSslCertificate &cert, const QString &peerName) { - const QString lowerPeerName = peerName.toLower(); + const QString lowerPeerName = QString::fromLatin1(QUrl::toAce(peerName)); const QStringList commonNames = cert.subjectInfo(QSslCertificate::CommonName); for (const QString &commonName : commonNames) { - if (isMatchingHostname(commonName.toLower(), lowerPeerName)) + if (isMatchingHostname(commonName, lowerPeerName)) return true; } const auto subjectAlternativeNames = cert.subjectAlternativeNames(); const auto altNames = subjectAlternativeNames.equal_range(QSsl::DnsEntry); for (auto it = altNames.first; it != altNames.second; ++it) { - if (isMatchingHostname(it->toLower(), lowerPeerName)) + if (isMatchingHostname(*it, lowerPeerName)) return true; } return false; } +/*! \internal + Checks if the certificate's name \a cn matches the \a hostname. + \a hostname must be normalized in ASCII-Compatible Encoding, but \a cn is not normalized + */ bool QSslSocketPrivate::isMatchingHostname(const QString &cn, const QString &hostname) { int wildcard = cn.indexOf(QLatin1Char('*')); // Check this is a wildcard cert, if not then just compare the strings if (wildcard < 0) - return cn == hostname; + return QLatin1String(QUrl::toAce(cn)) == hostname; int firstCnDot = cn.indexOf(QLatin1Char('.')); int secondCnDot = cn.indexOf(QLatin1Char('.'), firstCnDot+1); @@ -2714,13 +2719,21 @@ bool QSslSocketPrivate::isMatchingHostname(const QString &cn, const QString &hos if (cn.lastIndexOf(QLatin1Char('*')) != wildcard) return false; + // Reject wildcard character embedded within the A-labels or U-labels of an internationalized + // domain name (RFC6125 section 7.2) + if (cn.startsWith(QLatin1String("xn--"), Qt::CaseInsensitive)) + return false; + // Check characters preceding * (if any) match - if (wildcard && (hostname.leftRef(wildcard) != cn.leftRef(wildcard))) + if (wildcard && hostname.leftRef(wildcard).compare(cn.leftRef(wildcard), Qt::CaseInsensitive) != 0) return false; // Check characters following first . match - if (hostname.midRef(hostname.indexOf(QLatin1Char('.'))) != cn.midRef(firstCnDot)) + int hnDot = hostname.indexOf(QLatin1Char('.')); + if (hostname.midRef(hnDot + 1) != cn.midRef(firstCnDot + 1) + && hostname.midRef(hnDot + 1) != QLatin1String(QUrl::toAce(cn.mid(firstCnDot + 1)))) { return false; + } // Check if the hostname is an IP address, if so then wildcards are not allowed QHostAddress addr(hostname); diff --git a/src/network/ssl/qsslsocket_p.h b/src/network/ssl/qsslsocket_p.h index cec61d07c1..aec3437422 100644 --- a/src/network/ssl/qsslsocket_p.h +++ b/src/network/ssl/qsslsocket_p.h @@ -151,7 +151,8 @@ public: QRegExp::PatternSyntax syntax); static void addDefaultCaCertificate(const QSslCertificate &cert); static void addDefaultCaCertificates(const QList &certs); - static bool isMatchingHostname(const QSslCertificate &cert, const QString &peerName); + Q_AUTOTEST_EXPORT static bool isMatchingHostname(const QSslCertificate &cert, + const QString &peerName); Q_AUTOTEST_EXPORT static bool isMatchingHostname(const QString &cn, const QString &hostname); #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) diff --git a/tests/auto/network/ssl/qsslsocket/certs/xn--schufele-2za.crt b/tests/auto/network/ssl/qsslsocket/certs/xn--schufele-2za.crt new file mode 100644 index 0000000000..b34847b8d2 --- /dev/null +++ b/tests/auto/network/ssl/qsslsocket/certs/xn--schufele-2za.crt @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBfTCCASegAwIBAgIJANTX9XvqKXllMA0GCSqGSIb3DQEBCwUAMBoxGDAWBgNV +BAMMDyouU0NIw4RVRkVMRS5ERTAeFw0xNzAzMDcwOTIwMzZaFw0xNzA0MDYwOTIw +MzZaMBoxGDAWBgNVBAMMDyouU0NIw4RVRkVMRS5ERTBcMA0GCSqGSIb3DQEBAQUA +A0sAMEgCQQDZGJFl85dMxps7EryuNyP1apWrBqp7mlwgfbCN9wboyUZBLCWM58UK +6oJPg0X19IJUv3UTxD8GH7qQteCXPl4bAgMBAAGjUDBOMB0GA1UdDgQWBBSLuHtk +CfEpYnW5Rw7XcO53P/1+jDAfBgNVHSMEGDAWgBSLuHtkCfEpYnW5Rw7XcO53P/1+ +jDAMBgNVHRMEBTADAQH/MA0GCSqGSIb3DQEBCwUAA0EAXrGVzhGlekW5Zm78tASr +5lTF6wn3klqvd2zTqTKuAKWAuBNX46p79g8OGsfY76X9M+/RbUYa7h5BuTL9b45K +dw== +-----END CERTIFICATE----- diff --git a/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp b/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp index 4eb26d17fe..8a8522760c 100644 --- a/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp +++ b/tests/auto/network/ssl/qsslsocket/tst_qsslsocket.cpp @@ -191,6 +191,7 @@ private slots: void supportedCiphers(); void systemCaCertificates(); void wildcardCertificateNames(); + void isMatchingHostname(); void wildcard(); void setEmptyKey(); void spontaneousWrite(); @@ -1643,10 +1644,15 @@ void tst_QSslSocket::wildcardCertificateNames() { // Passing CN matches QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("www.example.com"), QString("www.example.com")), true ); + QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("WWW.EXAMPLE.COM"), QString("www.example.com")), true ); QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("*.example.com"), QString("www.example.com")), true ); QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("xxx*.example.com"), QString("xxxwww.example.com")), true ); QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("f*.example.com"), QString("foo.example.com")), true ); QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("192.168.0.0"), QString("192.168.0.0")), true ); + QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("foo.éxample.com"), QString("foo.xn--xample-9ua.com")), true ); + QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("*.éxample.com"), QString("foo.xn--xample-9ua.com")), true ); + QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("xn--kcry6tjko.example.org"), QString("xn--kcry6tjko.example.org")), true); + QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("*.xn--kcry6tjko.example.org"), QString("xn--kcr.xn--kcry6tjko.example.org")), true); // Failing CN matches QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("xxx.example.com"), QString("www.example.com")), false ); @@ -1661,6 +1667,26 @@ void tst_QSslSocket::wildcardCertificateNames() QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString(""), QString("www")), false ); QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("*"), QString("www")), false ); QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("*.168.0.0"), QString("192.168.0.0")), false ); + QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("xn--kcry6tjko*.example.org"), QString("xn--kcry6tjkoanc.example.org")), false ); // RFC 6125 §7.2 + QCOMPARE( QSslSocketPrivate::isMatchingHostname(QString("å*.example.org"), QString("xn--la-xia.example.org")), false ); +} + +void tst_QSslSocket::isMatchingHostname() +{ + // with normalization: (the certificate has *.SCHÄUFELE.DE as a CN) + // openssl req -x509 -nodes -subj "/CN=*.SCHÄUFELE.DE" -newkey rsa:512 -keyout /dev/null -out xn--schufele-2za.crt + QList certs = QSslCertificate::fromPath(SRCDIR "certs/xn--schufele-2za.crt"); + QVERIFY(!certs.isEmpty()); + QSslCertificate cert = certs.first(); + + QCOMPARE(QSslSocketPrivate::isMatchingHostname(cert, QString::fromUtf8("WWW.SCHÄUFELE.DE")), true); + QCOMPARE(QSslSocketPrivate::isMatchingHostname(cert, QString::fromUtf8("www.xn--schufele-2za.de")), true); + QCOMPARE(QSslSocketPrivate::isMatchingHostname(cert, QString::fromUtf8("www.schäufele.de")), true); + QCOMPARE(QSslSocketPrivate::isMatchingHostname(cert, QString::fromUtf8("föo.schäufele.de")), true); + + QCOMPARE(QSslSocketPrivate::isMatchingHostname(cert, QString::fromUtf8("foo.foo.xn--schufele-2za.de")), false); + QCOMPARE(QSslSocketPrivate::isMatchingHostname(cert, QString::fromUtf8("www.schaufele.de")), false); + QCOMPARE(QSslSocketPrivate::isMatchingHostname(cert, QString::fromUtf8("www.schufele.de")), false); } void tst_QSslSocket::wildcard() -- cgit v1.2.3