Merge pull request #1877 from dbnicholson/specific-gpg-errors

More specific GPG errors

Makefile-tests.am

@@ -220,10 +220,17 @@ dist_gpginsttest_DATA = tests/gpghome/secring.gpg \
 	tests/gpghome/key3.asc
 gpginsttest_trusteddir = $(installed_testdir)/gpghome/trusted
 dist_gpginsttest_trusted_DATA = tests/gpghome/trusted/pubring.gpg
+gpginsttest_revocdir = $(installed_testdir)/gpghome/revocations
+dist_gpginsttest_revoc_DATA = \
+	tests/gpghome/revocations/key1.rev \
+	tests/gpghome/revocations/key2.rev \
+	tests/gpghome/revocations/key3.rev \
+	$(NULL)
 
 gpgvinsttestdir = $(installed_testdir)/gpg-verify-data
 dist_gpgvinsttest_DATA = $(addprefix tests/gpg-verify-data/, \
-	gpg.conf lgpl2 lgpl2.sig pubring.gpg secring.gpg trustdb.gpg)
+	gpg.conf lgpl2 lgpl2.sig lgpl2.sig0 lgpl2.sig1 lgpl2.sig2 lgpl2.sig3 \
+	lgpl2.sig4 pubring.gpg secring.gpg trustdb.gpg)
 endif
 endif
 

ci/flatpak-1.4.1-ostree-gpg-errors.patch

