QDnsLookup: implement DNS-over-TLS

For the libresolv (Unix) implementation, we already had the packet
prepared by res_nmkquery(). This commit moves the res_nsend() to a
separate function so QDnsLookupRunnable::query() can be more concise.

On the Windows side, this commit creates a separate function for the DoT
case, because we now need to use two other functions from WinDNS so we
can create a query and parse the reply.

The rest is just QSslSocket.

Change-Id: I455fe22ef4ad4b2f9b01fffd17c805a3cb0466eb
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

src/network/kernel/qdnslookup.cpp

@@ -8,14 +8,22 @@
 #include <qapplicationstatic.h>
 #include <qcoreapplication.h>
 #include <qdatetime.h>
+#include <qendian.h>
 #include <qloggingcategory.h>
 #include <qrandom.h>
+#include <qspan.h>
 #include <qurl.h>
 
+#if QT_CONFIG(ssl)
+#  include <qsslsocket.h>
+#endif
+
 #include <algorithm>
 
 QT_BEGIN_NAMESPACE
 
+using namespace Qt::StringLiterals;
+
 static Q_LOGGING_CATEGORY(lcDnsLookup, "qt.network.dnslookup", QtCriticalMsg)
 
 namespace {
@@ -261,6 +269,10 @@ bool QDnsLookup::isProtocolSupported(Protocol protocol)
     case QDnsLookup::Standard:
         return true;
     case QDnsLookup::DnsOverTls:
+#  if QT_CONFIG(ssl)
+        if (QSslSocket::supportsSsl())
+            return true;
+#  endif
         return false;
     }
 #else
@@ -658,7 +670,8 @@ QList<QDnsTextRecord> QDnsLookup::textRecords() const
 */
 void QDnsLookup::setSslConfiguration(const QSslConfiguration &sslConfiguration)
 {
-    Q_UNUSED(sslConfiguration)
+    Q_D(QDnsLookup);
+    d->sslConfiguration.emplace(sslConfiguration);
 }
 
 /*!
@@ -668,7 +681,8 @@ void QDnsLookup::setSslConfiguration(const QSslConfiguration &sslConfiguration)
 */
 QSslConfiguration QDnsLookup::sslConfiguration() const
 {
-    return {};
+    const Q_D(QDnsLookup);
+    return d->sslConfiguration.value_or(QSslConfiguration::defaultConfiguration());
 }
 #endif
 
@@ -1291,6 +1305,82 @@ inline QDebug operator<<(QDebug &d, QDnsLookupRunnable *r)
     return d;
 }
 
