Blob Blame History Raw
From 1f63621d098741158b5e1e7158cc570a415d88cd Mon Sep 17 00:00:00 2001
From: Panu Matilainen <pmatilai@redhat.com>
Date: Mon, 29 Nov 2021 14:01:39 +0200
Subject: [PATCH] Fix IMA signature lengths assumed constant (#1833,
 RhBug:2018937)

At least ECDSA and RSA signatures can vary in length, but the IMA code
assumes constant lengths and thus may either place invalid signatures on
disk from either truncating or overshooting, and segfault if the stars are
just so.

Luckily the signatures are stored as strings so we can calculate the
actual lengths at runtime and ignore the stored constant length info.
Extend hex2bin() to optionally calculate the lengths and maximum,
and use these for returning IMA data from the rpmfi(les) API.

Additionally update the signing code to store the largest IMA signature
length rather than what happened to be last to be on the safe side.
We can't rely on this value due to invalid packages being out there,
but then we need to calculate the lengths on rpmfiles populate so there's
not a lot to gain anyhow.

Fixes: #1833

Backported for 4.16.1.3 and combined with:
31e9daf823f7052135d1decc0802b6fa775a88c5 (fix-up)
0c1ad364d65c4144ff71c376e0b49fbc322b686d (python bindings)

Note that the test case has been removed due to it including a binary
file (test package) for which we'd have to use -Sgit with %autopatch and
thus depend on git-core at build time.  Nevertheless, we do have this BZ
covered in our internal test suite, so no need for it anyway.
---
 lib/rpmfi.c          | 59 +++++++++++++++++++++++++++++++++-----------
 python/rpmfiles-py.c | 18 ++++++++++++++
 sign/rpmsignfiles.c  |  5 +++-
 3 files changed, 67 insertions(+), 15 deletions(-)

diff --git a/lib/rpmfi.c b/lib/rpmfi.c
index af428468c..ed8927fd5 100644
--- a/lib/rpmfi.c
+++ b/lib/rpmfi.c
@@ -115,7 +115,8 @@ struct rpmfiles_s {
     struct fingerPrint_s * fps;	/*!< File fingerprint(s). */
 
     int digestalgo;		/*!< File digest algorithm */
-    int signaturelength;	/*!< File signature length */
+    int *signaturelengths;	/*!< File signature lengths */
+    int signaturemaxlen;	/*!< Largest file signature length */
     unsigned char * digests;	/*!< File digests in binary. */
     unsigned char * signatures; /*!< File signatures in binary. */
 
@@ -575,9 +576,9 @@ const unsigned char * rpmfilesFSignature(rpmfiles fi, int ix, size_t *len)
 
     if (fi != NULL && ix >= 0 && ix < rpmfilesFC(fi)) {
 	if (fi->signatures != NULL)
-	    signature = fi->signatures + (fi->signaturelength * ix);
+	    signature = fi->signatures + (fi->signaturemaxlen * ix);
 	if (len)
-	    *len = fi->signaturelength;
+	    *len = fi->signaturelengths ? fi->signaturelengths[ix] : 0;
     }
     return signature;
 }
@@ -1257,6 +1258,7 @@ rpmfiles rpmfilesFree(rpmfiles fi)
 	fi->flangs = _free(fi->flangs);
 	fi->digests = _free(fi->digests);
 	fi->signatures = _free(fi->signatures);
+	fi->signaturelengths = _free(fi->signaturelengths);
 	fi->fcaps = _free(fi->fcaps);
 
 	fi->cdict = _free(fi->cdict);
@@ -1486,23 +1488,52 @@ err:
 }
 
 /* Convert a tag of hex strings to binary presentation */
