fix: create DNS WFP sublayer for app split tunneling (#2768)

CMakeLists.txt

@@ -4,7 +4,7 @@ set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 set(PROJECT AmneziaVPN)
-set(AMNEZIAVPN_VERSION 4.9.0.2)
+set(AMNEZIAVPN_VERSION 4.9.0.3)
 
 set(QT_CREATOR_SKIP_PACKAGE_MANAGER_SETUP ON CACHE BOOL "" FORCE)
 set(CMAKE_PROJECT_TOP_LEVEL_INCLUDES
@@ -28,7 +28,7 @@ string(TIMESTAMP CURRENT_DATE "%Y-%m-%d")
 set(RELEASE_DATE "${CURRENT_DATE}")
 
 set(APP_MAJOR_VERSION ${CMAKE_PROJECT_VERSION_MAJOR}.${CMAKE_PROJECT_VERSION_MINOR}.${CMAKE_PROJECT_VERSION_PATCH})
-set(APP_ANDROID_VERSION_CODE 2123)
+set(APP_ANDROID_VERSION_CODE 2126)
 
 if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
     set(MZ_PLATFORM_NAME "linux")

client/cmake/3rdparty.cmake

@@ -2,7 +2,7 @@ set(CLIENT_ROOT_DIR ${CMAKE_CURRENT_LIST_DIR}/..)
 
 set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/Modules;${CMAKE_MODULE_PATH}")
 
-add_subdirectory(${CLIENT_ROOT_DIR}/3rd/SortFilterProxyModel ${CMAKE_BINARY_DIR}/3rd/SortFilterProxyModel)
+add_subdirectory(${CLIENT_ROOT_DIR}/3rd/SortFilterProxyModel)
 set(LIBS ${LIBS} SortFilterProxyModel)
 include(${CLIENT_ROOT_DIR}/cmake/QSimpleCrypto.cmake)
 
@@ -12,20 +12,20 @@ add_compile_definitions(_WINSOCKAPI_)
 
 set(BUILD_SHARED_LIBS OFF CACHE BOOL "" FORCE)
 set(BUILD_WITH_QT6 ON)
-add_subdirectory(${CLIENT_ROOT_DIR}/3rd/qtkeychain ${CMAKE_BINARY_DIR}/3rd/qtkeychain EXCLUDE_FROM_ALL)
+add_subdirectory(${CLIENT_ROOT_DIR}/3rd/qtkeychain EXCLUDE_FROM_ALL)
 
 if(ANDROID)
     # Use qtgamepad from amnezia-vpn/qtgamepad repository
     # Only if Qt6CorePrivate is available (required by qtgamepad)
     find_package(Qt6CorePrivate CONFIG QUIET)
     if(Qt6CorePrivate_FOUND)
-        add_subdirectory(${CLIENT_ROOT_DIR}/3rd/qtgamepad ${CMAKE_BINARY_DIR}/3rd/qtgamepad)
+        add_subdirectory(${CLIENT_ROOT_DIR}/3rd/qtgamepad)
         # Link both the C++ module and QML plugin
         if(TARGET GamepadLegacy)
-            list(APPEND LIBS GamepadLegacy)
+            target_link_libraries(${PROJECT} PRIVATE GamepadLegacy)
         endif()
         if(TARGET GamepadLegacyQuickPrivate)
-            list(APPEND LIBS GamepadLegacyQuickPrivate)
+            target_link_libraries(${PROJECT} PRIVATE GamepadLegacyQuickPrivate)
         endif()
         message(STATUS "Gamepad support enabled for Android")
     else()

client/core/controllers/coreController.cpp

@@ -22,8 +22,7 @@
 #endif
 
 CoreController::CoreController(const QSharedPointer<VpnConnection> &vpnConnection, SecureQSettings* settings,
-                               QQmlApplicationEngine *engine, QObject *parent,
-                               bool skipPlatformControllerInit)
+                               QQmlApplicationEngine *engine, QObject *parent)
     : QObject(parent), m_vpnConnection(vpnConnection), m_settings(settings), m_engine(engine)
 {
     initRepositories();
@@ -32,10 +31,8 @@ CoreController::CoreController(const QSharedPointer<VpnConnection> &vpnConnectio
     initControllers();
     initSignalHandlers();
 
-    if (!skipPlatformControllerInit) {
-        initAndroidController();
-        initAppleController();
-    }
+    initAndroidController();
+    initAppleController();
     initLogging();
 
     m_translator = new QTranslator(this);

client/core/controllers/coreController.h

@@ -90,8 +90,7 @@ class CoreController : public QObject
 
 public:
     explicit CoreController(const QSharedPointer<VpnConnection> &vpnConnection, SecureQSettings* settings,
-                            QQmlApplicationEngine *engine, QObject *parent = nullptr,
-                            bool skipPlatformControllerInit = false);
+                            QQmlApplicationEngine *engine, QObject *parent = nullptr);
 
     PageController* pageController() const;
     void setQmlRoot();

client/platforms/windows/daemon/windowsdaemon.cpp

@@ -62,12 +62,29 @@ void WindowsDaemon::prepareActivation(const InterfaceConfig& config, int inetAda
 }
 
 void WindowsDaemon::activateSplitTunnel(const InterfaceConfig& config, int vpnAdapterIndex) {
-    if (m_splitTunnelManager == nullptr)
+    if (m_splitTunnelManager == nullptr) {
+        if (config.m_vpnDisabledApps.length() > 0) {
+            logger.error() << "Split tunnel manager is not initialized";
+            emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_INIT_FAILURE);
+        }
         return;
+    }
 
   if (config.m_vpnDisabledApps.length() > 0) {
-      m_splitTunnelManager->start(m_inetAdapterIndex, vpnAdapterIndex);
-      m_splitTunnelManager->excludeApps(config.m_vpnDisabledApps);
+      if (!m_splitTunnelManager->start(m_inetAdapterIndex, vpnAdapterIndex)) {
+          logger.error() << "Failed to start split tunnel";
+          emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_START_FAILURE);
+          return;
+      }
+      if (!m_splitTunnelManager->excludeApps(config.m_vpnDisabledApps)) {
+          logger.error() << "Failed to apply split tunnel app exclusions";
+          emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_EXCLUDE_FAILURE);
+          return;
+      }
+      if (!m_splitTunnelManager->isRunning()) {
+          logger.error() << "Split tunnel did not reach running state";
+          emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_START_FAILURE);
+      }
   } else {
       m_splitTunnelManager->stop();
   }
