RE: [PATCH 00/14] KEYS: Add support for PGP keys and signatures

[Roberto Sassu]

> From: Maciej S. Szmigiero [mailto:mail@xxxxxxxxxxxxxxxxxxxxx]
> Sent: Wednesday, January 12, 2022 9:16 PM
> On 12.01.2022 10:16, Roberto Sassu wrote:
> >> From: Maciej S. Szmigiero [mailto:mail@xxxxxxxxxxxxxxxxxxxxx]
> >> Sent: Tuesday, January 11, 2022 9:33 PM
> >> On 11.01.2022 19:03, Roberto Sassu wrote:
> >>> Support for PGP keys and signatures was proposed by David long time ago,
> >>> before the decision of using PKCS#7 for kernel modules signatures
> >>> verification was made. After that, there has been not enough interest to
> >>> support PGP too.
> >>>
> >>> Lately, when discussing a proposal of introducing fsverity signatures in
> >>> Fedora [1], developers expressed their preference on not having a separate
> >>> key for signing, which would complicate the management of the
> distribution.
> >>> They would be more in favor of using the same PGP key, currently used for
> >>> signing RPM headers, also for file-based signatures (not only fsverity, but
> >>> also IMA ones).
> >>
> >> Aren't PGP keys simply RSA / ECC / EdDSA keys with additional metadata?
> >> Can't they be unwrapped from their (complex) PGP format in userspace and
> >> loaded raw into the kernel, in a similar way as they are sometimes used
> >> for SSH authentication?
> >
> > Probably, this would be possible by introducing a new asymmetric
> > key subtype parsing PGP keys and signatures in a more simple format,
> > after conversion by user space. But still, a parser would be required.
> > To be honest, I would prefer to implement (actually David did) a
> > parser following an RFC, than developing a new one.
> 
> A parser in userspace is preferred to one in kernel since if there is
> a bug somewhere its consequences are much less severe.
> And experience shows that parsers are especially prone to bugs.
> A userspace implementation can also be tightly sandboxed for extra
> security.
> 
> There are many existing OpenPGP parsing libraries to choose from.

I understand your point. However, it does not seem to me less
risky to define a new format to upload the RSA key and the signature
to avoid the complexity of PGP. Also, it does not seem more
complex than PKCS#7, which is already in the kernel.

In addition, there are aspects of PGP that anyway have to be taken
into account. One example is the digest calculation, which depends
also on the PGP packet. Whenever the kernel verifies the signature,
the additional data need to be appended to the original data. This
risks to create more confusion, as the consumer of the data being
verified might not be prepared to handle the additional data
required for signature verification.

The kernel has already a well-defined way to process data with
a signature. It expects a data structure called module_signature
at the end of the data to verify, which include information required
for the verification such as the digest algorithm, key ID, etc. It
also has a selector called PKEY_ID_PGP, so that the code would
handle a PGP signature. This data structure does not include space
for the additional data required for the signature verification.

This patch set instead offers the new function verify_pgp_signature(),
which takes the same arguments as verify_pkcs7_signature(), and
can be used as in the example I mentioned above in a switch() where
the selector is the signature type.

This patch set also offers the individual functions called inside
verify_pgp_signature(), to support the case where the signature
verification process is split in multiple parts
(e.g. security/integrity/ima/ima_modsig.c). Also in this case,
the arguments passed to the PGP-related functions are similar
to the PKCS#7 ones.

Another concern that I have is that, the approach of using an
OpenPGP library still requires the Linux distribution vendors to
do a transformation from the source data they have to another
format. They have anyway to certify this transformation, even
if it is done in user space. Maybe it is easier to keep the original
data and verify the minimum necessary to handle PGP keys and
signature in the kernel, rather than verifying a library running
in user space with many other functions.

> >> This will save us from having to add complex parsers (a well-known source
> >> of bugs) into the kernel - I guess there aren't any plans to add an
> >> in-kernel PGP Web of Trust implementation.
> >
> > I extensively tested the implementation with an ad-hoc fault injector,
> > to see if the code can correctly handle errors. I also developed a
> > fuzzer to corrupt the data before it is read by the kernel. Finally,
> > I checked that there are not memory leaks. But I agree, there could
> > still be bugs.
> >
> > If you mean that a key can be added to the kernel if is vouched for
> > by another key in the built-in keyring, I actually implemented this
> > (was missing in the original implementation). Some keyrings, e.g. .ima,
> > have this restriction.
> >
> > The way this works is that, whenever you add a PGP key to the
> > kernel, the parser takes not only the public key and the user ID,
> > but also its signature by the same or another PGP key.
> >
> > The signature is verified when the key is added to the keyring
> > with that restriction, and only if the verification is successful
> > the key can be added.
> 
> I understand but it would be great to make use as much as possible of
> the existing in-kernel signature verification mechanisms.

Yes. I think this is the purpose of the asymmetric subtypes. They
introduce a parser for the specific format, but once the relevant
information are extracted, the in-kernel mechanisms are used.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> > Roberto
> 
> Thanks,
> Maciej