Historically the p2p resolve code always did a parallel call to find
all the available commits for the refs, and then it took the results
and pulled only the commits for all the refs so that it could resolve
against the exact commits that were available (which might not match
with whatever metadata we have in the local ostree-metadata copy.
This splits this into two phases, the first that uses the summary only,
and a second one that pulls the commit.
The reason for this is that we want to be able to do some stuff inbetween
these, such as resolving some refs via the ostree-metadata and maybe
requesting bearer tokens that we need for pulling the commit objects.
These are explicitly made short to save space, so lets have defines
for them to make sure we don't mistype them, especially as we
will be adding new keys.
These were added so that extra-data would work in #2954, however that
was a bit broad. We only need extension deps for extensions that:
1) Uses extra data (so we can run apply-extra)
2) Doesn't specify NoRuntime=true (because the apply-extra is static)
Fixes https://github.com/flatpak/flatpak/issues/3173
We were using i instead of handle to index the file descriptors, which
is likely to be the same (due to ordering) but not really
correct. Also rename the variables to make this code easier to read.
Also, add some bounds check on the handles wrt the fd list.
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.
This allows you to open up things (if the calling app has access) for
the sandboxed child.
Rather than duplicating all possible sandboxing technical details we
specify things at a higher level. We just assume you want the same
access as the caller (i.e. x11[-fallback] and/or wayland), as this is
easier to use for the caller and more flexible for us to later add new
technical details as needed.
Mount ~/.local/share/icons at /run/host/user-share/icons in
the sandbox, so runtimes can add /run/host/user-share to
their XDG_DATA_DIRS and get access to locally-installed
icon themes.
Mark installed apps to be updatable if an installed in missing
its runtime for some reasons. In pre-FlatpakTransaction world,
an app migration from runtime X to runtime Y would have rendered
an app unusable because runtime Y would not be installed
automatically by clients like gnome-software.
The goal here is that clients like gnome-software can show
those installed apps as updatable again, if such a situation
arises. The FlatpakTransaction API can automatically resolve one of
its ops to install the new runtime, provided we can mark the app
as updatable again at the first place.
While updating, if the related extension is missing on
the installation of an installed ref (could be an app or
runtime), FlatpakTransaction tends to "repair" the ref by
automatically downloading the related extension again and
restoring the overall functionality of the ref.
The related extension concerned that are the ones associated with
`should-download` to TRUE only.
Hence, teach the libflatpak API to do that same, so that clients
like gnome-software can mark those refs as updatable, if their
related extensions is missing.
FlatpakTransaction will automatically download the related ref
(.Locale ref in this case). In case of mentioning previous-ids
in the deploy file, the related ref(.Locale) should be also
be requiring an update.
Hence, flatpak_installation_list_installed_refs_for_update now
will return 2 refs (app and related .Locale extension) to be
updated, instead of 1.
The previous code checked whether the progress object had
"outstanding-fetches" set in order to decide whether it had been
initialized enough to show progress information. However, if the
callback saw a progress object on which flatpak_dir_setup_extra_data()
had not yet been called, then it would crash.
Therefore it seems that we should additionally be checking for the
presence of "outstanding-extra-data" which is set in
flatpak_dir_setup_extra_data().
It's likely that this wasn't previously a problem because the callback
would never get called due to the progress object's associated main
context not being iterated. It crashes now because that problem was
fixed in a previous commit.
Previously, in flatpak_dir_setup_extra_data(), n_extra_data (a gsize
which is 8 bytes wide on x86_64) was passed in a varargs list where an
unsigned int (4 bytes wide) was expected due to the "u" variant type
specifier.
This doesn't seem to have directly caused any crashes for me, but it's
undefined behaviour.
Therefore, this changes the affected keys "outstanding-extra-data" and
"total-extra-data" to be guint64 types instead of unsigned ints. The
gsize returned from g_variant_n_children() is cast to guint64 by virtue
of being assigned to a guint64-typed variable, but should not lose any
bits on supported platforms.
It's a common idiom in this codebase to push a temporary GMainContext as
the thread default context in order to run an async operation as if it
were sync. If we are not expecting progress callbacks this isn't a
problem, but it becomes a problem if we pass in an OstreeAsyncProgress
object that was created under a different GMainContext. The reason for
this is that OstreeAsyncProgress creates an idle source and attaches it
to the thread default context, so if we are iterating a temporary
context then the OstreeAsyncProgress's context never gets iterated, and
so no progress signals are fired.
To fix this, we introduce flatpak_progress_chain() and a RAII helper
FlatpakAsyncProgressChained which creates a new OstreeAsyncProgress
under the temporary GMainContext, but forwards all its state and updates
to the previous OstreeAsyncProgress's callbacks.
This is documented in a comment in the code as well.
All known instances of this problem in the existing code are fixed in
this commit.
This uses new API in libostree which is proposed in
ostreedev/ostree#1968. In anticipation of it being included in libostree
version 2019.6, the bug fix is predicated on that version being present.
If compiling against an older version, the old buggy behaviour will be
the fallback.
This problem was solved conceptually by Philip Withnall, I only wrote
the code.
There is no need to read the links, just look at the inode nr
which is the same info, and that also works on the bind-mounted
.userns thing where readlink fails.
Also, don't fail for non-existing namespaces.
Assuming unprivileged namespaces works we can now user the .userns
bindmount to access the intermediate bubblewrap user namespace.
This also drops the warning about root, and make sure we drop all caps
at the end.
Neither of these ever need a polkit agent, and run/build are somewhat
performance sensitive and we don't want to connect to dbus unnecessarily.
For enter this is critical though, as the dbus connection starts a thread
which is not compatible with the setns syscall.
There is a kernel issue which has been fixed in linux 4.9:
e98d413703
Which makes it impossible (on older kernels) to mount devpts unless
uid 0 is mapped in the user namespace. Bubblewrap works around this
by using two namespaces, the base one which sets up everything (and
thus owns all the other namespaces), and then at the end a child of that
that remaps uid 0 to the real uid.
Unfortunately, this makes it impossible to enter the bubblewrap user
namespace, because there are no references to the intermediate
user namespace we can use. To work around this we make a bind mount
of the intermediate namespace during setup using --ro-bind-try which
we can use for nsenter.
Due to bug #3215 some systems have refs in refs/mirrors/ in addition to
the usual refs/remotes/ location. The remote refs are always at least as
new as the mirror ones since the repo_pull() invocation in
flatpak_dir_pull() which does not use OSTREE_PULL_FLAGS_MIRROR happened
after the one that did. Cleaning up these mirror refs is important since
otherwise when the remote ref is either updated or removed (by an
uninstall) disk space will be leaked since the mirror ref will point to
a no longer needed commit.
So, remove (almost) all mirror refs during flatpak repair, uninstall,
or update operations. And for the uninstall and update operations do it
in FlatpakDir so that it happens regardless of if the CLI of libflatpak
are used.
Also, add a unit test for this.
Fixes https://github.com/flatpak/flatpak/issues/3222
This reverts commit 915ad583a7.
This commit turned out to have unintended side effects. Specifically,
with it we do a pull with OSTREE_REPO_PULL_FLAGS_MIRROR, and then
flatpak_dir_setup_extra_data() does a non-mirror pull in the same
transaction, so the ref being pulled ends up being written to disk under
both refs/remotes/ and refs/mirrors/ in
ostree_repo_commit_transaction(). This is a problem because only the
remote ref is deleted during an uninstall, so the disk space is leaked,
and we don't have the infrastructure in place to keep both refs up to
date as they're updated.
It would be nice to consistently use OSTREE_REPO_PULL_FLAGS_MIRROR for
all pulls but that turns out to be a deep rabbit hole to go down; see
the discussion in https://github.com/flatpak/flatpak/pull/3220
So revert the commit instead (with a few exceptions: keep a
still-relevant FIXME comment, keep an assertion in the "out:"
section, and keep a debug statement printing out the resolved rev).
Note that this means that since we're no longer checking commit
signatures during ref resolution, in theory remote B could try to set
the same collection ID as remote A and serve a malicious update for
something from remote A, but the signature would be found to be invalid
during the pull phase due to our use of "ref-keyring-map" so the
transaction would fail.
All the other uses of OSTREE_REPO_PULL_FLAGS_MIRROR across the codebase
should be kept I think:
- flatpak create-usb uses it when pulling into the repo on the USB which
works perfectly well with refs/mirrors/ (and the USB is mirroring the
collection-refs!)
- it's used when pulling into a temporary "child" repo in a few places
and there it makes sense since the child repo is mirroring the refs so
they can be pulled into the main repo. In fact, in the case of
flatpak_dir_do_resolve_p2p_refs(), we need MIRROR since otherwise
ostree_repo_resolve_collection_ref() gives us the commit on-disk
rather than the just-pulled one that's in memory.
This test was intended to verify that updates from remote B can't
interfere with updates from remote A even if remote B maliciously sets
the same collection ID as remote A. However, the commits intended to
protect against this turned out to have nasty side effects and need to
be reverted.[1] A subsequent commit will revert the use of
OSTREE_REPO_PULL_FLAGS_MIRROR which means this attack is not exploitable
(since refs will be resolved using a refspec which includes the remote
name), at the cost of not supporting more than one remote having the
same collection ID configured. Since we don't support that, it doesn't
make sense to keep this unit test.
Also, the test seems to be failing.
[1] https://github.com/flatpak/flatpak/issues/3215
After an unprivileged client calls GetRevokefsFd(), the `revokefs-fuse
--backend` process busyloops as follows:
poll([{fd=3, events=POLLIN}, {fd=4, events=POLLIN}], 2, -1) = 1 ([{fd=4, revents=POLLIN}])
Here is the command line for this process:
revokefs-fuse --backend --socket=3 --exit-with-fd=4 /var/lib/flatpak/repo/tmp/flatpak-cache-JBUHB0
The intention here is to poll() until fd 3 is readable (at which
point the writer process serves a client request and writes back a
response, synchronously) or fd 4 encounters an error. fd 4 is meant to
be one side of a pipe that the system helper holds the other end of;
when the pipe is broken, the system helper must have gone away, and the
`revokefs-fuse --backend` process treats this as a signal to exit.
However, fd 4 is not a pipe. In fact, it is the dirfd for the target directory:
root@camille:/var/roothome# ls -l /proc/31717/fd
total 0
lr-x------ 1 wjt wjt 64 Nov 19 21:21 0 -> /dev/null
lrwx------ 1 wjt wjt 64 Nov 19 21:21 1 -> /dev/pts/1
lrwx------ 1 wjt wjt 64 Nov 19 21:21 2 -> /dev/pts/1
lrwx------ 1 wjt wjt 64 Nov 19 21:21 3 -> 'socket:[2558007]'
lr-x------ 1 wjt wjt 64 Nov 19 21:21 4 -> /var/lib/flatpak/repo/tmp/flatpak-cache-JBUHB0
This is because revokefs_fuse_backend_child_setup() erroneously closes
fd 4 before the `revokefs-fuse --backend` process is exec()d. This
regressed in d91660fe2a.
Fix this by only closing fds 5 and above. With this change, we see the
expected set of open file descriptors:
root@camille:/var/roothome# ls -l /proc/32493/fd
total 0
lr-x------ 1 wjt wjt 64 Nov 19 21:24 0 -> /dev/null
lrwx------ 1 wjt wjt 64 Nov 19 21:24 1 -> /dev/pts/1
lrwx------ 1 wjt wjt 64 Nov 19 21:24 2 -> /dev/pts/1
lrwx------ 1 wjt wjt 64 Nov 19 21:24 3 -> 'socket:[2552594]'
lr-x------ 1 wjt wjt 64 Nov 19 21:24 4 -> 'pipe:[2552596]'
lr-x------ 1 wjt wjt 64 Nov 19 21:24 5 -> /var/lib/flatpak/repo/tmp/flatpak-cache-JBUHB0
Fixes#2882.
In commit 0772ab6c9 we changed "in preference of" to "in favor of" in an
informational message produced by FlatpakCliTransaction for clarity. Do
the same for FlatpakQuietTransaction.
If content_rating == NULL, then no value will be assigned to
appdata_value, but its value will be used anyway – if it happens to be
non-NULL, it will be dereferenced.
common/flatpak-parental-controls.c: In function ‘flatpak_oars_check_rating’:
common/flatpak-parental-controls.c:121:10: warning: ‘appdata_value’ may be used uninitialized in this function [-Wmaybe-uninitialized]
if (appdata_value != NULL)
^
