diff options
author | Alexander Larsson <alexl@redhat.com> | 2020-03-17 14:13:14 +0100 |
---|---|---|
committer | Alexander Larsson <alexl@redhat.com> | 2020-03-23 17:58:04 +0100 |
commit | 116a8b848788c582fb2c8577f1250438b100da25 (patch) | |
tree | 17abe210b8be92efa666dd3ba8e8320800bc5dc7 | |
parent | 32194f2d29490d809323054b0d87988fff07d764 (diff) | |
download | flatpak-116a8b848788c582fb2c8577f1250438b100da25.tar.gz |
transaction: Ensure the metadata in the pulled commit matches what we resolved
We're using the metadata from the summary, ostree-metadata or available
commit when making security sensitive decisions, so lets verify this
matches what we get in the actual commit we pulled.
We already did check that this then actually also matches what gets deployed,
so the new check shares code with that.
Note, we don't do this for OCI installs, because it seems the current
fedora flatpaks don't have this set, and we don't want to break
existing remotes.
-rw-r--r-- | common/flatpak-dir-private.h | 3 | ||||
-rw-r--r-- | common/flatpak-dir.c | 76 | ||||
-rw-r--r-- | common/flatpak-installation.c | 4 | ||||
-rw-r--r-- | common/flatpak-transaction.c | 2 | ||||
-rw-r--r-- | system-helper/flatpak-system-helper.c | 6 | ||||
-rwxr-xr-x | tests/test-extensions.sh | 4 | ||||
-rw-r--r-- | tests/test-run.sh | 6 |
7 files changed, 71 insertions, 30 deletions
diff --git a/common/flatpak-dir-private.h b/common/flatpak-dir-private.h index 35aaa886..4a22b387 100644 --- a/common/flatpak-dir-private.h +++ b/common/flatpak-dir-private.h @@ -563,6 +563,7 @@ gboolean flatpak_dir_pull (FlatpakDir *self, const char *opt_rev, const OstreeRepoFinderResult * const *results, const char **subpaths, + GBytes *require_metadata, const char *token, OstreeRepo *repo, FlatpakPullFlags flatpak_flags, @@ -668,6 +669,7 @@ gboolean flatpak_dir_install (FlatpakDir *self, const char *opt_commit, const char **subpaths, const char **previous_ids, + GBytes *require_metadata, const char *token, OstreeAsyncProgress *progress, GCancellable *cancellable, @@ -714,6 +716,7 @@ gboolean flatpak_dir_update (FlatpakDir *self, const OstreeRepoFinderResult * const *results, const char **opt_subpaths, const char **opt_previous_ids, + GBytes *require_metadata, const char *token, OstreeAsyncProgress *progress, GCancellable *cancellable, diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c index a8b4ffa0..52138e79 100644 --- a/common/flatpak-dir.c +++ b/common/flatpak-dir.c @@ -1072,6 +1072,32 @@ flatpak_get_user_base_dir_location (void) return g_object_ref ((GFile *) file); } +static gboolean +validate_commit_metadata (GVariant *commit_data, + const char *ref, + const char *required_metadata, + gboolean require_xa_metadata, + GError **error) +{ + g_autoptr(GVariant) commit_metadata = NULL; + const char *xa_metadata = NULL; + + commit_metadata = g_variant_get_child_value (commit_data, 0); + + if (commit_metadata != NULL) + g_variant_lookup (commit_metadata, "xa.metadata", "&s", &xa_metadata); + + if ((xa_metadata == NULL && require_xa_metadata) || + (xa_metadata != NULL && g_strcmp0 (required_metadata, xa_metadata) != 0)) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED, + _("Commit metadata for %s not matching expected metadata"), ref); + return FALSE; + } + + return TRUE; +} + /* This is a cache directory similar to ~/.cache/flatpak/system-cache, * but in /var/tmp. This is useful for things like the system child * repos, because it is more likely to be on the same filesystem as @@ -4513,12 +4539,12 @@ flatpak_dir_update_appstream (FlatpakDir *self, /* No need to use an existing OstreeRepoFinderResult array, since * appstream updates do not need to be atomic wrt other updates. */ used_branch = new_branch; - if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, + if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, NULL, child_repo, FLATPAK_PULL_FLAGS_NONE, OSTREE_REPO_PULL_FLAGS_MIRROR, progress, cancellable, &first_error)) { used_branch = old_branch; - if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, + if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, NULL, child_repo, FLATPAK_PULL_FLAGS_NONE, OSTREE_REPO_PULL_FLAGS_MIRROR, progress, cancellable, &second_error)) { @@ -4570,12 +4596,12 @@ flatpak_dir_update_appstream (FlatpakDir *self, /* No need to use an existing OstreeRepoFinderResult array, since * appstream updates do not need to be atomic wrt other updates. */ used_branch = new_branch; - if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, NULL, + if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, NULL, NULL, FLATPAK_PULL_FLAGS_NONE, OSTREE_REPO_PULL_FLAGS_NONE, progress, cancellable, &first_error)) { used_branch = old_branch; - if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, NULL, + if (!flatpak_dir_pull (self, state, used_branch, NULL, NULL, NULL, NULL, NULL, NULL, FLATPAK_PULL_FLAGS_NONE, OSTREE_REPO_PULL_FLAGS_NONE, progress, cancellable, &second_error)) { @@ -5462,6 +5488,7 @@ flatpak_dir_pull (FlatpakDir *self, const char *opt_rev, const OstreeRepoFinderResult * const *opt_results, const char **subpaths, + GBytes *require_metadata, const char *token, OstreeRepo *repo, FlatpakPullFlags flatpak_flags, @@ -5650,6 +5677,15 @@ flatpak_dir_pull (FlatpakDir *self, goto out; } + + if (require_metadata) + { + g_autoptr(GVariant) commit_data = NULL; + if (!ostree_repo_load_commit (repo, rev, &commit_data, NULL, error) || + !validate_commit_metadata (commit_data, ref, (const char *)g_bytes_get_data (require_metadata, NULL), TRUE, error)) + return FALSE; + } + if (!flatpak_dir_pull_extra_data (self, repo, state->remote_name, ref, rev, @@ -7956,7 +7992,6 @@ flatpak_dir_deploy (FlatpakDir *self, glnx_autofd int checkoutdir_dfd = -1; g_autoptr(GFile) tmp_dir_template = NULL; g_autofree char *tmp_dir_path = NULL; - const char *xa_metadata = NULL; const char *xa_ref = NULL; g_autofree char *checkout_basename = NULL; gboolean created_extra_data = FALSE; @@ -7966,6 +8001,7 @@ flatpak_dir_deploy (FlatpakDir *self, g_autofree char *metadata_contents = NULL; g_auto(GStrv) ref_parts = NULL; gboolean is_app; + gboolean is_oci; if (!flatpak_dir_ensure_repo (self, cancellable, error)) return FALSE; @@ -8221,18 +8257,14 @@ flatpak_dir_deploy (FlatpakDir *self, } /* Check the metadata in the commit to make sure it matches the actual - deployed metadata, in case we relied on the one in the commit for - a decision */ - g_variant_lookup (commit_metadata, "xa.metadata", "&s", &xa_metadata); - if (xa_metadata != NULL) - { - if (g_strcmp0 (metadata_contents, xa_metadata) != 0) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED, - _("Deployed metadata does not match commit")); - return FALSE; - } - } + * deployed metadata, in case we relied on the one in the commit for + * a decision + * Note: For historical reason we don't enforce commits to contain xa.metadata + * since this was lacking in fedora builds. + */ + is_oci = flatpak_dir_get_remote_oci (self, origin); + if (!validate_commit_metadata (commit_data, ref, metadata_contents, !is_oci, error)) + return FALSE; dotref = g_file_resolve_relative_path (checkoutdir, "files/.ref"); if (!g_file_replace_contents (dotref, "", 0, NULL, FALSE, @@ -8887,6 +8919,7 @@ flatpak_dir_install (FlatpakDir *self, const char *opt_commit, const char **opt_subpaths, const char **opt_previous_ids, + GBytes *require_metadata, const char *token, OstreeAsyncProgress *progress, GCancellable *cancellable, @@ -9047,7 +9080,7 @@ flatpak_dir_install (FlatpakDir *self, flatpak_flags |= FLATPAK_PULL_FLAGS_SIDELOAD_EXTRA_DATA; - if (!flatpak_dir_pull (self, state, ref, opt_commit, NULL, subpaths, token, + if (!flatpak_dir_pull (self, state, ref, opt_commit, NULL, subpaths, require_metadata, token, child_repo, flatpak_flags, OSTREE_REPO_PULL_FLAGS_MIRROR, @@ -9123,7 +9156,7 @@ flatpak_dir_install (FlatpakDir *self, if (!no_pull) { - if (!flatpak_dir_pull (self, state, ref, opt_commit, NULL, opt_subpaths, token, NULL, + if (!flatpak_dir_pull (self, state, ref, opt_commit, NULL, opt_subpaths, require_metadata, token, NULL, flatpak_flags, OSTREE_REPO_PULL_FLAGS_NONE, progress, cancellable, error)) return FALSE; @@ -9568,6 +9601,7 @@ flatpak_dir_update (FlatpakDir *self, const OstreeRepoFinderResult * const *results, const char **opt_subpaths, const char **opt_previous_ids, + GBytes *require_metadata, const char *token, OstreeAsyncProgress *progress, GCancellable *cancellable, @@ -9740,7 +9774,7 @@ flatpak_dir_update (FlatpakDir *self, } flatpak_flags |= FLATPAK_PULL_FLAGS_SIDELOAD_EXTRA_DATA; - if (!flatpak_dir_pull (self, state, ref, commit, results, subpaths, token, + if (!flatpak_dir_pull (self, state, ref, commit, results, subpaths, require_metadata, token, child_repo, flatpak_flags, OSTREE_REPO_PULL_FLAGS_MIRROR, progress, cancellable, error)) @@ -9812,7 +9846,7 @@ flatpak_dir_update (FlatpakDir *self, if (!no_pull) { - if (!flatpak_dir_pull (self, state, ref, commit, results, subpaths, token, + if (!flatpak_dir_pull (self, state, ref, commit, results, subpaths, require_metadata, token, NULL, flatpak_flags, OSTREE_REPO_PULL_FLAGS_NONE, progress, cancellable, error)) return FALSE; diff --git a/common/flatpak-installation.c b/common/flatpak-installation.c index 42d6806f..ce2e9a26 100644 --- a/common/flatpak-installation.c +++ b/common/flatpak-installation.c @@ -2140,7 +2140,7 @@ flatpak_installation_install_full (FlatpakInstallation *self, (flags & FLATPAK_INSTALL_FLAGS_NO_DEPLOY) != 0, (flags & FLATPAK_INSTALL_FLAGS_NO_STATIC_DELTAS) != 0, FALSE, FALSE, state, - ref, NULL, (const char **) subpaths, NULL, NULL, + ref, NULL, (const char **) subpaths, NULL, NULL, NULL, ostree_progress, cancellable, error)) return NULL; @@ -2305,7 +2305,7 @@ flatpak_installation_update_full (FlatpakInstallation *self, FALSE, FALSE, FALSE, state, ref, target_commit, (const OstreeRepoFinderResult * const *) check_results, - (const char **) subpaths, NULL, NULL, + (const char **) subpaths, NULL, NULL, NULL, ostree_progress, cancellable, error)) return NULL; diff --git a/common/flatpak-transaction.c b/common/flatpak-transaction.c index acd05265..c635b379 100644 --- a/common/flatpak-transaction.c +++ b/common/flatpak-transaction.c @@ -3764,6 +3764,7 @@ _run_op_kind (FlatpakTransaction *self, remote_state, op->ref, op->resolved_commit, (const char **) op->subpaths, (const char **) op->previous_ids, + op->resolved_metadata, op->resolved_token, progress->ostree_progress, cancellable, error); @@ -3814,6 +3815,7 @@ _run_op_kind (FlatpakTransaction *self, NULL, (const char **) op->subpaths, (const char **) op->previous_ids, + op->resolved_metadata, op->resolved_token, progress->ostree_progress, cancellable, &local_error); diff --git a/system-helper/flatpak-system-helper.c b/system-helper/flatpak-system-helper.c index f69b9b40..61f82e30 100644 --- a/system-helper/flatpak-system-helper.c +++ b/system-helper/flatpak-system-helper.c @@ -674,7 +674,7 @@ handle_deploy (FlatpakSystemHelper *object, ostree_progress = ostree_async_progress_new_and_connect (no_progress_cb, NULL); - if (!flatpak_dir_pull (system, state, arg_ref, NULL, NULL, (const char **) arg_subpaths, NULL, NULL, + if (!flatpak_dir_pull (system, state, arg_ref, NULL, NULL, (const char **) arg_subpaths, NULL, NULL, NULL, FLATPAK_PULL_FLAGS_NONE, OSTREE_REPO_PULL_FLAGS_UNTRUSTED, ostree_progress, NULL, &error)) { @@ -911,11 +911,11 @@ handle_deploy_appstream (FlatpakSystemHelper *object, ostree_progress = ostree_async_progress_new_and_connect (no_progress_cb, NULL); - if (!flatpak_dir_pull (system, state, new_branch, NULL, NULL, NULL, NULL, NULL, + if (!flatpak_dir_pull (system, state, new_branch, NULL, NULL, NULL, NULL, NULL, NULL, FLATPAK_PULL_FLAGS_NONE, OSTREE_REPO_PULL_FLAGS_UNTRUSTED, ostree_progress, NULL, &first_error)) { - if (!flatpak_dir_pull (system, state, old_branch, NULL, NULL, NULL, NULL, NULL, + if (!flatpak_dir_pull (system, state, old_branch, NULL, NULL, NULL, NULL, NULL, NULL, FLATPAK_PULL_FLAGS_NONE, OSTREE_REPO_PULL_FLAGS_UNTRUSTED, ostree_progress, NULL, &second_error)) { diff --git a/tests/test-extensions.sh b/tests/test-extensions.sh index e3326e09..ee859c60 100755 --- a/tests/test-extensions.sh +++ b/tests/test-extensions.sh @@ -101,7 +101,7 @@ $(dirname $0)/make-test-app.sh repos/test "" master "" > /dev/null # Modify platform metadata ostree checkout -U --repo=repos/test runtime/org.test.Platform/${ARCH}/master platform add_extensions platform -ostree commit --repo=repos/test --owner-uid=0 --owner-gid=0 --no-xattrs --canonical-permissions --branch=runtime/org.test.Platform/${ARCH}/master -s "modified metadata" platform +${FLATPAK} build-export --disable-sandbox repos/test platform --files=files master ${FLATPAK} build-update-repo repos/test ${FLATPAK} remote-add --user --no-gpg-verify test-repo repos/test @@ -155,7 +155,7 @@ ok "runtime extensions" # Modify app metadata ostree checkout -U --repo=repos/test app/org.test.Hello/${ARCH}/master hello add_extensions hello -ostree commit --repo=repos/test --owner-uid=0 --owner-gid=0 --no-xattrs --canonical-permissions --branch=app/org.test.Hello/${ARCH}/master -s "modified metadata" hello +${FLATPAK} build-export --disable-sandbox repos/test hello master ${FLATPAK} build-update-repo repos/test ${FLATPAK} --user update -y org.test.Hello master diff --git a/tests/test-run.sh b/tests/test-run.sh index 8212e50a..fe6e9add 100644 --- a/tests/test-run.sh +++ b/tests/test-run.sh @@ -416,7 +416,8 @@ mkdir -p app/files/a-dir chmod a+rwx app/files/a-dir flatpak build-finish --command=hello.sh app # Note: not --canonical-permissions -ostree --repo=repos/test commit --owner-uid=0 --owner-gid=0 --no-xattrs ${FL_GPGARGS} --branch=app/org.test.Writable/$ARCH/stable app +${FLATPAK} build-export -vv --disable-sandbox --files=files repos/test app stable +ostree --repo=repos/test commit --keep-metadata=xa.metadata --owner-uid=0 --owner-gid=0 --no-xattrs ${FL_GPGARGS} --branch=app/org.test.Writable/$ARCH/stable app update_repo # In the system-helper case this fails to install due to the permission canonicalization happening in the @@ -435,7 +436,8 @@ touch app/files/exe chmod u+s app/files/exe flatpak build-finish --command=hello.sh app # Note: not --canonical-permissions -ostree --repo=repos/test commit --owner-uid=0 --owner-gid=0 --no-xattrs ${FL_GPGARGS} --branch=app/org.test.Setuid/$ARCH/stable app +${FLATPAK} build-export -vv --disable-sandbox --files=files repos/test app stable +ostree -v --repo=repos/test commit --keep-metadata=xa.metadata --owner-uid=0 --owner-gid=0 --no-xattrs ${FL_GPGARGS} --branch=app/org.test.Setuid/$ARCH/stable app update_repo if ${FLATPAK} ${U} install -y test-repo org.test.Setuid &> err2.txt; then |