On Fri, Dec 01, 2023 at 05:11:00PM -0500, Josef Bacik wrote:
> We have fscrypt_file_open() which is meant to be called on files being
> opened so that their key is loaded when we start reading data from them.
>
> However for btrfs send we are opening the inode directly without a filp,
> so we need a different helper to make sure we can load the fscrypt
> context for the inode before reading its contents.
>
> We need a different helper as opposed to simply using
> fscrypt_has_permitted_context() directly because of '-o
> test_dummy_encryption', which allows for encrypted files to be created
> with !IS_ENCRYPTED set on the directory (the root directory in this
> case). fscrypt_file_open() already does the appropriate check where it
> simply doesn't call fscrypt_has_permitted_context() if the parent
> directory isn't marked with IS_ENCRYPTED in order to facilitate this
> invariant when using '-o test_dummy_encryption'.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> fs/crypto/hooks.c | 42 +++++++++++++++++++++++++++++++++++++++++
> include/linux/fscrypt.h | 8 ++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> index 52504dd478d3..a391a987c58f 100644
> --- a/fs/crypto/hooks.c
> +++ b/fs/crypto/hooks.c
> @@ -49,6 +49,48 @@ int fscrypt_file_open(struct inode *inode, struct file *filp)
> }
> EXPORT_SYMBOL_GPL(fscrypt_file_open);
>
> +/**
> + * fscrypt_inode_open() - prepare to open a possibly-encrypted regular file
> + * @dir: the directory that contains this inode
> + * @inode: the inode being opened
> + *
> + * Currently, an encrypted regular file can only be opened if its encryption key
> + * is available; access to the raw encrypted contents is not supported.
> + * Therefore, we first set up the inode's encryption key (if not already done)
> + * and return an error if it's unavailable.
> + *
> + * We also verify that if the parent directory is encrypted, then the inode
> + * being opened uses the same encryption policy. This is needed as part of the
> + * enforcement that all files in an encrypted directory tree use the same
> + * encryption policy, as a protection against certain types of offline attacks.
> + * Note that this check is needed even when opening an *unencrypted* file, since
> + * it's forbidden to have an unencrypted file in an encrypted directory.
> + *
> + * File systems should be using fscrypt_file_open in their open callback. This
> + * is for file systems that may need to open inodes outside of the normal file
> + * open path, btrfs send for example.
> + *
> + * Return: 0 on success, -ENOKEY if the key is missing, or another -errno code
> + */
> +int fscrypt_inode_open(struct inode *dir, struct inode *inode)
> +{
> + int err;
> +
> + err = fscrypt_require_key(inode);
> + if (err)
> + return err;
> +
> + if (IS_ENCRYPTED(dir) &&
> + !fscrypt_has_permitted_context(dir, inode)) {
> + fscrypt_warn(inode,
> + "Inconsistent encryption context (parent directory: %lu)",
> + dir->i_ino);
> + err = -EPERM;
> + }
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_inode_open);
The comment and code is heavily copy+pasted from fscrypt_file_open(), which is
not great. How about naming the new function __fscrypt_file_open(),
implementing fscrypt_file_open() on top of it, and making the comment describe
the differences vs. fscrypt_file_open()?
So fscrypt_file_open() would do:
struct inode *dir;
int err;
dir = dget_parent(file_dentry(filp));
err = __fscrypt_file_open(dir, filp);
dput(dir);
return err;
... and the comment for the new function would be something like:
/**
* __fscrypt_file_open() - prepare for filesystem-internal access to a
* possibly-encrypted regular file
* @dir: the inode for the directory via which the file is being accessed
* @inode: the inode being "opened"
*
* This is like fscrypt_file_open(), but instead of taking the 'struct file'
* being opened it takes the parent directory explicitly. This is intended for
* use cases such as "send/receive" which involve the filesystem accessing file
* contents without setting up a 'struct file'.
*
* Return: 0 on success, -ENOKEY if the key is missing, or another -errno code
*/
int __fscrypt_file_open(struct inode *dir, struct inode *inode)
Note, we need to be careful when describing "@dir". It's not simply "the
directory that contains this inode". An inode can be in multiple directories.
- Eric
