FXVPN-377 Fix DNS Inclusion (#10257)

* Fix the DNS handling in StateOnPartial

wip fix dns

Add test code

FIX test code

remove unsued tests

cleanup

* cleanup

* Fix compile thingy

src/cmake/sources.cmake

@@ -77,6 +77,8 @@ target_sources(mozillavpn-sources INTERFACE
     ${CMAKE_CURRENT_SOURCE_DIR}/controller.h
     ${CMAKE_CURRENT_SOURCE_DIR}/controllerimpl.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/controllerimpl.h
+    ${CMAKE_CURRENT_SOURCE_DIR}/controller_p.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/controller_p.h
     ${CMAKE_CURRENT_SOURCE_DIR}/daemon/daemon.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/daemon/daemon.h
     ${CMAKE_CURRENT_SOURCE_DIR}/daemon/daemonlocalserverconnection.cpp

src/controller.cpp

@@ -9,6 +9,7 @@
 
 #include "app.h"
 #include "constants.h"
+#include "controller_p.h"
 #include "controllerimpl.h"
 #include "dnshelper.h"
 #include "feature/feature.h"
@@ -46,12 +47,6 @@
 #  include "platforms/wasm/wasmcontroller.h"
 #endif
 
-// The Mullvad proxy services are located at internal IPv4 addresses in the
-// 10.124.0.0/20 address range, which is a subset of the 10.0.0.0/8 Class-A
-// private address range.
-constexpr const char* MULLVAD_PROXY_RANGE = "10.124.0.0";
-constexpr const int MULLVAD_PROXY_RANGE_LENGTH = 20;
-
 namespace {
 Logger logger("Controller");
 
@@ -76,6 +71,8 @@ Controller::Reason stateToReason(Controller::State state) {
 }
 }  // namespace
 
+using namespace ControllerPrivate;
+
 Controller::Controller() {
   MZ_COUNT_CTOR(Controller);
 
@@ -591,24 +588,6 @@ QList<IPAddress> Controller::getAllowedIPAddressRanges(
   return list;
 }
 
-// static
-QList<IPAddress> Controller::getExtensionProxyAddressRanges(
-    const Server& exitServer) {
-  QList<IPAddress> ranges = {
-      IPAddress(QHostAddress(exitServer.ipv4Gateway()), 32),
-      IPAddress(QHostAddress{MULLVAD_PROXY_RANGE}, MULLVAD_PROXY_RANGE_LENGTH)};
-  if (!exitServer.ipv6Gateway().isEmpty()) {
-    ranges.append(IPAddress(QHostAddress(exitServer.ipv6Gateway()), 128));
-  }
-
-  auto const dns = DNSHelper::getDNSDetails(exitServer.ipv4Gateway());
-  if (dns.dnsType != "Default" && dns.dnsType != "Custom") {
-    ranges.append(IPAddress(QHostAddress(dns.ipAddress), 32));
-  }
-
-  return ranges;
-}
-
 void Controller::activateNext() {
   MozillaVPN* vpn = MozillaVPN::instance();
   const Device* device = vpn->deviceModel()->currentDevice(vpn->keys());

src/controller.h

@@ -169,8 +169,6 @@ class Controller : public QObject, public LogSerializer {
   };
   static IPAddressList getExcludedIPAddressRanges();
   static QList<IPAddress> getAllowedIPAddressRanges(const Server& server);
-  static QList<IPAddress> getExtensionProxyAddressRanges(
-      const Server& exitServer);
 
   enum ServerCoolDownPolicyForSilentSwitch {
     eServerCoolDownNeeded,

src/controller_p.cpp

@@ -0,0 +1,53 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "controller_p.h"
+
+#include <QHostAddress>
+
+#include "dnshelper.h"
+#include "ipaddress.h"
+#include "models/server.h"
+#include "rfc/rfc1918.h"
+#include "rfc/rfc4193.h"
+
+namespace ControllerPrivate {
+
+QList<IPAddress> getExtensionProxyAddressRanges(
+    const Server& exitServer, std::optional<const dnsData> dnsServer) {
+  QList<IPAddress> ranges = {
+      IPAddress(QHostAddress(exitServer.ipv4Gateway()), 32),
+      IPAddress(QHostAddress{MULLVAD_PROXY_RANGE}, MULLVAD_PROXY_RANGE_LENGTH)};
+  if (!exitServer.ipv6Gateway().isEmpty()) {
+    ranges.append(IPAddress(QHostAddress(exitServer.ipv6Gateway()), 128));
+  }
+
+  const dnsData dns = [&dnsServer, &exitServer]() {
+    if (dnsServer.has_value()) {
+      return dnsServer.value();
+    }
+    return DNSHelper::getDNSDetails(exitServer.ipv4Gateway());
+  }();
+
+  if (dns.dnsType == "Default") {
+    // The DNS Adress is already Resolved.
+    return ranges;
+  }
+  if (dns.dnsType != "Custom") {
+    // It's a vpn-internal DNS, always add it
+    ranges.append(IPAddress(QHostAddress(dns.ipAddress), 32));
+    return ranges;
+  }
+  Q_ASSERT(dns.dnsType == "Custom");
+  const QHostAddress addr{dns.ipAddress};
+  // If it is a custom dns only add it to the tunnel if it is **NOT**
+  // a "lan" adress
+  if (!RFC1918::contains(addr) && !RFC4193::contains(addr) &&
+      !addr.isLoopback()) {
+    ranges.append(IPAddress(QHostAddress(dns.ipAddress), 32));
+  }
+  return ranges;
+}
+
+}  // namespace ControllerPrivate

src/controller_p.h

@@ -0,0 +1,33 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#pragma once
+
+#include <QHostAddress>
+#include <QList>
+
+#include "dnshelper.h"
+
+class IPAddress;
+class Server;
+
+/**
+ * Controller.cpp is currently mock'ed out of the unit tests
+ * stopping us from testing it. This namespace should contain
+ * pure functions so that we can start writing unit-tests for that code.
+ *
+ */
+namespace ControllerPrivate {
+
+// The Mullvad proxy services are located at internal IPv4 addresses in the
+// 10.124.0.0/20 address range, which is a subset of the 10.0.0.0/8 Class-A
+// private address range.
+constexpr const char* MULLVAD_PROXY_RANGE = "10.124.0.0";
+constexpr const int MULLVAD_PROXY_RANGE_LENGTH = 20;
+
+QList<IPAddress> getExtensionProxyAddressRanges(
+    const Server& exitServer,
+    std::optional<const dnsData> dnsServer = std::nullopt);
+
+}  // namespace ControllerPrivate

src/platforms/windows/daemon/windowsfirewall.cpp

@@ -10,7 +10,6 @@
 #include <initguid.h>
 #include <netfw.h>
 #include <qaccessible.h>
-#include <qassert.h>
 #include <stdio.h>
 #include <windows.h>
 

tests/unit/CMakeLists.txt

@@ -53,6 +53,8 @@ target_sources(unit_tests PRIVATE
     ${MZ_SOURCE_DIR}/constants.cpp
     ${MZ_SOURCE_DIR}/constants.h
     ${MZ_SOURCE_DIR}/controller.h
+    ${MZ_SOURCE_DIR}/controller_p.h
+    ${MZ_SOURCE_DIR}/controller_p.cpp
     ${MZ_SOURCE_DIR}/dnshelper.cpp
     ${MZ_SOURCE_DIR}/dnshelper.h
     ${MZ_SOURCE_DIR}/dnspingsender.cpp
@@ -248,6 +250,8 @@ target_sources(unit_tests PRIVATE
     ${MZ_SOURCE_DIR}/qmlpath.h
     ${MZ_SOURCE_DIR}/resourceloader.cpp
     ${MZ_SOURCE_DIR}/resourceloader.h
+    ${MZ_SOURCE_DIR}/rfc/rfc1112.cpp
+    ${MZ_SOURCE_DIR}/rfc/rfc1112.h
     ${MZ_SOURCE_DIR}/rfc/rfc1918.cpp
     ${MZ_SOURCE_DIR}/rfc/rfc1918.h
     ${MZ_SOURCE_DIR}/rfc/rfc4193.cpp
@@ -325,6 +329,8 @@ target_sources(unit_tests PRIVATE
     testaddon.h
     testconnectionhealth.cpp
     testconnectionhealth.h
+    testcontroller_p.cpp
+    testcontroller_p.h
     testcommandlineparser.cpp
     testcommandlineparser.h
     testcryptosettings.cpp

tests/unit/main.cpp

@@ -13,6 +13,17 @@
 #include "networkrequest.h"
 #include "settingsholder.h"
 
+#ifdef _WIN32
+#  include <windows.h>
+
+#  include <chrono>
+#  include <thread>
+#else
+#  include <unistd.h>
+
+#  include <cstdio>
+#endif
+
 QVector<TestHelper::NetworkConfig> TestHelper::networkConfig;
 Controller::State TestHelper::controllerState = Controller::StateInitializing;
 QVector<QObject*> TestHelper::testList;
@@ -29,6 +40,58 @@ QObject* TestHelper::findTest(const QString& name) {
   return nullptr;
 }
 
+void maybeWaitForDebugger(int argc, char* argv[]) {
+  bool debuggerFlag = false;
+  for (int i = 1; i < argc; ++i) {
+    if (std::string(argv[i]) == "--debugger") {
+      debuggerFlag = true;
+      break;
+    }
+  }
+  if (!debuggerFlag) {
+    // Not asked to wait for a debugger; just return.
+    return;
+  }
+#ifdef _WIN32
+  DWORD pid = GetCurrentProcessId();
+#else
+  pid_t pid = getpid();
+#endif
+  qDebug() << "PID: " << pid << " - waiting for debugger to attach...\n";
+
+#ifdef _WIN32
+  // Use Win32 API to detect debugger
+  while (!IsDebuggerPresent()) {
+    // Sleep 1 second before checking again
+    std::this_thread::sleep_for(std::chrono::seconds(1));
+  }
+  qDebug() << "Debugger attached. Continuing...\n";
+
+  return;
+#else
+  // TODO: Check if that also works on macos
+  while (true) {
+    QFile file("/proc/self/status");
+    auto closeFile = qScopeGuard([&]() { file.close(); });
+
+    if (!file.open(QIODevice::ReadOnly | QIODevice::Text)) {
+      return;
+    }
+    while (!file.atEnd()) {
+      QString line = file.readLine();
+      if (line.startsWith("TracerPid:")) {
+        QString pidStr = line.mid(QString("TracerPid:").length()).trimmed();
+        if (pidStr.toInt() != 0) {
+          qDebug() << "Debugger attached. Continuing...\n";
+          return;
+        }
+      }
+    }
+    std::this_thread::sleep_for(std::chrono::seconds(1));
+  }
+#endif
+}
+
 TestHelper::TestHelper() { testList.append(this); }
 
 // static
@@ -64,6 +127,7 @@ bool TestHelper::networkRequestGeneric(NetworkRequest* request) {
 }
 
 int main(int argc, char* argv[]) {
+  maybeWaitForDebugger(argc, argv);
 #ifdef MZ_DEBUG
   LeakDetector leakDetector;
   Q_UNUSED(leakDetector);

tests/unit/testcontroller_p.cpp

@@ -0,0 +1,98 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "testcontroller_p.h"
+
+#include <QHostAddress>
+#include <QJsonArray>
+#include <QJsonObject>
+
+#include "controller_p.h"
+#include "dnshelper.h"
+#include "ipaddress.h"
+#include "models/server.h"
+namespace {
+
+Server createServer(QString ipv4Gateway) {
+  QJsonObject args;
+  args["hostname"] = "test";
+  args["ipv4_addr_in"] = "test";
+  args["ipv4_gateway"] = ipv4Gateway;
+  args["ipv6_addr_in"] = "2001:db8::1";
+  args["ipv6_gateway"] = "2001:db8::1";
+  args["public_key"] = "test";
+  args["weight"] = 0.0;
+  args["port_ranges"] = QJsonArray{};
+  Server out{};
+  bool ok = out.fromJson(args);
+  Q_UNUSED(ok);
+  return out;
+}
+
+bool includesAddress(QHostAddress addr, const QList<IPAddress>& l) {
+  for (auto const& laddr : l) {
+    if (laddr.contains(addr)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+};  // namespace
+
+void TestControllerPrivate::getExtensionProxyAddressRanges() {
+  auto const testIsDNSIncluded = [](dnsData dns, QString exitGateway) {
+    auto const res = ControllerPrivate::getExtensionProxyAddressRanges(
+        createServer(exitGateway), dns);
+    return includesAddress(QHostAddress{dns.ipAddress}, res);
+  };
+
+  // A Custom DNS that is not in the localhost network
+  // should be in the ProxyAdressRange (so it will be tunneled in the vpn)
+  QCOMPARE(testIsDNSIncluded(
+               {
+                   .ipAddress = "99.99.99.99",
+                   .dnsType = "Custom",
+               },
+               "1.2.3.5"),
+           true);
+
+  // Opposite: a LAN ip should not appear on this list.
+  QCOMPARE(testIsDNSIncluded(
+               {
+                   .ipAddress = "192.168.0.1",
+                   .dnsType = "Custom",
+               },
+               "1.2.3.5"),
+           false);
+
+  // If you run your dns on localhost. also not.
+  QCOMPARE(testIsDNSIncluded(
+               {
+                   .ipAddress = "127.0.0.1",
+                   .dnsType = "Custom",
+               },
+               "1.2.3.5"),
+           false);
+
+  // The default is the Exit node, so that should be tunneled
+  QCOMPARE(testIsDNSIncluded(
+               {
+                   .ipAddress = "1.2.3.5",
+                   .dnsType = "Default",
+               },
+               "1.2.3.5"),
+           true);
+
+  // If we add a filtering dns, that MUST always go through the tunnel.
+  QCOMPARE(testIsDNSIncluded(
+               {
+                   .ipAddress = "99.99.99.99",
+                   .dnsType = "BlockMalwareAndTrackers",
+               },
+               "1.2.3.5"),
+           true);
+}
+
+static TestControllerPrivate s_instance;

tests/unit/testcontroller_p.h

@@ -0,0 +1,11 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "helper.h"
+
+class TestControllerPrivate final : public TestHelper {
+  Q_OBJECT
+ private slots:
+  void getExtensionProxyAddressRanges();
+};