1. For security context creation, only relies on WAYLAND_DISPLAY, do not
use WAYLAND_SOCKET since the file descriptor defined by WAYLAND_SOCKET
can be only consumed once.
2. Due to the incompatiblity between WAYLAND_SOCKET and the security
context, add a new permission --socket=inherit-wayland-socket
to limit the usage of WAYLAND_SOCKET to an opt-in feature. Only when
this flag is set, WAYLAND_SOCKET will be passed to the sandbox.
3. When WAYLAND_SOCKET is not inherited, set FD_CLOEXEC to avoid it to
be leaked the to sandbox.
Closes: #5614
By giving the context a bit more knowledge of which directories Flatpak
uses for what, we can stop it from depending on the dir/run modules,
which themselves depend on FlatpakContext.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previously we were relying on flatpak-utils-private.h having a circular
dependency on this, but I'm intending to remove that during future
refactoring.
Signed-off-by: Simon McVittie <smcv@collabora.com>
There isn't much in the way of legit reasons for this, but it's a
potential security footgun when displaying the text.
CVE-2023-28101, GHSA-h43h-fwqx-mpp8
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
Co-authored-by: Simon McVittie <smcv@collabora.com>
If $HOME is below a reserved path (for example `/usr/home/thompson`
for Unix traditionalists) or otherwise cannot be shared, or is a
symbolic link to somewhere that cannot be shared, then we will end
up running the app with $HOME not existing. This is unexpected, so
we should make more noise about it.
There are two situations here, both of which get a warning: if we have
--filesystem=home or --filesystem=host then we are trying to share the
real $HOME with the application, and if we do not, then we are trying
to create a directory at the location of the real $HOME and replicate
the chain of symlinks (if any) leading from $HOME to that location.
Unlike the previous commit, this is not expected to happen during unit
testing, so we do not use a g_warning() for this.
Diagnoses: https://github.com/flatpak/flatpak/issues/5035
Signed-off-by: Simon McVittie <smcv@collabora.com>
If the user gives us a override or command-line argument that we cannot
obey, like --filesystem=/usr/share/whatever or
--filesystem=/run/flatpak/whatever, then it's confusing that we silently
ignore it. We should give them an opportunity to see that their override
was ineffective.
However, there are a few situations where we still want to keep quiet.
If there is a --filesystem argument for something that simply doesn't
exist, we don't diagnose the failure to share it: that avoids creating
unnecessary noise for apps that opportunistically share locations that
might or might not exist, like the way the Steam app on Flathub asks
for access to $XDG_RUNTIME_DIR/app/com.discordapp.Discord.
Similarly, if we have been asked for --filesystem=host, the root
directory is very likely to contain symlinks into a reserved path, like
/lib -> usr/lib. We don't need a user-visible warning for that.
We actually use the equivalent of g_message() rather than g_warning(),
to avoid this being fatal during unit testing (in particular when we
do a `flatpak info` on an app that has never been run, which will
be unable to share its `.var/app` subdirectory). `app/flatpak-main.c`
currently displays them as equivalent to each other anyway.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This brings us one step closer to being able to stop using the flatpak2
log domain for messages that are exclusive to `flatpak -v -v`.
Signed-off-by: Simon McVittie <smcv@collabora.com>
When filesystem=host access is provided, some root folders are hidden, including /boot.
The bootloader specification now recommends mounting the system EFI filesystem in /efi
(currently visible) instead of /boot/efi (currently hidden). This hides /efi for the same
reasons /boot is already hidden.
To make indentation work with less effort. The modeline was copied from
libostree with minor modification and the .editorconfig from GLib.
The advantage of having both a modeline and an editorconfig is we can
work out of the box on more editor setups, and the modeline allows us to
specify the style with a lot more fine grained control.
We already allow normal apps to own MPRIS names but subsandboxes could not.
This allows them with the same dbus restrictions that they must be
prefixed by $app_id.Sandboxed.
This will be used by WebKitGTK.
Of the 19 instances where g_mkdir_with_parents() is used, these are
the only ones where the return value is ignored. This triggers
Coverity.
It might not be strictly necessary to handle the errors, but doing so
can only help with debugging.
If we're using a system copy of xdg-dbus-proxy, it's not really correct
to include a header from our subproject (which we are otherwise not
going to be compiling), and Meson is stricter about this than Autotools.
Instead, duplicate the FlatpakPolicy enum, which is the only part we
actually need.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Recent Meson versions have warnings if you add the subprojects
directory as an include path, because the way Meson wants to consume
subprojects is by the subproject's build system producing a Meson
dependency object that encapsulates its include directory. Flatpak
doesn't have a Meson build system yet, but I'm working on that.
libglnx seems to be set up to have the libglnx directory be its include
path instead: for example, ostree (by the author of libglnx) already
uses "libglnx.h" or <libglnx.h> everywhere. Do the same here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
When generating flattened permissions (i.e. for --show-permissions or
for the /.flatpak-info file) we're currently flattening the permissions
i.e. don't show things that would only affect layering the permissions).
However, the code doesn't currently do this for the filesystem key, so
implement that. This means we only display the permissions that are
in effect, and don't display "negative" permissions like !host which
are not meaningful in this context.
This reintroduces the special case that existed in Flatpak 1.12.3, but
under a different name, so that it will be backwards-compatible. With
this change, flatpak-builder will be able to resolve CVE-2022-21682 by
using --filesystem=host:reset.
We want to implement this as a suffix rather than as a new keyword,
because unknown suffixes are ignored with a warning, rather than causing
a fatal error. This means that the new version of flatpak-builder will
be able to run against older versions of flatpak: it will still be
vulnerable to CVE-2022-21682 in that situation, but at least it will run.
Co-authored-by: Alexander Larsson <alexl@redhat.com>
This caused regressions for some previously-working use cases. For
example, some Flatpak users previously used a global
`flatpak override --nofilesystem=home` or
`flatpak override --nofilesystem=host`, but expected that individual apps
would still be able to have finer-grained filesystem access granted by the
app manifest, such as Zoom's `--filesystem=~/Documents/Zoom:create`. With
the changes in 1.12.3, this no longer has the desired result, because
`--nofilesystem=home` was special-cased to disallow inheriting the
finer-grained `--filesystem`.
This reverts commit 445bddeee6.
This reverts the initial solution to CVE-2022-21682, which we intend to
resolve differently, by introducing a new feature in Flatpak and making
use of it in a new flatpak-builder version.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Previously --nofilesystem=host only removed specifically access to the
`host` permissions, and not necessarily other filesystems (like `home`
or `/some/path`). This isn't very useful to limit access because you
don't know what other filesystems the app may have access too.
We change this to mean that `--nofilesystem=host` removes *all* filesystem
access from the parent layer, and `--nofilesystem=home` removes all
file access to the homedir and paths inside it.
The available layers are, in order:
* app permissions
* overrides
* commandline args
This allows you to start from scratch with the filesystem permissions
in the overrides or the commandline. This is a small change in
behaviour, but not a lot of things use --nofilesystem, and the ones
that do probably expects this behaviour.
If XDG_RUNTIME_DIR is under app control, as it will be with #4120, we
don't want to be mounting pieces of filesystem directly into it, because
that will mean that the app could create a symlink that will cause us
to create a mount point for it at the target of the symlink, potentially
elsewhere in the host filesystem.
Instead, we mount them in /run/flatpak, which is a per-instance
directory entirely controlled by Flatpak; and then create (relative)
symlinks in XDG_RUNTIME_DIR, pointing into /run/flatpak.
In this commit, we still know that the XDG_RUNTIME_DIR is a
per-instance tmpfs, so we can safely create the symlinks using
the --symlink option. In a subsequent commit this will change to
creating them in a shared XDG_RUNTIME_DIR, if any.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Similar to /tmp, applications might well use /dev/shm as an IPC
rendezvous between instances, which wouldn't have worked without
--device=shm until now.
Because /dev/shm has specific characteristics (in particular it's
meant to always be a tmpfs), we offload the actual storage into a
subdirectory of the real /dev/shm. Because /dev/shm is a shared
directory between all uids, we have to be extra-careful how we
do this, which is why the test coverage here is important.
This is done on an opt-in basis because of its extra complexity.
Signed-off-by: Simon McVittie <smcv@collabora.com>
A subsequent commit will need to look at the FlatpakExports before
we are ready to append their arguments to the FlatpakBwrap.
Signed-off-by: Simon McVittie <smcv@collabora.com>
flatpak_context_get_exports_full() previously copied the interface of
flatpak_context_export(), which appended entries to a caller-supplied
GString, but it's a more GLib-style API if we use an "out" argument.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This combines the functionality of flatpak_context_get_exports() and its
open-coded version in flatpak_context_append_bwrap_filesystem().
Signed-off-by: Simon McVittie <smcv@collabora.com>
When the portal's Spawn method is used with the environment cleared,
it's very likely that the "flatpak run" that ends up being run will be
in an environment without UTF-8 support.
If one of the files or directories we try to expose to the sub-sandbox
contains UTF-8/non-ASCII characters, then "flatpak run" would fail with:
error: Invalid byte sequence in conversion input
This is caused by GOption trying to parse the --filesystem option for
flatpak, as, when using the G_OPTION_ARG_CALLBACK argument type, GOption
will split the option name from its value, and try to convert the value
to UTF-8. Which will fail because there's no UTF-8.
It won't however do that if we tell the option parser that the value is
a filename using G_OPTION_FLAG_FILENAME, so set it.
This follows up from GHSA-4ppf-fxf6-vxg2 to fix missing functionality
that I noticed while resolving that vulnerability, but is not required
for fixing the vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Building with Clang 11 shows a warning for the cast of the 'gpointer' to
'FlatpakFilesystemMode'.
This is due to '-Wpointer-to-int-cast' being now enabled by default.
common/flatpak-context.c:2144:13: warning: cast to smaller integer type
'FlatpakFilesystemMode' from 'gpointer' (aka 'void *')
[-Wvoid-pointer-to-enum-cast]
fs_mode = (FlatpakFilesystemMode) g_hash_table_lookup (context->filesystems, "host");
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com>
Avoid shadowing variables that are already declared in a previous scope,
and make such occurrences compile-time errors. These are not functional
changes.
In a few places do related code cleanup.
A similar ostree PR is here:
https://github.com/ostreedev/ostree/pull/2195
flatpak doesn't yet use -Wswitch-enum, but perhaps it should at some
point. Now that FLATPAK_FILESYSTEM_MODE_NONE is a member of the enum,
it should be handled; and if we're doing that, we might as well make
the same function fully responsible for it.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Historically we didn't accept them, but there's no real reason why not.
They're normalized to the form in which earlier Flatpak releases would
want to see them.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Making it an equivalent of --filesystem=host would be misleading,
because it wouldn't do what you'd think it does: host mounts some host
system directories in their usual places, but others below /run/host.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Paths containing ".." are rejected: they're almost certainly a
terrible idea.
Paths containing "." or multiple slashes are syntactically normalized.
This assumes that nobody is going to use "--filesystem=/foo/bar/" to
mean "make /foo/bar available, unless it's a non-directory, in which
case fail".
Signed-off-by: Simon McVittie <smcv@collabora.com>
This gives us the ability for the parse function (the former verify
function) to carry out a normalization step as well.
Signed-off-by: Simon McVittie <smcv@collabora.com>
These are subsets of the host keyword, which provide access to operating
system files but not to users' personal files.
In particular, the experimental support for namespace-based sandboxes
in the Steam Runtime[1] uses the graphics stack from the host system,
which requires access to the host /usr/libQUAL, /libQUAL (even if the
host OS has undergone the /usr merge, the canonical paths of ELF
interpreters start with /lib), /etc/ld.so.cache, and for some libraries
on Debian-based systems, /etc/alternatives. It will not be possible to
do similar things in Flatpak without either allowing full host
filesystem access (which exposes personal files, and in any case cannot
be done by the Steam app because it is incompatible with --persist=.),
or adding the ability to expose /usr and related directories without
including the rest of the host filesystem.
To the best of my knowledge, host-etc is not necessary for anything;
I've mainly provided it for symmetry, since it's the other significant
thing that we mount in /run/host and cannot get via --filesystem=/path.
Some notes on the security/privacy implications of the new keywords:
- Neither new keyword allows anything that was not already allowed
by "host".
- Neither new keyword can allow anything that was not already allowed
to the user outside the sandbox.
- "host-os" allows enumeration of the installed packages on the host
system, and often their version numbers too. A malicious app could
use this to look for exploitable security vulnerabilities on the
host system. An app could also use this for fingerprinting, although
this is not a regression, because the systemd/D-Bus machine ID,
MAC addresses, hostname, kernel boot UUID, DMI product ID and many
other unique or relatively unique properties are already available
inside the sandbox.
- "host-os" allows read access, and possibly write access (if the user
has it outside the sandbox, for example members of group 'staff' in
older Debian installations), to /usr/local.
- "host-etc" allows reading configuration files whose contents might
be considered sensitive, such as /etc/passwd.
[1] https://steamcommunity.com/app/221410/discussions/0/1638675549018366706/
Signed-off-by: Simon McVittie <smcv@collabora.com>
Currently there are only "home" and "host", but I'm going to add one
that represents /usr and friends (/usr, /lib, ...), and one for /etc.
These differ from ordinary filesystem mounts because they are redirected
into /run/host to avoid conflicting with the runtime.
Signed-off-by: Simon McVittie <smcv@collabora.com>
It was called flatpak_exports_add_home_expose(), but it actually
exposed the entire host filesystem, to the extent possible.
Rename it to flatpak_exports_add_host_expose() to reflect that.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Don't allow adding access to things like ~/foo xdg-foo/bar or similar
things just because you used to have home access, because such files
may be outside the homedir (for instance, if they are symlinks or configured
via xdg-user-dirs).
This new permission exposes the host /dev, which is normally not visible
even with --device=all, as it is not really a device node but rather
a bunch of shared memory blocks available on the host.
This access is needed by jack, as explained at:
https://github.com/flatpak/flatpak/issues/1509
Long term I think a better solution for pro audio (like pipewire) is
a better solution, but for now we should at least allow jack apps to work.
This means sandboxes with dbus access can use portals etc, but they can't
talk to the main app, or impersonate it, but you can still use dbus and
well-known names to talk to them if needed.
It does mean however that if you use this, different sandboxes can see each
other on the bus, so be careful.