@@ -79,7 +96,9 @@ bool WindowsDaemon::run(Op op, const InterfaceConfig& config) {
       // The Client has sent us a list of disabled apps, but we failed
       // to init the the split tunnel driver.
       // So let the client know this was not possible
+      logger.error() << "Split tunnel manager is not initialized";
       emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_INIT_FAILURE);
+      return false;
     }
     return true;
   }
@@ -90,14 +109,20 @@ bool WindowsDaemon::run(Op op, const InterfaceConfig& config) {
   }
   if (config.m_vpnDisabledApps.length() > 0) {
     if (!m_splitTunnelManager->start(m_inetAdapterIndex)) {
+      logger.error() << "Split tunnel start failed";
       emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_START_FAILURE);
+      return false;
     };
     if (!m_splitTunnelManager->excludeApps(config.m_vpnDisabledApps)) {
+      logger.error() << "Split tunnel app exclusion failed";
       emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_EXCLUDE_FAILURE);
+      return false;
     };
     // Now the driver should be running (State == 4)
     if (!m_splitTunnelManager->isRunning()) {
+      logger.error() << "Split tunnel did not reach running state";
       emit backendFailure(DaemonError::ERROR_SPLIT_TUNNEL_START_FAILURE);
+      return false;
     }
     return true;
   }

