mirror of
https://github.com/amnezia-vpn/amnezia-client.git
synced 2026-06-21 02:01:03 +07:00
fix: resolve critical IPC security vulnerabilities
- Validate IP/CIDR values from IPC before passing to Linux firewall - Replace shell interpolation with direct execve in firewall update functions - Block dangerous OpenVPN/WireGuard arguments in sanitizeArguments() - Add programId bounds check in IpcServerProcess::setProgram() - Add SO_PEERCRED peer authentication for IPC connections on Linux
This commit is contained in:
@@ -448,16 +448,33 @@ void LinuxFirewall::updateDNSServers(const QStringList& servers)
|
||||
static QStringList existingServers {};
|
||||
|
||||
existingServers = servers;
|
||||
execute(QStringLiteral("iptables -F %1.320.allowDNS").arg(kAnchorName));
|
||||
for (const QString& rule : getDNSRules(servers))
|
||||
execute(QStringLiteral("iptables -A %1.320.allowDNS %2").arg(kAnchorName, rule));
|
||||
const QString chain = QStringLiteral("%1.320.allowDNS").arg(kAnchorName);
|
||||
executeIptables(QStringLiteral("iptables"), {QStringLiteral("-F"), chain});
|
||||
const QStringList ifaces = {
|
||||
QStringLiteral("amn0+"), QStringLiteral("tun0+"), QStringLiteral("tun2+")
|
||||
};
|
||||
for (const QString& server : servers) {
|
||||
for (const QString& iface : ifaces) {
|
||||
executeIptables(QStringLiteral("iptables"),
|
||||
{QStringLiteral("-A"), chain, QStringLiteral("-o"), iface,
|
||||
QStringLiteral("-d"), server, QStringLiteral("-p"), QStringLiteral("udp"),
|
||||
QStringLiteral("--dport"), QStringLiteral("53"), QStringLiteral("-j"), QStringLiteral("ACCEPT")});
|
||||
executeIptables(QStringLiteral("iptables"),
|
||||
{QStringLiteral("-A"), chain, QStringLiteral("-o"), iface,
|
||||
QStringLiteral("-d"), server, QStringLiteral("-p"), QStringLiteral("tcp"),
|
||||
QStringLiteral("--dport"), QStringLiteral("53"), QStringLiteral("-j"), QStringLiteral("ACCEPT")});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void LinuxFirewall::updateAllowNets(const QStringList& servers)
|
||||
{
|
||||
execute(QStringLiteral("iptables -F %1.110.allowNets").arg(kAnchorName));
|
||||
for (const QString& rule : getAllowRule(servers))
|
||||
execute(QStringLiteral("iptables -A %1.110.allowNets %2").arg(kAnchorName, rule));
|
||||
const QString chain = QStringLiteral("%1.110.allowNets").arg(kAnchorName);
|
||||
executeIptables(QStringLiteral("iptables"), {QStringLiteral("-F"), chain});
|
||||
for (const QString& server : servers)
|
||||
executeIptables(QStringLiteral("iptables"),
|
||||
{QStringLiteral("-A"), chain, QStringLiteral("-d"), server,
|
||||
QStringLiteral("-j"), QStringLiteral("ACCEPT")});
|
||||
}
|
||||
|
||||
void LinuxFirewall::updateBlockNets(const QStringList& servers)
|
||||
@@ -465,9 +482,12 @@ void LinuxFirewall::updateBlockNets(const QStringList& servers)
|
||||
static QStringList existingServers {};
|
||||
|
||||
existingServers = servers;
|
||||
execute(QStringLiteral("iptables -F %1.120.blockNets").arg(kAnchorName));
|
||||
for (const QString& rule : getBlockRule(servers))
|
||||
execute(QStringLiteral("iptables -A %1.120.blockNets %2").arg(kAnchorName, rule));
|
||||
const QString chain = QStringLiteral("%1.120.blockNets").arg(kAnchorName);
|
||||
executeIptables(QStringLiteral("iptables"), {QStringLiteral("-F"), chain});
|
||||
for (const QString& server : servers)
|
||||
executeIptables(QStringLiteral("iptables"),
|
||||
{QStringLiteral("-A"), chain, QStringLiteral("-d"), server,
|
||||
QStringLiteral("-j"), QStringLiteral("REJECT")});
|
||||
}
|
||||
|
||||
int waitForExitCode(QProcess& process)
|
||||
@@ -500,6 +520,24 @@ int LinuxFirewall::execute(const QString &command, bool ignoreErrors)
|
||||
return exitCode;
|
||||
}
|
||||
|
||||
int LinuxFirewall::executeIptables(const QString &program, const QStringList &args, bool ignoreErrors)
|
||||
{
|
||||
QProcess p;
|
||||
p.start(program, args, QProcess::ReadOnly);
|
||||
p.closeWriteChannel();
|
||||
|
||||
int exitCode = waitForExitCode(p);
|
||||
auto out = p.readAllStandardOutput().trimmed();
|
||||
auto err = p.readAllStandardError().trimmed();
|
||||
if ((exitCode != 0 || !err.isEmpty()) && !ignoreErrors)
|
||||
logger.warning() << "(" << exitCode << ") $ " << program << args.join(QLatin1Char(' '));
|
||||
if (!out.isEmpty())
|
||||
logger.info() << out;
|
||||
if (!err.isEmpty())
|
||||
logger.warning() << err;
|
||||
return exitCode;
|
||||
}
|
||||
|
||||
void LinuxFirewall::setupTrafficSplitting()
|
||||
{
|
||||
auto cGroupDir = "/sys/fs/cgroup/net_cls/" BRAND_CODE "vpnexclusions/";
|
||||
|
||||
@@ -85,6 +85,7 @@ private:
|
||||
static void setupTrafficSplitting();
|
||||
static void teardownTrafficSplitting();
|
||||
static int execute(const QString& command, bool ignoreErrors = false);
|
||||
static int executeIptables(const QString& program, const QStringList& args, bool ignoreErrors = false);
|
||||
private:
|
||||
// Chain names
|
||||
static QString kOutputChain, kRootChain, kPostRoutingChain, kPreRoutingChain;
|
||||
|
||||
@@ -3,6 +3,8 @@
|
||||
|
||||
#include <QObject>
|
||||
#include <QString>
|
||||
#include <QRegularExpression>
|
||||
#include <QSet>
|
||||
|
||||
#include "../client/utilities.h"
|
||||
|
||||
@@ -15,7 +17,8 @@ enum PermittedProcess {
|
||||
OpenVPN,
|
||||
Wireguard,
|
||||
Tun2Socks,
|
||||
CertUtil
|
||||
CertUtil,
|
||||
_Count
|
||||
};
|
||||
|
||||
inline QString permittedProcessPath(PermittedProcess pid)
|
||||
@@ -57,16 +60,56 @@ inline QStringList sanitizeArguments(PermittedProcess proc, const QStringList &a
|
||||
QList<Validator> positionalArgs;
|
||||
|
||||
switch (proc) {
|
||||
case OpenVPN: {
|
||||
static const QSet<QString> blocked = {
|
||||
QStringLiteral("--script-security"),
|
||||
QStringLiteral("--up"),
|
||||
QStringLiteral("--down"),
|
||||
QStringLiteral("--route-up"),
|
||||
QStringLiteral("--ipchange"),
|
||||
QStringLiteral("--tls-verify"),
|
||||
QStringLiteral("--plugin"),
|
||||
QStringLiteral("--auth-user-pass-verify"),
|
||||
QStringLiteral("--learn-address"),
|
||||
QStringLiteral("--client-connect"),
|
||||
QStringLiteral("--client-disconnect"),
|
||||
QStringLiteral("--management"),
|
||||
QStringLiteral("--management-external-key")
|
||||
};
|
||||
QStringList out;
|
||||
for (int i = 0; i < args.size(); ++i) {
|
||||
if (blocked.contains(args[i])) {
|
||||
qWarning() << "IPC: blocked OpenVPN argument:" << args[i];
|
||||
++i; // skip following value
|
||||
continue;
|
||||
}
|
||||
out << args[i];
|
||||
}
|
||||
return out;
|
||||
}
|
||||
case Wireguard: {
|
||||
static const QRegularExpression hookRe(
|
||||
QStringLiteral(R"((?i)(PostUp|PreUp|PostDown|PreDown)\s*=)"));
|
||||
QStringList out;
|
||||
for (const QString& a : args) {
|
||||
if (hookRe.match(a).hasMatch()) {
|
||||
qWarning() << "IPC: blocked WireGuard hook argument:" << a;
|
||||
continue;
|
||||
}
|
||||
out << a;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
case Tun2Socks:
|
||||
namedArgs["-device"] = [](const QString& v) { return v.startsWith("tun://"); };
|
||||
namedArgs["-proxy"] = [](const QString& v) { return v.startsWith("socks5://"); };
|
||||
break;
|
||||
default:
|
||||
//FIXME
|
||||
case CertUtil:
|
||||
return args;
|
||||
default:
|
||||
return {};
|
||||
}
|
||||
|
||||
|
||||
QStringList sanitized;
|
||||
|
||||
for (int i = 0, pos = 0; i < args.size(); i++) {
|
||||
|
||||
+30
-1
@@ -22,6 +22,27 @@
|
||||
#include "tapcontroller_win.h"
|
||||
#endif
|
||||
|
||||
#ifdef Q_OS_LINUX
|
||||
#include <sys/socket.h>
|
||||
#include <sys/types.h>
|
||||
|
||||
extern uid_t g_allowedUid;
|
||||
extern bool g_allowedUidSet;
|
||||
|
||||
static bool checkPrivPeerCredentials(QLocalSocket *socket) {
|
||||
struct ucred cred{};
|
||||
socklen_t len = sizeof(cred);
|
||||
if (getsockopt(socket->socketDescriptor(), SOL_SOCKET, SO_PEERCRED, &cred, &len) != 0) {
|
||||
qWarning() << "IpcServer: SO_PEERCRED failed, rejecting privileged process connection";
|
||||
return false;
|
||||
}
|
||||
if (cred.uid == 0) return true;
|
||||
if (g_allowedUidSet && cred.uid == g_allowedUid) return true;
|
||||
qWarning() << "IpcServer: rejected privileged process connection from unauthorized UID" << cred.uid;
|
||||
return false;
|
||||
}
|
||||
#endif
|
||||
|
||||
|
||||
IpcServer::IpcServer(QObject *parent) : IpcInterfaceSource(parent)
|
||||
{
|
||||
@@ -48,8 +69,16 @@ int IpcServer::createPrivilegedProcess()
|
||||
// Make sure any connections are handed to QtRO
|
||||
QObject::connect(pd.localServer.data(), &QLocalServer::newConnection, this, [pd]() {
|
||||
qDebug() << "IpcServer new connection";
|
||||
QLocalSocket *conn = pd.localServer->nextPendingConnection();
|
||||
#ifdef Q_OS_LINUX
|
||||
if (!checkPrivPeerCredentials(conn)) {
|
||||
conn->close();
|
||||
conn->deleteLater();
|
||||
return;
|
||||
}
|
||||
#endif
|
||||
if (pd.serverNode) {
|
||||
pd.serverNode->addHostSideConnection(pd.localServer->nextPendingConnection());
|
||||
pd.serverNode->addHostSideConnection(conn);
|
||||
pd.serverNode->enableRemoting(pd.ipcProcess.data());
|
||||
}
|
||||
});
|
||||
|
||||
@@ -77,6 +77,11 @@ void IpcServerProcess::setProcessChannelMode(QProcess::ProcessChannelMode mode)
|
||||
|
||||
void IpcServerProcess::setProgram(int programId)
|
||||
{
|
||||
if (programId <= static_cast<int>(amnezia::PermittedProcess::Invalid) ||
|
||||
programId >= static_cast<int>(amnezia::PermittedProcess::_Count)) {
|
||||
qWarning() << "IPC: invalid programId" << programId << ", ignoring";
|
||||
return;
|
||||
}
|
||||
m_program = static_cast<amnezia::PermittedProcess>(programId);
|
||||
m_process->setProgram(amnezia::permittedProcessPath(m_program));
|
||||
m_process->setArguments({});
|
||||
|
||||
@@ -3,11 +3,43 @@
|
||||
|
||||
#include <QApplication>
|
||||
#include <QHostAddress>
|
||||
#include <QRegularExpression>
|
||||
|
||||
#include "../client/protocols/protocols_defs.h"
|
||||
#include "qjsonarray.h"
|
||||
#include "version.h"
|
||||
|
||||
#ifdef Q_OS_LINUX
|
||||
static bool isValidIpOrCidr(const QString &value) {
|
||||
static const QRegularExpression re(
|
||||
QStringLiteral(R"(^(\d{1,3}\.){3}\d{1,3}(/\d{1,2})?$)"));
|
||||
if (!re.match(value).hasMatch()) return false;
|
||||
const QStringList ipParts = value.split(QLatin1Char('/'))[0].split(QLatin1Char('.'));
|
||||
for (const QString &part : ipParts) {
|
||||
bool ok;
|
||||
int octet = part.toInt(&ok);
|
||||
if (!ok || octet < 0 || octet > 255) return false;
|
||||
}
|
||||
if (value.contains(QLatin1Char('/'))) {
|
||||
bool ok;
|
||||
int prefix = value.split(QLatin1Char('/'))[1].toInt(&ok);
|
||||
if (!ok || prefix < 0 || prefix > 32) return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
static QStringList filterIpList(const QStringList &values) {
|
||||
QStringList safe;
|
||||
for (const QString &v : values) {
|
||||
if (isValidIpOrCidr(v))
|
||||
safe << v;
|
||||
else
|
||||
qWarning() << "IPC: rejected invalid IP/CIDR value:" << v;
|
||||
}
|
||||
return safe;
|
||||
}
|
||||
#endif
|
||||
|
||||
#ifdef Q_OS_WIN
|
||||
#include "../client/platforms/windows/daemon/windowsfirewall.h"
|
||||
#include "../client/platforms/windows/daemon/windowsdaemon.h"
|
||||
@@ -159,7 +191,11 @@ bool KillSwitch::disableAllTraffic() {
|
||||
|
||||
bool KillSwitch::resetAllowedRange(const QStringList &ranges) {
|
||||
|
||||
#ifdef Q_OS_LINUX
|
||||
m_allowedRanges = filterIpList(ranges);
|
||||
#else
|
||||
m_allowedRanges = ranges;
|
||||
#endif
|
||||
|
||||
#ifdef Q_OS_LINUX
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv4, QStringLiteral("110.allowNets"), true);
|
||||
@@ -182,7 +218,12 @@ bool KillSwitch::resetAllowedRange(const QStringList &ranges) {
|
||||
}
|
||||
|
||||
bool KillSwitch::addAllowedRange(const QStringList &ranges) {
|
||||
for (const QString &range : ranges) {
|
||||
#ifdef Q_OS_LINUX
|
||||
const QStringList safeRanges = filterIpList(ranges);
|
||||
#else
|
||||
const QStringList &safeRanges = ranges;
|
||||
#endif
|
||||
for (const QString &range : safeRanges) {
|
||||
if (!range.isEmpty() && !m_allowedRanges.contains(range)) {
|
||||
m_allowedRanges.append(range);
|
||||
}
|
||||
@@ -307,9 +348,9 @@ bool KillSwitch::enableKillSwitch(const QJsonObject &configStr, int vpnAdapterIn
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::Both, QStringLiteral("000.allowLoopback"), true);
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::Both, QStringLiteral("100.blockAll"), blockAll);
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv4, QStringLiteral("110.allowNets"), allowNets);
|
||||
LinuxFirewall::updateAllowNets(allownets);
|
||||
LinuxFirewall::updateAllowNets(filterIpList(allownets));
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv4, QStringLiteral("120.blockNets"), blockAll);
|
||||
LinuxFirewall::updateBlockNets(blocknets);
|
||||
LinuxFirewall::updateBlockNets(filterIpList(blocknets));
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv4, QStringLiteral("200.allowVPN"), true);
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv6, QStringLiteral("250.blockIPv6"), true);
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::Both, QStringLiteral("290.allowDHCP"), true);
|
||||
@@ -317,23 +358,35 @@ bool KillSwitch::enableKillSwitch(const QJsonObject &configStr, int vpnAdapterIn
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv4, QStringLiteral("310.blockDNS"), true);
|
||||
QStringList dnsServers;
|
||||
|
||||
dnsServers.append(configStr.value(amnezia::config_key::dns1).toString());
|
||||
const QString dns1 = configStr.value(amnezia::config_key::dns1).toString();
|
||||
if (isValidIpOrCidr(dns1))
|
||||
dnsServers.append(dns1);
|
||||
else if (!dns1.isEmpty())
|
||||
qWarning() << "IPC: rejected invalid dns1:" << dns1;
|
||||
|
||||
// We don't use secondary DNS if primary DNS is AmneziaDNS
|
||||
if (!configStr.value(amnezia::config_key::dns1).toString().contains(amnezia::protocols::dns::amneziaDnsIp)) {
|
||||
dnsServers.append(configStr.value(amnezia::config_key::dns2).toString());
|
||||
if (!dns1.contains(amnezia::protocols::dns::amneziaDnsIp)) {
|
||||
const QString dns2 = configStr.value(amnezia::config_key::dns2).toString();
|
||||
if (isValidIpOrCidr(dns2))
|
||||
dnsServers.append(dns2);
|
||||
else if (!dns2.isEmpty())
|
||||
qWarning() << "IPC: rejected invalid dns2:" << dns2;
|
||||
}
|
||||
|
||||
dnsServers.append("127.0.0.1");
|
||||
dnsServers.append("127.0.0.53");
|
||||
|
||||
|
||||
for (auto dns : configStr.value(amnezia::config_key::allowedDnsServers).toArray()) {
|
||||
if (!dns.isString()) {
|
||||
break;
|
||||
}
|
||||
dnsServers.append(dns.toString());
|
||||
const QString dnsStr = dns.toString();
|
||||
if (isValidIpOrCidr(dnsStr))
|
||||
dnsServers.append(dnsStr);
|
||||
else if (!dnsStr.isEmpty())
|
||||
qWarning() << "IPC: rejected invalid allowedDnsServer:" << dnsStr;
|
||||
}
|
||||
|
||||
|
||||
LinuxFirewall::updateDNSServers(dnsServers);
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::IPv4, QStringLiteral("320.allowDNS"), true);
|
||||
LinuxFirewall::setAnchorEnabled(LinuxFirewall::Both, QStringLiteral("400.allowPIA"), true);
|
||||
|
||||
@@ -17,6 +17,35 @@
|
||||
#include "tapcontroller_win.h"
|
||||
#endif
|
||||
|
||||
#ifdef Q_OS_LINUX
|
||||
#include <sys/socket.h>
|
||||
#include <sys/types.h>
|
||||
#include <unistd.h>
|
||||
|
||||
uid_t g_allowedUid = static_cast<uid_t>(-1);
|
||||
bool g_allowedUidSet = false;
|
||||
|
||||
static bool checkPeerCredentials(QLocalSocket *socket) {
|
||||
struct ucred cred{};
|
||||
socklen_t len = sizeof(cred);
|
||||
if (getsockopt(socket->socketDescriptor(), SOL_SOCKET, SO_PEERCRED, &cred, &len) != 0) {
|
||||
qWarning() << "LocalServer: SO_PEERCRED failed, rejecting connection";
|
||||
return false;
|
||||
}
|
||||
if (cred.uid == 0) return true;
|
||||
if (!g_allowedUidSet) {
|
||||
g_allowedUid = cred.uid;
|
||||
g_allowedUidSet = true;
|
||||
qDebug() << "LocalServer: registered session UID" << g_allowedUid;
|
||||
}
|
||||
if (cred.uid != g_allowedUid) {
|
||||
qWarning() << "LocalServer: rejected connection from unauthorized UID" << cred.uid;
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
#endif
|
||||
|
||||
namespace {
|
||||
Logger logger("WgDaemonServer");
|
||||
}
|
||||
@@ -35,7 +64,15 @@ LocalServer::LocalServer(QObject *parent) : QObject(parent),
|
||||
|
||||
QObject::connect(m_server.data(), &QLocalServer::newConnection, this, [this]() {
|
||||
qDebug() << "LocalServer new connection";
|
||||
m_serverNode.addHostSideConnection(m_server->nextPendingConnection());
|
||||
QLocalSocket *conn = m_server->nextPendingConnection();
|
||||
#ifdef Q_OS_LINUX
|
||||
if (!checkPeerCredentials(conn)) {
|
||||
conn->close();
|
||||
conn->deleteLater();
|
||||
return;
|
||||
}
|
||||
#endif
|
||||
m_serverNode.addHostSideConnection(conn);
|
||||
|
||||
if (!m_isRemotingEnabled) {
|
||||
m_isRemotingEnabled = true;
|
||||
|
||||
Reference in New Issue
Block a user