@@ -0,0 +1,65 @@
+From 8e649d094e9dd91adbb430015b2621c66e086df7 Mon Sep 17 00:00:00 2001
+From: Dan Nicholson <nicholson@endlessm.com>
+Date: Tue, 21 Jan 2020 15:32:27 -0700
+Subject: [PATCH] tests: Accommodate new OSTree GPG error strings
+
+Recently OSTree has been updated to provide proper error strings when
+validating GPG signatures instead of a single generic string[1]. Allow
+either in the tests so they work against new or old ostree.
+
+1. https://github.com/ostreedev/ostree/pull/1877
+---
+ tests/test-p2p-security.sh | 2 +-
+ tests/test-repo.sh         | 8 ++++----
+ 2 files changed, 5 insertions(+), 5 deletions(-)
+
+diff --git a/tests/test-p2p-security.sh b/tests/test-p2p-security.sh
+index db929dd4..9b0ca1d0 100644
+--- a/tests/test-p2p-security.sh
++++ b/tests/test-p2p-security.sh
+@@ -53,7 +53,7 @@ GPGARGS="${FL_GPGARGS2}" make_updated_app test-impostor org.test.Collection
+ if G_MESSAGES_DEBUG=all ${FLATPAK} ${U} update -y org.test.Hello >failed-p2p-update-log; then
+     assert_not_reached "Update of org.test.Hello was successful despite malicious commit"
+ fi
+-assert_file_has_content failed-p2p-update-log "GPG signatures found, but none are in trusted keyring"
++assert_file_has_content failed-p2p-update-log "\(GPG signatures found, but none are in trusted keyring\|public key not found\)"
+ 
+ COMMIT_AFTER_FAILED_UPDATE=$(${FLATPAK} ${U} info -c org.test.Hello)
+ if [ "x${INITIAL_COMMIT}" != "x${COMMIT_AFTER_FAILED_UPDATE}" ]; then
+diff --git a/tests/test-repo.sh b/tests/test-repo.sh
+index 01ca6a94..14cb1179 100644
+--- a/tests/test-repo.sh
++++ b/tests/test-repo.sh
+@@ -183,25 +183,25 @@ ${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello
+ if ${FLATPAK} ${U} install -y test-missing-gpg-repo org.test.Platform 2> install-error-log; then
+     assert_not_reached "Should not be able to install with missing gpg key"
+ fi
+-assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring"
++assert_file_has_content install-error-log "\(GPG signatures found, but none are in trusted keyring\|public key not found\)"
+ 
+ 
+ if ${FLATPAK} ${U} install test-missing-gpg-repo org.test.Hello 2> install-error-log; then
+     assert_not_reached "Should not be able to install with missing gpg key"
+ fi
+-assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring"
++assert_file_has_content install-error-log "\(GPG signatures found, but none are in trusted keyring\|public key not found\)"
+ 
+ echo "ok fail with missing gpg key"
+ 
+ if ${FLATPAK} ${U} install test-wrong-gpg-repo org.test.Platform 2> install-error-log; then
+     assert_not_reached "Should not be able to install with wrong gpg key"
+ fi
+-assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring"
++assert_file_has_content install-error-log "\(GPG signatures found, but none are in trusted keyring\|public key not found\)"
+ 
+ if ${FLATPAK} ${U} install test-wrong-gpg-repo org.test.Hello 2> install-error-log; then
+     assert_not_reached "Should not be able to install with wrong gpg key"
+ fi
+-assert_file_has_content install-error-log "GPG signatures found, but none are in trusted keyring"
++assert_file_has_content install-error-log "\(GPG signatures found, but none are in trusted keyring\|public key not found\)"
+ 
+ echo "ok fail with wrong gpg key"
+ 
+-- 
+2.20.1
+

ci/flatpak.sh

@@ -22,6 +22,12 @@ tmpd=$(mktemp -d)
 cd ${tmpd}
 git clone --recursive --depth=1 -b ${FLATPAK_TAG} https://github.com/flatpak/flatpak
 cd ${tmpd}/flatpak
+
+# Some of flatpak's tests assert GPG error strings from ostree, but
+# those have been changed. Patch the test assertions until this can get
+# into a tagged flatpak.
+git apply ${codedir}/ci/flatpak-1.4.1-ostree-gpg-errors.patch
+
 # This is a copy of flatpak/ci/build.sh, but we can't use that as we want to install
 # our built ostree over it.
 pkg_install sudo which attr fuse bison \

src/libostree/ostree-gpg-verify-result.c

@@ -548,14 +548,10 @@ append_expire_info (GString *output_buffer,
                     gint64 exp_timestamp,
                     gboolean expired)
 {
-  g_autoptr(GDateTime) date_time_utc = NULL;
-  g_autoptr(GDateTime) date_time_local = NULL;
-  g_autofree char *formatted_date_time = NULL;
-
   if (line_prefix != NULL)
     g_string_append (output_buffer, line_prefix);
 
-  date_time_utc = g_date_time_new_from_unix_utc (exp_timestamp);
+  g_autoptr(GDateTime) date_time_utc = g_date_time_new_from_unix_utc (exp_timestamp);
   if (date_time_utc == NULL)
     {
       g_string_append_printf (output_buffer,
@@ -565,8 +561,8 @@ append_expire_info (GString *output_buffer,
       return;
     }
 
-  date_time_local = g_date_time_to_local (date_time_utc);
+  g_autoptr(GDateTime) date_time_local = g_date_time_to_local (date_time_utc);
-  formatted_date_time = g_date_time_format (date_time_local, "%c");
+  g_autofree char *formatted_date_time = g_date_time_format (date_time_local, "%c");
 
   if (expired)
     {
@@ -773,8 +769,58 @@ ostree_gpg_verify_result_require_valid_signature (OstreeGpgVerifyResult *result,
 
   if (ostree_gpg_verify_result_count_valid (result) == 0)
     {
-      g_set_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_MISSING_KEY,
+      /*
-                   "GPG signatures found, but none are in trusted keyring");
+       * Join the description of each failed signature for the error message.
+       * Only one error code can be returned, so if there was more than one
+       * signature, use the error of the last one under the assumption that
+       * it's the most recent and hopefully most likely to be made with a
+       * valid key.
+       */
+      gint code = OSTREE_GPG_ERROR_NO_SIGNATURE;
+      g_autoptr(GString) buffer = g_string_sized_new (256);
+      guint nsigs = ostree_gpg_verify_result_count_all (result);
+
+      if (nsigs == 0)
+        /* In case an empty result was passed in */
+        g_string_append (buffer, "No GPG signatures found");
+      else
+        {
+          for (int i = nsigs - 1; i >= 0; i--)
+            {
+              g_autoptr(GVariant) info = ostree_gpg_verify_result_get_all (result, i);
+              ostree_gpg_verify_result_describe_variant (info, buffer, "",
+                                                         OSTREE_GPG_SIGNATURE_FORMAT_DEFAULT);
+
+              if (i == nsigs - 1)
+                {
+                  gboolean key_missing, key_revoked, key_expired, sig_expired;
+                  g_variant_get_child (info, OSTREE_GPG_SIGNATURE_ATTR_KEY_MISSING,
+                                       "b", &key_missing);
+                  g_variant_get_child (info, OSTREE_GPG_SIGNATURE_ATTR_KEY_REVOKED,
+                                       "b", &key_revoked);
+                  g_variant_get_child (info, OSTREE_GPG_SIGNATURE_ATTR_KEY_EXPIRED,
+                                       "b", &key_expired);
+                  g_variant_get_child (info, OSTREE_GPG_SIGNATURE_ATTR_SIG_EXPIRED,
+                                       "b", &sig_expired);
+
+                  if (key_missing)
+                    code = OSTREE_GPG_ERROR_MISSING_KEY;
+                  else if (key_revoked)
+                    code = OSTREE_GPG_ERROR_REVOKED_KEY;
+                  else if (key_expired)
+                    code = OSTREE_GPG_ERROR_EXPIRED_KEY;
+                  else if (sig_expired)
+                    code = OSTREE_GPG_ERROR_EXPIRED_SIGNATURE;
+                  else
+                    /* Assume any other issue is a bad signature */
+                    code = OSTREE_GPG_ERROR_INVALID_SIGNATURE;
+                }
+            }
+        }
+
+      /* Strip any trailing newlines */
+      g_strchomp (buffer->str);
+      g_set_error_literal (error, OSTREE_GPG_ERROR, code, buffer->str);
       return FALSE;
     }
 

src/libostree/ostree-gpg-verify-result.h

@@ -159,6 +159,11 @@ gboolean ostree_gpg_verify_result_require_valid_signature (OstreeGpgVerifyResult
  * @OSTREE_GPG_ERROR_NO_SIGNATURE: A signature was expected, but not found.
  * @OSTREE_GPG_ERROR_INVALID_SIGNATURE: A signature was malformed.
  * @OSTREE_GPG_ERROR_MISSING_KEY: A signature was found, but was created with a key not in the configured keyrings.
+ * @OSTREE_GPG_ERROR_EXPIRED_SIGNATURE: A signature was expired. Since: 2019.7.
+ * @OSTREE_GPG_ERROR_EXPIRED_KEY: A signature was found, but the key used to
+ *   sign it has expired. Since: 2019.7.
+ * @OSTREE_GPG_ERROR_REVOKED_KEY: A signature was found, but the key used to
+ *   sign it has been revoked. Since: 2019.7.
  *
  * Errors returned by signature creation and verification operations in OSTree.
  * These may be returned by any API which creates or verifies signatures.
@@ -169,6 +174,9 @@ typedef enum {
   OSTREE_GPG_ERROR_NO_SIGNATURE = 0,
   OSTREE_GPG_ERROR_INVALID_SIGNATURE,
   OSTREE_GPG_ERROR_MISSING_KEY,
+  OSTREE_GPG_ERROR_EXPIRED_SIGNATURE,
+  OSTREE_GPG_ERROR_EXPIRED_KEY,
+  OSTREE_GPG_ERROR_REVOKED_KEY,
 } OstreeGpgError;
 
 /**

tests/gpg-verify-data/README.md

@@ -1,5 +1,7 @@
 This is a GPG config directory for use with the OstreeGpgVerifyResult
-test cases. The test data (`lgplv2`) is signed with a variety of valid
+test cases. The test data (`lgpl2`) is signed with a variety of valid
-and invalid GPG keys in a detached signature file (`lgplv2.sig`).
+and invalid GPG keys in a detached signature file (`lgpl2.sig`). In
+addition, each detached signature is available in a separate file
+(`lgpgl2.sig<N>`).
 
 The passphrase for all the keys is `redhat`.

tests/gpghome/revocations/key1.rev

@@ -0,0 +1,12 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+Comment: This is a revocation certificate
+
+iQE9BCABCgAnFiEEXmXedascUBhi1HY0f8oj2Ecs2voFAl0MvqgJHQBUZXN0aW5n
+AAoJEH/KI9hHLNr6/dQH/iPZjfJgFAc/TIR4xE4kB0yL4zdMqxgV1ef/atQDLEN4
+MBiqIltzb8WyG+cpNfNZgFmqXmCRN+1IAla9piixe76ZwOqcQ6S5MU/8nMcyMsD9
+edg+9sg0DH8SEzejVma3ZLfaJ/6ZpU7c6a4vCPNcRBC7PxAvAc0LnAN6KQYGU7GR
+gv2k/JsGYgvmUAajhVFy0jc4jGkhRBHMDksGGFdYY94RATFF4gWtlUXyRYMTXBCf
+eM3bxEeSMbU7lXCZg9zjoxP6XzJuNW1SLz3zL90GnO19uNh8Pf6pHmkCNTO/L1Ua
+Cc6fb3ubtdqgs6an84U/aod1VcK7BNASqZ2gYUsF2KE=
+=owvo
+-----END PGP PUBLIC KEY BLOCK-----

tests/gpghome/revocations/key2.rev

@@ -0,0 +1,12 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+Comment: This is a revocation certificate
+
+iQE9BCABCgAnFiEEezsQINdEeWh/2yJz2CKM/sqVDUEFAl0Mvs4JHQBUZXN0aW5n
+AAoJENgijP7KlQ1BzH4IAMUoTrW6XraDYq/d/b0qa3sZ1NTBPUXLp4gFaedZwKk/
+AKSUCK05RWRQO3HrSvmhdSUF6/9tFLGpbu7P56ihjAnq2vpzRyeNTEGQ02IzfCpM
+SJup0R6iA7KmjiDutDoEgjhAzxCKbnU71SQ3PmjyaQT1KCBCDJVptcY6HDbo0dRN
+vcjTfjtFqkPgqbHyXwGv3rlm4uctSVfACrOS/fKF4Q9Fs4prsUXjQpGLaTHdxhiL
+pMRCTfZ4DBEMwAY7s9FpMpIh9rdOwE+zkv5CsE0uJVZq0WW5r0CBDCta6Sopt6uk
+pIA+QHL9GPOrG2E3SJxyIBC37Sl40MGAJQ1djmecIGk=
+=0KEe
+-----END PGP PUBLIC KEY BLOCK-----

tests/gpghome/revocations/key3.rev

@@ -0,0 +1,12 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+Comment: This is a revocation certificate
+
+iQE9BCABCgAnFiEEfSnPBguCac32O/vdDRX6599ETWcFAl0MvuYJHQBUZXN0aW5n
+AAoJEA0V+uffRE1n/p8H/3mmSK2gtbxJ5sfu1z44ra82fLRAUupJzf53dAvvJCEK
+4RSJFtHYu+hoPVFd9bmToxo2YQWe67MMZW7cHtq9D/a755SYOrty//KpXsGS22W/
+ZGatBjl36zuE6BoR18Q6VAMgVBwovPSlSuCEW+Ro8JZYyA/LbA95AnMprNod6Jw9
+VSsGC39au5rUlhEOHLL1Iw3dl4blxa6tf/roljbXzaN+Qh2/ez7Cy532oocak2FL
+bbblBGrIdfYLAXpLqhnQk2vgEHZ+ZylvStBndpLWwEskXhmaHpW7+WapFhLCUOr+
+arzbc9XQ7ghhF9hSoKiToJqU5PRjaOex85BEDwE5gWY=
+=ykAF
+-----END PGP PUBLIC KEY BLOCK-----

tests/libtest.sh

@@ -78,16 +78,20 @@ unset TAR_OPTIONS
 # easily clean up.
 export OSTREE_SYSROOT_DEBUG=mutable-deployments
 
-export TEST_GPG_KEYID_1="472CDAFA"
+export TEST_GPG_KEYID_1="7FCA23D8472CDAFA"
-export TEST_GPG_KEYID_2="CA950D41"
+export TEST_GPG_KEYFPR_1="5E65DE75AB1C501862D476347FCA23D8472CDAFA"
-export TEST_GPG_KEYID_3="DF444D67"
+export TEST_GPG_KEYID_2="D8228CFECA950D41"
+export TEST_GPG_KEYFPR_2="7B3B1020D74479687FDB2273D8228CFECA950D41"
+export TEST_GPG_KEYID_3="0D15FAE7DF444D67"
+export TEST_GPG_KEYFPR_3="7D29CF060B8269CDF63BFBDD0D15FAE7DF444D67"
 
-# GPG when creating signatures demands a writable
+# GPG when creating signatures demands a private writable
 # homedir in order to create lockfiles.  Work around
 # this by copying locally.
 echo "Copying gpghome to ${test_tmpdir}"
 cp -a "${test_srcdir}/gpghome" ${test_tmpdir}
 chmod -R u+w "${test_tmpdir}"
+chmod 700 "${test_tmpdir}/gpghome"
 export TEST_GPG_KEYHOME=${test_tmpdir}/gpghome
 export OSTREE_GPG_HOME=${test_tmpdir}/gpghome/trusted
 

tests/test-gpg-verify-result.c

@@ -36,6 +36,18 @@
     } \
   } G_STMT_END
 
+#define assert_str_contains(s1, s2) \
+  G_STMT_START { \
+    const char *__s1 = (s1), *__s2 = (s2); \
+    if (strstr (__s1, __s2) == NULL) { \
+      g_autoptr(GString) string = g_string_new ("assertion failed (" #s1 " contains " #s2 "): "); \
+      g_autofree char *__es1 = g_strescape (__s1, NULL); \
+      g_autofree char *__es2 = g_strescape (__s2, NULL); \
+      g_string_append_printf (string, "(\"%s\", \"%s\")", __es1, __es2); \
+      g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, string->str); \
+    } \
+  } G_STMT_END
+
 typedef struct {
   OstreeGpgVerifyResult *result;
 } TestFixture;
@@ -53,12 +65,12 @@ static void
 test_fixture_setup (TestFixture *fixture,
                     gconstpointer user_data)
 {
+  const char * const *sig_files = user_data;
   gpgme_error_t gpg_error;
   gpgme_data_t data_buffer;
   gpgme_data_t signature_buffer;
   OstreeGpgVerifyResult *result;
   g_autofree char *homedir = NULL;
-  g_autofree char *filename = NULL;
   GError *local_error = NULL;
 
   /* Mimic what OstreeGpgVerifier does to create OstreeGpgVerifyResult.
@@ -74,15 +86,47 @@ test_fixture_setup (TestFixture *fixture,
                            NULL, &local_error, NULL);
   g_assert_no_error (local_error);
 
-  filename = g_build_filename (homedir, "lgpl2", NULL);
+  g_autofree char *data_filename = g_build_filename (homedir, "lgpl2", NULL);
-  gpg_error = gpgme_data_new_from_file (&data_buffer, filename, 1);
+  gpg_error = gpgme_data_new_from_file (&data_buffer, data_filename, 1);
-  assert_no_gpg_error (gpg_error, filename);
+  assert_no_gpg_error (gpg_error, data_filename);
 
-  g_clear_pointer (&filename, g_free);
+  if (sig_files == NULL)
+    {
+      /* No signature files specified, use full lgpl2.sig file */
+      g_autofree char *filename = g_build_filename (homedir, "lgpl2.sig", NULL);
+      gpg_error = gpgme_data_new_from_file (&signature_buffer, filename, 1);
+      assert_no_gpg_error (gpg_error, filename);
+    }
+  else
+    {
+      /* Read all the specified files into the signature buffer */
+      gpg_error = gpgme_data_new (&signature_buffer);
+      assert_no_gpg_error (gpg_error, NULL);
 
-  filename = g_build_filename (homedir, "lgpl2.sig", NULL);
+      for (const char * const *name = sig_files; *name != NULL; name++)
-  gpg_error = gpgme_data_new_from_file (&signature_buffer, filename, 1);
+        {
-  assert_no_gpg_error (gpg_error, filename);
+          g_autofree char *path = g_build_filename (homedir, *name, NULL);
+          g_autoptr(GFile) sig_file = g_file_new_for_path (path);
+
+          g_autofree char *contents = NULL;
+          gsize len;
+          g_assert_true (g_file_load_contents (sig_file, NULL, &contents,
+                                               &len, NULL, &local_error));
+          g_assert_no_error (local_error);
+
+          char *cur = contents;
+          while (len > 0)
+            {
+              ssize_t written = gpgme_data_write (signature_buffer, cur, len);
+              if (written == -1)
+                assert_no_gpg_error (gpgme_error_from_syserror (), path);
+              cur += written;
+              len -= written;
+            }
+        }
+
+      gpgme_data_seek (signature_buffer, 0, SEEK_SET);
+    }
 
   gpg_error = gpgme_op_verify (result->context,
                                signature_buffer, data_buffer, NULL);
@@ -115,7 +159,7 @@ test_check_counts (TestFixture *fixture,
   count_valid = ostree_gpg_verify_result_count_valid (fixture->result);
 
   g_assert_cmpint (count_all, ==, 5);
-  g_assert_cmpint (count_valid, ==, 2);
+  g_assert_cmpint (count_valid, ==, 1);
 }
 
 static void
@@ -123,7 +167,7 @@ test_signature_lookup (TestFixture *fixture,
                        gconstpointer user_data)
 {
   /* Checking the signature with the revoked key for this case. */
-  guint expected_signature_index = GPOINTER_TO_UINT (user_data);
+  guint expected_signature_index = 2;
 
   /* Lowercase letters to ensure OstreeGpgVerifyResult handles it. */
   const char *fingerprint = "68dcc2db4bec5811c2573590bd9d2a44b7f541a6";
@@ -215,7 +259,7 @@ static void
 test_valid_signature (TestFixture *fixture,
                       gconstpointer user_data)
 {
-  guint signature_index = GPOINTER_TO_UINT (user_data);
+  guint signature_index = 0;
   g_autoptr(GVariant) tuple = NULL;
   gboolean valid;
   gboolean sig_expired;
@@ -249,7 +293,7 @@ static void
 test_expired_key (TestFixture *fixture,
                   gconstpointer user_data)
 {
-  guint signature_index = GPOINTER_TO_UINT (user_data);
+  guint signature_index = 1;
   g_autoptr(GVariant) tuple = NULL;
   gboolean valid;
   gboolean sig_expired;
@@ -283,7 +327,7 @@ static void
 test_revoked_key (TestFixture *fixture,
                   gconstpointer user_data)
 {
-  guint signature_index = GPOINTER_TO_UINT (user_data);
+  guint signature_index = 2;
   g_autoptr(GVariant) tuple = NULL;
   gboolean valid;
   gboolean sig_expired;
@@ -317,7 +361,7 @@ static void
 test_missing_key (TestFixture *fixture,
                   gconstpointer user_data)
 {
-  guint signature_index = GPOINTER_TO_UINT (user_data);
+  guint signature_index = 3;
   g_autoptr(GVariant) tuple = NULL;
   gboolean valid;
   gboolean sig_expired;
@@ -351,7 +395,7 @@ static void
 test_expired_signature (TestFixture *fixture,
                         gconstpointer user_data)
 {
-  guint signature_index = GPOINTER_TO_UINT (user_data);
+  guint signature_index = 4;
   g_autoptr(GVariant) tuple = NULL;
   gboolean valid;
   gboolean sig_expired;
@@ -373,7 +417,7 @@ test_expired_signature (TestFixture *fixture,
                  &key_missing,
                  &key_exp_timestamp);
 
-  g_assert_true (valid);
+  g_assert_false (valid);
   g_assert_true (sig_expired);
   g_assert_false (key_expired);
   g_assert_false (key_revoked);
@@ -381,6 +425,83 @@ test_expired_signature (TestFixture *fixture,
   g_assert_cmpint (key_exp_timestamp, ==, 0);
 }
 
+static void
+test_require_valid_signature (TestFixture *fixture,
+                              gconstpointer user_data)
+{
+  GError *error = NULL;
+  gboolean res = ostree_gpg_verify_result_require_valid_signature (fixture->result,
+                                                                   &error);
+  g_assert_true (res);
+  g_assert_no_error (error);
+}
+
+static void
+test_require_valid_signature_expired_key (TestFixture *fixture,
+                                          gconstpointer user_data)
+{
+  GError *error = NULL;
+  gboolean res = ostree_gpg_verify_result_require_valid_signature (fixture->result,
+                                                                   &error);
+  g_assert_false (res);
+  g_assert_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_EXPIRED_KEY);
+  assert_str_contains (error->message, "Key expired");
+}
+
+static void
+test_require_valid_signature_revoked_key (TestFixture *fixture,
+                                          gconstpointer user_data)
+{
+  GError *error = NULL;
+  gboolean res = ostree_gpg_verify_result_require_valid_signature (fixture->result,
+                                                                   &error);
+  g_assert_false (res);
+  g_assert_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_REVOKED_KEY);
+  assert_str_contains (error->message, "Key revoked");
+}
+
+static void
+test_require_valid_signature_missing_key (TestFixture *fixture,
+                                          gconstpointer user_data)
+{
+  GError *error = NULL;
+  gboolean res = ostree_gpg_verify_result_require_valid_signature (fixture->result,
+                                                                   &error);
+  g_assert_false (res);
+  g_assert_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_MISSING_KEY);
+  assert_str_contains (error->message, "public key not found");
+}
+
+static void
+test_require_valid_signature_expired_signature (TestFixture *fixture,
+                                                gconstpointer user_data)
+{
+  GError *error = NULL;
+  gboolean res = ostree_gpg_verify_result_require_valid_signature (fixture->result,
+                                                                   &error);
+  g_assert_false (res);
+  g_assert_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_EXPIRED_SIGNATURE);
+  assert_str_contains (error->message, "Signature expired");
+}
+
+static void
+test_require_valid_signature_expired_missing_key (TestFixture *fixture,
+                                                  gconstpointer user_data)
+{
+  GError *error = NULL;
+  gboolean res = ostree_gpg_verify_result_require_valid_signature (fixture->result,
+                                                                   &error);
+  g_assert_false (res);
+
+  /*
+   * The error will be for the last signature, which is for a missing key, but
+   * the message should show both issues.
+   */
+  g_assert_error (error, OSTREE_GPG_ERROR, OSTREE_GPG_ERROR_MISSING_KEY);
+  assert_str_contains (error->message, "Key expired");
+  assert_str_contains (error->message, "public key not found");
+}
+
 int
 main (int argc, char **argv)
 {
@@ -397,7 +518,7 @@ main (int argc, char **argv)
 
   g_test_add ("/gpg-verify-result/signature-lookup",
               TestFixture,
-              GINT_TO_POINTER (2),
+              NULL,
               test_fixture_setup,
               test_signature_lookup,
               test_fixture_teardown);
@@ -411,38 +532,85 @@ main (int argc, char **argv)
 
   g_test_add ("/gpg-verify-result/valid-signature",
               TestFixture,
-              GINT_TO_POINTER (0),  /* signature index */
+              NULL,
               test_fixture_setup,
               test_valid_signature,
               test_fixture_teardown);
 
   g_test_add ("/gpg-verify-result/expired-key",
               TestFixture,
-              GINT_TO_POINTER (1),  /* signature index */
+              NULL,
               test_fixture_setup,
               test_expired_key,
               test_fixture_teardown);
 
   g_test_add ("/gpg-verify-result/revoked-key",
               TestFixture,
-              GINT_TO_POINTER (2),  /* signature index */
+              NULL,
               test_fixture_setup,
               test_revoked_key,
               test_fixture_teardown);
 
   g_test_add ("/gpg-verify-result/missing-key",
               TestFixture,
-              GINT_TO_POINTER (3),  /* signature index */
+              NULL,
               test_fixture_setup,
               test_missing_key,
               test_fixture_teardown);
 
   g_test_add ("/gpg-verify-result/expired-signature",
               TestFixture,
-              GINT_TO_POINTER (4),  /* signature index */
+              NULL,
               test_fixture_setup,
               test_expired_signature,
               test_fixture_teardown);
 
+  g_test_add ("/gpg-verify-result/require-valid-signature",
+              TestFixture,
+              NULL,
+              test_fixture_setup,
+              test_require_valid_signature,
+              test_fixture_teardown);
+
+  const char *expired_key_files[] = { "lgpl2.sig1", NULL };
+  g_test_add ("/gpg-verify-result/require-valid-signature-expired-key",
+              TestFixture,
+              expired_key_files,
+              test_fixture_setup,
+              test_require_valid_signature_expired_key,
+              test_fixture_teardown);
+
+  const char *revoked_key_files[] = { "lgpl2.sig2", NULL };
+  g_test_add ("/gpg-verify-result/require-valid-signature-revoked-key",
+              TestFixture,
+              revoked_key_files,
+              test_fixture_setup,
+              test_require_valid_signature_revoked_key,
+              test_fixture_teardown);
+
+  const char *missing_key_files[] = { "lgpl2.sig3", NULL };
+  g_test_add ("/gpg-verify-result/require-valid-signature-missing-key",
+              TestFixture,
+              missing_key_files,
+              test_fixture_setup,
+              test_require_valid_signature_missing_key,
+              test_fixture_teardown);
+
+  const char *expired_signature_files[] = { "lgpl2.sig4", NULL };
+  g_test_add ("/gpg-verify-result/require-valid-signature-expired-signature",
+              TestFixture,
+              expired_signature_files,
+              test_fixture_setup,
+              test_require_valid_signature_expired_signature,
+              test_fixture_teardown);
+
+  const char *expired_missing_key_files[] = { "lgpl2.sig1", "lgpl2.sig3", NULL };
+  g_test_add ("/gpg-verify-result/require-valid-signature-expired-missing-key",
+              TestFixture,
+              expired_missing_key_files,
+              test_fixture_setup,
+              test_require_valid_signature_expired_missing_key,
+              test_fixture_teardown);
+
   return g_test_run ();
 }

tests/test-pull-summary-sigs.sh

@@ -189,7 +189,7 @@ cp ${test_tmpdir}/ostree-srv/gnomerepo/summary.sig{.2,}
 if ${OSTREE} --repo=repo pull origin main 2>err.txt; then
     assert_not_reached "Successful pull with old summary"
 fi
-assert_file_has_content err.txt "none are in trusted keyring"
+assert_file_has_content err.txt "BAD signature"
 assert_has_file repo/tmp/cache/summaries/origin
 assert_has_file repo/tmp/cache/summaries/origin.sig
 cmp repo/tmp/cache/summaries/origin ${test_tmpdir}/ostree-srv/gnomerepo/summary.1 >&2

tests/test-remote-gpg-import.sh

@@ -28,7 +28,12 @@ unset OSTREE_GPG_HOME
 
 setup_fake_remote_repo1 "archive"
 
-echo "1..4"
+# Some tests require an appropriate gpg
+num_non_gpg_tests=4
+num_gpg_tests=2
+num_tests=$((num_non_gpg_tests + num_gpg_tests))
+
+echo "1..${num_tests}"
 
 cd ${test_tmpdir}
 mkdir repo
@@ -163,7 +168,7 @@ ${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key1.asc,${test_tmp
 if ${OSTREE} pull R8:main 2>err.txt; then
     assert_not_reached "Unexpectedly succeeded at pulling with different key"
 fi
-assert_file_has_content err.txt "GPG signatures found, but none are in trusted keyring"
+assert_file_has_content err.txt "public key not found"
 
 # Test gpgkeypath success with directory containing a valid key
 ${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/ R9 $(cat httpd-address)/ostree/gnomerepo
@@ -243,7 +248,7 @@ ${OSTREE} remote add --set=gpgkeypath=${test_tmpdir}/gpghome/key2.asc R6 $(cat h
 if ${OSTREE} pull R6:main 2>err.txt; then
     assert_not_reached "Unexpectedly succeeded at pulling with different key"
 fi
-assert_file_has_content err.txt "GPG signatures found, but none are in trusted keyring"
+assert_file_has_content err.txt "public key not found"
 
 echo "ok"
 
@@ -269,7 +274,7 @@ newrev=$(${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo rev-par
 if ${OSTREE} pull --require-static-deltas R1:main 2>err.txt; then
     assert_not_reached "Unexpectedly succeeded at pulling commit signed with untrusted key"
 fi
-assert_file_has_content err.txt "GPG signatures found, but none are in trusted keyring"
+assert_file_has_content err.txt "public key not found"
 
 echo "ok gpg untrusted signed commit for delta upgrades"
 
@@ -280,3 +285,57 @@ ${CMD_PREFIX} ostree --repo=${test_tmpdir}/ostree-srv/gnomerepo summary -u --gpg
 ${OSTREE} pull --require-static-deltas R1:main
 
 echo "ok gpg trusted signed commit for delta upgrades"
+
+# Run some more tests if an appropriate gpg is available
+GPG=$(which_gpg)
+if [ -z "${GPG}" ]; then
+  # Print a skip message per skipped test
+  for (( i = 0; i < num_gpg_tests; i++ )); do
+    echo "ok # SKIP this test requires gpg"
+  done
+else
+  # Create a commit signed with keyid 1
+  echo $(date) > workdir/testfile-for-key-mangling
+  ${CMD_PREFIX} ostree  --repo=${test_tmpdir}/ostree-srv/gnomerepo commit -b main --gpg-sign ${TEST_GPG_KEYID_1} --gpg-homedir ${test_tmpdir}/gpghome workdir
+
+  # Re-add the remote
+  ${OSTREE} remote delete R1
+  ${OSTREE} remote add --gpg-import ${test_tmpdir}/gpghome/key1.asc R1 $(cat httpd-address)/ostree/gnomerepo | grep -o 'Imported [[:digit:]] GPG key' > result
+  assert_file_has_content result 'Imported 1 GPG key'
+
+  # Expire key 1, wait for it to be expired and import the expired key. Only
+  # new keys are reported.
+  ${GPG} --homedir=${test_tmpdir}/gpghome --quick-set-expire ${TEST_GPG_KEYFPR_1} seconds=1
+  sleep 2
+  ${GPG} --homedir=${test_tmpdir}/gpghome --armor --export ${TEST_GPG_KEYID_1} > ${test_tmpdir}/key1expired.asc
+  ${OSTREE} remote gpg-import --keyring ${test_tmpdir}/key1expired.asc R1 | grep -o 'Imported [[:digit:]] GPG key' > result
+  assert_file_has_content result 'Imported 0 GPG key'
+
+  # Pulling should fail since the key is expired
+  rm repo/refs/remotes/* -rf
+  ${OSTREE} prune --refs-only
+  if ${OSTREE} pull R1:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling commit signed with expired key"
+  fi
+  assert_file_has_content err.txt "Key expired"
+
+  echo "ok imported expired key"
+
+  # Unexpire keyid 1 and revoke it. Revoking is done by importing the
+  # pre-generated revocation certificate.
+  ${GPG} --homedir=${test_tmpdir}/gpghome --quick-set-expire ${TEST_GPG_KEYFPR_1} seconds=0
+  ${GPG} --homedir=${TEST_GPG_KEYHOME} --import ${TEST_GPG_KEYHOME}/revocations/key1.rev
+  ${GPG} --homedir=${test_tmpdir}/gpghome --armor --export ${TEST_GPG_KEYID_1} > ${test_tmpdir}/key1revoked.asc
+  ${OSTREE} remote gpg-import --keyring ${test_tmpdir}/key1revoked.asc R1 | grep -o 'Imported [[:digit:]] GPG key' > result
+  assert_file_has_content result 'Imported 0 GPG key'
+
+  # Pulling should fail since the key is revoked
+  rm repo/refs/remotes/* -rf
+  ${OSTREE} prune --refs-only
+  if ${OSTREE} pull R1:main 2>err.txt; then
+    assert_not_reached "Unexpectedly succeeded at pulling commit signed with revoked key"
+  fi
+  assert_file_has_content err.txt "Key revoked"
+
+  echo "ok imported revoked key"
+fi