rpm-software-management / rpm

The RPM package manager
http://rpm.org
Other
498 stars 359 forks source link

Remove the internal OpenPGP parser #2414

Closed pmatilai closed 6 months ago

pmatilai commented 1 year ago

Continuing where #1935 left off: the internal OpenPGP parser has now been deprecated and declared essentially frozen, but this is a difficult and cumbersome position to hold for a number of reasons, including hindering other development work in this area.

We simply can't have Rust as a hard build dependency (at least not in a long while), so we need some exit strategy here. Basically, we either need to accept that rust-free means no signature checking at all, or we need to come up with an alternative, C-family based OpenPGP parser that we can actually use.

pmatilai commented 1 year ago

Actually there is a way to have the cake and eat it too: once upon a time, rpm called an external program (gpg back then) to verify signatures. It could do it again. It might be cumbersome to fit into the current API, it would be slower and there would be other compromises to be made no doubt, but between no security and slow security...

DemiMarie commented 1 year ago

If RPM goes this route, it should keep a small part of the internal parser. Specifically, it should keep the checks that the signature is a single OpenPGP signature packet of the correct type. This is a workaround for a known and unfixed denial-of-service vulnerability in GnuPG that I reported back in 2022, and should not increase the maintenance burden significantly. It also ensures consistency with the Sequoia implementation, which has a much stricter parser than GnuPG has.

pmatilai commented 1 year ago

Nope. Rpm is better off not knowing the damnest thing about OpenPGP format. An external helper would be considered a stop-gap measure for those unable/unwilling to use rpm-sequoia for some reason, nothing more.

