pull: Cleanly error out on unknown schemes

Previous to this we'd trip an assertion `abort()` deep in the curl code if e.g.
a user did `ostree remote add foo htttp://...` etc.

Motivated by considering supporting "external remotes" where code outside
ostree does a pull, but we want to reuse the signing verification infrastructure.

diff --git a/src/libostree/ostree-fetcher-uri.c b/src/libostree/ostree-fetcher-uri.c
index 485ed187dcc77a8578feef20230a7880ecc2e48d..a08c623e7c1ce3bb7c204086e7f128dee42dc1f3 100644 (file)
--- a/src/libostree/ostree-fetcher-uri.c
+++ b/src/libostree/ostree-fetcher-uri.c
@@ -117,3 +117,21 @@ _ostree_fetcher_uri_to_string (OstreeFetcherURI *uri)
 {
   return soup_uri_to_string ((SoupURI*)uri, FALSE);
 }
+
+
+/* Only accept http, https, and file; particularly curl has a ton of other
+ * backends like sftp that we don't want, and this also gracefully filters
+ * out invalid input.
+ */
+gboolean
+_ostree_fetcher_uri_validate (OstreeFetcherURI *uri, GError **error) 
+{
+  const char *scheme = soup_uri_get_scheme ((SoupURI*)uri);
+  // TODO only allow file if explicitly requested by a higher level
+  if (!(g_str_equal (scheme, "http") || g_str_equal (scheme, "https") || g_str_equal (scheme, "file")))
+    {
+      g_autofree char *s = _ostree_fetcher_uri_to_string (uri);
+      return glnx_throw (error, "Invalid URI scheme in %s", s);
+    }
+  return TRUE;
+}

diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h
index 2065ee54d3059e94b0ea309398854bc97873e9ea..c02ac38c377fb868a84517a15023fe6d50d0d85f 100644 (file)
--- a/src/libostree/ostree-fetcher.h
+++ b/src/libostree/ostree-fetcher.h
@@ -90,6 +90,9 @@ _ostree_fetcher_uri_get_path (OstreeFetcherURI *uri);
 char *
 _ostree_fetcher_uri_to_string (OstreeFetcherURI *uri);
 
+gboolean
+_ostree_fetcher_uri_validate (OstreeFetcherURI *uri, GError **error);
+
 GType   _ostree_fetcher_get_type (void) G_GNUC_CONST;
 
 OstreeFetcher *_ostree_fetcher_new (int                      tmpdir_dfd,

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index ab47a2a40b0ec54a8737506d5e27509752ed414b..12409e633cff6e39ca4437b63b7293a9ca734c84 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -3446,6 +3446,9 @@ compute_effective_mirrorlist (OstreeRepo    *self,
       if (!baseuri)
         return FALSE;
 
+      if (!_ostree_fetcher_uri_validate (baseuri, error))
+        return FALSE;
+
       *out_mirrorlist =
         g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free);
       g_ptr_array_add (*out_mirrorlist, g_steal_pointer (&baseuri));

diff --git a/tests/pull-test.sh b/tests/pull-test.sh
index 5e719bf832d03c4332e1f88c496f991637ad8a19..274bd97863b98a84fc2b6048e43d5832d51fce65 100644 (file)
--- a/tests/pull-test.sh
+++ b/tests/pull-test.sh
@@ -55,10 +55,10 @@ function verify_initial_contents() {
 }
 
 if has_gpgme; then
-    echo "1..36"
+    echo "1..37"
 else
     # 3 tests needs GPG support
-    echo "1..33"
+    echo "1..34"
 fi
 
 # Try both syntaxes
@@ -142,6 +142,14 @@ ${CMD_PREFIX} ostree --repo=mirrorrepo pull origin main
 ${CMD_PREFIX} ostree --repo=mirrorrepo fsck
 echo "ok pull (refuses deltas)"
 
+${CMD_PREFIX} ostree --repo=mirrorrepo remote add broken badscheme://something
+if ${CMD_PREFIX} ostree --repo=mirrorrepo pull broken main 2>err.txt; then
+  assert_not_reached "pulled from invalid"
+fi
+assert_file_has_content_literal err.txt "Invalid URI scheme in badscheme://something"
+${CMD_PREFIX} ostree --repo=mirrorrepo remote delete broken
+echo "ok clean error on invalid scheme"
+
 cd ${test_tmpdir}
 rm mirrorrepo/refs/remotes/* -rf
 ${CMD_PREFIX} ostree --repo=mirrorrepo prune --refs-only