[PATCH v1 00/12] fscrypt: add extent encryption

[PATCH v1 00/12] fscrypt: add extent encryption

[Sweet Tea Dorminy]

This changeset adds extent-based data encryption to fscrypt.
Some filesystems need to encrypt data based on extents, rather than on
inodes, due to features incompatible with inode-based encryption. For
instance, btrfs can have multiple inodes referencing a single block of
data, and moves logical data blocks to different physical locations on
disk in the background. 

As per discussion last year in [1] and later in [2], we would like to
allow the use of fscrypt with btrfs, with authenticated encryption. This
is the first step of that work, adding extent-based encryption to
fscrypt; authenticated encryption is the next step. Extent-based
encryption should be usable by other filesystems which wish to support
snapshotting or background data rearrangement also, but btrfs is the
first user. 

This changeset requires extent encryption to use inlinecrypt, as
discussed previously. There are two questionable parts: the
forget_extent_info hook is not yet in use by btrfs, as I haven't yet
written a test exercising a race where it would be relevant; and saving
the session key credentials just to enable v1 session-based policies is
perhaps less good than 

This applies atop [3], which itself is based on kdave/misc-next. It
passes most encryption fstests with suitable changes to btrfs-progs, but
not generic/580 or generic/595 due to different timing involved in
extent encryption. Tests and btrfs progs updates to follow.


[1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
[2] https://lore.kernel.org/linux-fscrypt/80496cfe-161d-fb0d-8230-93818b966b1b@dorminy.me/T/#t
[3]
https://lore.kernel.org/linux-fscrypt/cover.1687988119.git.sweettea-kernel@dorminy.me/

Sweet Tea Dorminy (12):
  fscrypt: factor helper for locking master key
  fscrypt: factor getting info for a specific block
  fscrypt: adjust effective lblks based on extents
  fscrypt: add a super_block pointer to fscrypt_info
  fscrypt: setup leaf inodes for extent encryption
  fscrypt: allow infos to be owned by extents
  fscrypt: notify per-extent infos if master key vanishes
  fscrypt: use an optional ino equivalent for per-extent infos
  fscrypt: add creation/usage/freeing of per-extent infos
  fscrypt: allow load/save of extent contexts
  fscrypt: save session key credentials for extent infos
  fscrypt: update documentation for per-extent keys

 Documentation/filesystems/fscrypt.rst |  38 +++-
 fs/crypto/crypto.c                    |   6 +-
 fs/crypto/fscrypt_private.h           |  91 ++++++++++
 fs/crypto/inline_crypt.c              |  28 ++-
 fs/crypto/keyring.c                   |  32 +++-
 fs/crypto/keysetup.c                  | 244 ++++++++++++++++++++++----
 fs/crypto/keysetup_v1.c               |   7 +-
 fs/crypto/policy.c                    |  20 +++
 include/linux/fscrypt.h               |  74 ++++++++
 9 files changed, 480 insertions(+), 60 deletions(-)


base-commit: accadeb67609a5a5d088ebde8409c3f6db0b84b4
-- 
2.40.1

[PATCH v1 01/12] fscrypt: factor helper for locking master key

[Sweet Tea Dorminy]

When we are making extent infos, we'll need to lock the master key in
more places, so go on and factor out a helper.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/keysetup.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index be3a84508806..6cbb42a8a537 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -106,7 +106,18 @@ select_encryption_mode(const union fscrypt_policy *policy,
 	return ERR_PTR(-EINVAL);
 }
 
-/* Create a symmetric cipher object for the given encryption mode and key */
+static int lock_master_key(struct fscrypt_master_key *mk)
+{
+	down_read(&mk->mk_sem);
+
+	/* Has the secret been removed (via FS_IOC_REMOVE_ENCRYPTION_KEY)? */
+	if (!is_master_key_secret_present(&mk->mk_secret))
+		return -ENOKEY;
+
+	return 0;
+}
+
+/* Create a symmetric cipher object for the given encryption mode */
 static struct crypto_skcipher *
 fscrypt_allocate_skcipher(struct fscrypt_mode *mode, const u8 *raw_key,
 			  const struct inode *inode)
@@ -556,13 +567,10 @@ static int find_and_lock_master_key(const struct fscrypt_info *ci,
 		*mk_ret = NULL;
 		return 0;
 	}
-	down_read(&mk->mk_sem);
 
-	/* Has the secret been removed (via FS_IOC_REMOVE_ENCRYPTION_KEY)? */
-	if (!is_master_key_secret_present(&mk->mk_secret)) {
-		err = -ENOKEY;
+	err = lock_master_key(mk);
+	if (err)
 		goto out_release_key;
-	}
 
 	if (!fscrypt_valid_master_key_size(mk, ci)) {
 		err = -ENOKEY;
-- 
2.40.1

[PATCH v1 02/12] fscrypt: factor getting info for a specific block

[Sweet Tea Dorminy]

For filesystems using extent-based encryption, the content of each
extent will be encrypted with a different fscrypt_info for each extent.
Meanwhile, directories and symlinks will continue to use the
fscrypt_info for the inode. Therefore, merely grabbing
inode->i_crypt_info will be insufficient; the caller must specifically
request the inode info or the info for a specific block.

Add fscrypt_get_lblk_info() to get info for a specific block, and update
all relevant callsites.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/crypto.c          |  3 ++-
 fs/crypto/fscrypt_private.h | 29 +++++++++++++++++++++++++++++
 fs/crypto/inline_crypt.c    | 10 ++++++----
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 9f3bda18c797..1b7e375b1c6b 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -107,7 +107,8 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
 	struct skcipher_request *req = NULL;
 	DECLARE_CRYPTO_WAIT(wait);
 	struct scatterlist dst, src;
-	struct fscrypt_info *ci = inode->i_crypt_info;
+	struct fscrypt_info *ci =
+		fscrypt_get_lblk_info(inode, lblk_num, NULL, NULL);
 	struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
 	int res = 0;
 
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index e726a1fb9f7e..4d1e67bc1e62 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -277,6 +277,35 @@ typedef enum {
 	FS_ENCRYPT,
 } fscrypt_direction_t;
 
+/**
+ * fscrypt_get_lblk_info() - get the fscrypt_info to crypt a particular block
+ *
+ * @inode:      the inode to which the block belongs
+ * @lblk:       the offset of the block within the file which the inode
+ *              references
+ * @offset:     a pointer to return the offset of the block from the first block
+ *              that the info covers. For inode-based encryption, this will
+ *              always be @lblk; for extent-based encryption, this will be in
+ *              the range [0, lblk]. Can be NULL
+ * @extent_len: a pointer to return the minimum number of lblks starting at
+ *              this offset which also belong to the same fscrypt_info. Can be
+ *              NULL
+ *
+ * Return: the appropriate fscrypt_info if there is one, else NULL.
+ */
+static inline struct fscrypt_info *
+fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
+		      u64 *extent_len)
+{
+	if (offset)
+		*offset = lblk;
+	if (extent_len)
+		*extent_len = U64_MAX;
+
+	return inode->i_crypt_info;
+}
+
+
 /* crypto.c */
 extern struct kmem_cache *fscrypt_info_cachep;
 int fscrypt_initialize(struct super_block *sb);
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 2063f7941ce6..885a2ec3d711 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -270,7 +270,7 @@ void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
 
 	if (!fscrypt_inode_uses_inline_crypto(inode))
 		return;
-	ci = inode->i_crypt_info;
+	ci = fscrypt_get_lblk_info(inode, first_lblk, NULL, NULL);
 
 	fscrypt_generate_dun(ci, first_lblk, dun);
 	bio_crypt_set_ctx(bio, ci->ci_enc_key->blk_key, dun, gfp_mask);
@@ -349,21 +349,23 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 {
 	const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
 	u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
+	struct fscrypt_info *ci;
 
 	if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
 		return false;
 	if (!bc)
 		return true;
 
+	ci = fscrypt_get_lblk_info(inode, next_lblk, NULL, NULL);
 	/*
 	 * Comparing the key pointers is good enough, as all I/O for each key
 	 * uses the same pointer.  I.e., there's currently no need to support
 	 * merging requests where the keys are the same but the pointers differ.
 	 */
-	if (bc->bc_key != inode->i_crypt_info->ci_enc_key->blk_key)
+	if (bc->bc_key != ci->ci_enc_key->blk_key)
 		return false;
 
-	fscrypt_generate_dun(inode->i_crypt_info, next_lblk, next_dun);
+	fscrypt_generate_dun(ci, next_lblk, next_dun);
 	return bio_crypt_dun_is_contiguous(bc, bio->bi_iter.bi_size, next_dun);
 }
 EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio);
@@ -465,7 +467,7 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
 	if (nr_blocks <= 1)
 		return nr_blocks;
 
-	ci = inode->i_crypt_info;
+	ci = fscrypt_get_lblk_info(inode, lblk, NULL, NULL);
 	if (!(fscrypt_policy_flags(&ci->ci_policy) &
 	      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
 		return nr_blocks;
-- 
2.40.1

[PATCH v1 03/12] fscrypt: adjust effective lblks based on extents

[Sweet Tea Dorminy]

If a filesystem uses extent-based encryption, then the offset within a
file is not a constant which can be used for calculating an IV.
For instance, the same extent could be blocks 0-8 in one file, and
blocks 100-108 in another file. Instead, the block offset within the
extent must be used instead.

Update all uses of logical block offset within the file to use logical
block offset within the extent, if applicable.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/crypto.c       |  3 ++-
 fs/crypto/inline_crypt.c | 22 ++++++++++++++++------
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 1b7e375b1c6b..d75f1b3f5795 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -107,8 +107,9 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
 	struct skcipher_request *req = NULL;
 	DECLARE_CRYPTO_WAIT(wait);
 	struct scatterlist dst, src;
+	u64 ci_offset = 0;
 	struct fscrypt_info *ci =
-		fscrypt_get_lblk_info(inode, lblk_num, NULL, NULL);
+		fscrypt_get_lblk_info(inode, lblk_num, &ci_offset, NULL);
 	struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
 	int res = 0;
 
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 885a2ec3d711..689ab342ae34 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -267,12 +267,15 @@ void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
 {
 	const struct fscrypt_info *ci;
 	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
+	u64 ci_offset = 0;
 
 	if (!fscrypt_inode_uses_inline_crypto(inode))
 		return;
-	ci = fscrypt_get_lblk_info(inode, first_lblk, NULL, NULL);
+	ci = fscrypt_get_lblk_info(inode, first_lblk, &ci_offset, NULL);
+	if (!ci)
+		return;
 
-	fscrypt_generate_dun(ci, first_lblk, dun);
+	fscrypt_generate_dun(ci, ci_offset, dun);
 	bio_crypt_set_ctx(bio, ci->ci_enc_key->blk_key, dun, gfp_mask);
 }
 EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx);
@@ -350,13 +353,14 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 	const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
 	u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
 	struct fscrypt_info *ci;
+	u64 ci_offset = 0;
 
 	if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
 		return false;
 	if (!bc)
 		return true;
 
-	ci = fscrypt_get_lblk_info(inode, next_lblk, NULL, NULL);
+	ci = fscrypt_get_lblk_info(inode, next_lblk, &ci_offset, NULL);
 	/*
 	 * Comparing the key pointers is good enough, as all I/O for each key
 	 * uses the same pointer.  I.e., there's currently no need to support
@@ -365,7 +369,7 @@ bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 	if (bc->bc_key != ci->ci_enc_key->blk_key)
 		return false;
 
-	fscrypt_generate_dun(ci, next_lblk, next_dun);
+	fscrypt_generate_dun(ci, ci_offset, next_dun);
 	return bio_crypt_dun_is_contiguous(bc, bio->bi_iter.bi_size, next_dun);
 }
 EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio);
@@ -460,6 +464,8 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
 {
 	const struct fscrypt_info *ci;
 	u32 dun;
+	u64 ci_offset = 0;
+	u64 extent_len = 0;
 
 	if (!fscrypt_inode_uses_inline_crypto(inode))
 		return nr_blocks;
@@ -467,14 +473,18 @@ u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk, u64 nr_blocks)
 	if (nr_blocks <= 1)
 		return nr_blocks;
 
-	ci = fscrypt_get_lblk_info(inode, lblk, NULL, NULL);
+	ci = fscrypt_get_lblk_info(inode, lblk, &ci_offset, &extent_len);
+
+	/* Spanning an extent boundary will change the DUN */
+	nr_blocks = min_t(u64, nr_blocks, extent_len);
+
 	if (!(fscrypt_policy_flags(&ci->ci_policy) &
 	      FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32))
 		return nr_blocks;
 
 	/* With IV_INO_LBLK_32, the DUN can wrap around from U32_MAX to 0. */
 
-	dun = ci->ci_hashed_ino + lblk;
+	dun = ci->ci_hashed_ino + ci_offset;
 
 	return min_t(u64, nr_blocks, (u64)U32_MAX + 1 - dun);
 }
-- 
2.40.1

[PATCH v1 04/12] fscrypt: add a super_block pointer to fscrypt_info

[Sweet Tea Dorminy]

