We've (imported) builds from 2001 in brew :-)

Oh, wow. Well, that explains a few things :D

afaict, the binary header format is not changing, just the headers available (and the payload format), correct?

I don't think Koji's header parsing is going to be a problem, just the change in available headers.

There's some strange logic surrounding SIGMD5 in ... that seems to suggest it verifies the value stored?

Koji uses the sigmd5 header as a unique identifier for rpms that does not change with signing. The code you linked (in add_rpm_sig) is verifying that the detached signature header uploaded by the client does in fact match the rpm entry the call is asking for it to be attached to. We're not performing any summing here; we're just comparing the sigmd5 value pulled from the header to the value we already have for the rpm in the database (calculated at import time using rpm lib).

That would all be from "initial code drop" from 2007 so any history as to what the heck that is supposed to refer to is lost in history, but the claim that on old rpm versions the md5 tag would give you something else looks very, very dubious to me.

I do have a copy of the old internal cvs repo exported to git. The commit (Oct 2006) isn't very enlightening though. It's entirely possible I misread the situation back then. I don't seem to have a reference to an example build (though whatever it was is probably still in Brew)

We've (imported) builds from 2001 in brew :-)

Brew was initialized with a complete import of Beehive, which itself contained rpms from Prospector. The earliest rpm buildtime in Brew is xscreensaver-2.16-2_gnome.i386.rpm, from 1998-06-10.

but the claim that on old rpm versions the md5 tag would give you something else looks very, very dubious to me

Ok, I was able to dig up an example of an rpm where Koji was misreading the sigmd5 from the header. Here's the one I found:

wu-ftpd-2.6.1-20 (circa 2001)
https://brewweb.engineering.redhat.com/brew/buildinfo?buildID=12145

After some digging it turns out the issue is that this signature header contains duplicate tag entries, two sigmd5 (1004) and two size (1000), out of seven total. These were presumably added by rpmsign.

Koji was tacitly assuming that there were no duplicate tags. It was using the length of the index dictionary to calculate the offset of the header store and getting incorrect values because the index dictionary had length 5 instead of 7. A stupid bug that hasn't come up in a long time.

So, my 2006 assessment of the problem was wrong, but here's the fix anyway.
https://pagure.io/koji/pull-request/4202

Right, so basically buggy package built by some rpm a long, long time ago. There are any number of such flaws out in the wild, even more so in the old days where no tests existed at all.
Thanks for the investigation!

The most significant issue here is probably not the bits where Koji does its own header parsing, but Koji's general reliance on sigmd5 as a validating key for rpms. The value is generally just fetched via rpmlib, not custom parsing.

It's used fairly broadly both to verify rpms and to uniquely identify them. For example, when we record the rpms that were used in a buildroot, we query not only the nvra, but the sigmd5. This is matched against the value in the db to make sure we have the rpm we think we do. Similar logic is used for recording image contents.

Addressing this properly will be a bit of work.

Such use should be relatively easy to replace with header checksums. I expect rpm 6.0 beta2 to have a tag extension for this, but basically you'd just wrap the package id retrieve into a function somewhat like this that would give you an allegedly unique id for any package across rpm v3 - v6:

def getID(hdr):
    for t in ['sha3_256header', 'sha256header', 'sha1header']:
        hid = hdr[t]
        if hid:
            return hid
    return hdr['sigmd5'].hex()

A less disruptive version of that would use sigmd5 if it exists, and otherwise fall back to the list of header checksums (but in that case the sha3 variant would always get used for v6 packages and md5 for anything else and the list of header checksum tags is kinda moot)

partial work -- https://pagure.io/koji/pull-request/4446#

Commit 86057e1b fixes this issue

That ended up being quite a big change for koji. It's a little bit tragicomic how many places outside rpm itself rely on the obsolete sigmd5 thing being there. But of course, if you need to deal with v3 rpms, that's all there is...

Thanks so much for your work on this!

This issue has been migrated to Fedora Forge:
https://forge.fedoraproject.org/koji/koji/issues/4200

Please continue any further discussion there.