Skip to content

Commit

Permalink
dir: Use the right keyring to verify P2P pulls
Browse files Browse the repository at this point in the history
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 flatpak
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 "override-keyring-remotes" 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 those latter cases
the pull from the malicious source will fail, and flatpak should then do
a successful pull from a legitimate source.

We only use the "override-keyring-remotes" option in repo_pull() even
though ostree_repo_pull_from_remotes_async() is also called in
flatpak_dir_do_resolve_p2p_refs() because (a) in that case it's
difficult to know if a result contains only refs that came from one
configured remote and (b) it would be futile to prevent those commit
metadata pulls because the later ostree_repo_find_remotes_async() call
in repo_pull() would still pull the commit metadata from the malicious
remote (but only the metadata).

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).

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. Therefore it might make sense to have other
mitigations for this issue beyond this commit.

Partial fix for flatpak#1447

[1] ostreedev/ostree#1810
  • Loading branch information
mwleeds committed Feb 8, 2019
1 parent 3b153ae commit 40cf9ba
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions common/flatpak-dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -3979,11 +3979,22 @@ repo_pull (OstreeRepo *self,
{
GVariantBuilder pull_builder;
g_autoptr(GVariant) pull_options = NULL;
g_autoptr(GPtrArray) keyring_remotes = NULL;
int i;

/* Pull options */
g_variant_builder_init (&pull_builder, G_VARIANT_TYPE ("a{sv}"));
get_common_pull_options (&pull_builder, ref_to_fetch, dirs_to_pull, current_checksum,
force_disable_deltas, flags, progress);

/* Ensure the results are signed with the GPG keys associated with the correct remote */
keyring_remotes = g_ptr_array_new ();
for (i = 0; results_to_fetch[i] != NULL; i++)
g_ptr_array_add (keyring_remotes, (char *)remote_name);
g_ptr_array_add (keyring_remotes, NULL);
g_variant_builder_add (&pull_builder, "{s@v}", "override-keyring-remotes",
g_variant_new_variant (g_variant_new_strv ((const char * const *) keyring_remotes->pdata, -1)));

pull_options = g_variant_ref_sink (g_variant_builder_end (&pull_builder));

ostree_repo_pull_from_remotes_async (self, results_to_fetch,
Expand Down

0 comments on commit 40cf9ba

Please sign in to comment.