When fscrypt_infos are attached to extents instead of inodes, we can't
go through the inode to get at the filesystems's superblock. Therefore,
add a dedicated superblock pointer to fscrypt_info to keep track of it.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h |  3 +++
 fs/crypto/inline_crypt.c    |  4 ++--
 fs/crypto/keysetup.c        | 10 +++++-----
 fs/crypto/keysetup_v1.c     |  6 +++---
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4d1e67bc1e62..c04454c289fd 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -241,6 +241,9 @@ struct fscrypt_info {
 	/* Back-pointer to the inode */
 	struct inode *ci_inode;
 
+	/* The superblock of the filesystem to which this info pertains */
+	struct super_block *ci_sb;
+
 	/*
 	 * The master key with which this inode was unlocked (decrypted).  This
 	 * will be NULL if the master key was found in a process-subscribed
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 689ab342ae34..f1a610a0f6bf 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -41,7 +41,7 @@ static struct block_device **fscrypt_get_devices(struct super_block *sb,
 
 static unsigned int fscrypt_get_dun_bytes(const struct fscrypt_info *ci)
 {
-	struct super_block *sb = ci->ci_inode->i_sb;
+	struct super_block *sb = ci->ci_sb;
 	unsigned int flags = fscrypt_policy_flags(&ci->ci_policy);
 	int ino_bits = 64, lblk_bits = 64;
 
@@ -155,7 +155,7 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
 				     const struct fscrypt_info *ci)
 {
 	const struct inode *inode = ci->ci_inode;
-	struct super_block *sb = inode->i_sb;
+	struct super_block *sb = ci->ci_sb;
 	enum blk_crypto_mode_num crypto_mode = ci->ci_mode->blk_crypto_mode;
 	struct blk_crypto_key *blk_key;
 	struct block_device **devs;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 6cbb42a8a537..67a5749a9543 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -250,8 +250,7 @@ static int setup_new_mode_prepared_key(struct fscrypt_master_key *mk,
 				       struct fscrypt_prepared_key *prep_key,
 				       const struct fscrypt_info *ci)
 {
-	const struct inode *inode = ci->ci_inode;
-	const struct super_block *sb = inode->i_sb;
+	const struct super_block *sb = ci->ci_sb;
 	unsigned int policy_flags = fscrypt_policy_flags(&ci->ci_policy);
 	struct fscrypt_mode *mode = ci->ci_mode;
 	const u8 mode_num = mode - fscrypt_modes;
@@ -525,7 +524,7 @@ static bool fscrypt_valid_master_key_size(const struct fscrypt_master_key *mk,
 static int find_and_lock_master_key(const struct fscrypt_info *ci,
 				    struct fscrypt_master_key **mk_ret)
 {
-	struct super_block *sb = ci->ci_inode->i_sb;
+	struct super_block *sb = ci->ci_sb;
 	struct fscrypt_key_specifier mk_spec;
 	struct fscrypt_master_key *mk;
 	int err;
@@ -599,7 +598,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
 		if (type == FSCRYPT_KEY_DIRECT_V1)
 			fscrypt_put_direct_key(ci->ci_enc_key);
 		if (type == FSCRYPT_KEY_PER_INFO) {
-			fscrypt_destroy_prepared_key(ci->ci_inode->i_sb,
+			fscrypt_destroy_prepared_key(ci->ci_sb,
 						     ci->ci_enc_key);
 			kfree_sensitive(ci->ci_enc_key);
 		}
@@ -616,7 +615,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
 		spin_lock(&mk->mk_decrypted_inodes_lock);
 		list_del(&ci->ci_master_key_link);
 		spin_unlock(&mk->mk_decrypted_inodes_lock);
-		fscrypt_put_master_key_activeref(ci->ci_inode->i_sb, mk);
+		fscrypt_put_master_key_activeref(ci->ci_sb, mk);
 	}
 	memzero_explicit(ci, sizeof(*ci));
 	kmem_cache_free(fscrypt_info_cachep, ci);
@@ -642,6 +641,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 		return -ENOMEM;
 
 	crypt_info->ci_inode = inode;
+	crypt_info->ci_sb = inode->i_sb;
 	crypt_info->ci_policy = *policy;
 	memcpy(crypt_info->ci_nonce, nonce, FSCRYPT_FILE_NONCE_SIZE);
 
diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c
index 1e785cedead0..41d317f08aeb 100644
--- a/fs/crypto/keysetup_v1.c
+++ b/fs/crypto/keysetup_v1.c
@@ -235,7 +235,7 @@ fscrypt_get_direct_key(const struct fscrypt_info *ci, const u8 *raw_key)
 	dk = kzalloc(sizeof(*dk), GFP_KERNEL);
 	if (!dk)
 		return ERR_PTR(-ENOMEM);
-	dk->dk_sb = ci->ci_inode->i_sb;
+	dk->dk_sb = ci->ci_sb;
 	refcount_set(&dk->dk_refcount, 1);
 	dk->dk_mode = ci->ci_mode;
 	dk->dk_key.type = FSCRYPT_KEY_DIRECT_V1;
@@ -309,8 +309,8 @@ int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci)
 	key = find_and_lock_process_key(FSCRYPT_KEY_DESC_PREFIX,
 					ci->ci_policy.v1.master_key_descriptor,
 					ci->ci_mode->keysize, &payload);
-	if (key == ERR_PTR(-ENOKEY) && ci->ci_inode->i_sb->s_cop->key_prefix) {
-		key = find_and_lock_process_key(ci->ci_inode->i_sb->s_cop->key_prefix,
+	if (key == ERR_PTR(-ENOKEY) && ci->ci_sb->s_cop->key_prefix) {
+		key = find_and_lock_process_key(ci->ci_sb->s_cop->key_prefix,
 						ci->ci_policy.v1.master_key_descriptor,
 						ci->ci_mode->keysize, &payload);
 	}
-- 
2.40.1

[PATCH v1 05/12] fscrypt: setup leaf inodes for extent encryption

[Sweet Tea Dorminy]

For extent-based encryption, leaf/regular file inodes are special: it's
useful to set their i_crypt_info field so that it's easy to inherit
their encryption policy for a new extent, but they never need to do any
encyption themselves. Additionally, since encryption can only be set up
on a directory, not a single file, their encryption policy can always
duplicate their parent inode's policy.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h | 17 +++++++++++++
 fs/crypto/keysetup.c        | 49 ++++++++++++++++++++++++++-----------
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index c04454c289fd..260635e8b558 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -308,6 +308,23 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
 	return inode->i_crypt_info;
 }
 
+/**
+ * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
+ *				       encryption
+ *
+ * @inode: the inode in question
+ *
+ * Return: true if the inode uses per-extent fscrypt_infos, false otherwise
+ */
+static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
+{
+	// Non-regular files don't have extents
+	if (!S_ISREG(inode->i_mode))
+		return false;
+
+	// No filesystems currently use per-extent infos
+	return false;
+}
 
 /* crypto.c */
 extern struct kmem_cache *fscrypt_info_cachep;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 67a5749a9543..d79a42a54906 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -747,27 +747,48 @@ fscrypt_setup_encryption_info(struct inode *inode,
 int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 {
 	int res;
-	union fscrypt_context ctx;
+	union fscrypt_context ctx = { 0 };
 	union fscrypt_policy policy;
 
 	if (fscrypt_has_encryption_key(inode))
 		return 0;
 
-	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
-	if (res < 0) {
-		if (res == -ERANGE && allow_unsupported)
-			return 0;
-		fscrypt_warn(inode, "Error %d getting encryption context", res);
-		return res;
-	}
+	if (fscrypt_uses_extent_encryption(inode)) {
+		/*
+		 * Nothing will be encrypted with this info, so we can borrow
+		 * the parent (dir) inode's policy and use a zero nonce.
+		 */
+		struct dentry *dentry = d_find_any_alias(inode);
+		struct dentry *parent_dentry = dget_parent(dentry);
+		struct inode *dir = parent_dentry->d_inode;
+		bool found = false;
 
-	res = fscrypt_policy_from_context(&policy, &ctx, res);
-	if (res) {
-		if (allow_unsupported)
+		if (dir->i_crypt_info) {
+			found = true;
+			policy = dir->i_crypt_info->ci_policy;
+			nonce = dir->i_crypt_info->ci_nonce;
+		}
+		dput(parent_dentry);
+		dput(dentry);
+		if (!found)
 			return 0;
-		fscrypt_warn(inode,
-			     "Unrecognized or corrupt encryption context");
-		return res;
+	} else {
+		res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
+		if (res < 0) {
+			if (res == -ERANGE && allow_unsupported)
+				return 0;
+			fscrypt_warn(inode, "Error %d getting encryption context", res);
+			return res;
+		}
+
+		res = fscrypt_policy_from_context(&policy, &ctx, res);
+		if (res) {
+			if (allow_unsupported)
+				return 0;
+			fscrypt_warn(inode,
+				     "Unrecognized or corrupt encryption context");
+			return res;
+		}
 	}
 
 	if (!fscrypt_supported_policy(&policy, inode)) {
-- 
2.40.1

[PATCH v1 06/12] fscrypt: allow infos to be owned by extents

[Sweet Tea Dorminy]

In order to notify extents when their info is part of a master key which
is going away, the fscrypt_info must have a backpointer to the extent
somehow. Similarly, if a fscrypt_info is owned by an extent, the info
must not have a pointer to an inode -- multiple inodes may reference a
extent, and the first inode to cause an extent's creation may have a
lifetime much shorter than the extent, so there is no inode pointer
safe to track in an extent-owned info. Therefore, this adds a new
pointer for extent-owned infos to track their extent and updates
fscrypt_setup_encryption_info() accordingly.

Since it's simple to track the piece of extent memory pointing to the
info, and for the extent to then go from such a pointer to the whole
extent via container_of(), we store that. Although some sort of generic
void * or some artificial fscrypt_extent embedded structure would also
work, those would require additional plumbing which doesn't seem
strictly required or clarifying.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h |  6 +++++
 fs/crypto/keysetup.c        | 49 ++++++++++++++++++++++++++++---------
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 260635e8b558..1674e66e72e3 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -241,6 +241,12 @@ struct fscrypt_info {
 	/* Back-pointer to the inode */
 	struct inode *ci_inode;
 
+	/*
+	 * Back-pointer to the info pointer in the extent, for infos owned by
+	 * an extent.
+	 */
+	struct fscrypt_info **ci_info_ptr;
+
 	/* The superblock of the filesystem to which this info pertains */
 	struct super_block *ci_sb;
 
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index d79a42a54906..1aa5b2a0096e 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -625,12 +625,17 @@ static int
 fscrypt_setup_encryption_info(struct inode *inode,
 			      const union fscrypt_policy *policy,
 			      const u8 nonce[FSCRYPT_FILE_NONCE_SIZE],
-			      bool need_dirhash_key)
+			      bool need_dirhash_key,
+			      struct fscrypt_info **info_ptr)
 {
 	struct fscrypt_info *crypt_info;
 	struct fscrypt_mode *mode;
 	struct fscrypt_master_key *mk = NULL;
 	int res;
+	bool info_for_extent = !!info_ptr;
+
+	if (!info_ptr)
+		info_ptr = &inode->i_crypt_info;
 
 	res = fscrypt_initialize(inode->i_sb);
 	if (res)
@@ -640,7 +645,11 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	if (!crypt_info)
 		return -ENOMEM;
 
-	crypt_info->ci_inode = inode;
+	if (fscrypt_uses_extent_encryption(inode) && info_for_extent)
+		crypt_info->ci_info_ptr = info_ptr;
+	else
+		crypt_info->ci_inode = inode;
+
 	crypt_info->ci_sb = inode->i_sb;
 	crypt_info->ci_policy = *policy;
 	memcpy(crypt_info->ci_nonce, nonce, FSCRYPT_FILE_NONCE_SIZE);
@@ -656,6 +665,12 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	res = fscrypt_select_encryption_impl(crypt_info);
 	if (res)
 		goto out;
+	if (info_for_extent && !fscrypt_using_inline_encryption(crypt_info)) {
+		fscrypt_warn(inode,
+			     "extent encryption requires inlinecrypt mount option");
+		res = -EINVAL;
+		goto out;
+	}
 
 	res = find_and_lock_master_key(crypt_info, &mk);
 	if (res)
@@ -701,7 +716,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	 * fscrypt_get_info().  I.e., here we publish ->i_crypt_info with a
 	 * RELEASE barrier so that other tasks can ACQUIRE it.
 	 */
-	if (cmpxchg_release(&inode->i_crypt_info, NULL, crypt_info) == NULL) {
+	if (cmpxchg_release(info_ptr, NULL, crypt_info) == NULL) {
 		/*
 		 * We won the race and set ->i_crypt_info to our crypt_info.
 		 * Now link it into the master key's inode list.
@@ -735,7 +750,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
  *		       %false unless the operation being performed is needed in
  *		       order for files (or directories) to be deleted.
  *
- * Set up ->i_crypt_info, if it hasn't already been done.
+ * Set up inode->i_crypt_info, if it hasn't already been done.
  *
  * Note: unless ->i_crypt_info is already set, this isn't %GFP_NOFS-safe.  So
  * generally this shouldn't be called from within a filesystem transaction.
@@ -747,8 +762,9 @@ fscrypt_setup_encryption_info(struct inode *inode,
 int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 {
 	int res;
-	union fscrypt_context ctx = { 0 };
+	union fscrypt_context ctx;
 	union fscrypt_policy policy;
+	const u8 *nonce;
 
 	if (fscrypt_has_encryption_key(inode))
 		return 0;
@@ -756,7 +772,7 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 	if (fscrypt_uses_extent_encryption(inode)) {
 		/*
 		 * Nothing will be encrypted with this info, so we can borrow
-		 * the parent (dir) inode's policy and use a zero nonce.
+		 * the parent (dir) inode's policy and nonce.
 		 */
 		struct dentry *dentry = d_find_any_alias(inode);
 		struct dentry *parent_dentry = dget_parent(dentry);
@@ -789,6 +805,7 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 				     "Unrecognized or corrupt encryption context");
 			return res;
 		}
+		nonce = fscrypt_context_nonce(&ctx);
 	}
 
 	if (!fscrypt_supported_policy(&policy, inode)) {
@@ -797,10 +814,10 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 		return -EINVAL;
 	}
 
-	res = fscrypt_setup_encryption_info(inode, &policy,
-					    fscrypt_context_nonce(&ctx),
+	res = fscrypt_setup_encryption_info(inode, &policy, nonce,
 					    IS_CASEFOLDED(inode) &&
-					    S_ISDIR(inode->i_mode));
+					    S_ISDIR(inode->i_mode),
+					    NULL);
 
 	if (res == -ENOPKG && allow_unsupported) /* Algorithm unavailable? */
 		res = 0;
@@ -834,7 +851,8 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
 			      bool *encrypt_ret)
 {
 	const union fscrypt_policy *policy;
-	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
+	u8 nonce_bytes[FSCRYPT_FILE_NONCE_SIZE];
+	const u8 *nonce;
 
 	policy = fscrypt_policy_to_inherit(dir);
 	if (policy == NULL)
@@ -856,10 +874,17 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
 
 	*encrypt_ret = true;
 
-	get_random_bytes(nonce, FSCRYPT_FILE_NONCE_SIZE);
+	if (fscrypt_uses_extent_encryption(inode)) {
+		nonce = dir->i_crypt_info->ci_nonce;
+	} else {
+		get_random_bytes(nonce_bytes, FSCRYPT_FILE_NONCE_SIZE);
+		nonce = nonce_bytes;
+	}
+
 	return fscrypt_setup_encryption_info(inode, policy, nonce,
 					     IS_CASEFOLDED(dir) &&
-					     S_ISDIR(inode->i_mode));
+					     S_ISDIR(inode->i_mode),
+					     NULL);
 }
 EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
 
-- 
2.40.1

[PATCH v1 07/12] fscrypt: notify per-extent infos if master key vanishes

[Sweet Tea Dorminy]

When fscrypt_infos can be owned by an extent, we need some way to
attempt to evict the extent infos in the same way we evict inodes.
This change adds a function pointer to allow a filesystem to optionally
provide a way to evict extents; locking if needed must be handled by the
filesystem.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/keyring.c     | 11 ++++++++++-
 include/linux/fscrypt.h |  9 +++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 7cbb1fd872ac..0aad825087c1 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -875,6 +875,16 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
 
 	list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
 		inode = ci->ci_inode;
+		if (!inode) {
+			if (!ci->ci_sb->s_cop->forget_extent_info)
+				continue;
+
+			spin_unlock(&mk->mk_decrypted_inodes_lock);
+			ci->ci_sb->s_cop->forget_extent_info(ci->ci_info_ptr);
+			spin_lock(&mk->mk_decrypted_inodes_lock);
+			continue;
+		}
+
 		spin_lock(&inode->i_lock);
 		if (inode->i_state & (I_FREEING | I_WILL_FREE | I_NEW)) {
 			spin_unlock(&inode->i_lock);
@@ -887,7 +897,6 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
 		shrink_dcache_inode(inode);
 		iput(toput_inode);
 		toput_inode = inode;
-
 		spin_lock(&mk->mk_decrypted_inodes_lock);
 	}
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index c895b12737a1..378a1f41c62f 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -129,6 +129,15 @@ struct fscrypt_operations {
 	 */
 	bool (*empty_dir)(struct inode *inode);
 
+	/*
+	 * Inform the filesystem that a particular extent must forget its
+	 * fscrypt_info (for instance, for a key removal).
+	 *
+	 * @info_ptr: a pointer to the location storing the fscrypt_info pointer
+	 *            within the opaque extent whose info is to be freed
+	 */
+	void (*forget_extent_info)(struct fscrypt_info **info_ptr);
+
 	/*
 	 * Check whether the filesystem's inode numbers and UUID are stable,
 	 * meaning that they will never be changed even by offline operations
-- 
2.40.1

[PATCH v1 08/12] fscrypt: use an optional ino equivalent for per-extent infos

[Sweet Tea Dorminy]

Since per-extent infos are not tied to inodes, an ino-based policy
cannot access the inode's i_ino to get the necessary information.
Instead, this adds an optional fscrypt_operation pointer to get the ino
equivalent for an extent, adds a wrapper to get the ino for an info, and
uses this wrapper everywhere where the ci's inode's i_ino is currently
accessed.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h | 18 ++++++++++++++++++
 fs/crypto/keyring.c         |  8 ++++----
 fs/crypto/keysetup.c        |  6 +++---
 include/linux/fscrypt.h     |  9 +++++++++
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 1674e66e72e3..8bf27ceeecd1 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -332,6 +332,24 @@ static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
 	return false;
 }
 
+/**
+ * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
+ *
+ * @ci: the fscrypt_info in question
+ *
+ * Return: For inode-based encryption, this will return the info's inode's ino.
+ * For extent-based encryption, this will return the extent's ino equivalent
+ * or 0 if it is not implemented.
+ */
+static inline u64 fscrypt_get_info_ino(const struct fscrypt_info *ci)
+{
+	if (ci->ci_inode)
+		return ci->ci_inode->i_ino;
+	if (!ci->ci_sb->s_cop->get_extent_ino_equivalent)
+		return 0;
+	return ci->ci_sb->s_cop->get_extent_ino_equivalent(ci->ci_info_ptr);
+}
+
 /* crypto.c */
 extern struct kmem_cache *fscrypt_info_cachep;
 int fscrypt_initialize(struct super_block *sb);
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0aad825087c1..bfcd2ecbe481 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -923,12 +923,12 @@ static int check_for_busy_inodes(struct super_block *sb,
 	}
 
 	{
-		/* select an example file to show for debugging purposes */
-		struct inode *inode =
+		/* select an example info to show for debugging purposes */
+		struct fscrypt_info *ci =
 			list_first_entry(&mk->mk_decrypted_inodes,
 					 struct fscrypt_info,
-					 ci_master_key_link)->ci_inode;
-		ino = inode->i_ino;
+					 ci_master_key_link);
+		ino = fscrypt_get_info_ino(ci);
 	}
 	spin_unlock(&mk->mk_decrypted_inodes_lock);
 
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 1aa5b2a0096e..f166fabb7f73 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -380,10 +380,10 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
 void fscrypt_hash_inode_number(struct fscrypt_info *ci,
 			       const struct fscrypt_master_key *mk)
 {
-	WARN_ON_ONCE(ci->ci_inode->i_ino == 0);
+	WARN_ON_ONCE(fscrypt_get_info_ino(ci) == 0);
 	WARN_ON_ONCE(!mk->mk_ino_hash_key_initialized);
 
-	ci->ci_hashed_ino = (u32)siphash_1u64(ci->ci_inode->i_ino,
+	ci->ci_hashed_ino = (u32)siphash_1u64(fscrypt_get_info_ino(ci),
 					      &mk->mk_ino_hash_key);
 }
 
@@ -706,7 +706,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 		if (res)
 			goto out;
 
-		if (inode->i_ino)
+		if (fscrypt_get_info_ino(crypt_info))
 			fscrypt_hash_inode_number(crypt_info, mk);
 	}
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 378a1f41c62f..22affbb15706 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -169,6 +169,15 @@ struct fscrypt_operations {
 	void (*get_ino_and_lblk_bits)(struct super_block *sb,
 				      int *ino_bits_ret, int *lblk_bits_ret);
 
+	/*
+	 * Get the inode number equivalent for filesystems using per-extent
+	 * encryption keys.
+	 *
+	 * This function only needs to be implemented if support for one of the
+	 * FSCRYPT_POLICY_FLAG_IV_INO_* flags is needed.
+	 */
+	u64 (*get_extent_ino_equivalent)(struct fscrypt_info **info_ptr);
+
 	/*
 	 * Return an array of pointers to the block devices to which the
 	 * filesystem may write encrypted file contents, NULL if the filesystem
-- 
2.40.1

[PATCH v1 09/12] fscrypt: add creation/usage/freeing of per-extent infos

[Sweet Tea Dorminy]

This change adds the superblock function pointer to get the info
corresponding to a specific block in an inode for a filesystem using
per-extent infos. It allows creating a info for a new extent and freeing
that info, and uses the extent's info if appropriate in encrypting
blocks of data.

This change does not deal with saving and loading an extent's info, but
introduces the mechanics necessary therefore.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/crypto.c          |  2 ++
 fs/crypto/fscrypt_private.h | 46 ++++++++++++++++++-----------
 fs/crypto/keyring.c         | 15 +++++++++-
 fs/crypto/keysetup.c        | 59 +++++++++++++++++++++++++++++++++++--
 include/linux/fscrypt.h     | 39 ++++++++++++++++++++++++
 5 files changed, 140 insertions(+), 21 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index d75f1b3f5795..0f0c721e40fe 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -113,6 +113,8 @@ int fscrypt_crypt_block(const struct inode *inode, fscrypt_direction_t rw,
 	struct crypto_skcipher *tfm = ci->ci_enc_key->tfm;
 	int res = 0;
 
+	if (!ci)
+		return -EINVAL;
 	if (WARN_ON_ONCE(len <= 0))
 		return -EINVAL;
 	if (WARN_ON_ONCE(len % FSCRYPT_CONTENTS_ALIGNMENT != 0))
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 8bf27ceeecd1..5eafa50a3298 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -286,6 +286,23 @@ typedef enum {
 	FS_ENCRYPT,
 } fscrypt_direction_t;
 
+/**
+ * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
+ *				       encryption
+ *
+ * @inode: the inode in question
+ *
+ * Return: true if the inode uses per-extent fscrypt_infos, false otherwise
+ */
+static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
+{
+	// Non-regular files don't have extents
+	if (!S_ISREG(inode->i_mode))
+		return false;
+
+	return !!inode->i_sb->s_cop->get_extent_info;
+}
+
 /**
  * fscrypt_get_lblk_info() - get the fscrypt_info to crypt a particular block
  *
@@ -306,6 +323,17 @@ static inline struct fscrypt_info *
 fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
 		      u64 *extent_len)
 {
+	if (fscrypt_uses_extent_encryption(inode)) {
+		struct fscrypt_info *info;
+		int res;
+
+		res = inode->i_sb->s_cop->get_extent_info(inode, lblk, &info,
+							  offset, extent_len);
+		if (res == 0)
+			return info;
+		return NULL;
+	}
+
 	if (offset)
 		*offset = lblk;
 	if (extent_len)
@@ -314,24 +342,6 @@ fscrypt_get_lblk_info(const struct inode *inode, u64 lblk, u64 *offset,
 	return inode->i_crypt_info;
 }
 
-/**
- * fscrypt_uses_extent_encryption() -- whether an inode uses per-extent
- *				       encryption
- *
- * @inode: the inode in question
- *
- * Return: true if the inode uses per-extent fscrypt_infos, false otherwise
- */
-static inline bool fscrypt_uses_extent_encryption(const struct inode *inode)
-{
-	// Non-regular files don't have extents
-	if (!S_ISREG(inode->i_mode))
-		return false;
-
-	// No filesystems currently use per-extent infos
-	return false;
-}
-
 /**
  * fscrypt_get_info_ino() - get the ino or ino equivalent for an info
  *
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index bfcd2ecbe481..93f99b0fc20a 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -875,7 +875,7 @@ static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
 
 	list_for_each_entry(ci, &mk->mk_decrypted_inodes, ci_master_key_link) {
 		inode = ci->ci_inode;
-		if (!inode) {
+		if (ci->ci_info_ptr) {
 			if (!ci->ci_sb->s_cop->forget_extent_info)
 				continue;
 
@@ -1036,6 +1036,19 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
 	mk = fscrypt_find_master_key(sb, &arg.key_spec);
 	if (!mk)
 		return -ENOKEY;
+
+	/*
+	 * For filesystems with extent encryption, there may be extents which
+	 * need to come into existence and get the key, lest their data be
+	 * stuck in memory and not be flushable for lack of key. So just sync
+	 * the filesystem to encourage all the dirty pages to be written out.
+	 */
+	if (sb->s_cop->get_extent_info) {
+		down_read(&sb->s_umount);
+		sync_filesystem(sb);
+		up_read(&sb->s_umount);
+	}
+
 	down_write(&mk->mk_sem);
 
 	/* If relevant, remove current user's (or all users) claim to the key */
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index f166fabb7f73..0efa2800b50d 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -647,8 +647,8 @@ fscrypt_setup_encryption_info(struct inode *inode,
 
 	if (fscrypt_uses_extent_encryption(inode) && info_for_extent)
 		crypt_info->ci_info_ptr = info_ptr;
-	else
-		crypt_info->ci_inode = inode;
+
+	crypt_info->ci_inode = inode;
 
 	crypt_info->ci_sb = inode->i_sb;
 	crypt_info->ci_policy = *policy;
@@ -662,6 +662,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	WARN_ON_ONCE(mode->ivsize > FSCRYPT_MAX_IV_SIZE);
 	crypt_info->ci_mode = mode;
 
+	pr_err("fsei1");
 	res = fscrypt_select_encryption_impl(crypt_info);
 	if (res)
 		goto out;
@@ -888,6 +889,60 @@ int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
 }
 EXPORT_SYMBOL_GPL(fscrypt_prepare_new_inode);
 
+/**
+ * fscrypt_prepare_new_extent() - set up the fscrypt_info for a new extent
+ * @inode: the inode to which the extent belongs
+ * @info_ptr: a pointer to return the extent's fscrypt_info into. Should be
+ *	      a pointer to a member of the extent struct, as it will be passed
+ *	      back to the filesystem if key removal demands removal of the
+ *	      info from the extent
+ * @encrypt_ret: (output) set to %true if the new inode will be encrypted
+ *
+ * If the extent is part of an encrypted inode, set up its fscrypt_info in
+ * preparation for encrypting data and set *encrypt_ret=true.
+ *
+ * This isn't %GFP_NOFS-safe, and therefore it should be called before starting
+ * any filesystem transaction to create the inode.
+ *
+ * This doesn't persist the new inode's encryption context.  That still needs to
+ * be done later by calling fscrypt_set_context().
+ *
+ * Return: 0 on success, -ENOKEY if the encryption key is missing, or another
+ *	   -errno code
+ */
+int fscrypt_prepare_new_extent(struct inode *inode,
+			       struct fscrypt_info **info_ptr)
+{
+	const union fscrypt_policy *policy;
+	u8 nonce[FSCRYPT_FILE_NONCE_SIZE];
+
+	policy = fscrypt_policy_to_inherit(inode);
+	if (policy == NULL)
+		return 0;
+	if (IS_ERR(policy))
+		return PTR_ERR(policy);
+
+	/* Only regular files can have extents.  */
+	if (WARN_ON_ONCE(!S_ISREG(inode->i_mode)))
+		return -EINVAL;
+
+	get_random_bytes(nonce, FSCRYPT_FILE_NONCE_SIZE);
+	return fscrypt_setup_encryption_info(inode, policy, nonce,
+					     false, info_ptr);
+}
+EXPORT_SYMBOL_GPL(fscrypt_prepare_new_extent);
+
+/**
+ * fscrypt_free_extent_info() - free an extent's fscrypt_info
+ * @info_ptr: a pointer containing the extent's fscrypt_info pointer.
+ */
+void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
+{
+	put_crypt_info(*info_ptr);
+	*info_ptr = NULL;
+}
+EXPORT_SYMBOL_GPL(fscrypt_free_extent_info);
+
 /**
  * fscrypt_put_encryption_info() - free most of an inode's fscrypt data
  * @inode: an inode being evicted
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 22affbb15706..1965b900f62f 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -113,6 +113,29 @@ struct fscrypt_operations {
 	int (*set_context)(struct inode *inode, const void *ctx, size_t len,
 			   void *fs_data);
 
+	/*
+	 * Get the fscrypt_info for the given inode at the given block, for
+	 * extent-based encryption only.
+	 *
+	 * @inode: the inode in question
+	 * @lblk: the logical block number in question
+	 * @ci: a pointer to return the fscrypt_info
+	 * @offset: a pointer to return the offset of @lblk into the extent,
+	 *          in blocks (may be NULL)
+	 * @extent_len: a pointer to return the number of blocks in this extent
+	 *              starting at this point (may be NULL)
+	 *
+	 * May cause the filesystem to allocate memory, which the filesystem
+	 * must do with %GFP_NOFS, including calls into fscrypt to create or
+	 * load an fscrypt_info.
+	 *
+	 * Return: 0 if an extent is found with an info, -ENODATA if the key is
+	 *         unavailable, or another -errno.
+	 */
+	int (*get_extent_info)(const struct inode *inode, u64 lblk,
+			       struct fscrypt_info **ci, u64 *offset,
+			       u64 *extent_len);
+
 	/*
 	 * Get the dummy fscrypt policy in use on the filesystem (if any).
 	 *
@@ -348,6 +371,10 @@ void fscrypt_put_encryption_info(struct inode *inode);
 void fscrypt_free_inode(struct inode *inode);
 int fscrypt_drop_inode(struct inode *inode);
 
+int fscrypt_prepare_new_extent(struct inode *inode,
+			       struct fscrypt_info **info_ptr);
+void fscrypt_free_extent_info(struct fscrypt_info **info_ptr);
+
 /* fname.c */
 int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
 			  u8 *out, unsigned int olen);
@@ -609,6 +636,18 @@ static inline int fscrypt_drop_inode(struct inode *inode)
 	return 0;
 }
 
+static inline int fscrypt_prepare_new_extent(struct inode *inode,
+					     struct fscrypt_info **info_ptr)
+{
+	if (IS_ENCRYPTED(inode))
+		return -EOPNOTSUPP;
+	return 0;
+}
+
+static inline void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
+{
+}
+
  /* fname.c */
 static inline int fscrypt_setup_filename(struct inode *dir,
 					 const struct qstr *iname,
-- 
2.40.1

Re: [PATCH v1 09/12] fscrypt: add creation/usage/freeing of per-extent infos

[Sweet Tea Dorminy]

> +	pr_err("fsei1");
Missed this bit of debug; will be fixed in the next version.

[PATCH v1 10/12] fscrypt: allow load/save of extent contexts

[Sweet Tea Dorminy]

The other half of using per-extent infos is saving and loading them from
disk.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/keysetup.c    | 50 ++++++++++++++++++++++++++++++++++++++++-
 fs/crypto/policy.c      | 20 +++++++++++++++++
 include/linux/fscrypt.h | 17 ++++++++++++++
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 0efa2800b50d..50fa8955ae2c 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -819,7 +819,6 @@ int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 					    IS_CASEFOLDED(inode) &&
 					    S_ISDIR(inode->i_mode),
 					    NULL);
-
 	if (res == -ENOPKG && allow_unsupported) /* Algorithm unavailable? */
 		res = 0;
 	if (res == -ENOKEY)
@@ -943,6 +942,55 @@ void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
 }
 EXPORT_SYMBOL_GPL(fscrypt_free_extent_info);
 
+/**
+ * fscrypt_load_extent_info() - set up a preexisting extent's fscrypt_info
+ * @inode: the inode to which the extent belongs. Must be encrypted.
+ * @buf: a buffer containing the extent's stored context
+ * @len: the length of the @ctx buffer
+ * @info_ptr: a pointer to return the extent's fscrypt_info into. Should be
+ *	      a pointer to a member of the extent struct, as it will be passed
+ *	      back to the filesystem if key removal demands removal of the
+ *	      info from the extent
+ *
+ * This is not %GFP_NOFS safe, so the caller is expected to call
+ * memalloc_nofs_save/restore() if appropriate.
+ *
+ * Return: 0 if successful, or -errno if it fails.
+ */
+int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
+			     struct fscrypt_info **info_ptr)
+{
+	int res;
+	union fscrypt_context ctx;
+	union fscrypt_policy policy;
+
+	if (!fscrypt_has_encryption_key(inode))
+		return -EINVAL;
+
+	memcpy(&ctx, buf, len);
+
+	res = fscrypt_policy_from_context(&policy, &ctx, len);
+	if (res) {
+		fscrypt_warn(inode,
+			     "Unrecognized or corrupt encryption context");
+		return res;
+	}
+
+	if (!fscrypt_supported_policy(&policy, inode))
+		return -EINVAL;
+
+	res = fscrypt_setup_encryption_info(inode, &policy,
+					    fscrypt_context_nonce(&ctx),
+					    IS_CASEFOLDED(inode) &&
+					    S_ISDIR(inode->i_mode),
+					    info_ptr);
+
+	if (res == -ENOPKG) /* Algorithm unavailable? */
+		res = 0;
+	return res;
+}
+EXPORT_SYMBOL_GPL(fscrypt_load_extent_info);
+
 /**
  * fscrypt_put_encryption_info() - free most of an inode's fscrypt data
  * @inode: an inode being evicted
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index f4456ecb3f87..9ecb01e49d33 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -762,6 +762,26 @@ int fscrypt_set_context(struct inode *inode, void *fs_data)
 }
 EXPORT_SYMBOL_GPL(fscrypt_set_context);
 
+/**
+ * fscrypt_set_extent_context() - Set the fscrypt context for an extent
+ * @ci: info from which to fetch policy and nonce
+ * @ctx: where context should be written
+ * @len: the size of ctx
+ *
+ * Given an fscrypt_info belonging to an extent (generated via
+ * fscrypt_prepare_new_extent()), generate a new context and write it to @ctx.
+ * len is checked to be at least FSCRYPT_SET_CONTEXT_MAX_SIZE bytes.
+ *
+ * Return: size of the resulting context or a negative error code.
+ */
+int fscrypt_set_extent_context(struct fscrypt_info *ci, void *ctx, size_t len)
+{
+	if (len < FSCRYPT_SET_CONTEXT_MAX_SIZE)
+		return -EINVAL;
+	return fscrypt_new_context(ctx, &ci->ci_policy, ci->ci_nonce);
+}
+EXPORT_SYMBOL_GPL(fscrypt_set_extent_context);
+
 /**
  * fscrypt_parse_test_dummy_encryption() - parse the test_dummy_encryption mount option
  * @param: the mount option
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 1965b900f62f..9c7255b48f1d 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -334,6 +334,8 @@ int fscrypt_ioctl_get_nonce(struct file *filp, void __user *arg);
 int fscrypt_has_permitted_context(struct inode *parent, struct inode *child);
 int fscrypt_context_for_new_inode(void *ctx, struct inode *inode);
 int fscrypt_set_context(struct inode *inode, void *fs_data);
+int fscrypt_set_extent_context(struct fscrypt_info *info, void *ctx,
+			       size_t len);
 
 struct fscrypt_dummy_policy {
 	const union fscrypt_policy *policy;
@@ -374,6 +376,8 @@ int fscrypt_drop_inode(struct inode *inode);
 int fscrypt_prepare_new_extent(struct inode *inode,
 			       struct fscrypt_info **info_ptr);
 void fscrypt_free_extent_info(struct fscrypt_info **info_ptr);
+int fscrypt_load_extent_info(struct inode *inode, void *buf, size_t len,
+			     struct fscrypt_info **info_ptr);
 
 /* fname.c */
 int fscrypt_fname_encrypt(const struct inode *inode, const struct qstr *iname,
@@ -550,6 +554,12 @@ static inline int fscrypt_set_context(struct inode *inode, void *fs_data)
 	return -EOPNOTSUPP;
 }
 
