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>
(cherry picked from commit 917a7f5870)
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.
(cherry picked from commit e2c8863fb62301cb05c64bbb32b04446e88ce11a)
If we have a bundle without metadata we wouldn't properly present
the permissions in the transaction.
(cherry picked from commit b250541302187ff2209b0bb1295e8223d0af860f)
This was disables a long time ago because the fedora remotes didn't
contain metadata, but that has been added since then. Requiring fixes
a security concern where an app claims to require no permissions (by
having no metadata in commit) but then actually requires permissions
in the installed app.
(cherry picked from commit f0f3a35f404b5bd533186095db055f8b3d135576)
If we fail to parse xa.metadata from the summary cache or the commit
xa.metadata we fail the resolve.
If xa.metadata is missing in the commit we fail the resolve (it is
always set in the summary cache, because summary update converts
missing xa.metadata to "", so we either get that, or cache miss which
leads to resolving from the commit.
This means that op->resolved_metadata is always set during install and
updates, which means we will show the app permissions. The transaction
will also always make sure that this data actually matches what gets
deployed.
Before this change an invalid metadata in the summary cache could lead
to a NULL resolved_metadata, which means we wouldn't print the app
permissions, yet we would still deploy some metadata file that could
have permissions. (NOTE: It would fail to deploy unless the
xa.metadata in the commit matched the metadata file, but in this
corner case we would't compare the summary and commit metadata, so
they may differ.)
(cherry picked from commit 5036bca4214d5b77e884dec42e36496a06e74081)
In particular, if a null terminator is placed inside the metadata file,
Flatpak will only compare the text *before* it to the value of
xa.metadata, but the full file will be parsed when permissions are set
at runtime. This means that any app can include a null terminator in its
permissions metadata, and Flatpak will only show the user the
permissions *preceding* the terminator during install, but the
permissions *after* the terminator are applied at runtime.
Fixes GHSA-qpjc-vq3c-572j / CVE-2021-43860
Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
(cherry picked from commit c9c3a667c09a846c0b230cf1cc8ed330028aa03c)
This was incorrectly looking at errno instead of -r.
Fixes: 0b38b0f0 "run: Handle unknown syscalls as intended"
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 3fc8c67267)
(cherry picked from commit 97e128c2c1)
The error-handling here was
if (r < 0 && r == -EFAULT)
but Alex says it was almost certainly intended to be
if (r < 0 && r != -EFAULT)
so that syscalls not known to libseccomp are not a fatal error.
Instead of literally making that change, emit a debug message on -EFAULT
so we can see what is going on.
This temporarily weakens our defence against CVE-2021-41133
(GHSA-67h7-w3jq-vh4q) in order to avoid regressions: if the installed
version of libseccomp does not know about the recently-added syscalls,
but the kernel does, then we will not prevent non-native executables
from using those syscalls.
Resolves: https://github.com/flatpak/flatpak/issues/4458
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d419fa6703)
If we don't allow pivot_root() then there seems no reason why we should
allow chroot().
Partially fixes GHSA-67h7-w3jq-vh4q.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we don't allow mounting filesystems, we shouldn't allow unmounting
either.
Partially fixes GHSA-67h7-w3jq-vh4q.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we don't allow unshare() or clone() with CLONE_NEWUSER, we also
shouldn't allow joining an existing (but different) namespace.
Partially fixes GHSA-67h7-w3jq-vh4q.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we don't allow mount() then we shouldn't allow these either.
Partially fixes GHSA-67h7-w3jq-vh4q.
Thanks: an anonymous reporter
Signed-off-by: Simon McVittie <smcv@collabora.com>
clone3() can be used to implement clone() with CLONE_NEWUSER, allowing
a sandboxed process to get CAP_SYS_ADMIN in a new namespace and
manipulate its root directory. We need to block this so that AF_UNIX-based
socket servers (X11, Wayland, etc.) can rely on
/proc/PID/root/.flatpak-info existing for all Flatpak-sandboxed apps.
Partially fixes GHSA-67h7-w3jq-vh4q.
Thanks: an anonymous reporter
Signed-off-by: Simon McVittie <smcv@collabora.com>
Historically, syscalls could take arbitrarily-different values on
different architectures, but new syscalls are added with syscall numbers
that align on each architecture.
Signed-off-by: Simon McVittie <smcv@collabora.com>
At the moment, if we block a syscall we always make it fail with EPERM,
but this is risky: user-space libraries can start to use new replacements
for old syscalls at any time, and will often treat EPERM as a fatal error.
For new syscalls, we should make the syscall fail with ENOSYS, which is
indistinguishable from running on an older kernel and will cause fallback
to an older implementation, for example clone3() to clone().
In future we should probably move from EPERM to ENOSYS for some of the
syscalls we already block, but for now keep the status quo.
This is a prerequisite for fixing the vulnerability tracked as
GHSA-67h7-w3jq-vh4q.
Signed-off-by: Simon McVittie <smcv@collabora.com>
These checks were broken in commit d762a2f, as the commit failed to
consider the fact that `flatpak_dir_check_parental_controls()` is run
both in the `flatpak` CLI process run by the user, but also in the
`flatpak-system-helper` process which always runs as root, and which
handles any installations done on the system repository.
As a result, parental controls were not working for the system
repository.
Fix that by limiting the scope of the check to only pass if running
without the system helper. flatpak calls from root never go through the
system helper.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: #4418
For reasons unknown, libarchive appears to generate broken gnutar format
tar archives when the archive contains files that are larger than 2 GB.
This commit switches to the pax format to work this around.
This should be a better default as it also removes 256 char filename
length limitation and matches what other libraries are doing, e.g.
Python 3.8 switched to the pax format by default as well.
See https://pagure.io/fedora-infrastructure/issue/9840
(cherry picked from commit b90d638601)
We use `bwrap --setenv XDG_RUNTIME_DIR` to set it to `/run/user/UID`,
regardless of what it is on the host system, but the changes made
to resolve CVE-2021-21261 unintentionally broke this by overwriting it
with the user's XDG_RUNTIME_DIR.
In practice this worked for most people, who either have
XDG_RUNTIME_DIR set to the same value we use (which is the conventional
setup from systemd-logind and elogind), or entirely unset (if they do not
have systemd-logind or elogind). However, it broke Wayland and other
XDG_RUNTIME_DIR-based protocols for people who intentionally set up an
XDG_RUNTIME_DIR that is different.
Fixes: 6d1773d2 "run: Convert all environment variables into bwrap arguments"
Resolves: https://github.com/flatpak/flatpak/issues/4372
Signed-off-by: Simon McVittie <smcv@collabora.com>
This is either a malicious/compromised app trying to do an attack, or
a mistake that will break handling of %f, %u and so on. Either way,
if we refuse to export the .desktop file, resulting in installation
failing, then it makes the rejection more obvious than quietly
removing the magic tokens.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 46b3ede524)
If we add new features analogous to file forwarding later, we might
find that we need a different magic token. Let's reserve the whole
@@* namespace so we can call it @@something-else.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 1e7e8fdb24)
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.
(cherry picked from commit e67847e253)
OpenSUSE inherits the hostname value from DHCP without updating its X11
authentication cookie, and it keeps the initial value in
`XAUTHLOCALHOSTNAME`.
To avoid breaking the X11 applications, OpenSUSE patches libxcb so that
it also considers the value in `XAUTHLOCALHOSTNAME` as another possible
hostname.
https://bugzilla.opensuse.org/show_bug.cgi?id=262309
To cope with that behavior we need to check `XAUTHLOCALHOSTNAME` too
and, if we have a cookie with that address, propagate it inside the
container adjusting its address to the canonical hostname
`unames.nodename`.
Fixes: #4043
Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com>
(cherry picked from commit fd6a3b79f4)
FLATPAK_ERROR_ALREADY_INSTALLED can be returned from, for example,
flatpak_installation_add_remote(), so document that it applies to
remotes not just apps and runtimes.
This means we can systematically pass the environment variables
through bwrap(1), even if it is setuid and thus is filtering out
security-sensitive environment variables. bwrap ends up being
run with an empty environment instead.
This did not regress in 6d1773d "run: Convert all environment variables
into bwrap arguments", because the LD_LIBRARY_PATH special case in
flatpak_run_add_environment_args() was already not used here; but it's
a bug fix along the same lines as fixing the regression.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This means we can systematically pass the environment variables
through bwrap(1), even if it is setuid and thus is filtering out
security-sensitive environment variables. bwrap ends up being
run with an empty environment instead.
As with the previous commit, this regressed while fixing CVE-2021-21261.
Fixes: 6d1773d2 "run: Convert all environment variables into bwrap arguments"
Signed-off-by: Simon McVittie <smcv@collabora.com>
This adds `NULL` protection to the `subpaths` argument to
`flatpak_dir_system_helper_call_deploy()`, just like already exists for
other arguments.
It’s a second layer of defence against bugs like fixed in the previous
commit.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
If an app had no deploy data, and `flatpak_dir_update()` was called with
`opt_subpaths == NULL`, then `arg_subpaths` passed to
`flatpak_dir_system_helper_call_deploy()` would be `NULL`, which causes
an assertion failure from an internal `g_variant_new()` call:
```
g_variant_new_strv: assertion 'length == 0 || strv != NULL' failed
```
Fix that by setting the `subpaths` to an empty array if nothing else is
specified.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
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>
Some consumers of environment variables distinguish between present
with an empty value and absent. For example, if an environment variable
represents a search path like VK_ICD_FILENAMES, unsetting it often
results in use of a default, but setting it to the empty string results
in not searching any locations, which is sometimes what is desired.
The shell syntax "${BAR-unset}" expands to the value of ${BAR} if it
is set to anything (even an empty string), or to "unset" if not.
We can use that in the unit test to check that BAR is set to the
empty string in this case.
This follows up from GHSA-4ppf-fxf6-vxg2 to fix an issue that I noticed
while resolving that vulnerability, but is not required for fixing the
vulnerability.
Signed-off-by: Simon McVittie <smcv@collabora.com>
This avoids some of them being filtered out by a setuid bwrap. It also
means that if they came from an untrusted source, they cannot be used
to inject arbitrary code into a non-setuid bwrap via mechanisms like
LD_PRELOAD.
Because they get bundled into a memfd or temporary file, they do not
actually appear in argv, ensuring that they remain inaccessible to
processes running under a different uid (which is important if their
values are tokens or other secrets).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
On systems with no XDG_RUNTIME_DIR, PulseAudio falls back to creating
a temporary directory in /tmp, then creating a symlink in
legacy directory ~/.pulse or its more modern replacement ~/.config/pulse.
The symlink is named according to the machine ID, falling back to the
hostname.
Resolves: #4058
Might-resolve: #2169
Signed-off-by: Simon McVittie <smcv@collabora.com>
As with flatpak run --parent-expose-pids, this will only work if we have
a working, non-setuid bwrap. Systems where user namespace creation is
restricted and bwrap needs to be setuid (Debian 10, RHEL/CentOS 7,
Arch Linux linux-hardened kernel) will have degraded functionality.
This option is similar to --expose-pids, except that instead of making
the subsandbox use a nested pid namespace inside the parent's, it makes
the subsandbox share the parent's pid namespace as-is, so that process
IDs in the parent and the subsandbox are interchangeable. This will
be useful if the parent and the subsandbox communicate via protocols
that assume a global view of the process ID namespace, for example
passing process IDs across an AF_UNIX socket or in shared memory.
In particular, this will be useful for Steam's pressure-vessel container
tool: the IPC between the Steam client and the "game overlay" loaded into
Steam games uses process IDs, and becomes confused if they don't match up.
This weakens the security boundary between a subsandbox and the parent,
but that's OK in some cases, especially if the subsandbox is being used
as a way to get a different runtime /usr (flatpak-spawn --latest-version
or #4018) rather than as a security boundary.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If network access is allowed, then we should probably allow name
resolution too.
This should be enough to make nss-resolve work inside flatpak. However,
it cannot be tested with GNOME runtimes, because GNOME runtimes do not
contain systemd. It also cannot be tested with the Fedora 33 flatpak
runtime, because this runtime contains systemd 246, where nss-resolve
uses D-Bus rather than varlink to communicate with systemd-resolved. And
there is no rawhide runtime, and will be no Fedora 34 runtime until F34
is branched. So currently it's not possible to actually test this
without building a custom runtime, which I have not attempted to do. I
have built flatpak myself and verified the resolved socket is mounted
properly inside the sandbox, but it would be better to test if it
actually works with a runtime that contains systemd 247.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1912131
This brings us closer to matching what happens in libpulse.so.0.
This path is the default for a system-wide PulseAudio instance, which
is discouraged on non-embedded systems but is easy to check for.
In particular, SteamOS, an "appliance"-style gaming OS, uses this
socket.
Adapted from Steam's pressure-vessel container tool, which reuses
various Flatpak modules, including this function.
Signed-off-by: Simon McVittie <smcv@collabora.com>
In addition to the old summary.idx.sig we now create an identical file
in `summaries/$DIGEST.idx.sig`, where the digest is of the summary
index itself. This means it will have a unique name and there will
never be issues where downloading the index and signature will race
during an update.
We keep the new and the previous digested sigs every update to avoid
removing the previous one while its in use.
As suggested by simon in https://github.com/flatpak/flatpak/issues/3983
and https://github.com/ostreedev/ostree/issues/2250
Currently if a deploy file was created (because a flatpak is installed)
using an old Flatpak version (such as 0.10.1 that's in Endless 3.3.16),
it will not include every metadata key, such as "eol" and "eolr". Those
keys were added before deploy version 1. Then when the deploy file is
upgraded to version 3, the metadata is only copied so eol/eolr are still
missing even though they exist in the commit metadata.
So this commit adds deploy version 4 which ensures all metadata keys in
the deploy file are present (when they are present in the source data).
All existing deploy files will be upgraded with this code.
This change is important because we use the end-of-life information to
decide whether an unused runtime should be uninstalled, so without this
change we are not freeing up disk space that we should be.
In elementary OS, we're building our images with some flatpaks pre-installed. To do this, we run noninteractive `flatpak install` commands in the chroot as part of the build scripts. We've bumped into some build failures recently after switching to a version of flatpak with malcontent support built in.
After some debugging, it turns out installations are failing the parental controls checks with errors like `Failed to install org.gnome.Epiphany: Could not connect: No such file or directory` message. I'm assuming this is a failure to connect to the system bus in this method (since there isn't one in the chroot).
Is it reasonable to assume that UID 0 should be allowed to install whatever they want regardless of parental controls? I believe this would fix our issue too.
