alexk / rpms / rpm

Forked from rpms/rpm 2 years ago
Clone
ac33b4
commit d6a86b5e69e46cc283b1e06c92343319beb42e21
ac33b4
Author: Panu Matilainen <pmatilai@redhat.com>
ac33b4
Date:   Thu Mar 4 13:21:19 2021 +0200
ac33b4
ac33b4
    Be much more careful about copying data from the signature header
ac33b4
    
ac33b4
    Only look for known tags, and ensure correct type and size where known
ac33b4
    before copying over. Bump the old arbitrary 16k count limit to 16M limit
ac33b4
    though, it's not inconceivable that a package could have that many files.
ac33b4
    While at it, ensure none of these tags exist in the main header,
ac33b4
    which would confuse us greatly.
ac33b4
    
ac33b4
    This is optimized for backporting ease, upstream can remove redundancies
ac33b4
    and further improve checking later.
ac33b4
    
ac33b4
    Reported and initial patches by Demi Marie Obenour.
ac33b4
    
ac33b4
    Fixes: RhBug:1935049, RhBug:1933867, RhBug:1935035, RhBug:1934125, ...
ac33b4
    
ac33b4
    Fixes: CVE-2021-3421, CVE-2021-20271
ac33b4
ac33b4
    Backported into 4.11.3 and combined with upstream commits:
ac33b4
      - f7b97593af5cf818a5c6c5b9bc55bba6d08c9cb0
ac33b4
      - e2f1f1931c5ccf3ecbe4e1e12cacb1e17a277776
ac33b4
      - 822c3dc2046c29718e34ac2da16a9757a9be11da
ac33b4
ac33b4
diff --git a/lib/package.c b/lib/package.c
ac33b4
index 985ac47d3..565e4bf6d 100644
ac33b4
--- a/lib/package.c
ac33b4
+++ b/lib/package.c
ac33b4
@@ -28,79 +28,75 @@ static unsigned int * keyids;
ac33b4
 extern int auditGpgResult;
ac33b4
 extern int auditEnabled;
ac33b4
 
ac33b4
+static struct taglate_s {
ac33b4
+    rpmTagVal stag;
ac33b4
+    rpmTagVal xtag;
ac33b4
+    rpm_count_t count;
ac33b4
+    int quirk;
ac33b4
+} const xlateTags[] = {
ac33b4
+    { RPMSIGTAG_SIZE, RPMTAG_SIGSIZE, 1, 0 },
ac33b4
+    { RPMSIGTAG_PGP, RPMTAG_SIGPGP, 0, 0 },
ac33b4
+    { RPMSIGTAG_MD5, RPMTAG_SIGMD5, 16, 0 },
ac33b4
+    { RPMSIGTAG_GPG, RPMTAG_SIGGPG, 0, 0 },
ac33b4
+    /* { RPMSIGTAG_PGP5, RPMTAG_SIGPGP5, 0, 0 }, */ /* long obsolete, dont use */
ac33b4
+    { RPMSIGTAG_PAYLOADSIZE, RPMTAG_ARCHIVESIZE, 1, 1 },
ac33b4
+    { RPMSIGTAG_SHA1, RPMTAG_SHA1HEADER, 1, 0 },
ac33b4
+    { RPMSIGTAG_DSA, RPMTAG_DSAHEADER, 0, 0 },
ac33b4
+    { RPMSIGTAG_RSA, RPMTAG_RSAHEADER, 0, 0 },
ac33b4
+    { RPMSIGTAG_LONGSIZE, RPMTAG_LONGSIGSIZE, 1, 0 },
ac33b4
+    { RPMSIGTAG_LONGARCHIVESIZE, RPMTAG_LONGARCHIVESIZE, 1, 0 },
ac33b4
+    { 0 }
ac33b4
+};
ac33b4
+
ac33b4
 /** \ingroup header
ac33b4
  * Translate and merge legacy signature tags into header.
ac33b4
  * @param h		header (dest)
ac33b4
  * @param sigh		signature header (src)
ac33b4
+ * @return		failing tag number, 0 on success
ac33b4
  */
