Backport multiple signatures to 4.20 #3715
Conversation
ffesti
force-pushed
the
multiple_signatures
branch
from
fe6f30f to
e9cc0be
Compare
|
The documentation talks a lot about v4 / v6 packages, which probably makes not much sense in the rpm-4.20 context. |
|
So I always need to use |
For v4 packages yes, because it affects how the "classic" signatures are handled. For v6 packages, v6 signatures are the default and you need --rpmv4 to add the "classic" signature in addition. Hmm. Now that you mention the v4/v6 situation: we probably should actually support signing v6 packages from 4.20. Shouldn't be too many commits, and that's just the kind of thing we've been promising for 4.20. These will be coexisting for a long, long time, and that will make the documentation look saner as well. |
|
Yep, seems so obvious now that it was brought up, guess I've just been head too deep in the backporting hole to see it: So pick up a40b6e9 too, that'll reduce the need for patch it up specifically. |
tests/rpmvfylevel.at
Outdated
| Header OpenPGP signature: NOTFOUND | ||
| Header OpenPGP RSA signature: NOTFOUND | ||
| Header OpenPGP DSA signature: NOTFOUND | ||
| Header signature: NOTFOUND |
There was a problem hiding this comment.
Hmm. These verbose messages are a headache. I'm thinking we should emit "Header OpenPGP signature" for the v6 signatures to have at least the new thing equal in both 4.x and 6.x, although we have to leave the RSA/DSA lines alone to avoid breakage. It wont eliminate the test-suite diff between 4.x and 6.x but will make it at least a bit smaller.
There was a problem hiding this comment.
On a related note, the existing signature/Signature on fail/success is nuts, but probably too risky to change in a stable series 😒
There was a problem hiding this comment.
This doesn't seem to have been addressed, the v6 signature should be "Header OpenPGP signature" to make it clear what it is and align with upstream.
tests/rpmsigdig.at
Outdated
|
|
||
| RPMTEST_CHECK([ | ||
| runroot rpmbuild -bb --quiet \ | ||
| --define "_rpmfilever 6" \ |
There was a problem hiding this comment.
This obviously wont produce v6 packages in 4.x, we'll need to ship a pre-built v6 package for testing purposes.
|
On a related note, we should also backport the couple of patches to eliminate hardwired "gpg" in the signing error messages. It doesn't have to be in this PR, but it does seem to fit in logically as we're expecting to sign with sq in this set. |
|
FWIW, this will need at least some of the fixes from #3757 too. |
Keep /usr/bin/sq hard coded. Automating the location would require adding sequoia-sq as a build dependency which we don't want for now.
Will be needed for the upcoming tests for multiple singature support. Move removal of ima plugin otherwise the installation of sequia-sq fails.
The function comments are several generations old plain wrong. No functional changes. (cherry picked from commit 51446f4)
No functional changes, just a tiny refactoring bit for the next steps. (cherry picked from commit b760b9c)
Pass the flags down to the function that needs to do stuff. No functional changes. (cherry picked from commit 41552ee)
ffesti
force-pushed
the
multiple_signatures
branch
from
e9cc0be to
f322fd2
Compare
|
Backport of ad114b0 is missing, that's a particularly important patch because it deals with our "main case" here: previously unsupported algorithms on v4 packages. |
|
To summarize, because some of these items aren't visible in the GH review at all:
Other than that, looks vaguely familiar, somehow 😅 |
Unfortunately RPMSIGTAG_RESERVEDSPACE clashes with RPMTAG_INSTALLTIME so we can't use it in v6. Introduce a new tag at the top of the signature range to make sure its always the last one - on v6 packages that is. For v4 packages we're better off not messing with the existing tag at this point, we just need to handle both. (backported from commit a40b6e9)
ffesti
force-pushed
the
multiple_signatures
branch
from
f322fd2 to
28b1ccc
Compare
Add support for multiple OpenPGP header signatures per package, base64 encoded in a string array, also known as rpm v6 signatures. --addsign no longer deletes any signatures, it only creates and adds a new signature if possible. --delete and --resign behave as before: they delete ALL signatures on the package, and the latter then creates and adds a new one. For v6 packages this is the default signature type, but if requested, one v4 compat signature can be created for compatible algorithms. v6 signatures on v4 packages are also supported, but have to be explicitly requested. In that case, v3/v4 signatures are only added if none already exist and a v4 compatible algorithm is used. v3 signatures on v6 packages are not supported (out of principle, not a technical limitation) On verification, if RPMTAG_OPENPGP exists then other signature tags are ignored because they're expected to only contain compat copies of the same content. As of now, all existing signatures must validate for signature checking of a package to pass, further policies are to be added later. Fixes: rpm-software-management#3385 (backported from commit 5630cf4)
Add a tag extension for RPMTAG_OPENPGP (on top of the concrete tag) to handle compatibility with v3/v4 signatures: the extension collects all legacy signatures under the same umbrella so users don't need to query multiple different tags, you just query for RPMTAG_OPENPGP to get all them at once. Extend :pgpsig tag format to handle the new string array/base64 variant. Update --info/-i query to use the extension and output all existing signatures, one per line. The no-signature case of "Signature : (none)" is preserved as-is to help backwards compatibility with scripts parsing the output. Related: rpm-software-management#3385 (backported from commit bea8f45)
rpm-4.20 has adifferent message format for signatures and checksums. Have this in a separate patch for now to make it easier to review the more meaningful changes to the test cases.
Output format and cli is different between rpm 6 and 4.20. Adjust the test to work with 4.20. * Use macros.rpmsign-sequoia to sign packages with sequoia. * Redirect output of rpmsign --addsign to /dev/null as everywhere else * Use short key IDs instead of long key IDs and fingerprints. * Add SHA1 and MD5 digests
We now have a nice way to centrally get the format number, use it in the signing code instead of the various ad-hoc methods added during multi-signature development, and use 'rpmformat' as the variable name for easy grepping. No functional changes. (cherry picked from commit ef5b538)
For OpenPGP v6 we need sequoia-sq >= 1.3 and rpm-sequoia >= 1.8 in the bare minimum, the latter is only available from rawhide so we need to pull it from there. This also requires a newer crypto-policies package than is available on any released version to permit ed25519 and x25519, otherwise we'll get funky "denied by policy" errors when signing. Talk about bleeding edge! (cherry picked from commit 8b1de0b)
Created using Sequoia SQ 1.3.1 with:
sq key generate
--own-key \
--userid v6-ed25519-testkey
--name "rpm.org ed25519 v6 testkey"
--email "[email protected]"
--profile rfc9580
--expiration=never
--cannot-authenticate
--cannot-encrypt
--without-password
(cherry picked from commit bffdc56)
Turns out the key / fingerprint length check isn't here yet. (backported from commit aa85c48)
(cherry picked from commit 1dccba4)
Turns out there are bugs in our signature size reservation handling, the reservation doesn't get used so the header grows when it doesn't need to (see rpm-software-management#3768). This means we're now testing for the WRONG size values here, but at least this lets us see that the right tags are there. (cherry picked from commit 1c68565)
Adding an rpm v6 signature fails with a message such as "error: Unsupported OpenPGP pubkey algorithm 27" if the algorithm isn't supported by rpm v4 signatures. The issue here seems simple enough: makeSigTag() assumes there'll always be a legacy tag to map to, but that's not the case with new algorithms such as those added in RFC-9580. Only, this tiny thing causes a bit of an avalance: we need to move the tag decision logic to putSignature(), but to do that we also need to move the check for identical signatures there, and to do that we need to pay more attention to putSignature() retuns. And then we can finally make the decisions we need, where we need them: When adding an rpm v4 signature, suppress the error from an "unknown" algorithm if we already added an rpm v6 signature for it in the same run. Add tests to the extent we can, rpm-sequoia 1.8 doesn't fully handle OpenPGP v6 it seems: rpm-software-management/rpm-sequoia#87 Fixes: rpm-software-management#3752 (cherry picked from commit ad114b0)
ffesti
force-pushed
the
multiple_signatures
branch
from
28b1ccc to
0e9c2f9
Compare
|
Seems as per requested now 👍 As noted in one of the commit messages, fixing up the tests in separate commits is useful for seeing the difference, but I think we'll eventually need to merge them into the relevant commits. Whether that time is now or later I'm dunno, we can discuss that privately. Thing is, we're not going to release 4.20.x with this in it before 6.0 is out, and merging this to 4.20.x now would hinder our ability to do a emergency fixes in the meanwhile. So I think this needs to stay out of three for the time being regardless of anything else. One thing I realized just now that is still missing here for the "full works" is the rfc-9580 algorithm IDs (9344367, e073410, c2f7601) but that doesn't show up because we're not able to test them yet anyhow. |
|
Converted to draft to get it out of our review lists until it is actual time to merge. |
There was a problem hiding this comment.
As per #3715 (comment) and #3713 (comment) - this is considered approved although not merged yet.
3 participants
Resolves: #3713
This is split up into more commits that it should probably end up with to make reviewing easier. We probably want the test cases to work properly from when they get added. But adding the fixed test cases makes it hard to see how they differe from the original ones.