rpm icon indicating copy to clipboard operation
rpm copied to clipboard

Provide a decent API for verifying package signatures

Open DemiMarie opened this issue 3 years ago • 39 comments

Currently, there is no decent API to verify package signatures. There are various APIs that can be used, but they all are flawed in one way or another.

I propose an rpmRC rpmVerifyPackageSignature(rpmContext *context, int fd, uint64_t flags); API that just does the right thing for normal RPMs, together with an rpmRC rpmVerifyDeltaPackageSignature(rpmContext *context, int fd, uint64_t flags); that does the right thing for delta RPMs. The difference is that in the second case, the header+payload signature is required, and the payload digest is ignored as it will always be wrong.

The context is meant to handle stuff like logging and other state that is currently global.

Some of the flags I can think of:

  • RPM_VERIFY_STRICT: request strict checking of the various headers in the package, including checks which might be incompatible with ancient broken packages.
  • RPM_VERIFY_STRONG: enforce the use of strong cryptography, even if not required by system-wide policies.
  • RPM_VERIFY_ALL: enforce all possible checks.

DemiMarie avatar Apr 28 '22 16:04 DemiMarie

Yup, a sane signature verification API is needed, it was always part of the plan when adding the rpmvs.* stuff. The problem is finding the time + energy of sitting down and designing a sane one that actually covers the needs now and sufficiently flexible for various future needs too. Short of that, I've considered exporting something close to rpmpkgVerifySigs() (minus the logging basically). It has it's shortcomings but as The Good API refuses to stand up...

It'd be useful to list the various flaws with the existing APIs, from someone trying to deal with it as an external API user. The existing stuff covers the needs of rpm sufficiently, but life on the outside is always very different. Been there, but long forgotten. For that reason it's also important that whatever is added is used by rpm itself too.

I disagree with the delta rpm API though. Header+payload signatures are very much on their way out, and nothing new should rely on them. Deltarpm tries to create bit-per-bit compatible payload, and most of the time succeeds, payload digest isn't any more wrong than a header+payload signature would be. However deltarpm should just start creating an uncompressed payload which allows payloaddigestalt to be used instead (that's it's primary use-case) and once we've phased out header+payload signatures (and digests), there's no need for any other magic wrt deltarpm.

pmatilai avatar Apr 29 '22 06:04 pmatilai

I disagree with the delta rpm API though. Header+payload signatures are very much on their way out, and nothing new should rely on them. Deltarpm tries to create bit-per-bit compatible payload, and most of the time succeeds, payload digest isn't any more wrong than a header+payload signature would be. However deltarpm should just start creating an uncompressed payload which allows payloaddigestalt to be used instead (that's it's primary use-case) and once we've phased out header+payload signatures (and digests), there's no need for any other magic wrt deltarpm.

The problem with this is that it requires deltarpms to be applied before any signatures can be checked. This is a very bad idea: it means that any code execution bug in the tools responsible for applying deltarpms becomes a remote root hole. The header+payload signature allows for v3 deltarpms to be verified before having to parse them, which is much, much safer. More generally, signature verification needs to come first; everything else (such as decompression, applying delta instructions, etc) should come afterwards, when the input is no longer untrusted. Since deltarpms are small, most of the drawbacks of the header+payload signature do not apply to them. The payload digest cannot be used because it will always be incorrect, and while payloaddigestalt is nice to check, it cannot be checked soon enough.

It'd be useful to list the various flaws with the existing APIs, from someone trying to deal with it as an external API user. The existing stuff covers the needs of rpm sufficiently, but life on the outside is always very different. Been there, but long forgotten. For that reason it's also important that whatever is added is used by rpm itself too.

Will do, eventually, once I get the time. Whenever I had to verify a package signature, I either invoked the rpmkeys binary or (as in the case of rpmcanon) did everything myself except for crypto.

DemiMarie avatar Apr 29 '22 07:04 DemiMarie

To elaborate: v3 deltarpms contain their own signature header, separate from the signature header of the reconstructed RPM. This means that a header+payload signature of a v3 deltarpm can actually be valid, as can the header signature. The payload digest will obviously be wrong, though. I believe this is one of the motivations for the v3 deltarpm format, and @Conan-Kudo and I have both expressed interest in using this feature.

DemiMarie avatar Apr 29 '22 07:04 DemiMarie

I have no clue about any deltarpm versions or plans in that area. Rpm will stop processing header+payload signatures and digests in foreseeable future though, so if deltarpm wants to verify such things it'll need to do it by itself.

pmatilai avatar Apr 29 '22 07:04 pmatilai

Is this issue the right place to talk about an updated API for the PGP backend, as mentioned elsewhere, or would it be better to open a different issue for that?

Actually, I have no clue what a good API for this would look like smile So if you do, by all means propose something like a rough outline and we can go from there. I guess I mostly care that it doesn't look too outlandish in the rpm codebase, but considering what an hodge-podge the rpm API is, minor deviances get lost in the noise.

nwalfield avatar Apr 29 '22 13:04 nwalfield

It's at least closely related, so I don't see any harm discussing it here. On that note, DNF is interested in this all too, but their needs go beyond package signatures. @j-mracek can tell more about that.

pmatilai avatar May 03 '22 11:05 pmatilai

Thank you for ping. DNF team is looking for a replacement of GPGME for verification of signed metadat in repositories, therefore if there will be an option to extend RPM capability to also verify alternative file using keys not only from internal RPMDB keyring it would be great and everyone will benefit from such a solution or we will share the same pain. It also means that DNF would like to help with solution development and support. Of course there are some limitation - it will be difficult to accept a solution that will enlarge a minimal image.

j-mracek avatar May 10 '22 11:05 j-mracek

FWIW, you can already use a custom keyring populated from whatever source you like. Similarly it should be entirely possible to verify arbitrary files with the existing API. It may not be the nicest or sanest API but still, should be possible as-is.

pmatilai avatar May 10 '22 11:05 pmatilai

Thank you for ping. DNF team is looking for a replacement of GPGME for verification of signed metadat in repositories, therefore if there will be an option to extend RPM capability to also verify alternative file using keys not only from internal RPMDB keyring it would be great and everyone will benefit from such a solution or we will share the same pain. It also means that DNF would like to help with solution development and support. Of course there are some limitation - it will be difficult to accept a solution that will enlarge a minimal image.

Are you referring to the newly introduced Sequoia dependency?

DemiMarie avatar May 10 '22 14:05 DemiMarie

We are opened to any solution even Sequoia, but after the last DNF community meetings it looks like that there is a price in installations requirements that will be may be to much for us.

j-mracek avatar May 10 '22 15:05 j-mracek

@j-mracek Thanks for following up! Can you elaborate on the requirements? (Or perhaps point me to a document or issue or...)

nwalfield avatar May 10 '22 16:05 nwalfield

We are opened to any solution even Sequoia, but after the last DNF community meetings it looks like that there is a price in installations requirements that will be may be to much for us.

That is going to be tricky. The current implementation is deprecated and has known security problems related to key revocation. It also does not support armored signatures. I have some C++ code (not yet published) that can handle dearmoring, but there is no way I will be able to implement certificate canonicalization.

DemiMarie avatar May 10 '22 16:05 DemiMarie

We are opened to any solution even Sequoia, but after the last DNF community meetings it looks like that there is a price in installations requirements that will be may be to much for us.

This seems a bit peculiar, as rpm will be using Sequoia based solution anyway. For an idea about the "cost", see https://download.copr.fedorainfracloud.org/results/decathorpe/sequoia-test-builds/fedora-36-x86_64/04419972-rust-rpm-sequoia/

The library is about 1.8M in size and the only "extra" dependency is nettle for the low-end crypto, and this replaces any other crypto dependencies in rpm (ie openssl/gcrypt). ~~AIUI Sequoia supports openssl as an alternative too.~~

pmatilai avatar Jun 03 '22 09:06 pmatilai

AIUI Sequoia supports openssl as an alternative too.

We don't currently support OpenSSL as backend, but if there is interest in that we could add it (now that OpenSSL 3 is out).

teythoon avatar Jun 03 '22 09:06 teythoon

To elaborate on what @teythoon said: Adding OpenSSL support shouldn't be a major undertaking. Sequoia already supports three cryptographic libraries (Nettle, Rust Crypto and Windows CNG), so the existing abstractions probably won't require any refactoring. Nevertheless, it would be good to understand why Nettle is not sufficient. AIUI, Nettle is officially supported by RHEL.

nwalfield avatar Jun 03 '22 09:06 nwalfield

Doh, sorry I realize now that I mixed it with something else just recently gaining the openssl option.

The problem with nettle is that it's not openssl :laughing: I mean, for minimal container images and such people are pushing quite hard to minimize the dependencies and that includes consolidating on a single crypto library to the extent possible. The higher up in the stack you go, the less it matters because a crypto library gets lost in the other noise, but in down at the plumbing level where rpm lives, it still matters. And openssl just happens to be the most ubiquitous of those libraries, for the better or worse, and so that's what the consolidation efforts tend to gravitate to.

pmatilai avatar Jun 03 '22 10:06 pmatilai

for minimal container images and such people are pushing quite hard to minimize the dependencies and that includes consolidating on a single crypto library to the extent possible.

I hadn't considered that. Thanks for taking the time to follow up.

nwalfield avatar Jun 03 '22 10:06 nwalfield

Oh, and the other thing about openssl is FIPS, which AIUI nettle lacks. I may not care but it's a big deal in the enterprise world.

pmatilai avatar Jun 03 '22 13:06 pmatilai

Oh, and the other thing about openssl is FIPS, which AIUI nettle lacks. I may not care but it's a big deal in the enterprise world.