ac33b4
-static void headerMergeLegacySigs(Header h, Header sigh)
ac33b4
+static
ac33b4
+rpmTagVal headerMergeLegacySigs(Header h, Header sigh, char **msg)
ac33b4
 {
ac33b4
-    HeaderIterator hi;
ac33b4
+    const struct taglate_s *xl;
ac33b4
     struct rpmtd_s td;
ac33b4
 
ac33b4
-    hi = headerInitIterator(sigh);
ac33b4
-    for (; headerNext(hi, &td); rpmtdFreeData(&td))
ac33b4
-    {
ac33b4
-	switch (td.tag) {
ac33b4
-	/* XXX Translate legacy signature tag values. */
ac33b4
-	case RPMSIGTAG_SIZE:
ac33b4
-	    td.tag = RPMTAG_SIGSIZE;
ac33b4
-	    break;
ac33b4
-	case RPMSIGTAG_PGP:
ac33b4
-	    td.tag = RPMTAG_SIGPGP;
ac33b4
-	    break;
ac33b4
-	case RPMSIGTAG_MD5:
ac33b4
-	    td.tag = RPMTAG_SIGMD5;
ac33b4
-	    break;
ac33b4
-	case RPMSIGTAG_GPG:
ac33b4
-	    td.tag = RPMTAG_SIGGPG;
ac33b4
-	    break;
ac33b4
-	case RPMSIGTAG_PGP5:
ac33b4
-	    td.tag = RPMTAG_SIGPGP5;
ac33b4
-	    break;
ac33b4
-	case RPMSIGTAG_PAYLOADSIZE:
ac33b4
-	    td.tag = RPMTAG_ARCHIVESIZE;
ac33b4
-	    break;
ac33b4
-	case RPMSIGTAG_SHA1:
ac33b4
-	case RPMSIGTAG_DSA:
ac33b4
-	case RPMSIGTAG_RSA:
ac33b4
-	default:
ac33b4
-	    if (!(td.tag >= HEADER_SIGBASE && td.tag < HEADER_TAGBASE))
ac33b4
+    for (xl = xlateTags; xl->stag; xl++) {
ac33b4
+	/* There mustn't be one in the main header */
ac33b4
+	if (headerIsEntry(h, xl->xtag)) {
ac33b4
+	    /* Some tags may exist in either header, but never both */
ac33b4
+	    if (xl->quirk && !headerIsEntry(sigh, xl->stag))
ac33b4
 		continue;
ac33b4
-	    break;
ac33b4
+	    goto exit;
ac33b4
 	}
ac33b4
-	if (td.data == NULL) continue;	/* XXX can't happen */
ac33b4
-	if (!headerIsEntry(h, td.tag)) {
ac33b4
-	    if (hdrchkType(td.type))
ac33b4
-		continue;
ac33b4
-	    if (td.count < 0 || hdrchkData(td.count))
ac33b4
-		continue;
ac33b4
-	    switch(td.type) {
ac33b4
-	    case RPM_NULL_TYPE:
ac33b4
-		continue;
ac33b4
+    }
ac33b4
+
ac33b4
+    rpmtdReset(&td);
ac33b4
+    for (xl = xlateTags; xl->stag; xl++) {
ac33b4
+	if (headerGet(sigh, xl->stag, &td, HEADERGET_RAW|HEADERGET_MINMEM)) {
ac33b4
+	    /* Translate legacy tags */
ac33b4
+	    if (xl->stag != xl->xtag)
ac33b4
+		td.tag = xl->xtag;
ac33b4
+	    /* Ensure type and tag size match expectations */
ac33b4
+	    if (td.type != rpmTagGetTagType(td.tag))
ac33b4
 		break;
ac33b4
-	    case RPM_CHAR_TYPE:
ac33b4
-	    case RPM_INT8_TYPE:
ac33b4
-	    case RPM_INT16_TYPE:
ac33b4
-	    case RPM_INT32_TYPE:
ac33b4
-	    case RPM_INT64_TYPE:
ac33b4
-		if (td.count != 1)
ac33b4
-		    continue;
ac33b4
+	    if (td.count < 1 || td.count > 16*1024*1024)
ac33b4
 		break;
ac33b4
-	    case RPM_STRING_TYPE:
ac33b4
-	    case RPM_BIN_TYPE:
ac33b4
-		if (td.count >= 16*1024)
ac33b4
-		    continue;
ac33b4
+	    if (xl->count && td.count != xl->count)
ac33b4
 		break;
ac33b4
-	    case RPM_STRING_ARRAY_TYPE:
ac33b4
-	    case RPM_I18NSTRING_TYPE:
ac33b4
-		continue;
ac33b4
+	    if (!headerPut(h, &td, HEADERPUT_DEFAULT))
ac33b4
 		break;
ac33b4
-	    }
ac33b4
-	    (void) headerPut(h, &td, HEADERPUT_DEFAULT);
ac33b4
+	    rpmtdFreeData(&td);
ac33b4
 	}
ac33b4
     }
ac33b4
-    headerFreeIterator(hi);
ac33b4
+    rpmtdFreeData(&td);
ac33b4
+
ac33b4
+exit:
ac33b4
+    if (xl->stag) {
ac33b4
+	rasprintf(msg, "invalid signature tag %s (%d)",
ac33b4
+			rpmTagGetName(xl->xtag), xl->xtag);
ac33b4
+    }
ac33b4
+
ac33b4
+    return xl->stag;
ac33b4
 }
ac33b4
 
ac33b4
 /**
ac33b4
@@ -671,7 +667,7 @@ static rpmRC rpmpkgRead(rpmKeyring keyring, rpmVSFlags vsflags,
ac33b4
 	rpmlog(RPMLOG_ERR, "%s: %s", fn, msg);
ac33b4
 	break;
ac33b4
     }
ac33b4
-    free(msg);
ac33b4
+    msg = _free(msg);
ac33b4
 
ac33b4
 exit:
ac33b4
     if (rc != RPMRC_FAIL && h != NULL && hdrp != NULL) {
ac33b4
@@ -701,10 +697,13 @@ exit:
ac33b4
 	    headerConvert(h, HEADERCONV_COMPRESSFILELIST);
ac33b4
 	
ac33b4
 	/* Append (and remap) signature tags to the metadata. */
ac33b4
-	headerMergeLegacySigs(h, sigh);
ac33b4
-
ac33b4
-	/* Bump reference count for return. */
ac33b4
-	*hdrp = headerLink(h);
ac33b4
+	if (headerMergeLegacySigs(h, sigh, &msg)) {
ac33b4
+	    rpmlog(RPMLOG_ERR, "%s: %s\n", fn, msg);
ac33b4
+	    free(msg);
ac33b4
+	    rc = RPMRC_FAIL;
ac33b4
+	} else
ac33b4
+	    /* Bump reference count for return. */
ac33b4
+	    *hdrp = headerLink(h);
ac33b4
     }
ac33b4
     rpmtdFreeData(&sigtd);
ac33b4
     rpmDigestFinal(ctx, NULL, NULL, 0);
ac33b4
ac33b4
commit 441a0998fca9b3d01052bc3d33f8a72f725d5865
ac33b4
Author: Panu Matilainen <pmatilai@redhat.com>
ac33b4
Date:   Fri Oct 21 17:37:26 2016 +0300
ac33b4
ac33b4
    Require exact match on header vs region sizes when reading package files
ac33b4
    
ac33b4
    In rpm V4 packages both the signature and mail header consist of one
ac33b4
    contiguous immutable region when read from package files and any
ac33b4
    disparities in index or data size means malformed package.
ac33b4
    
ac33b4
    Once the signature header is merged into the main header this is
ac33b4
    no longer true of course and installation adds further tags, so for
ac33b4
    headers from other sources (such as rpmdb), tags outside the immutable
ac33b4
    region (known as "dribbles" in rpm lore) must be allowed, and eg
ac33b4
    headerCheck() cannot require exact match.
ac33b4
ac33b4
    Backported into 4.11.3.  Note that the upstream patch also enforces
ac33b4
    exact_size for signature headers, but this was later reverted in commit
ac33b4
    34c2ba3c6a80a778cdf2e42a9193b3264e08e1b3, so we omit that part here.
ac33b4
ac33b4
diff --git a/lib/package.c b/lib/package.c
ac33b4
index 08f0bc634..fea27e9ca 100644
ac33b4
--- a/lib/package.c
ac33b4
+++ b/lib/package.c
ac33b4
@@ -275,7 +275,8 @@ exit:
ac33b4
 }
ac33b4
 
ac33b4
 static rpmRC headerVerify(rpmKeyring keyring, rpmVSFlags vsflags,
ac33b4
-			  const void * uh, size_t uc, char ** msg)
ac33b4
+			  const void * uh, size_t uc, int exact_size,
ac33b4
+			  char ** msg)
ac33b4
 {
ac33b4
     char *buf = NULL;
ac33b4
     int32_t * ei = (int32_t *) uh;
ac33b4
@@ -287,6 +288,7 @@ static rpmRC headerVerify(rpmKeyring keyring, rpmVSFlags vsflags,
ac33b4
     struct indexEntry_s entry;
ac33b4
     struct entryInfo_s info;
ac33b4
     int32_t ril = 0;
ac33b4
+    int32_t rdl = 0;
ac33b4
     unsigned char * regionEnd = NULL;
ac33b4
     rpmRC rc = RPMRC_FAIL;	/* assume failure */
ac33b4
 
ac33b4
@@ -337,6 +339,7 @@ static rpmRC headerVerify(rpmKeyring keyring, rpmVSFlags vsflags,
ac33b4
     regionEnd = dataStart + entry.info.offset;
ac33b4
     (void) memcpy(&info, regionEnd, REGION_TAG_COUNT);
ac33b4
     regionEnd += REGION_TAG_COUNT;
ac33b4
+    rdl = regionEnd - dataStart;
ac33b4
 
ac33b4
     if (headerVerifyInfo(1, il * sizeof(*pe) + REGION_TAG_COUNT, &info, &entry.info, 1) != -1 ||
ac33b4
 	!(entry.info.tag == RPMTAG_HEADERIMMUTABLE
ac33b4
@@ -358,9 +361,17 @@ static rpmRC headerVerify(rpmKeyring keyring, rpmVSFlags vsflags,
ac33b4
 	goto exit;
ac33b4
     }
ac33b4
 
ac33b4
+    /* In package files region size is expected to match header size. */
ac33b4
+    if (exact_size && !(il == ril && dl == rdl)) {
ac33b4
+	rasprintf(&buf,
ac33b4
+		_("region %d: tag number mismatch %d ril %d dl %d rdl %d\n"),
ac33b4
+		entry.info.tag, il, ril, dl, rdl);
ac33b4
+	goto exit;
ac33b4
+    }
ac33b4
+
ac33b4
     /* Verify header-only digest/signature if there is one we can use. */
ac33b4
     rc = headerSigVerify(keyring, vsflags,
ac33b4
-			 il, dl, ril, (regionEnd - dataStart),
ac33b4
+			 il, dl, ril, rdl,
ac33b4
 			 pe, dataStart, &buf;;
ac33b4
 
ac33b4
 exit:
ac33b4
@@ -394,7 +405,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
ac33b4
     rpmKeyring keyring = rpmtsGetKeyring(ts, 1);
ac33b4
 
ac33b4
     rpmswEnter(rpmtsOp(ts, RPMTS_OP_DIGEST), 0);
ac33b4
-    rc = headerVerify(keyring, vsflags, uh, uc, msg);
ac33b4
+    rc = headerVerify(keyring, vsflags, uh, uc, 0, msg);
ac33b4
     rpmswExit(rpmtsOp(ts, RPMTS_OP_DIGEST), uc);
ac33b4
     rpmKeyringFree(keyring);
ac33b4
 
ac33b4
@@ -453,7 +464,7 @@ static rpmRC rpmpkgReadHeader(rpmKeyring keyring, rpmVSFlags vsflags,
ac33b4
     }
ac33b4
 
ac33b4
     /* Sanity check header tags */
ac33b4
-    rc = headerVerify(keyring, vsflags, ei, uc, &buf;;
ac33b4
+    rc = headerVerify(keyring, vsflags, ei, uc, 1, &buf;;
ac33b4
     if (rc != RPMRC_OK)
ac33b4
 	goto exit;
ac33b4