+static inline int fscrypt_set_extent_context(struct fscrypt_info *info,
+					     void *ctx, size_t len)
+{
+	return -EOPNOTSUPP;
+}
+
 struct fscrypt_dummy_policy {
 };
 
@@ -648,6 +658,13 @@ static inline void fscrypt_free_extent_info(struct fscrypt_info **info_ptr)
 {
 }
 
+static inline int fscrypt_load_extent_info(struct inode *inode, void *buf,
+					   size_t len,
+					   struct fscrypt_info **info_ptr)
+{
+	return -EOPNOTSUPP;
+}
+
  /* fname.c */
 static inline int fscrypt_setup_filename(struct inode *dir,
 					 const struct qstr *iname,
-- 
2.40.1

[PATCH v1 11/12] fscrypt: save session key credentials for extent infos

[Sweet Tea Dorminy]

For v1 encryption policies using per-session keys, the thread which
opens the inode and therefore initializes the encryption info is part of
the session, so it can get the key from the session keyring. However,
for extent encryption, the extent infos are likely loaded from a
different thread, which does not have access to the session keyring.
This change saves the credentials of the inode opening thread and reuses
those credentials temporarily when dealing with extent infos, allowing
finding the encryption key correctly.

v1 encryption policies using per-session keys should probably not exist
for new usages such as extent encryption, but this makes more tests
work without change; maybe the right answer is to disallow v1 session
keys plus extent encryption and deal with editing tests to not use v1
session encryption so much.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/fscrypt_private.h |  8 ++++++++
 fs/crypto/keysetup.c        | 13 +++++++++++++
 fs/crypto/keysetup_v1.c     |  1 +
 3 files changed, 22 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5eafa50a3298..5c7649717aac 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -231,6 +231,14 @@ struct fscrypt_info {
 	 */
 	bool ci_inlinecrypt;
 #endif
+	/* Credential struct from the thread which created this info. This is
+	 * only used in v1 session keyrings with extent encryption; it allows
+	 * the thread creating extents for an inode to join the session
+	 * keyring temporarily, since otherwise the thread is usually part of
+	 * kernel writeback and therefore unrelated to the thread with the
+	 * right session key.
+	 */
+	struct cred *ci_session_creds;
 
 	/*
 	 * Encryption mode used for this inode.  It corresponds to either the
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 50fa8955ae2c..cfe0f93919fe 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -617,6 +617,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
 		spin_unlock(&mk->mk_decrypted_inodes_lock);
 		fscrypt_put_master_key_activeref(ci->ci_sb, mk);
 	}
+	if (ci->ci_session_creds)
+		abort_creds(ci->ci_session_creds);
 	memzero_explicit(ci, sizeof(*ci));
 	kmem_cache_free(fscrypt_info_cachep, ci);
 }
@@ -633,6 +635,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	struct fscrypt_master_key *mk = NULL;
 	int res;
 	bool info_for_extent = !!info_ptr;
+	const struct cred *creds = NULL;
 
 	if (!info_ptr)
 		info_ptr = &inode->i_crypt_info;
@@ -677,7 +680,17 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	if (res)
 		goto out;
 
+	if (info_for_extent && inode->i_crypt_info->ci_session_creds)
+		 creds = override_creds(inode->i_crypt_info->ci_session_creds);
+		/*
+		 * The inode this is being created for is using a session key,
+		 * so we have to join this thread to that session temporarily
+		 * in order to be able to find the right key...
+		 */
+
 	res = fscrypt_setup_file_key(crypt_info, mk);
+	if (creds)
+		revert_creds(creds);
 	if (res)
 		goto out;
 
diff --git a/fs/crypto/keysetup_v1.c b/fs/crypto/keysetup_v1.c
index 41d317f08aeb..4f2be2377dfa 100644
--- a/fs/crypto/keysetup_v1.c
+++ b/fs/crypto/keysetup_v1.c
@@ -318,6 +318,7 @@ int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci)
 		return PTR_ERR(key);
 
 	err = fscrypt_setup_v1_file_key(ci, payload->raw);
+	ci->ci_session_creds = prepare_creds();
 	up_read(&key->sem);
 	key_put(key);
 	return err;
-- 
2.40.1

[PATCH v1 12/12] fscrypt: update documentation for per-extent keys

[Sweet Tea Dorminy]

Add some documentation of how extent-based encryption works, hopefully
enough for future filesystem users.

Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 Documentation/filesystems/fscrypt.rst | 38 ++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index eccd327e6df5..9bbfcd84a29f 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -31,7 +31,7 @@ However, except for filenames, fscrypt does not encrypt filesystem
 metadata.
 
 Unlike eCryptfs, which is a stacked filesystem, fscrypt is integrated
-directly into supported filesystems --- currently ext4, F2FS, and
+directly into supported filesystems --- currently btrfs, ext4, F2FS, and
 UBIFS.  This allows encrypted files to be read and written without
 caching both the decrypted and encrypted pages in the pagecache,
 thereby nearly halving the memory used and bringing it in line with
@@ -280,6 +280,11 @@ included in the IV.  Moreover:
   key derived using the KDF.  Users may use the same master key for
   other v2 encryption policies.
 
+For filesystems with extent-based content encryption (e.g. btrfs),
+this is the only choice. Data shared among multiple inodes must share
+the exact same key, therefore necessitating inodes using the same key
+for contents encryption.
+
 IV_INO_LBLK_64 policies
 -----------------------
 
@@ -381,12 +386,13 @@ to individual filesystems.  However, authenticated encryption (AE)
 modes are not currently supported because of the difficulty of dealing
 with ciphertext expansion.
 
-Contents encryption
--------------------
+Inode-based contents encryption
+-------------------------------
 
-For file contents, each filesystem block is encrypted independently.
-Starting from Linux kernel 5.5, encryption of filesystems with block
-size less than system's page size is supported.
+Most filesystems use the previously discussed per-file keys. For these
+filesystems, for file contents, each filesystem block is encrypted
+independently.  Starting from Linux kernel 5.5, encryption of filesystems
+with block size less than system's page size is supported.
 
 Each block's IV is set to the logical block number within the file as
 a little endian number, except that:
@@ -410,6 +416,26 @@ Note that because file logical block numbers are included in the IVs,
 filesystems must enforce that blocks are never shifted around within
 encrypted files, e.g. via "collapse range" or "insert range".
 
+Extent-based contents encryption
+--------------------------------
+
+For certain filesystems (currently only btrfs), data is encrypted on a
+per-extent basis, for whatever the filesystem's notion of an extent is. The
+scheme is exactly as with inode-based contents encryption, except that the
+'inode number' for an extent is requested from the filesystem instead of from
+the file's inode, and the 'logical block number' refers to an offset within the
+extent.
+
+Because the encryption material is per-extent instead of per-inode, as long
+as the extent's encryption context does not change, the filesystem may shift
+around the position of the extent, and may have multiple files referring to
+the same encrypted extent.
+
+Not all extents within a file are decrypted simultaneously, so it is possible
+for a file read to fail partway through the file if it crosses into an extent
+whose key is unavailable.  However, all writes will succeed, unless the key is
+removed mid-write.
+
 Filenames encryption
 --------------------
 
-- 
2.40.1

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Eric Biggers]

