[Rpm-maint] Porting RPM to Sequoia PGP

[Panu Matilainen]

On 10/27/21 14:41, Justus Winter wrote:
> Justus Winter <justus at sequoia-pgp.org> writes:
> 
>> Panu Matilainen <pmatilai at redhat.com> writes:
>>
>>> Decoupling the implementation from the API would be beneficial to rpm in
>>> any case because
>>> a) it'd also enable implementing support for other libraries as well (eg
>>> RNP which is much closer in language family) and as long as the internal
>>> implementation is preserved, bootstrapping with minimal dependencies.
>>> b) doing so tends to have a positive impact on codebase
>>> c) having someone experienced with OpenPGP do it, the resulting API may
>>> even make some sense...
>>>
>>> So while I'm not at all eager to gain a Rust dependency and there'll be
>>> somewhat more (not less) code to maintain, but as per the plan above I
>>> think this sounds like a net positive for us. Always assuming somebody
>>> is willing to do the work that is.
>>
>> Great.  I'll give it a try and will likely come back with questions in
>> the process.
> 
> To get familiar with the code and interface, I wrote a SOP
> implementation on top of RPM's OpenPGP implementation.
> 
> https://www.ietf.org/archive/id/draft-dkg-openpgp-stateless-cli-03.html
> 
> It only verifies signatures, of course.  Nevertheless, I was then able
> to plug it into our interoperability test suite, and it turned up
> interesting results.  See below.
> 
> In the process, I had to fix a few issues in the implementation to get
> it to a point where it was able to verify a signature made by Sequoia
> PGP using a key created by Sequoia.

Interesting.

> 
> https://github.com/rpm-software-management/rpm/pull/1813
> 
> It seems to me that it is not only a point solution to verify
> signatures, but it has been written around the material that GnuPG
> creates rather than being a first-principles implementation.  That is
> unfortunate for a number of reasons.  First, it leads to a rather
> brittle implementation.  Second, it prevents users of RPM from
> transitioning to a different PGP implementation.  Finally, it even
> prevents GnuPG from ever evolving.

I would say that any GnuPG orientation is circumstancial: the 
specification is what the parser has been written to, but since GnuPG 
has traditionally been the only meaningful implementation people have 
had access to, it's the only thing ever tested. Which always taints the 
colors regardless of intentions.

> Regarding the interface, I was surprised how low-level it was.  Not only
> is that hard to use, but it also leaks implementation and OpenPGP
> details to the call sites.  Ideally, there should be one function that
> given a set of certificates, a signature, and the data should return
> whether the data could be authenticated.  Plus a couple of functions for
> diagnostics and miscellaneous stuff.
> 
> If I were to add a second backend, I'd first rework the interface to be
> considerably more high-level.

Yup. It's not the most loved subsystem of rpm, exactly. Nobody ever 
stepped up to do the major rework (I would say redesign but that would 
imply a previous design...) it needs, so whatever "interface" there is, 
is pretty much all ad-hoc added for whatever the current need was.

> Here are the interoperability test suite results.  Note that many tests
> simply fail for rpmsop because it doesn't do encryption:
> 
> https://tests.sequoia-pgp.org/rpmsop.html
> 
> These are my notes interpreting the results.  !!! marks security
> problems:
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Detached_signature_with_Subpackets
> 
> - issuer fingerprint handling would be nice
> - signatures with multiple issuers not well supported
> - invalid signatures with missing or unhashed creation time are considered valid
> - signatures with future creation time are considered valid

Future creation time is kinda intentionally ignored, because rpm is a 
low-level system tool that gets used in all manner of unfriendly 
circumstances such as rescue images where the clock being off the whack 
is the least of your worries. Throwing fits about future timestamps when 
the admin is trying to get the system back to functioning order would be 
rather antisocial. This is far from the only exception of its kind in 
the rpm context.

> 
> https://tests.sequoia-pgp.org/rpmsop.html#Detached_signatures__Linebreak_normalization
> 
> - text mode line ending normalization is broken (or not implemented)
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Detached_signatures_with_unknown_packets
> 
> - unknown signature versions should be ignored
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Detached_Sign-Verify_roundtrip_with_key__Bob___MD5
> 
> - accepts MD5 signatures !!!
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Signature_over_the_shattered_collision
> 
> - accepts SHA1 signatures !!!

Rpm needs to be able to work with content from the nineties, when MD5 
was still the hottest thing around, ditto with SHA1. At least openssl 
backend supports FIPS mode, which is where these get rejected as 
expected nowadays.

> https://tests.sequoia-pgp.org/rpmsop.html#Primary_key_binding_signatures
> 
> - accepts signing-capable subkeys without primary key binding signature !!!
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Key_Flags_Composition
> 
> - accepts signatures made by encryption subkeys !!!
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Temporary_validity
> 
> - pays no attention to timestamps and relations between cert and signature !!!
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Marker_Packet
> 
> - marker packets must be ignored
> 
> https://tests.sequoia-pgp.org/rpmsop.html#Mangled_ASCII_Armored_Signatures
> 
> - ASCII armor parser is very brittle

Interesting stuff. Not everything here seems relevant in rpm context but 
many are I'm sure.

> 
> The tests don't reflect that, but I saw that ECDSA is not supported.
> AIUI that means that in FIPS mode, only RSA signatures can be used.
> 
> 
> I also looked at the fix for CVE-2021-3521.  It adds code that does what
> I'd call partial certificate canonicalization.  There are some crucial
> steps missing, like reasoning about key metadata (at least keyflags) and
> checking primary key binding signatures. 

Well, the CVE was about subkey binding self-signatures, so that's what 
it tries to fix in a minimal way (considering backports and all). If it 
looks half-hearted then perhaps is because I find the whole issue rather 
moot: if you can convince an admin to 'rpm --import' a keyfile you can 
control, you don't need to fiddle with subkeys.

> Also, the code rejects some conforming certificates, like certs with
> an encryption-capable, non-RSA subkey followed by an signing-capable
> subkey.

Uh, if you say so. We have no means of wider testing of such material. 
Rpm traditionally gets fed rather "curated" keys that are made for that 
purpose only, and while we do fix such stuff when people complain, 
people very rarely complain. Whatever than means.

> I'm also worried that the test vectors added in that commit are somewhat
> misleading, at least the names don't match exactly what is in them:
> 
> CVE-2021-3521-badbind.asc: Sounds like it has a bad binding signature,
> but infact the binding signature is missing.
> 
> CVE-2021-3521-nosubsig.asc: Sounds like the subkey binding signature is
> missing when in fact the subkey packet has been duplicated.  There is a
> subkey with a valid binding signature, this is a perfectly valid
> certificate, but the test expects it to be rejected.
> 
> CVE-2021-3521-nosubsig-last.asc: This is the same file as
> CVE-2021-3521-badbind.asc.

Ugh. Seems like I've gotten my reproducers mixed up at some point in the 
process. Thanks for spotting, I'll look into it.

	- Panu -

> 
> Justus
>