| Message ID | 20221208033548.122704-1-ebiggers@kernel.org (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | fsverity: mark builtin signatures as deprecated | expand |
On Wed, 2022-12-07 at 19:35 -0800, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > fsverity builtin signatures, at least as currently implemented, are a > mistake and should not be used. They mix the authentication policy > between the kernel and userspace, which is not a clean design and causes > confusion. For builtin signatures to actually provide any security > benefit, userspace still has to enforce that specific files have > fsverity enabled. Since userspace needs to do this, a better design is > to have that same userspace code do the signature check too. > > That allows better signature formats and algorithms to be used, avoiding > in-kernel parsing of the notoriously bad PKCS#7 format. It is also > needed anyway when different keys need to be trusted for different > files, or when it's desired to use fsverity for integrity-only or > auditing on some files and for authenticity on other files. Basically, > the builtin signatures don't work for any nontrivial use case. > > (IMA appraisal is another alternative. It goes in the opposite > direction -- the full policy is moved into the kernel.) > > For these reasons, the master branch of AOSP no longer uses builtin > signatures. It still uses fsverity for some files, but signatures are > verified in userspace when needed. > > None of the public uses of builtin signatures outside Android seem to > have gotten going, either. Support for builtin signatures was added to > RPM. However, https://fedoraproject.org/wiki/Changes/FsVerityRPM was > subsequently rejected from Fedora and seems to have been abandoned. > There is also https://github.com/ostreedev/ostree/pull/2269, which was > never merged. Neither proposal mentioned a plan to set > fs.verity.require_signatures=1 and enforce that files have fs-verity > enabled -- so, they would have had no security benefit on their own. > > I'd be glad to hear about any other users of builtin signatures that may > exist, and help with the details of what should be used instead. > > Anyway, the feature can't simply be removed, due to the need to maintain > backwards compatibility. But let's at least make it clear that it's > deprecated. Update the documentation accordingly, and rename the > kconfig option to CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG. Also remove > the kconfig option from the s390 defconfigs, as it's unneeded there. Hi, Thanks for starting this discussion, it's an interesting topic. At MSFT we use fsverity in production, with signatures enforced by the kernel (and policy enforced by the IPE LSM). It's just too easy to fool userspace with well-timed swaps and who knows what else. This is not any different from dm-verity from our POV, it complements it. I very much want the kernel to be in charge of verification and validation, at the time of use. In essence, I very strongly object to marking this as deprecated. It is entirely ok if at Google you want to move everything out of the kernel, you know your use case best so if that works better for you that's absolutely fine (and thus your other patch looks good to me), but I don't think it should be deprecated for everybody else too.
On Thu, Dec 08, 2022 at 10:43:01AM +0000, Luca Boccassi wrote: > On Wed, 2022-12-07 at 19:35 -0800, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > fsverity builtin signatures, at least as currently implemented, are a > > mistake and should not be used. They mix the authentication policy > > between the kernel and userspace, which is not a clean design and causes > > confusion. For builtin signatures to actually provide any security > > benefit, userspace still has to enforce that specific files have > > fsverity enabled. Since userspace needs to do this, a better design is > > to have that same userspace code do the signature check too. > > > > That allows better signature formats and algorithms to be used, avoiding > > in-kernel parsing of the notoriously bad PKCS#7 format. It is also > > needed anyway when different keys need to be trusted for different > > files, or when it's desired to use fsverity for integrity-only or > > auditing on some files and for authenticity on other files. Basically, > > the builtin signatures don't work for any nontrivial use case. > > > > (IMA appraisal is another alternative. It goes in the opposite > > direction -- the full policy is moved into the kernel.) > > > > For these reasons, the master branch of AOSP no longer uses builtin > > signatures. It still uses fsverity for some files, but signatures are > > verified in userspace when needed. > > > > None of the public uses of builtin signatures outside Android seem to > > have gotten going, either. Support for builtin signatures was added to > > RPM. However, https://fedoraproject.org/wiki/Changes/FsVerityRPM was > > subsequently rejected from Fedora and seems to have been abandoned. > > There is also https://github.com/ostreedev/ostree/pull/2269, which was > > never merged. Neither proposal mentioned a plan to set > > fs.verity.require_signatures=1 and enforce that files have fs-verity > > enabled -- so, they would have had no security benefit on their own. > > > > I'd be glad to hear about any other users of builtin signatures that may > > exist, and help with the details of what should be used instead. > > > > Anyway, the feature can't simply be removed, due to the need to maintain > > backwards compatibility. But let's at least make it clear that it's > > deprecated. Update the documentation accordingly, and rename the > > kconfig option to CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG. Also remove > > the kconfig option from the s390 defconfigs, as it's unneeded there. > > Hi, > > Thanks for starting this discussion, it's an interesting topic. > > At MSFT we use fsverity in production, with signatures enforced by the > kernel (and policy enforced by the IPE LSM). It's just too easy to fool > userspace with well-timed swaps and who knows what else. This is not > any different from dm-verity from our POV, it complements it. I very > much want the kernel to be in charge of verification and validation, at > the time of use. Well, IPE is not upstream, and it duplicates functionality that already exists upstream (IMA appraisal). So from an upstream perspective it doesn't really matter currently. That's interesting that you've already deployed IPE in production anyway. To re-iterate my question at https://lore.kernel.org/r/YqKGcdM3t5gjqBpq@sol.localdomain which was ignored, can you elaborate on why IPE should exist when IMA appraisal already exists (and already supports fsverity), and why IPE uses the fsverity builtin signatures? And are you sure that X.509 and PKCS#7 should be used in a new system? These days, if you go through any sort of crypto or security review, you will be told not to use those formats since they are overly complex and prone to bugs. - Eric
On Thu, 2022-12-08 at 12:55 -0800, Eric Biggers wrote: > On Thu, Dec 08, 2022 at 10:43:01AM +0000, Luca Boccassi wrote: > > On Wed, 2022-12-07 at 19:35 -0800, Eric Biggers wrote: > > > From: Eric Biggers <ebiggers@google.com> > > > > > > fsverity builtin signatures, at least as currently implemented, > > > are a > > > mistake and should not be used. They mix the authentication > > > policy > > > between the kernel and userspace, which is not a clean design and > > > causes > > > confusion. For builtin signatures to actually provide any > > > security > > > benefit, userspace still has to enforce that specific files have > > > fsverity enabled. Since userspace needs to do this, a better > > > design is > > > to have that same userspace code do the signature check too. > > > > > > That allows better signature formats and algorithms to be used, > > > avoiding > > > in-kernel parsing of the notoriously bad PKCS#7 format. It is > > > also > > > needed anyway when different keys need to be trusted for > > > different > > > files, or when it's desired to use fsverity for integrity-only or > > > auditing on some files and for authenticity on other files. > > > Basically, > > > the builtin signatures don't work for any nontrivial use case. > > > > > > (IMA appraisal is another alternative. It goes in the opposite > > > direction -- the full policy is moved into the kernel.) > > > > > > For these reasons, the master branch of AOSP no longer uses > > > builtin > > > signatures. It still uses fsverity for some files, but > > > signatures are > > > verified in userspace when needed. > > > > > > None of the public uses of builtin signatures outside Android > > > seem to > > > have gotten going, either. Support for builtin signatures was > > > added to > > > RPM. However, > > > https://fedoraproject.org/wiki/Changes/FsVerityRPM was > > > subsequently rejected from Fedora and seems to have been > > > abandoned. > > > There is also https://github.com/ostreedev/ostree/pull/2269, > > > which was > > > never merged. Neither proposal mentioned a plan to set > > > fs.verity.require_signatures=1 and enforce that files have fs- > > > verity > > > enabled -- so, they would have had no security benefit on their > > > own. > > > > > > I'd be glad to hear about any other users of builtin signatures > > > that may > > > exist, and help with the details of what should be used instead. > > > > > > Anyway, the feature can't simply be removed, due to the need to > > > maintain > > > backwards compatibility. But let's at least make it clear that > > > it's > > > deprecated. Update the documentation accordingly, and rename the > > > kconfig option to CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG. Also > > > remove > > > the kconfig option from the s390 defconfigs, as it's unneeded > > > there. > > > > Hi, > > > > Thanks for starting this discussion, it's an interesting topic. > > > > At MSFT we use fsverity in production, with signatures enforced by > > the > > kernel (and policy enforced by the IPE LSM). It's just too easy to > > fool > > userspace with well-timed swaps and who knows what else. This is > > not > > any different from dm-verity from our POV, it complements it. I > > very > > much want the kernel to be in charge of verification and > > validation, at > > the time of use. > > Well, IPE is not upstream, and it duplicates functionality that > already exists > upstream (IMA appraisal). So from an upstream perspective it doesn't > really > matter currently. That's interesting that you've already deployed > IPE in > production anyway. To re-iterate my question at > https://lore.kernel.org/r/YqKGcdM3t5gjqBpq@sol.localdomain which was > ignored, > can you elaborate on why IPE should exist when IMA appraisal already > exists (and > already supports fsverity), and why IPE uses the fsverity builtin > signatures? Yes, IPE has been in production for years, it used in the feature described in the couple of minutes of Ignite starting at: https://www.youtube.com/watch?v=PO5ijv6WDv0&t=608s But I am not in the team that works on IPE, so I am not the best person to answer the first question, I do not have a good enough understanding of the implementation details of IPE/IMA to be able to say. I believe a new revision will be submitted soon, the submitter is the right person to ask that question. The second question is easy: because the kernel is the right place for our use case to do this verification and enforcement, exactly like dm- verity does. Userspace is largely untrusted, or much lower trust anyway. > And are you sure that X.509 and PKCS#7 should be used in a new > system? These > days, if you go through any sort of crypto or security review, you > will be told > not to use those formats since they are overly complex and prone to > bugs. Yes. We need to use the same mechanism as dm-verity, and in fact many more systems, already make use of.
On Thu, Dec 08, 2022 at 12:55:33PM -0800, Eric Biggers wrote: > On Thu, Dec 08, 2022 at 10:43:01AM +0000, Luca Boccassi wrote: > > On Wed, 2022-12-07 at 19:35 -0800, Eric Biggers wrote: > > > From: Eric Biggers <ebiggers@google.com> > > > > > > fsverity builtin signatures, at least as currently implemented, are a > > > mistake and should not be used. They mix the authentication policy > > > between the kernel and userspace, which is not a clean design and causes > > > confusion. For builtin signatures to actually provide any security > > > benefit, userspace still has to enforce that specific files have > > > fsverity enabled. Since userspace needs to do this, a better design is > > > to have that same userspace code do the signature check too. > > > > > > That allows better signature formats and algorithms to be used, avoiding > > > in-kernel parsing of the notoriously bad PKCS#7 format. It is also > > > needed anyway when different keys need to be trusted for different > > > files, or when it's desired to use fsverity for integrity-only or > > > auditing on some files and for authenticity on other files. Basically, > > > the builtin signatures don't work for any nontrivial use case. > > > > > > (IMA appraisal is another alternative. It goes in the opposite > > > direction -- the full policy is moved into the kernel.) > > > > > > For these reasons, the master branch of AOSP no longer uses builtin > > > signatures. It still uses fsverity for some files, but signatures are > > > verified in userspace when needed. > > > > > > None of the public uses of builtin signatures outside Android seem to > > > have gotten going, either. Support for builtin signatures was added to > > > RPM. However, https://fedoraproject.org/wiki/Changes/FsVerityRPM was > > > subsequently rejected from Fedora and seems to have been abandoned. > > > There is also https://github.com/ostreedev/ostree/pull/2269, which was > > > never merged. Neither proposal mentioned a plan to set > > > fs.verity.require_signatures=1 and enforce that files have fs-verity > > > enabled -- so, they would have had no security benefit on their own. > > > > > > I'd be glad to hear about any other users of builtin signatures that may > > > exist, and help with the details of what should be used instead. > > > > > > Anyway, the feature can't simply be removed, due to the need to maintain > > > backwards compatibility. But let's at least make it clear that it's > > > deprecated. Update the documentation accordingly, and rename the > > > kconfig option to CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG. Also remove > > > the kconfig option from the s390 defconfigs, as it's unneeded there. > > > > Hi, > > > > Thanks for starting this discussion, it's an interesting topic. > > > > At MSFT we use fsverity in production, with signatures enforced by the > > kernel (and policy enforced by the IPE LSM). It's just too easy to fool > > userspace with well-timed swaps and who knows what else. This is not > > any different from dm-verity from our POV, it complements it. I very > > much want the kernel to be in charge of verification and validation, at > > the time of use. > > Well, IPE is not upstream, and it duplicates functionality that already exists > upstream (IMA appraisal). So from an upstream perspective it doesn't really > matter currently. That's interesting that you've already deployed IPE in > production anyway. To re-iterate my question at > https://lore.kernel.org/r/YqKGcdM3t5gjqBpq@sol.localdomain which was ignored, > can you elaborate on why IPE should exist when IMA appraisal already exists (and > already supports fsverity), and why IPE uses the fsverity builtin signatures? > And are you sure that X.509 and PKCS#7 should be used in a new system? These > days, if you go through any sort of crypto or security review, you will be told > not to use those formats since they are overly complex and prone to bugs. > > - Eric IPE is just a different approach, like btrfs and ext4, for a small subset of IMA's features: appraisal. IMA is a fantastic solution for a whole lot of scenarios that IPE will not cover, now or ever. In fact, in production, we use IMA (both measurement and appraisal) and IPE side-by-side. They're not mutually exclusive. IPE's approach is to really focus on the policy enforcement side, delegating the authenticity (or even just integrity) claim to other subsystems. It does this by enforcing an opaqueness between these integrations with other subsystems, drawing a clear line between the mechanism of the authenticity claim, and the enforcement of that claim against authorized values in policy. This allows for a variable security bar based on what mechanisms are selected to be used in the system, and what are present in the policy. For instance, the security bar of a system using dm-verity with signed root hashes as its primary authenticity claim, as opposed to fs-verity will have a different security bar. This is why IPE uses the built-in signatures: they are managed by fs-verity, and IPE does not implement its own signature format or other method of validating either the integrity or authenticity of a data source, explicitly, as that would be contrary to its design philosophy. We have also noticed many use cases for the fs-verity build-in signatures. Proposals exist to use them[1]. Package managers were updated to use them[2]. We are successfully using them in production. Therefore we prefer to keep the existing build-in signatures. [1]https://fedoraproject.org/wiki/Changes/FsVerityRPM#Enable_fs-verity_in_RPM [2]https://github.com/rpm-software-management/rpm/issues/1121 - Fan
On Thu, Dec 08, 2022 at 09:37:29PM +0000, Luca Boccassi wrote: > > The second question is easy: because the kernel is the right place for > our use case to do this verification and enforcement, exactly like dm- > verity does. Well, dm-verity's in-kernel signature verification support is a fairly new feature. Most users of dm-verity don't use it, and will not be using it. > Userspace is largely untrusted, or much lower trust anyway. Yes, which means the kernel is highly trusted. Which is why parsing complex binary formats, X.509 and PKCS#7, in C code in the kernel is not a great idea... - Eric
On Fri, Dec 09, 2022 at 02:17:19PM -0800, Fan Wu wrote: > We have also noticed many use cases for the fs-verity build-in signatures. Proposals > exist to use them[1]. Package managers were updated to use them[2]. We are > successfully using them in production. Therefore we prefer to keep the existing > build-in signatures. > > [1]https://fedoraproject.org/wiki/Changes/FsVerityRPM#Enable_fs-verity_in_RPM > [2]https://github.com/rpm-software-management/rpm/issues/1121 Aren't those the same project? I already mentioned in the commit message that it was rejected from Fedora and seems to have been abandoned. So it seems to be something that didn't actually work out. Let me know if you know of anything to the contrary... - Eric
On Fri, 16 Dec 2022 at 20:55, Eric Biggers <ebiggers@kernel.org> wrote: > > On Thu, Dec 08, 2022 at 09:37:29PM +0000, Luca Boccassi wrote: > > > > The second question is easy: because the kernel is the right place for > > our use case to do this verification and enforcement, exactly like dm- > > verity does. > > Well, dm-verity's in-kernel signature verification support is a fairly new > feature. Most users of dm-verity don't use it, and will not be using it. I'm not sure what you mean by "most users" - systemd has support for dm-verity signatures all over the place, libcryptsetup/veritysetup supports them, and even libmount has native first-class mount options for them. > > Userspace is largely untrusted, or much lower trust anyway. > > Yes, which means the kernel is highly trusted. Which is why parsing complex > binary formats, X.509 and PKCS#7, in C code in the kernel is not a great idea... Maybe, but it's there and it's used for multiple purposes and userspace relies on it. If you want to add a new alternative and optional formats I don't think it would be a problem, I certainly wouldn't mind. Kind regards, Luca Boccassi
diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst index cb8e7573882a1..f3dba9c53affe 100644 --- a/Documentation/filesystems/fsverity.rst +++ b/Documentation/filesystems/fsverity.rst @@ -69,24 +69,17 @@ still be used on read-only filesystems. fs-verity is for files that must live on a read-write filesystem because they are independently updated and potentially user-installed, so dm-verity cannot be used. -The base fs-verity feature is a hashing mechanism only; actually -authenticating the files may be done by: - -* Userspace-only - -* Builtin signature verification + userspace policy - - fs-verity optionally supports a simple signature verification - mechanism where users can configure the kernel to require that - all fs-verity files be signed by a key loaded into a keyring; - see `Built-in signature verification`_. - -* Integrity Measurement Architecture (IMA) - - IMA supports including fs-verity file digests and signatures in the - IMA measurement list and verifying fs-verity based file signatures - stored as security.ima xattrs, based on policy. - +The base fs-verity feature is a hashing mechanism only. Actually +authenticating files' fs-verity digests can be done by userspace, as +described above. Alternatively, the Integrity Measurement +Architecture (IMA) can be used. IMA supports including fs-verity file +digests and signatures in the IMA measurement list and verifying +fs-verity based file signatures stored as security.ima xattrs, based +on policy. + +Another alternative is `Builtin signature verification (deprecated)`_. +However, that approach has been shown to not be a good way of doing +signatures with fs-verity, so its use is discouraged. User API ======== @@ -111,8 +104,7 @@ follows:: }; This structure contains the parameters of the Merkle tree to build for -the file, and optionally contains a signature. It must be initialized -as follows: +the file. It must be initialized as follows: - ``version`` must be 1. - ``hash_algorithm`` must be the identifier for the hash algorithm to @@ -128,12 +120,12 @@ as follows: file or device. Currently the maximum salt size is 32 bytes. - ``salt_ptr`` is the pointer to the salt, or NULL if no salt is provided. -- ``sig_size`` is the size of the signature in bytes, or 0 if no - signature is provided. Currently the signature is (somewhat - arbitrarily) limited to 16128 bytes. See `Built-in signature - verification`_ for more information. -- ``sig_ptr`` is the pointer to the signature, or NULL if no - signature is provided. +- ``sig_size`` is the size of the builtin signature in bytes, or 0 if + no builtin signature is provided. Currently the builtin signature + is (somewhat arbitrarily) limited to 16128 bytes. See + `Builtin signature verification (deprecated)`_ for more information. +- ``sig_ptr`` is the pointer to the builtin signature, or NULL if no + builtin signature is provided. - All reserved fields must be zeroed. FS_IOC_ENABLE_VERITY causes the filesystem to build a Merkle tree for @@ -157,7 +149,7 @@ fatal signal), no changes are made to the file. FS_IOC_ENABLE_VERITY can fail with the following errors: - ``EACCES``: the process does not have write access to the file -- ``EBADMSG``: the signature is malformed +- ``EBADMSG``: the builtin signature is malformed - ``EBUSY``: this ioctl is already running on the file - ``EEXIST``: the file already has verity enabled - ``EFAULT``: the caller provided inaccessible memory @@ -166,10 +158,10 @@ FS_IOC_ENABLE_VERITY can fail with the following errors: reserved bits are set; or the file descriptor refers to neither a regular file nor a directory. - ``EISDIR``: the file descriptor refers to a directory -- ``EKEYREJECTED``: the signature doesn't match the file -- ``EMSGSIZE``: the salt or signature is too long +- ``EKEYREJECTED``: the builtin signature doesn't match the file +- ``EMSGSIZE``: the salt or builtin signature is too long - ``ENOKEY``: the fs-verity keyring doesn't contain the certificate - needed to verify the signature + needed to verify the builtin signature - ``ENOPKG``: fs-verity recognizes the hash algorithm, but it's not available in the kernel's crypto API as currently configured (e.g. for SHA-512, missing CONFIG_CRYPTO_SHA512). @@ -178,8 +170,8 @@ FS_IOC_ENABLE_VERITY can fail with the following errors: support; or the filesystem superblock has not had the 'verity' feature enabled on it; or the filesystem does not support fs-verity on this file. (See `Filesystem support`_.) -- ``EPERM``: the file is append-only; or, a signature is required and - one was not provided. +- ``EPERM``: the file is append-only; or, a builtin signature is + required and one was not provided. - ``EROFS``: the filesystem is read-only - ``ETXTBSY``: someone has the file open for writing. This can be the caller's file descriptor, another open file descriptor, or the file @@ -268,9 +260,9 @@ This ioctl takes in a pointer to the following structure:: - ``FS_VERITY_METADATA_TYPE_DESCRIPTOR`` reads the fs-verity descriptor. See `fs-verity descriptor`_. -- ``FS_VERITY_METADATA_TYPE_SIGNATURE`` reads the signature which was - passed to FS_IOC_ENABLE_VERITY, if any. See `Built-in signature - verification`_. +- ``FS_VERITY_METADATA_TYPE_SIGNATURE`` reads the builtin signature + which was passed to FS_IOC_ENABLE_VERITY, if any. + See `Builtin signature verification (deprecated)`_. The semantics are similar to those of ``pread()``. ``offset`` specifies the offset in bytes into the metadata item to read from, and @@ -297,7 +289,7 @@ FS_IOC_READ_VERITY_METADATA can fail with the following errors: overflowed - ``ENODATA``: the file is not a verity file, or FS_VERITY_METADATA_TYPE_SIGNATURE was requested but the file doesn't - have a built-in signature + have a builtin signature - ``ENOTTY``: this type of filesystem does not implement fs-verity, or this ioctl is not yet implemented on it - ``EOPNOTSUPP``: the kernel was not configured with fs-verity @@ -345,8 +337,9 @@ non-verity one, with the following exceptions: with EIO (for read()) or SIGBUS (for mmap() reads). - If the sysctl "fs.verity.require_signatures" is set to 1 and the - file is not signed by a key in the fs-verity keyring, then opening - the file will fail. See `Built-in signature verification`_. + file does not have a builtin signature that verifies against the + fs-verity keyring, then opening the file will fail. + See `Builtin signature verification (deprecated)`_. Direct access to the Merkle tree is not supported. Therefore, if a verity file is copied, or is backed up and restored, then it will lose @@ -428,32 +421,42 @@ root hash as well as other fields such as the file size:: __u8 __reserved[144]; /* must be 0's */ }; -Built-in signature verification -=============================== +Builtin signature verification (deprecated) +=========================================== + +Authenticating the contents of a file via fs-verity requires (a) +checking that the file has fs-verity enabled, and (b) verifying that +the file's fs-verity digest is signed by a trusted key. -With CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y, fs-verity supports putting -a portion of an authentication policy (see `Use cases`_) in the -kernel. Specifically, it adds support for: +Userspace can do both (a) and (b). Alternatively, the kernel can do +both (a) and (b); that is possible with IMA appraisal. -1. At fs-verity module initialization time, a keyring ".fs-verity" is - created. The root user can add trusted X.509 certificates to this - keyring using the add_key() system call, then (when done) - optionally use keyctl_restrict_keyring() to prevent additional - certificates from being added. +fs-verity builtin signatures are a hybrid solution where userspace +handles (a), but the kernel handles (b). Due to this odd division of +responsibilities, the inflexibility of doing signature verification in +the kernel, and the use of PKCS#7, this solution has been shown to not +be a good way of doing signatures with fs-verity. Therefore, it is +**now considered deprecated**. Users of it should migrate to a better +solution -- usually userspace signature verification. -2. `FS_IOC_ENABLE_VERITY`_ accepts a pointer to a PKCS#7 formatted - detached signature in DER format of the file's fs-verity digest. - On success, this signature is persisted alongside the Merkle tree. - Then, any time the file is opened, the kernel will verify the - file's actual digest against this signature, using the certificates - in the ".fs-verity" keyring. +That being said, it optionally remains available in the kernel for +backwards compatibility, and is documented below. -3. A new sysctl "fs.verity.require_signatures" is made available. - When set to 1, the kernel requires that all verity files have a - correctly signed digest as described in (2). +CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG=y enables support for the +deprecated builtin signatures feature. It makes available the kernel +keyring ".fs-verity" and the sysctl "fs.verity.require_signatures". -fs-verity file digests must be signed in the following format, which -is similar to the structure used by `FS_IOC_MEASURE_VERITY`_:: +The ".fs-verity" keyring contains the X.509 certificates that are +trusted for builtin signatures. The root user can add certificates to +this keyring using the add_key() system call, and then (when done) +optionally use keyctl_restrict_keyring() to prevent additional +certificates from being added. + +When fs-verity is enabled on a file using `FS_IOC_ENABLE_VERITY`_, a +pointer to a builtin signature can be provided. On success, the +builtin signature is stored internally within the file's fs-verity +metadata. The builtin signature must be a PKCS#7 detached signature +in DER format of the file's fs-verity digest in the following format:: struct fsverity_formatted_digest { char magic[8]; /* must be "FSVerity" */ @@ -462,13 +465,13 @@ is similar to the structure used by `FS_IOC_MEASURE_VERITY`_:: __u8 digest[]; }; -fs-verity's built-in signature verification support is meant as a -relatively simple mechanism that can be used to provide some level of -authenticity protection for verity files, as an alternative to doing -the signature verification in userspace or using IMA-appraisal. -However, with this mechanism, userspace programs still need to check -that the verity bit is set, and there is no protection against verity -files being swapped around. +Finally, while the sysctl "fs.verity.require_signatures" is set to 1, +the kernel requires that all verity files have a builtin signature +that verifies against the ".fs-verity" keyring. + +Note that there is no point in using builtin signatures when +"fs.verity.require_signatures" is 0 or when userspace doesn't check +which files have fs-verity enabled. Filesystem support ================== diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig index 63807bd0b5364..a5e503f8ca647 100644 --- a/arch/s390/configs/debug_defconfig +++ b/arch/s390/configs/debug_defconfig @@ -629,7 +629,6 @@ CONFIG_FS_DAX=y CONFIG_EXPORTFS_BLOCK_OPS=y CONFIG_FS_ENCRYPTION=y CONFIG_FS_VERITY=y -CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y CONFIG_FANOTIFY=y CONFIG_FANOTIFY_ACCESS_PERMISSIONS=y CONFIG_QUOTA_NETLINK_INTERFACE=y diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig index 4f9a982474423..a754fa5bf2e87 100644 --- a/arch/s390/configs/defconfig +++ b/arch/s390/configs/defconfig @@ -615,7 +615,6 @@ CONFIG_FS_DAX=y CONFIG_EXPORTFS_BLOCK_OPS=y CONFIG_FS_ENCRYPTION=y CONFIG_FS_VERITY=y -CONFIG_FS_VERITY_BUILTIN_SIGNATURES=y CONFIG_FANOTIFY=y CONFIG_FANOTIFY_ACCESS_PERMISSIONS=y CONFIG_QUOTA_NETLINK_INTERFACE=y diff --git a/fs/verity/Kconfig b/fs/verity/Kconfig index aad1f1d998b9d..089e2194650ba 100644 --- a/fs/verity/Kconfig +++ b/fs/verity/Kconfig @@ -42,19 +42,24 @@ config FS_VERITY_DEBUG Say N unless you are an fs-verity developer. -config FS_VERITY_BUILTIN_SIGNATURES - bool "FS Verity builtin signature support" +config FS_VERITY_DEPRECATED_BUILTINSIG + bool "FS Verity builtin signature support (deprecated)" depends on FS_VERITY select SYSTEM_DATA_VERIFICATION help - Support verifying signatures of verity files against the X.509 - certificates that have been loaded into the ".fs-verity" - kernel keyring. + Support verifying builtin signatures of verity files against + the X.509 certificates that have been loaded into the + ".fs-verity" kernel keyring. - This is meant as a relatively simple mechanism that can be - used to provide an authenticity guarantee for verity files, as - an alternative to IMA appraisal. Userspace programs still - need to check that the verity bit is set in order to get an - authenticity guarantee. + This is *not* the recommended way to do signatures with + fs-verity. This was meant as a proof-of-concept of using + fs-verity to ensure the authenticity of files. It has been + shown that this is not a good way to do signatures with + fs-verity. Instead, users should manage and verify + signatures in userspace, or use IMA's fs-verity support. - If unsure, say N. + Note: this feature is useless unless userspace sets the + fs.verity.require_signatures sysctl to 1 and verifies that + specific files have fs-verity enabled. + + Say N unless you need this for backwards compatibility. diff --git a/fs/verity/Makefile b/fs/verity/Makefile index 435559a4fa9ea..ec8899d58cbf0 100644 --- a/fs/verity/Makefile +++ b/fs/verity/Makefile @@ -8,4 +8,4 @@ obj-$(CONFIG_FS_VERITY) += enable.o \ read_metadata.o \ verify.o -obj-$(CONFIG_FS_VERITY_BUILTIN_SIGNATURES) += signature.o +obj-$(CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG) += signature.o diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h index dbe1ce5b450a8..a4d708a8ea57e 100644 --- a/fs/verity/fsverity_private.h +++ b/fs/verity/fsverity_private.h @@ -127,12 +127,12 @@ void __init fsverity_exit_info_cache(void); /* signature.c */ -#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES +#ifdef CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG int fsverity_verify_signature(const struct fsverity_info *vi, const u8 *signature, size_t sig_size); int __init fsverity_init_signature(void); -#else /* !CONFIG_FS_VERITY_BUILTIN_SIGNATURES */ +#else /* !CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG */ static inline int fsverity_verify_signature(const struct fsverity_info *vi, const u8 *signature, size_t sig_size) @@ -144,7 +144,7 @@ static inline int fsverity_init_signature(void) { return 0; } -#endif /* !CONFIG_FS_VERITY_BUILTIN_SIGNATURES */ +#endif /* !CONFIG_FS_VERITY_DEPRECATED_BUILTINSIG */ /* verify.c */ diff --git a/fs/verity/signature.c b/fs/verity/signature.c index dc6935701abda..24c50072e9cc2 100644 --- a/fs/verity/signature.c +++ b/fs/verity/signature.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 /* - * Verification of builtin signatures + * Verification of builtin signatures (deprecated) * * Copyright 2019 Google LLC */ @@ -27,14 +27,18 @@ static int fsverity_require_signatures; static struct key *fsverity_keyring; /** - * fsverity_verify_signature() - check a verity file's signature + * fsverity_verify_signature() - check a verity file's builtin signature * @vi: the file's fsverity_info - * @signature: the file's built-in signature + * @signature: the file's builtin signature * @sig_size: size of signature in bytes, or 0 if no signature * * If the file includes a signature of its fs-verity file digest, verify it * against the certificates in the fs-verity keyring. * + * Please don't introduce new uses of fs-verity builtin signatures. They are a + * proof-of-concept that turned out to not be a good way to do signatures with + * fs-verity. Use userspace signature verification or IMA appraisal instead. + * * Return: 0 on success (signature valid or not required); -errno on failure */ int fsverity_verify_signature(const struct fsverity_info *vi, @@ -96,7 +100,7 @@ int fsverity_verify_signature(const struct fsverity_info *vi, return err; } - pr_debug("Valid signature for file digest %s:%*phN\n", + pr_debug("Valid builtin signature for file digest %s:%*phN\n", hash_alg->name, hash_alg->digest_size, vi->file_digest); return 0; }