client/platforms/windows/daemon/windowsfirewall.cpp

@@ -36,6 +36,8 @@
 // ID for the Firewall Sublayer
 DEFINE_GUID(ST_FW_WINFW_BASELINE_SUBLAYER_KEY, 0xc78056ff, 0x2bc1, 0x4211, 0xaa,
             0xdd, 0x7f, 0x35, 0x8d, 0xef, 0x20, 0x2d);
+DEFINE_GUID(ST_FW_WINFW_DNS_SUBLAYER_KEY, 0x60090787, 0xcca1, 0x4937, 0xaa,
+            0xce, 0x51, 0x25, 0x6e, 0xf4, 0x81, 0xf3);
 // ID for the Mullvad Split-Tunnel Sublayer Provider
 DEFINE_GUID(ST_FW_PROVIDER_KEY, 0xe2c114ee, 0xf32a, 0x4264, 0xa6, 0xcb, 0x3f,
             0xa7, 0x99, 0x63, 0x56, 0xd9);
@@ -49,6 +51,53 @@ constexpr uint8_t LOW_WEIGHT = 0;
 constexpr uint8_t MED_WEIGHT = 7;
 constexpr uint8_t HIGH_WEIGHT = 13;
 constexpr uint8_t MAX_WEIGHT = 15;
