When we export a manifest to the index, always pass the ref we're
targeting instead of relying on the org.opencontainers.image.ref.name
annotation, because that may not be set if we're using labels instead.
This is no big deal, because we know what ref we're handling anyway.
We now pull the image config as well as the manifest and fall
back on the labels field if the keys we're looking for are not
in the annotations field.
This lets us support docker manifests too, which don't have
annotations (but do have labels).
This prevents a crash in flatpak_installation_list_remotes_by_type() if
the `FlatpakDir` can’t ensure it has a repo configured.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #3028
Approved by: alexlarsson
The org.freedesktop.Flatpak user D-Bus service isn't just used by
flatpak(1) or applications running as Flatpaks. It's also used by
toolbox(1) for similar reasons:
* To keep various configuration files inside the container
synchronized with the host
* To let the container request certain commands to be run on the host
The org.freedesktop.Flatpak D-Bus service itself doesn't need much in
the way of dependencies, but inherits a lot of unused shared library
linkages through the libflatpak-common.la convenience library. Removing
these unused shared libraries reduces the footprint of toolbox(1) for
those who care about such things. eg., Fedora CoreOS.
This commit brings down the number of shared libraries to 19 from 62.
Closes: #3052
Approved by: alexlarsson
... by moving the definition of FlatpakHostCommandFlags from the
'common' sub-directory to 'session-helper'. It hasn't been used by
anything else ever since flatpak-builder was moved to a separate
repository in commit 52bd146561.
Closes: #3052
Approved by: alexlarsson
The libxml API is used in a relatively few places inside the 'common'
sub-directory. It's definitely not as widespread as GLib. A subsequent
commit will leverage this to prevent unused shared libraries from
being linked to the session helper.
Closes: #3052
Approved by: alexlarsson
The libarchive API is used in a relatively few places inside the
'common' sub-directory. It's definitely not as widespread as GLib. A
subsequent commit will leverage this to prevent unused shared
libraries from being linked to the session helper.
Closes: #3052
Approved by: alexlarsson
If the download URL for an icon was already cached locally, the HTTP
code returned FLATPAK_OCI_ERROR_NOT_CHANGED - this was treated as a real
error, and after downloading missing icons, all present icons were
deleted!
See https://bugzilla.redhat.com/show_bug.cgi?id=1683375Closes: #3048
Approved by: alexlarsson
It's an error to call g_set_error() on an error that's already set, and
flatpak_dir_get_deploy_data() already sets FLATPAK_ERROR_NOT_INSTALLED
for us when necessary, so just pass on any errors to the caller of
flatpak_dir_get_origin(). In the case that the error is something else,
that should be treated as an error, because we never expect the deploy
directory to exist but not have a deploy file (see the implementations
of flatpak_dir_deploy() and flatpak_dir_undeploy()).
Closes: #3032
Approved by: alexlarsson
The docs for flatpak_is_valid_branch() say branch names can't start with
a digit but the implementation doesn't enforce this, and we have lots of
branches out in the wild that start with a digit (e.g. "3.32" and
"5.12"). So make the docs imply that branches can start with a digit.
The implementation also disallows "." as the leading character for a
branch, so add that to the docs. I'm just guessing that "." was intended
to be disallowed but it makes sense; otherwise the file we create named
with the branch would be hidden.
Closes: #3023
Approved by: alexlarsson
This restores the ABI to the pre-1.4.0 version. This moves the new signal to
the *end* of the struct and also correctly decrements the padding.
Fixes https://github.com/flatpak/flatpak/issues/2957, although we probably need
a 1.4.1 release with this included pretty quickly to avoid chaos.
Closes: #2958
Approved by: alexlarsson
I spotted this line in the output from `flatpak history`:
Jun 4 16:17:20 deploy install com.discordapp.Discord x86_64 stable system flathub 8a0fc700c701 Installed %s from %s root (test) flatpak-system-helper (gnome-software) 1.3.3
This is because the format string is passed as the 'url' parameter, the
first format parameter (the ref) is passed as the 'format' parameter,
and 'origin' is ignored because (fortunately) as far as I know, no
character significant to printf (like '%') is permitted in ref names.
Fix this by passing a NULL 'url', like the neighbouring call in
flatpak_dir_deploy_update().
This adds a new error to FlatpakError and uses it everywhere a ref is not
found, either locally or in a remote. This should hopefully be more useful than
the status quo of either returning FLATPAK_ERROR_INVALID_DATA or
G_IO_ERROR_NOT_FOUND or something else. Technically this is an API break but it
seems worth the risk. I checked gnome-software which does not seem affected by
this, and I checked eos-updater which does check for G_IO_ERROR_NOT_FOUND in
one place that will be affected by this but we can patch that.
Closes: #2895
Approved by: matthiasclasen
This is basically modify_remote, but it fails if the remote is
already configured instead of modifying it. Although it also
has a if_needed option, and if that is set it will silently complete
as a no-op (exept resetting filters).
Closes: #2888
Approved by: alexlarsson
We need to check whether the remote is gpg verified after handling
the oci case, because OCI is fine to update systemwide without gpg
verification (in fact it doesn't support verification).
This just reorders the code, matching what is done in the install
case already.
Closes: #2891
Approved by: alexlarsson
We still look at the regular file to pick up changes, but to avoid
issues if the file disappears (for example if it was in a package
that was uninstalled) then we still have something to show so we don't
start accidentally showing unfiltered stuff.
Closes: #2890
Approved by: alexlarsson
Rearrange a few things for the purposes of readability and adherence to
conventions around using GError. This introduces no functional changes.
Closes: #2705
Approved by: alexlarsson
ostree_repo_find_remotes_finish() can return an empty array of
OstreeRepoFinderResults without setting @error and expects the caller to
check for this before doing a pull. Currently in a few different places
Flatpak does not check if the array is empty after calling
ostree_repo_find_remotes_finish() which leads to a critical warning in
ostree_repo_pull_from_remotes_async() which expects a non-empty results
array. This is causing one of the unit tests to fail.
So properly handle the case of an empty results array in each place it
can occur. In most places it's an error condition so the error pointer
is set appropriately. In list_remotes_for_configured_remote() it's not
an error because there may legitimately be no LAN/USB remotes available,
so in that case just properly handle the case of (results == NULL).
Also, add a debug statement in flatpak_dir_do_resolve_p2p_refs() since
we're now building a string of the refs being resolved.
Closes: #2705
Approved by: alexlarsson
Currently flatpak_dir_pull() has a phase where it tries to resolve a ref
to a commit before doing the pull, which is good because it means we're
pulling the same commit even if we do multiple subpath pulls, and it
allows us to get set up for accurate progress reporting. On the P2P code
path, this resolution is accomplished with an
ostree_repo_find_remotes_async() call, and then checking the results
from that. Normally that works fine, but in case a remote tries to
maliciously serve an update to refs which didn't originate from it (by
setting the same collection ID as the victim remote) things break. The
find_remotes_async() will use the malicious remote's keyring for
verification and return that commit as the most recent. This causes
errors later during the pull phase.
For example, if we're trying to update example-ref from good-remote,
and good-remote is offering commit v1 and malicious-remote is offering
commit v2, we resolve example-ref to commit v2. Then pulling that commit
from malicious-remote using good-remote's keyring fails, and pulling
commit v2 from good-remote fails because it doesn't exist there.
So this commit changes flatpak_dir_pull() so that it pulls commit
metadata before deciding on a commit. Since the pull code uses the
"ref-keyring-map" option, the bad signatures will be found and the
latest good commit will be returned. This requires a few changes:
1) Move the ostree_repo_prepare_transaction() call up to before the new
pull, which also means using "goto out;" in a few more places.
2) Use OSTREE_REPO_PULL_FLAGS_MIRROR for the pull and
flatpak_repo_resolve_rev() after the pull. That is more correct but we
need the patch in this PR[1] for it to work so the commit signature
check is conditional on a check for ostree v2019.2.
3) Change repo_pull() so that it will accept results_to_fetch != NULL &&
rev_to_fetch == NULL. This means making a g_autofree version of
rev_to_fetch and resolving it after the pull if necessary.
This is all working toward the goal of getting the unit test in the
following commit, test-p2p-security.sh, to succeed.
[1] https://github.com/ostreedev/ostree/pull/1821Closes: #2705
Approved by: alexlarsson
With Flatpak you should only have to trust each remote to provide good
updates for the apps provided by it. However the P2P support in OSTree
considers each remote to be equally trustworthy, which opens a possible
attack vector. For example if I have a flathub remote configured and
apps installed from it and I also have a remote "sketchy-remote"
configured which I have one app installed from, I expect the Flathub
apps to update from Flathub (or to update from LAN/USB sources with
Flathub GPG signatures) and not from the sketchy-remote.
The way this attack would play out is that the sketchy-remote would
deploy the same collection ID as the victim remote (in this case
org.flathub.Stable) in order to serve updates for it. So this commit
mitigates the issue by using the new "ref-keyring-map" option
added to libostree[1], which means that pulls of updates to Flathub apps
will always be verified using the Flathub GPG keyring, even if they're
coming from another source like another configured remote or a LAN/USB
source signed with the malicious remote's keyring. In the latter case
the pull from the malicious source will fail, and flatpak should then do
a successful pull from a legitimate source.
We use the "ref-keyring-map" option in both
flatpak_dir_do_resolve_p2p_refs() and repo_pull() because if we only use
it in the latter place the ref could be resolved to the malicious commit
(which would be checked with the malicious keyring), and then in
repo_pull() we would try unsuccessfully to pull the malicious commit
from the legitimate remote.
Since pulls into the system installation already use the correct
remote's keyring (see the use of ostree_repo_verify_commit_for_remote()
in flatpak_dir_pull_untrusted_local()) this mitigation is only needed
for per-user installations (or other scenarios that circumvent the
system helper). It's also only needed since the commit "dir: Fix an edge
case of resolving collection-refs" because before that commit this
attack vector wasn't exploitable.
Unfortunately this implementation is not perfect, because there's not
always a one-to-one mapping between configured remotes and GPG keyrings.
On Endless OS some remotes have keyrings in /usr/share/keyrings/ rather
than /var/lib/flatpak/repo/remote_name.trustedkeys.gpg as do remotes
added by Flatpak. However presumably you would only add a keyring to a
global directory if you trust it to the same extent as the others.
A subsequent commit will add a unit test for this.
Fixes https://github.com/flatpak/flatpak/issues/1447
[1] https://github.com/ostreedev/ostree/pull/1810Closes: #2705
Approved by: alexlarsson
As described in the last commit message, it makes sense to move toward
using flatpak_repo_resolve_rev() rather than ostree_repo_resolve_rev()
for a few reasons:
1) It means we can use OSTREE_REPO_PULL_FLAGS_MIRROR which causes refs
to be pulled into repo/refs/mirrors/ rather than repo/refs/remotes/
which fixes a few edge cases of using collection IDs[1]
2) It falls back to using ostree_repo_resolve_rev() if
ostree_repo_resolve_collection_ref() fails so we can maintain backwards
compatibility for repo/refs/remotes/
3) It distinguishes between remote and local refs, and in the local case
uses OSTREE_REPO_RESOLVE_REV_EXT_LOCAL_ONLY and
ostree_repo_resolve_rev_ext() to make sure we don't for example
accidentally use a remote's repo metadata rather than the local repo's
metadata for the "flatpak repo" command.
So this commit changes every instance of ostree_repo_resolve_rev() in
the codebase to flatpak_repo_resolve_rev().
[1] https://github.com/flatpak/flatpak/issues/1832Closes: #2705
Approved by: alexlarsson
In flatpak_dir_do_resolve_p2p_refs() after pulling a ref we use
ostree_repo_resolve_rev() and pass a refspec with the remote from which
the ref originated. This has a couple side effects, one good and one
bad:
1) The good side effect is that the attack I speculated about in this
comment[1] is not exploitable. Because if the ref in question is pulled
from any remote other than its origin (or a LAN/USB source using another
remote's keyring) it will not be found by ostree_repo_resolve_rev() and
the malicious commit will not be used.
2) The bad side effect is that there are some legitimate reasons a ref
could be pulled from another remote (say, a configured mirror), and in
those cases the pulled ref will not be found. So if I have remote A and
remote B both configured with the same collection ID, a ref installed
from one could be pulled from the other. See this issue[2] for a
concrete example.
The solution is to use OSTREE_REPO_PULL_FLAGS_MIRROR for the pull and
use ostree_repo_resolve_collection_ref() to resolve the ref. This is
done in the caller as well for consistency
(flatpak_dir_resolve_p2p_refs()). This fixes the bad side effect
described above and brings us a step closer to fixing issue #1832. This
also means the attack from #1447 is exploitable, but that is addressed
in a subsequent commit.
This change is conditional on a version check for OSTree 2019.2 because
we need this bug fix[3].
Also, add a helper function flatpak_repo_resolve_rev() which falls back
to using ostree_repo_resolve_rev() when
ostree_repo_resolve_collection_ref() fails, so we start to move toward
using /refs/mirrors/ but maintain backwards compatibility for
/refs/remotes/. A subsequent commit will make wider use of
flatpak_repo_resolve_rev() across the codebase; for now just use it for
the case described above.
[1] https://github.com/flatpak/flatpak/issues/1447#issuecomment-445347590
[2] https://github.com/flatpak/flatpak/issues/1832
[3] https://github.com/ostreedev/ostree/pull/1821Closes: #2705
Approved by: alexlarsson
There's no point building up the options GVariant for a find or a pull
that doesn't happen, so move that work to the innermost scope.
As an aside, it's much more pleasant to look at the diff for this commit
using git's --patience option than the default diff algorithm.
Closes: #2705
Approved by: alexlarsson
A *.flatpakrepo file in this directory will be automatically
added as a system remote with the basename (sans extension) as the
name unless that name already exist. Also, once this is done we
record the name in the repo config so that it is not applied again if
the remote is removed.
Closes: #2884
Approved by: alexlarsson
In general xa.filter being "" means don't filter, so that it can be
used to override-unset a previous filter. However, We canonicalize
to unset/NULL when comparing, returing, or setting the ostree config.
Closes: #2884
Approved by: alexlarsson
