Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webextensions: add a portal for managing WebExtensions native messaging servers #705

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
dbc110c
webextensions: add a portal for managing WebExtensions native messagi…
jhenstridge Feb 1, 2022
c9e02e9
webextensions: add a permission prompt when starting an extension
jhenstridge Mar 4, 2022
4949408
webextensions: lock session in child watch handler
jhenstridge Mar 4, 2022
d8885ee
webextensions: provide the stderr file descriptor to the client.
jhenstridge Mar 4, 2022
b495e5d
webextensions: make handle_start_in_thread exit early if session is i…
jhenstridge Mar 31, 2022
d5a5260
webextensions: have the child watch hold a reference to the session.
jhenstridge Mar 31, 2022
e84d0fa
webextensions: don't retain the server's file descriptors after GetPi…
jhenstridge Apr 11, 2022
66edca5
webextensions: include per-user manifest directories in default searc…
jhenstridge Apr 11, 2022
da96923
webextensions: pass a valid desktop ID to the access dialog
May 6, 2022
ec70a7b
webextensions: handle case of a NULL GAppInfo
jhenstridge Sep 27, 2022
e101338
webextensions: add a GetManifest method
jhenstridge Sep 27, 2022
28df915
tests: add a basic test for the webextensions portal API
jhenstridge Sep 30, 2022
640d4fc
webextensions: add additional manifest search paths
jhenstridge Nov 12, 2022
78ab279
webextensions: fixes based on Georges's and Jonas's reviews
jhenstridge Nov 21, 2022
d2375d5
web-extensions: send a SIGKILL rather than SIGTERM when closing the s…
jhenstridge Nov 4, 2023
e48d9eb
webextensions: add a mode option to CreateSession that determines whe…
jhenstridge Nov 7, 2023
bec4a1b
webextensions: default to mozilla behaviour
jhenstridge Nov 7, 2023
7b14ac4
webextensions: reject manifest files with non-absolute paths for the …
jhenstridge Nov 7, 2023
5026bd9
webextensions: update terminology to consistently use "native messagi…
jhenstridge Nov 8, 2023
bcef3c7
webextensions: small documentation updates and path typo fixes
Mar 5, 2024
20cd02f
web-extensions: pass the cancellable to xdp_..._call_access_dialog_sy…
jhenstridge Mar 12, 2024
916ae2a
webextensions: add some comments about flow of execution
May 31, 2024
f2cdd8e
webextensions: add references, explain session closure behaviour
Jun 28, 2024
838cadc
webextensions: add $XDG_CONFIG_HOME/mozilla/native-messaging-hosts to…
Aug 1, 2024
21e3a9a
po: add src/webextensions.c to POTFILES.in
jhenstridge Jan 13, 2025
f2d0e70
data: add QtDBus annotations to interface
jhenstridge Jan 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions data/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ portal_sources = files(
'org.freedesktop.portal.Trash.xml',
'org.freedesktop.portal.Usb.xml',
'org.freedesktop.portal.Wallpaper.xml',
'org.freedesktop.portal.WebExtensions.xml',
)

