ScreenshotEffect: Use Service Property to authorize screenshot without confirmation

Summary:
Restrict to process with `X-KDE-DBUS-Restricted-Interfaces=org.kde.kwin.Screenshot` in their corresponding Service file,
 to take screenshots.
Such a program can now take immediate screenshots.

Adds a utility file to group KService related logic.

Needed for D29408

Reviewers: #kwin, apol, davidedmundson, bport, zzag

Reviewed By: #kwin, davidedmundson

Subscribers: ngraham, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D29407

data/org_kde_kwin.categories

@@ -1,4 +1,5 @@
 kwin_core KWin Core DEFAULT_SEVERITY [CRITICAL] IDENTIFIER [KWIN_CORE]
+kwin_utils KWin utils DEFAULT_SEVERITY [CRITICAL] IDENTIFIER [KWIN_UTILS]
 kwin_virtualkeyboard KWin Virtual Keyboard Integration DEFAULT_SEVERITY [CRITICAL] IDENTIFIER [KWIN_VIRTUALKEYBOARD]
 kwineffects KWin Effects DEFAULT_SEVERITY [CRITICAL] IDENTIFIER [KWINEFFECTS]
 libkwineffects KWin Effects Library DEFAULT_SEVERITY [CRITICAL] IDENTIFIER [LIBKWINEFFECTS]

effects/CMakeLists.txt

@@ -23,6 +23,7 @@ set(kwin_effect_KDE_LIBS
     KF5::Notifications # screenshot effect
     KF5::Plasma # screenedge effect
     KF5::WindowSystem
+    KF5::Service # utils / screenshot effect
 )
 
 if (HAVE_ACCESSIBILITY)
@@ -110,6 +111,7 @@ set(kwin4_effect_builtins_sources
     windowgeometry/windowgeometry.cpp
     wobblywindows/wobblywindows.cpp
     zoom/zoom.cpp
+    ../service_utils.cpp
 )
 
 if (HAVE_ACCESSIBILITY)

effects/screenshot/CMakeLists.txt

@@ -3,5 +3,6 @@
 
 # Source files
 set(kwin4_effect_builtins_sources ${kwin4_effect_builtins_sources}
+    ../service_utils.cpp
     screenshot/screenshot.cpp
 )

effects/screenshot/screenshot.cpp

@@ -27,6 +27,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <QTemporaryFile>
 #include <QDir>
 #include <QDBusConnection>
+#include <QDBusConnectionInterface>
+#include <QDBusReply>
 #include <QVarLengthArray>
 #include <QPainter>
 #include <QMatrix4x4>
@@ -37,6 +39,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <KNotification>
 
 #include <unistd.h>
