richardphibel / rpms / rpm

Forked from rpms/rpm 2 years ago
Clone
83a7c7
From df089e178da0918dc74a8572a99324b0987bce30 Mon Sep 17 00:00:00 2001
83a7c7
Message-Id: <df089e178da0918dc74a8572a99324b0987bce30.1554983206.git.pmatilai@redhat.com>
83a7c7
In-Reply-To: <2ec0832287bd1443ebf336f8a98293f30bfa2036.1554983205.git.pmatilai@redhat.com>
83a7c7
References: <2ec0832287bd1443ebf336f8a98293f30bfa2036.1554983205.git.pmatilai@redhat.com>
83a7c7
From: Panu Matilainen <pmatilai@redhat.com>
83a7c7
Date: Mon, 18 Mar 2019 15:56:34 +0200
83a7c7
Subject: [PATCH 3/3] Verify packages before signing (RhBug:1646388)
83a7c7
83a7c7
Permitting corrupted packages to be signed is bad business for everybody
83a7c7
involved, this is something we should've always done. Besides being an
83a7c7
actual security risk, it can lead to odd results with verification
83a7c7
especially with the payload digest on signed packages.
83a7c7
83a7c7
One point worth noting is that this means that pre 4.14-packages cannot
83a7c7
be signed in FIPS mode now because there's no way to validate the package
83a7c7
payload range due to MD5 being disabled. This seems like a feature and
83a7c7
not a limitation, so disabler for the verify step intentionally left out.
83a7c7
83a7c7
Optimally we'd verify the package on the same read that's passed
83a7c7
to gpg but for simplicitys sake that's left as an future exercise,
83a7c7
now we simply read the package twice.
83a7c7
---
83a7c7
 sign/rpmgensig.c   | 32 ++++++++++++++++++++++++++++++++
83a7c7
 tests/rpmsigdig.at | 20 ++++++++++++++++++++
83a7c7
 2 files changed, 52 insertions(+)
83a7c7
83a7c7
diff --git a/sign/rpmgensig.c b/sign/rpmgensig.c
83a7c7
index 2bcbab768..5be542001 100644
83a7c7
--- a/sign/rpmgensig.c
83a7c7
+++ b/sign/rpmgensig.c
83a7c7
@@ -21,6 +21,7 @@
83a7c7
 
83a7c7
 #include "lib/rpmlead.h"
83a7c7
 #include "lib/signature.h"
83a7c7
+#include "lib/rpmvs.h"
83a7c7
 #include "sign/rpmsignfiles.h"
83a7c7
 
83a7c7
 #include "debug.h"
83a7c7
@@ -489,6 +490,31 @@ static rpmRC includeFileSignatures(Header *sigp, Header *hdrp)
83a7c7
 #endif
83a7c7
 }
83a7c7
 
83a7c7
+static int msgCb(struct rpmsinfo_s *sinfo, void *cbdata)
83a7c7
+{
83a7c7
+    char **msg = cbdata;
83a7c7
+    if (sinfo->rc && *msg == NULL)
83a7c7
+	*msg = rpmsinfoMsg(sinfo);
83a7c7
+    return (sinfo->rc != RPMRC_FAIL);
83a7c7
+}
83a7c7
+
83a7c7
+/* Require valid digests on entire package for signing. */
83a7c7
+static int checkPkg(FD_t fd, char **msg)
83a7c7
+{
83a7c7
+    int rc;
83a7c7
+    struct rpmvs_s *vs = rpmvsCreate(RPMSIG_DIGEST_TYPE, 0, NULL);
83a7c7
+    off_t offset = Ftell(fd);
83a7c7
+
83a7c7
+    Fseek(fd, 0, SEEK_SET);
83a7c7
+    rc = rpmpkgRead(vs, fd, NULL, NULL, msg);
83a7c7
+    if (!rc)
83a7c7
+	rc = rpmvsVerify(vs, RPMSIG_DIGEST_TYPE, msgCb, msg);
83a7c7
+    Fseek(fd, offset, SEEK_SET);
83a7c7
+
83a7c7
+    rpmvsFree(vs);
83a7c7
+    return rc;
83a7c7
+}
83a7c7
+
83a7c7
 /** \ingroup rpmcli
83a7c7
  * Create/modify elements in signature header.
83a7c7
  * @param rpm		path to package
83a7c7
@@ -519,6 +545,12 @@ static int rpmSign(const char *rpm, int deleting, int signfiles)
83a7c7
     if (manageFile(&fd, rpm, O_RDWR))
83a7c7
 	goto exit;
83a7c7
 
83a7c7
+    /* Ensure package is intact before attempting to sign */
83a7c7
+    if ((rc = checkPkg(fd, &msg))) {
83a7c7
+	rpmlog(RPMLOG_ERR, "not signing corrupt package %s: %s\n", rpm, msg);
83a7c7
+	goto exit;
83a7c7
+    }
83a7c7
+
83a7c7
     if ((rc = rpmLeadRead(fd, &msg)) != RPMRC_OK) {
83a7c7
 	rpmlog(RPMLOG_ERR, "%s: %s\n", rpm, msg);
83a7c7
 	goto exit;
83a7c7
diff --git a/tests/rpmsigdig.at b/tests/rpmsigdig.at
83a7c7
index 413c3d2c8..e93420306 100644
83a7c7
--- a/tests/rpmsigdig.at
83a7c7
+++ b/tests/rpmsigdig.at
83a7c7
@@ -472,3 +472,23 @@ run rpmsign --key-id 1964C5FC --addsign "${RPMTEST}"/tmp/hello-2.0-1.x86_64-sign
83a7c7
 [],
83a7c7
 [])
83a7c7
 AT_CLEANUP
83a7c7
+
83a7c7
+AT_SETUP([rpmsign --addsign <corrupted>])
83a7c7
+AT_KEYWORDS([rpmsign signature])
83a7c7
+AT_CHECK([
83a7c7
+RPMDB_CLEAR
83a7c7
+RPMDB_INIT
83a7c7
+rm -rf "${TOPDIR}"
83a7c7
+
83a7c7
+pkg="hello-2.0-1.x86_64.rpm"
83a7c7
+cp "${RPMTEST}"/data/RPMS/${pkg} "${RPMTEST}"/tmp/${pkg}
83a7c7
+dd if=/dev/zero of="${RPMTEST}"/tmp/${pkg} \
83a7c7
+   conv=notrunc bs=1 seek=333 count=4 2> /dev/null
83a7c7
+run rpmsign --key-id 1964C5FC --addsign "${RPMTEST}/tmp/${pkg}"
83a7c7
+],
83a7c7
+[1],
83a7c7
+[/home/pmatilai/repos/rpm/tests/testing/tmp/hello-2.0-1.x86_64.rpm:
83a7c7
+],
83a7c7
+[error: not signing corrupt package /home/pmatilai/repos/rpm/tests/testing/tmp/hello-2.0-1.x86_64.rpm: MD5 digest: BAD (Expected 007ca1d8b35cca02a1854ba301c5432e != 137ca1d8b35cca02a1854ba301c5432e)
83a7c7
+])
83a7c7
+AT_CLEANUP
83a7c7
-- 
83a7c7
2.20.1
83a7c7