[Rpm-maint] [rpm-software-management/rpm] Require creation time to be unique and hashed (PR #1912)

[Demi Marie Obenour]

@DemiMarie commented on this pull request.



>  	    impl = *p;
-	    if (!(_digp->saved & PGPDIG_SAVED_TIME) &&
-		(sigtype == PGPSIGTYPE_POSITIVE_CERT || sigtype == PGPSIGTYPE_BINARY || sigtype == PGPSIGTYPE_TEXT || sigtype == PGPSIGTYPE_STANDALONE))

> > > > > `gpgv` canonicalizes certificates. Not doing so is unsafe. Saying RPM implements a subset of that is like saying `/bin/true` implements a subset of `gpgv`.
> > > > 
> > > > 
> > > > Is there a specification for certificate canonicalization anywhere?
> > > 
> > > 
> > > Not that I'm aware, but Paul has a good write-up on how to properly verify signatures: https://blog.jabberhead.tk/2021/04/03/why-signature-verification-in-openpgp-is-hard/
> > 
> > 
> > Would it be possible for the specification to include detailed algorithms for verifying a signature, canonicalizing a certificate, determining when a signature is malformed and should be rejected, and other important facets of OpenPGP?
> 
> Historically, the OpenPGP spec has mostly concerned itself with syntax, not semantics. Therefore, implementing OpenPGP securely requires a lot of tribal knowledge. Reading RFC4880 and implementing something based only on that reading is a recipe for disaster.

This is very much a problem, and I suspect it is one of the reasons that OpenPGP has a reputation for being a very brittle protocol.

Some of what I would like to know:

- Is it safe to reject critical unhashed subpackets?  These seem to make no sense at all in most use-cases, but I could see them arising when the entire signature (including the unhashed subpackets) is itself signed.
- Is it safe to reject timestamps of length other than 4?  Or should I only do so if the subpacket is marked critical?
- Is it safe to reject signatures that have subpackets that make no sense for the type of signature they are?  An example would be a key flags or key expiration subpacket in a data signature.
- Is it safe to reject a transferable public key that does not have its packets in strictly the correct order according to RFC4880?
- Is it safe to reject signatures with inconsistent information?  For instance, if there is a v4 fingerprint subpacket and an 8-byte key ID subpacket, but the two are not consistent with each other, this seems to imply a problem on the signer’s end.  A single signature could validly have both v4 and v5 fingerprint subpackets, but my understanding is that an 8-byte key ID subpacket only makes sense as the id of a v4 key, so it should match the v4 fingerprint.
- Where should I ask questions like this?

> Many things are possible. We have often talked about writing up that tribal knowledge in a separate document, like an annotated version of the RFC. That takes people with that kind of knowledge a lot of time. As always, someone needs to do it.

This definitely needs to be done.  Otherwise there _will_ be more half-baked implementations, because there _will_ be times when people need to write their own library for some reason or another.

> > * Some implementations, by design, only implement a subset of OpenPGP.  For instance, a package manager only needs to implement signature verification, so there is no reason to implement other parts of OpenPGP such as signing, encryption, or decryption.  It would be nice to split the test suite in such a way as to be able to easily exclude tests that simply are not applicable for a given implementation.
> 
> Yeah, that is half-way implemented.  It "only" requires annotating each test with the required operations, then selecting to run just those tests.  But, I'm very skeptical about these point solution implementations.  RPM's is the first verification-only implementation I have encountered.  And, I have plugged it into the test suite and it was informative even though some tests were not applicable.

The implementation I have been working on in my spare time is also verification-only, for the simple reason that none of the use-cases I have for it will ever need to handle any other aspect of OpenPGP.

> > * It would be nice to have more tests for very malformed packets and subpackets, such as ones that might be produced by a fuzzer.  This is very important for finding and fixing memory unsafety and other bugs in implementations.  In RPM-Oxide, for instance, I am much less worried about wrongly rejecting a good signature than about wrongly accepting a bad one.
> 
> The test suite is about semantics of OpenPGP, not about the memory safety of implementations. I think implementations are better off with compiler-supported fuzzing techniques. Having said that, Sequoia includes a protocol-aware fuzzer and uses it in its tests. It could conceivably be adapted to test another implementation.

Even the raw fuzz corpus would be quite useful.  It should catch should-fail cases like “MPIs don’t fit in the packet” or “wrong number of MPIs”.  I started adding a fuzz harness to RPM a while back, but it stalled.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1912#discussion_r812151315
You are receiving this because you are subscribed to this thread.

Message ID: <rpm-software-management/rpm/pull/1912/review/890078497 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20220302/8e53ded9/attachment-0001.html>