From 3a16689f05a0c447b360c451ca467c3424a43139 Mon Sep 17 00:00:00 2001 From: Matthew Leeds Date: Wed, 17 Oct 2018 12:55:38 -0700 Subject: [PATCH] Allow disabling pulling from LAN/USB/Internet Currently libostree essentially has two modes when it's pulling refs: the "legacy" code paths pull only from the Internet, and the code paths that are aware of collection IDs try to pull from the Internet, the local network, and mounted filesystems (such as USB drives). The problem is that while we eventually want to migrate everyone to using collection IDs, we don't want to force checking LAN and USB sources if the user just wants to pull from the Internet, since the LAN/USB code paths can have privacy[1], security[2], and performance[3] implications. So this commit implements a new repo config option called "repo-finders" which can be configured to, for example, "config;lan;mount;" to check all three sources or "config;mount;" to disable searching the LAN. The set of values mirror those used for the --finders option of the find-remotes command. This configuration affects pulls in three places: 1. the ostree_repo_find_remotes_async() API, regardless of whether or not the user of the API provided a list of OstreeRepoFinders 2. the ostree_repo_finder_resolve_async() / ostree_repo_finder_resolve_all_async() API 3. the find-remotes command This feature is especially important right now since we soon want to have Flathub publish a metadata key which will have Flatpak clients update the remote config to add a collection ID.[4] This effectively fixes https://github.com/flatpak/flatpak/issues/1863 but I'll patch Flatpak too, so it doesn't pass finders to libostree only to then have them be removed. [1] https://github.com/flatpak/flatpak/issues/1863#issuecomment-404128824 [2] https://github.com/ostreedev/ostree/issues/1527 [3] Based on how long the "ostree find-remotes" command takes to complete, having the LAN finder enabled slows down that step of the pull process by about 40%. See also https://github.com/flatpak/flatpak/issues/1862 [4] https://github.com/flathub/flathub/issues/676 --- man/ostree.repo-config.xml | 13 +++++ src/libostree/ostree-repo-private.h | 1 + src/libostree/ostree-repo-pull.c | 81 +++++++++++++++++++--------- src/libostree/ostree-repo.c | 34 ++++++++++++ src/ostree/ot-builtin-find-remotes.c | 18 +++++++ 5 files changed, 121 insertions(+), 26 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 8942a00bcb..a0cfd16477 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -215,6 +215,19 @@ Boston, MA 02111-1307, USA. of -1 means block indefinitely. The default value is 30. + + + repo-finders + Semicolon separated list of finders (sources for refs) + to use when pulling instead of the default set. This can be + used to disable pulling from mounted filesystems, peers on the + local network, or the Internet. Possible values: + config, lan, and + mount (or any combination thereof). The default is + to pull from all three, unless Avahi support was disabled at compile + time, in which case lan is disabled. + + diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index ad1269b325..48df54e3df 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -168,6 +168,7 @@ struct OstreeRepo { gint lock_timeout_seconds; guint64 payload_link_threshold; gint fs_support_reflink; /* The underlying filesystem has support for ioctl (FICLONE..) */ + gchar **repo_finders; /* nullable */ OstreeRepo *parent_repo; }; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index f5c52e4a96..5026d1a58d 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -4970,6 +4970,8 @@ ostree_repo_find_remotes_async (OstreeRepo *self, g_autoptr(GTask) task = NULL; g_autoptr(FindRemotesData) data = NULL; OstreeRepoFinder *default_finders[4] = { NULL, }; + OstreeRepoFinder *sanitized_finders[4] = { NULL, }; + guint finder_index = 0; g_autoptr(OstreeRepoFinder) finder_config = NULL; g_autoptr(OstreeRepoFinder) finder_mount = NULL; g_autoptr(OstreeRepoFinder) finder_avahi = NULL; @@ -5004,43 +5006,70 @@ ostree_repo_find_remotes_async (OstreeRepo *self, g_autoptr(GError) local_error = NULL; #endif /* HAVE_AVAHI */ - finder_config = OSTREE_REPO_FINDER (ostree_repo_finder_config_new ()); - finder_mount = OSTREE_REPO_FINDER (ostree_repo_finder_mount_new (NULL)); + if (self->repo_finders == NULL || g_strv_contains ((const char * const *)self->repo_finders, "config")) + default_finders[finder_index++] = finder_config = OSTREE_REPO_FINDER (ostree_repo_finder_config_new ()); + + if (self->repo_finders == NULL || g_strv_contains ((const char * const *)self->repo_finders, "mount")) + default_finders[finder_index++] = finder_mount = OSTREE_REPO_FINDER (ostree_repo_finder_mount_new (NULL)); + #ifdef HAVE_AVAHI - finder_avahi = OSTREE_REPO_FINDER (ostree_repo_finder_avahi_new (context)); + if (self->repo_finders == NULL || g_strv_contains ((const char * const *)self->repo_finders, "lan")) + default_finders[finder_index++] = finder_avahi = OSTREE_REPO_FINDER (ostree_repo_finder_avahi_new (context)); #endif /* HAVE_AVAHI */ - default_finders[0] = finder_config; - default_finders[1] = finder_mount; - default_finders[2] = finder_avahi; - + g_assert (default_finders != NULL); finders = default_finders; #ifdef HAVE_AVAHI - ostree_repo_finder_avahi_start (OSTREE_REPO_FINDER_AVAHI (finder_avahi), - &local_error); - - if (local_error != NULL) + if (finder_avahi != NULL) { - /* See ostree-repo-finder-avahi.c:ostree_repo_finder_avahi_start, we - * intentionally throw this so as to distinguish between the Avahi - * finder failing because the Avahi daemon wasn't running and - * the Avahi finder failing because of some actual error. - * - * We need to distinguish between g_debug and g_warning here because - * unit tests that use this code may set G_DEBUG=fatal-warnings which - * would cause client code to abort if a warning were emitted. - */ - if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) - g_debug ("Avahi finder failed under normal operation; removing it: %s", local_error->message); - else - g_warning ("Avahi finder failed abnormally; removing it: %s", local_error->message); + ostree_repo_finder_avahi_start (OSTREE_REPO_FINDER_AVAHI (finder_avahi), + &local_error); + + if (local_error != NULL) + { + /* See ostree-repo-finder-avahi.c:ostree_repo_finder_avahi_start, we + * intentionally throw this so as to distinguish between the Avahi + * finder failing because the Avahi daemon wasn't running and + * the Avahi finder failing because of some actual error. + * + * We need to distinguish between g_debug and g_warning here because + * unit tests that use this code may set G_DEBUG=fatal-warnings which + * would cause client code to abort if a warning were emitted. + */ + if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) + g_debug ("Avahi finder failed under normal operation; removing it: %s", local_error->message); + else + g_warning ("Avahi finder failed abnormally; removing it: %s", local_error->message); - default_finders[2] = NULL; - g_clear_object (&finder_avahi); + default_finders[2] = NULL; + g_clear_object (&finder_avahi); + } } #endif /* HAVE_AVAHI */ } + else if (self->repo_finders != NULL) + { + guint i; + for (i = 0; finders[i] != NULL; i++)• + { + OstreeRepoFinder *finder = OSTREE_REPO_FINDER (finders[i]); + + if (OSTREE_IS_REPO_FINDER_CONFIG (finder) && + !g_strv_contains ((const char * const *)self->repo_finders, "config")) + g_debug ("Config repo finder passed to ostree_repo_find_remotes_async but repo config prevents use of it"); + else if (OSTREE_IS_REPO_FINDER_MOUNT (finder) && + !g_strv_contains ((const char * const *)self->repo_finders, "mount")) + g_debug ("Mount repo finder passed to ostree_repo_find_remotes_async but repo config prevents use of it"); + else if (OSTREE_IS_REPO_FINDER_AVAHI (finder) && + !g_strv_contains ((const char * const *)self->repo_finders, "lan")) + g_debug ("Avahi repo finder passed to ostree_repo_find_remotes_async but repo config prevents use of it"); + else + sanitized_finders[finder_index++] = finder; + } + + finders = sanitized_finders; + } /* We need to keep a pointer to the default Avahi finder so we can stop it * again after the operation, which happens implicitly by dropping the final diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 857b2d4ccc..091f682e0c 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1035,6 +1035,7 @@ ostree_repo_finalize (GObject *object) g_mutex_clear (&self->cache_lock); g_mutex_clear (&self->txn_lock); g_free (self->collection_id); + g_strfreev (self->repo_finders); g_clear_pointer (&self->remotes, g_hash_table_destroy); g_mutex_clear (&self->remotes_lock); @@ -2938,6 +2939,39 @@ reload_core_config (OstreeRepo *self, self->payload_link_threshold = g_ascii_strtoull (payload_threshold, NULL, 10); } + { g_auto(GStrv) configured_finders = NULL; + g_autoptr(GError) local_error = NULL; + + configured_finders = g_key_file_get_string_list (self->config, "core", "repo-finders", + NULL, &local_error); + if (g_error_matches (local_error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND)) + g_clear_error (&local_error); + else if (local_error) + return FALSE; + + g_clear_pointer (&self->repo_finders, g_strfreev); + self->repo_finders = g_steal_pointer (&configured_finders); + + if (self->repo_finders != NULL) + { + char **repo_finders_iter; + + if (*self->repo_finders == '\0') + return glnx_throw (error, "Invalid empty repo-finders configuration"); + + repo_finders_iter = self->repo_finders; + for (;repo_finders_iter && *repo_finders_iter; repo_finders_iter++) + { + const char *repo_finder = *repo_finders_iter; + + if (strcmp (repo_finder, "config") != 0 && + strcmp (repo_finder, "lan") != 0 && + strcmp (repo_finder, "mount") != 0) + return glnx_throw (error, "Invalid configured repo-finder '%s'", repo_finder); + } + } + } + return TRUE; } diff --git a/src/ostree/ot-builtin-find-remotes.c b/src/ostree/ot-builtin-find-remotes.c index f255501ad1..df7a8d189e 100644 --- a/src/ostree/ot-builtin-find-remotes.c +++ b/src/ostree/ot-builtin-find-remotes.c @@ -245,11 +245,23 @@ ostree_builtin_find_remotes (int argc, { if (g_strcmp0 (*iter, "config") == 0) { + if (repo->repo_finders != NULL && !g_strv_contains ((const char * const *)repo->repo_finders, "config")) + { + ot_util_usage_error (context, "Config repo finder requested but disabled in config", error); + return FALSE; + } + finder_config = OSTREE_REPO_FINDER (ostree_repo_finder_config_new ()); g_ptr_array_add (finders, finder_config); } else if (g_strcmp0 (*iter, "mount") == 0) { + if (repo->repo_finders != NULL && !g_strv_contains ((const char * const *)repo->repo_finders, "mount")) + { + ot_util_usage_error (context, "Mount repo finder requested but disabled in config", error); + return FALSE; + } + finder_mount = OSTREE_REPO_FINDER (ostree_repo_finder_mount_new (NULL)); g_ptr_array_add (finders, finder_mount); } @@ -259,6 +271,12 @@ ostree_builtin_find_remotes (int argc, GMainContext *main_context = g_main_context_get_thread_default (); g_autoptr(GError) local_error = NULL; + if (repo->repo_finders != NULL && !g_strv_contains ((const char * const *)repo->repo_finders, "lan")) + { + ot_util_usage_error (context, "LAN repo finder requested but disabled in config", error); + return FALSE; + } + finder_avahi = OSTREE_REPO_FINDER (ostree_repo_finder_avahi_new (main_context)); ostree_repo_finder_avahi_start (OSTREE_REPO_FINDER_AVAHI (finder_avahi), &local_error);