Hi Sweet Tea,

On Wed, Jun 28, 2023 at 08:29:30PM -0400, Sweet Tea Dorminy wrote:
> This changeset adds extent-based data encryption to fscrypt.
> Some filesystems need to encrypt data based on extents, rather than on
> inodes, due to features incompatible with inode-based encryption. For
> instance, btrfs can have multiple inodes referencing a single block of
> data, and moves logical data blocks to different physical locations on
> disk in the background. 
> 
> As per discussion last year in [1] and later in [2], we would like to
> allow the use of fscrypt with btrfs, with authenticated encryption. This
> is the first step of that work, adding extent-based encryption to
> fscrypt; authenticated encryption is the next step. Extent-based
> encryption should be usable by other filesystems which wish to support
> snapshotting or background data rearrangement also, but btrfs is the
> first user. 
> 
> This changeset requires extent encryption to use inlinecrypt, as
> discussed previously. There are two questionable parts: the
> forget_extent_info hook is not yet in use by btrfs, as I haven't yet
> written a test exercising a race where it would be relevant; and saving
> the session key credentials just to enable v1 session-based policies is
> perhaps less good than 
> 
> This applies atop [3], which itself is based on kdave/misc-next. It
> passes most encryption fstests with suitable changes to btrfs-progs, but
> not generic/580 or generic/595 due to different timing involved in
> extent encryption. Tests and btrfs progs updates to follow.
> 
> 
> [1] https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
> [2] https://lore.kernel.org/linux-fscrypt/80496cfe-161d-fb0d-8230-93818b966b1b@dorminy.me/T/#t
> [3]
> https://lore.kernel.org/linux-fscrypt/cover.1687988119.git.sweettea-kernel@dorminy.me/
> 
> Sweet Tea Dorminy (12):
>   fscrypt: factor helper for locking master key
>   fscrypt: factor getting info for a specific block
>   fscrypt: adjust effective lblks based on extents
>   fscrypt: add a super_block pointer to fscrypt_info
>   fscrypt: setup leaf inodes for extent encryption
>   fscrypt: allow infos to be owned by extents
>   fscrypt: notify per-extent infos if master key vanishes
>   fscrypt: use an optional ino equivalent for per-extent infos
>   fscrypt: add creation/usage/freeing of per-extent infos
>   fscrypt: allow load/save of extent contexts
>   fscrypt: save session key credentials for extent infos
>   fscrypt: update documentation for per-extent keys
> 
>  Documentation/filesystems/fscrypt.rst |  38 +++-
>  fs/crypto/crypto.c                    |   6 +-
>  fs/crypto/fscrypt_private.h           |  91 ++++++++++
>  fs/crypto/inline_crypt.c              |  28 ++-
>  fs/crypto/keyring.c                   |  32 +++-
>  fs/crypto/keysetup.c                  | 244 ++++++++++++++++++++++----
>  fs/crypto/keysetup_v1.c               |   7 +-
>  fs/crypto/policy.c                    |  20 +++
>  include/linux/fscrypt.h               |  74 ++++++++
>  9 files changed, 480 insertions(+), 60 deletions(-)
> 
> 
> base-commit: accadeb67609a5a5d088ebde8409c3f6db0b84b4