portal_impl_sources = files(
Expand Down
158 changes: 158 additions & 0 deletions data/org.freedesktop.portal.WebExtensions.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
<?xml version="1.0"?>
<!--
Copyright (C) 2022 Canonical Ltd

This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation; either
version 2 of the License, or (at your option) any later version.

This library 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
Lesser General Public License for more details.

You should have received a copy of the GNU Lesser General Public
License along with this library. If not, see <http://www.gnu.org/licenses/>.
-->

<node name="/" xmlns:doc="http://www.freedesktop.org/dbus/1.0/doc.dtd">
<!--
org.freedesktop.portal.WebExtensions:
@short_description: WebExtensions portal

The WebExtensions portal allows sandboxed web browsers to start
native messaging hosts installed on the host system.

Accompanying documentation for Firefox's implementation is
available: `Native messaging for a strictly-confined Firefox
<https://firefox-source-docs.mozilla.org/toolkit/components/extensions/webextensions/native-messaging-portal-design.html>`_.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link leads to a 404.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


This documentation describes version 1 of this interface.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add a link to https://firefox-source-docs.mozilla.org/toolkit/components/extensions/webextensions/native-messaging-portal-design.html

That link is currently 404, but it will be populated once the patch lands, following the documentation changes + review requested after https://phabricator.services.mozilla.com/D140803#5413606

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link added.

-->
<interface name="org.freedesktop.portal.WebExtensions">
<!--
CreateSession:
@options: Vardict with optional further information
@session_handle: Object path for the #org.freedesktop.portal.Session created by this call.

Create a web extensions session. A successfully created
session can at any time be closed using
org.freedesktop.portal.Session::Close, or may at any time be
closed by the portal implementation, which will be signalled
via org.freedesktop.portal.Session::Closed.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Elsewhere in this review (and earlier, at #705 (comment)) I described the expected behavior when a session is closed.

Let's explicitly document what one can expect to happen when the Close is received, and what cause when Closed is signaled. See my comment elsewhere in this review round to get an idea of what you could document.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explanatory comment added.


To close a session, the browser should:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The steps below assume the presence of a process + pipes. That is only if Start + GetPipes have already returned. If it is before, then we should just close the session, right? Here is a tweak to clarify when the following sequence should be followed:

Suggested change
To close a session, the browser should:
To close a session that progressed past Start() + GetPipes(), the browser should:


1. close the process's stdin/stdout/stderr file descriptors
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox doesn't close stdout/stderr, only stdin. It is possible for the native application to continue writing to stderr which could help with debugging if desired. Here is a suggestion that is more accurate.

Suggested change
1. close the process's stdin/stdout/stderr file descriptors
1. close the process's stdin file descriptors, optionally also stdout/stderr.

obtained from the portal;
2. wait for a D-Bus Closed signal from the portal on the
org.freedesktop.portal.Session object (which will be
triggered on SIGCHLD via the g_child_watch_add_full
handler); and
3. if the Closed signal from the portal doesn't come in time,
call the Close method on the org.freedesktop.portal.Session
object.

Supported keys in the @options vardict include:
<variablelist>
<varlistentry>
<term>mode s</term>
<listitem><para>
A string indicating which behaviour the portal should
use when locating and starting native messaging
hosts. Valid values are "mozilla" and "chromium". By
default, mozilla behaviour is used.
</para></listitem>
</varlistentry>
<varlistentry>
<term>session_handle_token s</term>
<listitem><para>
A string that will be used as the last element of the session handle. Must be a valid
object path element. See the #org.freedesktop.portal.Session documentation for
more information about the session handle.
</para></listitem>
</varlistentry>
</variablelist>
-->
<method name="CreateSession">
<annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="QVariantMap"/>
<arg type="a{sv}" name="options" direction="in"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this need annotations for Qt. See #1531

<arg type="o" name="session_handle" direction="out"/>
</method>
<!--
GetManifest:
@session_handle: Object path for the #org.freedesktop.portal.Session object
@name: name of the native messaging host
@extension_or_origin: extension ID or origin URI identifying the extension
@json_manifest: the JSON manifest for the native messaging host

Return the JSON manifest of the native messaging host that
Start would invoke.
-->
<method name="GetManifest">
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason GetManifest can't be baked into Start? By the looks of it, this "leaks" information about available native messaging hosts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GetManifest method came out of this request in the Firefox PR:

https://phabricator.services.mozilla.com/D140803#inline-868567

We had the Mozilla side patches working without this method before hand, but they wanted to validate the manifest themselves (even though the browser won't be able to directly launch the native messaging server).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't they validate after starting?

The end result here, with this method, is that any application can without any interference or feedback to the user probe the system for installed native messaging hosts.

I realize it's not as bad as arbitrary access to $HOME etc, but it's still a sandbox hole.

<arg type="o" name="session_handle" direction="in"/>
<arg type="s" name="name" direction="in"/>
<arg type="s" name="extension_or_origin" direction="in"/>
<arg type="s" name="json_manifest" direction="out"/>
</method>
<!--
Start:
@session_handle: Object path for the #org.freedesktop.portal.Session object
@name: name of the native messaging host
@extension_or_origin: extension ID or origin URI identifying the extension
@options: Vardict with optional further information
@handle: Object path for the #org.freedesktop.portal.Request object representing this call

Start the named native messaging host. The caller must
indicate the requesting web extension (either by extension ID
for Firefox, or origin URI for Chrome), which will be matched
against the host's access control list.

If the host can't be started, or invalid data is provided,
the session will be closed.

Supported keys in the @options vardict include:
<variablelist>
<varlistentry>
<term>handle_token s</term>
<listitem><para>
A string that will be used as the last element of the @handle. Must be a valid
object path element. See the #org.freedesktop.portal.Request documentation for
more information about the @handle.
</para></listitem>
</varlistentry>
</variablelist>
-->
<method name="Start">
<arg type="o" name="session_handle" direction="in"/>
<arg type="s" name="name" direction="in"/>
<arg type="s" name="extension_or_origin" direction="in"/>
<annotation name="org.qtproject.QtDBus.QtTypeName.In3" value="QVariantMap"/>
<arg type="a{sv}" name="options" direction="in"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

annotation here too.

<arg type="o" name="handle" direction="out"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have a parent_window argument to get correct stacking (would regular dialogs and not system modal dialogs be used)?

</method>
<!--
GetPipes:
@session_handle: Object path for the #org.freedesktop.portal.Session object
@options: Vardict with optional further information
@stdin: File descriptor representing the hosts's stdin.
@stdout: File descriptor representing the host's stdout.
@stderr: File descriptor representing the host's stderr.

Retrieve file descriptors for the native messaging host
identified by the session. This method should only be called
after the Start request recveives a successful response.
-->
<method name="GetPipes">
<annotation name="org.gtk.GDBus.C.UnixFD" value="true"/>
<arg type="o" name="session_handle" direction="in"/>
<annotation name="org.qtproject.QtDBus.QtTypeName.In1" value="QVariantMap"/>
<arg type="a{sv}" name="options" direction="in"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and annotation here.

<arg type="h" name="stdin" direction="out"/>
<arg type="h" name="stdout" direction="out"/>
<arg type="h" name="stderr" direction="out"/>
</method>
<property name="version" type="u" access="read"/>
</interface>
</node>
2 changes: 2 additions & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ libdir = prefix / get_option('libdir')
libexecdir = prefix / get_option('libexecdir')
sysconfdir = prefix / get_option('sysconfdir')
localedir = prefix / get_option('localedir')
sysconfdir = prefix / get_option('sysconfdir')
dbus_service_dir = get_option('dbus-service-dir')
if dbus_service_dir == ''
dbus_service_dir = prefix / datadir / 'dbus-1' / 'services'
Expand Down Expand Up @@ -82,6 +83,7 @@ config_h = configuration_data()
config_h.set('_GNU_SOURCE', 1)
config_h.set_quoted('G_LOG_DOMAIN', 'xdg-desktop-portal')
config_h.set_quoted('DATADIR', datadir)
config_h.set_quoted('LIBDIR', libdir)
config_h.set_quoted('LIBEXECDIR', libexecdir)
config_h.set_quoted('LOCALEDIR', localedir)
config_h.set_quoted('SYSCONFDIR', sysconfdir)
Expand Down
1 change: 1 addition & 0 deletions po/POTFILES.in
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ src/screenshot.c
src/settings.c
src/usb.c
src/wallpaper.c
src/web-extensions.c
1 change: 1 addition & 0 deletions src/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ xdg_desktop_portal_sources = files(
'settings.c',
'trash.c',
'wallpaper.c',
'web-extensions.c',
'xdg-desktop-portal.c',
'xdp-app-launch-context.c',
'xdp-background-monitor.c',
Expand Down
Loading
Loading