* Re: [PATCH v5 10/16] fs-verity: implement FS_IOC_ENABLE_VERITY ioctl
From: Jaegeuk Kim @ 2019-06-22 22:43 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y . Ts'o, Darrick J . Wong, linux-api, Dave Chinner,
linux-f2fs-devel, linux-fscrypt, linux-fsdevel, linux-integrity,
linux-ext4, Linus Torvalds, Christoph Hellwig, Victor Hsieh
In-Reply-To: <20190620205043.64350-11-ebiggers@kernel.org>
On 06/20, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Add a function for filesystems to call to implement the
> FS_IOC_ENABLE_VERITY ioctl. This ioctl enables fs-verity on a file.
>
> See the "FS_IOC_ENABLE_VERITY" section of
> Documentation/filesystems/fsverity.rst for the documentation.
>
Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/verity/Makefile | 3 +-
> fs/verity/enable.c | 341 +++++++++++++++++++++++++++++++++++++++
> include/linux/fsverity.h | 64 ++++++++
> 3 files changed, 407 insertions(+), 1 deletion(-)
> create mode 100644 fs/verity/enable.c
>
> diff --git a/fs/verity/Makefile b/fs/verity/Makefile
> index 7fa628cd5eba24..04b37475fd280a 100644
> --- a/fs/verity/Makefile
> +++ b/fs/verity/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> -obj-$(CONFIG_FS_VERITY) += hash_algs.o \
> +obj-$(CONFIG_FS_VERITY) += enable.o \
> + hash_algs.o \
> init.o \
> open.o \
> verify.o
> diff --git a/fs/verity/enable.c b/fs/verity/enable.c
> new file mode 100644
> index 00000000000000..144721bbe4aab9
> --- /dev/null
> +++ b/fs/verity/enable.c
> @@ -0,0 +1,341 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * fs/verity/enable.c: ioctl to enable verity on a file
> + *
> + * Copyright 2019 Google LLC
> + */
> +
> +#include "fsverity_private.h"
> +
> +#include <crypto/hash.h>
> +#include <linux/mount.h>
> +#include <linux/pagemap.h>
> +#include <linux/sched/signal.h>
> +#include <linux/uaccess.h>
> +
> +static int build_merkle_tree_level(struct inode *inode, unsigned int level,
> + u64 num_blocks_to_hash,
> + const struct merkle_tree_params *params,
> + u8 *pending_hashes,
> + struct ahash_request *req)
> +{
> + const struct fsverity_operations *vops = inode->i_sb->s_vop;
> + unsigned int pending_size = 0;
> + u64 dst_block_num;
> + u64 i;
> + int err;
> +
> + if (WARN_ON(params->block_size != PAGE_SIZE)) /* checked earlier too */
> + return -EINVAL;
> +
> + if (level < params->num_levels) {
> + dst_block_num = params->level_start[level];
> + } else {
> + if (WARN_ON(num_blocks_to_hash != 1))
> + return -EINVAL;
> + dst_block_num = 0; /* unused */
> + }
> +
> + for (i = 0; i < num_blocks_to_hash; i++) {
> + struct page *src_page;
> +
> + if ((pgoff_t)i % 10000 == 0 || i + 1 == num_blocks_to_hash)
> + pr_debug("Hashing block %llu of %llu for level %u\n",
> + i + 1, num_blocks_to_hash, level);
> +
> + if (level == 0)
> + /* Leaf: hashing a data block */
> + src_page = read_mapping_page(inode->i_mapping, i, NULL);
> + else
> + /* Non-leaf: hashing hash block from level below */
> + src_page = vops->read_merkle_tree_page(inode,
> + params->level_start[level - 1] + i);
> + if (IS_ERR(src_page)) {
> + err = PTR_ERR(src_page);
> + fsverity_err(inode,
> + "Error %d reading Merkle tree page %llu",
> + err, params->level_start[level - 1] + i);
> + return err;
> + }
> +
> + err = fsverity_hash_page(params, inode, req, src_page,
> + &pending_hashes[pending_size]);
> + put_page(src_page);
> + if (err)
> + return err;
> + pending_size += params->digest_size;
> +
> + if (level == params->num_levels) /* Root hash? */
> + return 0;
> +
> + if (pending_size + params->digest_size > params->block_size ||
> + i + 1 == num_blocks_to_hash) {
> + /* Flush the pending hash block */
> + memset(&pending_hashes[pending_size], 0,
> + params->block_size - pending_size);
> + err = vops->write_merkle_tree_block(inode,
> + pending_hashes,
> + dst_block_num,
> + params->log_blocksize);
> + if (err) {
> + fsverity_err(inode,
> + "Error %d writing Merkle tree block %llu",
> + err, dst_block_num);
> + return err;
> + }
> + dst_block_num++;
> + pending_size = 0;
> + }
> +
> + if (fatal_signal_pending(current))
> + return -EINTR;
> + cond_resched();
> + }
> + return 0;
> +}
> +
> +/*
> + * Build the Merkle tree for the given inode using the given parameters, and
> + * return the root hash in @root_hash.
> + *
> + * The tree is written to a filesystem-specific location as determined by the
> + * ->write_merkle_tree_block() method. However, the blocks that comprise the
> + * tree are the same for all filesystems.
> + */
> +static int build_merkle_tree(struct inode *inode,
> + const struct merkle_tree_params *params,
> + u8 *root_hash)
> +{
> + u8 *pending_hashes;
> + struct ahash_request *req;
> + u64 blocks;
> + unsigned int level;
> + int err = -ENOMEM;
> +
> + if (inode->i_size == 0) {
> + /* Empty file is a special case; root hash is all 0's */
> + memset(root_hash, 0, params->digest_size);
> + return 0;
> + }
> +
> + pending_hashes = kmalloc(params->block_size, GFP_KERNEL);
> + req = ahash_request_alloc(params->hash_alg->tfm, GFP_KERNEL);
> + if (!pending_hashes || !req)
> + goto out;
> +
> + /*
> + * Build each level of the Merkle tree, starting at the leaf level
> + * (level 0) and ascending to the root node (level 'num_levels - 1').
> + * Then at the end (level 'num_levels'), calculate the root hash.
> + */
> + blocks = (inode->i_size + params->block_size - 1) >>
> + params->log_blocksize;
> + for (level = 0; level <= params->num_levels; level++) {
> + err = build_merkle_tree_level(inode, level, blocks, params,
> + pending_hashes, req);
> + if (err)
> + goto out;
> + blocks = (blocks + params->hashes_per_block - 1) >>
> + params->log_arity;
> + }
> + memcpy(root_hash, pending_hashes, params->digest_size);
> + err = 0;
> +out:
> + kfree(pending_hashes);
> + ahash_request_free(req);
> + return err;
> +}
> +
> +static int enable_verity(struct file *filp,
> + const struct fsverity_enable_arg *arg)
> +{
> + struct inode *inode = file_inode(filp);
> + const struct fsverity_operations *vops = inode->i_sb->s_vop;
> + struct merkle_tree_params params = { };
> + struct fsverity_descriptor *desc;
> + size_t desc_size = sizeof(*desc);
> + struct fsverity_info *vi;
> + int err;
> +
> + /* Start initializing the fsverity_descriptor */
> + desc = kzalloc(desc_size, GFP_KERNEL);
> + if (!desc)
> + return -ENOMEM;
> + desc->version = 1;
> + desc->hash_algorithm = arg->hash_algorithm;
> + desc->log_blocksize = ilog2(arg->block_size);
> +
> + /* Get the salt if the user provided one */
> + if (arg->salt_size &&
> + copy_from_user(desc->salt,
> + (const u8 __user *)(uintptr_t)arg->salt_ptr,
> + arg->salt_size)) {
> + err = -EFAULT;
> + goto out;
> + }
> + desc->salt_size = arg->salt_size;
> +
> + desc->data_size = cpu_to_le64(inode->i_size);
> +
> + pr_debug("Building Merkle tree...\n");
> +
> + /* Prepare the Merkle tree parameters */
> + err = fsverity_init_merkle_tree_params(¶ms, inode,
> + arg->hash_algorithm,
> + desc->log_blocksize,
> + desc->salt, desc->salt_size);
> + if (err)
> + goto out;
> +
> + /* Tell the filesystem that verity is being enabled on the file */
> + err = vops->begin_enable_verity(filp);
> + if (err)
> + goto out;
> +
> + /* Build the Merkle tree */
> + BUILD_BUG_ON(sizeof(desc->root_hash) < FS_VERITY_MAX_DIGEST_SIZE);
> + err = build_merkle_tree(inode, ¶ms, desc->root_hash);
> + if (err) {
> + fsverity_err(inode, "Error %d building Merkle tree", err);
> + goto rollback;
> + }
> + pr_debug("Done building Merkle tree. Root hash is %s:%*phN\n",
> + params.hash_alg->name, params.digest_size, desc->root_hash);
> +
> + /*
> + * Create the fsverity_info. Don't bother trying to save work by
> + * reusing the merkle_tree_params from above. Instead, just create the
> + * fsverity_info from the fsverity_descriptor as if it were just loaded
> + * from disk. This is simpler, and it serves as an extra check that the
> + * metadata we're writing is valid before actually enabling verity.
> + */
> + vi = fsverity_create_info(inode, desc, desc_size);
> + if (IS_ERR(vi)) {
> + err = PTR_ERR(vi);
> + goto rollback;
> + }
> +
> + /* Tell the filesystem to finish enabling verity on the file */
> + err = vops->end_enable_verity(filp, desc, desc_size, params.tree_size);
> + if (err) {
> + fsverity_err(inode, "%ps() failed with err %d",
> + vops->end_enable_verity, err);
> + fsverity_free_info(vi);
> + } else if (WARN_ON(!IS_VERITY(inode))) {
> + err = -EINVAL;
> + fsverity_free_info(vi);
> + } else {
> + /* Successfully enabled verity */
> +
> + /*
> + * Readers can start using ->i_verity_info immediately, so it
> + * can't be rolled back once set. So don't set it until just
> + * after the filesystem has successfully enabled verity.
> + */
> + fsverity_set_info(inode, vi);
> + }
> +out:
> + kfree(params.hashstate);
> + kfree(desc);
> + return err;
> +
> +rollback:
> + (void)vops->end_enable_verity(filp, NULL, 0, params.tree_size);
> + goto out;
> +}
> +
> +/**
> + * fsverity_ioctl_enable() - enable verity on a file
> + *
> + * Enable fs-verity on a file. See the "FS_IOC_ENABLE_VERITY" section of
> + * Documentation/filesystems/fsverity.rst for the documentation.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int fsverity_ioctl_enable(struct file *filp, const void __user *uarg)
> +{
> + struct inode *inode = file_inode(filp);
> + struct fsverity_enable_arg arg;
> + int err;
> +
> + if (copy_from_user(&arg, uarg, sizeof(arg)))
> + return -EFAULT;
> +
> + if (arg.version != 1)
> + return -EINVAL;
> +
> + if (arg.__reserved1 ||
> + memchr_inv(arg.__reserved2, 0, sizeof(arg.__reserved2)))
> + return -EINVAL;
> +
> + if (arg.block_size != PAGE_SIZE)
> + return -EINVAL;
> +
> + if (arg.salt_size > FIELD_SIZEOF(struct fsverity_descriptor, salt))
> + return -EMSGSIZE;
> +
> + if (arg.sig_size)
> + return -EINVAL;
> +
> + /*
> + * Require a regular file with write access. But the actual fd must
> + * still be readonly so that we can lock out all writers. This is
> + * needed to guarantee that no writable fds exist to the file once it
> + * has verity enabled, and to stabilize the data being hashed.
> + */
> +
> + err = inode_permission(inode, MAY_WRITE);
> + if (err)
> + return err;
> +
> + if (IS_APPEND(inode))
> + return -EPERM;
> +
> + if (S_ISDIR(inode->i_mode))
> + return -EISDIR;
> +
> + if (!S_ISREG(inode->i_mode))
> + return -EINVAL;
> +
> + err = mnt_want_write_file(filp);
> + if (err) /* -EROFS */
> + return err;
> +
> + err = deny_write_access(filp);
> + if (err) /* -ETXTBSY */
> + goto out_drop_write;
> +
> + inode_lock(inode);
> +
> + if (IS_VERITY(inode)) {
> + err = -EEXIST;
> + goto out_unlock;
> + }
> +
> + err = enable_verity(filp, &arg);
> + if (err)
> + goto out_unlock;
> +
> + /*
> + * Some pages of the file may have been evicted from pagecache after
> + * being used in the Merkle tree construction, then read into pagecache
> + * again by another process reading from the file concurrently. Since
> + * these pages didn't undergo verification against the file measurement
> + * which fs-verity now claims to be enforcing, we have to wipe the
> + * pagecache to ensure that all future reads are verified.
> + */
> + filemap_write_and_wait(inode->i_mapping);
> + truncate_inode_pages(inode->i_mapping, 0);
> +
> + /*
> + * allow_write_access() is needed to pair with deny_write_access().
> + * Regardless, the filesystem won't allow writing to verity files.
> + */
> +out_unlock:
> + inode_unlock(inode);
> + allow_write_access(filp);
> +out_drop_write:
> + mnt_drop_write_file(filp);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(fsverity_ioctl_enable);
> diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
> index ecd47e748c7f64..7ef2ef82653409 100644
> --- a/include/linux/fsverity.h
> +++ b/include/linux/fsverity.h
> @@ -17,6 +17,42 @@
> /* Verity operations for filesystems */
> struct fsverity_operations {
>
> + /**
> + * Begin enabling verity on the given file.
> + *
> + * @filp: a readonly file descriptor for the file
> + *
> + * The filesystem must do any needed filesystem-specific preparations
> + * for enabling verity, e.g. evicting inline data.
> + *
> + * i_rwsem is held for write.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> + int (*begin_enable_verity)(struct file *filp);
> +
> + /**
> + * End enabling verity on the given file.
> + *
> + * @filp: a readonly file descriptor for the file
> + * @desc: the verity descriptor to write, or NULL on failure
> + * @desc_size: size of verity descriptor, or 0 on failure
> + * @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.
> + *
> + * i_rwsem is held for write.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> + int (*end_enable_verity)(struct file *filp, const void *desc,
> + size_t desc_size, u64 merkle_tree_size);
> +
> /**
> * Get the verity descriptor of the given inode.
> *
> @@ -50,6 +86,22 @@ struct fsverity_operations {
> */
> struct page *(*read_merkle_tree_page)(struct inode *inode,
> pgoff_t index);
> +
> + /**
> + * Write a Merkle tree block to the given inode.
> + *
> + * @inode: the inode for which the Merkle tree is being built
> + * @buf: block to write
> + * @index: 0-based index of the block within the Merkle tree
> + * @log_blocksize: log base 2 of the Merkle tree block size
> + *
> + * This is only called between ->begin_enable_verity() and
> + * ->end_enable_verity(). i_rwsem is held for write.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> + int (*write_merkle_tree_block)(struct inode *inode, const void *buf,
> + u64 index, int log_blocksize);
> };
>
> #ifdef CONFIG_FS_VERITY
> @@ -60,6 +112,10 @@ static inline struct fsverity_info *fsverity_get_info(const struct inode *inode)
> return READ_ONCE(inode->i_verity_info);
> }
>
> +/* enable.c */
> +
> +extern int fsverity_ioctl_enable(struct file *filp, const void __user *arg);
> +
> /* open.c */
>
> extern int fsverity_file_open(struct inode *inode, struct file *filp);
> @@ -79,6 +135,14 @@ static inline struct fsverity_info *fsverity_get_info(const struct inode *inode)
> return NULL;
> }
>
> +/* enable.c */
> +
> +static inline int fsverity_ioctl_enable(struct file *filp,
> + const void __user *arg)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> /* open.c */
>
> static inline int fsverity_file_open(struct inode *inode, struct file *filp)
> --
> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH v5 11/16] fs-verity: implement FS_IOC_MEASURE_VERITY ioctl
From: Jaegeuk Kim @ 2019-06-22 22:43 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y . Ts'o, Darrick J . Wong, linux-api, Dave Chinner,
linux-f2fs-devel, linux-fscrypt, linux-fsdevel, linux-integrity,
linux-ext4, Linus Torvalds, Christoph Hellwig, Victor Hsieh
In-Reply-To: <20190620205043.64350-12-ebiggers@kernel.org>
On 06/20, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Add a function for filesystems to call to implement the
> FS_IOC_MEASURE_VERITY ioctl. This ioctl retrieves the file measurement
> that fs-verity calculated for the given file and is enforcing for reads;
> i.e., reads that don't match this hash will fail. This ioctl can be
> used for authentication or logging of file measurements in userspace.
>
> See the "FS_IOC_MEASURE_VERITY" section of
> Documentation/filesystems/fsverity.rst for the documentation.
>
> Reviewed-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/verity/Makefile | 1 +
> fs/verity/measure.c | 57 ++++++++++++++++++++++++++++++++++++++++
> include/linux/fsverity.h | 11 ++++++++
> 3 files changed, 69 insertions(+)
> create mode 100644 fs/verity/measure.c
>
> diff --git a/fs/verity/Makefile b/fs/verity/Makefile
> index 04b37475fd280a..6f7675ae0a3110 100644
> --- a/fs/verity/Makefile
> +++ b/fs/verity/Makefile
> @@ -3,5 +3,6 @@
> obj-$(CONFIG_FS_VERITY) += enable.o \
> hash_algs.o \
> init.o \
> + measure.o \
> open.o \
> verify.o
> diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> new file mode 100644
> index 00000000000000..05049b68c74553
> --- /dev/null
> +++ b/fs/verity/measure.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * fs/verity/measure.c: ioctl to get a verity file's measurement
> + *
> + * Copyright 2019 Google LLC
> + */
> +
> +#include "fsverity_private.h"
> +
> +#include <linux/uaccess.h>
> +
> +/**
> + * fsverity_ioctl_measure() - get a verity file's measurement
> + *
> + * Retrieve the file measurement that the kernel is enforcing for reads from a
> + * verity file. See the "FS_IOC_MEASURE_VERITY" section of
> + * Documentation/filesystems/fsverity.rst for the documentation.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
> +{
> + const struct inode *inode = file_inode(filp);
> + struct fsverity_digest __user *uarg = _uarg;
> + const struct fsverity_info *vi;
> + const struct fsverity_hash_alg *hash_alg;
> + struct fsverity_digest arg;
> +
> + vi = fsverity_get_info(inode);
> + if (!vi)
> + return -ENODATA; /* not a verity file */
> + hash_alg = vi->tree_params.hash_alg;
> +
> + /*
> + * The user specifies the digest_size their buffer has space for; we can
> + * return the digest if it fits in the available space. We write back
> + * the actual size, which may be shorter than the user-specified size.
> + */
> +
> + if (get_user(arg.digest_size, &uarg->digest_size))
> + return -EFAULT;
> + if (arg.digest_size < hash_alg->digest_size)
> + return -EOVERFLOW;
> +
> + memset(&arg, 0, sizeof(arg));
> + arg.digest_algorithm = hash_alg - fsverity_hash_algs;
> + arg.digest_size = hash_alg->digest_size;
> +
> + if (copy_to_user(uarg, &arg, sizeof(arg)))
> + return -EFAULT;
> +
> + if (copy_to_user(uarg->digest, vi->measurement, hash_alg->digest_size))
> + return -EFAULT;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(fsverity_ioctl_measure);
> diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
> index 7ef2ef82653409..247359c86b72e0 100644
> --- a/include/linux/fsverity.h
> +++ b/include/linux/fsverity.h
> @@ -116,6 +116,10 @@ static inline struct fsverity_info *fsverity_get_info(const struct inode *inode)
>
> extern int fsverity_ioctl_enable(struct file *filp, const void __user *arg);
>
> +/* measure.c */
> +
> +extern int fsverity_ioctl_measure(struct file *filp, void __user *arg);
> +
> /* open.c */
>
> extern int fsverity_file_open(struct inode *inode, struct file *filp);
> @@ -143,6 +147,13 @@ static inline int fsverity_ioctl_enable(struct file *filp,
> return -EOPNOTSUPP;
> }
>
> +/* measure.c */
> +
> +static inline int fsverity_ioctl_measure(struct file *filp, void __user *arg)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> /* open.c */
>
> static inline int fsverity_file_open(struct inode *inode, struct file *filp)
> --
> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH v5 12/16] fs-verity: add SHA-512 support
From: Jaegeuk Kim @ 2019-06-22 22:44 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y . Ts'o, Darrick J . Wong, linux-api, Dave Chinner,
linux-f2fs-devel, linux-fscrypt, linux-fsdevel, linux-integrity,
linux-ext4, Linus Torvalds, Christoph Hellwig, Victor Hsieh
In-Reply-To: <20190620205043.64350-13-ebiggers@kernel.org>
On 06/20, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Add SHA-512 support to fs-verity. This is primarily a demonstration of
> the trivial changes needed to support a new hash algorithm in fs-verity;
> most users will still use SHA-256, due to the smaller space required to
> store the hashes. But some users may prefer SHA-512.
>
> Reviewed-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/verity/fsverity_private.h | 2 +-
> fs/verity/hash_algs.c | 5 +++++
> include/uapi/linux/fsverity.h | 1 +
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> index eaa2b3b93bbf6b..02a547f0667c13 100644
> --- a/fs/verity/fsverity_private.h
> +++ b/fs/verity/fsverity_private.h
> @@ -29,7 +29,7 @@ struct ahash_request;
> * Largest digest size among all hash algorithms supported by fs-verity.
> * Currently assumed to be <= size of fsverity_descriptor::root_hash.
> */
> -#define FS_VERITY_MAX_DIGEST_SIZE SHA256_DIGEST_SIZE
> +#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
>
> /* A hash algorithm supported by fs-verity */
> struct fsverity_hash_alg {
> diff --git a/fs/verity/hash_algs.c b/fs/verity/hash_algs.c
> index 46df17094fc252..e0462a010cabfb 100644
> --- a/fs/verity/hash_algs.c
> +++ b/fs/verity/hash_algs.c
> @@ -17,6 +17,11 @@ struct fsverity_hash_alg fsverity_hash_algs[] = {
> .digest_size = SHA256_DIGEST_SIZE,
> .block_size = SHA256_BLOCK_SIZE,
> },
> + [FS_VERITY_HASH_ALG_SHA512] = {
> + .name = "sha512",
> + .digest_size = SHA512_DIGEST_SIZE,
> + .block_size = SHA512_BLOCK_SIZE,
> + },
> };
>
> /**
> diff --git a/include/uapi/linux/fsverity.h b/include/uapi/linux/fsverity.h
> index 57d1d7fc0c345a..da0daf6c193b4b 100644
> --- a/include/uapi/linux/fsverity.h
> +++ b/include/uapi/linux/fsverity.h
> @@ -14,6 +14,7 @@
> #include <linux/types.h>
>
> #define FS_VERITY_HASH_ALG_SHA256 1
> +#define FS_VERITY_HASH_ALG_SHA512 2
>
> struct fsverity_enable_arg {
> __u32 version;
> --
> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH v5 16/16] f2fs: add fs-verity support
From: Jaegeuk Kim @ 2019-06-22 23:12 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y . Ts'o, Darrick J . Wong, linux-api, Dave Chinner,
linux-f2fs-devel, linux-fscrypt, linux-fsdevel, linux-integrity,
linux-ext4, Linus Torvalds, Christoph Hellwig, Victor Hsieh
In-Reply-To: <20190620205043.64350-17-ebiggers@kernel.org>
On 06/20, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Add fs-verity support to f2fs. fs-verity is a filesystem feature that
> enables transparent integrity protection and authentication of read-only
> files. It uses a dm-verity like mechanism at the file level: a Merkle
> tree is used to verify any block in the file in log(filesize) time. It
> is implemented mainly by helper functions in fs/verity/. See
> Documentation/filesystems/fsverity.rst for the full documentation.
>
> The f2fs support for fs-verity consists of:
>
> - Adding a filesystem feature flag and an inode flag for fs-verity.
>
> - Implementing the fsverity_operations to support enabling verity on an
> inode and reading/writing the verity metadata.
>
> - Updating ->readpages() to verify data as it's read from verity files
> and to support reading verity metadata pages.
>
> - Updating ->write_begin(), ->write_end(), and ->writepages() to support
> writing verity metadata pages.
>
> - Calling the fs-verity hooks for ->open(), ->setattr(), and ->ioctl().
>
> Like ext4, f2fs stores the verity metadata (Merkle tree and
> fsverity_descriptor) past the end of the file, starting at the first 64K
> boundary beyond i_size. This approach works because (a) verity files
> are readonly, and (b) pages fully beyond i_size aren't visible to
> userspace but can be read/written internally by f2fs with only some
> relatively small changes to f2fs. Extended attributes cannot be used
> because (a) f2fs limits the total size of an inode's xattr entries to
> 4096 bytes, which wouldn't be enough for even a single Merkle tree
> block, and (b) f2fs encryption doesn't encrypt xattrs, yet the verity
> metadata *must* be encrypted when the file is because it contains hashes
> of the plaintext data.
>
Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/f2fs/Makefile | 1 +
> fs/f2fs/data.c | 72 +++++++++++++--
> fs/f2fs/f2fs.h | 23 ++++-
> fs/f2fs/file.c | 40 ++++++++
> fs/f2fs/inode.c | 5 +-
> fs/f2fs/super.c | 3 +
> fs/f2fs/sysfs.c | 11 +++
> fs/f2fs/verity.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++
> fs/f2fs/xattr.h | 2 +
> 9 files changed, 376 insertions(+), 14 deletions(-)
> create mode 100644 fs/f2fs/verity.c
>
> diff --git a/fs/f2fs/Makefile b/fs/f2fs/Makefile
> index 776c4b93650496..2aaecc63834fc8 100644
> --- a/fs/f2fs/Makefile
> +++ b/fs/f2fs/Makefile
> @@ -8,3 +8,4 @@ f2fs-$(CONFIG_F2FS_STAT_FS) += debug.o
> f2fs-$(CONFIG_F2FS_FS_XATTR) += xattr.o
> f2fs-$(CONFIG_F2FS_FS_POSIX_ACL) += acl.o
> f2fs-$(CONFIG_F2FS_IO_TRACE) += trace.o
> +f2fs-$(CONFIG_FS_VERITY) += verity.o
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index eda4181d20926b..8f175d47291d0b 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -73,6 +73,7 @@ static enum count_type __read_io_type(struct page *page)
> enum bio_post_read_step {
> STEP_INITIAL = 0,
> STEP_DECRYPT,
> + STEP_VERITY,
> };
>
> struct bio_post_read_ctx {
> @@ -119,8 +120,23 @@ static void decrypt_work(struct work_struct *work)
> bio_post_read_processing(ctx);
> }
>
> +static void verity_work(struct work_struct *work)
> +{
> + struct bio_post_read_ctx *ctx =
> + container_of(work, struct bio_post_read_ctx, work);
> +
> + fsverity_verify_bio(ctx->bio);
> +
> + bio_post_read_processing(ctx);
> +}
> +
> static void bio_post_read_processing(struct bio_post_read_ctx *ctx)
> {
> + /*
> + * We use different work queues for decryption and for verity because
> + * verity may require reading metadata pages that need decryption, and
> + * we shouldn't recurse to the same workqueue.
> + */
> switch (++ctx->cur_step) {
> case STEP_DECRYPT:
> if (ctx->enabled_steps & (1 << STEP_DECRYPT)) {
> @@ -130,6 +146,14 @@ static void bio_post_read_processing(struct bio_post_read_ctx *ctx)
> }
> ctx->cur_step++;
> /* fall-through */
> + case STEP_VERITY:
> + if (ctx->enabled_steps & (1 << STEP_VERITY)) {
> + INIT_WORK(&ctx->work, verity_work);
> + fsverity_enqueue_verify_work(&ctx->work);
> + return;
> + }
> + ctx->cur_step++;
> + /* fall-through */
> default:
> __read_end_io(ctx->bio);
> }
> @@ -553,8 +577,15 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
> up_write(&io->io_rwsem);
> }
>
> +static inline bool f2fs_need_verity(const struct inode *inode, pgoff_t idx)
> +{
> + return fsverity_active(inode) &&
> + idx < DIV_ROUND_UP(inode->i_size, PAGE_SIZE);
> +}
> +
> static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
> - unsigned nr_pages, unsigned op_flag)
> + unsigned nr_pages, unsigned op_flag,
> + pgoff_t first_idx)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> struct bio *bio;
> @@ -570,6 +601,10 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
>
> if (f2fs_encrypted_file(inode))
> post_read_steps |= 1 << STEP_DECRYPT;
> +
> + if (f2fs_need_verity(inode, first_idx))
> + post_read_steps |= 1 << STEP_VERITY;
> +
> if (post_read_steps) {
> ctx = mempool_alloc(bio_post_read_ctx_pool, GFP_NOFS);
> if (!ctx) {
> @@ -591,7 +626,7 @@ static int f2fs_submit_page_read(struct inode *inode, struct page *page,
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> struct bio *bio;
>
> - bio = f2fs_grab_read_bio(inode, blkaddr, 1, 0);
> + bio = f2fs_grab_read_bio(inode, blkaddr, 1, 0, page->index);
> if (IS_ERR(bio))
> return PTR_ERR(bio);
>
> @@ -1514,6 +1549,15 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> return ret;
> }
>
> +static inline loff_t f2fs_readpage_limit(struct inode *inode)
> +{
> + if (IS_ENABLED(CONFIG_FS_VERITY) &&
> + (IS_VERITY(inode) || f2fs_verity_in_progress(inode)))
> + return inode->i_sb->s_maxbytes;
> +
> + return i_size_read(inode);
> +}
> +
> static int f2fs_read_single_page(struct inode *inode, struct page *page,
> unsigned nr_pages,
> struct f2fs_map_blocks *map,
> @@ -1532,7 +1576,7 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
>
> block_in_file = (sector_t)page->index;
> last_block = block_in_file + nr_pages;
> - last_block_in_file = (i_size_read(inode) + blocksize - 1) >>
> + last_block_in_file = (f2fs_readpage_limit(inode) + blocksize - 1) >>
> blkbits;
> if (last_block > last_block_in_file)
> last_block = last_block_in_file;
> @@ -1576,6 +1620,11 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
> } else {
> zero_out:
> zero_user_segment(page, 0, PAGE_SIZE);
> + if (f2fs_need_verity(inode, page->index) &&
> + !fsverity_verify_page(page)) {
> + ret = -EIO;
> + goto out;
> + }
> if (!PageUptodate(page))
> SetPageUptodate(page);
> unlock_page(page);
> @@ -1594,7 +1643,7 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
> }
> if (bio == NULL) {
> bio = f2fs_grab_read_bio(inode, block_nr, nr_pages,
> - is_readahead ? REQ_RAHEAD : 0);
> + is_readahead ? REQ_RAHEAD : 0, page->index);
> if (IS_ERR(bio)) {
> ret = PTR_ERR(bio);
> bio = NULL;
> @@ -1991,7 +2040,7 @@ static int __write_data_page(struct page *page, bool *submitted,
> if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING)))
> goto redirty_out;
>
> - if (page->index < end_index)
> + if (page->index < end_index || f2fs_verity_in_progress(inode))
> goto write;
>
> /*
> @@ -2383,7 +2432,8 @@ static int prepare_write_begin(struct f2fs_sb_info *sbi,
> * the block addresses when there is no need to fill the page.
> */
> if (!f2fs_has_inline_data(inode) && len == PAGE_SIZE &&
> - !is_inode_flag_set(inode, FI_NO_PREALLOC))
> + !is_inode_flag_set(inode, FI_NO_PREALLOC) &&
> + !f2fs_verity_in_progress(inode))
> return 0;
>
> /* f2fs_lock_op avoids race between write CP and convert_inline_page */
> @@ -2522,7 +2572,8 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping,
> if (len == PAGE_SIZE || PageUptodate(page))
> return 0;
>
> - if (!(pos & (PAGE_SIZE - 1)) && (pos + len) >= i_size_read(inode)) {
> + if (!(pos & (PAGE_SIZE - 1)) && (pos + len) >= i_size_read(inode) &&
> + !f2fs_verity_in_progress(inode)) {
> zero_user_segment(page, len, PAGE_SIZE);
> return 0;
> }
> @@ -2585,7 +2636,8 @@ static int f2fs_write_end(struct file *file,
>
> set_page_dirty(page);
>
> - if (pos + copied > i_size_read(inode))
> + if (pos + copied > i_size_read(inode) &&
> + !f2fs_verity_in_progress(inode))
> f2fs_i_size_write(inode, pos + copied);
> unlock_out:
> f2fs_put_page(page, 1);
> @@ -2906,7 +2958,9 @@ void f2fs_clear_page_cache_dirty_tag(struct page *page)
>
> int __init f2fs_init_post_read_processing(void)
> {
> - bio_post_read_ctx_cache = KMEM_CACHE(bio_post_read_ctx, 0);
> + bio_post_read_ctx_cache =
> + kmem_cache_create("f2fs_bio_post_read_ctx",
> + sizeof(struct bio_post_read_ctx), 0, 0, NULL);
> if (!bio_post_read_ctx_cache)
> goto fail;
> bio_post_read_ctx_pool =
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 06b89a9862ab2b..8477191ad1c9b2 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -25,6 +25,7 @@
> #include <crypto/hash.h>
>
> #include <linux/fscrypt.h>
> +#include <linux/fsverity.h>
>
> #ifdef CONFIG_F2FS_CHECK_FS
> #define f2fs_bug_on(sbi, condition) BUG_ON(condition)
> @@ -148,7 +149,7 @@ struct f2fs_mount_info {
> #define F2FS_FEATURE_QUOTA_INO 0x0080
> #define F2FS_FEATURE_INODE_CRTIME 0x0100
> #define F2FS_FEATURE_LOST_FOUND 0x0200
> -#define F2FS_FEATURE_VERITY 0x0400 /* reserved */
> +#define F2FS_FEATURE_VERITY 0x0400
> #define F2FS_FEATURE_SB_CHKSUM 0x0800
>
> #define __F2FS_HAS_FEATURE(raw_super, mask) \
> @@ -626,7 +627,7 @@ enum {
> #define FADVISE_ENC_NAME_BIT 0x08
> #define FADVISE_KEEP_SIZE_BIT 0x10
> #define FADVISE_HOT_BIT 0x20
> -#define FADVISE_VERITY_BIT 0x40 /* reserved */
> +#define FADVISE_VERITY_BIT 0x40
>
> #define FADVISE_MODIFIABLE_BITS (FADVISE_COLD_BIT | FADVISE_HOT_BIT)
>
> @@ -646,6 +647,8 @@ enum {
> #define file_is_hot(inode) is_file(inode, FADVISE_HOT_BIT)
> #define file_set_hot(inode) set_file(inode, FADVISE_HOT_BIT)
> #define file_clear_hot(inode) clear_file(inode, FADVISE_HOT_BIT)
> +#define file_is_verity(inode) is_file(inode, FADVISE_VERITY_BIT)
> +#define file_set_verity(inode) set_file(inode, FADVISE_VERITY_BIT)
>
> #define DEF_DIR_LEVEL 0
>
> @@ -2344,6 +2347,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
> #define F2FS_TOPDIR_FL 0x00020000 /* Top of directory hierarchies*/
> #define F2FS_HUGE_FILE_FL 0x00040000 /* Set to each huge file */
> #define F2FS_EXTENTS_FL 0x00080000 /* Inode uses extents */
> +#define F2FS_VERITY_FL 0x00100000 /* Verity protected inode */
> #define F2FS_EA_INODE_FL 0x00200000 /* Inode used for large EA */
> #define F2FS_EOFBLOCKS_FL 0x00400000 /* Blocks allocated beyond EOF */
> #define F2FS_NOCOW_FL 0x00800000 /* Do not cow file */
> @@ -2351,7 +2355,7 @@ static inline void f2fs_change_bit(unsigned int nr, char *addr)
> #define F2FS_PROJINHERIT_FL 0x20000000 /* Create with parents projid */
> #define F2FS_RESERVED_FL 0x80000000 /* reserved for ext4 lib */
>
> -#define F2FS_FL_USER_VISIBLE 0x30CBDFFF /* User visible flags */
> +#define F2FS_FL_USER_VISIBLE 0x30DBDFFF /* User visible flags */
> #define F2FS_FL_USER_MODIFIABLE 0x204BC0FF /* User modifiable flags */
>
> /* Flags we can manipulate with through F2FS_IOC_FSSETXATTR */
> @@ -2417,6 +2421,7 @@ enum {
> FI_PROJ_INHERIT, /* indicate file inherits projectid */
> FI_PIN_FILE, /* indicate file should not be gced */
> FI_ATOMIC_REVOKE_REQUEST, /* request to drop atomic data */
> + FI_VERITY_IN_PROGRESS, /* building fs-verity Merkle tree */
> };
>
> static inline void __mark_inode_dirty_flag(struct inode *inode,
> @@ -2456,6 +2461,12 @@ static inline void clear_inode_flag(struct inode *inode, int flag)
> __mark_inode_dirty_flag(inode, flag, false);
> }
>
> +static inline bool f2fs_verity_in_progress(struct inode *inode)
> +{
> + return IS_ENABLED(CONFIG_FS_VERITY) &&
> + is_inode_flag_set(inode, FI_VERITY_IN_PROGRESS);
> +}
> +
> static inline void set_acl_inode(struct inode *inode, umode_t mode)
> {
> F2FS_I(inode)->i_acl_mode = mode;
> @@ -3524,6 +3535,9 @@ void f2fs_exit_sysfs(void);
> int f2fs_register_sysfs(struct f2fs_sb_info *sbi);
> void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi);
>
> +/* verity.c */
> +extern const struct fsverity_operations f2fs_verityops;
> +
> /*
> * crypto support
> */
> @@ -3546,7 +3560,7 @@ static inline void f2fs_set_encrypted_inode(struct inode *inode)
> */
> static inline bool f2fs_post_read_required(struct inode *inode)
> {
> - return f2fs_encrypted_file(inode);
> + return f2fs_encrypted_file(inode) || fsverity_active(inode);
> }
>
> #define F2FS_FEATURE_FUNCS(name, flagname) \
> @@ -3564,6 +3578,7 @@ F2FS_FEATURE_FUNCS(flexible_inline_xattr, FLEXIBLE_INLINE_XATTR);
> F2FS_FEATURE_FUNCS(quota_ino, QUOTA_INO);
> F2FS_FEATURE_FUNCS(inode_crtime, INODE_CRTIME);
> F2FS_FEATURE_FUNCS(lost_found, LOST_FOUND);
> +F2FS_FEATURE_FUNCS(verity, VERITY);
> F2FS_FEATURE_FUNCS(sb_chksum, SB_CHKSUM);
>
> #ifdef CONFIG_BLK_DEV_ZONED
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 45b45f37d347e4..6706c2081941a2 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -493,6 +493,10 @@ static int f2fs_file_open(struct inode *inode, struct file *filp)
> {
> int err = fscrypt_file_open(inode, filp);
>
> + if (err)
> + return err;
> +
> + err = fsverity_file_open(inode, filp);
> if (err)
> return err;
>
> @@ -781,6 +785,10 @@ int f2fs_setattr(struct dentry *dentry, struct iattr *attr)
> if (err)
> return err;
>
> + err = fsverity_prepare_setattr(dentry, attr);
> + if (err)
> + return err;
> +
> if (is_quota_modification(inode, attr)) {
> err = dquot_initialize(inode);
> if (err)
> @@ -1656,6 +1664,8 @@ static int f2fs_ioc_getflags(struct file *filp, unsigned long arg)
>
> if (IS_ENCRYPTED(inode))
> flags |= F2FS_ENCRYPT_FL;
> + if (IS_VERITY(inode))
> + flags |= F2FS_VERITY_FL;
> if (f2fs_has_inline_data(inode) || f2fs_has_inline_dentry(inode))
> flags |= F2FS_INLINE_DATA_FL;
> if (is_inode_flag_set(inode, FI_PIN_FILE))
> @@ -2980,6 +2990,30 @@ static int f2fs_ioc_precache_extents(struct file *filp, unsigned long arg)
> return f2fs_precache_extents(file_inode(filp));
> }
>
> +static int f2fs_ioc_enable_verity(struct file *filp, unsigned long arg)
> +{
> + struct inode *inode = file_inode(filp);
> +
> + f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
> +
> + if (!f2fs_sb_has_verity(F2FS_I_SB(inode))) {
> + f2fs_msg(inode->i_sb, KERN_WARNING,
> + "Can't enable fs-verity on inode %lu: the verity feature is not enabled on this filesystem.\n",
> + inode->i_ino);
> + return -EOPNOTSUPP;
> + }
> +
> + return fsverity_ioctl_enable(filp, (const void __user *)arg);
> +}
> +
> +static int f2fs_ioc_measure_verity(struct file *filp, unsigned long arg)
> +{
> + if (!f2fs_sb_has_verity(F2FS_I_SB(file_inode(filp))))
> + return -EOPNOTSUPP;
> +
> + return fsverity_ioctl_measure(filp, (void __user *)arg);
> +}
> +
> long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> {
> if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(filp)))))
> @@ -3036,6 +3070,10 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> return f2fs_ioc_set_pin_file(filp, arg);
> case F2FS_IOC_PRECACHE_EXTENTS:
> return f2fs_ioc_precache_extents(filp, arg);
> + case FS_IOC_ENABLE_VERITY:
> + return f2fs_ioc_enable_verity(filp, arg);
> + case FS_IOC_MEASURE_VERITY:
> + return f2fs_ioc_measure_verity(filp, arg);
> default:
> return -ENOTTY;
> }
> @@ -3149,6 +3187,8 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> case F2FS_IOC_GET_PIN_FILE:
> case F2FS_IOC_SET_PIN_FILE:
> case F2FS_IOC_PRECACHE_EXTENTS:
> + case FS_IOC_ENABLE_VERITY:
> + case FS_IOC_MEASURE_VERITY:
> break;
> default:
> return -ENOIOCTLCMD;
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index ccb02226dd2c0c..b2f945b1afe501 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -46,9 +46,11 @@ void f2fs_set_inode_flags(struct inode *inode)
> new_fl |= S_DIRSYNC;
> if (file_is_encrypt(inode))
> new_fl |= S_ENCRYPTED;
> + if (file_is_verity(inode))
> + new_fl |= S_VERITY;
> inode_set_flags(inode, new_fl,
> S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|
> - S_ENCRYPTED);
> + S_ENCRYPTED|S_VERITY);
> }
>
> static void __get_inode_rdev(struct inode *inode, struct f2fs_inode *ri)
> @@ -749,6 +751,7 @@ void f2fs_evict_inode(struct inode *inode)
> }
> out_clear:
> fscrypt_put_encryption_info(inode);
> + fsverity_cleanup_inode(inode);
> clear_inode(inode);
> }
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 6b959bbb336a30..ea4a247d6ed6f7 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3177,6 +3177,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> sb->s_op = &f2fs_sops;
> #ifdef CONFIG_FS_ENCRYPTION
> sb->s_cop = &f2fs_cryptops;
> +#endif
> +#ifdef CONFIG_FS_VERITY
> + sb->s_vop = &f2fs_verityops;
> #endif
> sb->s_xattr = f2fs_xattr_handlers;
> sb->s_export_op = &f2fs_export_ops;
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index 729f46a3c9ee0b..b3e28467db7279 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -117,6 +117,9 @@ static ssize_t features_show(struct f2fs_attr *a,
> if (f2fs_sb_has_lost_found(sbi))
> len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
> len ? ", " : "", "lost_found");
> + if (f2fs_sb_has_verity(sbi))
> + len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
> + len ? ", " : "", "verity");
> if (f2fs_sb_has_sb_chksum(sbi))
> len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
> len ? ", " : "", "sb_checksum");
> @@ -350,6 +353,7 @@ enum feat_id {
> FEAT_QUOTA_INO,
> FEAT_INODE_CRTIME,
> FEAT_LOST_FOUND,
> + FEAT_VERITY,
> FEAT_SB_CHECKSUM,
> };
>
> @@ -367,6 +371,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
> case FEAT_QUOTA_INO:
> case FEAT_INODE_CRTIME:
> case FEAT_LOST_FOUND:
> + case FEAT_VERITY:
> case FEAT_SB_CHECKSUM:
> return snprintf(buf, PAGE_SIZE, "supported\n");
> }
> @@ -455,6 +460,9 @@ F2FS_FEATURE_RO_ATTR(flexible_inline_xattr, FEAT_FLEXIBLE_INLINE_XATTR);
> F2FS_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
> F2FS_FEATURE_RO_ATTR(inode_crtime, FEAT_INODE_CRTIME);
> F2FS_FEATURE_RO_ATTR(lost_found, FEAT_LOST_FOUND);
> +#ifdef CONFIG_FS_VERITY
> +F2FS_FEATURE_RO_ATTR(verity, FEAT_VERITY);
> +#endif
> F2FS_FEATURE_RO_ATTR(sb_checksum, FEAT_SB_CHECKSUM);
>
> #define ATTR_LIST(name) (&f2fs_attr_##name.attr)
> @@ -517,6 +525,9 @@ static struct attribute *f2fs_feat_attrs[] = {
> ATTR_LIST(quota_ino),
> ATTR_LIST(inode_crtime),
> ATTR_LIST(lost_found),
> +#ifdef CONFIG_FS_VERITY
> + ATTR_LIST(verity),
> +#endif
> ATTR_LIST(sb_checksum),
> NULL,
> };
> diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
> new file mode 100644
> index 00000000000000..dd9bb47ced0093
> --- /dev/null
> +++ b/fs/f2fs/verity.c
> @@ -0,0 +1,233 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * fs/f2fs/verity.c: fs-verity support for f2fs
> + *
> + * Copyright 2019 Google LLC
> + */
> +
> +/*
> + * Implementation of fsverity_operations for f2fs.
> + *
> + * Like ext4, f2fs stores the verity metadata (Merkle tree and
> + * fsverity_descriptor) past the end of the file, starting at the first 64K
> + * boundary beyond i_size. This approach works because (a) verity files are
> + * readonly, and (b) pages fully beyond i_size aren't visible to userspace but
> + * can be read/written internally by f2fs with only some relatively small
> + * changes to f2fs. Extended attributes cannot be used because (a) f2fs limits
> + * the total size of an inode's xattr entries to 4096 bytes, which wouldn't be
> + * enough for even a single Merkle tree block, and (b) f2fs encryption doesn't
> + * encrypt xattrs, yet the verity metadata *must* be encrypted when the file is
> + * because it contains hashes of the plaintext data.
> + *
> + * Using a 64K boundary rather than a 4K one keeps things ready for
> + * architectures with 64K pages, and it doesn't necessarily waste space on-disk
> + * since there can be a hole between i_size and the start of the Merkle tree.
> + */
> +
> +#include <linux/f2fs_fs.h>
> +
> +#include "f2fs.h"
> +#include "xattr.h"
> +
> +static inline loff_t f2fs_verity_metadata_pos(const struct inode *inode)
> +{
> + return round_up(inode->i_size, 65536);
> +}
> +
> +/*
> + * Read some verity metadata from the inode. __vfs_read() can't be used because
> + * we need to read beyond i_size.
> + */
> +static int pagecache_read(struct inode *inode, void *buf, size_t count,
> + loff_t pos)
> +{
> + while (count) {
> + size_t n = min_t(size_t, count,
> + PAGE_SIZE - offset_in_page(pos));
> + struct page *page;
> + void *addr;
> +
> + page = read_mapping_page(inode->i_mapping, pos >> PAGE_SHIFT,
> + NULL);
> + if (IS_ERR(page))
> + return PTR_ERR(page);
> +
> + addr = kmap_atomic(page);
> + memcpy(buf, addr + offset_in_page(pos), n);
> + kunmap_atomic(addr);
> +
> + put_page(page);
> +
> + buf += n;
> + pos += n;
> + count -= n;
> + }
> + return 0;
> +}
> +
> +/*
> + * Write some verity metadata to the inode for FS_IOC_ENABLE_VERITY.
> + * kernel_write() can't be used because the file descriptor is readonly.
> + */
> +static int pagecache_write(struct inode *inode, const void *buf, size_t count,
> + loff_t pos)
> +{
> + while (count) {
> + size_t n = min_t(size_t, count,
> + PAGE_SIZE - offset_in_page(pos));
> + struct page *page;
> + void *fsdata;
> + void *addr;
> + int res;
> +
> + res = pagecache_write_begin(NULL, inode->i_mapping, pos, n, 0,
> + &page, &fsdata);
> + if (res)
> + return res;
> +
> + addr = kmap_atomic(page);
> + memcpy(addr + offset_in_page(pos), buf, n);
> + kunmap_atomic(addr);
> +
> + res = pagecache_write_end(NULL, inode->i_mapping, pos, n, n,
> + page, fsdata);
> + if (res < 0)
> + return res;
> + if (res != n)
> + return -EIO;
> +
> + buf += n;
> + pos += n;
> + count -= n;
> + }
> + return 0;
> +}
> +
> +/*
> + * Format of f2fs verity xattr. This points to the location of the verity
> + * descriptor within the file data rather than containing it directly because
> + * the verity descriptor *must* be encrypted when f2fs encryption is used. But,
> + * f2fs encryption does not encrypt xattrs.
> + */
> +struct fsverity_descriptor_location {
> + __le32 version;
> + __le32 size;
> + __le64 pos;
> +};
> +
> +static int f2fs_begin_enable_verity(struct file *filp)
> +{
> + struct inode *inode = file_inode(filp);
> + int err;
> +
> + err = f2fs_convert_inline_inode(inode);
> + if (err)
> + return err;
> +
> + err = dquot_initialize(inode);
> + if (err)
> + return err;
> +
> + set_inode_flag(inode, FI_VERITY_IN_PROGRESS);
> + return 0;
> +}
> +
> +static int f2fs_end_enable_verity(struct file *filp, const void *desc,
> + size_t desc_size, u64 merkle_tree_size)
> +{
> + struct inode *inode = file_inode(filp);
> + u64 desc_pos = f2fs_verity_metadata_pos(inode) + merkle_tree_size;
> + struct fsverity_descriptor_location dloc = {
> + .version = cpu_to_le32(1),
> + .size = cpu_to_le32(desc_size),
> + .pos = cpu_to_le64(desc_pos),
> + };
> + int err = 0;
> +
> + if (desc != NULL) {
> + /* Succeeded; write the verity descriptor. */
> + err = pagecache_write(inode, desc, desc_size, desc_pos);
> +
> + /* Write all pages before clearing FI_VERITY_IN_PROGRESS. */
> + if (!err)
> + err = filemap_write_and_wait(inode->i_mapping);
> + } else {
> + /* Failed; truncate anything we wrote past i_size. */
> + f2fs_truncate(inode);
> + }
> +
> + clear_inode_flag(inode, FI_VERITY_IN_PROGRESS);
> +
> + if (desc != NULL && !err) {
> + err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_VERITY,
> + F2FS_XATTR_NAME_VERITY, &dloc, sizeof(dloc),
> + NULL, XATTR_CREATE);
> + if (!err) {
> + file_set_verity(inode);
> + f2fs_set_inode_flags(inode);
> + f2fs_mark_inode_dirty_sync(inode, true);
> + }
> + }
> + return err;
> +}
> +
> +static int f2fs_get_verity_descriptor(struct inode *inode, void *buf,
> + size_t buf_size)
> +{
> + struct fsverity_descriptor_location dloc;
> + int res;
> + u32 size;
> + u64 pos;
> +
> + /* Get the descriptor location */
> + res = f2fs_getxattr(inode, F2FS_XATTR_INDEX_VERITY,
> + F2FS_XATTR_NAME_VERITY, &dloc, sizeof(dloc), NULL);
> + if (res < 0 && res != -ERANGE)
> + return res;
> + if (res != sizeof(dloc) || dloc.version != cpu_to_le32(1)) {
> + f2fs_msg(inode->i_sb, KERN_WARNING,
> + "unknown verity xattr format");
> + return -EINVAL;
> + }
> + size = le32_to_cpu(dloc.size);
> + pos = le64_to_cpu(dloc.pos);
> +
> + /* Get the descriptor */
> + if (pos + size < pos || pos + size > inode->i_sb->s_maxbytes ||
> + pos < f2fs_verity_metadata_pos(inode) || size > INT_MAX) {
> + f2fs_msg(inode->i_sb, KERN_WARNING, "invalid verity xattr");
> + return -EUCLEAN; /* EFSCORRUPTED */
> + }
> + if (buf_size) {
> + if (size > buf_size)
> + return -ERANGE;
> + res = pagecache_read(inode, buf, size, pos);
> + if (res)
> + return res;
> + }
> + return size;
> +}
> +
> +static struct page *f2fs_read_merkle_tree_page(struct inode *inode,
> + pgoff_t index)
> +{
> + index += f2fs_verity_metadata_pos(inode) >> PAGE_SHIFT;
> +
> + return read_mapping_page(inode->i_mapping, index, NULL);
> +}
> +
> +static int f2fs_write_merkle_tree_block(struct inode *inode, const void *buf,
> + u64 index, int log_blocksize)
> +{
> + loff_t pos = f2fs_verity_metadata_pos(inode) + (index << log_blocksize);
> +
> + return pagecache_write(inode, buf, 1 << log_blocksize, pos);
> +}
> +
> +const struct fsverity_operations f2fs_verityops = {
> + .begin_enable_verity = f2fs_begin_enable_verity,
> + .end_enable_verity = f2fs_end_enable_verity,
> + .get_verity_descriptor = f2fs_get_verity_descriptor,
> + .read_merkle_tree_page = f2fs_read_merkle_tree_page,
> + .write_merkle_tree_block = f2fs_write_merkle_tree_block,
> +};
> diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
> index a90920e2f94980..de0c600b9cab09 100644
> --- a/fs/f2fs/xattr.h
> +++ b/fs/f2fs/xattr.h
> @@ -34,8 +34,10 @@
> #define F2FS_XATTR_INDEX_ADVISE 7
> /* Should be same as EXT4_XATTR_INDEX_ENCRYPTION */
> #define F2FS_XATTR_INDEX_ENCRYPTION 9
> +#define F2FS_XATTR_INDEX_VERITY 11
>
> #define F2FS_XATTR_NAME_ENCRYPTION_CONTEXT "c"
> +#define F2FS_XATTR_NAME_VERITY "v"
>
> struct f2fs_xattr_header {
> __le32 h_magic; /* magic number for identification */
> --
> 2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH V34 01/29] security: Support early LSMs
From: Kees Cook @ 2019-06-22 23:36 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett
In-Reply-To: <20190622000358.19895-2-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:30PM -0700, Matthew Garrett wrote:
> The lockdown module is intended to allow for kernels to be locked down
> early in boot - sufficiently early that we don't have the ability to
> kmalloc() yet. Add support for early initialisation of some LSMs, and
> then add them to the list of names when we do full initialisation later.
> Early LSMs are initialised in link order and cannot be overridden via
> boot parameters, and cannot make use of kmalloc() (since the allocator
> isn't initialised yet).
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> include/asm-generic/vmlinux.lds.h | 8 ++++-
> include/linux/lsm_hooks.h | 6 ++++
> include/linux/security.h | 1 +
> init/main.c | 1 +
> security/security.c | 50 ++++++++++++++++++++++++++-----
> 5 files changed, 57 insertions(+), 9 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index f8f6f04c4453..e1963352fdb6 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -208,8 +208,13 @@
> __start_lsm_info = .; \
> KEEP(*(.lsm_info.init)) \
> __end_lsm_info = .;
> +#define EARLY_LSM_TABLE() . = ALIGN(8); \
> + __start_early_lsm_info = .; \
> + KEEP(*(.early_lsm_info.init)) \
> + __end_early_lsm_info = .;
> #else
> #define LSM_TABLE()
> +#define EARLY_LSM_TABLE()
> #endif
>
> #define ___OF_TABLE(cfg, name) _OF_TABLE_##cfg(name)
> @@ -610,7 +615,8 @@
> ACPI_PROBE_TABLE(irqchip) \
> ACPI_PROBE_TABLE(timer) \
> EARLYCON_TABLE() \
> - LSM_TABLE()
> + LSM_TABLE() \
> + EARLY_LSM_TABLE()
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index a240a3fc5fc4..66fd1eac7a32 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2085,12 +2085,18 @@ struct lsm_info {
> };
>
> extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
> +extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
>
> #define DEFINE_LSM(lsm) \
> static struct lsm_info __lsm_##lsm \
> __used __section(.lsm_info.init) \
> __aligned(sizeof(unsigned long))
>
> +#define DEFINE_EARLY_LSM(lsm) \
> + static struct lsm_info __early_lsm_##lsm \
> + __used __section(.early_lsm_info.init) \
> + __aligned(sizeof(unsigned long))
> +
> #ifdef CONFIG_SECURITY_SELINUX_DISABLE
> /*
> * Assuring the safety of deleting a security module is up to
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 49f2685324b0..1bb6fb2f1523 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -194,6 +194,7 @@ int unregister_lsm_notifier(struct notifier_block *nb);
>
> /* prototypes */
> extern int security_init(void);
> +extern int early_security_init(void);
>
> /* Security operations */
> int security_binder_set_context_mgr(struct task_struct *mgr);
> diff --git a/init/main.c b/init/main.c
> index 598e278b46f7..f3faeb89c75f 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -563,6 +563,7 @@ asmlinkage __visible void __init start_kernel(void)
> boot_cpu_init();
> page_address_init();
> pr_notice("%s", linux_banner);
> + early_security_init();
> setup_arch(&command_line);
> /*
> * Set up the the initial canary and entropy after arch
> diff --git a/security/security.c b/security/security.c
> index 23cbb1a295a3..487e1f3eb2df 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -37,6 +37,7 @@
>
> /* How many LSMs were built into the kernel? */
> #define LSM_COUNT (__end_lsm_info - __start_lsm_info)
> +#define EARLY_LSM_COUNT (__end_early_lsm_info - __start_early_lsm_info)
>
> struct security_hook_heads security_hook_heads __lsm_ro_after_init;
> static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
> @@ -281,6 +282,8 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> static void __init lsm_early_cred(struct cred *cred);
> static void __init lsm_early_task(struct task_struct *task);
>
> +static int lsm_append(const char *new, char **result);
> +
> static void __init ordered_lsm_init(void)
> {
> struct lsm_info **lsm;
> @@ -327,6 +330,26 @@ static void __init ordered_lsm_init(void)
> kfree(ordered_lsms);
> }
>
> +int __init early_security_init(void)
> +{
> + int i;
> + struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
> + struct lsm_info *lsm;
> +
> + for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
> + i++)
> + INIT_HLIST_HEAD(&list[i]);
> +
> + for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
> + if (!lsm->enabled)
> + lsm->enabled = &lsm_enabled_true;
> + prepare_lsm(lsm);
> + initialize_lsm(lsm);
> + }
> +
> + return 0;
> +}
> +
> /**
> * security_init - initializes the security framework
> *
> @@ -334,14 +357,18 @@ static void __init ordered_lsm_init(void)
> */
> int __init security_init(void)
> {
> - int i;
> - struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
> + struct lsm_info *lsm;
>
> pr_info("Security Framework initializing\n");
>
> - for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
> - i++)
> - INIT_HLIST_HEAD(&list[i]);
> + /*
> + * Append the names of the early LSM modules now that kmalloc() is
> + * available
> + */
> + for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
> + if (lsm->enabled)
> + lsm_append(lsm->name, &lsm_names);
> + }
>
> /* Load LSMs in specified order. */
> ordered_lsm_init();
> @@ -388,7 +415,7 @@ static bool match_last_lsm(const char *list, const char *lsm)
> return !strcmp(last, lsm);
> }
>
> -static int lsm_append(char *new, char **result)
> +static int lsm_append(const char *new, char **result)
> {
> char *cp;
>
> @@ -426,8 +453,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> hooks[i].lsm = lsm;
> hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> }
> - if (lsm_append(lsm, &lsm_names) < 0)
> - panic("%s - Cannot get early memory.\n", __func__);
> +
> + /*
> + * Don't try to append during early_security_init(), we'll come back
> + * and fix this up afterwards.
> + */
> + if (slab_is_available()) {
> + if (lsm_append(lsm, &lsm_names) < 0)
> + panic("%s - Cannot get early memory.\n", __func__);
> + }
> }
>
> int call_lsm_notifier(enum lsm_event event, void *data)
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 02/29] security: Add a "locked down" LSM hook
From: Kees Cook @ 2019-06-22 23:37 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett
In-Reply-To: <20190622000358.19895-3-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:31PM -0700, Matthew Garrett wrote:
> Add a mechanism to allow LSMs to make a policy decision around whether
> kernel functionality that would allow tampering with or examining the
> runtime state of the kernel should be permitted.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> include/linux/lsm_hooks.h | 2 ++
> include/linux/security.h | 11 +++++++++++
> security/security.c | 6 ++++++
> 3 files changed, 19 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 66fd1eac7a32..df2aebc99838 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1790,6 +1790,7 @@ union security_list_options {
> int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
> void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
> #endif /* CONFIG_BPF_SYSCALL */
> + int (*locked_down)(enum lockdown_reason what);
> };
>
> struct security_hook_heads {
> @@ -2027,6 +2028,7 @@ struct security_hook_heads {
> struct hlist_head bpf_prog_alloc_security;
> struct hlist_head bpf_prog_free_security;
> #endif /* CONFIG_BPF_SYSCALL */
> + struct hlist_head locked_down;
> } __randomize_layout;
>
> /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1bb6fb2f1523..9eaf02e70707 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -76,6 +76,12 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +enum lockdown_reason {
> + LOCKDOWN_NONE,
> + LOCKDOWN_INTEGRITY_MAX,
> + LOCKDOWN_CONFIDENTIALITY_MAX,
> +};
> +
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> int cap, unsigned int opts);
> @@ -389,6 +395,7 @@ void security_inode_invalidate_secctx(struct inode *inode);
> int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> +int security_locked_down(enum lockdown_reason what);
> #else /* CONFIG_SECURITY */
>
> static inline int call_lsm_notifier(enum lsm_event event, void *data)
> @@ -1189,6 +1196,10 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
> {
> return -EOPNOTSUPP;
> }
> +static inline int security_locked_down(enum lockdown_reason what)
> +{
> + return 0;
> +}
> #endif /* CONFIG_SECURITY */
>
> #ifdef CONFIG_SECURITY_NETWORK
> diff --git a/security/security.c b/security/security.c
> index 487e1f3eb2df..553f50e9a106 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2382,3 +2382,9 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux)
> call_void_hook(bpf_prog_free_security, aux);
> }
> #endif /* CONFIG_BPF_SYSCALL */
> +
> +int security_locked_down(enum lockdown_reason what)
> +{
> + return call_int_hook(locked_down, 0, what);
> +}
> +EXPORT_SYMBOL(security_locked_down);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 03/29] security: Add a static lockdown policy LSM
From: Kees Cook @ 2019-06-22 23:37 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett, David Howells
In-Reply-To: <20190622000358.19895-4-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:32PM -0700, Matthew Garrett wrote:
> While existing LSMs can be extended to handle lockdown policy,
> distributions generally want to be able to apply a straightforward
> static policy. This patch adds a simple LSM that can be configured to
> reject either integrity or all lockdown queries, and can be configured
> at runtime (through securityfs), boot time (via a kernel parameter) or
> build time (via a kconfig option). Based on initial code by David
> Howells.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Cc: David Howells <dhowells@redhat.com>
> ---
> .../admin-guide/kernel-parameters.txt | 9 +
> include/linux/security.h | 4 +
> security/Kconfig | 3 +-
> security/Makefile | 2 +
> security/lockdown/Kconfig | 47 +++++
> security/lockdown/Makefile | 1 +
> security/lockdown/lockdown.c | 172 ++++++++++++++++++
> 7 files changed, 237 insertions(+), 1 deletion(-)
> create mode 100644 security/lockdown/Kconfig
> create mode 100644 security/lockdown/Makefile
> create mode 100644 security/lockdown/lockdown.c
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2b8ee90bb644..fa336f6cd5bc 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2239,6 +2239,15 @@
> lockd.nlm_udpport=M [NFS] Assign UDP port.
> Format: <integer>
>
> + lockdown= [SECURITY]
> + { integrity | confidentiality }
> + Enable the kernel lockdown feature. If set to
> + integrity, kernel features that allow userland to
> + modify the running kernel are disabled. If set to
> + confidentiality, kernel features that allow userland
> + to extract confidential information from the kernel
> + are also disabled.
> +
> locktorture.nreaders_stress= [KNL]
> Set the number of locking read-acquisition kthreads.
> Defaults to being automatically set based on the
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 9eaf02e70707..c808d344ec75 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -76,6 +76,10 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +/*
> + * If you add to this, remember to extend lockdown_reasons in
> + * security/lockdown/lockdown.c.
> + */
> enum lockdown_reason {
> LOCKDOWN_NONE,
> LOCKDOWN_INTEGRITY_MAX,
> diff --git a/security/Kconfig b/security/Kconfig
> index 1d6463fb1450..c35aa72103df 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -236,12 +236,13 @@ source "security/apparmor/Kconfig"
> source "security/loadpin/Kconfig"
> source "security/yama/Kconfig"
> source "security/safesetid/Kconfig"
> +source "security/lockdown/Kconfig"
>
> source "security/integrity/Kconfig"
>
> config LSM
> string "Ordered list of enabled LSMs"
> - default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> + default "lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> help
> A comma-separated list of LSMs, in initialization order.
> Any LSMs left off this list will be ignored. This can be
> diff --git a/security/Makefile b/security/Makefile
> index c598b904938f..be1dd9d2cb2f 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -11,6 +11,7 @@ subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> subdir-$(CONFIG_SECURITY_YAMA) += yama
> subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin
> subdir-$(CONFIG_SECURITY_SAFESETID) += safesetid
> +subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown
>
> # always enable default capabilities
> obj-y += commoncap.o
> @@ -27,6 +28,7 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/
> obj-$(CONFIG_SECURITY_YAMA) += yama/
> obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
> obj-$(CONFIG_SECURITY_SAFESETID) += safesetid/
> +obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown/
> obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
>
> # Object integrity file lists
> diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
> new file mode 100644
> index 000000000000..7374ba76d8eb
> --- /dev/null
> +++ b/security/lockdown/Kconfig
> @@ -0,0 +1,47 @@
> +config SECURITY_LOCKDOWN_LSM
> + bool "Basic module for enforcing kernel lockdown"
> + depends on SECURITY
> + help
> + Build support for an LSM that enforces a coarse kernel lockdown
> + behaviour.
> +
> +config SECURITY_LOCKDOWN_LSM_EARLY
> + bool "Enable lockdown LSM early in init"
> + depends on SECURITY_LOCKDOWN_LSM
> + help
> + Enable the lockdown LSM early in boot. This is necessary in order
> + to ensure that lockdown enforcement can be carried out on kernel
> + boot parameters that are otherwise parsed before the security
> + subsystem is fully initialised. If enabled, lockdown will
> + unconditionally be called before any other LSMs.
> +
> +choice
> + prompt "Kernel default lockdown mode"
> + default LOCK_DOWN_KERNEL_FORCE_NONE
> + depends on SECURITY_LOCKDOWN_LSM
> + help
> + The kernel can be configured to default to differing levels of
> + lockdown.
> +
> +config LOCK_DOWN_KERNEL_FORCE_NONE
> + bool "None"
> + help
> + No lockdown functionality is enabled by default. Lockdown may be
> + enabled via the kernel commandline or /sys/kernel/security/lockdown.
> +
> +config LOCK_DOWN_KERNEL_FORCE_INTEGRITY
> + bool "Integrity"
> + help
> + The kernel runs in integrity mode by default. Features that allow
> + the kernel to be modified at runtime are disabled.
> +
> +config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
> + bool "Confidentiality"
> + help
> + The kernel runs in confidentiality mode by default. Features that
> + allow the kernel to be modified at runtime or that permit userland
> + code to read confidential material held inside the kernel are
> + disabled.
> +
> +endchoice
> +
> diff --git a/security/lockdown/Makefile b/security/lockdown/Makefile
> new file mode 100644
> index 000000000000..e3634b9017e7
> --- /dev/null
> +++ b/security/lockdown/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown.o
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> new file mode 100644
> index 000000000000..8e39b36b8f33
> --- /dev/null
> +++ b/security/lockdown/lockdown.c
> @@ -0,0 +1,172 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Lock down the kernel
> + *
> + * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +#include <linux/security.h>
> +#include <linux/export.h>
> +#include <linux/lsm_hooks.h>
> +
> +static enum lockdown_reason kernel_locked_down;
> +
> +static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> + [LOCKDOWN_NONE] = "none",
> + [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> + [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> +};
> +
> +static enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
> + LOCKDOWN_INTEGRITY_MAX,
> + LOCKDOWN_CONFIDENTIALITY_MAX};
> +
> +/*
> + * Put the kernel into lock-down mode.
> + */
> +static int lock_kernel_down(const char *where, enum lockdown_reason level)
> +{
> + if (kernel_locked_down >= level)
> + return -EPERM;
> +
> + kernel_locked_down = level;
> + pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
> + where);
> + return 0;
> +}
> +
> +static int __init lockdown_param(char *level)
> +{
> + if (!level)
> + return -EINVAL;
> +
> + if (strcmp(level, "integrity") == 0)
> + lock_kernel_down("command line", LOCKDOWN_INTEGRITY_MAX);
> + else if (strcmp(level, "confidentiality") == 0)
> + lock_kernel_down("command line", LOCKDOWN_CONFIDENTIALITY_MAX);
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +early_param("lockdown", lockdown_param);
> +
> +/**
> + * lockdown_is_locked_down - Find out if the kernel is locked down
> + * @what: Tag to use in notice generated if lockdown is in effect
> + */
> +static int lockdown_is_locked_down(enum lockdown_reason what)
> +{
> + if ((kernel_locked_down >= what)) {
> + if (lockdown_reasons[what])
> + pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
> + lockdown_reasons[what]);
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +
> +static struct security_hook_list lockdown_hooks[] __lsm_ro_after_init = {
> + LSM_HOOK_INIT(locked_down, lockdown_is_locked_down),
> +};
> +
> +static int __init lockdown_lsm_init(void)
> +{
> +#if defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY)
> + lock_kernel_down("Kernel configuration", LOCKDOWN_INTEGRITY_MAX);
> +#elif defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY)
> + lock_kernel_down("Kernel configuration", LOCKDOWN_CONFIDENTIALITY_MAX);
> +#endif
> + security_add_hooks(lockdown_hooks, ARRAY_SIZE(lockdown_hooks),
> + "lockdown");
> + return 0;
> +}
> +
> +static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + char temp[80];
> + int i, offset=0;
> +
> + for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {
> + enum lockdown_reason level = lockdown_levels[i];
> +
> + if (lockdown_reasons[level]) {
> + const char *label = lockdown_reasons[level];
> +
> + if (kernel_locked_down == level)
> + offset += sprintf(temp+offset, "[%s] ", label);
> + else
> + offset += sprintf(temp+offset, "%s ", label);
> + }
> + }
> +
> + /* Convert the last space to a newline if needed. */
> + if (offset > 0)
> + temp[offset-1] = '\n';
> +
> + return simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
> +}
> +
> +static ssize_t lockdown_write(struct file *file, const char __user *buf,
> + size_t n, loff_t *ppos)
> +{
> + char *state;
> + int i, len, err = -EINVAL;
> +
> + state = memdup_user_nul(buf, n);
> + if (IS_ERR(state))
> + return PTR_ERR(state);
> +
> + len = strlen(state);
> + if (len && state[len-1] == '\n') {
> + state[len-1] = '\0';
> + len--;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {
> + enum lockdown_reason level = lockdown_levels[i];
> + const char *label = lockdown_reasons[level];
> +
> + if (label && !strcmp(state, label))
> + err = lock_kernel_down("securityfs", level);
> + }
> +
> + kfree(state);
> + return err ? err : n;
> +}
> +
> +static const struct file_operations lockdown_ops = {
> + .read = lockdown_read,
> + .write = lockdown_write,
> +};
> +
> +static int __init lockdown_secfs_init(void)
> +{
> + struct dentry *dentry;
> +
> + dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
> + &lockdown_ops);
> + if (IS_ERR(dentry))
> + return PTR_ERR(dentry);
> +
> + return 0;
> +}
> +
> +core_initcall(lockdown_secfs_init);
> +
> +#ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
> +DEFINE_EARLY_LSM(lockdown) = {
> +#else
> +DEFINE_LSM(lockdown) = {
> +#endif
> + .name = "lockdown",
> + .init = lockdown_lsm_init,
> +};
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 04/29] Enforce module signatures if the kernel is locked down
From: Kees Cook @ 2019-06-22 23:48 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Jessica Yu
In-Reply-To: <20190622000358.19895-5-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:33PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> If the kernel is locked down, require that all modules have valid
> signatures that we can verify.
>
> I have adjusted the errors generated:
>
> (1) If there's no signature (ENODATA) or we can't check it (ENOPKG,
> ENOKEY), then:
>
> (a) If signatures are enforced then EKEYREJECTED is returned.
>
> (b) If there's no signature or we can't check it, but the kernel is
> locked down then EPERM is returned (this is then consistent with
> other lockdown cases).
>
> (2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails
> the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we
> return the error we got.
>
> Note that the X.509 code doesn't check for key expiry as the RTC might not
> be valid or might not have been transferred to the kernel's clock yet.
>
> [Modified by Matthew Garrett to remove the IMA integration. This will
> be replaced with integration with the IMA architecture policy
> patchset.]
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
> Cc: Jessica Yu <jeyu@kernel.org>
> ---
> include/linux/security.h | 1 +
> kernel/module.c | 38 +++++++++++++++++++++++++++++-------
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c808d344ec75..46d85cd63b06 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -82,6 +82,7 @@ enum lsm_event {
> */
> enum lockdown_reason {
> LOCKDOWN_NONE,
> + LOCKDOWN_MODULE_SIGNATURE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/module.c b/kernel/module.c
> index 0b9aa8ab89f0..6aa681edd660 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2763,8 +2763,9 @@ static inline void kmemleak_load_module(const struct module *mod,
> #ifdef CONFIG_MODULE_SIG
> static int module_sig_check(struct load_info *info, int flags)
> {
> - int err = -ENOKEY;
> + int ret, err = -ENODATA;
> const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> + const char *reason;
> const void *mod = info->hdr;
>
> /*
> @@ -2779,16 +2780,39 @@ static int module_sig_check(struct load_info *info, int flags)
> err = mod_verify_sig(mod, info);
> }
>
> - if (!err) {
> + switch (err) {
> + case 0:
> info->sig_ok = true;
> return 0;
> - }
>
> - /* Not having a signature is only an error if we're strict. */
> - if (err == -ENOKEY && !is_module_sig_enforced())
> - err = 0;
> + /* We don't permit modules to be loaded into trusted kernels
> + * without a valid signature on them, but if we're not
> + * enforcing, certain errors are non-fatal.
> + */
> + case -ENODATA:
> + reason = "Loading of unsigned module";
> + goto decide;
> + case -ENOPKG:
> + reason = "Loading of module with unsupported crypto";
> + goto decide;
> + case -ENOKEY:
> + reason = "Loading of module with unavailable key";
> + decide:
> + if (is_module_sig_enforced()) {
> + pr_notice("%s is rejected\n", reason);
> + return -EKEYREJECTED;
> + }
>
> - return err;
> + ret = security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> + return ret;
return security_locked_down(LOCKDOWN_MODULE_SIGNATURE); ? Means no need
to add "ret". Regardless:
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> +
> + /* All other errors are fatal, including nomem, unparseable
> + * signatures and signature check failures - even if signatures
> + * aren't required.
> + */
> + default:
> + return err;
> + }
> }
> #else /* !CONFIG_MODULE_SIG */
> static int module_sig_check(struct load_info *info, int flags)
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 8e39b36b8f33..25a3a5b0aa9c 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -18,6 +18,7 @@ static enum lockdown_reason kernel_locked_down;
>
> static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_NONE] = "none",
> + [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 05/29] Restrict /dev/{mem,kmem,port} when the kernel is locked down
From: Kees Cook @ 2019-06-22 23:52 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett, David Howells, Matthew Garrett, x86
In-Reply-To: <20190622000358.19895-6-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:34PM -0700, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> Allowing users to read and write to core kernel memory makes it possible
> for the kernel to be subverted, avoiding module loading restrictions, and
> also to steal cryptographic information.
>
> Disallow /dev/mem and /dev/kmem from being opened this when the kernel has
> been locked down to prevent this.
>
> Also disallow /dev/port from being opened to prevent raw ioport access and
> thus DMA from being used to accomplish the same thing.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: x86@kernel.org
> ---
> drivers/char/mem.c | 6 +++++-
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index b08dc50f9f26..93c02493f0fa 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -29,8 +29,8 @@
> #include <linux/export.h>
> #include <linux/io.h>
> #include <linux/uio.h>
> -
> #include <linux/uaccess.h>
> +#include <linux/security.h>
>
> #ifdef CONFIG_IA64
> # include <linux/efi.h>
> @@ -786,6 +786,10 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
>
> static int open_port(struct inode *inode, struct file *filp)
> {
> + int ret = security_locked_down(LOCKDOWN_DEV_MEM);
> +
> + if (ret)
> + return ret;
> return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
Usually the ordering for LSM tests tends to follow capable checks, which
allows for things like audit to generate logs for capability rejections,
etc. I'd expect this to be:
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
return security_locked_down(LOCKDOWN_DEV_MEM)
With that fixed:
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> }
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 46d85cd63b06..200175c8605a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -83,6 +83,7 @@ enum lsm_event {
> enum lockdown_reason {
> LOCKDOWN_NONE,
> LOCKDOWN_MODULE_SIGNATURE,
> + LOCKDOWN_DEV_MEM,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 25a3a5b0aa9c..565c87451f0f 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -19,6 +19,7 @@ static enum lockdown_reason kernel_locked_down;
> static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_NONE] = "none",
> [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> + [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 06/29] kexec_load: Disable at runtime if the kernel is locked down
From: Kees Cook @ 2019-06-22 23:52 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett, David Howells, Matthew Garrett, Dave Young,
kexec
In-Reply-To: <20190622000358.19895-7-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:35PM -0700, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> The kexec_load() syscall permits the loading and execution of arbitrary
> code in ring 0, which is something that lock-down is meant to prevent. It
> makes sense to disable kexec_load() in this situation.
>
> This does not affect kexec_file_load() syscall which can check for a
> signature on the image to be booted.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Acked-by: Dave Young <dyoung@redhat.com>
> cc: kexec@lists.infradead.org
> ---
> include/linux/security.h | 1 +
> kernel/kexec.c | 8 ++++++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 200175c8605a..00a31ab2e5ba 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -84,6 +84,7 @@ enum lockdown_reason {
> LOCKDOWN_NONE,
> LOCKDOWN_MODULE_SIGNATURE,
> LOCKDOWN_DEV_MEM,
> + LOCKDOWN_KEXEC,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 68559808fdfa..ec3f07a4b1c0 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -207,6 +207,14 @@ static inline int kexec_load_check(unsigned long nr_segments,
> if (result < 0)
> return result;
>
> + /*
> + * kexec can be used to circumvent module loading restrictions, so
> + * prevent loading in that case
> + */
> + result = security_locked_down(LOCKDOWN_KEXEC);
> + if (result)
> + return result;
> +
> /*
> * Verify we have a legal set of flags
> * This leaves us room for future extensions.
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 565c87451f0f..08fcd8116db3 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -20,6 +20,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_NONE] = "none",
> [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> + [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 07/29] Copy secure_boot flag in boot params across kexec reboot
From: Kees Cook @ 2019-06-22 23:53 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Dave Young, David Howells, Matthew Garrett, kexec
In-Reply-To: <20190622000358.19895-8-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:36PM -0700, Matthew Garrett wrote:
> From: Dave Young <dyoung@redhat.com>
>
> Kexec reboot in case secure boot being enabled does not keep the secure
> boot mode in new kernel, so later one can load unsigned kernel via legacy
> kexec_load. In this state, the system is missing the protections provided
> by secure boot.
>
> Adding a patch to fix this by retain the secure_boot flag in original
> kernel.
>
> secure_boot flag in boot_params is set in EFI stub, but kexec bypasses the
> stub. Fixing this issue by copying secure_boot flag across kexec reboot.
>
> Signed-off-by: Dave Young <dyoung@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: kexec@lists.infradead.org
> ---
> arch/x86/kernel/kexec-bzimage64.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 22f60dd26460..4243359ac509 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -182,6 +182,7 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
> if (efi_enabled(EFI_OLD_MEMMAP))
> return 0;
>
> + params->secure_boot = boot_params.secure_boot;
> ei->efi_loader_signature = current_ei->efi_loader_signature;
> ei->efi_systab = current_ei->efi_systab;
> ei->efi_systab_hi = current_ei->efi_systab_hi;
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 09/29] kexec_file: Restrict at runtime if the kernel is locked down
From: Kees Cook @ 2019-06-22 23:54 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Jiri Bohac, David Howells, Matthew Garrett, kexec
In-Reply-To: <20190622000358.19895-10-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:38PM -0700, Matthew Garrett wrote:
> From: Jiri Bohac <jbohac@suse.cz>
>
> When KEXEC_SIG is not enabled, kernel should not load images through
> kexec_file systemcall if the kernel is locked down.
>
> [Modified by David Howells to fit with modifications to the previous patch
> and to return -EPERM if the kernel is locked down for consistency with
> other lockdowns. Modified by Matthew Garrett to remove the IMA
> integration, which will be replaced by integrating with the IMA
> architecture policy patches.]
>
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Reviewed-by: Jiri Bohac <jbohac@suse.cz>
> cc: kexec@lists.infradead.org
> ---
> kernel/kexec_file.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index eec7e5bb2a08..27adb4312b03 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -237,7 +237,10 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> goto out;
> }
>
> - ret = 0;
> + ret = security_locked_down(LOCKDOWN_KEXEC);
> + if (ret)
> + goto out;
> +
> break;
>
> /* All other errors are fatal, including nomem, unparseable
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 10/29] hibernate: Disable when the kernel is locked down
From: Kees Cook @ 2019-06-22 23:55 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Josh Boyer, David Howells, Matthew Garrett, rjw, pavel, linux-pm
In-Reply-To: <20190622000358.19895-11-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:39PM -0700, Matthew Garrett wrote:
> From: Josh Boyer <jwboyer@fedoraproject.org>
>
> There is currently no way to verify the resume image when returning
> from hibernate. This might compromise the signed modules trust model,
> so until we can work with signed hibernate images we disable it when the
> kernel is locked down.
>
> Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: rjw@rjwysocki.net
> Cc: pavel@ucw.cz
> cc: linux-pm@vger.kernel.org
> ---
> include/linux/security.h | 1 +
> kernel/power/hibernate.c | 3 ++-
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 00a31ab2e5ba..a051f21a1144 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -85,6 +85,7 @@ enum lockdown_reason {
> LOCKDOWN_MODULE_SIGNATURE,
> LOCKDOWN_DEV_MEM,
> LOCKDOWN_KEXEC,
> + LOCKDOWN_HIBERNATION,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index abef759de7c8..3a9cb2d3da4a 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -32,6 +32,7 @@
> #include <linux/ctype.h>
> #include <linux/genhd.h>
> #include <linux/ktime.h>
> +#include <linux/security.h>
> #include <trace/events/power.h>
>
> #include "power.h"
> @@ -70,7 +71,7 @@ static const struct platform_hibernation_ops *hibernation_ops;
>
> bool hibernation_available(void)
> {
> - return (nohibernate == 0);
> + return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
> }
>
> /**
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 08fcd8116db3..ce5b3da9bd09 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -21,6 +21,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> + [LOCKDOWN_HIBERNATION] = "hibernation",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 11/29] PCI: Lock down BAR access when the kernel is locked down
From: Kees Cook @ 2019-06-22 23:55 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett, David Howells, Matthew Garrett, Bjorn Helgaas,
linux-pci
In-Reply-To: <20190622000358.19895-12-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:40PM -0700, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> Any hardware that can potentially generate DMA has to be locked down in
> order to avoid it being possible for an attacker to modify kernel code,
> allowing them to circumvent disabled module loading or module signing.
> Default to paranoid - in future we can potentially relax this for
> sufficiently IOMMU-isolated devices.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> cc: linux-pci@vger.kernel.org
> ---
> drivers/pci/pci-sysfs.c | 16 ++++++++++++++++
> drivers/pci/proc.c | 14 ++++++++++++--
> drivers/pci/syscall.c | 4 +++-
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 5 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 25794c27c7a4..e1011efb5a31 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -903,6 +903,11 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
> unsigned int size = count;
> loff_t init_off = off;
> u8 *data = (u8 *) buf;
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
>
> if (off > dev->cfg_size)
> return 0;
> @@ -1165,6 +1170,11 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
> int bar = (unsigned long)attr->private;
> enum pci_mmap_state mmap_type;
> struct resource *res = &pdev->resource[bar];
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
>
> if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
> return -EINVAL;
> @@ -1241,6 +1251,12 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
> struct bin_attribute *attr, char *buf,
> loff_t off, size_t count)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
> +
> return pci_resource_io(filp, kobj, attr, buf, off, count, true);
> }
>
> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index 6fa1627ce08d..a72258d70407 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -13,6 +13,7 @@
> #include <linux/seq_file.h>
> #include <linux/capability.h>
> #include <linux/uaccess.h>
> +#include <linux/security.h>
> #include <asm/byteorder.h>
> #include "pci.h"
>
> @@ -115,7 +116,11 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
> struct pci_dev *dev = PDE_DATA(ino);
> int pos = *ppos;
> int size = dev->cfg_size;
> - int cnt;
> + int cnt, ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
>
> if (pos >= size)
> return 0;
> @@ -196,6 +201,10 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
> #endif /* HAVE_PCI_MMAP */
> int ret = 0;
>
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
> +
> switch (cmd) {
> case PCIIOC_CONTROLLER:
> ret = pci_domain_nr(dev->bus);
> @@ -237,7 +246,8 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
> struct pci_filp_private *fpriv = file->private_data;
> int i, ret, write_combine = 0, res_bit = IORESOURCE_MEM;
>
> - if (!capable(CAP_SYS_RAWIO))
> + if (!capable(CAP_SYS_RAWIO) ||
> + security_locked_down(LOCKDOWN_PCI_ACCESS))
> return -EPERM;
>
> if (fpriv->mmap_state == pci_mmap_io) {
> diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
> index d96626c614f5..31e39558d49d 100644
> --- a/drivers/pci/syscall.c
> +++ b/drivers/pci/syscall.c
> @@ -7,6 +7,7 @@
>
> #include <linux/errno.h>
> #include <linux/pci.h>
> +#include <linux/security.h>
> #include <linux/syscalls.h>
> #include <linux/uaccess.h>
> #include "pci.h"
> @@ -90,7 +91,8 @@ SYSCALL_DEFINE5(pciconfig_write, unsigned long, bus, unsigned long, dfn,
> u32 dword;
> int err = 0;
>
> - if (!capable(CAP_SYS_ADMIN))
> + if (!capable(CAP_SYS_ADMIN) ||
> + security_locked_down(LOCKDOWN_PCI_ACCESS))
> return -EPERM;
>
> dev = pci_get_domain_bus_and_slot(0, bus, dfn);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index a051f21a1144..1b849f10dec6 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -86,6 +86,7 @@ enum lockdown_reason {
> LOCKDOWN_DEV_MEM,
> LOCKDOWN_KEXEC,
> LOCKDOWN_HIBERNATION,
> + LOCKDOWN_PCI_ACCESS,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index ce5b3da9bd09..e2ee8a16b94c 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -22,6 +22,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> [LOCKDOWN_HIBERNATION] = "hibernation",
> + [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 12/29] x86: Lock down IO port access when the kernel is locked down
From: Kees Cook @ 2019-06-22 23:58 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett, Matthew Garrett, David Howells, x86
In-Reply-To: <20190622000358.19895-13-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:41PM -0700, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> IO port access would permit users to gain access to PCI configuration
> registers, which in turn (on a lot of hardware) give access to MMIO
> register space. This would potentially permit root to trigger arbitrary
> DMA, so lock it down by default.
>
> This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
> KDDISABIO console ioctls.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: x86@kernel.org
> ---
> arch/x86/kernel/ioport.c | 7 +++++--
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
> index 0fe1c8782208..61a89d3c0382 100644
> --- a/arch/x86/kernel/ioport.c
> +++ b/arch/x86/kernel/ioport.c
> @@ -11,6 +11,7 @@
> #include <linux/errno.h>
> #include <linux/types.h>
> #include <linux/ioport.h>
> +#include <linux/security.h>
> #include <linux/smp.h>
> #include <linux/stddef.h>
> #include <linux/slab.h>
> @@ -31,7 +32,8 @@ long ksys_ioperm(unsigned long from, unsigned long num, int turn_on)
>
> if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
> return -EINVAL;
> - if (turn_on && !capable(CAP_SYS_RAWIO))
> + if (turn_on && (!capable(CAP_SYS_RAWIO) ||
> + security_locked_down(LOCKDOWN_IOPORT)))
> return -EPERM;
>
> /*
> @@ -126,7 +128,8 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
> return -EINVAL;
> /* Trying to gain more privileges? */
> if (level > old) {
> - if (!capable(CAP_SYS_RAWIO))
> + if (!capable(CAP_SYS_RAWIO) ||
> + security_locked_down(LOCKDOWN_IOPORT))
> return -EPERM;
> }
> regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1b849f10dec6..60569b7e9465 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -87,6 +87,7 @@ enum lockdown_reason {
> LOCKDOWN_KEXEC,
> LOCKDOWN_HIBERNATION,
> LOCKDOWN_PCI_ACCESS,
> + LOCKDOWN_IOPORT,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index e2ee8a16b94c..895ef3ba1b4c 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -23,6 +23,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> [LOCKDOWN_HIBERNATION] = "hibernation",
> [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
> + [LOCKDOWN_IOPORT] = "raw io port access",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 14/29] ACPI: Limit access to custom_method when the kernel is locked down
From: Kees Cook @ 2019-06-22 23:59 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Matthew Garrett, Matthew Garrett, David Howells, linux-acpi
In-Reply-To: <20190622000358.19895-15-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:43PM -0700, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> custom_method effectively allows arbitrary access to system memory, making
> it possible for an attacker to circumvent restrictions on module loading.
> Disable it if the kernel is locked down.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-acpi@vger.kernel.org
> ---
> drivers/acpi/custom_method.c | 6 ++++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 8 insertions(+)
>
> diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
> index aa972dc5cb7e..6e56f9f43492 100644
> --- a/drivers/acpi/custom_method.c
> +++ b/drivers/acpi/custom_method.c
> @@ -8,6 +8,7 @@
> #include <linux/uaccess.h>
> #include <linux/debugfs.h>
> #include <linux/acpi.h>
> +#include <linux/security.h>
>
> #include "internal.h"
>
> @@ -28,6 +29,11 @@ static ssize_t cm_write(struct file *file, const char __user * user_buf,
>
> struct acpi_table_header table;
> acpi_status status;
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
> + if (ret)
> + return ret;
>
> if (!(*ppos)) {
> /* parse the table header to get the table length */
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 30bc6f058926..cc2b5ee4cadd 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -89,6 +89,7 @@ enum lockdown_reason {
> LOCKDOWN_PCI_ACCESS,
> LOCKDOWN_IOPORT,
> LOCKDOWN_MSR,
> + LOCKDOWN_ACPI_TABLES,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 297a065e6261..1725224f0024 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -25,6 +25,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
> [LOCKDOWN_IOPORT] = "raw io port access",
> [LOCKDOWN_MSR] = "raw MSR access",
> + [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
From: Kees Cook @ 2019-06-22 23:59 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Josh Boyer, David Howells, Matthew Garrett, Dave Young,
linux-acpi
In-Reply-To: <20190622000358.19895-16-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:44PM -0700, Matthew Garrett wrote:
> From: Josh Boyer <jwboyer@redhat.com>
>
> This option allows userspace to pass the RSDP address to the kernel, which
> makes it possible for a user to modify the workings of hardware . Reject
> the option when the kernel is locked down.
>
> Signed-off-by: Josh Boyer <jwboyer@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: Dave Young <dyoung@redhat.com>
> cc: linux-acpi@vger.kernel.org
> ---
> drivers/acpi/osl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index f29e427d0d1d..60cda8a0f36b 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -40,6 +40,7 @@
> #include <linux/list.h>
> #include <linux/jiffies.h>
> #include <linux/semaphore.h>
> +#include <linux/security.h>
>
> #include <asm/io.h>
> #include <linux/uaccess.h>
> @@ -194,7 +195,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
> acpi_physical_address pa;
>
> #ifdef CONFIG_KEXEC
> - if (acpi_rsdp)
> + if (acpi_rsdp && !security_locked_down(LOCKDOWN_ACPI_TABLES))
> return acpi_rsdp;
> #endif
> pa = acpi_arch_get_root_pointer();
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 16/29] acpi: Disable ACPI table override if the kernel is locked down
From: Kees Cook @ 2019-06-23 0:00 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
Linn Crosetto, David Howells, Matthew Garrett, linux-acpi
In-Reply-To: <20190622000358.19895-17-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:45PM -0700, Matthew Garrett wrote:
> From: Linn Crosetto <linn@hpe.com>
>
> From the kernel documentation (initrd_table_override.txt):
>
> If the ACPI_INITRD_TABLE_OVERRIDE compile option is true, it is possible
> to override nearly any ACPI table provided by the BIOS with an
> instrumented, modified one.
>
> When lockdown is enabled, the kernel should disallow any unauthenticated
> changes to kernel space. ACPI tables contain code invoked by the kernel,
> so do not allow ACPI tables to be overridden if the kernel is locked down.
>
> Signed-off-by: Linn Crosetto <linn@hpe.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: linux-acpi@vger.kernel.org
> ---
> drivers/acpi/tables.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 8fccbe49612a..41d9ccd0e075 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -34,6 +34,7 @@
> #include <linux/memblock.h>
> #include <linux/earlycpio.h>
> #include <linux/initrd.h>
> +#include <linux/security.h>
> #include "internal.h"
>
> #ifdef CONFIG_ACPI_CUSTOM_DSDT
> @@ -539,6 +540,11 @@ void __init acpi_table_upgrade(void)
> if (table_nr == 0)
> return;
>
> + if (security_locked_down(LOCKDOWN_ACPI_TABLES)) {
> + pr_notice("kernel is locked down, ignoring table override\n");
> + return;
> + }
> +
> acpi_tables_addr =
> memblock_find_in_range(0, ACPI_TABLE_UPGRADE_MAX_PHYS,
> all_tables_size, PAGE_SIZE);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 17/29] Prohibit PCMCIA CIS storage when the kernel is locked down
From: Kees Cook @ 2019-06-23 0:00 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Dominik Brodowski, Matthew Garrett
In-Reply-To: <20190622000358.19895-18-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:46PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Prohibit replacement of the PCMCIA Card Information Structure when the
> kernel is locked down.
>
> Suggested-by: Dominik Brodowski <linux@dominikbrodowski.net>
> Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> drivers/pcmcia/cistpl.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
> index ac0672b8dfca..379c53610102 100644
> --- a/drivers/pcmcia/cistpl.c
> +++ b/drivers/pcmcia/cistpl.c
> @@ -24,6 +24,7 @@
> #include <linux/pci.h>
> #include <linux/ioport.h>
> #include <linux/io.h>
> +#include <linux/security.h>
> #include <asm/byteorder.h>
> #include <asm/unaligned.h>
>
> @@ -1578,6 +1579,10 @@ static ssize_t pccard_store_cis(struct file *filp, struct kobject *kobj,
> struct pcmcia_socket *s;
> int error;
>
> + error = security_locked_down(LOCKDOWN_PCMCIA_CIS);
> + if (error)
> + return error;
> +
> s = to_socket(container_of(kobj, struct device, kobj));
>
> if (off)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index cc2b5ee4cadd..03c125b277ca 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -90,6 +90,7 @@ enum lockdown_reason {
> LOCKDOWN_IOPORT,
> LOCKDOWN_MSR,
> LOCKDOWN_ACPI_TABLES,
> + LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 1725224f0024..7be3e8fb5847 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -26,6 +26,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_IOPORT] = "raw io port access",
> [LOCKDOWN_MSR] = "raw MSR access",
> [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> + [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 18/29] Lock down TIOCSSERIAL
From: Kees Cook @ 2019-06-23 0:01 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Greg Kroah-Hartman, Matthew Garrett, Jiri Slaby,
linux-serial
In-Reply-To: <20190622000358.19895-19-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:47PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Lock down TIOCSSERIAL as that can be used to change the ioport and irq
> settings on a serial port. This only appears to be an issue for the serial
> drivers that use the core serial code. All other drivers seem to either
> ignore attempts to change port/irq or give an error.
>
> Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: Jiri Slaby <jslaby@suse.com>
> Cc: linux-serial@vger.kernel.org
> ---
> drivers/tty/serial/serial_core.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 351843f847c0..a84f231a5df4 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -22,6 +22,7 @@
> #include <linux/serial_core.h>
> #include <linux/delay.h>
> #include <linux/mutex.h>
> +#include <linux/security.h>
>
> #include <linux/irq.h>
> #include <linux/uaccess.h>
> @@ -852,6 +853,10 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
> new_flags = (__force upf_t)new_info->flags;
> old_custom_divisor = uport->custom_divisor;
>
> + retval = security_locked_down(LOCKDOWN_TIOCSSERIAL);
> + if (retval && (change_port || change_irq))
> + goto exit;
> +
> if (!capable(CAP_SYS_ADMIN)) {
> retval = -EPERM;
> if (change_irq || change_port ||
This should be moved after the capable test. With that fixed:
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 03c125b277ca..61e3f4a62d16 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -91,6 +91,7 @@ enum lockdown_reason {
> LOCKDOWN_MSR,
> LOCKDOWN_ACPI_TABLES,
> LOCKDOWN_PCMCIA_CIS,
> + LOCKDOWN_TIOCSSERIAL,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 7be3e8fb5847..c89046dc2155 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -27,6 +27,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MSR] = "raw MSR access",
> [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> + [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 19/29] Lock down module params that specify hardware parameters (eg. ioport)
From: Kees Cook @ 2019-06-23 0:04 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Alan Cox, Matthew Garrett
In-Reply-To: <20190622000358.19895-20-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:48PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Provided an annotation for module parameters that specify hardware
> parameters (such as io ports, iomem addresses, irqs, dma channels, fixed
> dma buffers and other types).
>
> Suggested-by: Alan Cox <gnomes@lxorguk.ukuu.org.uk>
> Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> include/linux/security.h | 1 +
> kernel/params.c | 27 ++++++++++++++++++++++-----
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 61e3f4a62d16..88064d7f6827 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -92,6 +92,7 @@ enum lockdown_reason {
> LOCKDOWN_ACPI_TABLES,
> LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_TIOCSSERIAL,
> + LOCKDOWN_MODULE_PARAMETERS,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/params.c b/kernel/params.c
> index ce89f757e6da..f94fe79e331d 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -24,6 +24,7 @@
> #include <linux/err.h>
> #include <linux/slab.h>
> #include <linux/ctype.h>
> +#include <linux/security.h>
>
> #ifdef CONFIG_SYSFS
> /* Protects all built-in parameters, modules use their own param_lock */
> @@ -108,13 +109,19 @@ bool parameq(const char *a, const char *b)
> return parameqn(a, b, strlen(a)+1);
> }
>
> -static void param_check_unsafe(const struct kernel_param *kp)
> +static bool param_check_unsafe(const struct kernel_param *kp,
> + const char *doing)
> {
> if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
> pr_notice("Setting dangerous option %s - tainting kernel\n",
> kp->name);
> add_taint(TAINT_USER, LOCKDEP_STILL_OK);
> }
> +
> + if (kp->flags & KERNEL_PARAM_FL_HWPARAM &&
> + security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
> + return false;
> + return true;
> }
>
> static int parse_one(char *param,
> @@ -144,8 +151,10 @@ static int parse_one(char *param,
> pr_debug("handling %s with %p\n", param,
> params[i].ops->set);
> kernel_param_lock(params[i].mod);
> - param_check_unsafe(¶ms[i]);
> - err = params[i].ops->set(val, ¶ms[i]);
> + if (param_check_unsafe(¶ms[i], doing))
> + err = params[i].ops->set(val, ¶ms[i]);
> + else
> + err = -EPERM;
> kernel_param_unlock(params[i].mod);
> return err;
> }
> @@ -553,6 +562,12 @@ static ssize_t param_attr_show(struct module_attribute *mattr,
> return count;
> }
>
> +#ifdef CONFIG_MODULES
> +#define mod_name(mod) (mod)->name
> +#else
> +#define mod_name(mod) "unknown"
> +#endif
> +
> /* sysfs always hands a nul-terminated string in buf. We rely on that. */
> static ssize_t param_attr_store(struct module_attribute *mattr,
> struct module_kobject *mk,
> @@ -565,8 +580,10 @@ static ssize_t param_attr_store(struct module_attribute *mattr,
> return -EPERM;
>
> kernel_param_lock(mk->mod);
> - param_check_unsafe(attribute->param);
> - err = attribute->param->ops->set(buf, attribute->param);
> + if (param_check_unsafe(attribute->param, mod_name(mk->mod)))
> + err = attribute->param->ops->set(buf, attribute->param);
> + else
> + err = -EPERM;
> kernel_param_unlock(mk->mod);
> if (!err)
> return len;
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index c89046dc2155..d03c4c296af7 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -28,6 +28,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> + [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 20/29] x86/mmiotrace: Lock down the testmmiotrace module
From: Kees Cook @ 2019-06-23 0:04 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Thomas Gleixner, Matthew Garrett, Steven Rostedt,
Ingo Molnar, H. Peter Anvin, x86
In-Reply-To: <20190622000358.19895-21-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:49PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> The testmmiotrace module shouldn't be permitted when the kernel is locked
> down as it can be used to arbitrarily read and write MMIO space. This is
> a runtime check rather than buildtime in order to allow configurations
> where the same kernel may be run in both locked down or permissive modes
> depending on local policy.
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: David Howells <dhowells@redhat.com
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: Thomas Gleixner <tglx@linutronix.de>
> cc: Steven Rostedt <rostedt@goodmis.org>
> cc: Ingo Molnar <mingo@kernel.org>
> cc: "H. Peter Anvin" <hpa@zytor.com>
> cc: x86@kernel.org
> ---
> arch/x86/mm/testmmiotrace.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
> index f6ae6830b341..6b9486baa2e9 100644
> --- a/arch/x86/mm/testmmiotrace.c
> +++ b/arch/x86/mm/testmmiotrace.c
> @@ -7,6 +7,7 @@
> #include <linux/module.h>
> #include <linux/io.h>
> #include <linux/mmiotrace.h>
> +#include <linux/security.h>
>
> static unsigned long mmio_address;
> module_param_hw(mmio_address, ulong, iomem, 0);
> @@ -114,6 +115,10 @@ static void do_test_bulk_ioremapping(void)
> static int __init init(void)
> {
> unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
> + int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
> +
> + if (ret)
> + return ret;
>
> if (mmio_address == 0) {
> pr_err("you have to use the module argument mmio_address.\n");
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 88064d7f6827..c649cb91e762 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -93,6 +93,7 @@ enum lockdown_reason {
> LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_TIOCSSERIAL,
> LOCKDOWN_MODULE_PARAMETERS,
> + LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index d03c4c296af7..cd86ed9f4d4b 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -29,6 +29,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> + [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 21/29] Lock down /proc/kcore
From: Kees Cook @ 2019-06-23 0:05 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Matthew Garrett
In-Reply-To: <20190622000358.19895-22-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:50PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Disallow access to /proc/kcore when the kernel is locked down to prevent
> access to cryptographic data. This is limited to lockdown
> confidentiality mode and is still permitted in integrity mode.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
> fs/proc/kcore.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
> index d29d869abec1..4e95edb1e282 100644
> --- a/fs/proc/kcore.c
> +++ b/fs/proc/kcore.c
> @@ -31,6 +31,7 @@
> #include <linux/ioport.h>
> #include <linux/memory.h>
> #include <linux/sched/task.h>
> +#include <linux/security.h>
> #include <asm/sections.h>
> #include "internal.h"
>
> @@ -545,6 +546,10 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
>
> static int open_kcore(struct inode *inode, struct file *filp)
> {
> + int ret = security_locked_down(LOCKDOWN_KCORE);
> +
> + if (ret)
> + return ret;
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
Another capable() check ordering fix needed. With that:
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c649cb91e762..3875f6df2ecc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -95,6 +95,7 @@ enum lockdown_reason {
> LOCKDOWN_MODULE_PARAMETERS,
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> + LOCKDOWN_KCORE,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index cd86ed9f4d4b..4c9b324dfc55 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -31,6 +31,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> + [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 22/29] Lock down tracing and perf kprobes when in confidentiality mode
From: Kees Cook @ 2019-06-23 0:09 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Alexei Starovoitov, Matthew Garrett,
Naveen N . Rao, Anil S Keshavamurthy, davem, Masami Hiramatsu
In-Reply-To: <20190622000358.19895-23-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:51PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> Disallow the creation of perf and ftrace kprobes when the kernel is
> locked down in confidentiality mode by preventing their registration.
> This prevents kprobes from being used to access kernel memory to steal
> crypto data, but continues to allow the use of kprobes from signed
> modules.
>
> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Naveen N. Rao <naveen.n.rao@linux.ibm.com>
> Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
> Cc: davem@davemloft.net
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> include/linux/security.h | 1 +
> kernel/trace/trace_kprobe.c | 5 +++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3875f6df2ecc..e6e3e2403474 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -96,6 +96,7 @@ enum lockdown_reason {
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> + LOCKDOWN_KPROBES,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5d5129b05df7..5a76a0f79d48 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -11,6 +11,7 @@
> #include <linux/uaccess.h>
> #include <linux/rculist.h>
> #include <linux/error-injection.h>
> +#include <linux/security.h>
>
> #include "trace_dynevent.h"
> #include "trace_kprobe_selftest.h"
> @@ -415,6 +416,10 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> int i, ret;
>
> + ret = security_locked_down(LOCKDOWN_KPROBES);
> + if (ret)
> + return ret;
> +
> if (trace_probe_is_registered(&tk->tp))
> return -EINVAL;
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 4c9b324dfc55..5a08c17f224d 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -32,6 +32,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> + [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Kees Cook @ 2019-06-23 0:09 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, linux-api,
David Howells, Alexei Starovoitov, Matthew Garrett, netdev,
Chun-Yi Lee, Daniel Borkmann
In-Reply-To: <20190622000358.19895-24-matthewgarrett@google.com>
On Fri, Jun 21, 2019 at 05:03:52PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
>
> There are some bpf functions can be used to read kernel memory:
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> private keys in kernel memory (e.g. the hibernation image signing key) to
> be read by an eBPF program and kernel memory to be altered without
> restriction. Disable them if the kernel has been locked down in
> confidentiality mode.
>
> Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
-Kees
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> cc: netdev@vger.kernel.org
> cc: Chun-Yi Lee <jlee@suse.com>
> cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> ---
> include/linux/security.h | 1 +
> kernel/trace/bpf_trace.c | 20 +++++++++++++++++++-
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index e6e3e2403474..de0d37b1fe79 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -97,6 +97,7 @@ enum lockdown_reason {
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> + LOCKDOWN_BPF_READ,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..638f9b00a8df 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -137,6 +137,10 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> ret = probe_kernel_read(dst, unsafe_ptr, size);
> if (unlikely(ret < 0))
> memset(dst, 0, size);
> @@ -156,6 +160,12 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
> BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
> u32, size)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * Ensure we're in user context which is safe for the helper to
> * run. This helper has no business in a kthread.
> @@ -205,7 +215,11 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> int fmt_cnt = 0;
> u64 unsafe_addr;
> char buf[64];
> - int i;
> + int i, ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
>
> /*
> * bpf_check()->check_func_arg()->check_stack_boundary()
> @@ -534,6 +548,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * The strncpy_from_unsafe() call will likely not fill the entire
> * buffer, but that's okay in this circumstance as we're probing
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 5a08c17f224d..2eea2cc13117 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> + [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox