Deprecate the internal OpenPGP parser

[pmatilai]

There's something seriously wrong when a significant percent of package manager development discussions is about OpenPGP specification and its interpretation in the RPM context. This is negatively impacting development of RPM "core business", to the point that this has to stop. There's exactly one way to stop it, and that's getting rid of the internal parser, one way or the other.

Targeted for RPM 4.19 in 2023, and this also means that no major developments to the existing parser should be done, from here on it's strictly in critical fixes only -mode.

Ideally we'd allow alternative implementations through an rpm level abstraction, just like with the low-level crypto now, but that's probably beyond the initial scope. It'll be hard enough to port rpm to any single library alone I suspect, but that should also prune most of the roadblocks to using althernatives.

[DemiMarie]

Is a library written in C++ acceptable? Because I am currently working on one.

[mlschroe]

This most likely would mean that the library also will have to do the signature verification and thus also the file digest calculation.

[DemiMarie]

This most likely would mean that the library also will have to do the signature verification and thus also the file digest calculation.

Does this mean it would replace all of RPM’s internal crypto code?

[pmatilai]

@mlschroe , I don't see how file digest calculation is related?

This ticket at this point is merely a statement of intent, anything else is implementation details that will be worked out in time. I'm expecting this to be a bumpy ride with lots of unpleasant surprises of course. There's no intention to remove out rpm's hashing abstraction but if we get rid of the signature verification then that's a definite plus in my book.

[mlschroe]

My reasoning is this: the external library has a need to verify the self-sigs in the keys. Thus, it needs to do the hashing of the key material and run the pubkey algorithm.

I don't think you'll find a library that offers callbacks for this, so it will probably link against some of the standard crypto backend libs.

I guess is that it makes no sense to have another digesting layer in rpm when the pgp lib already provides this functionality. (The crypto backend needs to be identical anyway for consistency/certification reasons.)

[pmatilai]

If the PGP library actually exports digesting functionality then perhaps it could be used, but I wouldn't worry about that at this point. Rpm would operate without PGP of any kind, but it will not operate without hashes, so clearly hashes are a key functionality in rpm and from that POV it makes sense to have an abstraction over them within rpm regardless of what happens with PGP.

To me, the point of going to an external library is largely to be able to say "la-la-la, talk to those fellas" regarding bugs, crypto implementation choices and whatnot.

[Conan-Kudo]

Is a library written in C++ acceptable? Because I am currently working on one.

If it exports a C API interface, it should be fine.

[teythoon]

I'd love to see RPM use SOP to verify signatures. SOP is the OpenPGP community converging on a common API.

SOP: https://www.ietf.org/archive/id/draft-dkg-openpgp-stateless-cli-03.html

Using a commonly implemented API avoids vendor lock-in, and allows for a lot of flexibility: downstream users can select the SOP implementation based on their needs or constraints. Implementations can compete on features, quality, code size, or TCB size.

This is orthogonal to Demi's proposal: if the new library implements SOP or has a SOP implementation on top, it can simply be plugged in.

The signature verification interface has been designed with securing software supply chains in mind. See
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=872271 for the rationale.

Signature interface: https://www.ietf.org/archive/id/draft-dkg-openpgp-stateless-cli-03.html#name-verify-verify-detached-sign

Currently, there is no C API defined, but this is coming. We have the CLI interface, which is designed to be used programmatically. We also have Rust, Java, Python, and Ruby interfaces in various states of completeness.

[pmatilai]

@teythoon , thanks for the pointer. A common API that we don't need to try (and fail) invent sounds good on the outset.

[nwalfield]

Hi,

I've pushed an experimental OpenPGP backend based on Sequoia. Sequoia is released under the terms of the LGPL2+. Sequoia is in Fedora. Sequoia's default cryptographic backend is Nettle, which is one of the cryptographic libraries that RHEL supports.

For this port, I took the following approach: reimplement the existing PGP interface in terms of Sequoia using a Rust shim. To make life easier, I made pgpDigParams opaque to rpm. To accomplish this, I had to add a few accessor functions.

The port takes advantage of Sequoia's machinery to canonicalize OpenPGP certificates. This is extremely complicated and accounts for nearly a fifth of Sequoia's code. The port is also careful to check the validity of signatures.