Thanks for sending this out!

It's going to take me a while to go through everything, so please bear with me.
In general I'd also really like to be seeing more feedback from the other btrfs
developers.  This is a hard project that really needs more eyes on it.

From a brief look through your patchsets, there's one thing I want to bring up
right away.  It seems that one important design choice that you've made that has
impacted much of your patchsets is that you've made each extent a fully
standalone thing, similar to inodes currently.  I.e.,

    (a) Each extent gets a full 'fscrypt_context' stored along with it.  That
        includes not just the nonce, but also the encryption modes and master
        key identifier.

    (b) For runtime caching, each extent gets a full 'struct fscrypt_info'
        object.  It doesn't "belong" to any inode; it's set up in a fully
        standalone way, and the master key lookup and removal logic operates
        directly on the extent's 'struct fscrypt_info'.

I'm not sure this is a good idea.  What I had thought it was going to look like
is that the encryption context/policy and 'struct fscrypt_info' would stay a
property of the inode, and the extents themselves would be much more lightweight
-- both on disk and in the cache.  On-disk, all that should really be needed for
an extent is the nonce for deriving the per-extent key.  And in-memory, all that
should really be needed is a "fscrypt_prepared_key" for the per-extent key, and
a reference to the owning inode.

I think that would avoid many of the problems that it seems you've had to work
around or have had to change user-visible semantics for.  For example the
problems involving master keys being added and removed.  It would also avoid
having to overload 'fscrypt_info' to be either a per-inode or a per-extent key.
And it would save space on disk and in memory.

Can you elaborate on why you went with a more "heavyweight" extents design?

Maybe your motivation is that extents can be referenced by more than one inode
and thus do not have a unique owning inode?  That's true, but I don't think that
really matters.  All the inodes that reference an extent will have the same
encryption policy, right?  Also, it looks like the "struct extent_map" that
you're caching the per-extent key in is already cached on a per-inode basis, in
btrfs_inode::extent_tree, similar to the pagecache which is also per-inode.  So
if the same extent happens to be accessed via multiple inodes, that's still
going to cause the fscrypt key to be set up twice anyway.

- Eric

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Sweet Tea Dorminy]

>> base-commit: accadeb67609a5a5d088ebde8409c3f6db0b84b4
> 
> Thanks for sending this out!
> 
> It's going to take me a while to go through everything, so please bear with me.
> In general I'd also really like to be seeing more feedback from the other btrfs
> developers.  This is a hard project that really needs more eyes on it.

I appreciate your time on it!
> 
>  From a brief look through your patchsets, there's one thing I want to bring up
> right away.  It seems that one important design choice that you've made that has
> impacted much of your patchsets is that you've made each extent a fully
> standalone thing, similar to inodes currently.  I.e.,
> 
>      (a) Each extent gets a full 'fscrypt_context' stored along with it.  That
>          includes not just the nonce, but also the encryption modes and master
>          key identifier.
> 
>      (b) For runtime caching, each extent gets a full 'struct fscrypt_info'
>          object.  It doesn't "belong" to any inode; it's set up in a fully
>          standalone way, and the master key lookup and removal logic operates
>          directly on the extent's 'struct fscrypt_info'.
> 
> I'm not sure this is a good idea.  What I had thought it was going to look like
> is that the encryption context/policy and 'struct fscrypt_info' would stay a
> property of the inode, and the extents themselves would be much more lightweight
> -- both on disk and in the cache.  On-disk, all that should really be needed for
> an extent is the nonce for deriving the per-extent key.  And in-memory, all that
> should really be needed is a "fscrypt_prepared_key" for the per-extent key, and
> a reference to the owning inode.
 >

The in memory reduction is plausible. For extents that are in memory but 
not yet written to disk, we need some way to keep track of the context, 
but we could drop the nonce/policy after that. I was aiming to have the 
same structure so that there's maximal similarity in info creation and 
things like fscrypt_generate_iv would always be getting an info, 
regardless of inode vs extent, but we could throw a conditional in there 
and create a different structure for in-memory extent infos.

However, it seems like an extent and a leaf inode in inode fscrypt need 
the same information, so if splitting the fscrypt_info structure makes 
sense, maybe it should be on that boundary?

> 
> I think that would avoid many of the problems that it seems you've had to work
> around or have had to change user-visible semantics for.  For example the
> problems involving master keys being added and removed.  It would also avoid
> having to overload 'fscrypt_info' to be either a per-inode or a per-extent key.
> And it would save space on disk and in memory.

I might be misunderstanding what you're referencing, but I think you're 
talking about the change where with extent fscrypt, IO has to be forced 
down before removing a key, otherwise it is lost. I think that's a 
fundamental problem given the filesystem has no way to know that there 
are new, dirty pages in the pagecache until those pages are issued for 
write, so it can't create a new extent or few until that point, 
potentially after the relevant key has been evicted. But maybe I'm 
missing a hook that would let us make extents earlier.

I suppose we could give each leaf inode a proper nonce/prepared key 
instead of borrowing its parent dir's: if a write came in after the key 
was removed but the inode is still open, the new extent(s) could grab 
the key material out of the inode's info. I don't like this very much 
since it could result in multiple extents grabbing the same key 
material, but I suppose it could work if it's important to maintain that 
behavior.
> 
> Can you elaborate on why you went with a more "heavyweight" extents design?
Being able to rekey a directory is the reason for having full contexts: 
suppose I take a snapshot of an encrypted dir and want to change the key 
for new data going forward, to avoid using a single key on too much 
data. It's too expensive to reencrypt every extent with the new key, 
since the whole point of a snapshot is to make a lightweight copy that 
gets COWed on write. Then each extent needs to know what its own master 
key identifier/policy/flags are, since different extents in the same 
file could have different master keys. We could say the mode and flags 
have to match, but it doesn't seem to me worth saving seven bytes to add 
a new structure to just store the master key identifier and nonce.

For a non-Meta usecase, from what I've heard from Fedora-land, it's 
possibly interesting to them to be able to ship an encrypted image, and 
then be able to change the key after encrypted install to something 
user-controlled.

Without rekeying, my understanding is that we may write too much data 
with one key for safety; notes in the updated design doc 
https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing 
are that writing more than 1P per key raises cryptographic concerns, and 
since btrfs is COW and could have volumes up to the full 16E size that 
btrfs supports, we don't want to have just one immutable key per subvol.

To me the lightweight-on-disk vision sounds a lot like the original 
design: 
https://lore.kernel.org/linux-btrfs/YXGyq+buM79A1S0L@relinquished.localdomain 
and the Nov '22 version of the patchset: 
https://lore.kernel.org/linux-btrfs/cover.1667389115.git.sweettea-kernel@dorminy.me/ 
(which didn't have rekeying). I think rekeying is worth the higher disk 
usage; but I'm probably missing something about how your vision differs 
from the original. Could you please take a look at it again?

> 
> Maybe your motivation is that extents can be referenced by more than one inode
> and thus do not have a unique owning inode?  That's true, but I don't think that
> really matters.  All the inodes that reference an extent will have the same
> encryption policy, right? 
As above, not necessarily

> Also, it looks like the "struct extent_map" that
> you're caching the per-extent key in is already cached on a per-inode basis, in
> btrfs_inode::extent_tree, similar to the pagecache which is also per-inode.  So
> if the same extent happens to be accessed via multiple inodes, that's still
> going to cause the fscrypt key to be set up twice anyway.

A good point, and if you want me to take advantage of the 
one-copy-per-inode fact for general extent-based fscrypt I can do so.

Many thanks!

Sweet Tea

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Eric Biggers]

On Mon, Jul 03, 2023 at 01:06:17PM -0400, Sweet Tea Dorminy wrote:
> > 
> > I think that would avoid many of the problems that it seems you've had to work
> > around or have had to change user-visible semantics for.  For example the
> > problems involving master keys being added and removed.  It would also avoid
> > having to overload 'fscrypt_info' to be either a per-inode or a per-extent key.
> > And it would save space on disk and in memory.
> 
> I might be misunderstanding what you're referencing, but I think you're
> talking about the change where with extent fscrypt, IO has to be forced down
> before removing a key, otherwise it is lost. I think that's a fundamental
> problem given the filesystem has no way to know that there are new, dirty
> pages in the pagecache until those pages are issued for write, so it can't
> create a new extent or few until that point, potentially after the relevant
> key has been evicted. But maybe I'm missing a hook that would let us make
> extents earlier.
> 
> I suppose we could give each leaf inode a proper nonce/prepared key instead
> of borrowing its parent dir's: if a write came in after the key was removed
> but the inode is still open, the new extent(s) could grab the key material
> out of the inode's info. I don't like this very much since it could result
> in multiple extents grabbing the same key material, but I suppose it could
> work if it's important to maintain that behavior.

Right, if extent keys are derived directly from the master key, and
FS_IOC_REMOVE_ENCRYPTION_KEY is executed which causes the master key secret to
be wiped, then no more extent keys can be derived.

So I see why you implemented the behavior you did.  It does seem a bit
dangerous, though.  If I understand correctly, under your proposal, if
FS_IOC_REMOVE_ENCRYPTION_KEY is executed while a process is doing buffered
writes to an encrypted file protected by that key, then past writes will get
synced out just before the key removal, but future writes will just be silently
thrown away.  (Remember, writes in Linux are asynchronous; processes don't get
informed of write errors unless they call fsync() or are doing direct I/O.)

I wonder if we should just keep the master key around, for per-extent key
derivation only, until all files using that master key have been closed.  That
would minimize the changes from the current fscrypt semantics.