+#include "../service_utils.h"
 
 class ComparableQPoint : public QPoint
 {
@@ -63,6 +66,8 @@ namespace KWin
 
 const static QString s_errorAlreadyTaking = QStringLiteral("org.kde.kwin.Screenshot.Error.AlreadyTaking");
 const static QString s_errorAlreadyTakingMsg = QStringLiteral("A screenshot is already been taken");
+const static QString s_errorNotAuthorized = QStringLiteral("org.kde.kwin.Screenshot.Error.NoAuthorized");
+const static QString s_errorNotAuthorizedMsg = QStringLiteral("The process is not authorized to take a screenshot");
 const static QString s_errorFd = QStringLiteral("org.kde.kwin.Screenshot.Error.FileDescriptor");
 const static QString s_errorFdMsg = QStringLiteral("No valid file descriptor");
 const static QString s_errorCancelled = QStringLiteral("org.kde.kwin.Screenshot.Error.Cancelled");
@@ -71,6 +76,7 @@ const static QString s_errorInvalidArea = QStringLiteral("org.kde.kwin.Screensho
 const static QString s_errorInvalidAreaMsg = QStringLiteral("Invalid area requested");
 const static QString s_errorInvalidScreen = QStringLiteral("org.kde.kwin.Screenshot.Error.InvalidScreen");
 const static QString s_errorInvalidScreenMsg = QStringLiteral("Invalid screen requested");
+const static QString s_dbusInterfaceName = QStringLiteral("org.kde.kwin.Screenshot");
 
 bool ScreenShotEffect::supported()
 {
@@ -496,13 +502,35 @@ void ScreenShotEffect::screenshotForWindow(qulonglong winid, int mask)
     }
 }
 
-QString ScreenShotEffect::interactive(int mask)
+bool ScreenShotEffect::checkCall() const
 {
     if (!calledFromDBus()) {
-        return QString();
+        return false;
     }
+
+    const QDBusReply<uint> reply = connection().interface()->servicePid(message().service());
+    if (reply.isValid()) {
+        const uint pid = reply.value();
+        const auto interfaces = KWin::fetchRestrictedDBusInterfacesFromPid(pid);
+        if (!interfaces.contains(s_dbusInterfaceName)) {
+            sendErrorReply(s_errorNotAuthorized, s_errorNotAuthorizedMsg);
+            qCWarning(KWINEFFECTS) << "Process " << pid << " tried to take a screenshot without being granted to DBus interface" << s_dbusInterfaceName;
+            return false;
+        }
+    } else {
+        return false;
+    }
+
     if (isTakingScreenshot()) {
         sendErrorReply(s_errorAlreadyTaking, s_errorAlreadyTakingMsg);
+        return false;
+    }
+    return true;
+}
+
+QString ScreenShotEffect::interactive(int mask)
+{
+    if (!checkCall()) {
         return QString();
     }
     m_type = (ScreenShotType) mask;
@@ -528,11 +556,7 @@ QString ScreenShotEffect::interactive(int mask)
 
 void ScreenShotEffect::interactive(QDBusUnixFileDescriptor fd, int mask)
 {
-    if (!calledFromDBus()) {
+    if (!checkCall()) {
-        return;
-    }
-    if (isTakingScreenshot()) {
-        sendErrorReply(s_errorAlreadyTaking, s_errorAlreadyTakingMsg);
         return;
     }
     m_fd = dup(fd.fileDescriptor());
@@ -581,11 +605,7 @@ void ScreenShotEffect::hideInfoMessage()
 
 QString ScreenShotEffect::screenshotFullscreen(bool captureCursor)
 {
-    if (!calledFromDBus()) {
+    if (!checkCall()) {
-        return QString();
-    }
-    if (isTakingScreenshot()) {
-        sendErrorReply(s_errorAlreadyTaking, s_errorAlreadyTakingMsg);
         return QString();
     }
     m_replyMessage = message();
@@ -598,11 +618,7 @@ QString ScreenShotEffect::screenshotFullscreen(bool captureCursor)
 
 void ScreenShotEffect::screenshotFullscreen(QDBusUnixFileDescriptor fd, bool captureCursor, bool shouldReturnNativeSize)
 {
-    if (!calledFromDBus()) {
+    if (!checkCall()) {
-        return;
-    }
-    if (isTakingScreenshot()) {
-        sendErrorReply(s_errorAlreadyTaking, s_errorAlreadyTakingMsg);
         return;
     }
     m_fd = dup(fd.fileDescriptor());
@@ -613,29 +629,13 @@ void ScreenShotEffect::screenshotFullscreen(QDBusUnixFileDescriptor fd, bool cap
     m_captureCursor = captureCursor;
     m_nativeSize = shouldReturnNativeSize;
 
-    showInfoMessage(InfoMessageMode::Screen);
+    m_scheduledGeometry = effects->virtualScreenGeometry();
-    effects->startInteractivePositionSelection(
+    effects->addRepaint(m_scheduledGeometry);
-        [this] (const QPoint &p) {
-            hideInfoMessage();
-            if (p == QPoint(-1, -1)) {
-                // error condition
-                close(m_fd);
-                m_fd = -1;
-            } else {
-                m_scheduledGeometry = effects->virtualScreenGeometry();
-                effects->addRepaint(m_scheduledGeometry);
-            }
-        }
-    );
 }
 
 QString ScreenShotEffect::screenshotScreen(int screen, bool captureCursor)
 {
-    if (!calledFromDBus()) {
+    if (!checkCall()) {
-        return QString();
-    }
-    if (isTakingScreenshot()) {
-        sendErrorReply(s_errorAlreadyTaking, s_errorAlreadyTakingMsg);
         return QString();
     }
     m_scheduledGeometry = effects->clientArea(FullScreenArea, screen, 0);
@@ -652,11 +652,7 @@ QString ScreenShotEffect::screenshotScreen(int screen, bool captureCursor)
 
 void ScreenShotEffect::screenshotScreen(QDBusUnixFileDescriptor fd, bool captureCursor)
 {
-    if (!calledFromDBus()) {
+    if (!checkCall()) {
-        return;
-    }
-    if (isTakingScreenshot()) {
-        sendErrorReply(s_errorAlreadyTaking, s_errorAlreadyTakingMsg);
         return;
     }
     m_fd = dup(fd.fileDescriptor());
@@ -689,11 +685,7 @@ void ScreenShotEffect::screenshotScreen(QDBusUnixFileDescriptor fd, bool capture
 
 QString ScreenShotEffect::screenshotArea(int x, int y, int width, int height, bool captureCursor)
 {
-    if (!calledFromDBus()) {
+    if (!checkCall()) {
-        return QString();
-    }
-    if (isTakingScreenshot()) {
-        sendErrorReply(s_errorAlreadyTaking, s_errorAlreadyTakingMsg);
         return QString();
     }
     m_scheduledGeometry = QRect(x, y, width, height);

effects/screenshot/screenshot.h

@@ -154,6 +154,7 @@ private:
     void hideInfoMessage();
     bool isTakingScreenshot() const;
     void computeCoordinatesAfterScaling();
+    bool checkCall() const;
 
     EffectWindow *m_scheduledScreenshot;
     ScreenShotType m_type;

service_utils.cpp

@@ -0,0 +1,32 @@
+/********************************************************************
+ KWin - the KDE window manager
+ This file is part of the KDE project.
+
+Copyright (C) 2020, Méven Car <meven.car@enika.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*********************************************************************/
+
+/*
+
+ This file is for (very) small utility relating to services/process.
+
+*/
+#include "service_utils.h"
+
+namespace KWin
+{
+
+}// namespace
+

service_utils.h

@@ -0,0 +1,77 @@
+/********************************************************************
+ KWin - the KDE window manager
+ This file is part of the KDE project.
+
+Copyright (C) 2020, Méven Car <meven.car@enika.com>
+
+This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*********************************************************************/
+
+#ifndef SERVICE_UTILS_H
+#define SERVICE_UTILS_H
+
+// cmake stuff
+#include <config-kwin.h>
+// kwin
+#include <kwinglobals.h>
+// Qt
+#include <QFileInfo>
+#include <QLoggingCategory>
+//KF
+#include <KApplicationTrader>
+
+namespace KWin
+{
+
+const static QString s_waylandInterfaceName = QStringLiteral("X-KDE-Wayland-Interfaces");
+const static QString s_dbusRestrictedInterfaceName = QStringLiteral("X-KDE-DBUS-Restricted-Interfaces");
+
+static QStringList fetchProcessServiceField(const QString &executablePath, const QString &fieldName)
+{
+    // needed to be able to use the logging category in a header static function
+    static QLoggingCategory KWIN_UTILS ("KWIN_UTILS");
+    const auto servicesFound = KApplicationTrader::query([&executablePath] (const KService::Ptr &service) {
+
+        if (service->exec().isEmpty() || service->exec() != executablePath)
+            return false;
+
+        return true;
+    });
+
+    if (servicesFound.isEmpty()) {
+        qCDebug(KWIN_UTILS) << "Could not find the desktop file for" << executablePath;
+        return {};
+    }
+
+    const auto fieldValues = servicesFound.first()->property(fieldName).toStringList();
+    if (KWIN_UTILS().isDebugEnabled()) {
+        qCDebug(KWIN_UTILS) << "Interfaces found for" << executablePath << fieldName << ":" << fieldValues;
+    }
+    return fieldValues;
+}
+
+static QStringList fetchRequestedInterfaces(const QString &executablePath)
+{
+    return fetchProcessServiceField(executablePath, s_waylandInterfaceName);
+}
+
+static QStringList fetchRestrictedDBusInterfacesFromPid(const uint pid)
+{
+    const auto executablePath = QFileInfo(QStringLiteral("/proc/%1/exe").arg(pid)).symLinkTarget();
+    return fetchProcessServiceField(executablePath, s_dbusRestrictedInterfaceName);
+}
+
+}// namespace
+
+#endif // SERVICE_UTILS_H

wayland_server.cpp

@@ -26,6 +26,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "waylandxdgshellintegration.h"
 #include "workspace.h"
 #include "xdgshellclient.h"
+#include "service_utils.h"
 
 // Client
 #include <KWayland/Client/connection_thread.h>
@@ -68,9 +69,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include <KWaylandServer/filtered_display.h>
 #include <KWaylandServer/keyboard_shortcuts_inhibit_v1_interface.h>
 
-// KF
-#include <KServiceTypeTrader>
-
 // Qt
 #include <QCryptographicHash>
 #include <QDir>
@@ -243,17 +241,7 @@ public:
     }
 
     QStringList fetchRequestedInterfaces(KWaylandServer::ClientConnection *client) const {
-        const auto serviceQuery = QStringLiteral("exist Exec and exist [X-KDE-Wayland-Interfaces] and '%1' =~ Exec").arg(client->executablePath());
+        return KWin::fetchRequestedInterfaces(client->executablePath());
-        const auto servicesFound = KServiceTypeTrader::self()->query(QStringLiteral("Application"), serviceQuery);
-
-        if (servicesFound.isEmpty()) {
-            qCDebug(KWIN_CORE) << "Could not find the desktop file for" << client->executablePath();
-            return {};
-        }
-
-        const auto interfaces = servicesFound.first()->property("X-KDE-Wayland-Interfaces").toStringList();
-        qCDebug(KWIN_CORE) << "Interfaces for" << client->executablePath() << interfaces;
-        return interfaces;
     }
 
     const QSet<QByteArray> interfacesBlackList = {"org_kde_kwin_remote_access_manager", "org_kde_plasma_window_management", "org_kde_kwin_fake_input", "org_kde_kwin_keystate"};