[lipstick] Revert broken window model protection. Contributes to JB#...

On this setup calledFromDBus() was always returning false. This reverts commit 24ed407.
sailfishos · Jan 3, 2019 · 8c55e94 · 8c55e94
1 parent 8f897ee
commit 8c55e94
Show file tree

Hide file tree

Showing 11 changed files with 58 additions and 106 deletions.
diff --git a/src/compositor/windowmodel.cpp b/src/compositor/windowmodel.cpp
@@ -173,7 +173,7 @@ void WindowModel::refresh()
 void WindowModel::launchProcess(const QString &binaryName)
 {
     LipstickCompositor *c = LipstickCompositor::instance();
-    if (!m_complete || !c || !isPrivileged())
+    if (!m_complete || !c)
         return;
 
     QStringList binaryParts = binaryName.split(QRegExp(QRegExp("\\s+")));

diff --git a/src/compositor/windowmodel.h b/src/compositor/windowmodel.h
@@ -18,16 +18,13 @@
 
 #include "lipstickdbus.h"
 #include "lipstickglobal.h"
-#include "privilegeddbuscontext.h"
-
 #include <QQmlParserStatus>
 #include <QAbstractListModel>
 
 class LipstickCompositor;
 class LipstickCompositorWindow;
 class LIPSTICK_EXPORT WindowModel : public QAbstractListModel,
-                                    public QQmlParserStatus,
-                                    public PrivilegedDBusContext
+                                    public QQmlParserStatus
 {
     Q_OBJECT
     Q_INTERFACES(QQmlParserStatus)

diff --git a/src/notifications/notificationmanager.cpp b/src/notifications/notificationmanager.cpp
@@ -138,6 +138,7 @@ NotificationManager *NotificationManager::instance(bool owner)
 
 NotificationManager::NotificationManager(QObject *parent, bool owner) :
     QObject(parent),
+    QDBusContext(),
     m_previousNotificationID(0),
     m_categoryDefinitionStore(new CategoryDefinitionStore(CATEGORY_DEFINITION_FILE_DIRECTORY, MAX_CATEGORY_DEFINITION_FILES, this)),
     m_androidPriorityStore(new AndroidPriorityStore(ANDROID_PRIORITY_DEFINITION_PATH, this)),
@@ -1063,6 +1064,31 @@ void NotificationManager::execSQL(const QString &command, const QVariantList &ar
     m_databaseCommitTimer.start();
 }
 
+uint NotificationManager::callerProcessId() const
+{
+    if (calledFromDBus()) {
+        return connection().interface()->servicePid(message().service()).value();
+    } else {
+        return QCoreApplication::applicationPid();
+    }
+}
+
+bool NotificationManager::isPrivileged() const
+{
+    if (!calledFromDBus()) {
+        return true;
+    }
+
+    uint pid = callerProcessId();
+    QFileInfo info(QString("/proc/%1").arg(pid));
+    if (info.group() != QLatin1String("privileged") && info.owner() != QLatin1String("root")) {
+        QString errorString = QString("PID %1 is not in privileged group").arg(pid);
+        sendErrorReply(QDBusError::AccessDenied, errorString);
+        return false;
+    }
+    return true;
+}
+
 void NotificationManager::invokeAction(const QString &action)
 {
     LipstickNotification *notification = qobject_cast<LipstickNotification *>(sender());

diff --git a/src/notifications/notificationmanager.h b/src/notifications/notificationmanager.h
@@ -23,8 +23,6 @@
 #include <QSet>
 #include <QDBusContext>
 
-#include <privilegeddbuscontext.h>
-
 class AndroidPriorityStore;
 class CategoryDefinitionStore;
 class QSqlDatabase;
@@ -39,7 +37,7 @@ class QSqlDatabase;
  * The service is registered as org.freedesktop.Notifications on the D-Bus
  * session bus in the path /org/freedesktop/Notifications.
  */
-class LIPSTICK_EXPORT NotificationManager : public QObject, public PrivilegedDBusContext
+class LIPSTICK_EXPORT NotificationManager : public QObject, public QDBusContext
 {
     Q_OBJECT
 
@@ -384,6 +382,9 @@ private slots:
      */
     void execSQL(const QString &command, const QVariantList &args = QVariantList());
 
+    uint callerProcessId() const;
+    bool isPrivileged() const;
+
     //! The singleton notification manager instance
     static NotificationManager *s_instance;
 

diff --git a/src/privilegeddbuscontext.cpp b/src/privilegeddbuscontext.cpp
diff --git a/src/privilegeddbuscontext.h b/src/privilegeddbuscontext.h
diff --git a/src/shutdownscreen.cpp b/src/shutdownscreen.cpp
@@ -29,6 +29,7 @@
 
 ShutdownScreen::ShutdownScreen(QObject *parent) :
     QObject(parent),
+    QDBusContext(),
     m_window(0),
     m_systemState(new MeeGo::QmSystemState(this))
 {
@@ -119,3 +120,24 @@ void ShutdownScreen::setShutdownMode(const QString &mode)
     m_shutdownMode = mode;
     applySystemState(MeeGo::QmSystemState::Shutdown);
 }
+
+bool ShutdownScreen::isPrivileged()
+{
+    if (!calledFromDBus()) {
+        // Local function calls are always privileged
+        return true;
+    }
+
+    // Get the PID of the calling process
+    pid_t pid = connection().interface()->servicePid(message().service());
+
+    // The /proc/<pid> directory is owned by EUID:EGID of the process
+    QFileInfo info(QString("/proc/%1").arg(pid));
+    if (info.group() != "privileged" && info.owner() != "root") {
+        sendErrorReply(QDBusError::AccessDenied,
+                QString("PID %1 is not in privileged group").arg(pid));
+        return false;
+    }
+
+    return true;
+}
diff --git a/src/shutdownscreen.h b/src/shutdownscreen.h
@@ -20,11 +20,10 @@
 #include <QDBusContext>
 #include "lipstickglobal.h"
 #include <qmsystemstate.h>
-#include <privilegeddbuscontext.h>
 
 class HomeWindow;
 
-class LIPSTICK_EXPORT ShutdownScreen : public QObject, protected PrivilegedDBusContext
+class LIPSTICK_EXPORT ShutdownScreen : public QObject, protected QDBusContext
 {
     Q_OBJECT
     Q_PROPERTY(bool windowVisible READ windowVisible WRITE setWindowVisible NOTIFY windowVisibleChanged)
@@ -87,6 +86,8 @@ private slots:
 #ifdef UNIT_TEST
     friend class Ut_ShutdownScreen;
 #endif
+
+    bool isPrivileged();
 };
 
 #endif // SHUTDOWNSCREEN_H
diff --git a/src/src.pro b/src/src.pro
@@ -54,8 +54,7 @@ PUBLICHEADERS += \
     qmsystem2/system_global.h \
     vpnagent.h \
     connectivitymonitor.h \
-    connectionselector.h \
-    privilegeddbuscontext.h
+    connectionselector.h
 
 INSTALLS += publicheaderfiles dbus_policy
 publicheaderfiles.files = $$PUBLICHEADERS
@@ -138,7 +137,6 @@ SOURCES += \
     qmsystem2/qmsystemstate.cpp \
     qmsystem2/qmthermal.cpp \
     qmsystem2/qmipcinterface.cpp \
-    privilegeddbuscontext.cpp \
 
 CONFIG += link_pkgconfig mobility qt warn_on depend_includepath qmake_cache target_qt
 CONFIG -= link_prl

diff --git a/tests/ut_notificationmanager/ut_notificationmanager.pro b/tests/ut_notificationmanager/ut_notificationmanager.pro
@@ -8,7 +8,6 @@ PKGCONFIG += mlite5
 # unit test and unit
 SOURCES += \
     ut_notificationmanager.cpp \
-    $$SRCDIR/privilegeddbuscontext.cpp \
     $$NOTIFICATIONSRCDIR/notificationmanager.cpp \
     $$NOTIFICATIONSRCDIR/lipsticknotification.cpp \
     $$STUBSDIR/stubbase.cpp \

diff --git a/tests/ut_shutdownscreen/ut_shutdownscreen.pro b/tests/ut_shutdownscreen/ut_shutdownscreen.pro
@@ -9,7 +9,6 @@ PKGCONFIG += dsme_dbus_if thermalmanager_dbus_if usb_moded
 # unit test and unit
 SOURCES += \
     $$SRCDIR/shutdownscreen.cpp \
-    $$SRCDIR/privilegeddbuscontext.cpp \
     $$NOTIFICATIONSRCDIR/lipsticknotification.cpp \
     $$STUBSDIR/stubbase.cpp \
     $$STUBSDIR/homewindow.cpp \