Michal Domonkos 43a9b0
From 1f63621d098741158b5e1e7158cc570a415d88cd Mon Sep 17 00:00:00 2001
Michal Domonkos e2e5e2
From: Panu Matilainen <pmatilai@redhat.com>
Michal Domonkos e2e5e2
Date: Mon, 29 Nov 2021 14:01:39 +0200
Michal Domonkos e2e5e2
Subject: [PATCH] Fix IMA signature lengths assumed constant (#1833,
Michal Domonkos e2e5e2
 RhBug:2018937)
Michal Domonkos e2e5e2
Michal Domonkos e2e5e2
At least ECDSA and RSA signatures can vary in length, but the IMA code
Michal Domonkos e2e5e2
assumes constant lengths and thus may either place invalid signatures on
Michal Domonkos e2e5e2
disk from either truncating or overshooting, and segfault if the stars are
Michal Domonkos e2e5e2
just so.
Michal Domonkos e2e5e2
Michal Domonkos e2e5e2
Luckily the signatures are stored as strings so we can calculate the
Michal Domonkos e2e5e2
actual lengths at runtime and ignore the stored constant length info.
Michal Domonkos e2e5e2
Extend hex2bin() to optionally calculate the lengths and maximum,
Michal Domonkos e2e5e2
and use these for returning IMA data from the rpmfi(les) API.
Michal Domonkos e2e5e2
Michal Domonkos e2e5e2
Additionally update the signing code to store the largest IMA signature
Michal Domonkos e2e5e2
length rather than what happened to be last to be on the safe side.
Michal Domonkos e2e5e2
We can't rely on this value due to invalid packages being out there,
Michal Domonkos e2e5e2
but then we need to calculate the lengths on rpmfiles populate so there's
Michal Domonkos e2e5e2
not a lot to gain anyhow.
Michal Domonkos e2e5e2
Michal Domonkos e2e5e2
Fixes: #1833
Michal Domonkos e2e5e2
Michal Domonkos 43a9b0
Backported for 4.16.1.3 and combined with:
Michal Domonkos 43a9b0
31e9daf823f7052135d1decc0802b6fa775a88c5 (fix-up)
Michal Domonkos 43a9b0
0c1ad364d65c4144ff71c376e0b49fbc322b686d (python bindings)
Michal Domonkos 43a9b0
Michal Domonkos 43a9b0
Note that the test case has been removed due to it including a binary
Michal Domonkos 43a9b0
file (test package) for which we'd have to use -Sgit with %autopatch and
Michal Domonkos 43a9b0
thus depend on git-core at build time.  Nevertheless, we do have this BZ
Michal Domonkos 43a9b0
covered in our internal test suite, so no need for it anyway.
Michal Domonkos e2e5e2
---
Michal Domonkos 43a9b0
 lib/rpmfi.c          | 59 +++++++++++++++++++++++++++++++++-----------
Michal Domonkos 43a9b0
 python/rpmfiles-py.c | 18 ++++++++++++++
Michal Domonkos 43a9b0
 sign/rpmsignfiles.c  |  5 +++-
Michal Domonkos 43a9b0
 3 files changed, 67 insertions(+), 15 deletions(-)
