From 264e97625abe2e0334f97de17f6ffb52582888ab Mon Sep 17 00:00:00 2001 From: Albert Astals Cid Date: Wed, 10 May 2017 10:06:07 +0200 Subject: Verify that whoever is calling us is actually who he says he is CVE-2017-8422 --- kdecore/auth/AuthBackend.cpp | 5 ++++ kdecore/auth/AuthBackend.h | 7 ++++++ kdecore/auth/backends/dbus/DBusHelperProxy.cpp | 27 ++++++++++++++++++++-- kdecore/auth/backends/dbus/DBusHelperProxy.h | 6 ++++- .../auth/backends/policykit/PolicyKitBackend.cpp | 5 ++++ kdecore/auth/backends/policykit/PolicyKitBackend.h | 1 + kdecore/auth/backends/polkit-1/Polkit1Backend.cpp | 5 ++++ kdecore/auth/backends/polkit-1/Polkit1Backend.h | 1 + 8 files changed, 54 insertions(+), 3 deletions(-) diff --git a/kdecore/auth/AuthBackend.cpp b/kdecore/auth/AuthBackend.cpp index c953b81..0ba4650 100644 --- a/kdecore/auth/AuthBackend.cpp +++ b/kdecore/auth/AuthBackend.cpp @@ -54,6 +54,11 @@ void AuthBackend::setCapabilities(AuthBackend::Capabilities capabilities) d->capabilities = capabilities; } +AuthBackend::ExtraCallerIDVerificationMethod AuthBackend::extraCallerIDVerificationMethod() const +{ + return NoExtraCallerIDVerificationMethod; +} + bool AuthBackend::actionExists(const QString& action) { Q_UNUSED(action); diff --git a/kdecore/auth/AuthBackend.h b/kdecore/auth/AuthBackend.h index a86732e..6f4b1bc 100644 --- a/kdecore/auth/AuthBackend.h +++ b/kdecore/auth/AuthBackend.h @@ -43,6 +43,12 @@ public: }; Q_DECLARE_FLAGS(Capabilities, Capability) + enum ExtraCallerIDVerificationMethod { + NoExtraCallerIDVerificationMethod, + VerifyAgainstDBusServiceName, + VerifyAgainstDBusServicePid, + }; + AuthBackend(); virtual ~AuthBackend(); virtual void setupAction(const QString &action) = 0; @@ -50,6 +56,7 @@ public: virtual Action::AuthStatus authorizeAction(const QString &action) = 0; virtual Action::AuthStatus actionStatus(const QString &action) = 0; virtual QByteArray callerID() const = 0; + virtual ExtraCallerIDVerificationMethod extraCallerIDVerificationMethod() const; virtual bool isCallerAuthorized(const QString &action, QByteArray callerID) = 0; virtual bool actionExists(const QString &action); diff --git a/kdecore/auth/backends/dbus/DBusHelperProxy.cpp b/kdecore/auth/backends/dbus/DBusHelperProxy.cpp index 9557a0f..ca59f1c 100644 --- a/kdecore/auth/backends/dbus/DBusHelperProxy.cpp +++ b/kdecore/auth/backends/dbus/DBusHelperProxy.cpp @@ -271,6 +271,29 @@ void DBusHelperProxy::performActions(QByteArray blob, const QByteArray &callerID } } +bool DBusHelperProxy::isCallerAuthorized(const QString &action, const QByteArray &callerID) +{ + // Check the caller is really who it says it is + switch (BackendsManager::authBackend()->extraCallerIDVerificationMethod()) { + case AuthBackend::NoExtraCallerIDVerificationMethod: + break; + + case AuthBackend::VerifyAgainstDBusServiceName: + if (message().service().toUtf8() != callerID) { + return false; + } + break; + + case AuthBackend::VerifyAgainstDBusServicePid: + if (connection().interface()->servicePid(message().service()).value() != callerID.toUInt()) { + return false; + } + break; + } + + return BackendsManager::authBackend()->isCallerAuthorized(action, callerID); +} + QByteArray DBusHelperProxy::performAction(const QString &action, const QByteArray &callerID, QByteArray arguments) { if (!responder) { @@ -295,7 +318,7 @@ QByteArray DBusHelperProxy::performAction(const QString &action, const QByteArra QTimer *timer = responder->property("__KAuth_Helper_Shutdown_Timer").value(); timer->stop(); - if (BackendsManager::authBackend()->isCallerAuthorized(action, callerID)) { + if (isCallerAuthorized(action, callerID)) { QString slotname = action; if (slotname.startsWith(m_name + QLatin1Char('.'))) { slotname = slotname.right(slotname.length() - m_name.length() - 1); @@ -338,7 +361,7 @@ uint DBusHelperProxy::authorizeAction(const QString& action, const QByteArray& c QTimer *timer = responder->property("__KAuth_Helper_Shutdown_Timer").value(); timer->stop(); - if (BackendsManager::authBackend()->isCallerAuthorized(action, callerID)) { + if (isCallerAuthorized(action, callerID)) { retVal = static_cast(Action::Authorized); } else { retVal = static_cast(Action::Denied); diff --git a/kdecore/auth/backends/dbus/DBusHelperProxy.h b/kdecore/auth/backends/dbus/DBusHelperProxy.h index 455cf51..264f6cc 100644 --- a/kdecore/auth/backends/dbus/DBusHelperProxy.h +++ b/kdecore/auth/backends/dbus/DBusHelperProxy.h @@ -21,6 +21,7 @@ #ifndef DBUS_HELPER_PROXY_H #define DBUS_HELPER_PROXY_H +#include #include #include "HelperProxy.h" #include "kauthactionreply.h" @@ -28,7 +29,7 @@ namespace KAuth { -class DBusHelperProxy : public HelperProxy +class DBusHelperProxy : public HelperProxy, protected QDBusContext { Q_OBJECT Q_INTERFACES(KAuth::HelperProxy) @@ -73,6 +74,9 @@ signals: private slots: void remoteSignalReceived(int type, const QString &action, QByteArray blob); + +private: + bool isCallerAuthorized(const QString &action, const QByteArray &callerID); }; } // namespace Auth diff --git a/kdecore/auth/backends/policykit/PolicyKitBackend.cpp b/kdecore/auth/backends/policykit/PolicyKitBackend.cpp index 3be97f2..9d041d1 100644 --- a/kdecore/auth/backends/policykit/PolicyKitBackend.cpp +++ b/kdecore/auth/backends/policykit/PolicyKitBackend.cpp @@ -78,6 +78,11 @@ QByteArray PolicyKitBackend::callerID() const return a; } +AuthBackend::ExtraCallerIDVerificationMethod Polkit1Backend::extraCallerIDVerificationMethod() const +{ + return VerifyAgainstDBusServicePid; +} + bool PolicyKitBackend::isCallerAuthorized(const QString &action, QByteArray callerID) { QDataStream s(&callerID, QIODevice::ReadOnly); diff --git a/kdecore/auth/backends/policykit/PolicyKitBackend.h b/kdecore/auth/backends/policykit/PolicyKitBackend.h index 7154e93..0d3d8f9 100644 --- a/kdecore/auth/backends/policykit/PolicyKitBackend.h +++ b/kdecore/auth/backends/policykit/PolicyKitBackend.h @@ -40,6 +40,7 @@ public: virtual Action::AuthStatus authorizeAction(const QString&); virtual Action::AuthStatus actionStatus(const QString&); virtual QByteArray callerID() const; + virtual ExtraCallerIDVerificationMethod extraCallerIDVerificationMethod() const; virtual bool isCallerAuthorized(const QString &action, QByteArray callerID); private Q_SLOTS: diff --git a/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp b/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp index 732d2cb..63c0e1e 100644 --- a/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp +++ b/kdecore/auth/backends/polkit-1/Polkit1Backend.cpp @@ -163,6 +163,11 @@ QByteArray Polkit1Backend::callerID() const return QDBusConnection::systemBus().baseService().toUtf8(); } +AuthBackend::ExtraCallerIDVerificationMethod Polkit1Backend::extraCallerIDVerificationMethod() const +{ + return VerifyAgainstDBusServiceName; +} + bool Polkit1Backend::isCallerAuthorized(const QString &action, QByteArray callerID) { PolkitQt1::SystemBusNameSubject subject(QString::fromUtf8(callerID)); diff --git a/kdecore/auth/backends/polkit-1/Polkit1Backend.h b/kdecore/auth/backends/polkit-1/Polkit1Backend.h index 18ed1a2..d579da2 100644 --- a/kdecore/auth/backends/polkit-1/Polkit1Backend.h +++ b/kdecore/auth/backends/polkit-1/Polkit1Backend.h @@ -48,6 +48,7 @@ public: virtual Action::AuthStatus authorizeAction(const QString&); virtual Action::AuthStatus actionStatus(const QString&); virtual QByteArray callerID() const; + virtual ExtraCallerIDVerificationMethod extraCallerIDVerificationMethod() const; virtual bool isCallerAuthorized(const QString &action, QByteArray callerID); virtual bool actionExists(const QString& action); -- cgit v0.11.2