+
+bool ensureSublayer(HANDLE wfp, const GUID& key, const wchar_t* name,
+                    const wchar_t* description) {
+  FWPM_SUBLAYER0* maybeLayer = nullptr;
+  DWORD result = FwpmSubLayerGetByKey0(wfp, &key, &maybeLayer);
+  if (result == ERROR_SUCCESS) {
+    logger.debug() << "The Sublayer Already Exists:"
+                   << QString::fromWCharArray(name);
+    FwpmFreeMemory0(reinterpret_cast<void**>(&maybeLayer));
+    return true;
+  }
+  if (result != FWP_E_SUBLAYER_NOT_FOUND) {
+    logger.error() << "FwpmSubLayerGetByKey0 failed. Return value:.\n"
+                   << result;
+    return false;
+  }
+
+  result = FwpmTransactionBegin(wfp, NULL);
+  if (result != ERROR_SUCCESS) {
+    logger.error() << "FwpmTransactionBegin0 failed. Return value:.\n"
+                   << result;
+    return false;
+  }
+
+  FWPM_SUBLAYER0 subLayer;
+  memset(&subLayer, 0, sizeof(subLayer));
+  subLayer.subLayerKey = key;
+  subLayer.displayData.name = const_cast<PWSTR>(name);
+  subLayer.displayData.description = const_cast<PWSTR>(description);
+  subLayer.weight = 0xFFFF;
+
+  result = FwpmSubLayerAdd0(wfp, &subLayer, NULL);
+  if (result != ERROR_SUCCESS) {
+    FwpmTransactionAbort0(wfp);
+    logger.error() << "FwpmSubLayerAdd0 failed. Return value:.\n" << result;
+    return false;
+  }
+
+  result = FwpmTransactionCommit0(wfp);
+  if (result != ERROR_SUCCESS) {
+    logger.error() << "FwpmTransactionCommit0 failed. Return value:.\n"
+                   << result;
+    return false;
+  }
+  logger.debug() << "Initialised Sublayer:" << QString::fromWCharArray(name);
+  return true;
+}
 }  // namespace
 
 WindowsFirewall* WindowsFirewall::create(QObject* parent) {
@@ -116,47 +165,12 @@ bool WindowsFirewall::initSublayer() {
   }
   auto cleanup = qScopeGuard([&] { FwpmEngineClose0(wfp); });
 
-  // Check if the Layer Already Exists
-  FWPM_SUBLAYER0* maybeLayer;
-  result = FwpmSubLayerGetByKey0(wfp, &ST_FW_WINFW_BASELINE_SUBLAYER_KEY,
-                                 &maybeLayer);
-  if (result == ERROR_SUCCESS) {
-    logger.debug() << "The Sublayer Already Exists!";
-    FwpmFreeMemory0((void**)&maybeLayer);
-    return true;
-  }
-
-  // Step 1: Start Transaction
-  result = FwpmTransactionBegin(wfp, NULL);
-  if (result != ERROR_SUCCESS) {
-    logger.error() << "FwpmTransactionBegin0 failed. Return value:.\n"
-                   << result;
-    return false;
-  }
-
-  // Step 3: Add Sublayer
-  FWPM_SUBLAYER0 subLayer;
-  memset(&subLayer, 0, sizeof(subLayer));
-  subLayer.subLayerKey = ST_FW_WINFW_BASELINE_SUBLAYER_KEY;
-  subLayer.displayData.name = (PWSTR)L"Amnezia-SplitTunnel-Sublayer";
-  subLayer.displayData.description =
-      (PWSTR)L"Filters that enforce a good baseline";
-  subLayer.weight = 0xFFFF;
-
-  result = FwpmSubLayerAdd0(wfp, &subLayer, NULL);
-  if (result != ERROR_SUCCESS) {
-    logger.error() << "FwpmSubLayerAdd0 failed. Return value:.\n" << result;
-    return false;
-  }
-  // Step 4: Commit!
-  result = FwpmTransactionCommit0(wfp);
-  if (result != ERROR_SUCCESS) {
-    logger.error() << "FwpmTransactionCommit0 failed. Return value:.\n"
-                   << result;
-    return false;
-  }
-  logger.debug() << "Initialised Sublayer";
-  return true;
+  return ensureSublayer(wfp, ST_FW_WINFW_BASELINE_SUBLAYER_KEY,
+                        L"Amnezia-SplitTunnel-Sublayer",
+                        L"Filters that enforce a good baseline") &&
+         ensureSublayer(wfp, ST_FW_WINFW_DNS_SUBLAYER_KEY,
+                        L"Amnezia-SplitTunnel-DNS-Sublayer",
+                        L"DNS filters for split tunneling");
 }
 
 bool WindowsFirewall::enableInterface(int vpnAdapterIndex) {

client/platforms/windows/daemon/windowssplittunnel.cpp

@@ -24,6 +24,7 @@
 #include <QFileInfo>
 #include <QNetworkInterface>
 #include <QScopeGuard>
+#include <QUrl>
 
 #pragma region
 
@@ -148,6 +149,18 @@ ProcessInfo getProcessInfo(HANDLE process, const PROCESSENTRY32W& processMeta) {
   return pi;
 }
 
+QString normalizeExecutablePath(const QString& path) {
+  QString normalized = path.trimmed();
+  if (normalized.startsWith("file:", Qt::CaseInsensitive)) {
+    const QString localPath = QUrl(normalized).toLocalFile();
+    if (!localPath.isEmpty()) {
+      normalized = localPath;
+    }
+  }
+  normalized.replace('/', '\\');
+  return normalized;
+}
+
 }  // namespace
 
 std::unique_ptr<WindowsSplitTunnel> WindowsSplitTunnel::create(
@@ -287,6 +300,10 @@ bool WindowsSplitTunnel::excludeApps(const QStringList& appPaths) {
 
   logger.debug() << "Pushing new Ruleset for Split-Tunnel " << state;
   auto config = generateAppConfiguration(appPaths);
+  if (config.empty()) {
+    logger.error() << "No valid split-tunnel application rules generated";
+    return false;
+  }
 
   DWORD bytesReturned;
   auto ok = DeviceIoControl(m_driver, IOCTL_SET_CONFIGURATION, &config[0],
@@ -314,7 +331,7 @@ bool WindowsSplitTunnel::start(int inetAdapterIndex, int vpnAdapterIndex) {
     auto ok = DeviceIoControl(m_driver, IOCTL_INITIALIZE, nullptr, 0, nullptr,
                               0, &bytesReturned, nullptr);
     if (!ok) {
-      logger.error() << "Driver init failed";
+      logger.error() << "Driver init failed. Error:" << GetLastError();
       return false;
     }
   }
@@ -323,11 +340,16 @@ bool WindowsSplitTunnel::start(int inetAdapterIndex, int vpnAdapterIndex) {
   if (getState() == STATE_INITIALIZED) {
     logger.debug() << "State is Init, requires process config";
     auto config = generateProcessBlob();
+    if (config.empty()) {
+      logger.error() << "Process configuration blob is empty";
+      return false;
+    }
     auto ok = DeviceIoControl(m_driver, IOCTL_REGISTER_PROCESSES, &config[0],
                               (DWORD)config.size(), nullptr, 0, &bytesReturned,
                               nullptr);
     if (!ok) {
-      logger.error() << "Failed to set Process Config";
+      logger.error() << "Failed to set Process Config. Error:"
+                     << GetLastError();
       return false;
     }
     logger.debug() << "Set Process Config ok || new State:" << stateString();
@@ -340,11 +362,16 @@ bool WindowsSplitTunnel::start(int inetAdapterIndex, int vpnAdapterIndex) {
   logger.debug() << "Driver is  ready || new State:" << stateString();
 
   auto config = generateIPConfiguration(inetAdapterIndex, vpnAdapterIndex);
+  if (config.empty()) {
+    logger.error() << "Network configuration blob is empty. Internet adapter:"
+                   << inetAdapterIndex << "VPN adapter:" << vpnAdapterIndex;
+    return false;
+  }
   auto ok = DeviceIoControl(m_driver, IOCTL_REGISTER_IP_ADDRESSES, &config[0],
                             (DWORD)config.size(), nullptr, 0, &bytesReturned,
                             nullptr);
   if (!ok) {
-    logger.error() << "Failed to set Network Config";
+    logger.error() << "Failed to set Network Config. Error:" << GetLastError();
     return false;
   }
   logger.debug() << "New Network Config Applied || new State:" << stateString();
@@ -404,13 +431,22 @@ std::vector<uint8_t> WindowsSplitTunnel::generateAppConfiguration(
   size_t cummulated_string_size = 0;
   QStringList dosPaths;
   for (auto const& path : appPaths) {
-    auto dosPath = convertPath(path);
+    const QString normalizedPath = normalizeExecutablePath(path);
+    auto dosPath = convertPath(normalizedPath);
+    if (dosPath.isEmpty()) {
+      logger.error() << "Rejecting split-tunnel app path with empty device "
+                        "conversion:"
+                     << normalizedPath;
+      continue;
+    }
     dosPaths.append(dosPath);
     cummulated_string_size += dosPath.toStdWString().size() * sizeof(wchar_t);
-    logger.debug() << dosPath;
+  }
+  if (dosPaths.isEmpty()) {
+    return {};
   }
   size_t bufferSize = sizeof(CONFIGURATION_HEADER) +
-                      (sizeof(CONFIGURATION_ENTRY) * appPaths.size()) +
+                      (sizeof(CONFIGURATION_ENTRY) * dosPaths.size()) +
                       cummulated_string_size;
   std::vector<uint8_t> outBuffer(bufferSize);
 
@@ -418,7 +454,7 @@ std::vector<uint8_t> WindowsSplitTunnel::generateAppConfiguration(
   auto entry = (CONFIGURATION_ENTRY*)(header + 1);
 
   auto stringDest = &outBuffer[0] + sizeof(CONFIGURATION_HEADER) +
-                    (sizeof(CONFIGURATION_ENTRY) * appPaths.size());
+                    (sizeof(CONFIGURATION_ENTRY) * dosPaths.size());
 
   SIZE_T stringOffset = 0;
 
@@ -437,7 +473,7 @@ std::vector<uint8_t> WindowsSplitTunnel::generateAppConfiguration(
     stringOffset += stringLength;
   }
 
-  header->NumEntries = appPaths.length();
+  header->NumEntries = dosPaths.length();
   header->TotalLength = bufferSize;
 
   return outBuffer;
@@ -449,9 +485,7 @@ std::vector<std::byte> WindowsSplitTunnel::generateIPConfiguration(
 
   auto config = reinterpret_cast<IP_ADDRESSES_CONFIG*>(&out[0]);
 
-  auto ifaces = QNetworkInterface::allInterfaces();
-
-    if (vpnAdapterIndex == 0) {
+  if (vpnAdapterIndex == 0) {
     vpnAdapterIndex = WindowsCommons::VPNAdapterIndex();
   }
   // Always the VPN
@@ -520,7 +554,7 @@ std::vector<uint8_t> WindowsSplitTunnel::generateProcessBlob() {
     auto process_handle = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, FALSE,
                                       currentProcess.th32ProcessID);
 
-    if (process_handle == INVALID_HANDLE_VALUE) {
+    if (process_handle == nullptr) {
       continue;
     }
     ProcessInfo info = getProcessInfo(process_handle, currentProcess);
@@ -640,23 +674,48 @@ bool WindowsSplitTunnel::isInstalled() {
 }
 
 QString WindowsSplitTunnel::convertPath(const QString& path) {
-  auto parts = path.split("/");
+  const QString normalizedPath = normalizeExecutablePath(path);
+  if (normalizedPath.isEmpty()) {
+    logger.error() << "Empty executable path for DOS device conversion";
+    return "";
+  }
+  auto parts = normalizedPath.split("\\", Qt::SkipEmptyParts);
+  if (parts.isEmpty()) {
+    logger.error() << "Invalid executable path for DOS device conversion:"
+                   << normalizedPath;
+    return "";
+  }
   QString driveLetter = parts.takeFirst();
   if (!driveLetter.contains(":") || parts.size() == 0) {
     // device should contain : for e.g C:
+    logger.error() << "Invalid executable path for DOS device conversion:"
+                   << normalizedPath;
     return "";
   }
-  QByteArray buffer(2048, 0xFFu);
-  auto ok = QueryDosDeviceW(qUtf16Printable(driveLetter),
-                            (wchar_t*)buffer.data(), buffer.size() / 2);
-
-  if (ok == ERROR_INSUFFICIENT_BUFFER) {
+  QByteArray buffer(2048 * sizeof(wchar_t), 0);
+  DWORD ok = 0;
+  DWORD err = ERROR_SUCCESS;
+  for (int attempt = 0; attempt < 4; ++attempt) {
+    ok = QueryDosDeviceW(reinterpret_cast<LPCWSTR>(driveLetter.utf16()),
+                         reinterpret_cast<LPWSTR>(buffer.data()),
+                         buffer.size() / sizeof(wchar_t));
+    if (ok != 0) {
+      break;
+    }
+    err = GetLastError();
+    if (err != ERROR_INSUFFICIENT_BUFFER) {
+      WindowsUtils::windowsLog("Err fetching dos path");
+      logger.error() << "QueryDosDeviceW failed for" << driveLetter
+                     << "error:" << err;
+      return "";
+    }
     buffer.resize(buffer.size() * 2);
-    ok = QueryDosDeviceW(qUtf16Printable(driveLetter), (wchar_t*)buffer.data(),
-                         buffer.size() / 2);
+    buffer.fill(0);
   }
   if (ok == 0) {
     WindowsUtils::windowsLog("Err fetching dos path");
+    logger.error() << "QueryDosDeviceW failed after buffer growth for"
+                   << driveLetter << "error:" << err;
     return "";
   }
   QString deviceName;