Michal Domonkos e2e5e2
Michal Domonkos e2e5e2
diff --git a/lib/rpmfi.c b/lib/rpmfi.c
Michal Domonkos 43a9b0
index af428468c..ed8927fd5 100644
Michal Domonkos e2e5e2
--- a/lib/rpmfi.c
Michal Domonkos e2e5e2
+++ b/lib/rpmfi.c
Michal Domonkos e2e5e2
@@ -115,7 +115,8 @@ struct rpmfiles_s {
Michal Domonkos e2e5e2
     struct fingerPrint_s * fps;	/*!< File fingerprint(s). */
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
     int digestalgo;		/*!< File digest algorithm */
Michal Domonkos e2e5e2
-    int signaturelength;	/*!< File signature length */
Michal Domonkos 43a9b0
+    int *signaturelengths;	/*!< File signature lengths */
Michal Domonkos e2e5e2
+    int signaturemaxlen;	/*!< Largest file signature length */
Michal Domonkos e2e5e2
     unsigned char * digests;	/*!< File digests in binary. */
Michal Domonkos e2e5e2
     unsigned char * signatures; /*!< File signatures in binary. */
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
@@ -575,9 +576,9 @@ const unsigned char * rpmfilesFSignature(rpmfiles fi, int ix, size_t *len)
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
     if (fi != NULL && ix >= 0 && ix < rpmfilesFC(fi)) {
Michal Domonkos e2e5e2
 	if (fi->signatures != NULL)
Michal Domonkos e2e5e2
-	    signature = fi->signatures + (fi->signaturelength * ix);
Michal Domonkos e2e5e2
+	    signature = fi->signatures + (fi->signaturemaxlen * ix);
Michal Domonkos e2e5e2
 	if (len)
Michal Domonkos e2e5e2
-	    *len = fi->signaturelength;
Michal Domonkos 43a9b0
+	    *len = fi->signaturelengths ? fi->signaturelengths[ix] : 0;
Michal Domonkos e2e5e2
     }
Michal Domonkos e2e5e2
     return signature;
Michal Domonkos e2e5e2
 }
Michal Domonkos e2e5e2
@@ -1257,6 +1258,7 @@ rpmfiles rpmfilesFree(rpmfiles fi)
Michal Domonkos e2e5e2
 	fi->flangs = _free(fi->flangs);
Michal Domonkos e2e5e2
 	fi->digests = _free(fi->digests);
Michal Domonkos e2e5e2
 	fi->signatures = _free(fi->signatures);
Michal Domonkos e2e5e2
+	fi->signaturelengths = _free(fi->signaturelengths);
Michal Domonkos e2e5e2
 	fi->fcaps = _free(fi->fcaps);
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
 	fi->cdict = _free(fi->cdict);
Michal Domonkos 43a9b0
@@ -1486,23 +1488,52 @@ err:
Michal Domonkos e2e5e2
 }
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
 /* Convert a tag of hex strings to binary presentation */