> > Can you elaborate on why you went with a more "heavyweight" extents design?
> Being able to rekey a directory is the reason for having full contexts:
> suppose I take a snapshot of an encrypted dir and want to change the key for
> new data going forward, to avoid using a single key on too much data. It's
> too expensive to reencrypt every extent with the new key, since the whole
> point of a snapshot is to make a lightweight copy that gets COWed on write.
> Then each extent needs to know what its own master key
> identifier/policy/flags are, since different extents in the same file could
> have different master keys. We could say the mode and flags have to match,
> but it doesn't seem to me worth saving seven bytes to add a new structure to
> just store the master key identifier and nonce.
> 
> For a non-Meta usecase, from what I've heard from Fedora-land, it's possibly
> interesting to them to be able to ship an encrypted image, and then be able
> to change the key after encrypted install to something user-controlled.
> 
> Without rekeying, my understanding is that we may write too much data with
> one key for safety; notes in the updated design doc https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
> are that writing more than 1P per key raises cryptographic concerns, and
> since btrfs is COW and could have volumes up to the full 16E size that btrfs
> supports, we don't want to have just one immutable key per subvol.
> 
> To me the lightweight-on-disk vision sounds a lot like the original design:
> https://lore.kernel.org/linux-btrfs/YXGyq+buM79A1S0L@relinquished.localdomain
> and the Nov '22 version of the patchset: https://lore.kernel.org/linux-btrfs/cover.1667389115.git.sweettea-kernel@dorminy.me/
> (which didn't have rekeying). I think rekeying is worth the higher disk
> usage; but I'm probably missing something about how your vision differs from
> the original. Could you please take a look at it again?

Your original design didn't use per-extent keys, but rather had a single
contents encryption key per master key.  We had discussed that that approach had
multiple disadvantages, one of which is that on btrfs it can run uncomfortably
close to the cryptographic limits for the contents encryption modes such as
AES-XTS.  So we decided to go with per-extent keys instead.

I don't think we discussed cryptographic limits on the master key itself.  That
is actually much less of a concern, as the master key is just used for
HKDF-SHA512.  I don't think there is any real need to ever change the master
key.  (Well, if it is compromised, it could be needed, but that's not really
relevant here.  If that happens you'd need to re-encrypt everything anyway.)

I do recall some discussion of making it possible to set an encryption policy on
an *unencrypted* directory, causing new files in that directory to be encrypted.
However, I don't recall any discussion of making it possible to add another
encryption policy to an *already-encrypted* directory.  I think this is the
first time this has been brought up.

I think that allowing directories to have multiple encryption policies would
bring in a lot of complexity.  How would it be configured, and what would the
semantics for accessing it be?  Where would the encryption policies be stored?
What if you have added some of the keys but not all of them?  What if some of
the keys get removed but not all of them?

Can you elaborate more on why you want this?  I was already a bit concerned
about the plan for making it possible to set an encryption policy on an
unencrypted directory, as that already diverges from the existing fscrypt
semantics.  But now it sounds like the scope has grown even more.

Keep in mind that in general, the closer we are able to stick to the existing
fscrypt semantics and design, the easier it is going to be to get the initial
btrfs fscrypt support merged.  Planning ahead for new features is good, but we
should also be careful not to overdesign.

- Eric

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Sweet Tea Dorminy]

On 7/3/23 14:17, Eric Riggers wrote:
> On Mon, Jul 03, 2023 at 01:06:17PM -0400, Sweet Tea Dorminy wrote:
>>>
>>> I think that would avoid many of the problems that it seems you've had to work
>>> around or have had to change user-visible semantics for.  For example the
>>> problems involving master keys being added and removed.  It would also avoid
>>> having to overload 'fscrypt_info' to be either a per-inode or a per-extent key.
>>> And it would save space on disk and in memory.
>>
>> I might be misunderstanding what you're referencing, but I think you're
>> talking about the change where with extent fscrypt, IO has to be forced down
>> before removing a key, otherwise it is lost. I think that's a fundamental
>> problem given the filesystem has no way to know that there are new, dirty
>> pages in the pagecache until those pages are issued for write, so it can't
>> create a new extent or few until that point, potentially after the relevant
>> key has been evicted. But maybe I'm missing a hook that would let us make
>> extents earlier.
>>
>> I suppose we could give each leaf inode a proper nonce/prepared key instead
>> of borrowing its parent dir's: if a write came in after the key was removed
>> but the inode is still open, the new extent(s) could grab the key material
>> out of the inode's info. I don't like this very much since it could result
>> in multiple extents grabbing the same key material, but I suppose it could
>> work if it's important to maintain that behavior.
> 
> Right, if extent keys are derived directly from the master key, and
> FS_IOC_REMOVE_ENCRYPTION_KEY is executed which causes the master key secret to
> be wiped, then no more extent keys can be derived.
> 
> So I see why you implemented the behavior you did.  It does seem a bit
> dangerous, though.  If I understand correctly, under your proposal, if
> FS_IOC_REMOVE_ENCRYPTION_KEY is executed while a process is doing buffered
> writes to an encrypted file protected by that key, then past writes will get
> synced out just before the key removal, but future writes will just be silently
> thrown away.  (Remember, writes in Linux are asynchronous; processes don't get
> informed of write errors unless they call fsync() or are doing direct I/O.)
> I wonder if we should just keep the master key around, for per-extent key
> derivation only, until all files using that master key have been closed.  That
> would minimize the changes from the current fscrypt semantics.

That would work, if you're good with that. It felt like a larger 
deviation, with the possibility of unbounded continued use of a 
soft-removed master key; should fscrypt restrict use of the master key 
only to extents that are part of inodes that are already open?

> 
>>> Can you elaborate on why you went with a more "heavyweight" extents design?
>> Being able to rekey a directory is the reason for having full contexts:
>> suppose I take a snapshot of an encrypted dir and want to change the key for
>> new data going forward, to avoid using a single key on too much data. It's
>> too expensive to reencrypt every extent with the new key, since the whole
>> point of a snapshot is to make a lightweight copy that gets COWed on write.
>> Then each extent needs to know what its own master key
>> identifier/policy/flags are, since different extents in the same file could
>> have different master keys. We could say the mode and flags have to match,
>> but it doesn't seem to me worth saving seven bytes to add a new structure to
>> just store the master key identifier and nonce.
>>
>> For a non-Meta usecase, from what I've heard from Fedora-land, it's possibly
>> interesting to them to be able to ship an encrypted image, and then be able
>> to change the key after encrypted install to something user-controlled.
>>
>> Without rekeying, my understanding is that we may write too much data with
>> one key for safety; notes in the updated design doc https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
>> are that writing more than 1P per key raises cryptographic concerns, and
>> since btrfs is COW and could have volumes up to the full 16E size that btrfs
>> supports, we don't want to have just one immutable key per subvol.
>>
>> To me the lightweight-on-disk vision sounds a lot like the original design:
>> https://lore.kernel.org/linux-btrfs/YXGyq+buM79A1S0L@relinquished.localdomain
>> and the Nov '22 version of the patchset: https://lore.kernel.org/linux-btrfs/cover.1667389115.git.sweettea-kernel@dorminy.me/
>> (which didn't have rekeying). I think rekeying is worth the higher disk
>> usage; but I'm probably missing something about how your vision differs from
>> the original. Could you please take a look at it again?
> 
> Your original design didn't use per-extent keys, but rather had a single
> contents encryption key per master key.  We had discussed that that approach had
> multiple disadvantages, one of which is that on btrfs it can run uncomfortably
> close to the cryptographic limits for the contents encryption modes such as
> AES-XTS.  So we decided to go with per-extent keys instead.

> I don't think we discussed cryptographic limits on the master key itself.  That
> is actually much less of a concern, as the master key is just used for
> HKDF-SHA512.  I don't think there is any real need to ever change the master
> key.  (Well, if it is compromised, it could be needed, but that's not really
> relevant here.  If that happens you'd need to re-encrypt everything anyway.)
So you're proposing just storing a nonce per extent, then setting up a 
prepared key only, taking the inode's master key and doing the hkdf 
expand with the nonce into the prepared key? A terse approach, a lot 
closer to the original design than I thought.

> I do recall some discussion of making it possible to set an encryption policy on
> an *unencrypted* directory, causing new files in that directory to be encrypted.
> However, I don't recall any discussion of making it possible to add another
> encryption policy to an *already-encrypted* directory.  I think this is the
> first time this has been brought up.

I think I referenced it in the updated design (fifth paragraph of 
"Extent encryption" 
https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing) 
but I didn't talk about it enough -- 'rekeying' is a substitute for 
adding a policy to a directory full of unencrypted data. Ya'll's points 
about the badness of having mixed unencrypted and encrypted data in a 
single dir were compelling. (As I recall it, the issue with having mixed 
enc/unenc data is that a bug or attacker could point an encrypted file 
autostarted in a container, say /container/my-service, at a unencrypted 
extent under their control, say /bin/bash, and thereby acquire a backdoor.)
> 
> I think that allowing directories to have multiple encryption policies would
> bring in a lot of complexity.  How would it be configured, and what would the
> semantics for accessing it be?  Where would the encryption policies be stored?
> What if you have added some of the keys but not all of them?  What if some of
> the keys get removed but not all of them?
I'd planned to use add_enckey to add all the necessary keys, 
set_encpolicy on an encrypted directory under the proper conditions 
(flags interpreted by ioctl? check if filesystem has hook?) recursively 
calls a filesystem-provided hook on each inode within to change the 
fscrypt_context. Either dir items could be reencrypted with the new key 
or dirs could keep around the oldest key for encryption and the newest 
key for new leaf inodes to inherit.

For leaf inodes, 2 options for the remaining questions:

1) leaf inodes just stores the policy to be used for writing new data or 
inherits it from a parent directory. If you're reading and you hit an 
extent whose key isn't loaded, you get an IO error. If you're writing 
and you try to write to an inode who needs a key that isn't loaded, you 
get an IO error eventually.

This is what I'd prefer: Josef and Chris and I discussed this a while 
back and thought this was acceptable semantics for our uses, I think, 
and it's simple; you can already get an IO error if you're reading a 
file and suddenly hit an invalid extent, or if you are writing a file 
and run out of space.

2) all inodes store a fscrypt_context for all policies configured for 
the inode. ->get_context() gets a index parameter to get the index'th 
context. directory inodes' info is loaded off the one needed for dir 
entry encryption, leaf inodes' off the newest context. When any inode is 
opened, all the contexts are read and checked that their key is loaded, 
and then discarded, except the one in use for writes to that inode.

If a key is removed, you get an IO error when you do IO that needs that 
key, with the usual async conditions. Or, some lightweight structure is 
added to each master key's open inode list to point at the inode using 
it, and the inode keeps the master key around while it's open, thereby 
preventing IO errors after open.
> 
> Can you elaborate more on why you want this?  I was already a bit concerned
> about the plan for making it possible to set an encryption policy on an
> unencrypted directory, as that already diverges from the existing fscrypt
> semantics.  But now it sounds like the scope has grown even more.
> 
> Keep in mind that in general, the closer we are able to stick to the existing
> fscrypt semantics and design, the easier it is going to be to get the initial
> btrfs fscrypt support merged.  Planning ahead for new features is good, but we
> should also be careful not to overdesign.

Here's a shot at elaboration of usecase more:

On various machines, we currently have a btrfs filesystem containing 
various volumes/snapshots containing starting states for containers. The 
snapshots are generated by common snapshot images built centrally. The 
machines, as the scheduler requests, start and stop containers on those 
volumes.

We want to be able to start a container on a snapshot/volume such that 
every write to the relevant snapshot/volume is using a per-container 
key, but we don't care about reads of the starting snapshot image being 
encrypted since the starting snapshot image is widely shared. When the 
container is stopped, no future different container (or human or host 
program) knows its key. This limits the data which could be lost to a 
malicious service/human on the host to only the volumes containing 
currently running containers.

Some other folks envision having a base image encrypted with some 
per-vendor key. Then the machine is rekeyed with a per-machine key in 
perhaps the TPM to use for updates and logfiles. When a user is created, 
a snapshot of a base homedir forms the base of their user 
subvolume/directory, which is then rekeyed with a per-user key. When the 
user logs in, systemd-homedir or the like could load their per-user key 
for their user subvolume/directory.

Since we don't care about encrypting the common image, we initially 
envisioned unencrypted snapshot images where we then turn on encryption 
and have mixed unenc/enc data. The other usecase, though, really needs 
key change so that everything's encrypted. And the argument that mixed 
unenc/enc data is not safe was compelling.

Hope that helps?

Sweet Tea

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Eric Biggers]

On Mon, Jul 03, 2023 at 04:37:52PM -0400, Sweet Tea Dorminy wrote:
> 
> On 7/3/23 14:17, Eric Riggers wrote:
> > On Mon, Jul 03, 2023 at 01:06:17PM -0400, Sweet Tea Dorminy wrote:
> > > > 
> > > > I think that would avoid many of the problems that it seems you've had to work
> > > > around or have had to change user-visible semantics for.  For example the
> > > > problems involving master keys being added and removed.  It would also avoid
> > > > having to overload 'fscrypt_info' to be either a per-inode or a per-extent key.
> > > > And it would save space on disk and in memory.
> > > 
> > > I might be misunderstanding what you're referencing, but I think you're
> > > talking about the change where with extent fscrypt, IO has to be forced down
> > > before removing a key, otherwise it is lost. I think that's a fundamental
> > > problem given the filesystem has no way to know that there are new, dirty
> > > pages in the pagecache until those pages are issued for write, so it can't
> > > create a new extent or few until that point, potentially after the relevant
> > > key has been evicted. But maybe I'm missing a hook that would let us make
> > > extents earlier.
> > > 
> > > I suppose we could give each leaf inode a proper nonce/prepared key instead
> > > of borrowing its parent dir's: if a write came in after the key was removed
> > > but the inode is still open, the new extent(s) could grab the key material
> > > out of the inode's info. I don't like this very much since it could result
> > > in multiple extents grabbing the same key material, but I suppose it could
> > > work if it's important to maintain that behavior.
> > 
> > Right, if extent keys are derived directly from the master key, and
> > FS_IOC_REMOVE_ENCRYPTION_KEY is executed which causes the master key secret to
> > be wiped, then no more extent keys can be derived.
> > 
> > So I see why you implemented the behavior you did.  It does seem a bit
> > dangerous, though.  If I understand correctly, under your proposal, if
> > FS_IOC_REMOVE_ENCRYPTION_KEY is executed while a process is doing buffered
> > writes to an encrypted file protected by that key, then past writes will get
> > synced out just before the key removal, but future writes will just be silently
> > thrown away.  (Remember, writes in Linux are asynchronous; processes don't get
> > informed of write errors unless they call fsync() or are doing direct I/O.)
> > I wonder if we should just keep the master key around, for per-extent key
> > derivation only, until all files using that master key have been closed.  That
> > would minimize the changes from the current fscrypt semantics.
> 
> That would work, if you're good with that. It felt like a larger deviation,
> with the possibility of unbounded continued use of a soft-removed master
> key;

