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
This test is meant to test the issue described in the message for the
commit "dir: Use the right keyring to verify P2P pulls".
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 not much point in using /bin/bash for all but three scripts and
using /bin/sh for those, so use bash for everything.
Closes: #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
This kind of remotes don't work very well, because they are not modifiable
or removable and can cause conflicts with pre-existing remotes.
We instead want to switch to a way to define default remotes that
is applied to the regular ostree repo config, allowing later changes.
Closes: #2884
Approved by: alexlarsson
This is in preparation for writing some unit tests
for these functions. Apparently they are not too
trivial to get them wrong.
Closes: #2880
Approved by: alexlarsson
If you're adding a remote with --if-not-exists and it exists,
apply the filter key anyway. Typically this is used to allow a
user to use the standard instructions to add a remote even if there
is a pre-existing filtered remote already configured.
Closes: #2869
Approved by: alexlarsson
This is kind of special as it handles non-existing filter keys
as "override to no filters" instead of "leave existing filters" whic
we do for other things. However, that matches how we want to use
.flatpakrepo files to override site-local filters.
Closes: #2869
Approved by: alexlarsson
If there is a filter for the remote, include the checksum of it
in the directory name for the deploy dir. This way we re-deploy the
appstream if the filter changes.
Closes: #2869
Approved by: alexlarsson
This means only refs that are allowed will be visible in the deployed
appstream.
This sometimes means we have to uncompress the xml.gz before rewriting
and then re-compressing that.
As a result of this we now always end up with both the compressed and
the uncompressed xml file, whereas we before didn't get an uncompressed
one if the remote was using the old appstream format.
Closes: #2869
Approved by: alexlarsson
Full names are:
* flatpak_remote_state_lookup_ref()
* flatpak_dir_list_all_remote_refs()
This should be enough to not get any stray refs, because due to the
complexities of p2p all accesses go via FlatpakRemoteState.
The one leftover to non-filtered stuff is the appstream file, a later
commit will handle that.
Closes: #2869
Approved by: alexlarsson
If this is set to a pathname in the configuration of a remote we load
that file and keep track of it as a filter. No filter is actually applied
yet though.
There is two levels of caching, consecutive lookups of the filter
with less than 500msec between use the previous value. Once its more
that that we stat() the file and compare the mtime, reloading the file
if the mtime changes. We also directly reload the file if the pathname
changes.
Closes: #2869
Approved by: alexlarsson
Filters are specified as a list of
op glob
where op can be allow or deny, and the blogs are partial refs
where * means anything in this part of the ref, and non-specified
trailing parts of the ref matches anything.
By default everything is allowed, but if you specify some
deny rules all that they match is denied, except if there is
an specific allow rule.
Internally this takes all the allow and deny globs and convert
them into two combined regular expressions.
Some examples:
* - match anything
app/* - match all apps
runtime/* - match all runtimes
app/*/x86_64 - match all x86-64 apps
app/org.gnome.*/*/stable - match all stable gnome apps
org.gnome.frogr* - match frogr and extensions
This means you can do a both whitelisting:
deny *
allow org.the.good.app*
Or blacklisting:
deny org.the.bad.app*
Closes: #2869
Approved by: alexlarsson
