fix: xray stability and split-tunneling (#2187)

* fix: xray heap corruption

* fix: use proper configuration for split-tunneled apps

* chore: enable killswitch

* chore: xray windows split-tunneling cleanup

* chore: proper xray killswitch log

* feat: add wait for the tun device

* chore: update amnezia_xray deps for macos

* fix: add nullptr check for split-tunnel on win

* fix: modernize vpnAdapter grabbing function

* fix: remove network watcher due to its fragileness

* chore: xrayprotocol cleanup

* fix: correct wrong iface index on win

* chore: move tun2socks implementation to the client from the service

* chore: xrayprotocol cleanup

* chore: more xrayprotocol cleanup

* fix: consistent tun device with GUID specified

* chore: tun2socks logs

* chore: PrivilegedProcess cleanup
* better error handling in establishment phase
* terminate&kill ops for remote process

* fix: straighforward killing the process on windows

* fix: finally remove GUID setting from tun2socks due to instability

* fix: add sanitizer to ipc process

* chore: do not collect sensitive info from tun2socks

ipc/ipc.h

@@ -11,6 +11,7 @@
 namespace amnezia {
 
 enum PermittedProcess {
+    Invalid,
     OpenVPN,
     Wireguard,
     Tun2Socks,
@@ -19,16 +20,18 @@ enum PermittedProcess {
 
 inline QString permittedProcessPath(PermittedProcess pid)
 {
-    if (pid == PermittedProcess::OpenVPN) {
-        return Utils::openVpnExecPath();
-    } else if (pid == PermittedProcess::Wireguard) {
-        return Utils::wireguardExecPath();
-    } else if (pid == PermittedProcess::CertUtil) {
-        return Utils::certUtilPath();
-    } else if (pid == PermittedProcess::Tun2Socks) {
-        return Utils::tun2socksPath();
+    switch (pid) {
+        case PermittedProcess::OpenVPN:
+            return Utils::openVpnExecPath();
+        case PermittedProcess::Wireguard:
+            return Utils::wireguardExecPath();
+        case PermittedProcess::CertUtil:
+            return Utils::certUtilPath();
+        case PermittedProcess::Tun2Socks:
+            return Utils::tun2socksPath();
+        default:
+            return "";
     }
-    return "";
 }
 
 
@@ -48,6 +51,51 @@ inline QString getIpcProcessUrl(int pid) {
 #endif
 }
 
+inline QStringList sanitizeArguments(PermittedProcess proc, const QStringList &args) {
+    using Validator = std::function<bool(const QString&)>;
+    QMap<QString, Validator> namedArgs;
+    QList<Validator> positionalArgs;
+
+    switch (proc) {
+    case Tun2Socks:
+        namedArgs["-device"] = [](const QString& v) { return v.startsWith("tun://"); };
+        namedArgs["-proxy"] = [](const QString& v) { return v.startsWith("socks5://"); };
+        break;
+    default:
+        //FIXME
+        return args;
+    }
+
+
+    QStringList sanitized;
+
+    for (int i = 0, pos = 0; i < args.size(); i++) {
+        const auto& key = args[i];
+
+        if (const auto found = namedArgs.find(key); found != namedArgs.end()) {
+            const auto validator = found.value();
+
+            if (validator) {
+                if (i + 1 < args.size()) {
+                    const auto& value = args[i+1];
+                    if (validator(value)) {
+                        sanitized << key << value;
+                        i++;
+                    }
+                }
+            } else {
+                sanitized << key;
+            }
+        } else if (pos < positionalArgs.size()) {
+            if (const auto validator = positionalArgs[pos]; validator && validator(key)) {
+                sanitized << key;
+                pos++;
+            }
+        }
+    }
+
+    return sanitized;
+}
 
 } // namespace amnezia
 

ipc/ipc_interface.rep

@@ -38,8 +38,8 @@ class IpcInterface
     SLOT( bool updateResolvers(const QString& ifname, const QList<QHostAddress>& resolvers) );
     SLOT( bool restoreResolvers() );
 
-    SLOT(void xrayStart(const QString &config));
-    SLOT(void xrayStop());
+    SLOT(bool xrayStart(const QString &config));
+    SLOT(bool xrayStop());
 
     SLOT( bool startNetworkCheck(const QString& serverIpv4Gateway, const QString& deviceIpv4Address) );
     SLOT( bool stopNetworkCheck() );

ipc/ipc_process_interface.rep

@@ -4,6 +4,8 @@
 class IpcProcessInterface
 {
     SLOT( start() );
+    SLOT( terminate() );
+    SLOT( kill() );
     SLOT( close() );
 
     SLOT( setArguments(const QStringList &arguments) );
@@ -17,6 +19,11 @@ class IpcProcessInterface
     SLOT( QByteArray readAllStandardError() );
     SLOT( QByteArray readAllStandardOutput() );
 
+    SLOT( bool waitForFinished() );
+    SLOT( bool waitForFinished(int msecs) );
+    SLOT( bool waitForStarted() );
+    SLOT( bool waitForStarted(int msecs) );
+
 
     SIGNAL( errorOccurred(QProcess::ProcessError error) );
     SIGNAL( finished(int exitCode, QProcess::ExitStatus exitStatus) );

ipc/ipc_process_tun2socks.rep

@@ -1,11 +0,0 @@
-#include <QtCore>
-#include <QString>
-
-class IpcProcessTun2Socks
-{
-    SLOT( start() );
-    SLOT( stop() );
-
-    SIGNAL( setConnectionState(int state) );
-    SIGNAL( stateChanged(QProcess::ProcessState newState) );
-};

ipc/ipcserver.cpp

@@ -304,7 +304,7 @@ bool IpcServer::refreshKillSwitch(bool enabled)
     return KillSwitch::instance()->refresh(enabled);
 }
 
-void IpcServer::xrayStart(const QString& cfg)
+bool IpcServer::xrayStart(const QString& cfg)
 {
 #ifdef MZ_DEBUG
     qDebug() << "IpcServer::xrayStart";
@@ -313,7 +313,7 @@ void IpcServer::xrayStart(const QString& cfg)
     return Xray::getInstance().startXray(cfg);
 }
 
-void IpcServer::xrayStop()
+bool IpcServer::xrayStop()
 {
 #ifdef MZ_DEBUG
     qDebug() << "IpcServer::xrayStop";

ipc/ipcserver.h

@@ -10,10 +10,8 @@
 
 #include "ipc.h"
 #include "ipcserverprocess.h"
-#include "ipctun2socksprocess.h"
 
 #include "rep_ipc_interface_source.h"
-#include "rep_ipc_process_tun2socks_source.h"
 
 class IpcServer : public IpcInterfaceSource
 {
@@ -44,8 +42,8 @@ public:
     virtual bool refreshKillSwitch( bool enabled ) override;
     virtual bool updateResolvers(const QString& ifname, const QList<QHostAddress>& resolvers) override;
     virtual bool restoreResolvers() override;
-    virtual void xrayStart(const QString& cfg) override;
-    virtual void xrayStop() override;
+    virtual bool xrayStart(const QString& cfg) override;
+    virtual bool xrayStop() override;
     virtual bool startNetworkCheck(const QString& serverIpv4Gateway, const QString& deviceIpv4Address) override;
     virtual bool stopNetworkCheck() override;
 
@@ -56,12 +54,10 @@ private:
         ProcessDescriptor (QObject *parent = nullptr) {
             serverNode = QSharedPointer<QRemoteObjectHost>(new QRemoteObjectHost(parent));
             ipcProcess = QSharedPointer<IpcServerProcess>(new IpcServerProcess(parent));
-            tun2socksProcess = QSharedPointer<IpcProcessTun2Socks>(new IpcProcessTun2Socks(parent));
             localServer = QSharedPointer<QLocalServer>(new QLocalServer(parent));
         }
 
         QSharedPointer<IpcServerProcess> ipcProcess;
-        QSharedPointer<IpcProcessTun2Socks> tun2socksProcess;
         QSharedPointer<QRemoteObjectHost> serverNode;
         QSharedPointer<QLocalServer> localServer;
     };

ipc/ipcserverprocess.cpp

@@ -40,6 +40,14 @@ void IpcServerProcess::start()
     m_process->waitForStarted();
 }
 
+void IpcServerProcess::terminate() {
+    m_process->terminate();
+}
+
+void IpcServerProcess::kill() {
+    m_process->kill();
+}
+
 void IpcServerProcess::close()
 {
     m_process->close();
@@ -47,7 +55,7 @@ void IpcServerProcess::close()
 
 void IpcServerProcess::setArguments(const QStringList &arguments)
 {
-    m_process->setArguments(arguments);
+    m_process->setArguments(amnezia::sanitizeArguments(m_program, arguments));
 }
 
 void IpcServerProcess::setInputChannelMode(QProcess::InputChannelMode mode)
@@ -69,7 +77,9 @@ void IpcServerProcess::setProcessChannelMode(QProcess::ProcessChannelMode mode)
 
 void IpcServerProcess::setProgram(int programId)
 {
-    m_process->setProgram(amnezia::permittedProcessPath(static_cast<amnezia::PermittedProcess>(programId)));
+    m_program = static_cast<amnezia::PermittedProcess>(programId);
+    m_process->setProgram(amnezia::permittedProcessPath(m_program));
+    m_process->setArguments({});
 }
 
 void IpcServerProcess::setWorkingDirectory(const QString &dir)
@@ -92,4 +102,20 @@ QByteArray IpcServerProcess::readAllStandardOutput()
     return m_process->readAllStandardOutput();
 }
 
+bool IpcServerProcess::waitForStarted() {
+    return m_process->waitForStarted();
+}
+
+bool IpcServerProcess::waitForStarted(int msecs) {
+    return m_process->waitForStarted(msecs);
+}
+
+bool IpcServerProcess::waitForFinished() {
+    return m_process->waitForFinished();
+}
+
+bool IpcServerProcess::waitForFinished(int msecs) {
+    return m_process->waitForFinished(msecs);
+}
+
 #endif

ipc/ipcserverprocess.h

@@ -1,6 +1,7 @@
 #ifndef IPCSERVERPROCESS_H
 #define IPCSERVERPROCESS_H
 
+#include "ipc.h"
 #include <QObject>
 
 #ifndef Q_OS_IOS
@@ -14,6 +15,8 @@ public:
     virtual ~IpcServerProcess();
 
     void start() override;
+    void terminate() override;
+    void kill() override;
     void close() override;
 
     void setArguments(const QStringList &arguments) override;
@@ -27,9 +30,15 @@ public:
     QByteArray readAllStandardError() override;
     QByteArray readAllStandardOutput() override;
 
+    bool waitForStarted() override;
+    bool waitForStarted(int msecs) override;
+    bool waitForFinished() override;
+    bool waitForFinished(int msecs) override;
+
 signals:
 
 private:
+    amnezia::PermittedProcess m_program = amnezia::PermittedProcess::Invalid;
     QSharedPointer<QProcess> m_process;
 };
 

ipc/ipctun2socksprocess.cpp

@@ -1,75 +0,0 @@
-#include "ipctun2socksprocess.h"
-#include "ipc.h"
-#include <QProcess>
-#include <QString>
-
-#include "../protocols/protocols_defs.h"
-
-#ifndef Q_OS_IOS
-
-IpcProcessTun2Socks::IpcProcessTun2Socks(QObject *parent) :
-    IpcProcessTun2SocksSource(parent),
-    m_t2sProcess(QSharedPointer<QProcess>(new QProcess()))
-{
-    qDebug() << "IpcProcessTun2Socks::IpcProcessTun2Socks()";
-
-}
-
-IpcProcessTun2Socks::~IpcProcessTun2Socks()
-{
-    qDebug() << "IpcProcessTun2Socks::~IpcProcessTun2Socks()";
-}
-
-void IpcProcessTun2Socks::start()
-{
-    connect(m_t2sProcess.data(), &QProcess::stateChanged, this, &IpcProcessTun2Socks::stateChanged);
-    qDebug() << "IpcProcessTun2Socks::start()";
-    m_t2sProcess->setProgram(amnezia::permittedProcessPath(static_cast<amnezia::PermittedProcess>(amnezia::PermittedProcess::Tun2Socks)));
-
-    QString XrayConStr = "socks5://127.0.0.1:10808";
-
-#ifdef Q_OS_WIN
-    QStringList arguments({"-device", "tun://tun2?guid={081A8A84-8D12-4DF5-B8C4-396D5B0053E4}", "-proxy", XrayConStr });
-#endif
-#ifdef Q_OS_LINUX
-    QStringList arguments({"-device", "tun://tun2", "-proxy", XrayConStr});
-#endif
-#ifdef Q_OS_MAC
-    QStringList arguments({"-device", "utun22", "-proxy", XrayConStr});
-#endif
-
-    m_t2sProcess->setArguments(arguments);
-
-    if (Utils::processIsRunning(Utils::executable("tun2socks", false))) {
-        qDebug().noquote() << "kill previos tun2socks";
-        Utils::killProcessByName(Utils::executable("tun2socks", false));
-    }
-
-    connect(m_t2sProcess.data(), &QProcess::readyReadStandardOutput, this, [this]() {
-        QString line = m_t2sProcess.data()->readAllStandardOutput();
-        if (line.contains("[STACK] tun://") && line.contains("<-> socks5://127.0.0.1")) {
-            emit setConnectionState(Vpn::ConnectionState::Connected);
-        }
-    });
-
-    connect(m_t2sProcess.data(), QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished), this, [this](int exitCode, QProcess::ExitStatus exitStatus) {
-        qDebug().noquote() << "tun2socks finished, exitCode, exiStatus" << exitCode << exitStatus;
-        emit setConnectionState(Vpn::ConnectionState::Disconnected);
-        if ((exitStatus != QProcess::NormalExit) || (exitCode != 0)) {
-            emit setConnectionState(Vpn::ConnectionState::Error);
-        }
-
-    });
-
-    m_t2sProcess->start();
-    m_t2sProcess->waitForStarted();
-}
-
-void IpcProcessTun2Socks::stop()
-{
-    qDebug() << "IpcProcessTun2Socks::stop()";
-    m_t2sProcess->disconnect();
-    m_t2sProcess->kill();
-    m_t2sProcess->waitForFinished(3000);
-}
-#endif

ipc/ipctun2socksprocess.h

@@ -1,52 +0,0 @@
-#ifndef IPCTUN2SOCKSPROCESS_H
-#define IPCTUN2SOCKSPROCESS_H
-
-#include <QObject>
-
-#ifndef Q_OS_IOS
-#include "rep_ipc_process_tun2socks_source.h"
-
-namespace Vpn
-{
-Q_NAMESPACE
-    enum ConnectionState {
-        Unknown,
-        Disconnected,
-        Preparing,
-        Connecting,
-        Connected,
-        Disconnecting,
-        Reconnecting,
-        Error
-    };
-Q_ENUM_NS(ConnectionState)
-}
-
-
-class IpcProcessTun2Socks : public IpcProcessTun2SocksSource
-{
-    Q_OBJECT
-public:
-    explicit IpcProcessTun2Socks(QObject *parent = nullptr);
-    virtual ~IpcProcessTun2Socks();
-
-    void start() override;
-    void stop() override;
-
-signals:
-
-private:
-    QSharedPointer<QProcess> m_t2sProcess;
-};
-
-#else
-class IpcProcessTun2Socks : public QObject
-{
-    Q_OBJECT
-
-public:
-    explicit IpcProcessTun2Socks(QObject *parent = nullptr);
-};
-#endif
-
-#endif // IPCTUN2SOCKSPROCESS_H