Re: [PATCH] fsverity: improve documentation for builtin signature support

[Eric Biggers <ebiggers@kernel.org>]

On Fri, Jun 16, 2023 at 02:27:28PM +0100, Luca Boccassi wrote:
> > Unfortunately just because PKCS#7, X.509, and ASN.1 is being used does not mean
> > it is a good idea.  Have you read the kernel code that implements these formats?
> > A few years ago I went through some of it.  Here are some of the bugs I fixed:
> >
> >     2eb9eabf1e86 ("KEYS: fix out-of-bounds read during ASN.1 parsing")
> >     624f5ab8720b ("KEYS: fix NULL pointer dereference during ASN.1 parsing [ver #2]")
> >     e0058f3a874e ("ASN.1: fix out-of-bounds read when parsing indefinite length item")
> >     81a7be2cd69b ("ASN.1: check for error from ASN1_OP_END__ACT actions")
> >     0f30cbea005b ("X.509: reject invalid BIT STRING for subjectPublicKey")
> >     54c1fb39fe04 ("X.509: fix comparisons of ->pkey_algo")
> >     971b42c038dc ("PKCS#7: fix certificate chain verification")
> >     29f4a67c17e1 ("PKCS#7: fix certificate blacklisting")
> >     437499eea429 ("X.509: fix BUG_ON() when hash algorithm is unsupported")
> >     4b34968e77ad ("X.509: fix NULL dereference when restricting key with unsupported_sig")
> 
> I have no doubt that there are bugs, as I have no doubts that there
> are bugs in every other subsystem, including fsverity, once you start
> looking hard enough.

My point was not that there are bugs, but rather that there are *unnecessary*
bugs (many with possible security impact) that are directly caused by the
complexities of these formats versus the alternatives.

> That's not the point. The point is that having
> the documentation of one kernel subsystem disparaging the mechanisms
> that are central to other kernel subsystems' functionality is weird
> and out of place. Something like that is fine to post on social media
> or a blog post or so. A user jumping from one page of kernel doc
> saying, paraphrasing heavily for the sake of argument, "use pkcs7 to
> ensure the security of your system via secure boot, measured boot and
> signed kernel modules" and another saying "pkcs7 is bad and broken,
> stay away from it" is just strange, confusing and incoherent from the
> point of view of a reader.

I'll add a note that PKCS#7 and X.509 should still be used in situations where
they are the only option.  I think that would handle your main concern here,
which is that people might misunderstand the paragraph as recommending using no
signatures, instead of signatures using a PKCS#7 and X.509 based system.

I don't think it would be appropriate to remove the paragraph entirely.  It
provides useful information that helps users decide what type of signatures to
use.  I understand that people who are invested greatly into PKCS#7 and X.509
based systems might be resistant to learning about the problems with these
formats, but that is to be expected.

- Eric