Michal Domonkos e2e5e2
-static uint8_t *hex2bin(Header h, rpmTagVal tag, rpm_count_t num, size_t len)
Michal Domonkos e2e5e2
+/* If lengths is non-NULL, assume variable length strings */
Michal Domonkos e2e5e2
+static uint8_t *hex2bin(Header h, rpmTagVal tag, rpm_count_t num, size_t len,
Michal Domonkos e2e5e2
+			int **lengths, int *maxlen)
Michal Domonkos e2e5e2
 {
Michal Domonkos e2e5e2
     struct rpmtd_s td;
Michal Domonkos e2e5e2
     uint8_t *bin = NULL;
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
     if (headerGet(h, tag, &td, HEADERGET_MINMEM) && rpmtdCount(&td) == num) {
Michal Domonkos e2e5e2
-	uint8_t *t = bin = xmalloc(num * len);
Michal Domonkos e2e5e2
 	const char *s;
Michal Domonkos 43a9b0
+	int maxl = 0;
Michal Domonkos 43a9b0
+	int *lens = NULL;
Michal Domonkos 43a9b0
+
Michal Domonkos e2e5e2
+	/* Figure string sizes + max length for allocation purposes */
Michal Domonkos e2e5e2
+	if (lengths) {
Michal Domonkos e2e5e2
+	    int i = 0;
Michal Domonkos 43a9b0
+	    lens = xmalloc(num * sizeof(*lens));
Michal Domonkos e2e5e2
+
Michal Domonkos e2e5e2
+	    while ((s = rpmtdNextString(&td))) {
Michal Domonkos e2e5e2
+		lens[i] = strlen(s) / 2;
Michal Domonkos e2e5e2
+		if (lens[i] > maxl)
Michal Domonkos e2e5e2
+		    maxl = lens[i];
Michal Domonkos e2e5e2
+		i++;
Michal Domonkos e2e5e2
+	    }
Michal Domonkos e2e5e2
+
Michal Domonkos e2e5e2
+	    *lengths = lens;
Michal Domonkos e2e5e2
+	    *maxlen = maxl;
Michal Domonkos e2e5e2
+
Michal Domonkos e2e5e2
+	    /* Reinitialize iterator for next round */
Michal Domonkos e2e5e2
+	    rpmtdInit(&td);
Michal Domonkos 43a9b0
+	} else {
Michal Domonkos 43a9b0
+	    maxl = len;
Michal Domonkos e2e5e2
+	}
Michal Domonkos 43a9b0
 
Michal Domonkos 43a9b0
+	uint8_t *t = bin = xmalloc(num * maxl);
Michal Domonkos 43a9b0
+	int i = 0;
Michal Domonkos e2e5e2
 	while ((s = rpmtdNextString(&td))) {
Michal Domonkos e2e5e2
 	    if (*s == '\0') {
Michal Domonkos 43a9b0
-		memset(t, 0, len);
Michal Domonkos 43a9b0
-		t += len;
Michal Domonkos 43a9b0
-		continue;
Michal Domonkos 43a9b0
+		memset(t, 0, maxl);
Michal Domonkos 43a9b0
+	    } else {
Michal Domonkos 43a9b0
+		if (lens)
Michal Domonkos 43a9b0
+		    len = lens[i];
Michal Domonkos 43a9b0
+		for (int j = 0; j < len; j++, s += 2)
Michal Domonkos 43a9b0
+		    t[j] = (rnibble(s[0]) << 4) | rnibble(s[1]);
Michal Domonkos 43a9b0
 	    }
Michal Domonkos 43a9b0
-	    for (int j = 0; j < len; j++, t++, s += 2)
Michal Domonkos 43a9b0
-		*t = (rnibble(s[0]) << 4) | rnibble(s[1]);
Michal Domonkos 43a9b0
+	    t += maxl;
Michal Domonkos 43a9b0
+	    i++;
Michal Domonkos 43a9b0
 	}
Michal Domonkos 43a9b0
     }
Michal Domonkos 43a9b0
     rpmtdFreeData(&td);
Michal Domonkos 43a9b0
@@ -1570,15 +1601,15 @@ static int rpmfilesPopulate(rpmfiles fi, Header h, rpmfiFlags flags)
Michal Domonkos e2e5e2
     /* grab hex digests from header and store in binary format */
Michal Domonkos e2e5e2
     if (!(flags & RPMFI_NOFILEDIGESTS)) {
Michal Domonkos e2e5e2
 	size_t diglen = rpmDigestLength(fi->digestalgo);
Michal Domonkos e2e5e2
-	fi->digests = hex2bin(h, RPMTAG_FILEDIGESTS, totalfc, diglen);
Michal Domonkos e2e5e2
+	fi->digests = hex2bin(h, RPMTAG_FILEDIGESTS, totalfc, diglen,
Michal Domonkos e2e5e2
+				NULL, NULL);
Michal Domonkos e2e5e2
     }
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
     fi->signatures = NULL;
Michal Domonkos e2e5e2
     /* grab hex signatures from header and store in binary format */
Michal Domonkos e2e5e2
     if (!(flags & RPMFI_NOFILESIGNATURES)) {
Michal Domonkos e2e5e2
-	fi->signaturelength = headerGetNumber(h, RPMTAG_FILESIGNATURELENGTH);
Michal Domonkos e2e5e2
-	fi->signatures = hex2bin(h, RPMTAG_FILESIGNATURES,
Michal Domonkos e2e5e2
-				 totalfc, fi->signaturelength);
Michal Domonkos e2e5e2
+	fi->signatures = hex2bin(h, RPMTAG_FILESIGNATURES, totalfc, 0,
Michal Domonkos e2e5e2
+				&fi->signaturelengths, &fi->signaturemaxlen);
Michal Domonkos e2e5e2
     }
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
     /* XXX TR_REMOVED doesn;t need fmtimes, frdevs, finodes */
Michal Domonkos e2e5e2
diff --git a/python/rpmfiles-py.c b/python/rpmfiles-py.c
Michal Domonkos e2e5e2
index 27666021d..48189a0ac 100644
Michal Domonkos e2e5e2
--- a/python/rpmfiles-py.c
Michal Domonkos e2e5e2
+++ b/python/rpmfiles-py.c
Michal Domonkos e2e5e2
@@ -152,6 +152,22 @@ static PyObject *rpmfile_digest(rpmfileObject *s)
Michal Domonkos e2e5e2
     Py_RETURN_NONE;
Michal Domonkos e2e5e2
 }
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
+static PyObject *bytebuf(const unsigned char *buf, size_t len)
Michal Domonkos e2e5e2
+{
Michal Domonkos e2e5e2
+    if (buf) {
Michal Domonkos e2e5e2
+	PyObject *o = PyBytes_FromStringAndSize((const char *)buf, len);
Michal Domonkos e2e5e2
+	return o;
Michal Domonkos e2e5e2
+    }
Michal Domonkos e2e5e2
+    Py_RETURN_NONE;
Michal Domonkos e2e5e2
+}
Michal Domonkos e2e5e2
+
Michal Domonkos e2e5e2
+static PyObject *rpmfile_imasig(rpmfileObject *s)
Michal Domonkos e2e5e2
+{
Michal Domonkos e2e5e2
+    size_t len = 0;
Michal Domonkos e2e5e2
+    const unsigned char *sig = rpmfilesFSignature(s->files, s->ix, &len;;
Michal Domonkos e2e5e2
+    return bytebuf(sig, len);
Michal Domonkos e2e5e2
+}
Michal Domonkos e2e5e2
+
Michal Domonkos e2e5e2
 static PyObject *rpmfile_class(rpmfileObject *s)
Michal Domonkos e2e5e2
 {
Michal Domonkos e2e5e2
     return utf8FromString(rpmfilesFClass(s->files, s->ix));
Michal Domonkos e2e5e2
@@ -278,6 +294,8 @@ static PyGetSetDef rpmfile_getseters[] = {
Michal Domonkos e2e5e2
       "language the file provides (typically for doc files)" },
Michal Domonkos e2e5e2
     { "caps",		(getter) rpmfile_caps,		NULL,
Michal Domonkos e2e5e2
       "file capabilities" },
Michal Domonkos e2e5e2
+    { "imasig",	(getter) rpmfile_imasig,	NULL,
Michal Domonkos e2e5e2
+      "IMA signature" },
Michal Domonkos e2e5e2
     { NULL, NULL, NULL, NULL }
Michal Domonkos e2e5e2
 };
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
diff --git a/sign/rpmsignfiles.c b/sign/rpmsignfiles.c
Michal Domonkos e2e5e2
index b143c5b9b..6f39db6be 100644
Michal Domonkos e2e5e2
--- a/sign/rpmsignfiles.c
Michal Domonkos e2e5e2
+++ b/sign/rpmsignfiles.c
Michal Domonkos e2e5e2
@@ -98,8 +98,9 @@ rpmRC rpmSignFiles(Header sigh, Header h, const char *key, char *keypass)
Michal Domonkos e2e5e2
     td.count = 1;
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
     while (rpmfiNext(fi) >= 0) {
Michal Domonkos e2e5e2
+	uint32_t slen;
Michal Domonkos e2e5e2
 	digest = rpmfiFDigest(fi, NULL, NULL);
Michal Domonkos e2e5e2
-	signature = signFile(algoname, digest, diglen, key, keypass, &siglen);
Michal Domonkos e2e5e2
+	signature = signFile(algoname, digest, diglen, key, keypass, &slen);
Michal Domonkos e2e5e2
 	if (!signature) {
Michal Domonkos e2e5e2
 	    rpmlog(RPMLOG_ERR, _("signFile failed\n"));
Michal Domonkos e2e5e2
 	    goto exit;
Michal Domonkos e2e5e2
@@ -110,6 +111,8 @@ rpmRC rpmSignFiles(Header sigh, Header h, const char *key, char *keypass)
Michal Domonkos e2e5e2
 	    goto exit;
Michal Domonkos e2e5e2
 	}
Michal Domonkos e2e5e2
 	signature = _free(signature);
Michal Domonkos e2e5e2
+	if (slen > siglen)
Michal Domonkos e2e5e2
+	    siglen = slen;
Michal Domonkos e2e5e2
     }
Michal Domonkos e2e5e2
 
Michal Domonkos e2e5e2
     if (siglen > 0) {
Michal Domonkos e2e5e2
-- 
Michal Domonkos e2e5e2
2.33.1
Michal Domonkos e2e5e2