[PATCH v3 0/3] ext4 crypto: back up encrypted files

[PATCH v3 0/3] ext4 crypto: back up encrypted files

[Theodore Ts'o]

This patches allow backing up encrypted files without having access to
the key.  Unfortunately, the key *is* necessary to restore the files,
because establishing a link means that we have to manipulate the both
the encrypted directory and the encrypted file, and doing this through
the VFS interface is non-trivial.  So we have an ioctl which extracts
the encrypted file name, and that in combination with the encryption
metadata for the directory should be sufficient to restore the file
name assuming the restore is done with access to the user's master
key.

The other tricky bit is that if the file's i_size is not a multiple of
the AES block size, we need to be able to copy a handful of bytes
before i_size --- and O_DIRECT reads don't allow that.  There are two
ways of solving this.  One would be an new DIO_FLAG that rounds i_size
up to the file system blocksize, which we would pass when reading
encrypted files using O_DIRECT.  This would require changes to the
core direct I/O, and may be controversial.  It also may make it more
difficult to back port these patches to ancient BSP kernels.

So what we're doing for now is admittedly a hack.  Since encrypted
files are read-only without access to the key, it is safe to create a
shadow copy of the inode structure, and round up i_size in the shadow
structure.  We only do this when reading the last block in the file,
so the overhead shouldn't be too bad.

The process doing the store will need to truncate the file back down
to the original file when it has access to the key.  We can't do this
without the key because the kernel zero fills the block between i_size
and the end of the block.  (This is also why it's not a security issue
to round i_size up to the end of the block; there is no chance we will
be revealing stale data.)  So this means that for the purposes of
doing the encrypted backup, the backup will need to store the
encrypted file name, the directory's encrypted metadata, and the
original i_size in some convenient ouf-of-band backup metadata store.

Theodore Ts'o (3):
  ext4 crypto: add ciphertext_access mount option
  ext4 crypto: add ioctls to allow backup of encryption metadata
  ext4 crypto: add missing locking for keyring_key access

 fs/ext4/crypto_key.c  | 61 +++++++++++++++++++++++++++++++++++++++++++++++-
 fs/ext4/ext4.h        | 12 ++++++++++
 fs/ext4/ext4_crypto.h |  8 +++++++
 fs/ext4/file.c        |  5 +++-
 fs/ext4/indirect.c    | 24 +++++++++++++++----
 fs/ext4/inode.c       | 17 ++++++++------
 fs/ext4/ioctl.c       | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ext4/namei.c       | 24 +++++++++++++++++++
 fs/ext4/super.c       | 48 ++++++++++++++++++++++++++++++++++++++
 9 files changed, 249 insertions(+), 14 deletions(-)

-- 
2.5.0

[PATCH v3 1/3] ext4 crypto: add ciphertext_access mount option

[Theodore Ts'o]

Add a mount option which allows root to be able to access the
ciphertext of a file by reading it using O_DIRECT.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/ext4.h     |  3 +++
 fs/ext4/file.c     |  5 ++++-
 fs/ext4/indirect.c | 24 +++++++++++++++++++-----
 fs/ext4/inode.c    | 17 ++++++++++-------
 fs/ext4/super.c    | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 84 insertions(+), 13 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 1e20fa9..cf7a885 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1052,6 +1052,7 @@ struct ext4_inode_info {
 #define EXT4_MOUNT_DIOREAD_NOLOCK	0x400000 /* Enable support for dio read nolocking */
 #define EXT4_MOUNT_JOURNAL_CHECKSUM	0x800000 /* Journal checksums */
 #define EXT4_MOUNT_JOURNAL_ASYNC_COMMIT	0x1000000 /* Journal Async Commit */
+#define EXT4_MOUNT_CIPHERTEXT_ACCESS	0x2000000 /* Direct I/O to ciphertext */
 #define EXT4_MOUNT_DELALLOC		0x8000000 /* Delalloc support */
 #define EXT4_MOUNT_DATA_ERR_ABORT	0x10000000 /* Abort on file data write */
 #define EXT4_MOUNT_BLOCK_VALIDITY	0x20000000 /* Block validity checking */
@@ -2564,6 +2565,8 @@ extern int ext4_alloc_flex_bg_array(struct super_block *sb,
 				    ext4_group_t ngroup);
 extern const char *ext4_decode_error(struct super_block *sb, int errno,
 				     char nbuf[16]);
+extern struct inode *ext4_alloc_shadow_inode(struct inode *inode);
+extern void ext4_free_shadow_inode(struct inode *shadow);
 
 extern __printf(4, 5)
 void __ext4_error(struct super_block *, const char *, unsigned int,
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 749b222..60683ab 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -388,7 +388,10 @@ static int ext4_file_open(struct inode * inode, struct file * filp)
 		ret = ext4_get_encryption_info(inode);
 		if (ret)
 			return -EACCES;
-		if (ext4_encryption_info(inode) == NULL)
+		if ((ext4_encryption_info(inode) == NULL) &&
+		    !(test_opt(inode->i_sb, CIPHERTEXT_ACCESS) &&
+		      ((filp->f_flags & O_ACCMODE) == O_RDONLY) &&
+		      capable(CAP_SYS_ADMIN)))
 			return -ENOKEY;
 	}
 	/*
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 355ef9c..e685736 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -649,17 +649,17 @@ ssize_t ext4_ind_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file->f_mapping->host;
+	struct inode *shadow = inode;
 	struct ext4_inode_info *ei = EXT4_I(inode);
 	handle_t *handle;
 	ssize_t ret;
 	int orphan = 0;
 	size_t count = iov_iter_count(iter);
 	int retries = 0;
+	loff_t final_size = offset + count;
 
 	if (iov_iter_rw(iter) == WRITE) {
-		loff_t final_size = offset + count;
-
-		if (final_size > inode->i_size) {
+		if (final_size > i_size_read(inode)) {
 			/* Credits for sb + inode write */
 			handle = ext4_journal_start(inode, EXT4_HT_INODE, 2);
 			if (IS_ERR(handle)) {
@@ -676,6 +676,18 @@ ssize_t ext4_ind_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 			ext4_journal_stop(handle);
 		}
 	}
+	if (iov_iter_rw(iter) == READ &&
+	    ext4_encrypted_inode(inode) &&
+	    is_sync_kiocb(iocb) &&
+	    final_size > i_size_read(inode)) {
+		shadow = ext4_alloc_shadow_inode(inode);
+		if (shadow)
+			i_size_write(shadow,
+				     round_up(i_size_read(inode),
+					      inode->i_sb->s_blocksize));
+		else
+			shadow = inode;
+	}
 
 retry:
 	if (iov_iter_rw(iter) == READ && ext4_should_dioread_nolock(inode)) {
@@ -695,7 +707,7 @@ retry:
 			ret = dax_do_io(iocb, inode, iter, offset,
 					ext4_get_block, NULL, 0);
 		else
-			ret = __blockdev_direct_IO(iocb, inode,
+			ret = __blockdev_direct_IO(iocb, shadow,
 						   inode->i_sb->s_bdev, iter,
 						   offset, ext4_get_block, NULL,
 						   NULL, 0);
@@ -706,7 +718,7 @@ locked:
 			ret = dax_do_io(iocb, inode, iter, offset,
 					ext4_get_block, NULL, DIO_LOCKING);
 		else
-			ret = blockdev_direct_IO(iocb, inode, iter, offset,
+			ret = blockdev_direct_IO(iocb, shadow, iter, offset,
 						 ext4_get_block);
 
 		if (unlikely(iov_iter_rw(iter) == WRITE && ret < 0)) {
@@ -757,6 +769,8 @@ locked:
 			ret = err;
 	}
 out:
+	if (shadow != inode)
+		ext4_free_shadow_inode(shadow);
 	return ret;
 }
 
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ff2f3cd..16f6537 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3279,9 +3279,6 @@ static ssize_t ext4_ext_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 		get_block_func = ext4_get_block_write;
 		dio_flags = DIO_LOCKING;
 	}
-#ifdef CONFIG_EXT4_FS_ENCRYPTION
-	BUG_ON(ext4_encrypted_inode(inode) && S_ISREG(inode->i_mode));
-#endif
 	if (IS_DAX(inode))
 		ret = dax_do_io(iocb, inode, iter, offset, get_block_func,
 				ext4_end_io_dio, dio_flags);
@@ -3344,10 +3341,16 @@ static ssize_t ext4_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 	size_t count = iov_iter_count(iter);
 	ssize_t ret;
 
-#ifdef CONFIG_EXT4_FS_ENCRYPTION
-	if (ext4_encrypted_inode(inode) && S_ISREG(inode->i_mode))
-		return 0;
-#endif
+	if (ext4_encrypted_inode(inode) && S_ISREG(inode->i_mode)) {
+		if (iov_iter_rw(iter) == WRITE)
+			return 0;
+		if (test_opt(inode->i_sb, CIPHERTEXT_ACCESS) &&
+		    capable(CAP_SYS_ADMIN)) {
+			if (iov_iter_rw(iter) == WRITE)
+				return -EPERM;
+		} else
+			return 0;
+	}
 
 	/*
 	 * If we are doing data journalling we don't support O_DIRECT
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 486e869..6173b46 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1033,6 +1033,49 @@ void ext4_clear_inode(struct inode *inode)
 #endif
 }
 
+/*
+ * Create a copy of the inode structure so when we are reading the
+ * last block of an encrypted inode using direct I/O to get the
+ * ciphertext, we can futz with the i_size in the shadow inode.  This
+ * is necessary so that we can make a copy of the full AES block when
+ * i_size is not a multiple of the AES block size.
+ */
+struct inode *ext4_alloc_shadow_inode(struct inode *inode)
+{
+	struct ext4_inode_info *shadow_ei, *ei = EXT4_I(inode);
+	struct inode *shadow;
+
+	shadow_ei = kmem_cache_alloc(ext4_inode_cachep, GFP_NOFS);
+	if (!shadow_ei)
+		return NULL;
+
+	memcpy(shadow_ei, ei, sizeof(struct ext4_inode_info));
+	shadow = &shadow_ei->vfs_inode;
+
+	init_rwsem(&shadow_ei->xattr_sem);
+	init_rwsem(&shadow_ei->i_data_sem);
+	init_rwsem(&shadow_ei->i_mmap_sem);
+	i_size_ordered_init(shadow);
+	mutex_init(&shadow->i_mutex);
+	spin_lock_init(&shadow_ei->i_raw_lock);
+	spin_lock_init(&shadow_ei->i_prealloc_lock);
+	spin_lock_init(&(shadow_ei->i_block_reservation_lock));
+	spin_lock_init(&shadow_ei->i_completed_io_lock);
+	rwlock_init(&shadow_ei->i_es_lock);
+	ext4_es_init_tree(&shadow_ei->i_es_tree);
+	INIT_LIST_HEAD(&shadow_ei->i_es_list);
+	shadow_ei->i_es_all_nr = 0;
+	shadow_ei->i_es_shk_nr = 0;
+
+	return shadow;
+}
+
+void ext4_free_shadow_inode(struct inode *shadow)
+{
+	ext4_es_remove_extent(shadow, 0, EXT_MAX_BLOCKS);
+	kmem_cache_free(ext4_inode_cachep, EXT4_I(shadow));
+}
+
 static struct inode *ext4_nfs_get_inode(struct super_block *sb,
 					u64 ino, u32 generation)
 {
@@ -1182,6 +1225,7 @@ enum {
 	Opt_journal_path, Opt_journal_checksum, Opt_journal_async_commit,
 	Opt_abort, Opt_data_journal, Opt_data_ordered, Opt_data_writeback,
 	Opt_data_err_abort, Opt_data_err_ignore, Opt_test_dummy_encryption,
+	Opt_ciphertext_access, Opt_nociphertext_access,
 	Opt_usrjquota, Opt_grpjquota, Opt_offusrjquota, Opt_offgrpjquota,
 	Opt_jqfmt_vfsold, Opt_jqfmt_vfsv0, Opt_jqfmt_vfsv1, Opt_quota,
 	Opt_noquota, Opt_barrier, Opt_nobarrier, Opt_err,
@@ -1273,6 +1317,8 @@ static const match_table_t tokens = {
 	{Opt_noinit_itable, "noinit_itable"},
 	{Opt_max_dir_size_kb, "max_dir_size_kb=%u"},
 	{Opt_test_dummy_encryption, "test_dummy_encryption"},
+	{Opt_ciphertext_access, "ciphertext_access"},
+	{Opt_nociphertext_access, "nociphertext_access"},
 	{Opt_removed, "check=none"},	/* mount option from ext2/3 */
 	{Opt_removed, "nocheck"},	/* mount option from ext2/3 */
 	{Opt_removed, "reservation"},	/* mount option from ext2/3 */
@@ -1475,6 +1521,8 @@ static const struct mount_opts {
 	{Opt_jqfmt_vfsv1, QFMT_VFS_V1, MOPT_QFMT},
 	{Opt_max_dir_size_kb, 0, MOPT_GTE0},
 	{Opt_test_dummy_encryption, 0, MOPT_GTE0},
+	{Opt_ciphertext_access, EXT4_MOUNT_CIPHERTEXT_ACCESS, MOPT_SET},
+	{Opt_nociphertext_access, EXT4_MOUNT_CIPHERTEXT_ACCESS, MOPT_CLEAR},
 	{Opt_err, 0, 0}
 };
 
-- 
2.5.0

[PATCH v3 2/3] ext4 crypto: add ioctls to allow backup of encryption metadata

[Theodore Ts'o]

Add new ioctls which allow for the metadata of encrypted files (the
filename and the crypto policy) to be backed up.  We can restore the
crypto policy, but for now we can't restore the encrypted filename
because messing with encrypted directories directly while bypassing
the VFS would get fairly tricky/nasty.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/crypto_key.c  | 57 ++++++++++++++++++++++++++++++++++++++++++++-
 fs/ext4/ext4.h        |  9 ++++++++
 fs/ext4/ext4_crypto.h |  8 +++++++
 fs/ext4/ioctl.c       | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ext4/namei.c       | 24 +++++++++++++++++++
 5 files changed, 161 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
index c5882b3..039b22d 100644
--- a/fs/ext4/crypto_key.c
+++ b/fs/ext4/crypto_key.c
@@ -14,7 +14,7 @@
 #include <linux/scatterlist.h>
 #include <uapi/linux/keyctl.h>
 
-#include "ext4.h"
+#include "ext4_jbd2.h"
 #include "xattr.h"
 
 static void derive_crypt_complete(struct crypto_async_request *req, int rc)
@@ -270,3 +270,58 @@ int ext4_has_encryption_key(struct inode *inode)
 
 	return (ei->i_crypt_info != NULL);
 }
+
+int ext4_get_encryption_metadata(struct inode *inode,
+				 struct ext4_encrypted_metadata *mdata)
+{
+	int res;
+
+	if (mdata->len < sizeof(struct ext4_encryption_context))
+		return -EINVAL;
+
+	res = ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
+			     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
+			     &mdata->metadata, mdata->len);
+	if (res < 0)
+		return res;
+	mdata->len = res;
+	return 0;
+}
+
+int ext4_set_encryption_metadata(struct inode *inode,
+				 struct ext4_encrypted_metadata *mdata)
+{
+	struct ext4_encryption_context *ctx;
+	handle_t *handle;
+	int res;
+
+	if (mdata->len != sizeof(struct ext4_encryption_context))
+		return -EINVAL;
+	ctx = (struct ext4_encryption_context *) &mdata->metadata;
+	if (ctx->format != EXT4_ENCRYPTION_CONTEXT_FORMAT_V1)
+		return -EINVAL;
+
+	res = ext4_convert_inline_data(inode);
+	if (res)
+		return res;
+
+	handle = ext4_journal_start(inode, EXT4_HT_MISC,
+				    ext4_jbd2_credits_xattr(inode));
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	res = ext4_xattr_set(inode, EXT4_XATTR_INDEX_ENCRYPTION,
+			     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx,
+			     sizeof(struct ext4_encryption_context), 0);
+	if (res < 0)
+		goto errout;
+	ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
+	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
+	res = ext4_mark_inode_dirty(handle, inode);
+	if (res)
+		EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
+	else
+		res = ext4_get_encryption_info(inode);
+errout:
+	ext4_journal_stop(handle);
+	return res;
+}
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index cf7a885..c569430 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -617,6 +617,9 @@ enum {
 #define EXT4_IOC_SET_ENCRYPTION_POLICY	_IOR('f', 19, struct ext4_encryption_policy)
 #define EXT4_IOC_GET_ENCRYPTION_PWSALT	_IOW('f', 20, __u8[16])
 #define EXT4_IOC_GET_ENCRYPTION_POLICY	_IOW('f', 21, struct ext4_encryption_policy)
+#define EXT4_IOC_GET_ENCRYPTION_METADATA _IOWR('f', 22, struct ext4_encrypted_metadata)
+#define EXT4_IOC_SET_ENCRYPTION_METADATA _IOR('f', 23, struct ext4_encrypted_metadata)
+#define EXT4_IOC_GET_ENCRYPTED_FILENAME	_IOWR('f', 24, struct ext4_encrypted_metadata)
 
 #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
 /*
@@ -2311,6 +2314,10 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname) { }
 void ext4_free_crypt_info(struct ext4_crypt_info *ci);
 void ext4_free_encryption_info(struct inode *inode, struct ext4_crypt_info *ci);
 int _ext4_get_encryption_info(struct inode *inode);
+int ext4_set_encryption_metadata(struct inode *inode,
+				 struct ext4_encrypted_metadata *mdata);
+int ext4_get_encryption_metadata(struct inode *inode,
+				 struct ext4_encrypted_metadata *mdata);
 
 #ifdef CONFIG_EXT4_FS_ENCRYPTION
 int ext4_has_encryption_key(struct inode *inode);
@@ -2546,6 +2553,8 @@ extern int ext4_generic_delete_entry(handle_t *handle,
 				     int buf_size,
 				     int csum_size);
 extern int ext4_empty_dir(struct inode *inode);
+extern int ext4_get_encrypted_filename(struct file *filp,
+				       struct ext4_encrypted_metadata *mdata);
 
 /* resize.c */
 extern int ext4_group_add(struct super_block *sb,
diff --git a/fs/ext4/ext4_crypto.h b/fs/ext4/ext4_crypto.h
index ac7d4e8..f267cd3 100644
--- a/fs/ext4/ext4_crypto.h
+++ b/fs/ext4/ext4_crypto.h
@@ -156,4 +156,12 @@ static inline u32 encrypted_symlink_data_len(u32 l)
 	return (l + sizeof(struct ext4_encrypted_symlink_data) - 1);
 }
 
+/**
+ * Structure used for communicating encrypted metadata with userspace
+ */
+struct ext4_encrypted_metadata {
+	u32 len;
+	char metadata[288];
+};
+
 #endif	/* _EXT4_CRYPTO_H */
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 5e872fd..afb51f5 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -689,6 +689,67 @@ encryption_policy_out:
 		return -EOPNOTSUPP;
 #endif
 	}
+	case EXT4_IOC_GET_ENCRYPTION_METADATA: {
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
+		struct ext4_encrypted_metadata mdata;
+		int err = 0;
+
+		if (get_user(mdata.len, (u32 __user *) arg))
+			return -EFAULT;
+		if (mdata.len > sizeof(mdata.metadata))
+			return -EINVAL;
+
+		if (!ext4_encrypted_inode(inode))
+			return -ENOENT;
+		err = ext4_get_encryption_metadata(inode, &mdata);
+		if (err)
+			return err;
+		if (copy_to_user((void __user *)arg, &mdata, sizeof(mdata)))
+			return -EFAULT;
+		return 0;
+#else
+		return -EOPNOTSUPP;
+#endif
+	}
+	case EXT4_IOC_SET_ENCRYPTION_METADATA: {
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
+		struct ext4_encrypted_metadata mdata;
+		int err = 0;
+
+		if (ext4_encrypted_inode(inode))
+			return -EINVAL;
+		if (copy_from_user(&mdata,
+				   (struct ext4_encrypted_metadata __user *)arg,
+				   sizeof(mdata)))
+			return -EFAULT;
+		err = ext4_set_encryption_metadata(inode, &mdata);
+		return err;
+#else
+		return -EOPNOTSUPP;
+#endif
+	}
+	case EXT4_IOC_GET_ENCRYPTED_FILENAME: {
+#ifdef CONFIG_EXT4_FS_ENCRYPTION
+		struct ext4_encrypted_metadata mdata;
+		int err = 0;
+
+		if (get_user(mdata.len, (u32 __user *) arg))
+			return -EFAULT;
+		if (mdata.len > sizeof(mdata.metadata))
+			return -EINVAL;
+
+		if (!ext4_encrypted_inode(inode))
+			return -ENOENT;
+		err = ext4_get_encrypted_filename(filp, &mdata);
+		if (err)
+			return err;
+		if (copy_to_user((void __user *)arg, &mdata, sizeof(mdata)))
+			return -EFAULT;
+		return 0;
+#else
+		return -EOPNOTSUPP;
+#endif
+	}
 	default:
 		return -ENOTTY;
 	}
@@ -755,6 +816,9 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case EXT4_IOC_SET_ENCRYPTION_POLICY:
 	case EXT4_IOC_GET_ENCRYPTION_PWSALT:
 	case EXT4_IOC_GET_ENCRYPTION_POLICY:
+	case EXT4_IOC_GET_ENCRYPTION_METADATA:
+	case EXT4_IOC_SET_ENCRYPTION_METADATA:
+	case EXT4_IOC_GET_ENCRYPTED_FILENAME:
 		break;
 	default:
 		return -ENOIOCTLCMD;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a969ab3..1cc4eef 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -3861,3 +3861,27 @@ const struct inode_operations ext4_special_inode_operations = {
 	.get_acl	= ext4_get_acl,
 	.set_acl	= ext4_set_acl,
 };
+
+int ext4_get_encrypted_filename(struct file *filp,
+				struct ext4_encrypted_metadata *mdata)
+{
+	struct dentry *dentry = filp->f_path.dentry;
+	struct inode *dir = dentry->d_parent->d_inode;
+	struct buffer_head *bh;
+	struct ext4_dir_entry_2 *de;
+
+	if (!dir || !ext4_encrypted_inode(dir))
+		return -EINVAL;
+
+	bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
+	if (IS_ERR(bh))
+		return PTR_ERR(bh);
+	if (de == NULL)
+		return -ENOENT;
+
+	if (mdata->len < de->name_len)
+		return -ENOSPC;
+	mdata->len = de->name_len;
+	memcpy(mdata->metadata, de->name, de->name_len);
+	return 0;
+}
-- 
2.5.0

[PATCH v3 3/3] ext4 crypto: add missing locking for keyring_key access

[Theodore Ts'o]

Cc: stable@kernel.org
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/crypto_key.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
index 039b22d..f96a57a 100644
--- a/fs/ext4/crypto_key.c
+++ b/fs/ext4/crypto_key.c
@@ -213,9 +213,11 @@ retry:
 		res = -ENOKEY;
 		goto out;
 	}
+	down_read(&keyring_key->sem);
 	ukp = user_key_payload(keyring_key);
 	if (ukp->datalen != sizeof(struct ext4_encryption_key)) {
 		res = -EINVAL;
+		up_read(&keyring_key->sem);
 		goto out;
 	}
 	master_key = (struct ext4_encryption_key *)ukp->data;
@@ -226,10 +228,12 @@ retry:
 			    "ext4: key size incorrect: %d\n",
 			    master_key->size);
 		res = -ENOKEY;
+		up_read(&keyring_key->sem);
 		goto out;
 	}
 	res = ext4_derive_key_aes(ctx.nonce, master_key->raw,
 				  raw_key);
+	up_read(&keyring_key->sem);
 	if (res)
 		goto out;
 got_key:
-- 
2.5.0

Re: [PATCH v3 2/3] ext4 crypto: add ioctls to allow backup of encryption metadata

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 9657 bytes --]

On Dec 10, 2015, at 8:04 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> Add new ioctls which allow for the metadata of encrypted files (the
> filename and the crypto policy) to be backed up.  We can restore the
> crypto policy, but for now we can't restore the encrypted filename
> because messing with encrypted directories directly while bypassing
> the VFS would get fairly tricky/nasty.

Instead of exposing these via ioctls, why not use virtual xattrs
(e.g. "trusted.crypt_meta" or similar) so that this can be backed
up and restored using a normal backup tool instead of basically
nothing that exists today.

For reading the ciphertext, do existing tools such as tar and rsync
allow reading with O_DIRECT with CIPHERTEXT_ACCESS?  If not, one might
consider to allow normal reads w/o the file key to return the
ciphertext so that "tar --xattr" by some process with CAP_SYS_ADMIN
but out any keys could be used to backup and restore the ciphertext
in a sensible manner.  That is no more risk than the same process
reading the blocks from the disk device directly, but would be a lot
more useful (i.e. it is practical to use normal backup tools for
encrypted files).

Cheers, Andreas

> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
> fs/ext4/crypto_key.c  | 57 ++++++++++++++++++++++++++++++++++++++++++++-
> fs/ext4/ext4.h        |  9 ++++++++
> fs/ext4/ext4_crypto.h |  8 +++++++
> fs/ext4/ioctl.c       | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/ext4/namei.c       | 24 +++++++++++++++++++
> 5 files changed, 161 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
> index c5882b3..039b22d 100644
> --- a/fs/ext4/crypto_key.c
> +++ b/fs/ext4/crypto_key.c
> @@ -14,7 +14,7 @@
> #include <linux/scatterlist.h>
> #include <uapi/linux/keyctl.h>
> 
> -#include "ext4.h"
> +#include "ext4_jbd2.h"
> #include "xattr.h"
> 
> static void derive_crypt_complete(struct crypto_async_request *req, int rc)
> @@ -270,3 +270,58 @@ int ext4_has_encryption_key(struct inode *inode)
> 
> 	return (ei->i_crypt_info != NULL);
> }
> +
> +int ext4_get_encryption_metadata(struct inode *inode,
> +				 struct ext4_encrypted_metadata *mdata)
> +{
> +	int res;
> +
> +	if (mdata->len < sizeof(struct ext4_encryption_context))
> +		return -EINVAL;
> +
> +	res = ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
> +			     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
> +			     &mdata->metadata, mdata->len);
> +	if (res < 0)
> +		return res;
> +	mdata->len = res;
> +	return 0;
> +}
> +
> +int ext4_set_encryption_metadata(struct inode *inode,
> +				 struct ext4_encrypted_metadata *mdata)
> +{
> +	struct ext4_encryption_context *ctx;
> +	handle_t *handle;
> +	int res;
> +
> +	if (mdata->len != sizeof(struct ext4_encryption_context))
> +		return -EINVAL;
> +	ctx = (struct ext4_encryption_context *) &mdata->metadata;
> +	if (ctx->format != EXT4_ENCRYPTION_CONTEXT_FORMAT_V1)
> +		return -EINVAL;
> +
> +	res = ext4_convert_inline_data(inode);
> +	if (res)
> +		return res;
> +
> +	handle = ext4_journal_start(inode, EXT4_HT_MISC,
> +				    ext4_jbd2_credits_xattr(inode));
> +	if (IS_ERR(handle))
> +		return PTR_ERR(handle);
> +	res = ext4_xattr_set(inode, EXT4_XATTR_INDEX_ENCRYPTION,
> +			     EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx,
> +			     sizeof(struct ext4_encryption_context), 0);
> +	if (res < 0)
> +		goto errout;
> +	ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
> +	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
> +	res = ext4_mark_inode_dirty(handle, inode);
> +	if (res)
> +		EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
> +	else
> +		res = ext4_get_encryption_info(inode);
> +errout:
> +	ext4_journal_stop(handle);
> +	return res;
> +}
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index cf7a885..c569430 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -617,6 +617,9 @@ enum {
> #define EXT4_IOC_SET_ENCRYPTION_POLICY	_IOR('f', 19, struct ext4_encryption_policy)
> #define EXT4_IOC_GET_ENCRYPTION_PWSALT	_IOW('f', 20, __u8[16])
> #define EXT4_IOC_GET_ENCRYPTION_POLICY	_IOW('f', 21, struct ext4_encryption_policy)
> +#define EXT4_IOC_GET_ENCRYPTION_METADATA _IOWR('f', 22, struct ext4_encrypted_metadata)
> +#define EXT4_IOC_SET_ENCRYPTION_METADATA _IOR('f', 23, struct ext4_encrypted_metadata)
> +#define EXT4_IOC_GET_ENCRYPTED_FILENAME	_IOWR('f', 24, struct ext4_encrypted_metadata)
> 
> #if defined(__KERNEL__) && defined(CONFIG_COMPAT)
> /*
> @@ -2311,6 +2314,10 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname) { }
> void ext4_free_crypt_info(struct ext4_crypt_info *ci);
> void ext4_free_encryption_info(struct inode *inode, struct ext4_crypt_info *ci);
> int _ext4_get_encryption_info(struct inode *inode);
> +int ext4_set_encryption_metadata(struct inode *inode,
> +				 struct ext4_encrypted_metadata *mdata);
> +int ext4_get_encryption_metadata(struct inode *inode,
> +				 struct ext4_encrypted_metadata *mdata);
> 
> #ifdef CONFIG_EXT4_FS_ENCRYPTION
> int ext4_has_encryption_key(struct inode *inode);
> @@ -2546,6 +2553,8 @@ extern int ext4_generic_delete_entry(handle_t *handle,
> 				     int buf_size,
> 				     int csum_size);
> extern int ext4_empty_dir(struct inode *inode);
> +extern int ext4_get_encrypted_filename(struct file *filp,
> +				       struct ext4_encrypted_metadata *mdata);
> 
> /* resize.c */
> extern int ext4_group_add(struct super_block *sb,
> diff --git a/fs/ext4/ext4_crypto.h b/fs/ext4/ext4_crypto.h
> index ac7d4e8..f267cd3 100644
> --- a/fs/ext4/ext4_crypto.h
> +++ b/fs/ext4/ext4_crypto.h
> @@ -156,4 +156,12 @@ static inline u32 encrypted_symlink_data_len(u32 l)
> 	return (l + sizeof(struct ext4_encrypted_symlink_data) - 1);
> }
> 
> +/**
> + * Structure used for communicating encrypted metadata with userspace
> + */
> +struct ext4_encrypted_metadata {
> +	u32 len;
> +	char metadata[288];
> +};
> +
> #endif	/* _EXT4_CRYPTO_H */
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 5e872fd..afb51f5 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -689,6 +689,67 @@ encryption_policy_out:
> 		return -EOPNOTSUPP;
> #endif
> 	}
> +	case EXT4_IOC_GET_ENCRYPTION_METADATA: {
> +#ifdef CONFIG_EXT4_FS_ENCRYPTION
> +		struct ext4_encrypted_metadata mdata;
> +		int err = 0;
> +
> +		if (get_user(mdata.len, (u32 __user *) arg))
> +			return -EFAULT;
> +		if (mdata.len > sizeof(mdata.metadata))
> +			return -EINVAL;
> +
> +		if (!ext4_encrypted_inode(inode))
> +			return -ENOENT;
> +		err = ext4_get_encryption_metadata(inode, &mdata);
> +		if (err)
> +			return err;
> +		if (copy_to_user((void __user *)arg, &mdata, sizeof(mdata)))
> +			return -EFAULT;
> +		return 0;
> +#else
> +		return -EOPNOTSUPP;
> +#endif
> +	}
> +	case EXT4_IOC_SET_ENCRYPTION_METADATA: {
> +#ifdef CONFIG_EXT4_FS_ENCRYPTION
> +		struct ext4_encrypted_metadata mdata;
> +		int err = 0;
> +
> +		if (ext4_encrypted_inode(inode))
> +			return -EINVAL;
> +		if (copy_from_user(&mdata,
> +				   (struct ext4_encrypted_metadata __user *)arg,
> +				   sizeof(mdata)))
> +			return -EFAULT;
> +		err = ext4_set_encryption_metadata(inode, &mdata);
> +		return err;
> +#else
> +		return -EOPNOTSUPP;
> +#endif
> +	}
> +	case EXT4_IOC_GET_ENCRYPTED_FILENAME: {
> +#ifdef CONFIG_EXT4_FS_ENCRYPTION
> +		struct ext4_encrypted_metadata mdata;
> +		int err = 0;
> +
> +		if (get_user(mdata.len, (u32 __user *) arg))
> +			return -EFAULT;
> +		if (mdata.len > sizeof(mdata.metadata))
> +			return -EINVAL;
> +
> +		if (!ext4_encrypted_inode(inode))
> +			return -ENOENT;
> +		err = ext4_get_encrypted_filename(filp, &mdata);
> +		if (err)
> +			return err;
> +		if (copy_to_user((void __user *)arg, &mdata, sizeof(mdata)))
> +			return -EFAULT;
> +		return 0;
> +#else
> +		return -EOPNOTSUPP;
> +#endif
> +	}
> 	default:
> 		return -ENOTTY;
> 	}
> @@ -755,6 +816,9 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> 	case EXT4_IOC_SET_ENCRYPTION_POLICY:
> 	case EXT4_IOC_GET_ENCRYPTION_PWSALT:
> 	case EXT4_IOC_GET_ENCRYPTION_POLICY:
> +	case EXT4_IOC_GET_ENCRYPTION_METADATA:
> +	case EXT4_IOC_SET_ENCRYPTION_METADATA:
> +	case EXT4_IOC_GET_ENCRYPTED_FILENAME:
> 		break;
> 	default:
> 		return -ENOIOCTLCMD;
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index a969ab3..1cc4eef 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3861,3 +3861,27 @@ const struct inode_operations ext4_special_inode_operations = {
> 	.get_acl	= ext4_get_acl,
> 	.set_acl	= ext4_set_acl,
> };
> +
> +int ext4_get_encrypted_filename(struct file *filp,
> +				struct ext4_encrypted_metadata *mdata)
> +{
> +	struct dentry *dentry = filp->f_path.dentry;
> +	struct inode *dir = dentry->d_parent->d_inode;
> +	struct buffer_head *bh;
> +	struct ext4_dir_entry_2 *de;
> +
> +	if (!dir || !ext4_encrypted_inode(dir))
> +		return -EINVAL;
> +
> +	bh = ext4_find_entry(dir, &dentry->d_name, &de, NULL);
> +	if (IS_ERR(bh))
> +		return PTR_ERR(bh);
> +	if (de == NULL)
> +		return -ENOENT;
> +
> +	if (mdata->len < de->name_len)
> +		return -ENOSPC;
> +	mdata->len = de->name_len;
> +	memcpy(mdata->metadata, de->name, de->name_len);
> +	return 0;
> +}
> --
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 2/3] ext4 crypto: add ioctls to allow backup of encryption metadata

