diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst index 22b49b295d1f..e5d38439bf58 100644 --- a/Documentation/filesystems/fsverity.rst +++ b/Documentation/filesystems/fsverity.rst @@ -170,6 +170,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 +- ``EBADF``: the file descriptor is not open for reading - ``EBADMSG``: the builtin signature is malformed - ``EBUSY``: this ioctl is already running on the file - ``EEXIST``: the file already has verity enabled @@ -232,6 +233,7 @@ regardless of the size of the file. FS_IOC_MEASURE_VERITY can fail with the following errors: +- ``EBADF``: the file descriptor is not open for reading - ``EFAULT``: the caller provided inaccessible memory - ``ENODATA``: the file is not a verity file - ``ENOTTY``: this type of filesystem does not implement fs-verity @@ -310,6 +312,7 @@ blocks from disk without actually verifying the path to the root node. FS_IOC_READ_VERITY_METADATA can fail with the following errors: +- ``EBADF``: the file descriptor is not open for reading - ``EFAULT``: the caller provided inaccessible memory - ``EINTR``: the ioctl was interrupted before any data was read - ``EINVAL``: reserved fields were set, or ``offset + length`` diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c index ccfa2ef97e50..b34d2247cf84 100644 --- a/fs/btrfs/verity.c +++ b/fs/btrfs/verity.c @@ -483,8 +483,6 @@ static int rollback_verity(struct btrfs_inode *inode) inode->vfs_inode.i_ino); goto out; } - inode->ro_flags &= ~BTRFS_INODE_RO_VERITY; - btrfs_sync_inode_flags_to_i_flags(inode); ret = btrfs_update_inode(trans, inode); if (unlikely(ret)) { btrfs_abort_transaction(trans, ret); @@ -549,16 +547,19 @@ static int finish_verity(struct btrfs_inode *inode, const void *desc, if (IS_ERR(trans)) return PTR_ERR(trans); inode->ro_flags |= BTRFS_INODE_RO_VERITY; - btrfs_sync_inode_flags_to_i_flags(inode); ret = btrfs_update_inode(trans, inode); if (ret) goto end_trans; ret = del_orphan(trans, inode); if (ret) goto end_trans; + /* Now that rollback is no longer possible, set S_VERITY. */ + btrfs_sync_inode_flags_to_i_flags(inode); clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags); btrfs_set_fs_compat_ro(root->fs_info, VERITY); end_trans: + if (ret) + inode->ro_flags &= ~BTRFS_INODE_RO_VERITY; btrfs_end_transaction(trans); return ret; diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c index ac5c133f5529..56fe3e5b33e0 100644 --- a/fs/ext4/verity.c +++ b/fs/ext4/verity.c @@ -216,17 +216,19 @@ static int ext4_end_enable_verity(struct file *filp, const void *desc, goto stop_and_cleanup; ext4_set_inode_flag(inode, EXT4_INODE_VERITY); - ext4_set_inode_flags(inode, false); err = ext4_mark_iloc_dirty(handle, inode, &iloc); if (err) goto stop_and_cleanup; ext4_journal_stop(handle); + /* Now that rollback is no longer possible, set S_VERITY. */ + ext4_set_inode_flags(inode, false); ext4_clear_inode_state(inode, EXT4_STATE_VERITY_IN_PROGRESS); return 0; stop_and_cleanup: + ext4_clear_inode_flag(inode, EXT4_INODE_VERITY); ext4_journal_stop(handle); cleanup: /* diff --git a/fs/verity/enable.c b/fs/verity/enable.c index ad4ff71d7dd9..fe8493280b77 100644 --- a/fs/verity/enable.c +++ b/fs/verity/enable.c @@ -276,8 +276,8 @@ static int enable_verity(struct file *filp, * initialization so that we don't have to undo the enabling when memory * allocation for the hash table fails. This is safe because looking up * the fsverity_info always first checks the S_VERITY flag on the inode, - * which will only be set at the very end of the ->end_enable_verity - * method. + * which will only be set at the very end of a successful call to + * ->end_enable_verity(). */ err = fsverity_set_info(vi); if (err) { @@ -287,10 +287,7 @@ static int enable_verity(struct file *filp, /* * Tell the filesystem to finish enabling verity on the file. - * Serialized with ->begin_enable_verity() by the inode lock. The file - * system needs to set the S_VERITY flag on the inode at the very end of - * the method, at which point the fsverity information can be accessed - * by other threads. + * Serialized with ->begin_enable_verity() by the inode lock. */ inode_lock(inode); err = vops->end_enable_verity(filp, desc, desc_size, params.tree_size); @@ -298,10 +295,19 @@ static int enable_verity(struct file *filp, if (err) { fsverity_err(inode, "%ps() failed with err %d", vops->end_enable_verity, err); - fsverity_remove_info(vi); + if (!WARN_ON_ONCE(IS_VERITY(inode))) + fsverity_remove_info(vi); } else if (WARN_ON_ONCE(!IS_VERITY(inode))) { fsverity_remove_info(vi); err = -EINVAL; + } else { + /* + * Now that verity is enabled, report the signature (if any) to + * LSMs. We tolerate that a process racing with this ioctl can + * briefly see verity enabled with no signature reported yet. + */ + fsverity_report_valid_sig(inode, desc->signature, + le32_to_cpu(desc->sig_size)); } out: kfree(params.hashstate); diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h index 6f85f63861aa..2c41428d484b 100644 --- a/fs/verity/fsverity_private.h +++ b/fs/verity/fsverity_private.h @@ -160,6 +160,8 @@ void __init fsverity_init_info_cache(void); extern int fsverity_require_signatures; int fsverity_verify_signature(const struct fsverity_info *vi, const u8 *signature, size_t sig_size); +void fsverity_report_valid_sig(const struct inode *inode, const u8 *signature, + size_t sig_size); void __init fsverity_init_signature(void); #else /* !CONFIG_FS_VERITY_BUILTIN_SIGNATURES */ @@ -170,6 +172,12 @@ fsverity_verify_signature(const struct fsverity_info *vi, return 0; } +static inline void fsverity_report_valid_sig(const struct inode *inode, + const u8 *signature, + size_t sig_size) +{ +} + static inline void fsverity_init_signature(void) { } diff --git a/fs/verity/measure.c b/fs/verity/measure.c index 68dfccb69772..0c75616dca01 100644 --- a/fs/verity/measure.c +++ b/fs/verity/measure.c @@ -31,6 +31,16 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg) const struct fsverity_hash_alg *hash_alg; struct fsverity_digest arg; + /* + * This ioctl returns a hash of the file contents, so it should require + * read permission. Disallow it on ioctl-only (i.e., nonstandard access + * mode 3) fds, which can in some cases be acquired without passing all + * LSM read-permission checks. Also disallow write-only fds, which can + * refer only to non-verity files which would fail with ENODATA anyway. + */ + if (!(filp->f_mode & FMODE_READ)) + return -EBADF; + vi = fsverity_get_info(inode); if (!vi) return -ENODATA; /* not a verity file */ diff --git a/fs/verity/open.c b/fs/verity/open.c index 664629043038..d1ac9556e947 100644 --- a/fs/verity/open.c +++ b/fs/verity/open.c @@ -270,12 +270,6 @@ static bool validate_fsverity_descriptor(struct inode *inode, const struct fsverity_descriptor *desc, size_t desc_size) { - if (desc_size < sizeof(*desc)) { - fsverity_err(inode, "Unrecognized descriptor size: %zu bytes", - desc_size); - return false; - } - if (desc->version != 1) { fsverity_err(inode, "Unrecognized descriptor version: %u", desc->version); @@ -314,31 +308,37 @@ static bool validate_fsverity_descriptor(struct inode *inode, int fsverity_get_descriptor(struct inode *inode, struct fsverity_descriptor **desc_ret) { - int res; + int size, res; struct fsverity_descriptor *desc; - res = inode->i_sb->s_vop->get_verity_descriptor(inode, NULL, 0); - if (res < 0) { - fsverity_err(inode, - "Error %d getting verity descriptor size", res); - return res; + size = inode->i_sb->s_vop->get_verity_descriptor(inode, NULL, 0); + if (size < 0) { + fsverity_err(inode, "Error %d getting verity descriptor size", + size); + return size; } - if (res > FS_VERITY_MAX_DESCRIPTOR_SIZE) { - fsverity_err(inode, "Verity descriptor is too large (%d bytes)", - res); + if (size < sizeof(*desc) || size > FS_VERITY_MAX_DESCRIPTOR_SIZE) { + fsverity_err(inode, + "Verity descriptor has invalid size (%d bytes)", + size); return -EMSGSIZE; } - desc = kmalloc(res, GFP_KERNEL); + desc = kmalloc(size, GFP_KERNEL); if (!desc) return -ENOMEM; - res = inode->i_sb->s_vop->get_verity_descriptor(inode, desc, res); + res = inode->i_sb->s_vop->get_verity_descriptor(inode, desc, size); if (res < 0) { fsverity_err(inode, "Error %d reading verity descriptor", res); kfree(desc); return res; } - - if (!validate_fsverity_descriptor(inode, desc, res)) { + if (res != size) { + fsverity_err(inode, "Verity descriptor size changed (%d => %d)", + size, res); + kfree(desc); + return -EINVAL; + } + if (!validate_fsverity_descriptor(inode, desc, size)) { kfree(desc); return -EINVAL; } @@ -387,6 +387,15 @@ int fsverity_ensure_verity_info(struct inode *inode) goto out_free_desc; } + /* + * Now that the signature (if any) has been verified, report it to LSMs. + * Do this before inserting the fsverity_info into the hashtable, so + * that anyone who concurrently retrieves the fsverity_info for the same + * inode is guaranteed to see the LSM's signature status populated. + */ + fsverity_report_valid_sig(inode, desc->signature, + le32_to_cpu(desc->sig_size)); + /* * Multiple tasks may race to set the inode's verity info, in which case * we might find an existing fsverity_info in the hash table. diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c index b4c0892430cd..6694149f7b46 100644 --- a/fs/verity/read_metadata.c +++ b/fs/verity/read_metadata.c @@ -166,13 +166,19 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg) int length; void __user *buf; + /* + * This ioctl returns file contents block hashes, so it should require + * read permission. Disallow it on ioctl-only (i.e., nonstandard access + * mode 3) fds, which can in some cases be acquired without passing all + * LSM read-permission checks. Also disallow write-only fds, which can + * refer only to non-verity files which would fail with ENODATA anyway. + */ + if (!(filp->f_mode & FMODE_READ)) + return -EBADF; + vi = fsverity_get_info(inode); if (!vi) return -ENODATA; /* not a verity file */ - /* - * Note that we don't have to explicitly check that the file is open for - * reading, since verity files can only be opened for reading. - */ if (copy_from_user(&arg, uarg, sizeof(arg))) return -EFAULT; diff --git a/fs/verity/signature.c b/fs/verity/signature.c index 07657e913b22..2f49c5d9d987 100644 --- a/fs/verity/signature.c +++ b/fs/verity/signature.c @@ -42,13 +42,16 @@ static struct key *fsverity_keyring; * @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. Note that signatures - * are verified regardless of the state of the 'fsverity_require_signatures' - * variable and the LSM subsystem relies on this behavior to help enforce - * file integrity policies. Please discuss changes with the LSM list - * (thank you!). + * against the certificates in the fs-verity keyring. * - * Return: 0 on success (signature valid or not required); -errno on failure + * Note that signatures are verified regardless of the state of the + * 'fsverity_require_signatures' variable, and the LSM subsystem relies on this + * behavior to help enforce file integrity policies. (See + * fsverity_report_valid_sig(), which assumes that the signature, if present, + * was verified here.) Please discuss changes with the LSM list (thank you!). + * + * Return: 0 if the signature is present and valid, or absent and not required; + * -errno on failure */ int fsverity_verify_signature(const struct fsverity_info *vi, const u8 *signature, size_t sig_size) @@ -111,18 +114,40 @@ int fsverity_verify_signature(const struct fsverity_info *vi, return err; } - err = security_inode_setintegrity(inode, - LSM_INT_FSVERITY_BUILTINSIG_VALID, - signature, - sig_size); + return 0; +} - if (err) { +/* + * If the signature exists, this tells the LSM that it was verified. + * + * fsverity_verify_signature() must have already been called with the same + * signature, *and* the inode must have verity enabled with the signature + * matching it. If called from FS_IOC_ENABLE_VERITY, the latter means that we + * must be past the point of no return, i.e. verity won't fail to be enabled at + * a later point. This is critical, since LSMs must be told about the signature + * only when the verity status is permanent in the in-core inode. + */ +void fsverity_report_valid_sig(const struct inode *inode, const u8 *signature, + size_t sig_size) +{ + int err; + + if (WARN_ON_ONCE(!IS_VERITY(inode))) + return; + + if (sig_size == 0) + return; + + err = security_inode_setintegrity( + inode, LSM_INT_FSVERITY_BUILTINSIG_VALID, signature, sig_size); + if (err) fsverity_err(inode, "Error %d exposing file signature to LSMs", err); - return err; - } - - return 0; + /* + * Besides the logging above, ignore errors here. The LSM hook just + * records the signature and doesn't perform access control. And in the + * case of FS_IOC_ENABLE_VERITY verity was already enabled. + */ } void __init fsverity_init_signature(void) diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h index f9433332c274..61fe96ccdff6 100644 --- a/include/linux/fsverity.h +++ b/include/linux/fsverity.h @@ -54,11 +54,13 @@ struct fsverity_operations { * @merkle_tree_size: total bytes the Merkle tree took up * * If desc == NULL, then enabling verity failed and the filesystem only - * must do any necessary cleanups. Else, it must also store the given - * verity descriptor to a fs-specific location associated with the inode - * and do any fs-specific actions needed to mark the inode as a verity - * inode, e.g. setting a bit in the on-disk inode. The filesystem is - * also responsible for setting the S_VERITY flag in the VFS inode. + * must do any necessary cleanups. + * + * Else, the filesystem must store the given verity descriptor to a + * fs-specific location associated with the inode and do any fs-specific + * actions needed to mark the inode as a verity inode, e.g. set a bit in + * the on-disk inode. Finally, on success (only when rollback is no + * longer possible!) it must set S_VERITY in the VFS inode. * * i_rwsem is held for write, but it may have been dropped between * ->begin_enable_verity() and ->end_enable_verity().