From: Luca Boccassi <bluca@debian.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: fsverity@lists.linux.dev, linux-integrity@vger.kernel.org,
linux-doc@vger.kernel.org, Colin Walters <walters@verbum.org>,
Alexander Larsson <alexl@redhat.com>,
Victor Hsieh <victorhsieh@google.com>
Subject: Re: [PATCH] fsverity: improve documentation for builtin signature support
Date: Mon, 19 Jun 2023 20:39:08 +0100 [thread overview]
Message-ID: <CAMw=ZnQitwaR+oCKzxPxAZ1Ehs-86bfHbo0uPTeKe5a+5geV6A@mail.gmail.com> (raw)
In-Reply-To: <20230617045103.GA1090@sol.localdomain>
On Sat, 17 Jun 2023 at 05:51, Eric Biggers <ebiggers@kernel.org> wrote:
>
> 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.
Given this is a kernel doc (as opposed to, say, fsverity-utils's
readme), my concern is with having confusing and contradicting
documentation. If you reword it to make it specific to fsverity or so,
it would probably be fine.
Kind regards,
Luca Boccassi
prev parent reply other threads:[~2023-06-19 19:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-15 23:05 [PATCH] fsverity: improve documentation for builtin signature support Eric Biggers
2023-06-16 1:10 ` Luca Boccassi
2023-06-16 2:17 ` Eric Biggers
2023-06-16 9:31 ` Roberto Sassu
2023-06-16 12:57 ` Luca Boccassi
2023-06-16 13:15 ` Roberto Sassu
2023-06-16 13:27 ` Luca Boccassi
2023-06-17 4:51 ` Eric Biggers
2023-06-19 19:39 ` Luca Boccassi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMw=ZnQitwaR+oCKzxPxAZ1Ehs-86bfHbo0uPTeKe5a+5geV6A@mail.gmail.com' \
--to=bluca@debian.org \
--cc=alexl@redhat.com \
--cc=ebiggers@kernel.org \
--cc=fsverity@lists.linux.dev \
--cc=linux-doc@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=victorhsieh@google.com \
--cc=walters@verbum.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).