Justus Winter <jus...@sequoia-pgp.org> writes:

> Panu Matilainen <pmati...@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.

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.

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.


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

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 !!!

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


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.  Also, the code rejects some
conforming certificates, like certs with an encryption-capable, non-RSA
subkey followed by an signing-capable subkey.

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.

Justus

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to