webextensions: add a portal for managing WebExtensions native messaging servers

[xhorak]

This MR continues the work from the abandoned MR #705.

The following updates and improvements have been made:

• rebase
• annotation for qt
• fixed test

@jhenstridge thank you for your work on this! Let me know if you'd like to collaborate further.

This is intended to provide a way for a confined web browser to start native code helpers for their extensions. At present it can start the servers installed on the host system. But in future this could be extended to cover sandboxed native messaging servers too.

[Rob--W]

Has the other PR really been abandoned, or is the author merely focused on other tasks or even away from work? The latter would not be surprising considering that we are approaching the end of the year.

This patch squashes all changes from #705 into one, along with new changes. I suppose that this was the easiest way to rebase, but it causes the lost of context because some individual commits offered context that are missing from the squashed commit.

Out of all, I think that it would make most sense to include the context of 59d7b4b addresses #769 . E.g. by including the issue reference in the commit message ("Fixes xxx") and a code comment pointing to the discussion.
I have not reviewed the updated patch yet, but note that in the previous PR there was still an active discussion on this topic at #705 (comment)

[swick]

I'd really like if the test was moved over to python.

[xhorak]

I've been trying to contact authors since late October but did not received any reply. I would happily keep the original PR but I want to move this forward. Especially now when the Firefox part landed.

The squash was suggested, so I did it. I'll check the 59d7b4b
I plan to do some testing because I'm getting various app_ids as I've mentioned in #705 (comment)

[xhorak]

In latest update I've fixed the xdp_app_info_get_gappinfo call and stopped to use g_desktop_app_info_new.

[grulja]

I plan to do some testing because I'm getting various app_ids as I've mentioned in #705 (comment)

That's the usual issue we have with host apps. The application id comes from cgroups, which is set depending on the way you start the app, e.g. you start it from a terminal app it might get app id of the terminal app. Or you start it with Alt + F2 in GNOME and the app id will be just firefox. Flatpak apps are reliable in this case.

See #1512 what we did for the camera portal, where this was causing issues.

[swick]

Regarding the appid mismatch that unfortunately still happens: I'd much rather see us adding #1521 than special casing every portal that firefox actually depends on.

[xhorak]

I'd really like if the test was moved over to python.

Looking at the python vs C tests, there seems to be missing permission store service in Python. I'm not sufficiently familiar with the code to implement it for python tests or is it done somewhere already?

[swick]