[Theodore Ts'o]

On Thu, Dec 10, 2015 at 04:48:56PM -0700, Andreas Dilger wrote:
> 
> Instead of exposing these via ioctls, why not use virtual xattrs
> (e.g. "trusted.crypt_meta" or similar) so that this can be backed
> up and restored using a normal backup tool instead of basically
> nothing that exists today.

We're going to have to have an encrypted-aware backup tool anyway ---
if for no other reason than to deal with the fact that if i_size is
not a multiple of the cipher's block size, that we need to backup at
least ROUND_UP(i_size, cipher_block_size) bytes, but we need to store
the original i_size field somewhere else.

The problem gets even worse to do encrypted backup *restores* because
we need to be able to pass encrypted filenames that include NUL
characters through the VFS, but the raw encrypted filename is
different from the human-friendly encrypted filename which is what we
want in the dentry cache.  So I don't think it's worth the effort to
try to make it possible to do backups using standard tools.

Hmm, thinking about this some more, I might be able to support
encrypted restores w/o having access to the key by using a magic ioctl
that takes two file descriptors (one for the encrypted directory, and
another one for the encrypted file, plus the encrypted filename), and
do this bypassing the VFS layer entirely.  It's something I may look
at.  But the point is this is even further away from an interface that
could be hacked using xattrs.

> For reading the ciphertext, do existing tools such as tar and rsync
> allow reading with O_DIRECT with CIPHERTEXT_ACCESS?

No.  None of the standard tools understand that you might have to
remount the file system with a special mount option.  And neither tar
nor rsync have an option so they will read their files using O_DIRECT.

> If not, one might
> consider to allow normal reads w/o the file key to return the
> ciphertext so that "tar --xattr" by some process with CAP_SYS_ADMIN
> but out any keys could be used to backup and restore the ciphertext
> in a sensible manner.  That is no more risk than the same process
> reading the blocks from the disk device directly, but would be a lot
> more useful (i.e. it is practical to use normal backup tools for
> encrypted files).

The reason why I'm using O_DIRECT is so we don't have to worry about
potentially posioning the page cache with encrypted pages.  In our
current design, the page cache for an encrypted file always contains
the plaintext.  This is necessary so that mmap(2) will work correctly.
So if user B is logged onto a machine where user A is active, and user
A's keys are in the kernel, we don't determine access control
descisions for user B based on whether or not the user has the
appropriate keyring in a keyring accessible to them.  We use Unix
permissions to provide security isolation between two users which are
logged in.

In theory, when user A logs out, userspace should do the equivalent of
"echo 3 > /proc/sys/vm/drop_caches" to drop the plaintext pages from
the page cache.  But I don't want the correctness of an encrypted
backup to depend on that.  Also, if someone wants to insist on doing
an encrypted backup while the keys are present, that should work as
well.  That's why using O_DIRECT to bypass the page cache was a key
part of the design, and why it's problematic to allow normal file
reads to be used for this purpose.

The bottom line is that it's impossible to use normal backup tools for
encrypted files work correctly _anyway_ (because of the i_size /
ciphertext block size rounding issue), and between wanting to allow
for encrypted backups even if the user the keys are available, and the
page cache poisioning issue, it's simpler all around to use specific
ioctls.

					- Ted

P.S.  I actually have another use case in mind besides just encrypted
backups, but (a) I'm not allowed to comment about features that might
or might not show up in unreleased products, (b) you never know when
pesky reporters from Android Central might be hanging about, and (c)
being able to do encrypted backups is a feature which Dmitry Monakhov
explicitly requested, and a mark of a good design is one that will
work well for more than one use case.  :-)