Currently, the port is using Sequoia's null policy, which is not advisable. But, Sequoia's Standard Policy is stricter than what rpm currently appears to implement. If you decide that Sequoia is a reasonable way to go, we should discuss the right approach.

Currently, the code passes all tests except test #266. Test 266 checks that a certificate with a subkey that has no binding signature is rejected. (In particular, rpmchecksig.c:doImport calls rpmts.c:rpmtsImportPubkey, which calls rpmkeyring.c:rpmPubkeyNew, which calls rpmpgp.c:pgpPrtParams, which fails if a subkey is missing a binding signature.) This behavior of rejecting a certificate if there is an invalid component does not match the behavior of other OpenPGP implementations that I'm familiar with. Instead, invalid components are normally just ignored. This enables a degree of future compatibility.

After spending some time looking at the code, I've come to the conclusion that the current API is not ideal. One thing that I don't like is that the pgpDigGetParams can encapsulate either a certificate, a subkey, or a signature. I'd advise to switch to a new API, however, this might not be so easy given that the OpenPGP functionality is part of librpm's public API. An alternative approach is to add a second API, use it, recommend its use, and deprecate the existing API.

I look forward to your feedback.

[nwalfield]

I forgot to mention: I've written up build instructions to try the port.

[DemiMarie]

After spending some time looking at the code, I've come to the conclusion that the current API is not ideal. One thing that I don't like is that the pgpDigGetParams can encapsulate either a certificate, a subkey, or a signature. I'd advise to switch to a new API, however, this might not be so easy given that the OpenPGP functionality is part of librpm's public API. An alternative approach is to add a second API, use it, recommend its use, and deprecate the existing API.

You can (and should) arrange that calling functions with a pgpDigParams of the wrong type results in an error.

[pmatilai]

I've pushed an experimental OpenPGP backend based on Sequoia.

I'm getting '404 not found' for all these https://gitlab.com/sequoia-pgp/rpm-sequoia/ links so it's hard to appreciate this awesomeness 😅

What I could see is adding the missing API to make pgpDigParams opaque, that is very welcome as a PR regardless everything else. It was always the intention to make it opaque, just never got around to it.

[pmatilai]

This behavior of rejecting a certificate if there is an invalid component does not match the behavior of other OpenPGP implementations that I'm familiar with. Instead, invalid components are normally just ignored. This enables a degree of future compatibility.

If that's the case, I guess we should change rpm to follow suit.

[nwalfield]

I've pushed an experimental OpenPGP backend based on Sequoia.

I'm getting '404 not found' for all these https://gitlab.com/sequoia-pgp/rpm-sequoia/ links so it's hard to appreciate this awesomeness sweat_smile

What I could see is adding the missing API to make pgpDigParams opaque, that is very welcome as a PR regardless everything else. It was always the intention to make it opaque, just never got around to it.

Sorry about that! The repository was marked as private (that seems to be the default on gitlab). I've marked it public now, let me know if you still have issues.

[pmatilai]

@nwalfield , please open a separate ticket for discussing rpm-sequoia details, preferably with the intro from #1935 (comment). This is more of a general level tracker ticket, and I'd expect we have some more general level discussions to do still. Thanks.

[pmatilai]

Currently, there is no C API defined, but this is coming.

@teythoon (or others), is this being discussed in a place I could watch? Is there some (no matter how rough) draft of what it'd look like?

[teythoon]

Yes: https://gitlab.com/dkg/openpgp-stateless-cli/-/issues/32

[pmatilai]

Excellent, thanks!

[pmatilai]

This goal just took a major leap forward with the Sequoia-based backend by @nwalfield getting merged in #2043 🥳

[pmatilai]

Just FWIW, removing the internal parser in 4.19 may be premature still, but at least it needs to be non-default and strongly deprecated in that version. Technically the right place to drop is 4.20, but that would mean having to maintain it that much longer...

[pmatilai]

As mentioned in #1993 (comment), we can't pull the plug on the internal parser yet. But in order to keep the patient (that is, ourselves) alive through this, we need to saw off the bad limb, now. As in, for 4.19.

[pmatilai]

So, the internal parser is deprecated and non-default in the cmake build, and so the current target is achieved. Closing, removing will not be easy until we gain some alternative C-language family based alternative.