-static uint8_t *hex2bin(Header h, rpmTagVal tag, rpm_count_t num, size_t len)
+/* If lengths is non-NULL, assume variable length strings */
+static uint8_t *hex2bin(Header h, rpmTagVal tag, rpm_count_t num, size_t len,
+			int **lengths, int *maxlen)
 {
     struct rpmtd_s td;
     uint8_t *bin = NULL;
 
     if (headerGet(h, tag, &td, HEADERGET_MINMEM) && rpmtdCount(&td) == num) {
-	uint8_t *t = bin = xmalloc(num * len);
 	const char *s;
+	int maxl = 0;
+	int *lens = NULL;
+
+	/* Figure string sizes + max length for allocation purposes */
+	if (lengths) {
+	    int i = 0;
+	    lens = xmalloc(num * sizeof(*lens));
+
+	    while ((s = rpmtdNextString(&td))) {
+		lens[i] = strlen(s) / 2;
+		if (lens[i] > maxl)
+		    maxl = lens[i];
+		i++;
+	    }
+
+	    *lengths = lens;
+	    *maxlen = maxl;
+
+	    /* Reinitialize iterator for next round */
+	    rpmtdInit(&td);
+	} else {
+	    maxl = len;
+	}
 
+	uint8_t *t = bin = xmalloc(num * maxl);
+	int i = 0;
 	while ((s = rpmtdNextString(&td))) {
 	    if (*s == '\0') {
-		memset(t, 0, len);
-		t += len;
-		continue;
+		memset(t, 0, maxl);
+	    } else {
+		if (lens)
+		    len = lens[i];
+		for (int j = 0; j < len; j++, s += 2)
+		    t[j] = (rnibble(s[0]) << 4) | rnibble(s[1]);
 	    }
-	    for (int j = 0; j < len; j++, t++, s += 2)
-		*t = (rnibble(s[0]) << 4) | rnibble(s[1]);
+	    t += maxl;
+	    i++;
 	}
     }
     rpmtdFreeData(&td);
@@ -1570,15 +1601,15 @@ static int rpmfilesPopulate(rpmfiles fi, Header h, rpmfiFlags flags)
     /* grab hex digests from header and store in binary format */
     if (!(flags & RPMFI_NOFILEDIGESTS)) {
 	size_t diglen = rpmDigestLength(fi->digestalgo);
-	fi->digests = hex2bin(h, RPMTAG_FILEDIGESTS, totalfc, diglen);
+	fi->digests = hex2bin(h, RPMTAG_FILEDIGESTS, totalfc, diglen,
+				NULL, NULL);
     }
 
     fi->signatures = NULL;
     /* grab hex signatures from header and store in binary format */
     if (!(flags & RPMFI_NOFILESIGNATURES)) {
-	fi->signaturelength = headerGetNumber(h, RPMTAG_FILESIGNATURELENGTH);
-	fi->signatures = hex2bin(h, RPMTAG_FILESIGNATURES,
-				 totalfc, fi->signaturelength);
+	fi->signatures = hex2bin(h, RPMTAG_FILESIGNATURES, totalfc, 0,
+				&fi->signaturelengths, &fi->signaturemaxlen);
     }
 
     /* XXX TR_REMOVED doesn;t need fmtimes, frdevs, finodes */
diff --git a/python/rpmfiles-py.c b/python/rpmfiles-py.c
index 27666021d..48189a0ac 100644
--- a/python/rpmfiles-py.c
+++ b/python/rpmfiles-py.c
@@ -152,6 +152,22 @@ static PyObject *rpmfile_digest(rpmfileObject *s)
     Py_RETURN_NONE;
 }
 
+static PyObject *bytebuf(const unsigned char *buf, size_t len)
+{
+    if (buf) {
+	PyObject *o = PyBytes_FromStringAndSize((const char *)buf, len);
+	return o;
+    }
+    Py_RETURN_NONE;
+}
+
+static PyObject *rpmfile_imasig(rpmfileObject *s)
+{
+    size_t len = 0;
+    const unsigned char *sig = rpmfilesFSignature(s->files, s->ix, &len);
+    return bytebuf(sig, len);
+}
+
 static PyObject *rpmfile_class(rpmfileObject *s)
 {
     return utf8FromString(rpmfilesFClass(s->files, s->ix));
@@ -278,6 +294,8 @@ static PyGetSetDef rpmfile_getseters[] = {
       "language the file provides (typically for doc files)" },
     { "caps",		(getter) rpmfile_caps,		NULL,
       "file capabilities" },
+    { "imasig",	(getter) rpmfile_imasig,	NULL,
+      "IMA signature" },
     { NULL, NULL, NULL, NULL }
 };
 
diff --git a/sign/rpmsignfiles.c b/sign/rpmsignfiles.c
index b143c5b9b..6f39db6be 100644
--- a/sign/rpmsignfiles.c
+++ b/sign/rpmsignfiles.c
@@ -98,8 +98,9 @@ rpmRC rpmSignFiles(Header sigh, Header h, const char *key, char *keypass)
     td.count = 1;
 
     while (rpmfiNext(fi) >= 0) {
+	uint32_t slen;
 	digest = rpmfiFDigest(fi, NULL, NULL);
-	signature = signFile(algoname, digest, diglen, key, keypass, &siglen);
+	signature = signFile(algoname, digest, diglen, key, keypass, &slen);
 	if (!signature) {
 	    rpmlog(RPMLOG_ERR, _("signFile failed\n"));
 	    goto exit;
@@ -110,6 +111,8 @@ rpmRC rpmSignFiles(Header sigh, Header h, const char *key, char *keypass)
 	    goto exit;
 	}
 	signature = _free(signature);
+	if (slen > siglen)
+	    siglen = slen;
     }
 
     if (siglen > 0) {
-- 
2.33.1