The permission store is supposed to get started (https://github.com/flatpak/xdg-desktop-portal/blob/1c902cc77e53b422d49988bab58b6cca0ed9b112/tests/conftest.py#L458C5-L458C25) when the test case uses either the xdg_permission_store or the more general portals fixtures.

[xhorak]

The proposal (by @swick) suggests replacing xdg-desktop-portal with a separate service, org.freedesktop.native_messaging, for native messaging API support. This would make the feature exclusive to browsers using the --talk-name=org.freedesktop.native_messaging permission. On the other hand it removes the user access dialog provided by xdg-desktop-portal.

The service API remains unchanged, but as noted by @swick, this approach does not address potential sandbox escapes but it will be exclusive to those who want to use the org.freedesktop.native_messaging.

@Rob--W please share your thoughts from the Firefox point of view and all the work that has been done on the Firefox side.

[Rob--W]

Could you elaborate on the changes that you're considering? What are the current values and the proposed values?

It sounds like the proposal may effectively introduce a backwards-incompatible change, requiring changes to https://searchfox.org/mozilla-central/rev/7d1b5c88343879056168aa710a9ee743392604c0/toolkit/components/extensions/NativeMessagingPortal.cpp#110-112, and maybe other places too.

The feature in Firefox is currently disabled by default to offer room for such breaking changes if really needed. I would imagine Canonical to either apply distro-specific patches to Firefox, or update the portal to transition to the new state. But I cannot speak on behalf of them, so once you've clarified the exact changes that you're considering, I'll ping them.

[xhorak]

Could you elaborate on the changes that you're considering? What are the current values and the proposed values?

@swick I'm not sure how the separate service will support creating and maintaining sessions currently implemented by xdp. Could you help?

[swick]

Maybe I should try to explain why I would like to keep the native messaging host functionality out of portals and expand a bit on the alternative that I'm suggesting.

Portal APIs are meant to be generically useful platform APIs with high stability which work with the restrictions of a sandbox, and in particular don't allow sandbox escapes. In my opinion the native messaging host portal doesn't meet those criteria:

• It is only exposed to very specific apps instead of being generically useful
• It still can be used to escape the sandbox in some circumstances
• An API that doesn't have the potential sandbox escape might look very different and we wouldn't be able to guarantee stability

The potential sandbox escape comes from the fact that native messaging hosts (NMH) didn't have to deal with a sandboxed browser before as they were both running in the trusted computing base (TCB). If an attacker takes control over firefox, it can use the portal to use all APIs exposed via any NMH. If any of those provides an API that can be used to escape the sandbox, so can the attacker. This could be something as simple as providing an arbitrary path to write a file into.

The portal does ask the user the question "Allow %s to start WebExtension backend" but in general a user which has NHMs installed wants to use them and doesn't know if the browser is malicious, so this doesn't prevent the attack.

Besides not preventing the sandbox escape, the question serves no other purpose because installing the browser extension and having the NMH installed is already enough intent to let them communicate (under the assumption that there is no sandbox escape).

So I'm suggesting to move the code out of the x-d-p project and expose it on a bus name that is not visible to sandboxed apps such as org.freedesktop.NativeMessagingProvider. The API has to change slightly because we no longer have a concept of a session. We could merge everything into the GetManifest and Start methods: mode from the session becomes an argument to both and GetPipes can become part of Start.

It doesn't fix the inherent sandbox escape problem, but at least it gets us some time to figure out how to get there properly.

[Rob--W]

@jhenstridge resumed work on the original PR (#705). Have you all synchronized with each other? Which PR is intended to be the one to be merged?

To me (as a relatively outsider in this project who merely contributes reviews as a subject matter expert in the topic of extensions and NativeMessaging), it is unclear what the desired intermediate and long-term direction is:

• webextensions: add a portal for managing WebExtensions native messaging servers #705 was the original PR, and has now been revived.
• before 705 was revived, this PR was created.
• in comments above, a different design was sketched, that does not only diverge in terms of code, but also external API.

Could someone clarify?

[Mikenux]

@Rob--W: See #1191

Maybe this:

• A containerized browser can only exchange data with another containerized app. Need to determine how much the app needs to be containerized.
• The request to the portal should be about requesting to exchange data with a specific app.
• The discovery of the app to be contacted is the responsibility of the portal or the sandbox system. Same for launching the app (we should have the possibility to present a dialog box to confirm the launch of the app) .
• I wonder if something general can be proposed to allow the general exchange of data between app.

[xhorak]

@jhenstridge resumed work on the original PR (#705). Have you all synchronized with each other? Which PR is intended to be the one to be merged?

To me (as a relatively outsider in this project who merely contributes reviews as a subject matter expert in the topic of extensions and NativeMessaging), it is unclear what the desired intermediate and long-term direction is:

* [webextensions: add a portal for managing WebExtensions native messaging servers #705](https://github.com/flatpak/xdg-desktop-portal/pull/705) was the original PR, and has now been revived.

* before 705 was revived, this PR was created.

* in comments above, a different design was sketched, that does not only diverge in terms of code, but also external API.

Could someone clarify?

I've tried to contact @jhenstridge in past few months without any luck, so I've decide to move things forward with this PR.
We're trying to figure out how to proceed further.

[PhrozenByte]

I'd like to chip in a thought: Do we really need the portal to be able to start other applications?

I know that the original idea is about supporting the known and well established Native Messaging API with Flatpak and Snap. However, the API's capability of running and communicating with arbitrary binaries somewhat conflicts with the concept of containerization. The debate revolves around the question of how the Native Messaging API with its powerful capabilities could be limited to fit into Flatpak's security model and has ultimately led to a more or less finished solution not being integrated for several years now.

So I'd like to ask whether we even need it: Is there a reason (besides requiring changes to applications using Native Messaging, I rather mean conceptionally) why we couldn't flip things over, i.e. requiring other applications to provide the necessary communication endpoints and the browser connecting to these endpoints, rather than the browser starting other applications?

The exact solution doesn't really matter right now: It could be a DBus interface (requiring --talk-name), or it could even be a well-known runtime directory applications create socket files in for sandboxed browsers to use (requiring --filesystem). It would require the other applications to start in advance and possibly run in background to wait for a browser connecting with their endpoint to do their thing though. Are there any extensions that wouldn't work that way? I mean, applications running in background would fit into Flatpak's security model quite well: We even have great desktop integration and it is under full user control.

[swick]

The problem isn't who execs the native messaging hosts. The problem is that the native messaging hosts might expose APIs on that socket which can be used to escape the sandbox.

[PhrozenByte]

The problem isn't who execs the native messaging hosts. The problem is that the native messaging hosts might expose APIs on that socket which can be used to escape the sandbox.

@swick As you correctly stated before, this is by design and more or less the point of Native Messaging. I personally don't see much issue with it, because this isn't something Flatpak needs to or even can sort out, but something the browser vendors must sort out with the developers of applications using the Native Messaging API. This issue isn't even special to the Native Messaging API: As you correctly pointed out in the same post, the same could happen with any custom bus.

I therefore suggest focusing on things that increase the attack surface in comparison to the status quo, like the Native Messaging API being able to more or less execute arbitrary binaries outside the sandbox. IMHO this is a critical issue and requires not just "some" user interaction, but must allow the user to fully understand what's going on. @Mikenux's suggestions address this pretty well IMHO.

However, if the Native Messaging API couldn't start applications, I personally don't see much of an issue: Browsers then can only communicate with applications that already run and advertised that they want to talk with a browser using the Native Messaging API (i.e. e.g. by using a custom bus, or creating a socket file in a well-known runtime directory). The issue is then similar to any other custom bus, and users are made aware of running applications by other means already.

Long story short: I'm asking whether we even need a portal. And whether I've missed other issues.

[Mikenux]

@PhrozenByte: To clarify, what I've written is for long-term direction, not for intermediate direction. However, I guess people want to know what to make an intermediate solution on which the "right" solution can be built.

[swick]

As you correctly #1537 (comment), this is by design and more or less the point of Native Messaging

This is not by design, it's just how it turned out to be because native messaging hosts didn't have to care about not providing sandbox escapes for the browser before.

Focusing on another aspect is pointless because this is a fundamental flaw that must get sorted out before it can be exposed.

This issue isn't even special to the Native Messaging API: As you correctly pointed out in the same post, the same could happen with any custom bus.

That's why we only expose APIs (including busses) where the end-points understand the threat-model of having a potentially malicious actor on the other side which will try to escape the sandbox or otherwise mess up the system. The the Native Messaging APIs are not one of those.

like the Native Messaging API being able to more or less execute arbitrary binaries outside the sandbox

It cannot. There is no such issue. This is distracting from the main problem.

[PhrozenByte]

This is not by design, it's just how it turned out to be because native messaging hosts didn't have to care about not providing sandbox escapes for the browser before.

Focusing on another aspect is pointless because this is a fundamental flaw that must get sorted out before it can be exposed.

I don't agree with that assessment. However, I'd only repeat things that were said in #705 multiple times before. I agree that this whole discussion is a distraction though. Let's get this PR or #705 merged first and discuss alternative approaches later.

[swick]

Sorry, but the correct response to "this PR contains a security vulnerability" is not to go ahead and merge it.

[PhrozenByte]

This has been discussed extensively in #705 over the past three years. You have your opinion on the matter and were able to express it in detail. Others don't agree with it, yet respect your opinion and have addressed your concerns. You don't have to agree with the solution, but please accept that others have different opinions on the matter and please refrain from depicting your opinion as fact ("this PR contains a security vulnerability"). This is disruptive behaviour and rude.

[swick]

Rebased because of the testing changes. Works locally but somehow CI fails with the GetPipes call with org.freedesktop.DBus.Error.AccessDenied: Invalid session. https://github.com/swick/xdg-desktop-portal/commits/wip/webextensions