Another possibility (and these aren't exclusive) is to split the existing parser to a separate project that can maintained by those wishing to do so. I simply want the parser, all of it, out of my hair.

mlschroe commented 1 year ago

I'm in favor of an separate project. I'm willing to take maintainership if nobody else steps up...

nwalfield commented 1 year ago

@mlschroe : Do you mind sharing your motivation. Does OpenSUSE plan to stick with the internal OpenPGP implementation?

mlschroe commented 1 year ago

For now we'll stick with the internal implementation. That doesn't mean we will never switch to sequoia when it's proven to be mature enough in a year or two. (Thanks Fedora for beta testing ;-) )

nwalfield commented 1 year ago

Thanks for the explanation.

pmatilai commented 1 year ago

I looked at this a bit and properly externalizing seems quite a lot of work because of two-way dependencies between it and rest of rpm.

I didn't actually try it yet, but I think splitting this into a submodule rather than a separate project would be far more reasonably achievable. Thoughts on that, especially @mlschroe if you're to maintain it?

pmatilai commented 11 months ago

I did some cleanup surrounding this today, managed to remove quite a bit of related unused cruft that has been just sitting there for twenty years.

We're now annoyingly close to be able to bury the rpmpgpval.h table stuff into the internal parser too. The only things needing that data now in the rest of rpm are

We shouldn't need 170 lines worth of data to answer those two questions...

pmatilai commented 11 months ago

The really annoying part about this is that if it wasn't for the stupid pgpIdentItem() function in librpmio, we could just hide a these two val->string conversions into a private helpers someplace in librpm. Back when it was added, pgpIdentItem() was a shortcut to avoid exposing a struct or a tonne of API to access the individual items (which we now have). So the shortcut is biting back now, doh...

kanavin commented 10 months ago

Just wanted to add the Yocto perspective: we don't have anything against sequoia, except its build dependencies. It needs both rust and clang (via one of the crates), rust and cland are both extremely heavy items to build, and we can't inject them into the core build sequence because it would lengthen builds far too much, even if you have a 96 core epyc machine to build on.

So outsourcing the crypto to external gpg executable would be very welcome. We can live with rpm verification disabled too.

@Conan-Kudo FYI

DemiMarie commented 10 months ago

So outsourcing the crypto to external gpg executable would be very welcome.

This isn’t going to happen because spawning an external program breaks in too many situations.

We can live with rpm verification disabled too.

This is a terrible idea from a security perspective.

kanavin commented 10 months ago

We can live with rpm verification disabled too.

This is a terrible idea from a security perspective.

In embedded linux world, production systems are rarely if ever updated from package feeds by a package manager. Rather, the whole root filesystem gets overwritten from an image file. Package manager is used to compose that root filesystem from local packages in a controlled CI environment (where package-level security isn't needed), and to allow developers to install additional items into a running system on their desks used for development and testing (where there's no need to sign packages either).

So Yocto can accept that regression in package security, we'll make sure to place warnings where appropriate.

DemiMarie commented 10 months ago

We can live with rpm verification disabled too.

This is a terrible idea from a security perspective.

In embedded linux world, production systems are rarely if ever updated from package feeds by a package manager. Rather, the whole root filesystem gets overwritten from an image file. Package manager is used to compose that root filesystem from local packages in a controlled CI environment (where package-level security isn't needed), and to allow developers to install additional items into a running system on their desks used for development and testing (where there's no need to sign packages either).

So Yocto can accept that regression in package security, we'll make sure to place warnings where appropriate.

Another option would be to use the host system’s RPM for verifying the packages.

kanavin commented 10 months ago

So Yocto can accept that regression in package security, we'll make sure to place warnings where appropriate.

Another option would be to use the host system’s RPM for verifying the packages.

Using host distro tools in cross-compilation builds is problematic, as we don't have control over what versions we're going to get, and how they are built and configured. To ensure things work in a reproducible manner, yocto builds its own rpm executable that can run on the build machine.

FWIW, the only host tools allowed to bootstrap the yocto build are python, gcc, wget, tar, git and various (de)compression utilities - things you need to fetch the sources, and bootstrap a cross-compiler.

DemiMarie commented 10 months ago

So Yocto can accept that regression in package security, we'll make sure to place warnings where appropriate.

Another option would be to use the host system’s RPM for verifying the packages.

Using host distro tools in cross-compilation builds is problematic, as we don't have control over what versions we're going to get, and how they are built and configured. To ensure things work in a reproducible manner, yocto builds its own rpm executable that can run on the build machine.

What about fetching a correctly configured binary RPM and verifying its hash before using it?

FWIW, the only host tools allowed to bootstrap the yocto build are python, gcc, wget, tar, git and various (de)compression utilities - things you need to fetch the sources, and bootstrap a cross-compiler.

Do you also need tools to cryptographically verify the downloaded sources? Or is that done in Python?

kanavin commented 10 months ago

Using host distro tools in cross-compilation builds is problematic, as we don't have control over what versions we're going to get, and how they are built and configured. To ensure things work in a reproducible manner, yocto builds its own rpm executable that can run on the build machine.

What about fetching a correctly configured binary RPM and verifying its hash before using it?

Possible, but comes with a significant support burden - someone needs to write and support the code that does this, and provide and update the binaries, for all of the architectures that yocto builds can run on. There needs to be tests and documentation too. And a possibility to opt out of it, and build rpm binary locally anyway.

When there are alternatives, we pick one which puts the least pressure on our very limited maintainer resources, or ideally makes it less than it was. The standard approach is to build the tools locally, and we'd rather stick with it.

FWIW, the only host tools allowed to bootstrap the yocto build are python, gcc, wget, tar, git and various (de)compression utilities - things you need to fetch the sources, and bootstrap a cross-compiler.

Do you also need tools to cryptographically verify the downloaded sources? Or is that done in Python?

For tarballs, their checksums are verified against local record of what they should be with python. For git checkouts we trust the git executable that the specified commit id will result in the correct tree, or there will be an error.

DemiMarie commented 10 months ago

@kanavin Are all of the RPMs used also built locally? In that case disabling signature checking is fine.

FYI, both rustc and clang are native cross compilers with support for multiple targets. The same rustc and clang that are used to compile programs for the build environment can also be used to compile code for the target system.

DemiMarie commented 10 months ago

The reason for getting rid of the internal OpenPGP parser is that it turns out to have security vulnerabilities that are exploitable if someone does gpg2 --export --armor -o s.asc FINGERPRINT && rpmkeys --import s.asc. Patching these vulnerabilities isn’t practical, as it would require a whole bunch of logic nobody is interested in implementing.

kanavin commented 10 months ago

@kanavin Are all of the RPMs used also built locally? In that case disabling signature checking is fine.

Yes of course. Yocto is fully self-contained, except for the bootstrap items mentioned above. It builds components from source, then makes its own packages from the binaries, then makes a system image from those packages. You can also pick what package format you prefer: ipk/deb/rpm, or all three.

DemiMarie commented 10 months ago

@kanavin Are all of the RPMs used also built locally? In that case disabling signature checking is fine.

Yes of course. Yocto is fully self-contained, except for the bootstrap items mentioned above. It builds components from source, then makes its own packages from the binaries, then makes a system image from those packages. You can also pick what package format you prefer: ipk/deb/rpm, or all three.

Ah, then there is not a security concern at all.

pmatilai commented 6 months ago

Finally managed to convince myself that it should be feasible (with reasonable amount of work) to have a "nopgp" build option by adding a dummy implementation of the internal PGP interface that just returns -ENOTHOME for everything, and allow choosing between libgcrypt and openssl for the hash functionality using existing code.

That way there's a minimal bootstrap option capable of building legitimate rpm packages, without making people choose between Rust or unmaintained security code with known holes.

pmatilai commented 6 months ago

https://github.com/rpm-software-management/rpm/pull/2984 implements the dummy PGP option.

pmatilai commented 6 months ago

I'm in favor of an separate project. I'm willing to take maintainership if nobody else steps up...

@mlschroe , shall I transfer the ownership of https://github.com/rpm-software-management/rpmpgp_legacy to you? Otherwise I'll just archive the thing.

mlschroe commented 6 months ago

Yes, I'll take the ownership for now. Thanks.

pmatilai commented 6 months ago

Okay, made you the admin of that repo. Have fun, as they say :sweat_smile: