PR#4202: RawHeader: fix store offsets when duplicate tags are present - koji

koji

#4202 RawHeader: fix store offsets when duplicate tags are present

Merged 3 months ago by tkopecek. Opened 7 months ago by mikem.

mikem/koji rawheader-duptags into master

fix misleading comment

Mike McLean • 7 months ago

ae3c28f

show dups in header dump

Mike McLean • 7 months ago

64585b9

fix store offsets when duplicate tags are present

Mike McLean • 7 months ago

f35f98c

koji/__init__.py

file modified

+13 -13

		`@@ -689,7 +689,8 @@`
		`dl = multibyte(data[4:8])`

		`# read the index (starts at offset 16)`
		`- index = {}`
		`+ index = []`
		`+ tag_index = {}`
		`for i in range(il):`
		`entry = []`
		`for j in range(4):`
		`@@ -698,21 +699,22 @@`
		`entry.append(multibyte(data))`

		`# print("Tag: %d, Type: %d, Offset: %x, Count: %d" % tuple(entry))`
		`- index[entry[0]] = entry`
		`+ index.append(entry)`
		`+ tag_index[entry[0]] = entry`
		`+ # in case of duplicate tags, last one wins`
		`+`
		`self.datalen = dl`
		`- self.index = index`
		`+ self._store = 16 + il * 16`
		`+ self._index = index # list`
		`+ self.index = tag_index # dict`

		`def dump(self, sig=None):`
		`print("HEADER DUMP:")`
		`- # calculate start of store`
		`- il = len(self.index)`
		`- store = 16 + il * 16`
		`- # print("start is: %d" % start)`
		`- # print("index length: %d" % il)`
		`- print("Store at offset %d (%0x)" % (store, store))`
		`+ store = self._store`
		`+ print("Store at offset %d (0x%0x)" % (store, store))`
		`# sort entries by offset, dtype`
		`# also rearrange: tag, dtype, offset, count -> offset, dtype, tag, count`
		`- order = sorted([(x[2], x[1], x[0], x[3]) for x in six.itervalues(self.index)])`
		`+ order = sorted([(x[2], x[1], x[0], x[3]) for x in self._index])`
		`# map some rpmtag codes`
		`tags = {}`
		`if rpm:`
		`@@ -868,9 +870,7 @@`
		`def _getitem(self, dtype, offset, count, decode=None):`
		`if decode is None:`
		`decode = self.decode`
		`- # calculate start of store`
		`- il = len(self.index)`
		`- store = 16 + il * 16`
		`+ store = self._store`
		`pos = store + offset`
		`if dtype >= 2 and dtype <= 5:`
		`values = []`

kojihub/kojihub.py

file modified

+1 -6

		`@@ -8183,12 +8183,7 @@`
		`if not sigkey:`
		`sigkey = rawhdr.get(koji.RPM_SIGTAG_RSA)`
		`else:`
		`- # In older rpms, this field in the signature header does not actually match`
		`- # sigmd5 (I think rpmlib pulls it from SIGTAG_GPG). Anyway, this`
		`- # sanity check fails incorrectly for those rpms, so we fall back to`
		`- # a somewhat more expensive check.`
		`- # ALSO, for these older rpms, the layout of SIGTAG_GPG is different too, so`
		`- # we need to pull that differently as well`
		`+ # Double check using rpm in case we have somehow misread`
		`rpm_path = "%s/%s" % (builddir, koji.pathinfo.rpm(rinfo))`
		`sigmd5, sigkey = _scan_sighdr(sighdr, rpm_path)`
		`sigmd5 = koji.hex_string(sigmd5)`

mikem commented 7 months ago

For context, see https://pagure.io/koji/issue/4200

If an rpm has duplicate tags in a header, Koji would miscalculate the
start of that data store.

Fixes https://pagure.io/koji/issue/4203

1 new commit added

fix misleading comment

7 months ago

tkopecek commented 6 months ago

:thumbsup:

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

6 months ago

mikem commented 6 months ago

testing notes

an example rpm that triggers this (rh internal) is linked in #4200
the only place we actually use RawHeader in koji is when importing rpm signatures. The rpm referenced above is signed, so simply importing it into a test instance is a basic test of this
that said, it's hard to distinguish if the fallback code from the old workaround is happening

Testing the code directly is fairly straightforward. E.g. if I use the problem rpm, I get:

>>> import koji
>>> fn='/home/mikem/wu-ftpd-2.6.1-20.src.rpm'
>>> sighdr = koji.rip_rpm_sighdr(fn)
>>> rawhdr = koji.RawHeader(sighdr)
>>> koji.hex_string(rawhdr.get(koji.RPM_SIGTAG_MD5))
'f75f2047ffe5b35eb3ecb6ebd80baf41'

Which matches rpm

$ rpm -qp --qf '%{sigmd5}\n' wu-ftpd-2.6.1-20.src.rpm 
f75f2047ffe5b35eb3ecb6ebd80baf41

Running the same code without this fix gives an incorrect sigmd5

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

3 months ago

tkopecek commented 3 months ago

Commit 177ba35 fixes this pull-request

Pull-Request has been merged by tkopecek

3 months ago

Metadata

Assignee

None

Tags

Changes Summary 2

koji

Source Code

Documentation

#4202 RawHeader: fix store offsets when duplicate tags are present Merged 3 months ago by tkopecek. Opened 7 months ago by mikem. mikem/koji rawheader-duptags into master

Metadata

testing-ready testing-done

Changes Summary 2

#4202 RawHeader: fix store offsets when duplicate tags are present

Merged 3 months ago by tkopecek. Opened 7 months ago by mikem.

mikem/koji rawheader-duptags into master