Eliminate header/payload digests from pgpDig_s, they dont belong
authorPanu Matilainen <pmatilai@redhat.com>
Wed, 25 Mar 2009 11:23:19 +0000 (13:23 +0200)
committerPanu Matilainen <pmatilai@redhat.com>
Wed, 25 Mar 2009 11:23:19 +0000 (13:23 +0200)
- allocate+free digests locally where needed, pass around in separate argument
- use digest bundles to handle rpmVerifySignatures() needs
- kill-kill-kill fdStealDigest(), dup the contexts from bundles as needed

lib/package.c
lib/rpmchecksig.c
lib/signature.c
lib/signature.h
rpmio/digest.c
rpmio/digest.h
rpmio/rpmio_internal.h
rpmio/rpmpgp.c

index a56812b..6992d0e 100644 (file)
@@ -232,6 +232,7 @@ rpmRC headerCheck(rpmts ts, const void * uh, size_t uc, char ** msg)
     int xx;
     int i;
     struct rpmtd_s sigtd;
+    DIGEST_CTX ctx = NULL;
 
     /* Is the blob the right size? */
     if (uc > 0 && pvlen != uc) {
@@ -408,26 +409,26 @@ verifyinfo_exit:
        ildl[1] = htonl(ildl[1]);
 
        (void) rpmswEnter(rpmtsOp(ts, RPMTS_OP_DIGEST), 0);
-       dig->hdrmd5ctx = rpmDigestInit(dig->signature.hash_algo, RPMDIGEST_NONE);
+       ctx = rpmDigestInit(dig->signature.hash_algo, RPMDIGEST_NONE);
 
        b = (unsigned char *) rpm_header_magic;
        nb = sizeof(rpm_header_magic);
-        (void) rpmDigestUpdate(dig->hdrmd5ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
 
        b = (unsigned char *) ildl;
        nb = sizeof(ildl);
-        (void) rpmDigestUpdate(dig->hdrmd5ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
 
        b = (unsigned char *) pe;
        nb = (htonl(ildl[0]) * sizeof(*pe));
-        (void) rpmDigestUpdate(dig->hdrmd5ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
 
        b = (unsigned char *) dataStart;
        nb = htonl(ildl[1]);
-        (void) rpmDigestUpdate(dig->hdrmd5ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
        (void) rpmswExit(rpmtsOp(ts, RPMTS_OP_DIGEST), dig->nbytes);
 
@@ -443,26 +444,26 @@ verifyinfo_exit:
        ildl[1] = htonl(ildl[1]);
 
        (void) rpmswEnter(rpmtsOp(ts, RPMTS_OP_DIGEST), 0);
-       dig->hdrsha1ctx = rpmDigestInit(PGPHASHALGO_SHA1, RPMDIGEST_NONE);
+       ctx = rpmDigestInit(PGPHASHALGO_SHA1, RPMDIGEST_NONE);
 
        b = (unsigned char *) rpm_header_magic;
        nb = sizeof(rpm_header_magic);
-        (void) rpmDigestUpdate(dig->hdrsha1ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
 
        b = (unsigned char *) ildl;
        nb = sizeof(ildl);
-        (void) rpmDigestUpdate(dig->hdrsha1ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
 
        b = (unsigned char *) pe;
        nb = (htonl(ildl[0]) * sizeof(*pe));
-        (void) rpmDigestUpdate(dig->hdrsha1ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
 
        b = (unsigned char *) dataStart;
        nb = htonl(ildl[1]);
-        (void) rpmDigestUpdate(dig->hdrsha1ctx, b, nb);
+        (void) rpmDigestUpdate(ctx, b, nb);
         dig->nbytes += nb;
        (void) rpmswExit(rpmtsOp(ts, RPMTS_OP_DIGEST), dig->nbytes);
 
@@ -473,7 +474,7 @@ verifyinfo_exit:
     }
 
     {  rpmKeyring keyring = rpmtsGetKeyring(ts, 1);
-       rc = rpmVerifySignature(keyring, &sigtd, dig, &buf);
+       rc = rpmVerifySignature(keyring, &sigtd, dig, ctx, &buf);
        rpmKeyringFree(keyring);
     }
 
@@ -484,6 +485,7 @@ verifyinfo_exit:
 
     rpmtdFreeData(&sigtd);
     pgpFreeDig(dig);
+    rpmDigestFinal(ctx, NULL, NULL, 0);
     return rc;
 }
 
@@ -584,6 +586,7 @@ rpmRC rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
     rpmRC rc = RPMRC_FAIL;     /* assume failure */
     int leadtype = -1;
     headerGetFlags hgeflags = HEADERGET_DEFAULT;
+    DIGEST_CTX ctx = NULL;
 
     if (hdrp) *hdrp = NULL;
 
@@ -705,10 +708,10 @@ rpmRC rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
        if (!headerGet(h, RPMTAG_HEADERIMMUTABLE, &utd, hgeflags))
            break;
        (void) rpmswEnter(rpmtsOp(ts, RPMTS_OP_DIGEST), 0);
-       dig->hdrmd5ctx = rpmDigestInit(dig->signature.hash_algo, RPMDIGEST_NONE);
-       (void) rpmDigestUpdate(dig->hdrmd5ctx, rpm_header_magic, sizeof(rpm_header_magic));
+       ctx = rpmDigestInit(dig->signature.hash_algo, RPMDIGEST_NONE);
+       (void) rpmDigestUpdate(ctx, rpm_header_magic, sizeof(rpm_header_magic));
        dig->nbytes += sizeof(rpm_header_magic);
-       (void) rpmDigestUpdate(dig->hdrmd5ctx, utd.data, utd.count);
+       (void) rpmDigestUpdate(ctx, utd.data, utd.count);
        dig->nbytes += utd.count;
        (void) rpmswExit(rpmtsOp(ts, RPMTS_OP_DIGEST), dig->nbytes);
        rpmtsOp(ts, RPMTS_OP_DIGEST)->count--;  /* XXX one too many */
@@ -724,10 +727,10 @@ rpmRC rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
        if (!headerGet(h, RPMTAG_HEADERIMMUTABLE, &utd, hgeflags))
            break;
        (void) rpmswEnter(rpmtsOp(ts, RPMTS_OP_DIGEST), 0);
-       dig->hdrsha1ctx = rpmDigestInit(PGPHASHALGO_SHA1, RPMDIGEST_NONE);
-       (void) rpmDigestUpdate(dig->hdrsha1ctx, rpm_header_magic, sizeof(rpm_header_magic));
+       ctx = rpmDigestInit(PGPHASHALGO_SHA1, RPMDIGEST_NONE);
+       (void) rpmDigestUpdate(ctx, rpm_header_magic, sizeof(rpm_header_magic));
        dig->nbytes += sizeof(rpm_header_magic);
-       (void) rpmDigestUpdate(dig->hdrsha1ctx, utd.data, utd.count);
+       (void) rpmDigestUpdate(ctx, utd.data, utd.count);
        dig->nbytes += utd.count;
        (void) rpmswExit(rpmtsOp(ts, RPMTS_OP_DIGEST), dig->nbytes);
        if (sigtag == RPMSIGTAG_SHA1)
@@ -755,7 +758,8 @@ rpmRC rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
            goto exit;
        }
 
-       fdStealDigest(fd, dig);
+       ctx = rpmDigestBundleDupCtx(fd->digests, (sigtag == RPMSIGTAG_MD5) ?
+                                   PGPHASHALGO_MD5 : dig->signature.hash_algo);
        break;
     default:
        break;
@@ -763,7 +767,7 @@ rpmRC rpmReadPackageFile(rpmts ts, FD_t fd, const char * fn, Header * hdrp)
 
     /** @todo Implement disable/enable/warn/error/anal policy. */
     {  rpmKeyring keyring = rpmtsGetKeyring(ts, 1);
-       rc = rpmVerifySignature(keyring, &sigtd, dig, &msg);
+       rc = rpmVerifySignature(keyring, &sigtd, dig, ctx, &msg);
        rpmKeyringFree(keyring);
     }
        
@@ -818,6 +822,7 @@ exit:
        *hdrp = headerLink(h);
     }
     rpmtdFreeData(&sigtd);
+    rpmDigestFinal(ctx, NULL, NULL, 0);
     h = headerFree(h);
     pgpFreeDig(dig);
     sigh = rpmFreeSignature(sigh);
index a3df375..88e3c9d 100644 (file)
@@ -451,7 +451,8 @@ rpmtsClean(ts);
 /**
  * @todo If the GPG key was known available, the md5 digest could be skipped.
  */
-static int readFile(FD_t fd, const char * fn, pgpDig dig)
+static int readFile(FD_t fd, const char * fn, pgpDig dig,
+                   rpmDigestBundle plbundle, rpmDigestBundle hdrbundle)
 {
     unsigned char buf[4*BUFSIZ];
     ssize_t count;
@@ -478,12 +479,8 @@ static int readFile(FD_t fd, const char * fn, pgpDig dig)
                        "Corrupted package?\n"), fn);
                goto exit;
            }
-           dig->hdrsha1ctx = rpmDigestInit(PGPHASHALGO_SHA1, RPMDIGEST_NONE);
-           (void) rpmDigestUpdate(dig->hdrsha1ctx, rpm_header_magic, sizeof(rpm_header_magic));
-           (void) rpmDigestUpdate(dig->hdrsha1ctx, utd.data, utd.count);
-           dig->hdrmd5ctx = rpmDigestInit(dig->signature.hash_algo, RPMDIGEST_NONE);
-           (void) rpmDigestUpdate(dig->hdrmd5ctx, rpm_header_magic, sizeof(rpm_header_magic));
-           (void) rpmDigestUpdate(dig->hdrmd5ctx, utd.data, utd.count);
+           rpmDigestBundleUpdate(hdrbundle, rpm_header_magic, sizeof(rpm_header_magic));
+           rpmDigestBundleUpdate(hdrbundle, utd.data, utd.count);
            rpmtdFreeData(&utd);
        }
        h = headerFree(h);
@@ -496,7 +493,6 @@ static int readFile(FD_t fd, const char * fn, pgpDig dig)
        rpmlog(RPMLOG_ERR, _("%s: Fread failed: %s\n"), fn, Fstrerror(fd));
        goto exit;
     }
