From e5e52b4b79837807405764bc1ee85d2323aa35fa Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 7 Aug 2017 14:50:43 +0100 Subject: [PATCH] common/utils: Allow collection-id to be updated from repo config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to provide a transition path for repositories to add collection IDs to themselves and propagate those collection IDs to clients’ remote configurations, add another repo config key which controls whether the repository’s collection ID is published. If xa.collection-id is set in the repo’s published metadata, the client will update its configuration to the given ID — but only if no ID is set already. This is a one-time transition to prevent malicious repositories from remotely changing the user’s configuration to associate their remote with a well-known collection ID they don’t own. Add a test for this. Signed-off-by: Philip Withnall --- app/flatpak-builtins-repo-update.c | 6 ++ app/flatpak-builtins-repo.c | 4 + common/flatpak-utils.c | 29 ++++++- common/flatpak-utils.h | 3 + doc/flatpak-build-update-repo.xml | 13 +++ tests/Makefile.am.inc | 1 + tests/test-update-remote-configuration.sh | 97 +++++++++++++++++++++++ 7 files changed, 152 insertions(+), 1 deletion(-) create mode 100755 tests/test-update-remote-configuration.sh diff --git a/app/flatpak-builtins-repo-update.c b/app/flatpak-builtins-repo-update.c index 280958bb..36cc118e 100644 --- a/app/flatpak-builtins-repo-update.c +++ b/app/flatpak-builtins-repo-update.c @@ -37,6 +37,7 @@ static char *opt_title; static char *opt_redirect_url; static char *opt_default_branch; static char *opt_collection_id = NULL; +static gboolean opt_deploy_collection_id = FALSE; static char **opt_gpg_import; static char *opt_generate_delta_from; static char *opt_generate_delta_to; @@ -53,6 +54,7 @@ static GOptionEntry options[] = { { "default-branch", 0, 0, G_OPTION_ARG_STRING, &opt_default_branch, N_("Default branch to use for this repository"), N_("BRANCH") }, #ifdef FLATPAK_ENABLE_P2P { "collection-id", 0, 0, G_OPTION_ARG_STRING, &opt_collection_id, N_("Collection ID"), N_("COLLECTION-ID") }, + { "deploy-collection-id", 0, 0, G_OPTION_ARG_NONE, &opt_deploy_collection_id, N_("Permanently deploy collection ID to client remote configurations"), NULL }, #endif /* FLATPAK_ENABLE_P2P */ { "gpg-import", 0, 0, G_OPTION_ARG_FILENAME_ARRAY, &opt_gpg_import, N_("Import new default GPG public key from FILE"), N_("FILE") }, { "gpg-sign", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_gpg_key_ids, N_("GPG Key ID to sign the summary with"), N_("KEY-ID") }, @@ -443,6 +445,10 @@ flatpak_builtin_build_update_repo (int argc, char **argv, !flatpak_repo_set_collection_id (repo, opt_collection_id[0] ? opt_collection_id : NULL, error)) return FALSE; + if (opt_deploy_collection_id && + !flatpak_repo_set_deploy_collection_id (repo, TRUE, error)) + return FALSE; + if (opt_gpg_import) { g_autoptr(GBytes) gpg_data = flatpak_load_gpg_keys (opt_gpg_import, cancellable, error); diff --git a/app/flatpak-builtins-repo.c b/app/flatpak-builtins-repo.c index a4ae5629..d4c8baa7 100644 --- a/app/flatpak-builtins-repo.c +++ b/app/flatpak-builtins-repo.c @@ -41,6 +41,7 @@ print_info (GVariant *meta) const char *collection_id; const char *default_branch; const char *redirect_url; + const char *redirect_collection_id; g_autoptr(GVariant) gpg_keys = NULL; if (g_variant_lookup (meta, "xa.title", "&s", &title)) @@ -55,6 +56,9 @@ print_info (GVariant *meta) if (g_variant_lookup (meta, "xa.redirect-url", "&s", &redirect_url)) g_print ("Redirect URL: %s\n", redirect_url); + if (g_variant_lookup (meta, "xa.collection-id", "&s", &redirect_collection_id)) + g_print ("Redirect collection ID: %s\n", redirect_collection_id); + if ((gpg_keys = g_variant_lookup_value (meta, "xa.gpg-keys", G_VARIANT_TYPE_BYTESTRING)) != NULL) { const guchar *gpg_data = g_variant_get_data (gpg_keys); diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c index b420f683..04a97f74 100644 --- a/common/flatpak-utils.c +++ b/common/flatpak-utils.c @@ -2727,6 +2727,18 @@ flatpak_repo_set_redirect_url (OstreeRepo *repo, return TRUE; } +gboolean +flatpak_repo_set_deploy_collection_id (OstreeRepo *repo, + gboolean deploy_collection_id, + GError **error) +{ + g_autoptr(GKeyFile) config = NULL; + + config = ostree_repo_copy_config (repo); + g_key_file_set_boolean (config, "flatpak", "deploy-collection-id", deploy_collection_id); + return ostree_repo_write_config (repo, config, error); +} + gboolean flatpak_repo_set_gpg_keys (OstreeRepo *repo, GBytes *bytes, @@ -3093,7 +3105,14 @@ populate_commit_data_cache (GVariant *metadata, * If the repo has a collection ID set, additionally store the metadata on a * contentless commit in a well-known branch, which is the preferred way of * broadcasting per-repo metadata (putting it in the summary file is deprecated, - * but kept for backwards compatibility). */ + * but kept for backwards compatibility). + * + * Note that there are two keys for the collection ID: collection-id, and + * xa.collection-id. If a client does not currently have a collection ID configured + * for this remote, it will *only* update its configuration from xa.collection-id. + * This allows phased deployment of collection-based repositories. Clients will + * only update their configuration from an unset to a set collection ID once + * (otherwise the security properties of collection IDs are broken). */ gboolean flatpak_repo_update (OstreeRepo *repo, const char **gpg_key_ids, @@ -3119,6 +3138,7 @@ flatpak_repo_update (OstreeRepo *repo, const char *collection_id; g_autofree char *old_ostree_metadata_checksum = NULL; g_autoptr(GVariant) old_ostree_metadata_v = NULL; + gboolean deploy_collection_id = FALSE; g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); @@ -3130,6 +3150,7 @@ flatpak_repo_update (OstreeRepo *repo, default_branch = g_key_file_get_string (config, "flatpak", "default-branch", NULL); gpg_keys = g_key_file_get_string (config, "flatpak", "gpg-keys", NULL); redirect_url = g_key_file_get_string (config, "flatpak", "redirect-url", NULL); + deploy_collection_id = g_key_file_get_boolean (config, "flatpak", "deploy-collection-id", NULL); } #ifdef FLATPAK_ENABLE_P2P @@ -3150,6 +3171,12 @@ flatpak_repo_update (OstreeRepo *repo, g_variant_builder_add (&builder, "{sv}", "xa.default-branch", g_variant_new_string (default_branch)); + if (deploy_collection_id && collection_id != NULL) + g_variant_builder_add (&builder, "{sv}", "xa.collection-id", + g_variant_new_string (collection_id)); + else if (deploy_collection_id) + g_debug ("Ignoring deploy-collection-id=true because no collection ID is set."); + if (gpg_keys) { guchar *decoded; diff --git a/common/flatpak-utils.h b/common/flatpak-utils.h index a5748b51..48a84580 100644 --- a/common/flatpak-utils.h +++ b/common/flatpak-utils.h @@ -303,6 +303,9 @@ gboolean flatpak_repo_set_default_branch (OstreeRepo *repo, gboolean flatpak_repo_set_collection_id (OstreeRepo *repo, const char *collection_id, GError **error); +gboolean flatpak_repo_set_deploy_collection_id (OstreeRepo *repo, + gboolean deploy_collection_id, + GError **error); gboolean flatpak_repo_set_gpg_keys (OstreeRepo *repo, GBytes *bytes, GError **error); diff --git a/doc/flatpak-build-update-repo.xml b/doc/flatpak-build-update-repo.xml index 953fd0e3..c2eb1a49 100644 --- a/doc/flatpak-build-update-repo.xml +++ b/doc/flatpak-build-update-repo.xml @@ -115,6 +115,19 @@ the existing collection ID will be left unchanged. + + + + + + Deploy the collection ID (set using + in the static remote configuration for all clients. This is + irrevocable once published in a repository. Use it to decide + when to roll out a collection ID to users of an existing repository. + If constructing a new repository which has a collection ID, + you should typically always pass this option. + + --> diff --git a/tests/Makefile.am.inc b/tests/Makefile.am.inc index d6ffde63..14250a7c 100644 --- a/tests/Makefile.am.inc +++ b/tests/Makefile.am.inc @@ -140,6 +140,7 @@ dist_test_scripts = \ tests/test-builder-python.sh \ tests/test-oci.sh \ tests/test-unsigned-summaries.sh \ + tests/test-update-remote-configuration.sh \ $(NULL) test_programs = testdb test-doc-portal testlibrary diff --git a/tests/test-update-remote-configuration.sh b/tests/test-update-remote-configuration.sh new file mode 100755 index 00000000..76957ba8 --- /dev/null +++ b/tests/test-update-remote-configuration.sh @@ -0,0 +1,97 @@ +#!/bin/bash +# +# Copyright © 2017 Endless Mobile, Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place - Suite 330, +# Boston, MA 02111-1307, USA. +# +# Authors: +# - Philip Withnall + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +skip_without_bwrap +skip_without_user_xattrs +skip_without_p2p + +echo "1..3" + +# Configure a repository without a collection ID and pull it locally. +setup_repo +install_repo + +DIR=$(mktemp -d) +${FLATPAK} build-init ${DIR} org.test.App org.test.Platform org.test.Platform +mkdir -p ${DIR}/files/a +echo "a" > ${DIR}/files/a/data +${FLATPAK} build-finish ${DIR} --socket=x11 --share=network --command=true +${FLATPAK} build-export ${FL_GPGARGS} --update-appstream repos/test ${DIR} master +update_repo + +${FLATPAK} ${U} install test-repo org.test.App master + +assert_file_has_content ${FL_DIR}/repo/config '^gpg-verify-summary=true$' +assert_not_file_has_content ${FL_DIR}/repo/config '^gpg-verify-summary=false$' +assert_file_has_content ${FL_DIR}/repo/config '^gpg-verify=true$' +assert_not_file_has_content ${FL_DIR}/repo/config '^gpg-verify=false$' +assert_not_file_has_content ${FL_DIR}/repo/config '^collection-id=' + +# Change its configuration to include a collection ID, update the repository, +# but don’t mark the collection ID as to be deployed yet. Ensure it doesn’t +# appear in the client’s configuration. +echo -e "[core]\ncollection-id=org.test.Collection" >> repos/test/config +${FLATPAK} build-export ${FL_GPGARGS} --update-appstream repos/test --collection-id org.test.Collection ${DIR} master +UPDATE_REPO_ARGS="--collection-id=org.test.Collection" update_repo + +${FLATPAK} ${U} update org.test.App master + +assert_file_has_content ${FL_DIR}/repo/config '^gpg-verify-summary=true$' +assert_not_file_has_content ${FL_DIR}/repo/config '^gpg-verify-summary=false$' +assert_file_has_content ${FL_DIR}/repo/config '^gpg-verify=true$' +assert_not_file_has_content ${FL_DIR}/repo/config '^gpg-verify=false$' +assert_not_file_has_content ${FL_DIR}/repo/config '^collection-id=' + +echo "ok 1 update repo config without deploying collection ID" + +# Now mark the collection ID as to be deployed. The client configuration should +# be updated. +UPDATE_REPO_ARGS="--collection-id=org.test.Collection --deploy-collection-id" update_repo +${FLATPAK} ${U} update org.test.App master + +assert_file_has_content ${FL_DIR}/repo/config '^gpg-verify-summary=false$' +assert_not_file_has_content ${FL_DIR}/repo/config '^gpg-verify-summary=true$' +assert_file_has_content ${FL_DIR}/repo/config '^gpg-verify=true$' +assert_not_file_has_content ${FL_DIR}/repo/config '^gpg-verify=false$' +assert_file_has_content ${FL_DIR}/repo/config '^collection-id=org.test.Collection$' + +echo "ok 2 update repo config to deploy collection ID" + +# Try updating the collection ID to some other non-empty value on the server. +# The client should ignore the update (otherwise we have a security vulnerability). +# We have to manually add refs under the old collection ID so the client can pull +# using its old collection ID. +#UPDATE_REPO_ARGS="--collection-id=net.malicious.NewCollection --deploy-collection-id" update_repo +#for ref in app/org.test.App/x86_64/master app/org.test.Hello/x86_64/master appstream/x86_64 ostree-metadata runtime/org.test.Platform/x86_64/master; do +# ostree --repo=repos/test refs --collections --create=org.test.Collection:$ref $ref +#done +ostree --repo=repos/test summary --update --add-metadata="xa.collection-id='net.malicious.NewCollection'" +${FLATPAK} ${U} update org.test.App master + +assert_file_has_content ${FL_DIR}/repo/config '^collection-id=org.test.Collection$' +assert_not_file_has_content ${FL_DIR}/repo/config '^collection-id=net.malicious.NewCollection$' + +echo "ok 3 update repo config to with different collection ID"