Repository navigation
portal: Ignore unusable paths - #6588
Conversation
7ff2538 to
ba59126
Compare
|
Based on a mixture of swick@cc4309e and my earlier #6586. Compared with #6586, this has the more-backward-compatible behaviour of silently ignoring any attempt to sandbox-expose a fd that is unsuitable for whatever reason, and avoids refactoring the handle-to-fd mapping too much. I still think 8ddaf68 and its followups would be a good change, but let's not do that until after the dust has settled. Compared with @swick's version, this is several commits, more verbosely documented, and adjusts some variable names and whitespace to make the diff vs. 1.17.3 smaller, to try to make it easier to convince ourselves that this is really fixing regressions introduced when we resolved CVE-2026-34078. (It also uses an early-return for the checks that still need to be fatal, which I think makes the flow clearer.) |
|
cc @bertogg |
| * check things and abort if something is off. We do this only for backwards | ||
| * compatibility reasons: we need to be able to ignore the issue instead of | ||
| * aborting the entire sandbox setup later. */ | ||
| if (stat (path, &real_st_buf) < 0 || |
There was a problem hiding this comment.
If stat() fails maybe "different files inside and outside the sandbox" is a bit misleading, but I think that it is good enough for this case.
The patch looks good to me, thanks!
There was a problem hiding this comment.
I realise the message is not ideal, but I was intentionally putting back code that is very similar to we had before addressing CVE-2026-34078, so that we can diff vs. 1.16.3 and 1.17.3 to get extra confidence that this is right.
There was a problem hiding this comment.
I suppose we could even identify if the result of flatpak_get_path_for_fd() is a path inside one of the "reserved" directories (/app, /usr, ...) and silently ignore the fd because it's guaranteed to be available, but if the idea is that the caller stops trying to expose those directories then this is fine.
Thanks!
This was originally in flatpak-portal, then was duplicated into flatpak-run in commit ac62ebe "run: Use O_PATH fds for the runtime and app deploy directories", and subsequently removed from the portal in commit 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to avoid races". Now we want to use it in the portal again. Helps: flatpak#6584 Co-authored-by: Sebastian Wick <[email protected]> Signed-off-by: Simon McVittie <[email protected]>
If the handle is not in the range `0 <= handle < fds_len`, but no GError is set, we'd have crashed when we dereferenced error->message. Instead, log an error and early-return, matching what we do for app-fd, usr-fd and the array of inheritable fds. Fixes: 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to avoid races" Helps: flatpak#6584 Co-authored-by: Sebastian Wick <[email protected]> Signed-off-by: Simon McVittie <[email protected]>
For the sandbox expose fds, a historical quirk of this code is that if the checks in get_path_for_fd() failed, we would merely log at g_info() level (usually only shown when debugging the portal), and otherwise silently ignore the request to expose the fd in the sandbox. With hindsight this was probably not the right thing to do, but apps could well be relying on it now. For example, there are indications that Epiphany might send a memfd from the main instance to a subsandbox, which never actually worked, but will break that subsandbox process if that's treated as a fatal error. Fixes: 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to avoid races" Helps: flatpak#6584 Co-authored-by: Sebastian Wick <[email protected]> Signed-off-by: Simon McVittie <[email protected]>
As with the previous commit, historically we would debug-log but otherwise silently ignore attempts to expose a file in a sandboxed subsandbox that doesn't have a suitable path. For example, org.gnome.Epiphany (or possibly WebKitGTK) asks to expose files from /app and /usr in the subsandbox. When we ignored those requests (because /app and /usr have a different meaning on the host system), the app worked as intended anyway, because the subsandbox has access to the app's /app and the runtime's /usr whether they're explicitly added or not, so it all worked out OK. However, treating this as a fatal error (as it arguably should have been) broke Epiphany's subsandboxes. Fixes: 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to avoid races" Resolves: flatpak#6584 Co-authored-by: Sebastian Wick <[email protected]> Signed-off-by: Simon McVittie <[email protected]>
|
Rebased on #6589, fixing conflicts |
ba59126 to
146b4d7
Compare
3 participants
utils: Move flatpak_get_path_for_fd to here
This was originally in flatpak-portal, then was duplicated into
flatpak-run in commit ac62ebe "run: Use O_PATH fds for the runtime and
app deploy directories", and subsequently removed from the portal in
commit 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to
avoid races". Now we want to use it in the portal again.
Helps: [Bug]: Epiphany crashing with Flatpak 1.16.5: bwrap: Can't find source path /proc/self/fd/79: No such file or directory #6584
Co-authored-by: @swick
portal: Avoid crash if sandbox-expose-[ro-]fd is out of range
If the handle is not in the range
0 <= handle < fds_len, but noGError is set, we'd have crashed when we dereferenced error->message.
Instead, log an error and early-return, matching what we do for
app-fd, usr-fd and the array of inheritable fds.
Fixes: 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to avoid races"
Helps: [Bug]: Epiphany crashing with Flatpak 1.16.5: bwrap: Can't find source path /proc/self/fd/79: No such file or directory #6584
Co-authored-by: @swick
portal: Log and ignore unusable sandbox-expose fds instead of erroring
For the sandbox expose fds, a historical quirk of this code is that if
the checks in get_path_for_fd() failed, we would merely log at g_info()
level (usually only shown when debugging the portal), and otherwise
silently ignore the request to expose the fd in the sandbox.
With hindsight this was probably not the right thing to do, but apps
could well be relying on it now. For example, there are indications
that Epiphany might send a memfd from the main instance to a subsandbox,
which never actually worked, but will break that subsandbox process
if that's treated as a fatal error.
Fixes: 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to avoid races"
Helps: [Bug]: Epiphany crashing with Flatpak 1.16.5: bwrap: Can't find source path /proc/self/fd/79: No such file or directory #6584
Co-authored-by: @swick
portal: Reinstate flatpak_get_path_for_fd() checks
As with the previous commit, historically we would debug-log but
otherwise silently ignore attempts to expose a file in a sandboxed
subsandbox that doesn't have a suitable path.
For example, org.gnome.Epiphany (or possibly WebKitGTK) asks to expose
files from /app and /usr in the subsandbox. When we ignored those
requests (because /app and /usr have a different meaning on the host
system), the app worked as intended anyway, because the subsandbox has
access to the app's /app and the runtime's /usr whether they're
explicitly added or not, so it all worked out OK. However, treating
this as a fatal error (as it arguably should have been) broke
Epiphany's subsandboxes.
Fixes: 3c50014 "portal: Use --bind-fd, --app-fd and --usr-fd options to avoid races"
Resolves: [Bug]: Epiphany crashing with Flatpak 1.16.5: bwrap: Can't find source path /proc/self/fd/79: No such file or directory #6584
Co-authored-by: @swick