| |
@@ -0,0 +1,161 @@
|
| |
+ From 07f1d3132f0c7b7ecb69a47a9930edb534a9250e Mon Sep 17 00:00:00 2001
|
| |
+ From: Panu Matilainen <pmatilai@redhat.com>
|
| |
+ 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
|
| |
+
|
| |
richardphibel
commented a year ago
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
The fix has already been backported in c9s-sig-hyperscale branch:
https://git.centos.org/rpms/rpm/blob/c9s-sig-hyperscale/f/rpm.spec#_93