Yes, it's not ideal, but essentially that same behavior already exists for the
encryption settings that don't use per-file keys.

The fact is that key removal with in-use files is always going to be
"incomplete".  People who really need the key removal to work properly need to
ensure that all files are closed first, or at least ensure that they're closed
eventually and keep retrying the key removal until then...

> should fscrypt restrict use of the master key only to extents that are
> part of inodes that are already open?

Yes, I think so.  That way the user-visible semantics would stay the same as is
currently the case for fscrypt.

> > > > Can you elaborate on why you went with a more "heavyweight" extents design?
> > > Being able to rekey a directory is the reason for having full contexts:
> > > suppose I take a snapshot of an encrypted dir and want to change the key for
> > > new data going forward, to avoid using a single key on too much data. It's
> > > too expensive to reencrypt every extent with the new key, since the whole
> > > point of a snapshot is to make a lightweight copy that gets COWed on write.
> > > Then each extent needs to know what its own master key
> > > identifier/policy/flags are, since different extents in the same file could
> > > have different master keys. We could say the mode and flags have to match,
> > > but it doesn't seem to me worth saving seven bytes to add a new structure to
> > > just store the master key identifier and nonce.
> > > 
> > > For a non-Meta usecase, from what I've heard from Fedora-land, it's possibly
> > > interesting to them to be able to ship an encrypted image, and then be able
> > > to change the key after encrypted install to something user-controlled.
> > > 
> > > Without rekeying, my understanding is that we may write too much data with
> > > one key for safety; notes in the updated design doc https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing
> > > are that writing more than 1P per key raises cryptographic concerns, and
> > > since btrfs is COW and could have volumes up to the full 16E size that btrfs
> > > supports, we don't want to have just one immutable key per subvol.
> > > 
> > > To me the lightweight-on-disk vision sounds a lot like the original design:
> > > https://lore.kernel.org/linux-btrfs/YXGyq+buM79A1S0L@relinquished.localdomain
> > > and the Nov '22 version of the patchset: https://lore.kernel.org/linux-btrfs/cover.1667389115.git.sweettea-kernel@dorminy.me/
> > > (which didn't have rekeying). I think rekeying is worth the higher disk
> > > usage; but I'm probably missing something about how your vision differs from
> > > the original. Could you please take a look at it again?
> > 
> > Your original design didn't use per-extent keys, but rather had a single
> > contents encryption key per master key.  We had discussed that that approach had
> > multiple disadvantages, one of which is that on btrfs it can run uncomfortably
> > close to the cryptographic limits for the contents encryption modes such as
> > AES-XTS.  So we decided to go with per-extent keys instead.
> 
> > I don't think we discussed cryptographic limits on the master key itself.  That
> > is actually much less of a concern, as the master key is just used for
> > HKDF-SHA512.  I don't think there is any real need to ever change the master
> > key.  (Well, if it is compromised, it could be needed, but that's not really
> > relevant here.  If that happens you'd need to re-encrypt everything anyway.)
> So you're proposing just storing a nonce per extent, then setting up a
> prepared key only, taking the inode's master key and doing the hkdf expand
> with the nonce into the prepared key? A terse approach, a lot closer to the
> original design than I thought.

Yes.

> > I do recall some discussion of making it possible to set an encryption policy on
> > an *unencrypted* directory, causing new files in that directory to be encrypted.
> > However, I don't recall any discussion of making it possible to add another
> > encryption policy to an *already-encrypted* directory.  I think this is the
> > first time this has been brought up.
> 
> I think I referenced it in the updated design (fifth paragraph of "Extent
> encryption" https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing)
> but I didn't talk about it enough -- 'rekeying' is a substitute for adding a
> policy to a directory full of unencrypted data. Ya'll's points about the
> badness of having mixed unencrypted and encrypted data in a single dir were
> compelling. (As I recall it, the issue with having mixed enc/unenc data is
> that a bug or attacker could point an encrypted file autostarted in a
> container, say /container/my-service, at a unencrypted extent under their
> control, say /bin/bash, and thereby acquire a backdoor.)
> > 
> > I think that allowing directories to have multiple encryption policies would
> > bring in a lot of complexity.  How would it be configured, and what would the
> > semantics for accessing it be?  Where would the encryption policies be stored?
> > What if you have added some of the keys but not all of them?  What if some of
> > the keys get removed but not all of them?
> I'd planned to use add_enckey to add all the necessary keys, set_encpolicy
> on an encrypted directory under the proper conditions (flags interpreted by
> ioctl? check if filesystem has hook?) recursively calls a
> filesystem-provided hook on each inode within to change the fscrypt_context.

That sounds quite complex.  Recursive directory operations aren't really
something the kernel does.  It would also require updating every inode, causing
COW of every inode.  Isn't that something you'd really like to avoid, to make
starting a new container as fast and lightweight as possible?

> Either dir items could be reencrypted with the new key or dirs could keep
> around the oldest key for encryption and the newest key for new leaf inodes
> to inherit.
> 
> For leaf inodes, 2 options for the remaining questions:
> 
> 1) leaf inodes just stores the policy to be used for writing new data or
> inherits it from a parent directory. If you're reading and you hit an extent
> whose key isn't loaded, you get an IO error. If you're writing and you try
> to write to an inode who needs a key that isn't loaded, you get an IO error
> eventually.
> 
> This is what I'd prefer: Josef and Chris and I discussed this a while back
> and thought this was acceptable semantics for our uses, I think, and it's
> simple; you can already get an IO error if you're reading a file and
> suddenly hit an invalid extent, or if you are writing a file and run out of
> space.
> 
> 2) all inodes store a fscrypt_context for all policies configured for the
> inode. ->get_context() gets a index parameter to get the index'th context.
> directory inodes' info is loaded off the one needed for dir entry
> encryption, leaf inodes' off the newest context. When any inode is opened,
> all the contexts are read and checked that their key is loaded, and then
> discarded, except the one in use for writes to that inode.
> 
> If a key is removed, you get an IO error when you do IO that needs that key,
> with the usual async conditions. Or, some lightweight structure is added to
> each master key's open inode list to point at the inode using it, and the
> inode keeps the master key around while it's open, thereby preventing IO
> errors after open.
> > 
> > Can you elaborate more on why you want this?  I was already a bit concerned
> > about the plan for making it possible to set an encryption policy on an
> > unencrypted directory, as that already diverges from the existing fscrypt
> > semantics.  But now it sounds like the scope has grown even more.
> > 
> > Keep in mind that in general, the closer we are able to stick to the existing
> > fscrypt semantics and design, the easier it is going to be to get the initial
> > btrfs fscrypt support merged.  Planning ahead for new features is good, but we
> > should also be careful not to overdesign.
> 
> Here's a shot at elaboration of usecase more:
> 
> On various machines, we currently have a btrfs filesystem containing various
> volumes/snapshots containing starting states for containers. The snapshots
> are generated by common snapshot images built centrally. The machines, as
> the scheduler requests, start and stop containers on those volumes.
> 
> We want to be able to start a container on a snapshot/volume such that every
> write to the relevant snapshot/volume is using a per-container key, but we
> don't care about reads of the starting snapshot image being encrypted since
> the starting snapshot image is widely shared. When the container is stopped,
> no future different container (or human or host program) knows its key. This
> limits the data which could be lost to a malicious service/human on the host
> to only the volumes containing currently running containers.
> 
> Some other folks envision having a base image encrypted with some per-vendor
> key. Then the machine is rekeyed with a per-machine key in perhaps the TPM
> to use for updates and logfiles. When a user is created, a snapshot of a
> base homedir forms the base of their user subvolume/directory, which is then
> rekeyed with a per-user key. When the user logs in, systemd-homedir or the
> like could load their per-user key for their user subvolume/directory.
> 
> Since we don't care about encrypting the common image, we initially
> envisioned unencrypted snapshot images where we then turn on encryption and
> have mixed unenc/enc data. The other usecase, though, really needs key
> change so that everything's encrypted. And the argument that mixed unenc/enc
> data is not safe was compelling.
> 
> Hope that helps?

Maybe a dumb question: why aren't you just using overlayfs?  It's already
possible to use overlayfs with an fscrypt-encrypted upperdir and workdir.  When
creating a new container you can create a new directory and assign it an fscrypt
policy (with a per-container or per-user key or whatever that container wants),
and create two subdirectories 'upperdir' and 'workdir' in it.  Then just mount
an overlayfs with that upperdir and workdir, and lowerdir referring to the
starting rootfs.  Then use that overlayfs as the rootfs as the container.

Wouldn't that solve your use case exactly?  Is there a reason you really want to
create the container directly from a btrfs snapshot instead?

- Eric

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Sweet Tea Dorminy]

>>> I do recall some discussion of making it possible to set an encryption policy on
>>> an *unencrypted* directory, causing new files in that directory to be encrypted.
>>> However, I don't recall any discussion of making it possible to add another
>>> encryption policy to an *already-encrypted* directory.  I think this is the
>>> first time this has been brought up.
>>
>> I think I referenced it in the updated design (fifth paragraph of "Extent
>> encryption" https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing)
>> but I didn't talk about it enough -- 'rekeying' is a substitute for adding a
>> policy to a directory full of unencrypted data. Ya'll's points about the
>> badness of having mixed unencrypted and encrypted data in a single dir were
>> compelling. (As I recall it, the issue with having mixed enc/unenc data is
>> that a bug or attacker could point an encrypted file autostarted in a
>> container, say /container/my-service, at a unencrypted extent under their
>> control, say /bin/bash, and thereby acquire a backdoor.)
>>>
>>> I think that allowing directories to have multiple encryption policies would
>>> bring in a lot of complexity.  How would it be configured, and what would the
>>> semantics for accessing it be?  Where would the encryption policies be stored?
>>> What if you have added some of the keys but not all of them?  What if some of
>>> the keys get removed but not all of them?
>> I'd planned to use add_enckey to add all the necessary keys, set_encpolicy
>> on an encrypted directory under the proper conditions (flags interpreted by
>> ioctl? check if filesystem has hook?) recursively calls a
>> filesystem-provided hook on each inode within to change the fscrypt_context.
> 
> That sounds quite complex.  Recursive directory operations aren't really
> something the kernel does.  It would also require updating every inode, causing
> COW of every inode.  Isn't that something you'd really like to avoid, to make
> starting a new container as fast and lightweight as possible?

A fair point. Can move the penalty to open or write time instead though: 
btrfs could store a generation number with the new context on only the 
directory changed, then leaf inodes or new extent can traverse up the 
directory tree and grab context from the highest-generation-number 
directory in its path to inherit from. Or btrfs could disallow changing 
except on the base of a subvolume, and just go directly to the top of 
the subvolume to grab the appropriate context. Neither of those require 
recursion outside btrfs.

>> On various machines, we currently have a btrfs filesystem containing various
>> volumes/snapshots containing starting states for containers. The snapshots
>> are generated by common snapshot images built centrally. The machines, as
>> the scheduler requests, start and stop containers on those volumes.
>>
>> We want to be able to start a container on a snapshot/volume such that every
>> write to the relevant snapshot/volume is using a per-container key, but we
>> don't care about reads of the starting snapshot image being encrypted since
>> the starting snapshot image is widely shared. When the container is stopped,
>> no future different container (or human or host program) knows its key. This
>> limits the data which could be lost to a malicious service/human on the host
>> to only the volumes containing currently running containers.
>>
>> Some other folks envision having a base image encrypted with some per-vendor
>> key. Then the machine is rekeyed with a per-machine key in perhaps the TPM
>> to use for updates and logfiles. When a user is created, a snapshot of a
>> base homedir forms the base of their user subvolume/directory, which is then
>> rekeyed with a per-user key. When the user logs in, systemd-homedir or the
>> like could load their per-user key for their user subvolume/directory.
>>
>> Since we don't care about encrypting the common image, we initially
>> envisioned unencrypted snapshot images where we then turn on encryption and
>> have mixed unenc/enc data. The other usecase, though, really needs key
>> change so that everything's encrypted. And the argument that mixed unenc/enc
>> data is not safe was compelling.
>>
>> Hope that helps?
> 
> Maybe a dumb question: why aren't you just using overlayfs?  It's already
> possible to use overlayfs with an fscrypt-encrypted upperdir and workdir.  When
> creating a new container you can create a new directory and assign it an fscrypt
> policy (with a per-container or per-user key or whatever that container wants),
> and create two subdirectories 'upperdir' and 'workdir' in it.  Then just mount
> an overlayfs with that upperdir and workdir, and lowerdir referring to the
> starting rootfs.  Then use that overlayfs as the rootfs as the container.
> 
> Wouldn't that solve your use case exactly?  Is there a reason you really want to
> create the container directly from a btrfs snapshot instead?

Hardly; a quite intriguing idea. Let me think about this with folks when 
we get back to work on Wednesday. Not sure how it goes with the other 
usecase, the base image/per-machine/per-user combo, but will think about it.

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Neal Gompa]