RHEL has FIPS certification for its OpenSSL library, though I think it just has OpenSSL call into the kernel crypto API (only semi-reasonable option, IMO).

DemiMarie avatar Jun 03 '22 21:06 DemiMarie

@j-mracek Thanks for following up! Can you elaborate on the requirements? (Or perhaps point me to a document or issue or...)

I am sorry for the late answer.

DNF needs to verify RPM GPG signature and for that purpose we use RPM API. When it fails DNF imports GPG keys into RPM DB. Because it is user confirmed operation we need to provide information about a key that will be important to user and for that purpose we use gpgme to parse the key.

For verification of repositories we use gpgme and we store imported keys in certain destination for particular repository (outside of rpm DB). Even for each user the location of imported keys differs. For verification of repositories we not only want to replace gpgme but also we would like to improve user experience. We would like to use imported keys in RPM DB as a primary source of GPG keys and only when it fails we want to use keys imported for particular repository at user specific location and then when it fails we can try to import fresh keys. What is completely missing in our interface for handling repo gpg keys is a management of already imported keys.

j-mracek avatar Jul 08 '22 12:07 j-mracek

Thanks for following up, that is helpful. Could you also comment on why using Sequoia would increase the cost of the installation requirements too much.

nwalfield avatar Jul 08 '22 13:07 nwalfield

Thanks for following up, that is helpful. Could you also comment on why using Sequoia would increase the cost of the installation requirements too much.

I did not have a time to test the RPM with Sequoia backend but according to Panu it should be not a problem. On the other hand if we can make our stack smaller (dnf5, rpm) it would be beneficial for everyone especially in minimal containers.

j-mracek avatar Jul 11 '22 05:07 j-mracek

For the record I'm working on adding OpenSSL backend to Sequoia PGP and while the signatures (signing and verification) are already working for both RSA keys and NIST curves the entire backend is not complete yet. Notably missing are: OCB and ed25519.

I see we have a master ticket for crypto backends: https://gitlab.com/sequoia-pgp/sequoia/-/issues/333 but nothing OpenSSL specific :sweat_smile:

As for the timeline I plan on submitting a PR/MR early September for review (when I'll get all tests passing). If you want to I can notify you when it's complete and merged.

wiktor-k avatar Aug 18 '22 08:08 wiktor-k

As for the timeline I plan on submitting a PR/MR early September for review (when I'll get all tests passing). If you want to I can notify you when it's complete and merged.

That would be appreciated.

pmatilai avatar Aug 18 '22 09:08 pmatilai

This took a liiiitle bit longer than I expected but I've submitted a patch to Sequoia to include OpenSSL backend: https://gitlab.com/sequoia-pgp/sequoia/-/merge_requests/1361

It passes all tests (and adds some more) and I'm planning to run the test suite from rpm against it but for details just follow the link above.

Have a nice day folks! :wave:

wiktor-k avatar Sep 29 '22 13:09 wiktor-k

Just encountered https://github.com/rpm-software-management/dnf5/blob/8f46d64fb1a44b307bae5fed7ce5ba233ea0ab0e/libdnf/rpm/rpm_signature.cpp#L107

Like the old wisdom goes, perfect is the enemy of good (enough) and offering something significantly better than this is not hard, even if it may not be purrfect.

pmatilai avatar Oct 14 '22 09:10 pmatilai

For people that track the subject the OpenSSL backend has been merged and released in sequoia-openpgp crate 1.13.0: https://gitlab.com/sequoia-pgp/sequoia/-/blob/main/openpgp/NEWS#L6

Have a nice day! 👋

wiktor-k avatar Jan 07 '23 11:01 wiktor-k

Awesome, thanks for all the work on this (to everybody involved)!

pmatilai avatar Jan 09 '23 08:01 pmatilai

Okay, back to our scheduled program :laughing:

With the work to add better error reporting in #2453, it suddenly looks more appetizing to improve the upper APIs too. Right now the API is basically "here's the keyring, see if something fits". Which is sufficient for rpm's own current use, but eg dnf wants to track keys per repo, which we don't handle at all.

So it seems the lowest level public verification API should take a key instead of a keyring, and for that the keyring needs to provide an API to look up keys. We had one but it was just recently axed because it was so bad otherwise... So currently that's kinda backwards: the API that only rpm needs is public, and the ones that others need are private :facepalm:

Then on top of that, we need that package siganture verification, which also needs to take just a key, take at least vsflags for controlling operation and have error message return pointer. I need to take a closer look at https://gitlab.com/dkg/openpgp-stateless-cli/-/issues/32...

pmatilai avatar Mar 31 '23 09:03 pmatilai

There should be a function to read a keyring (binary, or one or more concatenated ASCII-armor blocks) and extract all of the certificates, cf. https://github.com/rpm-software-management/rpm/issues/2487 . In Sequoia, this is done with CertParser.

nwalfield avatar Apr 18 '23 08:04 nwalfield