On 10/27/21 14:41, Justus Winter wrote:
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.

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


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

Reply via email to