From 3bee7c7f4b6bea0d5749889339c860265cba55fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Thu, 2 Sep 2021 17:15:36 +0200 Subject: [PATCH 1/9] 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: --- js/ui/panel.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/js/ui/panel.js b/js/ui/panel.js index 89b082ad05..dafba690a5 100644 --- a/js/ui/panel.js +++ b/js/ui/panel.js @@ -507,6 +507,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 = {}) { @@ -568,6 +582,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); @@ -579,6 +594,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); -- GitLab From 7298ee23e91b756c7009b4d7687dfd8673856f8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Thu, 17 Jun 2021 01:50:50 +0200 Subject: [PATCH 2/9] 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: --- 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 6574cc5288..2f21ba41d7 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; -- GitLab From 3b9e672a09dc6cdd997a579be9b310b60dcbb7bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Thu, 2 Sep 2021 16:23:38 +0200 Subject: [PATCH 3/9] 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: --- 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 85f491a693..3cb021cb1f 100644 --- a/js/misc/introspect.js +++ b/js/misc/introspect.js @@ -137,21 +137,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; } @@ -163,10 +165,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; } -- GitLab From 317f0f5fe063356e961006fd75a9c8d9a38f47ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Wed, 1 Sep 2021 21:18:42 +0200 Subject: [PATCH 4/9] 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: --- 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 3cb021cb1f..8e469fbea3 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', 'org.freedesktop.impl.portal.desktop.gnome', @@ -36,10 +34,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', () => { @@ -73,10 +67,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); } @@ -138,7 +128,7 @@ var IntrospectService = class { } _checkInvocation(invocation) { - if (this._isIntrospectEnabled()) + if (global.context.unsafe_mode) return; if (this._isSenderAllowed(invocation.get_sender())) -- GitLab From d47478132593e4d661cbb5eec26554b424efbe93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Wed, 1 Sep 2021 21:25:26 +0200 Subject: [PATCH 5/9] data: Remove now unused "introspect" setting https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3943 Part-of: --- 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 cd6a2356dd..a805d5262c 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. - - false - Enable introspection API - - Enables a D-Bus API that allows to introspect the application state of - the shell. - - Date: Wed, 16 Jun 2021 19:09:42 +0200 Subject: [PATCH 6/9] 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: --- js/misc/introspect.js | 30 +++-------------------- js/misc/util.js | 57 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 27 deletions(-) diff --git a/js/misc/introspect.js b/js/misc/introspect.js index 8e469fbea3..22bd8319c4 100644 --- a/js/misc/introspect.js +++ b/js/misc/introspect.js @@ -9,6 +9,7 @@ const APP_ALLOWLIST = [ const INTROSPECT_DBUS_API_VERSION = 3; const { loadInterfaceXML } = imports.misc.fileUtils; +const { DBusSenderChecker } = imports.misc.util; const IntrospectDBusIface = loadInterfaceXML('org.gnome.Shell.Introspect'); @@ -43,14 +44,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', @@ -67,10 +61,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); @@ -127,21 +117,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; @@ -156,7 +134,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 8139d3f47c..bd57184728 100644 --- a/js/misc/util.js +++ b/js/misc/util.js @@ -1,7 +1,8 @@ // -*- mode: js; js-indent-level: 4; indent-tabs-mode: nil -*- /* exported findUrls, spawn, spawnCommandLine, spawnApp, trySpawnCommandLine, formatTime, formatTimeSpan, createTimeLabel, insertSorted, - ensureActorVisibleInScrollView, wiggle, lerp, GNOMEversionCompare */ + ensureActorVisibleInScrollView, wiggle, lerp, GNOMEversionCompare, + DBusSenderChecker */ const { Clutter, Gio, GLib, Shell, St, GnomeDesktop } = imports.gi; const Gettext = imports.gettext; @@ -477,3 +478,57 @@ 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 = []; + } +}; -- GitLab From 3adad0da810c39e9c7cd175c689aafca973e1705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Thu, 17 Jun 2021 15:29:42 +0200 Subject: [PATCH 7/9] 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: --- js/ui/shellDBus.js | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/js/ui/shellDBus.js b/js/ui/shellDBus.js index 2f21ba41d7..62c840094b 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,14 +112,31 @@ var GnomeShell = class { icon = Gio.Icon.new_for_string(serializedIcon); Main.osdWindowManager.show(monitorIndex, icon, label, level, maxLevel); + invocation.return_value(null); } - FocusApp(id) { + /** + * Focus specified app in the overview's app grid + * + * @param {string} id - an application ID + * @param {Gio.DBusMethodInvocation} invocation - the invocation + * @returns {void} + */ + FocusAppAsync([id], invocation) { 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) { -- GitLab From a628bbc485e11346d633879b3f734c55abcdf414 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Thu, 17 Jun 2021 15:29:42 +0200 Subject: [PATCH 8/9] 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: --- js/ui/shellDBus.js | 76 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/js/ui/shellDBus.js b/js/ui/shellDBus.js index 62c840094b..a8070eb925 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(); @@ -123,6 +143,13 @@ var GnomeShell = class { * @returns {void} */ FocusAppAsync([id], invocation) { + try { + this._senderChecker.checkInvocation(invocation); + } catch (e) { + invocation.return_gerror(e); + return; + } + Main.overview.selectApp(id); invocation.return_value(null); } @@ -135,11 +162,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); @@ -147,6 +188,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 = []; @@ -158,6 +206,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); @@ -166,6 +221,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; @@ -246,6 +308,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); @@ -253,6 +322,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); -- GitLab From dd2cd6286cd3175e1518038a173218671adc68ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20M=C3=BCllner?= Date: Wed, 16 Jun 2021 22:11:50 +0200 Subject: [PATCH 9/9] 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: --- js/ui/screenshot.js | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js index 81ab516b17..bf537b7d6d 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)) { -- GitLab