-    fdStealDigest(fd, dig);
 
     rc = 0;
 
@@ -637,6 +633,8 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
     int nodigests = !(qva->qva_flags & VERIFY_DIGEST);
     int nosignatures = !(qva->qva_flags & VERIFY_SIGNATURE);
     rpmKeyring keyring = rpmtsGetKeyring(ts, 1);
+    rpmDigestBundle plbundle = rpmDigestBundleNew();
+    rpmDigestBundle hdrbundle = rpmDigestBundleNew();
 
     rpmlead lead = rpmLeadNew();
     if ((rc = rpmLeadRead(fd, lead)) == RPMRC_OK) {
@@ -675,28 +673,34 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
     sigp = &dig->signature;
 
     /* XXX RSA needs the hash_algo, so decode early. */
-    if (sigtag == RPMSIGTAG_RSA || sigtag == RPMSIGTAG_PGP) {
+    if (sigtag == RPMSIGTAG_RSA || sigtag == RPMSIGTAG_PGP ||
+               sigtag == RPMSIGTAG_DSA || sigtag == RPMSIGTAG_GPG) {
        xx = headerGet(sigh, sigtag, &sigtd, HEADERGET_DEFAULT);
        xx = pgpPrtPkts(sigtd.data, sigtd.count, dig, 0);
        rpmtdFreeData(&sigtd);
        /* XXX assume same hash_algo in header-only and header+payload */
-       if ((headerIsEntry(sigh, RPMSIGTAG_PGP)
-         || headerIsEntry(sigh, RPMSIGTAG_PGP5))
-        && dig->signature.hash_algo != PGPHASHALGO_MD5)
-           fdInitDigest(fd, dig->signature.hash_algo, 0);
+       rpmDigestBundleAdd(plbundle, sigp->hash_algo, RPMDIGEST_NONE);
+       rpmDigestBundleAdd(hdrbundle, sigp->hash_algo, RPMDIGEST_NONE);
     }
 
-    if (headerIsEntry(sigh, RPMSIGTAG_PGP)
-    ||  headerIsEntry(sigh, RPMSIGTAG_PGP5)
-    ||  headerIsEntry(sigh, RPMSIGTAG_MD5))
-       fdInitDigest(fd, PGPHASHALGO_MD5, 0);
-    if (headerIsEntry(sigh, RPMSIGTAG_GPG))
-       fdInitDigest(fd, PGPHASHALGO_SHA1, 0);
+    if (headerIsEntry(sigh, RPMSIGTAG_PGP) ||
+                     headerIsEntry(sigh, RPMSIGTAG_PGP5) ||
+                     headerIsEntry(sigh, RPMSIGTAG_MD5)) {
+       rpmDigestBundleAdd(plbundle, PGPHASHALGO_MD5, RPMDIGEST_NONE);
+    }
+    if (headerIsEntry(sigh, RPMSIGTAG_GPG)) {
+       rpmDigestBundleAdd(plbundle, PGPHASHALGO_SHA1, RPMDIGEST_NONE);
+    }
+
+    /* always do sha1 hash of header */
+    rpmDigestBundleAdd(hdrbundle, PGPHASHALGO_SHA1, RPMDIGEST_NONE);
 
     /* Read the file, generating digest(s) on the fly. */
-    if (dig == NULL || sigp == NULL || readFile(fd, fn, dig)) {
+    fdSetBundle(fd, plbundle);
+    if (readFile(fd, fn, dig, plbundle, hdrbundle)) {
        goto exit;
     }
+    fdSetBundle(fd, NULL); /* XXX avoid double-free from fd close */
 
     rasprintf(&buf, "%s:%c", fn, (rpmIsVerbose() ? '\n' : ' ') );
 
@@ -704,7 +708,7 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
     for (; headerNext(hi, &sigtd) != 0; rpmtdFreeData(&sigtd)) {
        char *result = NULL;
        int havekey = 0;
-
+       DIGEST_CTX ctx = NULL;
        if (sigtd.data == NULL) /* XXX can't happen */
            continue;
 
@@ -723,18 +727,27 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
            if (parsePGP(&sigtd, fn, dig) != RPMRC_OK) {
                goto exit;
            }
+           ctx = rpmDigestBundleDupCtx(havekey ? plbundle : hdrbundle,
+                                       dig->signature.hash_algo);
            break;
        case RPMSIGTAG_SHA1:
+           if (nodigests)
+                continue;
+           ctx = rpmDigestBundleDupCtx(hdrbundle, PGPHASHALGO_SHA1);
+           break;
        case RPMSIGTAG_MD5:
            if (nodigests)
                 continue;
+           ctx = rpmDigestBundleDupCtx(plbundle, PGPHASHALGO_MD5);
            break;
        default:
            continue;
            break;
        }
 
-       rc = rpmVerifySignature(keyring, &sigtd, dig, &result);
+       rc = rpmVerifySignature(keyring, &sigtd, dig, ctx, &result);
+       rpmDigestFinal(ctx, NULL, NULL, 0);
+
        formatResult(sigtd.tag, rc, result, havekey, 
                     (rc == RPMRC_NOKEY ? &missingKeys : &untrustedKeys),
                     &buf);
@@ -764,6 +777,8 @@ int rpmVerifySignatures(QVA_t qva, rpmts ts, FD_t fd,
 
 exit:
     free(buf);
+    rpmDigestBundleFree(hdrbundle);
+    rpmDigestBundleFree(plbundle);
     sigh = rpmFreeSignature(sigh);
     hi = headerFreeIterator(hi);
     rpmKeyringFree(keyring);
index 7dcb50c..5ec1748 100644 (file)
@@ -1245,7 +1245,7 @@ exit:
 }
 
 rpmRC
-rpmVerifySignature(rpmKeyring keyring, rpmtd sigtd, pgpDig dig, char ** result)
+rpmVerifySignature(rpmKeyring keyring, rpmtd sigtd, pgpDig dig, DIGEST_CTX ctx, char ** result)
 {
     rpmRC res = RPMRC_NOTFOUND;
     char *msg = NULL;
@@ -1260,25 +1260,23 @@ rpmVerifySignature(rpmKeyring keyring, rpmtd sigtd, pgpDig dig, char ** result)
        res = verifySizeSignature(sigtd, dig->nbytes, &msg);
        break;
     case RPMSIGTAG_MD5:
-       res = verifyMD5Signature(sigtd, &msg, dig->md5ctx);
+       res = verifyMD5Signature(sigtd, &msg, ctx);
        break;
     case RPMSIGTAG_SHA1:
-       res = verifySHA1Signature(sigtd, &msg, dig->hdrsha1ctx);
+       res = verifySHA1Signature(sigtd, &msg, ctx);
        break;
     case RPMSIGTAG_RSA:
-       res = verifyRSASignature(keyring, sigtd, dig, &msg, dig->hdrmd5ctx);
+       res = verifyRSASignature(keyring, sigtd, dig, &msg, ctx);
        break;
     case RPMSIGTAG_PGP5:       /* XXX legacy */
     case RPMSIGTAG_PGP:
-       res = verifyRSASignature(keyring, sigtd, dig, &msg,
-               ((dig->signature.hash_algo == PGPHASHALGO_MD5)
-                       ? dig->md5ctx : dig->sha1ctx));
+       res = verifyRSASignature(keyring, sigtd, dig, &msg, ctx);
        break;
     case RPMSIGTAG_DSA:
-       res = verifyDSASignature(keyring, sigtd, dig, &msg, dig->hdrsha1ctx);
+       res = verifyDSASignature(keyring, sigtd, dig, &msg, ctx);
        break;
     case RPMSIGTAG_GPG:
-       res = verifyDSASignature(keyring, sigtd, dig, &msg, dig->sha1ctx);
+       res = verifyDSASignature(keyring, sigtd, dig, &msg, ctx);
        break;
     default:
        rasprintf(&msg, _("Signature: UNKNOWN (%d)\n"), sigtd->tag);
index 5846585..b3b527a 100644 (file)
@@ -76,7 +76,7 @@ int rpmAddSignature(Header sigh, const char * file,
  *                     (malloc'd)
  * @return             result of signature verification
  */
-rpmRC rpmVerifySignature(rpmKeyring keyring, rpmtd sigtd, pgpDig dig, char ** result);
+rpmRC rpmVerifySignature(rpmKeyring keyring, rpmtd sigtd, pgpDig dig, DIGEST_CTX ctx, char ** result);
 
 /** \ingroup signature
  * Destroy signature header from package.
index 0779b90..181a920 100644 (file)
@@ -279,32 +279,3 @@ void fdFiniDigest(FD_t fd, pgpHashAlgo hashalgo,
        fdstat_exit(fd, FDSTAT_DIGEST, (ssize_t) 0);
     }
 }
-
-void fdStealDigest(FD_t fd, pgpDig dig)
-{
-    if (fd && fd->digests) {
-       rpmDigestBundle bundle = fd->digests;
-       for (int i = bundle->index_max; i >= bundle->index_min; i--) {
-           DIGEST_CTX ctx = bundle->digests[i];
-           if (ctx == NULL)
-               continue;
-           switch (ctx->algo) {
-           case PGPHASHALGO_MD5:
-               assert(dig->md5ctx == NULL);
-               dig->md5ctx = ctx;
-               bundle->digests[i] = NULL;
-               break;
-           case PGPHASHALGO_SHA1:
-           case PGPHASHALGO_SHA256:
-           case PGPHASHALGO_SHA384:
-           case PGPHASHALGO_SHA512:
-               assert(dig->sha1ctx == NULL);
-               dig->sha1ctx = ctx;
-               bundle->digests[i] = NULL;
-               break;
-           default:
-               break;
-           }
-       }
-    }
-}
index 9870632..32856e4 100644 (file)
@@ -44,12 +44,6 @@ struct pgpDig_s {
 
     size_t nbytes;             /*!< No. bytes of plain text. */
 
-    DIGEST_CTX sha1ctx;                /*!< (dsa) sha1 hash context. */
-    DIGEST_CTX hdrsha1ctx;     /*!< (dsa) header sha1 hash context. */
-
-    DIGEST_CTX md5ctx;         /*!< (rsa) md5 hash context. */
-    DIGEST_CTX hdrmd5ctx;      /*!< (rsa) header md5 hash context. */
-
     /* DSA/RSA parameters */
     SECKEYPublicKey *keydata;
     SECItem *sigdata;
index b00c35d..65d6ead 100644 (file)
@@ -254,6 +254,13 @@ void fdSetCpioPos(FD_t fd, rpm_loff_t cpioPos)
     fd->fd_cpioPos = cpioPos;
 }
 
+static inline
+void fdSetBundle(FD_t fd, rpmDigestBundle bundle)
+{
+    FDSANE(fd);
+    fd->digests = bundle;
+}
+
 /** \ingroup rpmio
  */
 static inline
@@ -281,10 +288,6 @@ void fdFiniDigest(FD_t fd, pgpHashAlgo hashalgo,
                size_t * lenp,
                int asAscii);
 
-
-/* XXX Steal the digest-in-progress from the file handle. */
-void fdStealDigest(FD_t fd, pgpDig dig);
-
 /**
  * Read an entire file into a buffer.
  * @param fn           file name to read
index c4fd406..2cb10a5 100644 (file)
@@ -1252,23 +1252,6 @@ pgpDig pgpFreeDig(pgpDig dig)
 
        /* DUmp the signature/pubkey data. */
        pgpCleanDig(dig);
-
-       if (dig->hdrsha1ctx != NULL)
-           (void) rpmDigestFinal(dig->hdrsha1ctx, NULL, NULL, 0);
-       dig->hdrsha1ctx = NULL;
-
-       if (dig->sha1ctx != NULL)
-           (void) rpmDigestFinal(dig->sha1ctx, NULL, NULL, 0);
-       dig->sha1ctx = NULL;
-
-       if (dig->hdrmd5ctx != NULL)
-           (void) rpmDigestFinal(dig->hdrmd5ctx, NULL, NULL, 0);
-       dig->hdrmd5ctx = NULL;
-
-       if (dig->md5ctx != NULL)
-           (void) rpmDigestFinal(dig->md5ctx, NULL, NULL, 0);
-       dig->md5ctx = NULL;
-
        dig = _free(dig);
     }
     return dig;