[ostree] pull: Error if gpg=true and summary is 404, add more tests

[Colin Walters <walters src gnome org>]

commit 720e3b5f834056d3fa978f84c85cdf15274d5c75
Author: Colin Walters <walters verbum org>
Date:   Sat Jun 27 12:00:49 2015 -0400

    pull: Error if gpg=true and summary is 404, add more tests
    
    I did a quick audit pass through the pull code.  What I focused on the
    most is the case where `gpg-verify-summary=true`, and in particular
    where `gpg-verify=false` too.  This should be a valid and secure
    configuration.
    
    The primary change here is to error out very quickly if either
    `summary` or `summary.sig` are 404.  Previously, we'd only error out
    if we were processing deltas.
    
    Expand the existing test case to cover this, plus invalid summary and
    invalid sig.  (The test case was failing with current git master too).

 src/libostree/ostree-repo-pull.c |   45 ++++++++++++++++++++++++++-----------
 tests/test-pull-summary-sigs.sh  |   40 +++++++++++++++++++++++++++------
 2 files changed, 63 insertions(+), 22 deletions(-)
---
diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 96c3a18..29b72bf 100644
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -1318,7 +1318,10 @@ request_static_delta_superblock_sync (OtPullData  *pull_data,
         delta = g_strconcat (from_revision ? from_revision : "", from_revision ? "-" : "", to_revision, 
NULL);
         summary_csum = g_hash_table_lookup (pull_data->summary_deltas_checksums, delta);
 
-
+        /* At this point we've GPG verified the data, so in theory
+         * could trust that they provided the right data, but let's
+         * make this a hard error.
+         */
         if (pull_data->gpg_verify_summary && !summary_csum)
           {
             g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
@@ -1818,27 +1821,41 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
         soup_uri_free (uri);
      }
 
-    if (bytes_summary)
+    if (!bytes_summary && pull_data->gpg_verify_summary)
       {
-        g_autoptr(GVariant) sig_variant = NULL;
-        glnx_unref_object OstreeGpgVerifyResult *result = NULL;
-        pull_data->summary_data = g_bytes_ref (bytes_summary);
-        pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE);
+        g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                     "GPG verification enabled, but no summary found (use gpg-verify-summary=false in remote 
config to disable)");
+        goto out;
+      }
 
+    if (bytes_summary)
+      {
         uri = suburi_new (pull_data->base_uri, "summary.sig", NULL);
         if (!fetch_uri_contents_membuf_sync (pull_data, uri, FALSE, TRUE,
                                              &bytes_sig, cancellable, error))
           goto out;
         soup_uri_free (uri);
+      }
 
-        if (bytes_sig)
-          pull_data->summary_data_sig = g_bytes_ref (bytes_sig);
-        else
-          {
-            g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
-                         "GPG verification enabled, but no summary signatures found (use 
gpg-verify-summary=false in remote config to disable)");
-            goto out;
-          }
+    if (!bytes_sig && pull_data->gpg_verify_summary)
+      {
+        g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
+                     "GPG verification enabled, but no summary.sig found (use gpg-verify-summary=false in 
remote config to disable)");
+        goto out;
+      }
+
+    if (bytes_summary)
+      {
+        pull_data->summary_data = g_bytes_ref (bytes_summary);
+        pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, bytes_summary, FALSE);
+      }
+
+    if (bytes_summary && bytes_sig)
+      {
+        g_autoptr(GVariant) sig_variant = NULL;
+        glnx_unref_object OstreeGpgVerifyResult *result = NULL;
+
+        pull_data->summary_data_sig = g_bytes_ref (bytes_sig);
 
         sig_variant = g_variant_new_from_bytes (OSTREE_SUMMARY_SIG_GVARIANT_FORMAT, bytes_sig, FALSE);
         result = _ostree_repo_gpg_verify_with_metadata (self,
diff --git a/tests/test-pull-summary-sigs.sh b/tests/test-pull-summary-sigs.sh
index a57a9eb..b0acf46 100644
--- a/tests/test-pull-summary-sigs.sh
+++ b/tests/test-pull-summary-sigs.sh
@@ -60,22 +60,46 @@ cd $prev_dir
 
 ${OSTREE} --repo=${test_tmpdir}/ostree-srv/gnomerepo summary -u ${COMMIT_SIGN}
 
+repo_reinit () {
+  cd ${test_tmpdir}
+  rm -rf repo
+  mkdir repo
+  ${OSTREE} --repo=repo init --mode=archive-z2
+  ${OSTREE} --repo=repo remote add --set=gpg-verify-summary=true origin $(cat httpd-address)/ostree/gnomerepo
+}
+
 cd ${test_tmpdir}
-rm -rf repo
-mkdir repo
-${OSTREE} --repo=repo init --mode=archive-z2
-${OSTREE} --repo=repo remote add origin $(cat httpd-address)/ostree/gnomerepo
+repo_reinit
 ${OSTREE} --repo=repo pull origin main
 echo "ok pull with signed summary"
 
+cd ${test_tmpdir}
+repo_reinit
+mv ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{,.good}
+echo invalid > ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig
+if ${OSTREE} --repo=repo pull origin main 2>err.txt; then
+    assert_not_reached "Successful pull with invalid GPG sig"
+fi
+assert_file_has_content err.txt "no signatures found"
+mv ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.good,}
+echo "ok pull with invalid summary gpg signature fails"
+
+cd ${test_tmpdir}
+repo_reinit
+cp ${test_tmpdir}/ostree-srv/gnomerepo/summary{,.good}
+# Some leading garbage
+(echo invalid && cat ${test_tmpdir}/ostree-srv/gnomerepo/summary) > summary.bad.tmp && mv summary.bad.tmp 
${test_tmpdir}/ostree-srv/gnomerepo/summary
+if ${OSTREE} --repo=repo pull origin main; then
+    assert_not_reached "Successful pull with invalid summary"
+fi
+mv ${test_tmpdir}/ostree-srv/gnomerepo/summary{.good,}
+echo "ok pull with invalid summary (leading garbage) fails"
+
 # Generate a delta
 ${OSTREE} --repo=${test_tmpdir}/ostree-srv/gnomerepo static-delta generate --empty main
 ${OSTREE} --repo=${test_tmpdir}/ostree-srv/gnomerepo summary -u ${COMMIT_SIGN}
 
 cd ${test_tmpdir}
-rm -rf repo
-mkdir repo
-${OSTREE} --repo=repo init --mode=archive-z2
-${OSTREE} --repo=repo remote add origin $(cat httpd-address)/ostree/gnomerepo
+repo_reinit
 ${OSTREE} --repo=repo pull origin main
 echo "ok pull delta with signed summary"