rpms/gnome-shell
7
0
Fork 0
Code Issues Pull Requests Releases Activity
8fcd3f19f6
gnome-shell/restrict-dbus-callers.patch

1359 lines
46 KiB
Diff
Raw Normal View History

Restrict D-Bus callers Resolves: #2055366
2022-04-01 19:18:00 +00:00
From 71a4197471cdd482e4f7f7b2379598bb3d55dc2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Fri, 1 Apr 2022 19:40:31 +0200
Subject: [PATCH 01/12] shell/global: Expose shim context property
Parts of the following commits rely on the ShellGlobal:context
property that was added in GNOME 41 to expose the MetaContext
(likewise a GNOME 41 addition).
To prepare for that, expose a small shim object as context
property that mimicks the expected upstream API.
---
src/shell-global.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++
src/shell-global.h | 3 ++
2 files changed, 92 insertions(+)
diff --git a/src/shell-global.c b/src/shell-global.c
index 24e771f52..3f765602d 100644
--- a/src/shell-global.c
+++ b/src/shell-global.c
@@ -54,6 +54,7 @@ struct _ShellGlobal {
MetaBackend *backend;
MetaDisplay *meta_display;
+ ShellShimMetaContext *meta_context;
MetaWorkspaceManager *workspace_manager;
Display *xdisplay;
@@ -92,6 +93,7 @@ enum {
PROP_SESSION_MODE,
PROP_BACKEND,
+ PROP_CONTEXT,
PROP_DISPLAY,
PROP_WORKSPACE_MANAGER,
PROP_SCREEN_WIDTH,
@@ -235,6 +237,9 @@ shell_global_get_property(GObject *object,
case PROP_BACKEND:
g_value_set_object (value, global->backend);
break;
+ case PROP_CONTEXT:
+ g_value_set_object (value, global->meta_context);
+ break;
case PROP_DISPLAY:
g_value_set_object (value, global->meta_display);
break;
@@ -514,6 +519,13 @@ shell_global_class_init (ShellGlobalClass *klass)
"MetaBackend object",
META_TYPE_BACKEND,
G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
+ g_object_class_install_property (gobject_class,
+ PROP_CONTEXT,
+ g_param_spec_object ("context",
+ "Context",
+ "MetaContext object",
+ SHELL_TYPE_SHIM_META_CONTEXT,
+ G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
g_object_class_install_property (gobject_class,
PROP_DISPLAY,
g_param_spec_object ("display",
@@ -996,6 +1008,7 @@ _shell_global_set_plugin (ShellGlobal *global,
display = meta_plugin_get_display (plugin);
global->meta_display = display;
+ global->meta_context = g_object_new (SHELL_TYPE_SHIM_META_CONTEXT, NULL);
global->workspace_manager = meta_display_get_workspace_manager (display);
global->stage = CLUTTER_STAGE (meta_get_stage_for_display (display));
@@ -1888,3 +1901,79 @@ _shell_global_locate_pointer (ShellGlobal *global)
{
g_signal_emit (global, shell_global_signals[LOCATE_POINTER], 0);
}
+
+enum {
+ SHIM_PROP_0,
+
+ SHIM_PROP_UNSAFE_MODE,
+
+ N_SHIM_PROPS
+};
+
+static GParamSpec *shim_obj_props [N_SHIM_PROPS];
+
+struct _ShellShimMetaContext
+{
+ GObject parent_instance;
+};
+
+G_DEFINE_TYPE (ShellShimMetaContext, shell_shim_meta_context, G_TYPE_OBJECT);
+
+static void
+shell_shim_meta_context_get_property (GObject *object,
+ guint prop_id,
+ GValue *value,
+ GParamSpec *pspec)
+{
+ switch (prop_id)
+ {
+ case SHIM_PROP_UNSAFE_MODE:
+ {
+ gboolean unsafe_mode;
+
+ g_object_get (meta_get_backend (), "unsafe-mode", &unsafe_mode, NULL);
+ g_value_set_boolean (value, unsafe_mode);
+ }
+ break;
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+ }
+}
+
+static void
+shell_shim_meta_context_set_property (GObject *object,
+ guint prop_id,
+ const GValue *value,
+ GParamSpec *pspec)
+{
+ switch (prop_id)
+ {
+ case SHIM_PROP_UNSAFE_MODE:
+ g_object_set_property (G_OBJECT (meta_get_backend ()), "unsafe-mode", value);
+ break;
+ default:
+ G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+ }
+}
+
+static void
+shell_shim_meta_context_class_init (ShellShimMetaContextClass *klass)
+{
+ GObjectClass *object_class = G_OBJECT_CLASS (klass);
+
+ object_class->get_property = shell_shim_meta_context_get_property;
+ object_class->set_property = shell_shim_meta_context_set_property;
+
+ shim_obj_props[SHIM_PROP_UNSAFE_MODE] =
+ g_param_spec_boolean ("unsafe-mode",
+ "unsafe mode",
+ "Unsafe mode",
+ FALSE,
+ G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
+ g_object_class_install_properties (object_class, N_SHIM_PROPS, shim_obj_props);
+}
+
+static void
+shell_shim_meta_context_init (ShellShimMetaContext *self)
+{
+}
diff --git a/src/shell-global.h b/src/shell-global.h
index 60bda7131..35e72b4da 100644
--- a/src/shell-global.h
+++ b/src/shell-global.h
@@ -13,6 +13,9 @@ G_BEGIN_DECLS
#define SHELL_TYPE_GLOBAL (shell_global_get_type ())
G_DECLARE_FINAL_TYPE (ShellGlobal, shell_global, SHELL, GLOBAL, GObject)
+#define SHELL_TYPE_SHIM_META_CONTEXT shell_shim_meta_context_get_type ()
+G_DECLARE_FINAL_TYPE (ShellShimMetaContext, shell_shim_meta_context, SHELL, SHIM_META_CONTEXT, GObject)
+
ShellGlobal *shell_global_get (void);
ClutterStage *shell_global_get_stage (ShellGlobal *global);
--
2.35.1
From 784a0fc843837f584a2e056c77d9492b105e7e61 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Thu, 2 Sep 2021 17:15:36 +0200
Subject: [PATCH 02/12] panel: Show warning indicator when unsafe-mode is on
MetaContext added an unsafe-mode property, which we will use to restrict
a number of privileged operations unless it is enabled. It is meant to
only be enabled temporarily for development/debugging purposes, so add
a scary icon to the top bar as a reminder to turn it off again.
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/ui/panel.js | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/js/ui/panel.js b/js/ui/panel.js
index 380480744..c57c3ba8e 100644
--- a/js/ui/panel.js
+++ b/js/ui/panel.js
@@ -641,6 +641,20 @@ class PanelCorner extends St.DrawingArea {
}
});
+const UnsafeModeIndicator = GObject.registerClass(
+class UnsafeModeIndicator extends PanelMenu.SystemIndicator {
+ _init() {
+ super._init();
+
+ this._indicator = this._addIndicator();
+ this._indicator.icon_name = 'channel-insecure-symbolic';
+
+ global.context.bind_property('unsafe-mode',
+ this._indicator, 'visible',
+ GObject.BindingFlags.SYNC_CREATE);
+ }
+});
+
var AggregateLayout = GObject.registerClass(
class AggregateLayout extends Clutter.BoxLayout {
_init(params = {}) {
@@ -702,6 +716,7 @@ class AggregateMenu extends PanelMenu.Button {
this._location = new imports.ui.status.location.Indicator();
this._nightLight = new imports.ui.status.nightLight.Indicator();
this._thunderbolt = new imports.ui.status.thunderbolt.Indicator();
+ this._unsafeMode = new UnsafeModeIndicator();
this._indicators.add_child(this._remoteAccess);
this._indicators.add_child(this._thunderbolt);
@@ -713,6 +728,7 @@ class AggregateMenu extends PanelMenu.Button {
this._indicators.add_child(this._bluetooth);
this._indicators.add_child(this._rfkill);
this._indicators.add_child(this._volume);
+ this._indicators.add_child(this._unsafeMode);
this._indicators.add_child(this._power);
this._indicators.add_child(this._powerProfiles);
--
2.35.1
From 2d31984c43ed017322c78d1f4b62d2eb7e9317c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Thu, 17 Jun 2021 01:50:50 +0200
Subject: [PATCH 03/12] shellDBus: Use MetaContext:unsafe-mode to restrict
Eval()
The Eval() method is unarguably the most sensitive D-Bus method
we expose, since it allows running arbitrary code in the compositor.
It is currently tied to the `development-tools` settings that is
enabled by default. As users have become accustomed to the built-in
commands that are enabled by the same setting (restart, lg, ...),
that default cannot easily be changed.
In order to restrict the method without affecting the rather harmless
commands, guard it by the new MetaContext:unsafe-mode property instead
of the setting.
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/ui/shellDBus.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/js/ui/shellDBus.js b/js/ui/shellDBus.js
index 734ca4fc7..5a6edec74 100644
--- a/js/ui/shellDBus.js
+++ b/js/ui/shellDBus.js
@@ -54,7 +54,7 @@ var GnomeShell = class {
*
*/
Eval(code) {
- if (!global.settings.get_boolean('development-tools'))
+ if (!global.context.unsafe_mode)
return [false, ''];
let returnValue;
--
2.35.1
From 32000b5cccd25bdce59c67747c0d31acbbfb9d62 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Thu, 2 Sep 2021 16:23:38 +0200
Subject: [PATCH 04/12] introspect: Make invocation check error-based
If we throw an error when the invocation isn't allowed instead of
returning false, we can simply return that error instead of duplicating
the error handling.
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/misc/introspect.js | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/js/misc/introspect.js b/js/misc/introspect.js
index e46a7e8c5..318955be2 100644
--- a/js/misc/introspect.js
+++ b/js/misc/introspect.js
@@ -134,21 +134,23 @@ var IntrospectService = class {
type == Meta.WindowType.UTILITY;
}
- _isInvocationAllowed(invocation) {
+ _checkInvocation(invocation) {
if (this._isIntrospectEnabled())
- return true;
+ return;
if (this._isSenderAllowed(invocation.get_sender()))
- return true;
+ return;
- return false;
+ throw new GLib.Error(Gio.DBusError,
+ Gio.DBusError.ACCESS_DENIED,
+ 'App introspection not allowed');
}
GetRunningApplicationsAsync(params, invocation) {
- if (!this._isInvocationAllowed(invocation)) {
- invocation.return_error_literal(Gio.DBusError,
- Gio.DBusError.ACCESS_DENIED,
- 'App introspection not allowed');
+ try {
+ this._checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
return;
}
@@ -160,10 +162,10 @@ var IntrospectService = class {
let apps = this._appSystem.get_running();
let windowsList = {};
- if (!this._isInvocationAllowed(invocation)) {
- invocation.return_error_literal(Gio.DBusError,
- Gio.DBusError.ACCESS_DENIED,
- 'App introspection not allowed');
+ try {
+ this._checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
return;
}
--
2.35.1
From e6c14cbb6863030d17873087a9ab480918e561e4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Wed, 1 Sep 2021 21:18:42 +0200
Subject: [PATCH 05/12] introspect: Use MetaContext:unsafe-mode instead of
setting
The property was added precisely for this purpose, except that its
name isn't tied to the introspect API.
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/misc/introspect.js | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/js/misc/introspect.js b/js/misc/introspect.js
index 318955be2..967e7b830 100644
--- a/js/misc/introspect.js
+++ b/js/misc/introspect.js
@@ -1,8 +1,6 @@
/* exported IntrospectService */
const { Gio, GLib, Meta, Shell, St } = imports.gi;
-const INTROSPECT_SCHEMA = 'org.gnome.shell';
-const INTROSPECT_KEY = 'introspect';
const APP_ALLOWLIST = ['org.freedesktop.impl.portal.desktop.gtk'];
const INTROSPECT_DBUS_API_VERSION = 3;
@@ -33,10 +31,6 @@ var IntrospectService = class {
this._syncRunningApplications();
});
- this._introspectSettings = new Gio.Settings({
- schema_id: INTROSPECT_SCHEMA,
- });
-
let tracker = Shell.WindowTracker.get_default();
tracker.connect('notify::focus-app',
() => {
@@ -70,10 +64,6 @@ var IntrospectService = class {
return app.get_windows().some(w => w.transient_for == null);
}
- _isIntrospectEnabled() {
- return this._introspectSettings.get_boolean(INTROSPECT_KEY);
- }
-
_isSenderAllowed(sender) {
return [...this._allowlistMap.values()].includes(sender);
}
@@ -135,7 +125,7 @@ var IntrospectService = class {
}
_checkInvocation(invocation) {
- if (this._isIntrospectEnabled())
+ if (global.context.unsafe_mode)
return;
if (this._isSenderAllowed(invocation.get_sender()))
--
2.35.1
From 2c19455575a26053c2982c2976c4a546c445a4b1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Wed, 1 Sep 2021 21:25:26 +0200
Subject: [PATCH 06/12] data: Remove now unused "introspect" setting
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
data/org.gnome.shell.gschema.xml.in | 8 --------
1 file changed, 8 deletions(-)
diff --git a/data/org.gnome.shell.gschema.xml.in b/data/org.gnome.shell.gschema.xml.in
index d5ea1e35f..6f1c424ba 100644
--- a/data/org.gnome.shell.gschema.xml.in
+++ b/data/org.gnome.shell.gschema.xml.in
@@ -104,14 +104,6 @@
number can be used to effectively disable the dialog.
</description>
</key>
- <key name="introspect" type="b">
- <default>false</default>
- <summary>Enable introspection API</summary>
- <description>
- Enables a D-Bus API that allows to introspect the application state of
- the shell.
- </description>
- </key>
<key name="app-picker-layout" type="aa{sv}">
<default><![CDATA[
[{
--
2.35.1
From 15a2854c82766c0e00c218eb7bd47865d52e6ae0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Wed, 16 Jun 2021 19:09:42 +0200
Subject: [PATCH 07/12] introspect: Split out DBusSenderChecker
Restricting callers to a list of allowed senders is useful for
other D-Bus services as well, so split out the existing code
into a reusable class.
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/misc/introspect.js | 30 ++++-------------------
js/misc/util.js | 56 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 59 insertions(+), 27 deletions(-)
diff --git a/js/misc/introspect.js b/js/misc/introspect.js
index 967e7b830..e9d9260c0 100644
--- a/js/misc/introspect.js
+++ b/js/misc/introspect.js
@@ -6,6 +6,7 @@ const APP_ALLOWLIST = ['org.freedesktop.impl.portal.desktop.gtk'];
const INTROSPECT_DBUS_API_VERSION = 3;
const { loadInterfaceXML } = imports.misc.fileUtils;
+const { DBusSenderChecker } = imports.misc.util;
const IntrospectDBusIface = loadInterfaceXML('org.gnome.Shell.Introspect');
@@ -40,14 +41,7 @@ var IntrospectService = class {
this._syncRunningApplications();
- this._allowlistMap = new Map();
- APP_ALLOWLIST.forEach(appName => {
- Gio.DBus.watch_name(Gio.BusType.SESSION,
- appName,
- Gio.BusNameWatcherFlags.NONE,
- (conn, name, owner) => this._allowlistMap.set(name, owner),
- (conn, name) => this._allowlistMap.delete(name));
- });
+ this._senderChecker = new DBusSenderChecker(APP_ALLOWLIST);
this._settings = St.Settings.get();
this._settings.connect('notify::enable-animations',
@@ -64,10 +58,6 @@ var IntrospectService = class {
return app.get_windows().some(w => w.transient_for == null);
}
- _isSenderAllowed(sender) {
- return [...this._allowlistMap.values()].includes(sender);
- }
-
_getSandboxedAppId(app) {
let ids = app.get_windows().map(w => w.get_sandboxed_app_id());
return ids.find(id => id != null);
@@ -124,21 +114,9 @@ var IntrospectService = class {
type == Meta.WindowType.UTILITY;
}
- _checkInvocation(invocation) {
- if (global.context.unsafe_mode)
- return;
-
- if (this._isSenderAllowed(invocation.get_sender()))
- return;
-
- throw new GLib.Error(Gio.DBusError,
- Gio.DBusError.ACCESS_DENIED,
- 'App introspection not allowed');
- }
-
GetRunningApplicationsAsync(params, invocation) {
try {
- this._checkInvocation(invocation);
+ this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -153,7 +131,7 @@ var IntrospectService = class {
let windowsList = {};
try {
- this._checkInvocation(invocation);
+ this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
diff --git a/js/misc/util.js b/js/misc/util.js
index 802398d18..e6c183fbf 100644
--- a/js/misc/util.js
+++ b/js/misc/util.js
@@ -2,7 +2,7 @@
/* exported findUrls, spawn, spawnCommandLine, spawnApp, trySpawnCommandLine,
formatTime, formatTimeSpan, createTimeLabel, insertSorted,
ensureActorVisibleInScrollView, wiggle, lerp, GNOMEversionCompare,
- Highlighter */
+ DBusSenderChecker, Highlighter */
const { Clutter, Gio, GLib, Shell, St, GnomeDesktop } = imports.gi;
const Gettext = imports.gettext;
@@ -479,6 +479,60 @@ function GNOMEversionCompare(version1, version2) {
return 0;
}
+var DBusSenderChecker = class {
+ /**
+ * @param {string[]} allowList - list of allowed well-known names
+ */
+ constructor(allowList) {
+ this._allowlistMap = new Map();
+
+ this._watchList = allowList.map(name => {
+ return Gio.DBus.watch_name(Gio.BusType.SESSION,
+ name,
+ Gio.BusNameWatcherFlags.NONE,
+ (conn_, name_, owner) => this._allowlistMap.set(name, owner),
+ () => this._allowlistMap.delete(name));
+ });
+ }
+
+ /**
+ * @param {string} sender - the bus name that invoked the checked method
+ * @returns {bool}
+ */
+ _isSenderAllowed(sender) {
+ return [...this._allowlistMap.values()].includes(sender);
+ }
+
+ /**
+ * Check whether the bus name that invoked @invocation maps
+ * to an entry in the allow list.
+ *
+ * @throws
+ * @param {Gio.DBusMethodInvocation} invocation - the invocation
+ * @returns {void}
+ */
+ checkInvocation(invocation) {
+ if (global.context.unsafe_mode)
+ return;
+
+ if (this._isSenderAllowed(invocation.get_sender()))
+ return;
+
+ throw new GLib.Error(Gio.DBusError,
+ Gio.DBusError.ACCESS_DENIED,
+ '%s is not allowed'.format(invocation.get_method_name()));
+ }
+
+ /**
+ * @returns {void}
+ */
+ destroy() {
+ for (const id in this._watchList)
+ Gio.DBus.unwatch_name(id);
+ this._watchList = [];
+ }
+};
+
/* @class Highlighter Highlight given terms in text using markup. */
var Highlighter = class {
/**
--
2.35.1
From 01bc238d02785cef23e7481e8cd7c3ec3468eda9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Thu, 17 Jun 2021 15:29:42 +0200
Subject: [PATCH 08/12] shellDBus: Implement all methods asynchronously
In order to restrict callers, we will need access to the invocation,
not just the unpacked method parameters.
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/ui/shellDBus.js | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/js/ui/shellDBus.js b/js/ui/shellDBus.js
index 5a6edec74..aa5b4dc3c 100644
--- a/js/ui/shellDBus.js
+++ b/js/ui/shellDBus.js
@@ -72,11 +72,26 @@ var GnomeShell = class {
return [success, returnValue];
}
- FocusSearch() {
+ /**
+ * Focus the overview's search entry
+ *
+ * @param {...any} params - method parameters
+ * @param {Gio.DBusMethodInvocation} invocation - the invocation
+ * @returns {void}
+ */
+ FocusSearchAsync(params, invocation) {
Main.overview.focusSearch();
+ invocation.return_value(null);
}
- ShowOSD(params) {
+ /**
+ * Show OSD with the specified parameters
+ *
+ * @param {...any} params - method parameters
+ * @param {Gio.DBusMethodInvocation} invocation - the invocation
+ * @returns {void}
+ */
+ ShowOSDAsync([params], invocation) {
for (let param in params)
params[param] = params[param].deep_unpack();
@@ -97,6 +112,7 @@ var GnomeShell = class {
icon = Gio.Icon.new_for_string(serializedIcon);
Main.osdWindowManager.show(monitorIndex, icon, label, level, maxLevel);
+ invocation.return_value(null);
}
/**
@@ -118,10 +134,19 @@ var GnomeShell = class {
}
Main.overview.selectApp(id);
+ invocation.return_value(null);
}
- ShowApplications() {
+ /**
+ * Show the overview's app grid
+ *
+ * @param {...any} params - method parameters
+ * @param {Gio.DBusMethodInvocation} invocation - the invocation
+ * @returns {void}
+ */
+ ShowApplicationsAsync(params, invocation) {
Main.overview.show(ControlsState.APP_GRID);
+ invocation.return_value(null);
}
GrabAcceleratorAsync(params, invocation) {
--
2.35.1
From d830d87e781de15e6170af85c78086487cf53398 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Thu, 17 Jun 2021 15:29:42 +0200
Subject: [PATCH 09/12] shellDBus: Restrict callers
The org.gnome.Shell interface provides a private API to other core
components to implement desktop functionalities like Settings or
global keybindings. It is not meant as a public API, so limit it
to a set of expected callers.
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/ui/shellDBus.js | 76 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)
diff --git a/js/ui/shellDBus.js b/js/ui/shellDBus.js
index aa5b4dc3c..c511314f9 100644
--- a/js/ui/shellDBus.js
+++ b/js/ui/shellDBus.js
@@ -10,6 +10,7 @@ const Main = imports.ui.main;
const Screenshot = imports.ui.screenshot;
const { loadInterfaceXML } = imports.misc.fileUtils;
+const { DBusSenderChecker } = imports.misc.util;
const { ControlsState } = imports.ui.overviewControls;
const GnomeShellIface = loadInterfaceXML('org.gnome.Shell');
@@ -20,6 +21,11 @@ var GnomeShell = class {
this._dbusImpl = Gio.DBusExportedObject.wrapJSObject(GnomeShellIface, this);
this._dbusImpl.export(Gio.DBus.session, '/org/gnome/Shell');
+ this._senderChecker = new DBusSenderChecker([
+ 'org.gnome.ControlCenter',
+ 'org.gnome.SettingsDaemon.MediaKeys',
+ ]);
+
this._extensionsService = new GnomeShellExtensions();
this._screenshotService = new Screenshot.ScreenshotService();
@@ -80,6 +86,13 @@ var GnomeShell = class {
* @returns {void}
*/
FocusSearchAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
Main.overview.focusSearch();
invocation.return_value(null);
}
@@ -92,6 +105,13 @@ var GnomeShell = class {
* @returns {void}
*/
ShowOSDAsync([params], invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
for (let param in params)
params[param] = params[param].deep_unpack();
@@ -124,6 +144,13 @@ var GnomeShell = class {
* @returns {void}
*/
FocusAppAsync([id], invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
const appSys = Shell.AppSystem.get_default();
if (appSys.lookup_app(id) === null) {
invocation.return_error_literal(
@@ -145,11 +172,25 @@ var GnomeShell = class {
* @returns {void}
*/
ShowApplicationsAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
Main.overview.show(ControlsState.APP_GRID);
invocation.return_value(null);
}
GrabAcceleratorAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let [accel, modeFlags, grabFlags] = params;
let sender = invocation.get_sender();
let bindingAction = this._grabAcceleratorForSender(accel, modeFlags, grabFlags, sender);
@@ -157,6 +198,13 @@ var GnomeShell = class {
}
GrabAcceleratorsAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let [accels] = params;
let sender = invocation.get_sender();
let bindingActions = [];
@@ -168,6 +216,13 @@ var GnomeShell = class {
}
UngrabAcceleratorAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let [action] = params;
let sender = invocation.get_sender();
let ungrabSucceeded = this._ungrabAcceleratorForSender(action, sender);
@@ -176,6 +231,13 @@ var GnomeShell = class {
}
UngrabAcceleratorsAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let [actions] = params;
let sender = invocation.get_sender();
let ungrabSucceeded = true;
@@ -256,6 +318,13 @@ var GnomeShell = class {
}
ShowMonitorLabelsAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let sender = invocation.get_sender();
let [dict] = params;
Main.osdMonitorLabeler.show(sender, dict);
@@ -263,6 +332,13 @@ var GnomeShell = class {
}
HideMonitorLabelsAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let sender = invocation.get_sender();
Main.osdMonitorLabeler.hide(sender);
invocation.return_value(null);
--
2.35.1
From fbe05ef154492d9da9b3cc8e1cacfdd617f4223b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Wed, 16 Jun 2021 22:11:50 +0200
Subject: [PATCH 10/12] screenshot: Restrict callers
The shell D-Bus API was always meant as a private API for core
components, so enforce that by limiting caller to a list of
allowed well-known names.
Applications that want to request a screenshot can use the corresponding
desktop portal.
https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1970>
---
js/ui/screenshot.js | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js
index 81ab516b1..bf537b7d6 100644
--- a/js/ui/screenshot.js
+++ b/js/ui/screenshot.js
@@ -15,6 +15,7 @@ Gio._promisify(Shell.Screenshot.prototype,
'screenshot_area', 'screenshot_area_finish');
const { loadInterfaceXML } = imports.misc.fileUtils;
+const { DBusSenderChecker } = imports.misc.util;
const ScreenshotIface = loadInterfaceXML('org.gnome.Shell.Screenshot');
@@ -24,6 +25,12 @@ var ScreenshotService = class {
this._dbusImpl.export(Gio.DBus.session, '/org/gnome/Shell/Screenshot');
this._screenShooter = new Map();
+ this._senderChecker = new DBusSenderChecker([
+ 'org.gnome.SettingsDaemon.MediaKeys',
+ 'org.freedesktop.impl.portal.desktop.gtk',
+ 'org.freedesktop.impl.portal.desktop.gnome',
+ 'org.gnome.Screenshot',
+ ]);
this._lockdownSettings = new Gio.Settings({ schema_id: 'org.gnome.desktop.lockdown' });
@@ -46,6 +53,13 @@ var ScreenshotService = class {
Gio.IOErrorEnum, Gio.IOErrorEnum.PERMISSION_DENIED,
'Saving to disk is disabled');
return null;
+ } else {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return null;
+ }
}
let shooter = new Shell.Screenshot();
@@ -254,6 +268,13 @@ var ScreenshotService = class {
}
async SelectAreaAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let selectArea = new SelectArea();
try {
let areaRectangle = await selectArea.selectAsync();
@@ -269,6 +290,13 @@ var ScreenshotService = class {
}
FlashAreaAsync(params, invocation) {
+ try {
+ this._senderChecker.checkInvocation(invocation);
+ } catch (e) {
+ invocation.return_gerror(e);
+ return;
+ }
+
let [x, y, width, height] = params;
[x, y, width, height] = this._scaleArea(x, y, width, height);
if (!this._checkArea(x, y, width, height)) {
--
2.35.1
From f637187b816435dc4b347144a03c0d72e880780c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Sat, 25 Sep 2021 14:15:32 +0200
Subject: [PATCH 11/12] screenshot: Unrestrict PickColor
Commit dd2cd6286cd3 restricted callers of the screenshot methods to
portal implementations, gnome-settings-daemon and gnome-screenshot.
That restriction does make sense for the actual screenshot methods,
but `PickColor` is actually used by GTK in its color picker (and
therefore may be called from arbitrary applications).
Fix this by unrestricting access to `PickColor` again. Considering that
the method is always interactive, it's not very privacy/security-sensitive
anyway.
https://gitlab.gnome.org/GNOME/gtk/-/issues/4283
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1990>
---
js/ui/screenshot.js | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js
index bf537b7d6..ae1156f47 100644
--- a/js/ui/screenshot.js
+++ b/js/ui/screenshot.js
@@ -37,7 +37,7 @@ var ScreenshotService = class {
Gio.DBus.session.own_name('org.gnome.Shell.Screenshot', Gio.BusNameOwnerFlags.REPLACE, null, null);
}
- _createScreenshot(invocation, needsDisk = true) {
+ _createScreenshot(invocation, needsDisk = true, restrictCallers = true) {
let lockedDown = false;
if (needsDisk)
lockedDown = this._lockdownSettings.get_boolean('disable-save-to-disk');
@@ -53,7 +53,7 @@ var ScreenshotService = class {
Gio.IOErrorEnum, Gio.IOErrorEnum.PERMISSION_DENIED,
'Saving to disk is disabled');
return null;
- } else {
+ } else if (restrictCallers) {
try {
this._senderChecker.checkInvocation(invocation);
} catch (e) {
@@ -311,7 +311,7 @@ var ScreenshotService = class {
}
async PickColorAsync(params, invocation) {
- const screenshot = this._createScreenshot(invocation, false);
+ const screenshot = this._createScreenshot(invocation, false, false);
if (!screenshot)
return;
--
2.35.1
From f5d1dbfa38b9a3e7b6c9c5eec3b48d24e18e5732 Mon Sep 17 00:00:00 2001
From: Sebastian Keller <skeller@gnome.org>
Date: Tue, 23 Nov 2021 02:48:04 +0100
Subject: [PATCH 12/12] util: Wait for initial name owners in DBusSenderCheck
before checking
Otherwise an allowed caller might get rejected if the call is right
after a gnome-shell restart and the watchers have not finished running
their callbacks yet.
Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4813
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2048>
(cherry picked from commit 85609a232d4088b058f23f4922b9a993dea95199)
---
js/misc/introspect.js | 8 ++++----
js/misc/util.js | 33 ++++++++++++++++++++++++++++-----
js/ui/screenshot.js | 18 +++++++++---------
js/ui/shellDBus.js | 43 +++++++++++++++++++++++--------------------
4 files changed, 64 insertions(+), 38 deletions(-)
diff --git a/js/misc/introspect.js b/js/misc/introspect.js
index e9d9260c0..f3c938af9 100644
--- a/js/misc/introspect.js
+++ b/js/misc/introspect.js
@@ -114,9 +114,9 @@ var IntrospectService = class {
type == Meta.WindowType.UTILITY;
}
- GetRunningApplicationsAsync(params, invocation) {
+ async GetRunningApplicationsAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -125,13 +125,13 @@ var IntrospectService = class {
invocation.return_value(new GLib.Variant('(a{sa{sv}})', [this._runningApplications]));
}
- GetWindowsAsync(params, invocation) {
+ async GetWindowsAsync(params, invocation) {
let focusWindow = global.display.get_focus_window();
let apps = this._appSystem.get_running();
let windowsList = {};
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
diff --git a/js/misc/util.js b/js/misc/util.js
index e6c183fbf..6a0f6f641 100644
--- a/js/misc/util.js
+++ b/js/misc/util.js
@@ -486,20 +486,42 @@ var DBusSenderChecker = class {
constructor(allowList) {
this._allowlistMap = new Map();
+ this._uninitializedNames = new Set(allowList);
+ this._initializedPromise = new Promise(resolve => {
+ this._resolveInitialized = resolve;
+ });
+
this._watchList = allowList.map(name => {
return Gio.DBus.watch_name(Gio.BusType.SESSION,
name,
Gio.BusNameWatcherFlags.NONE,
- (conn_, name_, owner) => this._allowlistMap.set(name, owner),
- () => this._allowlistMap.delete(name));
+ (conn_, name_, owner) => {
+ this._allowlistMap.set(name, owner);
+ this._checkAndResolveInitialized(name);
+ },
+ () => {
+ this._allowlistMap.delete(name);
+ this._checkAndResolveInitialized(name);
+ });
});
}
/**
+ * @param {string} name - bus name for which the watcher got initialized
+ */
+ _checkAndResolveInitialized(name) {
+ if (this._uninitializedNames.delete(name) &&
+ this._uninitializedNames.size === 0)
+ this._resolveInitialized();
+ }
+
+ /**
+ * @async
* @param {string} sender - the bus name that invoked the checked method
* @returns {bool}
*/
- _isSenderAllowed(sender) {
+ async _isSenderAllowed(sender) {
+ await this._initializedPromise;
return [...this._allowlistMap.values()].includes(sender);
}
@@ -507,15 +529,16 @@ var DBusSenderChecker = class {
* Check whether the bus name that invoked @invocation maps
* to an entry in the allow list.
*
+ * @async
* @throws
* @param {Gio.DBusMethodInvocation} invocation - the invocation
* @returns {void}
*/
- checkInvocation(invocation) {
+ async checkInvocation(invocation) {
if (global.context.unsafe_mode)
return;
- if (this._isSenderAllowed(invocation.get_sender()))
+ if (await this._isSenderAllowed(invocation.get_sender()))
return;
throw new GLib.Error(Gio.DBusError,
diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js
index ae1156f47..97fcfacd0 100644
--- a/js/ui/screenshot.js
+++ b/js/ui/screenshot.js
@@ -37,7 +37,7 @@ var ScreenshotService = class {
Gio.DBus.session.own_name('org.gnome.Shell.Screenshot', Gio.BusNameOwnerFlags.REPLACE, null, null);
}
- _createScreenshot(invocation, needsDisk = true, restrictCallers = true) {
+ async _createScreenshot(invocation, needsDisk = true, restrictCallers = true) {
let lockedDown = false;
if (needsDisk)
lockedDown = this._lockdownSettings.get_boolean('disable-save-to-disk');
@@ -55,7 +55,7 @@ var ScreenshotService = class {
return null;
} else if (restrictCallers) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return null;
@@ -200,7 +200,7 @@ var ScreenshotService = class {
"Invalid params");
return;
}
- let screenshot = this._createScreenshot(invocation);
+ let screenshot = await this._createScreenshot(invocation);
if (!screenshot)
return;
@@ -223,7 +223,7 @@ var ScreenshotService = class {
async ScreenshotWindowAsync(params, invocation) {
let [includeFrame, includeCursor, flash, filename] = params;
- let screenshot = this._createScreenshot(invocation);
+ let screenshot = await this._createScreenshot(invocation);
if (!screenshot)
return;
@@ -246,7 +246,7 @@ var ScreenshotService = class {
async ScreenshotAsync(params, invocation) {
let [includeCursor, flash, filename] = params;
- let screenshot = this._createScreenshot(invocation);
+ let screenshot = await this._createScreenshot(invocation);
if (!screenshot)
return;
@@ -269,7 +269,7 @@ var ScreenshotService = class {
async SelectAreaAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -289,9 +289,9 @@ var ScreenshotService = class {
}
}
- FlashAreaAsync(params, invocation) {
+ async FlashAreaAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -311,7 +311,7 @@ var ScreenshotService = class {
}
async PickColorAsync(params, invocation) {
- const screenshot = this._createScreenshot(invocation, false, false);
+ const screenshot = await this._createScreenshot(invocation, false, false);
if (!screenshot)
return;
diff --git a/js/ui/shellDBus.js b/js/ui/shellDBus.js
index c511314f9..39bba7aa3 100644
--- a/js/ui/shellDBus.js
+++ b/js/ui/shellDBus.js
@@ -81,13 +81,14 @@ var GnomeShell = class {
/**
* Focus the overview's search entry
*
+ * @async
* @param {...any} params - method parameters
* @param {Gio.DBusMethodInvocation} invocation - the invocation
* @returns {void}
*/
- FocusSearchAsync(params, invocation) {
+ async FocusSearchAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -100,13 +101,14 @@ var GnomeShell = class {
/**
* Show OSD with the specified parameters
*
+ * @async
* @param {...any} params - method parameters
* @param {Gio.DBusMethodInvocation} invocation - the invocation
* @returns {void}
*/
- ShowOSDAsync([params], invocation) {
+ async ShowOSDAsync([params], invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -143,9 +145,9 @@ var GnomeShell = class {
* @param {Gio.DBusMethodInvocation} invocation - the invocation
* @returns {void}
*/
- FocusAppAsync([id], invocation) {
+ async FocusAppAsync([id], invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -167,13 +169,14 @@ var GnomeShell = class {
/**
* Show the overview's app grid
*
+ * @async
* @param {...any} params - method parameters
* @param {Gio.DBusMethodInvocation} invocation - the invocation
* @returns {void}
*/
- ShowApplicationsAsync(params, invocation) {
+ async ShowApplicationsAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -183,9 +186,9 @@ var GnomeShell = class {
invocation.return_value(null);
}
- GrabAcceleratorAsync(params, invocation) {
+ async GrabAcceleratorAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -197,9 +200,9 @@ var GnomeShell = class {
invocation.return_value(GLib.Variant.new('(u)', [bindingAction]));
}
- GrabAcceleratorsAsync(params, invocation) {
+ async GrabAcceleratorsAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -215,9 +218,9 @@ var GnomeShell = class {
invocation.return_value(GLib.Variant.new('(au)', [bindingActions]));
}
- UngrabAcceleratorAsync(params, invocation) {
+ async UngrabAcceleratorAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -230,9 +233,9 @@ var GnomeShell = class {
invocation.return_value(GLib.Variant.new('(b)', [ungrabSucceeded]));
}
- UngrabAcceleratorsAsync(params, invocation) {
+ async UngrabAcceleratorsAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -317,9 +320,9 @@ var GnomeShell = class {
this._grabbers.delete(name);
}
- ShowMonitorLabelsAsync(params, invocation) {
+ async ShowMonitorLabelsAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
@@ -331,9 +334,9 @@ var GnomeShell = class {
invocation.return_value(null);
}
- HideMonitorLabelsAsync(params, invocation) {
+ async HideMonitorLabelsAsync(params, invocation) {
try {
- this._senderChecker.checkInvocation(invocation);
+ await this._senderChecker.checkInvocation(invocation);
} catch (e) {
invocation.return_gerror(e);
return;
--
2.35.1
Reference in New Issue Copy Permalink