On Mon, Jul 3, 2023 at 10:03 PM Sweet Tea Dorminy
<sweettea-kernel@dorminy.me> wrote:
>
>
> >>> I do recall some discussion of making it possible to set an encryption policy on
> >>> an *unencrypted* directory, causing new files in that directory to be encrypted.
> >>> However, I don't recall any discussion of making it possible to add another
> >>> encryption policy to an *already-encrypted* directory.  I think this is the
> >>> first time this has been brought up.
> >>
> >> I think I referenced it in the updated design (fifth paragraph of "Extent
> >> encryption" https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing)
> >> but I didn't talk about it enough -- 'rekeying' is a substitute for adding a
> >> policy to a directory full of unencrypted data. Ya'll's points about the
> >> badness of having mixed unencrypted and encrypted data in a single dir were
> >> compelling. (As I recall it, the issue with having mixed enc/unenc data is
> >> that a bug or attacker could point an encrypted file autostarted in a
> >> container, say /container/my-service, at a unencrypted extent under their
> >> control, say /bin/bash, and thereby acquire a backdoor.)
> >>>
> >>> I think that allowing directories to have multiple encryption policies would
> >>> bring in a lot of complexity.  How would it be configured, and what would the
> >>> semantics for accessing it be?  Where would the encryption policies be stored?
> >>> What if you have added some of the keys but not all of them?  What if some of
> >>> the keys get removed but not all of them?
> >> I'd planned to use add_enckey to add all the necessary keys, set_encpolicy
> >> on an encrypted directory under the proper conditions (flags interpreted by
> >> ioctl? check if filesystem has hook?) recursively calls a
> >> filesystem-provided hook on each inode within to change the fscrypt_context.
> >
> > That sounds quite complex.  Recursive directory operations aren't really
> > something the kernel does.  It would also require updating every inode, causing
> > COW of every inode.  Isn't that something you'd really like to avoid, to make
> > starting a new container as fast and lightweight as possible?
>
> A fair point. Can move the penalty to open or write time instead though:
> btrfs could store a generation number with the new context on only the
> directory changed, then leaf inodes or new extent can traverse up the
> directory tree and grab context from the highest-generation-number
> directory in its path to inherit from. Or btrfs could disallow changing
> except on the base of a subvolume, and just go directly to the top of
> the subvolume to grab the appropriate context. Neither of those require
> recursion outside btrfs.
>
> >> On various machines, we currently have a btrfs filesystem containing various
> >> volumes/snapshots containing starting states for containers. The snapshots
> >> are generated by common snapshot images built centrally. The machines, as
> >> the scheduler requests, start and stop containers on those volumes.
> >>
> >> We want to be able to start a container on a snapshot/volume such that every
> >> write to the relevant snapshot/volume is using a per-container key, but we
> >> don't care about reads of the starting snapshot image being encrypted since
> >> the starting snapshot image is widely shared. When the container is stopped,
> >> no future different container (or human or host program) knows its key. This
> >> limits the data which could be lost to a malicious service/human on the host
> >> to only the volumes containing currently running containers.
> >>
> >> Some other folks envision having a base image encrypted with some per-vendor
> >> key. Then the machine is rekeyed with a per-machine key in perhaps the TPM
> >> to use for updates and logfiles. When a user is created, a snapshot of a
> >> base homedir forms the base of their user subvolume/directory, which is then
> >> rekeyed with a per-user key. When the user logs in, systemd-homedir or the
> >> like could load their per-user key for their user subvolume/directory.
> >>
> >> Since we don't care about encrypting the common image, we initially
> >> envisioned unencrypted snapshot images where we then turn on encryption and
> >> have mixed unenc/enc data. The other usecase, though, really needs key
> >> change so that everything's encrypted. And the argument that mixed unenc/enc
> >> data is not safe was compelling.
> >>
> >> Hope that helps?
> >
> > Maybe a dumb question: why aren't you just using overlayfs?  It's already
> > possible to use overlayfs with an fscrypt-encrypted upperdir and workdir.  When
> > creating a new container you can create a new directory and assign it an fscrypt
> > policy (with a per-container or per-user key or whatever that container wants),
> > and create two subdirectories 'upperdir' and 'workdir' in it.  Then just mount
> > an overlayfs with that upperdir and workdir, and lowerdir referring to the
> > starting rootfs.  Then use that overlayfs as the rootfs as the container.
> >
> > Wouldn't that solve your use case exactly?  Is there a reason you really want to
> > create the container directly from a btrfs snapshot instead?
>
> Hardly; a quite intriguing idea. Let me think about this with folks when
> we get back to work on Wednesday. Not sure how it goes with the other
> usecase, the base image/per-machine/per-user combo, but will think about it.

I like creating containers directly based on my host system for
development and destructive purposes. It saves space and is incredibly
useful.

But the layered key encryption thing is also core to the encryption
strategy we want to take in Fedora, so I would really like to see this
be possible with Btrfs encryption.

Critically, it means that unlocking a user subvolume will always be
multi-factor: something you have (machine key) and something you know
(user credentials).


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Eric Biggers]

On Wed, Jul 05, 2023 at 08:13:34AM -0400, Neal Gompa wrote:
> On Mon, Jul 3, 2023 at 10:03 PM Sweet Tea Dorminy
> <sweettea-kernel@dorminy.me> wrote:
> >
> >
> > >>> I do recall some discussion of making it possible to set an encryption policy on
> > >>> an *unencrypted* directory, causing new files in that directory to be encrypted.
> > >>> However, I don't recall any discussion of making it possible to add another
> > >>> encryption policy to an *already-encrypted* directory.  I think this is the
> > >>> first time this has been brought up.
> > >>
> > >> I think I referenced it in the updated design (fifth paragraph of "Extent
> > >> encryption" https://docs.google.com/document/d/1janjxewlewtVPqctkWOjSa7OhCgB8Gdx7iDaCDQQNZA/edit?usp=sharing)
> > >> but I didn't talk about it enough -- 'rekeying' is a substitute for adding a
> > >> policy to a directory full of unencrypted data. Ya'll's points about the
> > >> badness of having mixed unencrypted and encrypted data in a single dir were
> > >> compelling. (As I recall it, the issue with having mixed enc/unenc data is
> > >> that a bug or attacker could point an encrypted file autostarted in a
> > >> container, say /container/my-service, at a unencrypted extent under their
> > >> control, say /bin/bash, and thereby acquire a backdoor.)
> > >>>
> > >>> I think that allowing directories to have multiple encryption policies would
> > >>> bring in a lot of complexity.  How would it be configured, and what would the
> > >>> semantics for accessing it be?  Where would the encryption policies be stored?
> > >>> What if you have added some of the keys but not all of them?  What if some of
> > >>> the keys get removed but not all of them?
> > >> I'd planned to use add_enckey to add all the necessary keys, set_encpolicy
> > >> on an encrypted directory under the proper conditions (flags interpreted by
> > >> ioctl? check if filesystem has hook?) recursively calls a
> > >> filesystem-provided hook on each inode within to change the fscrypt_context.
> > >
> > > That sounds quite complex.  Recursive directory operations aren't really
> > > something the kernel does.  It would also require updating every inode, causing
> > > COW of every inode.  Isn't that something you'd really like to avoid, to make
> > > starting a new container as fast and lightweight as possible?
> >
> > A fair point. Can move the penalty to open or write time instead though:
> > btrfs could store a generation number with the new context on only the
> > directory changed, then leaf inodes or new extent can traverse up the
> > directory tree and grab context from the highest-generation-number
> > directory in its path to inherit from. Or btrfs could disallow changing
> > except on the base of a subvolume, and just go directly to the top of
> > the subvolume to grab the appropriate context. Neither of those require
> > recursion outside btrfs.
> >
> > >> On various machines, we currently have a btrfs filesystem containing various
> > >> volumes/snapshots containing starting states for containers. The snapshots
> > >> are generated by common snapshot images built centrally. The machines, as
> > >> the scheduler requests, start and stop containers on those volumes.
> > >>
> > >> We want to be able to start a container on a snapshot/volume such that every
> > >> write to the relevant snapshot/volume is using a per-container key, but we
> > >> don't care about reads of the starting snapshot image being encrypted since
> > >> the starting snapshot image is widely shared. When the container is stopped,
> > >> no future different container (or human or host program) knows its key. This
> > >> limits the data which could be lost to a malicious service/human on the host
> > >> to only the volumes containing currently running containers.
> > >>
> > >> Some other folks envision having a base image encrypted with some per-vendor
> > >> key. Then the machine is rekeyed with a per-machine key in perhaps the TPM
> > >> to use for updates and logfiles. When a user is created, a snapshot of a
> > >> base homedir forms the base of their user subvolume/directory, which is then
> > >> rekeyed with a per-user key. When the user logs in, systemd-homedir or the
> > >> like could load their per-user key for their user subvolume/directory.
> > >>
> > >> Since we don't care about encrypting the common image, we initially
> > >> envisioned unencrypted snapshot images where we then turn on encryption and
> > >> have mixed unenc/enc data. The other usecase, though, really needs key
> > >> change so that everything's encrypted. And the argument that mixed unenc/enc
> > >> data is not safe was compelling.
> > >>
> > >> Hope that helps?
> > >
> > > Maybe a dumb question: why aren't you just using overlayfs?  It's already
> > > possible to use overlayfs with an fscrypt-encrypted upperdir and workdir.  When
> > > creating a new container you can create a new directory and assign it an fscrypt
> > > policy (with a per-container or per-user key or whatever that container wants),
> > > and create two subdirectories 'upperdir' and 'workdir' in it.  Then just mount
> > > an overlayfs with that upperdir and workdir, and lowerdir referring to the
> > > starting rootfs.  Then use that overlayfs as the rootfs as the container.
> > >
> > > Wouldn't that solve your use case exactly?  Is there a reason you really want to
> > > create the container directly from a btrfs snapshot instead?
> >
> > Hardly; a quite intriguing idea. Let me think about this with folks when
> > we get back to work on Wednesday. Not sure how it goes with the other
> > usecase, the base image/per-machine/per-user combo, but will think about it.
> 
> I like creating containers directly based on my host system for
> development and destructive purposes. It saves space and is incredibly
> useful.

A solution for that already exists.  It's called btrfs snapshots.  Which you
probably already know, since it's probably what you're using :-)

Using overlayfs would simply mean that each container consists of an upper and
lower directory instead of a single directory.  Either or both could still be
btrfs subvolumes.  They could even be on the same subvolume.

> 
> But the layered key encryption thing is also core to the encryption
> strategy we want to take in Fedora, so I would really like to see this
> be possible with Btrfs encryption.
> 
> Critically, it means that unlocking a user subvolume will always be
> multi-factor: something you have (machine key) and something you know
> (user credentials).

That's possible with the existing fscrypt semantics.  Just use a unique master
key for each container, and protect it with a key derived from both the machine
key *and* the user credential.  Protecting the fscrypt master key(s) is a
userspace problem, not a kernel one.  The kernel just receives the raw key.

- Eric

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Sweet Tea Dorminy]

>> I like creating containers directly based on my host system for
>> development and destructive purposes. It saves space and is incredibly
>> useful.
> 
> A solution for that already exists.  It's called btrfs snapshots.  Which you
> probably already know, since it's probably what you're using :-)
> 
> Using overlayfs would simply mean that each container consists of an upper and
> lower directory instead of a single directory.  Either or both could still be
> btrfs subvolumes.  They could even be on the same subvolume.

This isn't a full response, still researching details of our setup and 
whether overlayfs could work for us. But in re this particular usecase:

as I understand it, the lower layer for overlayfs is immutable. So if 
you set up a container/VM image in this way with overlayfs, you end up 
not being able to reclaim space from the original image, if e.g. you 
uninstall or upgrade a package present in the original.

This wastes disk space, and, if you want to migrate that subvol over to 
another machine/disk via btrfs send/receive, wastes network/CPU. 
Hopefully not a lot -- hopefully your container image is mostly 
immutable -- but for long-lived VMs like I usually use personally it 
could be basically the whole VM image wasted.

Re: [PATCH v1 00/12] fscrypt: add extent encryption

[Sweet Tea Dorminy]

>>
>> Here's a shot at elaboration of usecase more:
>>
>> On various machines, we currently have a btrfs filesystem containing various
>> volumes/snapshots containing starting states for containers. The snapshots
>> are generated by common snapshot images built centrally. The machines, as
>> the scheduler requests, start and stop containers on those volumes.
>>
>> We want to be able to start a container on a snapshot/volume such that every
>> write to the relevant snapshot/volume is using a per-container key, but we
>> don't care about reads of the starting snapshot image being encrypted since
>> the starting snapshot image is widely shared. When the container is stopped,
>> no future different container (or human or host program) knows its key. This
>> limits the data which could be lost to a malicious service/human on the host
>> to only the volumes containing currently running containers.
>>
>> Some other folks envision having a base image encrypted with some per-vendor
>> key. Then the machine is rekeyed with a per-machine key in perhaps the TPM
>> to use for updates and logfiles. When a user is created, a snapshot of a
>> base homedir forms the base of their user subvolume/directory, which is then
>> rekeyed with a per-user key. When the user logs in, systemd-homedir or the
>> like could load their per-user key for their user subvolume/directory.
>>
>> Since we don't care about encrypting the common image, we initially
>> envisioned unencrypted snapshot images where we then turn on encryption and
>> have mixed unenc/enc data. The other usecase, though, really needs key
>> change so that everything's encrypted. And the argument that mixed unenc/enc
>> data is not safe was compelling.
>>
>> Hope that helps?
> 
> Maybe a dumb question: why aren't you just using overlayfs?  It's already
> possible to use overlayfs with an fscrypt-encrypted upperdir and workdir.  When
> creating a new container you can create a new directory and assign it an fscrypt
> policy (with a per-container or per-user key or whatever that container wants),
> and create two subdirectories 'upperdir' and 'workdir' in it.  Then just mount
> an overlayfs with that upperdir and workdir, and lowerdir referring to the
> starting rootfs.  Then use that overlayfs as the rootfs as the container.
> 
> Wouldn't that solve your use case exactly?  Is there a reason you really want to
> create the container directly from a btrfs snapshot instead?

After talking it over, nested containers/subvols don't work easily with 
this scheme. Right now, one can make arbitrarily nested subvols inside 
of subvols, so e.g. a container which only sees /subvol can make subvol 
/subvol/nested without elevated permissions, and a container which only 
sees /subvol/nested could make yet another nested subvol 
/subvol/nested/foo/nested2, ad infinitum. There aren't afaik limits on 
the recursive depth of subvols or containers, or limits on how close 
they are in the directory tree.

This isn't purely theoretical; I learned today there are a couple of 
workloads internally which run in a long-lived container on a subvol, 
and spin up a bunch of short-lived containers on shortlived subvols 
inside the long-lived container/subvol.

I don't think the overlayfs scheme works with this. From the point of 
view of the container overlayfs would be presenting a wholly encrypted 
filesystem (which is what we want). But from the container, even if we 
plumbed through making a new subvol within, it'd be hard to create a new 
overlayfs upper directory with a new key for a nested container, if dirs 
had to have the same key as their parent dir unless that's unencrypted. 
We'd need to allow the parent container to escape into an unencrypted 
directory to make a new encrypted upperdir for the nested container, 
which would defeat having the container only able to write to encrypted 
locations. I can't come up with a way to make the overlayfs scheme work 
with this, but maybe I don't know overlayfs well enough.

A decidedly intriguing idea! Thanks

Sweet Tea