On Fri, 16 Jun 2023 at 00:07, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > From: Eric Biggers <ebiggers@xxxxxxxxxx> > > fsverity builtin signatures (CONFIG_FS_VERITY_BUILTIN_SIGNATURES) aren't > the only way to do signatures with fsverity, and they have some major > limitations. Yet, more users have tried to use them, e.g. recently by > https://github.com/ostreedev/ostree/pull/2640. In most cases this seems > to be because users aren't sufficiently familiar with the limitations of > this feature and what the alternatives are. > > Therefore, make some updates to the documentation to try to clarify the > properties of this feature and nudge users in the right direction. > > Note that the Integrity Policy Enforcement (IPE) LSM, which is not yet > upstream, is planned to use the builtin signatures. (This differs from > IMA, which uses its own signature mechanism.) For that reason, my > earlier patch "fsverity: mark builtin signatures as deprecated" > (https://lore.kernel.org/r/20221208033548.122704-1-ebiggers@xxxxxxxxxx), > which marked builtin signatures as "deprecated", was controversial. > > This patch therefore stops short of marking the feature as deprecated. > I've also revised the language to focus on better explaining the feature > and what its alternatives are. > > Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx> > --- > > This patch applies to https://git.kernel.org/pub/scm/fs/fsverity/linux.git/log/?h=for-next > > Documentation/filesystems/fsverity.rst | 176 ++++++++++++++++--------- > fs/verity/Kconfig | 16 +-- > fs/verity/enable.c | 2 +- > fs/verity/open.c | 8 +- > fs/verity/read_metadata.c | 4 +- > fs/verity/signature.c | 8 ++ > 6 files changed, 139 insertions(+), 75 deletions(-) > > diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst > index ede672dedf110..e990149cfdf5c 100644 > --- a/Documentation/filesystems/fsverity.rst > +++ b/Documentation/filesystems/fsverity.rst Most of the patch looks fine, two notes: > +- Trusted userspace code. When the accesses to a file happen in a > + well-defined way, userspace code can authenticate the file's > + fs-verity digest before accessing the file. This can be done by > + verifying a signature of the fs-verity file digest using any > + userspace cryptographic library that supports digital signatures. > + Consider using `libsodium > + <https://libsodium.gitbook.io/doc/public-key_cryptography/public-key_signatures>`_ > + or `Tink <https://developers.google.com/tink/digitally-sign-data>`_. > + Other options include OpenSSL, JCA, and libgcrypt. This should at least mention something like "depending on whether the threat model allows trusting userspace with such tasks", because it is by no means guaranteed that it is the case. > +- fs-verity builtin signatures are in PKCS#7 format, and the public > + keys are in X.509 format. These data formats are complex and prone > + to vulnerabilities, so parsing them is preferably done in userspace. > + (fs-verity builtin signatures were made to use these formats because > + other kernel subsystems, such as the module loader, unfortunately > + used these formats already.) Most cryptographic libraries also > + support working with raw keys and signatures, which are much > + simpler. For example, consider using `libsodium > + <https://libsodium.gitbook.io/doc/public-key_cryptography/public-key_signatures>`_. > + > + IMA appraisal, which supports fs-verity, does not use PKCS#7, so it > + partially avoids this issue as well (though it does use X.509). The kernel makes extensive use of PKCS7, it's the foundation of the trust chain with secure boot (and kernel modules as noted) after all, among other things, so this description looks very out of place as part of the same project. Readers might be led to believe that using secure boot or signed modules is useless, or worse, dangerous, and that it's better not to, and I'm quite sure that's not something we want. Kind regards, Luca Boccassi
