From 3de5226674d8b505d619494d2548388dd0afa1b1 Mon Sep 17 00:00:00 2001 From: Richard Phibel Date: Sep 13 2023 20:44:31 +0000 Subject: Backport fix for IMA signature lengths assumed constant See https://github.com/rpm-software-management/rpm/pull/1844 for explanation of the issue. The fix backported is the lastest one: https://github.com/rpm-software-management/rpm/pull/1914 --- diff --git a/SOURCES/0035-fix-IMA-signature-lengths-assumed-constant.patch b/SOURCES/0035-fix-IMA-signature-lengths-assumed-constant.patch new file mode 100644 index 0000000..bed129d --- /dev/null +++ b/SOURCES/0035-fix-IMA-signature-lengths-assumed-constant.patch @@ -0,0 +1,161 @@ +From 07f1d3132f0c7b7ecb69a47a9930edb534a9250e Mon Sep 17 00:00:00 2001 +From: Panu Matilainen +Date: Mon, 7 Feb 2022 13:38:48 +0200 +Subject: [PATCH] Fix IMA signature fubar, take III (#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. + +As we can't assume static lengths and attempts to use maximum length +have proven problematic for other reasons, use a data structure that +can actually handle variable length data properly: store offsets into +the decoded binary blob and use them to calculate lengths when needed, +empty data is simply consequtive identical offsets. This avoids a whole +class of silly overflow issues with multiplying, makes zero-length data +actually presentable in the data structure and saves memory too. + +Add tests to show behavior with variable length signatures and missing +signatures. + +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 +--- + lib/rpmfi.c | 61 +++++++++++++++++++++++++++++++++++++++------ + sign/rpmsignfiles.c | 5 +++- + 2 files changed, 58 insertions(+), 8 deletions(-) + +diff --git a/lib/rpmfi.c b/lib/rpmfi.c +index 439179689..4673fbb85 100644 +--- a/lib/rpmfi.c ++++ b/lib/rpmfi.c +@@ -116,7 +116,7 @@ struct rpmfiles_s { + struct fingerPrint_s * fps; /*!< File fingerprint(s). */ + + int digestalgo; /*!< File digest algorithm */ +- int signaturelength; /*!< File signature length */ ++ uint32_t *signatureoffs; /*!< File signature offsets */ + int veritysiglength; /*!< Verity signature length */ + uint16_t verityalgo; /*!< Verity algorithm */ + unsigned char * digests; /*!< File digests in binary. */ +@@ -566,10 +566,15 @@ const unsigned char * rpmfilesFSignature(rpmfiles fi, int ix, size_t *len) + const unsigned char *signature = NULL; + + if (fi != NULL && ix >= 0 && ix < rpmfilesFC(fi)) { +- if (fi->signatures != NULL) +- signature = fi->signatures + (fi->signaturelength * ix); ++ size_t slen = 0; ++ if (fi->signatures != NULL && fi->signatureoffs != NULL) { ++ uint32_t off = fi->signatureoffs[ix]; ++ slen = fi->signatureoffs[ix+1] - off; ++ if (slen > 0) ++ signature = fi->signatures + off; ++ } + if (len) +- *len = fi->signaturelength; ++ *len = slen; + } + return signature; + } +@@ -1242,6 +1247,7 @@ rpmfiles rpmfilesFree(rpmfiles fi) + fi->flangs = _free(fi->flangs); + fi->digests = _free(fi->digests); + fi->signatures = _free(fi->signatures); ++ fi->signatureoffs = _free(fi->signatureoffs); + fi->veritysigs = _free(fi->veritysigs); + fi->fcaps = _free(fi->fcaps); + +@@ -1471,6 +1477,48 @@ err: + return; + } + ++/* ++ * Convert a tag of variable len hex strings to binary presentation, ++ * accessed via offsets to a contiguous binary blob. Empty values ++ * are represented by identical consequtive offsets. The offsets array ++ * always has one extra element to allow calculating the size of the ++ * last element. ++ */ ++static uint8_t *hex2binv(Header h, rpmTagVal tag, rpm_count_t num, ++ uint32_t **offsetp) ++{ ++ struct rpmtd_s td; ++ uint8_t *bin = NULL; ++ uint32_t *offs = NULL; ++ ++ if (headerGet(h, tag, &td, HEADERGET_MINMEM) && rpmtdCount(&td) == num) { ++ const char *s; ++ int i = 0; ++ uint8_t *t = bin = xmalloc(((rpmtdSize(&td) / 2) + 1)); ++ offs = xmalloc((num + 1) * sizeof(*offs)); ++ ++ while ((s = rpmtdNextString(&td))) { ++ uint32_t slen = strlen(s); ++ uint32_t len = slen / 2; ++ if (slen % 2) { ++ bin = rfree(bin); ++ offs = rfree(offs); ++ goto exit; ++ } ++ offs[i] = t - bin; ++ for (int j = 0; j < len; j++, t++, s += 2) ++ *t = (rnibble(s[0]) << 4) | rnibble(s[1]); ++ i++; ++ } ++ offs[i] = t - bin; ++ *offsetp = offs; ++ } ++ ++exit: ++ rpmtdFreeData(&td); ++ return bin; ++} ++ + /* Convert a tag of hex strings to binary presentation */ + static uint8_t *hex2bin(Header h, rpmTagVal tag, rpm_count_t num, size_t len) + { +@@ -1623,9 +1671,8 @@ static int rpmfilesPopulate(rpmfiles fi, Header h, rpmfiFlags flags) + 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 = hex2binv(h, RPMTAG_FILESIGNATURES, ++ totalfc, &fi->signatureoffs); + } + + fi->veritysigs = NULL; +diff --git a/sign/rpmsignfiles.c b/sign/rpmsignfiles.c +index b143c5b9b..372ba634c 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 = 0; + 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.40.1 + diff --git a/SPECS/rpm.spec b/SPECS/rpm.spec index 39504b8..ec4a51f 100644 --- a/SPECS/rpm.spec +++ b/SPECS/rpm.spec @@ -42,7 +42,7 @@ %global rpmver 4.14.3 #global snapver rc2 -%global rel 26.2 +%global rel 26.3 %global srcver %{version}%{?snapver:-%{snapver}} %global srcdir %{?snapver:testing}%{!?snapver:%{name}-%(echo %{version} | cut -d'.' -f1-2).x} @@ -202,6 +202,7 @@ Patch1980: 0031-Update-man-page-for-rpmsign.patch Patch1981: 0032-rpmsign-Add-argument-to-specify-algorithm-for-fsveri.patch Patch1982: 0033-Enable-fsverity-in-CI.patch Patch1983: 0034-rpmsign-Adopting-PKCS11-opaque-keys-support-in-libfsverity-for-fsverity-signatures.patch +Patch1984: 0035-fix-IMA-signature-lengths-assumed-constant.patch %endif %if %{with ndb} @@ -845,6 +846,9 @@ make check || cat tests/rpmtests.log %doc doc/librpm/html/* %changelog +* Wed Sep 13 2023 Richard Phibel - 4.14.3-26.3 +- Fix IMA signature lengths assumed constant + * Thu Aug 17 2023 Richard Phibel - 4.14.3-26.2 - Fix issue for transaction with transcoded and non-transcoded packages - Fix stack overflow in rpm2extents and various memory leaks