+#if QT_CONFIG(ssl)
+static constexpr std::chrono::milliseconds DnsOverTlsConnectTimeout(15'000);
+static constexpr std::chrono::milliseconds DnsOverTlsTimeout(120'000);
+
+static int makeReplyErrorFromSocket(QDnsLookupReply *reply, const QAbstractSocket *socket)
+{
+    QDnsLookup::Error error = [&] {
+        switch (socket->error()) {
+        case QAbstractSocket::SocketTimeoutError:
+        case QAbstractSocket::ProxyConnectionTimeoutError:
+            return QDnsLookup::TimeoutError;
+        default:
+            return QDnsLookup::ResolverError;
+        }
+    }();
+    reply->setError(error, socket->errorString());
+    return false;
+}
+
+bool QDnsLookupRunnable::sendDnsOverTls(QDnsLookupReply *reply, QSpan<unsigned char> query,
+                                        ReplyBuffer &response)
+{
+    QSslSocket socket;
+    socket.setSslConfiguration(sslConfiguration.value_or(QSslConfiguration::defaultConfiguration()));
+
+#  if QT_CONFIG(networkproxy)
+    socket.setProtocolTag("domain-s"_L1);
+#  endif
+
+    do {
+        quint16 size = qToBigEndian<quint16>(query.size());
+        QDeadlineTimer timeout(DnsOverTlsTimeout);
+
+        socket.connectToHostEncrypted(nameserver.toString(), port);
+        socket.write(reinterpret_cast<const char *>(&size), sizeof(size));
+        socket.write(reinterpret_cast<const char *>(query.data()), query.size());
+        if (!socket.waitForEncrypted(DnsOverTlsConnectTimeout.count()))
+            break;
+
+        reply->sslConfiguration = socket.sslConfiguration();
+
+        // accumulate reply
+        auto waitForBytes = [&](void *buffer, int count) {
+            int remaining = timeout.remainingTime();
+            while (remaining >= 0 && socket.bytesAvailable() < count) {
+                if (!socket.waitForReadyRead(remaining))
+                    return false;
+            }
+            return socket.read(static_cast<char *>(buffer), count) == count;
+        };
+        if (!waitForBytes(&size, sizeof(size)))
+            break;
+
+        // note: strictly speaking, we're allocating memory based on untrusted data
+        // but in practice, due to limited range of the data type (16 bits),
+        // the maximum allocation is small.
+        size = qFromBigEndian(size);
+        response.resize(size);
+        if (waitForBytes(response.data(), size))
+            return true;
+    } while (false);
+
+    // handle errors
+    return makeReplyErrorFromSocket(reply, &socket);
+}
+#else
+bool QDnsLookupRunnable::sendDnsOverTls(QDnsLookupReply *reply, QSpan<unsigned char> query,
+                                        ReplyBuffer &response)
+{
+    Q_UNUSED(query)
+    Q_UNUSED(response)
+    reply->setError(QDnsLookup::ResolverError, QDnsLookup::tr("SSL/TLS support not present"));
+    return false;
+}
+#endif
+
 QT_END_NAMESPACE
 
 #include "moc_qdnslookup.cpp"

src/network/kernel/qdnslookup_p.h

@@ -201,9 +201,13 @@ public:
 #else
     using EncodedLabel = QByteArray;
 #endif
+    // minimum IPv6 MTU (1280) minus the IPv6 (40) and UDP headers (8)
+    static constexpr qsizetype ReplyBufferSize = 1280 - 40 - 8;
+    using ReplyBuffer = QVarLengthArray<unsigned char, ReplyBufferSize>;
 
     QDnsLookupRunnable(const QDnsLookupPrivate *d);
     void run() override;
+    bool sendDnsOverTls(QDnsLookupReply *reply, QSpan<unsigned char> query, ReplyBuffer &response);
 
 signals:
     void finished(const QDnsLookupReply &reply);

src/network/kernel/qdnslookup_unix.cpp

@@ -6,6 +6,7 @@
 
 #include <qendian.h>
 #include <qscopedpointer.h>
+#include <qspan.h>
 #include <qurl.h>
 #include <qvarlengtharray.h>
 #include <private/qnativesocketengine_p.h>      // for setSockAddr
@@ -32,15 +33,13 @@ QT_REQUIRE_CONFIG(libresolv);
 QT_BEGIN_NAMESPACE
 
 using namespace Qt::StringLiterals;
-
+using ReplyBuffer = QDnsLookupRunnable::ReplyBuffer;
-// minimum IPv6 MTU (1280) minus the IPv6 (40) and UDP headers (8)
-static constexpr qsizetype ReplyBufferSize = 1280 - 40 - 8;
 
 // https://www.rfc-editor.org/rfc/rfc6891
 static constexpr unsigned char Edns0Record[] = {
     0x00,                                           // root label
     T_OPT >> 8, T_OPT & 0xff,                       // type OPT
-    ReplyBufferSize >> 8, ReplyBufferSize & 0xff,   // payload size
+    ReplyBuffer::PreallocatedSize >> 8, ReplyBuffer::PreallocatedSize & 0xff,   // payload size
     NOERROR,                                        // extended rcode
     0,                                              // version
     0x00, 0x00,                                     // flags
@@ -153,40 +152,19 @@ prepareQueryBuffer(res_state state, QueryBuffer &buffer, const char *label, ns_r
     return queryLength + sizeof(Edns0Record);
 }
 
-void QDnsLookupRunnable::query(QDnsLookupReply *reply)
+static int sendStandardDns(QDnsLookupReply *reply, res_state state, QSpan<unsigned char> qbuffer,
+                           ReplyBuffer &buffer, const QHostAddress &nameserver, quint16 port)
 {
-    if (protocol != QDnsLookup::Standard)
-        return reply->setError(QDnsLookup::ResolverError,
-                               QDnsLookup::tr("DNS over TLS not implemented"));
-
-    // Initialize state.
-    std::remove_pointer_t<res_state> state = {};
-    if (res_ninit(&state) < 0) {
-        int error = errno;
-        qErrnoWarning(error, "QDnsLookup: Resolver initialization failed");
-        return reply->makeResolverSystemError(error);
-    }
-    auto guard = qScopeGuard([&] { res_nclose(&state); });
-
     //Check if a nameserver was set. If so, use it
-    if (!applyNameServer(&state, nameserver, port))
+    if (!applyNameServer(state, nameserver, port)) {
-        return reply->setError(QDnsLookup::ResolverError,
+        reply->setError(QDnsLookup::ResolverError,
                         QDnsLookup::tr("IPv6 nameservers are currently not supported on this OS"));
-#ifdef QDNSLOOKUP_DEBUG
+        return -1;
-    state.options |= RES_DEBUG;
+    }
-#endif
 
-    // Prepare the DNS query.
-    QueryBuffer qbuffer;
-    int queryLength = prepareQueryBuffer(&state, qbuffer, requestName.constData(), ns_rcode(requestType));
-    if (Q_UNLIKELY(queryLength < 0))
-        return reply->makeResolverSystemError();
-
-    // Perform DNS query.
-    QVarLengthArray<unsigned char, ReplyBufferSize> buffer(ReplyBufferSize);
     auto attemptToSend = [&]() {
         std::memset(buffer.data(), 0, HFIXEDSZ);        // the header is enough
-        int responseLength = res_nsend(&state, qbuffer.data(), queryLength, buffer.data(), buffer.size());
+        int responseLength = res_nsend(state, qbuffer.data(), qbuffer.size(), buffer.data(), buffer.size());
         if (responseLength >= 0)
             return responseLength;  // success
 
@@ -206,10 +184,10 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply)
     };
 
     // strictly use UDP, we'll deal with truncated replies ourselves
-    state.options |= RES_IGNTC;
+    state->options |= RES_IGNTC;
     int responseLength = attemptToSend();
     if (responseLength < 0)
-        return;
+        return responseLength;
 
     // check if we need to use the virtual circuit (TCP)
     auto header = reinterpret_cast<HEADER *>(buffer.data());
@@ -220,17 +198,56 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply)
 
         // remove the EDNS record in the query
         reinterpret_cast<HEADER *>(qbuffer.data())->arcount = 0;
-        queryLength -= sizeof(Edns0Record);
+        qbuffer = qbuffer.first(qbuffer.size() - sizeof(Edns0Record));
 
         // send using the virtual circuit
-        state.options |= RES_USEVC;
+        state->options |= RES_USEVC;
         responseLength = attemptToSend();
         if (Q_UNLIKELY(responseLength > buffer.size())) {
             // Ok, we give up.
-            return reply->setError(QDnsLookup::ResolverError,
+            reply->setError(QDnsLookup::ResolverError, QDnsLookup::tr("Reply was too large"));
-                                   QDnsLookup::tr("Reply was too large"));
+            return -1;
         }
     }
+
+    return responseLength;
+}
+
+void QDnsLookupRunnable::query(QDnsLookupReply *reply)
+{
+    // Initialize state.
+    std::remove_pointer_t<res_state> state = {};
+    if (res_ninit(&state) < 0) {
+        int error = errno;
+        qErrnoWarning(error, "QDnsLookup: Resolver initialization failed");
+        return reply->makeResolverSystemError(error);
+    }
+    auto guard = qScopeGuard([&] { res_nclose(&state); });
+
+#ifdef QDNSLOOKUP_DEBUG
+    state.options |= RES_DEBUG;
+#endif
+
+    // Prepare the DNS query.
+    QueryBuffer qbuffer;
+    int queryLength = prepareQueryBuffer(&state, qbuffer, requestName.constData(), ns_rcode(requestType));
+    if (Q_UNLIKELY(queryLength < 0))
+        return reply->makeResolverSystemError();
+
+    // Perform DNS query.
+    ReplyBuffer buffer(ReplyBufferSize);
+    int responseLength = -1;
+    switch (protocol) {
+    case QDnsLookup::Standard:
+        responseLength = sendStandardDns(reply, &state, qbuffer, buffer, nameserver, port);
+        break;
+    case QDnsLookup::DnsOverTls:
+        if (!sendDnsOverTls(reply, qbuffer, buffer))
+            return;
+        responseLength = buffer.size();
+        break;
+    }
+
     if (responseLength < 0)
         return;
 
@@ -239,6 +256,7 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply)
         return reply->makeInvalidReplyError();
 
     // Parse the reply.
+    auto header = reinterpret_cast<HEADER *>(buffer.data());
     if (header->rcode)
         return reply->makeDnsRcodeError(header->rcode);
 

src/network/kernel/qdnslookup_win.cpp

@@ -5,9 +5,11 @@
 #include <winsock2.h>
 #include "qdnslookup_p.h"
 
-#include <qurl.h>
+#include <qendian.h>
 #include <private/qnativesocketengine_p.h>
 #include <private/qsystemerror_p.h>
+#include <qurl.h>
+#include <qspan.h>
 
 #include <qt_windows.h>
 #include <windns.h>
@@ -63,6 +65,58 @@ DNS_STATUS WINAPI DnsQueryEx(PDNS_QUERY_REQUEST pQueryRequest,
 
 QT_BEGIN_NAMESPACE
 
+static DNS_STATUS sendAlternate(QDnsLookupRunnable *self, QDnsLookupReply *reply,
+                                PDNS_QUERY_REQUEST request, PDNS_QUERY_RESULT results)
+{
+    // WinDNS wants MTU - IP Header - UDP header for some reason, in spite
+    // of never needing that much
+    QVarLengthArray<unsigned char, 1472> query(1472);
+
+    auto dnsBuffer = new (query.data()) DNS_MESSAGE_BUFFER;
+    DWORD dnsBufferSize = query.size();
+    WORD xid = 0;
+    bool recursionDesired = true;
+
+    SetLastError(ERROR_SUCCESS);
+
+    // MinGW winheaders incorrectly declare the third parameter as LPWSTR
+    if (!DnsWriteQuestionToBuffer_W(dnsBuffer, &dnsBufferSize,
+                                    const_cast<LPWSTR>(request->QueryName), request->QueryType,
+                                    xid, recursionDesired)) {
+        // let's try reallocating
+        query.resize(dnsBufferSize);
+        if (!DnsWriteQuestionToBuffer_W(dnsBuffer, &dnsBufferSize,
+                                        const_cast<LPWSTR>(request->QueryName), request->QueryType,
+                                        xid, recursionDesired)) {
+            return GetLastError();
+        }
+    }
+
+    // set AD bit: we want to trust this server
+    dnsBuffer->MessageHead.AuthenticatedData = true;
+
+    QDnsLookupRunnable::ReplyBuffer replyBuffer;
+    if (!self->sendDnsOverTls(reply, { query.data(), dnsBufferSize }, replyBuffer))
+        return DNS_STATUS(-1);   // error set in reply
+
+    // interpret the RCODE in the reply
+    auto response = reinterpret_cast<PDNS_MESSAGE_BUFFER>(replyBuffer.data());
+    DNS_HEADER *header = &response->MessageHead;
+    if (!header->IsResponse)
+        return DNS_ERROR_BAD_PACKET;        // not a reply
+
+    // Convert the byte order for the 16-bit quantities in the header, so
+    // DnsExtractRecordsFromMessage can parse the contents.
+    //header->Xid = qFromBigEndian(header->Xid);
+    header->QuestionCount = qFromBigEndian(header->QuestionCount);
+    header->AnswerCount = qFromBigEndian(header->AnswerCount);
+    header->NameServerCount = qFromBigEndian(header->NameServerCount);
+    header->AdditionalCount = qFromBigEndian(header->AdditionalCount);
+
+    results->QueryOptions = request->QueryOptions;
+    return DnsExtractRecordsFromMessage_W(response, replyBuffer.size(), &results->pQueryRecords);
+}
+
 void QDnsLookupRunnable::query(QDnsLookupReply *reply)
 {
     // Perform DNS query.
@@ -93,10 +147,12 @@ void QDnsLookupRunnable::query(QDnsLookupReply *reply)
         status = DnsQueryEx(&request, &results, nullptr);
         break;
     case QDnsLookup::DnsOverTls:
-        return reply->setError(QDnsLookup::ResolverError,
+        status = sendAlternate(this, reply, &request, &results);
-                               QDnsLookup::tr("DNS over TLS not implemented"));
+        break;
     }
 
+    if (status == DNS_STATUS(-1))
+        return;     // error already set in reply
     if (status >= DNS_ERROR_RCODE_FORMAT_ERROR && status <= DNS_ERROR_RCODE_LAST)
         return reply->makeDnsRcodeError(status - DNS_ERROR_RCODE_FORMAT_ERROR + 1);
     else if (status == ERROR_TIMEOUT)

tests/auto/network/kernel/qdnslookup/tst_qdnslookup.cpp

@@ -13,6 +13,13 @@
 #include <QtNetwork/QNetworkDatagram>
 #include <QtNetwork/QUdpSocket>
 
+#if QT_CONFIG(networkproxy)
+#  include <QtNetwork/QNetworkProxyFactory>
+#endif
+#if QT_CONFIG(ssl)
+#  include <QtNetwork/QSslSocket>
+#endif
+
 #ifdef Q_OS_UNIX
 #  include <QtCore/QFile>
 #else
@@ -135,9 +142,57 @@ static QList<QHostAddress> globalPublicNameservers(QDnsLookup::Protocol proto)
         //"9.9.9.9", "2620:fe::9",
     };
 
+    auto udpSendAndReceive = [](const QHostAddress &addr, QByteArray &data) {
+        QUdpSocket socket;
+        socket.connectToHost(addr, 53);
+        if (socket.waitForConnected(1))
+            socket.write(data);
+
+        if (!socket.waitForReadyRead(1000))
+            return socket.errorString();
+
+        QNetworkDatagram dgram = socket.receiveDatagram();
+        if (!dgram.isValid())
+            return socket.errorString();
+
+        data = dgram.data();
+        return QString();
+    };
+
+    auto tlsSendAndReceive = [](const QHostAddress &addr, QByteArray &data) {
+#if QT_CONFIG(ssl)
+        QSslSocket socket;
+        QDeadlineTimer timeout(2000);
+        socket.connectToHostEncrypted(addr.toString(), 853);
+        if (!socket.waitForEncrypted(2000))
+            return socket.errorString();
+
+        quint16 size = qToBigEndian<quint16>(data.size());
+        socket.write(reinterpret_cast<char *>(&size), sizeof(size));
+        socket.write(data);
+
+        if (!socket.waitForReadyRead(timeout.remainingTime()))
+            return socket.errorString();
+        if (socket.bytesAvailable() < 2)
+            return u"protocol error"_s;
+
+        socket.read(reinterpret_cast<char *>(&size), sizeof(size));
+        size = qFromBigEndian(size);
+
+        while (socket.bytesAvailable() < size) {
+            int remaining = timeout.remainingTime();
+            if (remaining < 0 || !socket.waitForReadyRead(remaining))
+                return socket.errorString();
+        }
+
+        data = socket.readAll();
+        return QString();
+#else
+        return u"SSL/TLS support not compiled in"_s;
+#endif
+    };
+
     QList<QHostAddress> result;
-    if (proto != QDnsLookup::Standard)
-        return result;
     QRandomGenerator &rng = *QRandomGenerator::system();
     for (auto name : candidates) {
         // check the candidates for reachability
@@ -147,23 +202,18 @@ static QList<QHostAddress> globalPublicNameservers(QDnsLookup::Protocol proto)
         char *ptr = data.data();
         qToBigEndian(id, ptr);
 
-        QUdpSocket socket;
+        QString errorString = [&] {
-        socket.connectToHost(addr, 53);
+            switch (proto) {
-        if (socket.waitForConnected(1))
+            case QDnsLookup::Standard:      return udpSendAndReceive(addr, data);
-            socket.write(data);
+            case QDnsLookup::DnsOverTls:    return tlsSendAndReceive(addr, data);
-
+            }
-        if (!socket.waitForReadyRead(1000)) {
+            Q_UNREACHABLE();
-            qDebug() << addr << "discarded:" << socket.errorString();
+        }();
+        if (!errorString.isEmpty()) {
+            qDebug() << addr << "discarded:" << errorString;
             continue;
         }
 
-        QNetworkDatagram dgram = socket.receiveDatagram();
-        if (!dgram.isValid()) {
-            qDebug() << addr << "discarded:" << socket.errorString();
-            continue;
-        }
-
-        data = dgram.data();
         ptr = data.data();
         if (data.size() < HeaderSize) {
             qDebug() << addr << "discarded: reply too small";
@@ -190,6 +240,11 @@ void tst_QDnsLookup::initTestCase()
 {
     if (qgetenv("QTEST_ENVIRONMENT") == "ci")
         dnsServersMustWork = true;
+
+#if QT_CONFIG(networkproxy)
+    // for DNS-over-TLS
+    QNetworkProxyFactory::setUseSystemConfiguration(true);
+#endif
 }
 
 QString tst_QDnsLookup::domainName(const QString &input)