From 74c4068b9da7f2dff16a8ee2605d3350a6a782f6 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Thu, 2 Feb 2017 15:41:29 -0500
Subject: [PATCH] core: Do GPG verification before importing

While reading a recent conversation about GPG checking at treecompose
time, I had a sudden thought - were we actually doing verification
client side?  Turned out, we aren't.  That happens as part of
`dnf_transaction_commit()` which we don't use.

That function verifies every package at one go, but for us I think it's better
to do it before "importing". We shouldn't have untrusted bits that we've
unpacked (they might have suid binaries, for one thing).

This is an embarassing problem, but it's worth emphasizing that everyone should
be retrieving repodata at a minimum over TLS, which sets a baseline. On RHEL, we
already do pinned TLS, and there are discussions about extending that elsewhere.
---
 libdnf                             |  2 +-
 src/libpriv/rpmostree-core.c       | 12 +++++++++++-
 tests/common/libvm.sh              | 16 ++++++++++++++++
 tests/vmcheck/test-layering-gpg.sh | 37 +++++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 2 deletions(-)
 create mode 100755 tests/vmcheck/test-layering-gpg.sh

diff --git a/src/libpriv/rpmostree-core.c b/src/libpriv/rpmostree-core.c
index 2daaa48..0b4a3ab 100644
--- a/src/libpriv/rpmostree-core.c
+++ b/src/libpriv/rpmostree-core.c
@@ -1152,18 +1152,25 @@ import_one_package (RpmOstreeContext *self,
   g_autofree char *ostree_commit = NULL;
   glnx_unref_object RpmOstreeUnpacker *unpacker = NULL;
   g_autofree char *pkg_path;
+  DnfRepo *pkg_repo;
   int flags = 0;
 
+  pkg_repo = dnf_package_get_repo (pkg);
+
   if (pkg_is_local (pkg))
     pkg_path = g_strdup (dnf_package_get_filename (pkg));
   else
     {
       const char *pkg_location = dnf_package_get_location (pkg);
       pkg_path =
-        g_build_filename (dnf_repo_get_location (dnf_package_get_repo (pkg)),
+        g_build_filename (dnf_repo_get_location (pkg_repo),
                           "packages", glnx_basename (pkg_location), NULL);
     }
 
+  /* Verify signatures if enabled */
+  if (!dnf_transaction_gpgcheck_package (dnf_context_get_transaction (hifctx), pkg, error))
+    goto out;
+
   flags = RPMOSTREE_UNPACKER_FLAGS_OSTREE_CONVENTION;
   if (self->unprivileged)
     flags |= RPMOSTREE_UNPACKER_FLAGS_UNPRIVILEGED;
@@ -1220,6 +1227,9 @@ rpmostree_context_import (RpmOstreeContext *self,
 
   g_return_val_if_fail (get_pkgcache_repo (self) != NULL, FALSE);
 
+  if (!dnf_transaction_import_keys (dnf_context_get_transaction (hifctx), error))
+    goto out;
+
   {
     glnx_unref_object DnfState *hifstate = dnf_state_new ();
     dnf_state_set_number_steps (hifstate, install->packages_to_import->len);
diff --git a/tests/common/libvm.sh b/tests/common/libvm.sh
index 3b7cb35..bbbcba2 100644
--- a/tests/common/libvm.sh
+++ b/tests/common/libvm.sh
@@ -51,6 +51,11 @@ vm_cmd() {
   $SSH "$@"
 }
 
+# Delete anything which we might change between runs
+vm_clean_caches() {
+    vm_cmd rm /ostree/repo/extensions/rpmostree/pkgcache/refs/heads/* -rf
+}
+
 # run rpm-ostree in vm
 # - $@    args
 vm_rpmostree() {
@@ -68,6 +73,7 @@ vm_send() {
 
 # copy the test repo to the vm
 vm_send_test_repo() {
+  gpgcheck=${1:-0}
   vm_cmd rm -rf /tmp/vmcheck
   vm_send /tmp/vmcheck ${commondir}/compose/yum/repo
 
@@ -77,6 +83,16 @@ name=test-repo
 baseurl=file:///tmp/vmcheck/repo
 EOF
 
+  if [ $gpgcheck -eq 1 ]; then
+      cat >> vmcheck.repo <<EOF
+gpgcheck=1
+gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-25-primary
+EOF
+  else
+      echo "Enabling vmcheck.repo without GPG"
+      echo 'gpgcheck=0' >> vmcheck.repo
+  fi
+
   vm_send /etc/yum.repos.d vmcheck.repo
 }
 
diff --git a/tests/vmcheck/test-layering-gpg.sh b/tests/vmcheck/test-layering-gpg.sh
new file mode 100755
index 0000000..6b66a79
--- /dev/null
+++ b/tests/vmcheck/test-layering-gpg.sh
@@ -0,0 +1,37 @@
+#!/bin/bash
+#
+# Copyright (C) 2017 Red Hat, 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.
+
+set -e
+
+. ${commondir}/libtest.sh
+. ${commondir}/libvm.sh
+
+set -x
+
+vm_send_test_repo 1
+vm_clean_caches
+
+# make sure the package is not already layered
+vm_assert_layered_pkg foo absent
+
+if vm_rpmostree pkg-add foo-1.0 2>err.txt; then
+    assert_not_reached "Installed unsigned package"
+fi
+assert_file_has_content err.txt 'package not signed: foo'
+echo "ok failed to install unsigned package"
-- 
2.9.3