Re: [PATCH v3 0/3] ext4 crypto: back up encrypted files

[Jan Kara]

On Thu 10-12-15 10:04:36, Ted Tso wrote:
> This patches allow backing up encrypted files without having access to
> the key.  Unfortunately, the key *is* necessary to restore the files,
> because establishing a link means that we have to manipulate the both
> the encrypted directory and the encrypted file, and doing this through
> the VFS interface is non-trivial.  So we have an ioctl which extracts
> the encrypted file name, and that in combination with the encryption
> metadata for the directory should be sufficient to restore the file
> name assuming the restore is done with access to the user's master
> key.
> 
> The other tricky bit is that if the file's i_size is not a multiple of
> the AES block size, we need to be able to copy a handful of bytes
> before i_size --- and O_DIRECT reads don't allow that.  There are two

Umm, I don't quite follow. O_DIRECT reads will actually read final file
block in full even if i_size is somewhere in the middle of it. We then
report only data upto i_size as transferred but that's not really
important for you. That being said I agree that relying on this is hacky
but for direct IO it kind of makes sense and creating special shadow
inode is IMO even bigger hack...

> ways of solving this.  One would be an new DIO_FLAG that rounds i_size
> up to the file system blocksize, which we would pass when reading
> encrypted files using O_DIRECT.  This would require changes to the
> core direct I/O, and may be controversial.  It also may make it more
> difficult to back port these patches to ancient BSP kernels.
> 
> So what we're doing for now is admittedly a hack.  Since encrypted
> files are read-only without access to the key, it is safe to create a
> shadow copy of the inode structure, and round up i_size in the shadow
> structure.  We only do this when reading the last block in the file,
> so the overhead shouldn't be too bad.

I'm rather concerned about the locking implications of the shadow inode.
For example holding i_data_sem for reading in ext4_get_blocks() no longer
protects against changes of inode allocation information. Thus if e.g.
another process does mmaped write allocating more blocks, we can see
inconsistent extent tree from direct read. And that's just one example of
problems we can hit, I'm not sure there are not others.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 0/3] ext4 crypto: back up encrypted files

[Theodore Ts'o]

On Wed, Dec 16, 2015 at 04:10:10PM +0100, Jan Kara wrote:
> 
> Umm, I don't quite follow. O_DIRECT reads will actually read final file
> block in full even if i_size is somewhere in the middle of it. We then
> report only data upto i_size as transferred but that's not really
> important for you.

I had tried this approach first but it appeared that the data was
getting zero'ed between i_size and the end of the block.  It turns out
it was a bug in my test program, sigh.

I agree about the locking issues.  It isn't so much of an issue since
without the encryption key, you can't modify the file, so this
prevents most of the nasty races.  Of course, it could be the case
that user A (say, root) doesn't have access to the key, but user B
(say the user account) does have access.  So dropping the shadow inode
is the better way to go.

Thanks!!

						- Ted