On Tue, Jun 20, 2023 at 08:15:42PM +0100, Luca Boccassi wrote: > On Tue, 20 Jun 2023 at 05:23, 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> > > --- > > > > v3: fixed ioctl name, and more updates to address Luca's comments > > v2: updated two paragraphs of fsverity.rst to address Luca's comments > > > > Documentation/filesystems/fsverity.rst | 192 ++++++++++++++++--------- > > 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, 149 insertions(+), 81 deletions(-) > <...> > > +- fs-verity builtin signatures are in PKCS#7 format, and the public > > + keys are in X.509 format. These formats are commonly used, > > + including by some other kernel features (which is why the fs-verity > > + builtin signatures use them), and are very feature rich. > > + Unfortunately, history has shown that code that parses and handles > > + these formats (which are from the 1990s and are based on ASN.1) > > + often has vulnerabilities as a result of their complexity. This > > + complexity is not inherent to the cryptography itself. > > + > > + fs-verity users who do not need advanced features of X.509 and > > + PKCS#7 should strongly consider using simpler formats, such as plain > > + Ed25519 keys and signatures, and verifying signatures in userspace. > > + > > + fs-verity users who choose to use X.509 and PKCS#7 anyway should > > + still consider that verifying those signatures in userspace is more > > + flexible (for other reasons mentioned earlier in this document) and > > + eliminates the need to enable CONFIG_FS_VERITY_BUILTIN_SIGNATURES > > + and its associated increase in kernel attack surface. In some cases > > + it can even be necessary, since advanced X.509 and PKCS#7 features > > + do not always work as intended with the kernel. For example, the > > + kernel does not check X.509 certificate validity times. > > + > > + Note: IMA appraisal, which supports fs-verity, does not use PKCS#7 > > + for its signatures, so it partially avoids the issues discussed > > + here. IMA appraisal does use X.509. > > Thank you for the update, I find this new version much better as it is > clearly scoped for the specific case of fs-verity, hence: > > Reviewed-by: Luca Boccassi <bluca@xxxxxxxxxx> Great, I'm glad we were finally able to arrive at a version that you're okay with. - Eric