On 04/03/2018 10:31 PM, Vladimir D. Seleznev wrote:
RPMTAG_IDENTITY is calculating as digest of part of package header that
does not contain irrelevant to package build tag entries.

Mathematically RPMTAG_IDENTITY value is a result of function of two
variable: a package header and an rpm utility, thus this value can
differ for same package and different version of rpm.

Despite tag entries are filtering by blacklist, rpm also filters tags
that it doesn't know while it calculates identity value. So technically,
it's a graylist.
---
  lib/rpmtag.h  |  3 +-
  lib/tagexts.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/lib/rpmtag.h b/lib/rpmtag.h
index 973a6b6..36dac85 100644
--- a/lib/rpmtag.h
+++ b/lib/rpmtag.h
@@ -371,11 +371,12 @@ typedef enum rpmTag_e {
      RPMTAG_PAYLOADDIGEST      = 5092, /* s[] */
      RPMTAG_PAYLOADDIGESTALGO  = 5093, /* i */
      RPMTAG_AUTOINSTALLED      = 5094, /* i reservation (unimplemented) */
-    RPMTAG_IDENTITY            = 5095, /* s reservation (unimplemented) */
+    RPMTAG_IDENTITY            = 5095, /* s extension */
RPMTAG_FIRSTFREE_TAG /*!< internal */
  } rpmTag;
+#define RPMTAG_IDENTITY_MAX_TAG RPMTAG_FIRSTFREE_TAG
  #define       RPMTAG_EXTERNAL_TAG             1000000
/** \ingroup rpmtag
diff --git a/lib/tagexts.c b/lib/tagexts.c
index f72ff60..ca8ac35 100644
--- a/lib/tagexts.c
+++ b/lib/tagexts.c
@@ -952,6 +952,98 @@ static int filenlinksTag(Header h, rpmtd td, 
headerGetFlags hgflags)
      return (fc > 0);
  }
+static int identityFilter(rpmTag tag)
+{
+    int ret = 0;
+
+    switch(tag) {
+    case RPMTAG_ARCH:
+    case RPMTAG_ARCHIVESIZE:
+    case RPMTAG_AUTOINSTALLED:
+    case RPMTAG_BUILDHOST:
+    case RPMTAG_BUILDTIME:
+    case RPMTAG_CHANGELOG:
+    case RPMTAG_COOKIE:
+    case RPMTAG_DBINSTANCE:
+    case RPMTAG_DISTRIBUTION:
+    case RPMTAG_DISTTAG:
+    case RPMTAG_DISTURL:
+    case RPMTAG_FILESTATES:
+    case RPMTAG_HEADERIMMUTABLE:
+    case RPMTAG_IDENTITY:
+    case RPMTAG_INSTALLCOLOR:
+    case RPMTAG_INSTALLTID:
+    case RPMTAG_INSTALLTIME:

Many of these are only ever present in installed headers, and of such tags at least RPMTAG_ORIGDIRINDEXES, RPMTAG_ORIGBASENAMES and
RPMTAG_ORIGDIRNAMES should also be filtered out.

But see below...

+    case RPMTAG_INSTFILENAMES:

RPMTAG_INSTFILENAMES is an extension and will never be part of a header, ditto for RPMTAG_IDENTITY in this implementation.

+    case RPMTAG_LONGARCHIVESIZE:
+    case RPMTAG_LONGSIGSIZE:
+    case RPMTAG_LONGSIZE:
+    case RPMTAG_PAYLOADDIGEST:

RPMTAG_PAYLOADDIGESTALGO too.

+    case RPMTAG_RSAHEADER:
+    case RPMTAG_SHA1HEADER:
+    case RPMTAG_SIGLEMD5_1:
+    case RPMTAG_SIGLEMD5_2:
+    case RPMTAG_SIGMD5:
+    case RPMTAG_SIGPGP:
+    case RPMTAG_SIGSIZE:
+    case RPMTAG_SIZE:

At least RPMTAG_SHA256HEADER, RPMSIGTAG_FILESIGNATURES and RPMSIGTAG_FILESIGNATURELENGTH should also be on this list.

Basically you'd want everything below HEADER_TAGBASE filtered out, with the possible exception of HEADER_I18NTABLE. And also you'd want to filter out everything that was added to the header as a part of installation. There's a better way to all those goals than list individual tags:

Grab the immutable region of the header, and do the iteration on that instead of the header you got as an argument to identityTag(). Something like (untested):

static int identityTag(Header h, rpmtd td, headerGetFlags hgflags)
{
    rpmtd_s itd;
    Header ih;

    /* rpm v3 packages dont have an immutable region */
    if (!headerGet(h, RPMTAG_HEADERIMMUTABLE, &itd, HEADERGET_DEFAULT))
        goto err;

    ih = headerImport(itd.data, itd.count, HEADERIMPORT_FAST);

    /* ... and then iterate + copy over that */
    HeaderIterator hi = headerInitIterator(h);
    while (headerNext(hi, &ttd) && rc) {
    ...
}

+       ret = 1;
+       break;
+    default:
+       break;
+    }
+
+    if (tag >= RPMTAG_IDENTITY_MAX_TAG)
+       ret = 1;
+
+    return ret;
+}
+
+static int identityTag(Header h, rpmtd td, headerGetFlags hgflags)
+{
+    int rc = 1;
+    unsigned int bsize;
+    void * hb;
+    DIGEST_CTX ctx;
+    char * identity = NULL;
+
+    struct rpmtd_s ttd;
+    Header ih = headerNew();
+    HeaderIterator hi = headerInitIterator(h);
+
+    while (headerNext(hi, &ttd) && rc) {
+       if (rpmtdCount(&ttd) > 0) {
+           if (!identityFilter(ttd.tag))
+               rc = headerPut(ih, &ttd, HEADERPUT_DEFAULT);
+       }
+       rpmtdFreeData(&ttd);
+    }
+    headerFreeIterator(hi);
+
+    if (!rc)
+       return 0;
+
+    hb = headerExport(ih, &bsize);
+
+    if (hb == NULL)
+       return 0;

These early returns would leak the "ih" header. In general, we prefer functions to have one point of return where all function-level allocations are freed and early failure points jump to the exit point with goto. That way there's less chance for these kinds of leaks.

+
+    ctx = rpmDigestInit(PGPHASHALGO_SHA256, RPMDIGEST_NONE);

rpmDigestInit() can fail, you should check for errors.

+    rpmDigestUpdate(ctx, hb, bsize);
+    rpmDigestFinal(ctx, (void **) &identity, NULL, 1);
+    headerFree(ih);
+
+    td->data = xstrdup(identity);
+    td->type = RPM_STRING_TYPE;
+    td->count = 1;
+    td->flags = RPMTD_ALLOCED;
+
+    if (identity != NULL)
+       identity = _free(identity);

Don't bother checking for NULL when freeing, free() will do that anyway.

Overall I do like this a whole lot better than the earlier version.

        - Panu -

_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to