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>
(cherry picked from commit 5709f1aaed)
We weren't distinguishing here between overrides that should have been
negated (xdg-documents) and overrides that should not have been negated
(everything else).
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 4e3d1d8b7b)
This tests for invalid metadata, missing xa.metadata and mismatched
values in xa.metadata and the real metadata, including the embedded
null leading to the hidden permissions of CVE-2021-43860.
(cherry picked from commit 2c2ce58c54b3e6c62f9c21c15efa0ba22f4bc09b)
On two different code paths we were using the "Title" field in
flatpakref files as the title of a remote, which is incorrect. In most
cases, the remote added via the RuntimeRepo key will be the same as the
remote the app is from, so when the remote is added for the runtime, its
title will be correctly set using the Title value from the flatpakrepo
file and the app will therefore have an origin remote with a title set.
This is not currently true for flatpakref files that use
SuggestRemoteName=, see https://github.com/flatpak/flatpak/pull/4513
For flatpakref files that use a different remote than the RuntimeRepo,
we don't currently have a way for the title to be set automatically;
perhaps we should (https://github.com/flatpak/flatpak/issues/4512).
Fixes https://github.com/flatpak/flatpak/issues/4499
(cherry picked from commit 9dbd265cdd)
It can happen that a related ref is installed from a different remote
than the thing it's related to. We always want to update things from
their origin remote. However as of now FlatpakTransaction resolves the
commit of a related ref to the one available from the main ref origin,
and later sets the remote for the operation to the installed origin (see
commit 6793d90b8). In case there is a newer commit in the main ref
origin than the installed origin, this leads to an update operation
being erroneously created, only to then error out with an HTTP 404
error, because the commit from the main ref origin is being pulled from
the installed ref origin. For specific steps to reproduce see
https://github.com/flatpak/flatpak/issues/3128#issuecomment-948948040
So, ensure that when a FLATPAK_TRANSACTION_OPERATION_INSTALL_OR_UPDATE
operation is created for something that's installed, whether it's a
related ref or something else, the remote used is always the origin. And
ensure that the remote is set correctly before the stage where the op is
resolved to a commit, to avoid the situation described above. This is
essentially a re-implementation of the fix in commit 6793d90b8.
Also, add a unit test for this behavior.
This commit also makes a few changes to documentation to make it clear
that this related-ref-different-origin situation is possible.
Fixes#3128
(cherry picked from commit 49d9052d22)
(only merge conflicts in tests/testlibrary.c)
Currently the xa.noenumerate option on a remote is documented as causing
the remote not to be used when presenting available apps/runtimes or
when searching for dependencies. The idea is that the remote is only
used for providing updates for things installed from it, and this
functionality is used when creating an origin remote for something
installed via a flatpakref file.
However, the implementation of this in flatpak_dir_list_remote_refs() is
buggy. It returns an empty set of refs even if something is both locally
installed and available from the remote. This is because it is using
hash table comparisons of FlatpkDecomposed objects (via
flatpak_decomposed_hash()) which take into account both the ref (or
refspec) and the collection ID, and the local refs' FlatpakDecomposed
objects are created from a refspec whereas the remote refs'
FlatpakDecomposed objects are created from a ref alone. We could fix
this by having them both use refspecs, but it is better to use a
collection-ref tuple for the following reasons:
(1) Changing flatpak_dir_list_all_remote_refs() to use a refspec to
create the FlatpakDecomposed objects would be a breaking change for the
other users of that function.
(2) Both the local and remote refs are from the same remote so we don't
need to use the remote name to disambiguate them, even if no collection
ID is configured.
(3) The whole point of collection IDs is to make refs uniquely
identifiable, so we're using them for the intended purpose.
In addition to fixing this bug, this commit adds a unit test in
testlibrary.c so it stays fixed.
(cherry picked from commit 7f3556d92c)
Currently the output produced when assert_remote_in_installation() or
assert_remote_not_in_installation() fails is not helpful: it doesn't
mention the remote name or where the assertion was called from. Fix
those problems by rewriting it as a #define.
(cherry picked from commit dd48e78652)
This fixes a few issues with the unit test logs that make them ugly:
1. Currently some lines of output from a command will appear after the
line from xtrace which has the next command, since the command was
printing to stdout and xtrace uses stderr. E.g. "Installation complete."
will appear after "+ flatpak --user install -y ..." but it is from the
previous install command.
2. Lines of output have many spaces after them to pad them to the table
width but this is not needed for non-fancy output.
3. Lines of output are mixed with output from httpd since they don't end
with a newline character, e.g. "Installing… ▊
4%127.0.0.1 - - [16/Nov/2021 00:18:24] "GET /..."
(cherry picked from commit 388c23cfc5)
In particular, this checks that CVE-2017-5226, CVE-2019-10063 and
CVE-2021-41133 are still prevented.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 7c5aec474c)
This exercises various syscalls. It's heavily based on the one from
<https://github.com/containers/bubblewrap/pull/459>, but with the
addition of a mode to output the numeric values of various expected
errno codes, which are not otherwise available to shell scripts.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 4ce251882c)
Files that are generated manually by a maintainer and committed to git
should be in the srcdir, not the builddir.
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 647c51380c)
a99b748931 introduced an environment variable for changing the flatpak path for exports.
On NixOS, we do not have a stable path we could use so we used to patch Flatpak to use `flatpak` program from `PATH`.
With the recent change, we drop our downstream patch in favour of setting the environment variable to `flatpak` but the tests do not expect that.
This is the test part of our former downstream patch so that tests can pass with exports using `flatpak` instead of an absolute path.
It still expects the binary path to end with `flatpak` so arbitrary `$FLATPAK_BINARY` will not work but we do not pass the environment variable to tests so we cannot do much better.
(cherry picked from commit 96dbe28cfa)
g_key_file_get_boolean() only accepts lower case "true" and "false", so
correct some instances of "False" in key files. This makes no functional
difference as long as the error pointer isn't checked since
g_key_file_get_boolean() also returns false when it can't parse the
value as a boolean. But it still seems good to be correct, and someone
could interpret the existence of "IsRuntime=False" as implying that
"IsRuntime=True" will work, which it doesn't.
(cherry picked from commit b098191252)
In Yocto systems /var/tmp is a symlink to /var/volatile/tmp
because in its implementation of read-only rootfs /var is read-only
so /var/volatile is mounted as a tmpfs
and a subset of the paths point into it.
This would result in flatpak emitting mount arguments of
`--symlink ../var/volatile/tmp /var/tmp --bind /var/volatile/tmp /var/volatile/tmp`
which fails because flatpak has already added `--dir /var/tmp`
and the call to symlink fails with EEXIST.
This is fixed by blacklisting /var/tmp from symlink exports
in the same way /tmp is, so the bind is emitted as
`--bind /var/tmp /var/tmp`, which results in /var/volatile/tmp
being mounted into /var/tmp.
If we do, it interferes with xdg-dbus-proxy, causing test failure under
some circumstances: the test passes on a development system, but fails
when run on a qemu virtual machine in Debian's autopkgtest framework.
Fixes: 6e5b02e2 "run: Don't let XDG_RUNTIME_DIR from user override the value we set"
Signed-off-by: Simon McVittie <smcv@collabora.com>
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>
If /tmp is a symlink, we mount the target directory on /tmp instead of
replicating the symlink, so that it will not interfere with "--dir /tmp".
Signed-off-by: Simon McVittie <smcv@collabora.com>
In particular this tests commit 3aaea7d2 "Expose /var/usrlocal if
"--filesystem=host" is specified" and checks that /var/usrlocal is
counted as part of --filesystem=host-usr.
Signed-off-by: Simon McVittie <smcv@collabora.com>
To be excluded from tarball releases, generated files need to be in
nodist_ lists of sources every time they appear.
Fixes: 412c1577 "portal: Add some test coverage"
Signed-off-by: Simon McVittie <smcv@collabora.com>
If we use x86_64 and i386 in places where any architecture would do,
it's harder to recognise whether the test is assuming every machine
is x86 and would fail elsewhere (e.g. as fixed in #4142). When just
parsing abstract strings where any syntactically valid architecture
would do, let's use rare/niche architectures that none of the maintainers
are running on, so that hard-coding x86_64 or i386 stands out more.
I've used mips64 and m68k because they happen to be the same length as
x86_64 and i386, avoiding re-indentation.
Signed-off-by: Simon McVittie <smcv@collabora.com>
If the tests are run as root, the system helper is not used even when
commands are run with `--system`. This means a different cache path is
used (see `_flatpak_dir_ensure_repo()`). The tests did not accommodate
for that.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
scan-build points out that bytes isn't read after it is assigned. While
this is not actually true (scan-build doesn't understand
__attribute__((__cleanup__)), which frees bytes), it's true that we
should ideally have an assertion here.
Signed-off-by: Simon McVittie <smcv@collabora.com>
scan-build detected that res was written but never read. Presumably
the use of ref here (carried over from the previous test) is a
copy/paste error.
Signed-off-by: Simon McVittie <smcv@collabora.com>
scan-build has a lot of false positives for this codebase because it
doesn't understand __attribute__((__cleanup__)) or GLib's GError
convention, but it seems to have been right about these.
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>
