[RFC PATCH 0/4] dm-default-key: target for filesystem metadata encryption

[RFC PATCH 0/4] dm-default-key: target for filesystem metadata encryption

[Eric Biggers]

This series adds "metadata encryption" support to ext4 and f2fs via a
new device-mapper target dm-default-key.  dm-default-key encrypts all
data on a block device that isn't already encrypted by the filesystem.

Except for the passthrough support, dm-default-key is basically the same
as the proposed dm-inlinecrypt which omits that feature
(https://lore.kernel.org/dm-devel/20241016232748.134211-1-ebiggers@kernel.org/).

I am sending this out for reference, as dm-default-key (which Android
has been using for a while) hasn't previously been sent to the lists in
full, and there has been interest in it.  However, my current impression
is that this feature will need to be redesigned as a filesystem native
feature in order to make it upstream.  If that is indeed the case, then
IMO it would make sense to merge dm-inlinecrypt in the mean time instead
(or add its functionality to dm-crypt) so that anyone who just wants
"dm-crypt + inline encryption hardware" gets a solution for that.

Eric Biggers (4):
  block: export blk-crypto symbols required by dm-default-key
  block: add the bi_skip_dm_default_key flag
  dm-default-key: add target for filesystem metadata encryption
  ext4,f2fs: support metadata encryption via dm-default-key

 block/bio.c                 |   3 +
 block/blk-crypto-fallback.c |   2 +
 block/blk-crypto.c          |   3 +
 drivers/md/Kconfig          |  20 ++
 drivers/md/Makefile         |   1 +
 drivers/md/dm-default-key.c | 431 ++++++++++++++++++++++++++++++++++++
 fs/crypto/inline_crypt.c    |  14 +-
 fs/f2fs/data.c              |   6 +-
 include/linux/blk-crypto.h  |  36 +++
 include/linux/blk_types.h   |   3 +
 include/linux/fscrypt.h     |  14 ++
 11 files changed, 531 insertions(+), 2 deletions(-)
 create mode 100644 drivers/md/dm-default-key.c


base-commit: c964ced7726294d40913f2127c3f185a92cb4a41
-- 
2.47.0

[RFC PATCH 1/4] block: export blk-crypto symbols required by dm-default-key

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

bio_crypt_set_ctx(), blk_crypto_init_key(), and
blk_crypto_start_using_key() are needed to use inline encryption; see
Documentation/block/inline-encryption.rst.  Export them so that
dm-default-key can use them.  The only reason these weren't exported
before was that inline encryption was previously used only by fs/crypto/
which is built-in code.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 block/blk-crypto.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/blk-crypto.c b/block/blk-crypto.c
index 4d760b092deb9..5a121b1292f9a 100644
--- a/block/blk-crypto.c
+++ b/block/blk-crypto.c
@@ -104,10 +104,11 @@ void bio_crypt_set_ctx(struct bio *bio, const struct blk_crypto_key *key,
 	bc->bc_key = key;
 	memcpy(bc->bc_dun, dun, sizeof(bc->bc_dun));
 
 	bio->bi_crypt_context = bc;
 }
+EXPORT_SYMBOL_GPL(bio_crypt_set_ctx);
 
 void __bio_crypt_free_ctx(struct bio *bio)
 {
 	mempool_free(bio->bi_crypt_context, bio_crypt_ctx_pool);
 	bio->bi_crypt_context = NULL;
@@ -354,10 +355,11 @@ int blk_crypto_init_key(struct blk_crypto_key *blk_key, const u8 *raw_key,
 	blk_key->size = mode->keysize;
 	memcpy(blk_key->raw, raw_key, mode->keysize);
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(blk_crypto_init_key);
 
 bool blk_crypto_config_supported_natively(struct block_device *bdev,
 					  const struct blk_crypto_config *cfg)
 {
 	return __blk_crypto_cfg_supported(bdev_get_queue(bdev)->crypto_profile,
@@ -396,10 +398,11 @@ int blk_crypto_start_using_key(struct block_device *bdev,
 {
 	if (blk_crypto_config_supported_natively(bdev, &key->crypto_cfg))
 		return 0;
 	return blk_crypto_fallback_start_using_mode(key->crypto_cfg.crypto_mode);
 }
+EXPORT_SYMBOL_GPL(blk_crypto_start_using_key);
 
 /**
  * blk_crypto_evict_key() - Evict a blk_crypto_key from a block_device
  * @bdev: a block_device on which I/O using the key may have been done
  * @key: the key to evict
-- 
2.47.0

[RFC PATCH 2/4] block: add the bi_skip_dm_default_key flag

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add a flag bi_skip_dm_default_key to struct bio.  This flag indicates
that dm-default-key should not en/decrypt the bio, due to it targeting
the contents of an encrypted file.

When a bio is cloned, copy the bi_skip_dm_default_key flag.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 block/bio.c                 |  3 +++
 block/blk-crypto-fallback.c |  2 ++
 include/linux/blk-crypto.h  | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/blk_types.h   |  3 +++
 4 files changed, 44 insertions(+)

diff --git a/block/bio.c b/block/bio.c
index ac4d77c889322..5ff0b66e47a42 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -267,10 +267,13 @@ void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table,
 	bio->bi_iocost_cost = 0;
 #endif
 #endif
 #ifdef CONFIG_BLK_INLINE_ENCRYPTION
 	bio->bi_crypt_context = NULL;
+#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
+	bio->bi_skip_dm_default_key = false;
+#endif
 #endif
 #ifdef CONFIG_BLK_DEV_INTEGRITY
 	bio->bi_integrity = NULL;
 #endif
 	bio->bi_vcnt = 0;
diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index b1e7415f8439c..dd5f1edcc44b3 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -179,10 +179,12 @@ static struct bio *blk_crypto_fallback_clone_bio(struct bio *bio_src)
 	bio_for_each_segment(bv, bio_src, iter)
 		bio->bi_io_vec[bio->bi_vcnt++] = bv;
 
 	bio_clone_blkg_association(bio, bio_src);
 
+	bio_clone_skip_dm_default_key(bio, bio_src);
+
 	return bio;
 }
 
 static bool
 blk_crypto_fallback_alloc_cipher_req(struct blk_crypto_keyslot *slot,
diff --git a/include/linux/blk-crypto.h b/include/linux/blk-crypto.h
index 5e5822c18ee41..f1f3d546c53e5 100644
--- a/include/linux/blk-crypto.h
+++ b/include/linux/blk-crypto.h
@@ -110,10 +110,13 @@ static inline bool bio_has_crypt_ctx(struct bio *bio)
 	return false;
 }
 
 #endif /* CONFIG_BLK_INLINE_ENCRYPTION */
 
+static inline void bio_clone_skip_dm_default_key(struct bio *dst,
+						 const struct bio *src);
+
 int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
 /**
  * bio_crypt_clone - clone bio encryption context
  * @dst: destination bio
  * @src: source bio
@@ -125,11 +128,44 @@ int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
  *	   @gfp_mask doesn't include %__GFP_DIRECT_RECLAIM.
  */
 static inline int bio_crypt_clone(struct bio *dst, struct bio *src,
 				  gfp_t gfp_mask)
 {
+	bio_clone_skip_dm_default_key(dst, src);
 	if (bio_has_crypt_ctx(src))
 		return __bio_crypt_clone(dst, src, gfp_mask);
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
+static inline void bio_set_skip_dm_default_key(struct bio *bio)
+{
+	bio->bi_skip_dm_default_key = true;
+}
+
+static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
+{
+	return bio->bi_skip_dm_default_key;
+}
+
+static inline void bio_clone_skip_dm_default_key(struct bio *dst,
+						 const struct bio *src)
+{
+	dst->bi_skip_dm_default_key = src->bi_skip_dm_default_key;
+}
+#else /* CONFIG_DM_DEFAULT_KEY */
+static inline void bio_set_skip_dm_default_key(struct bio *bio)
+{
+}
+
+static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
+{
+	return false;
+}
+
+static inline void bio_clone_skip_dm_default_key(struct bio *dst,
+						 const struct bio *src)
+{
+}
+#endif /* !CONFIG_DM_DEFAULT_KEY */
+
 #endif /* __LINUX_BLK_CRYPTO_H */
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index dce7615c35e7e..2ee6a7e570796 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -247,10 +247,13 @@ struct bio {
 #endif
 #endif
 
 #ifdef CONFIG_BLK_INLINE_ENCRYPTION
 	struct bio_crypt_ctx	*bi_crypt_context;
+#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
+	bool			bi_skip_dm_default_key;
+#endif
 #endif
 
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
 	struct bio_integrity_payload *bi_integrity; /* data integrity */
 #endif
-- 
2.47.0

Re: [RFC PATCH 2/4] block: add the bi_skip_dm_default_key flag

[Mikulas Patocka]

Hi

What about using the REQ_META flag (it is set on metadata bios and cleared 
on data bios), instead of adding a new flag with the same meaning?

Mikulas


On Fri, 18 Oct 2024, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> Add a flag bi_skip_dm_default_key to struct bio.  This flag indicates
> that dm-default-key should not en/decrypt the bio, due to it targeting
> the contents of an encrypted file.
> 
> When a bio is cloned, copy the bi_skip_dm_default_key flag.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  block/bio.c                 |  3 +++
>  block/blk-crypto-fallback.c |  2 ++
>  include/linux/blk-crypto.h  | 36 ++++++++++++++++++++++++++++++++++++
>  include/linux/blk_types.h   |  3 +++
>  4 files changed, 44 insertions(+)
> 
> diff --git a/block/bio.c b/block/bio.c
> index ac4d77c889322..5ff0b66e47a42 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -267,10 +267,13 @@ void bio_init(struct bio *bio, struct block_device *bdev, struct bio_vec *table,
>  	bio->bi_iocost_cost = 0;
>  #endif
>  #endif
>  #ifdef CONFIG_BLK_INLINE_ENCRYPTION
>  	bio->bi_crypt_context = NULL;
> +#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
> +	bio->bi_skip_dm_default_key = false;
> +#endif
>  #endif
>  #ifdef CONFIG_BLK_DEV_INTEGRITY
>  	bio->bi_integrity = NULL;
>  #endif
>  	bio->bi_vcnt = 0;
> diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
> index b1e7415f8439c..dd5f1edcc44b3 100644
> --- a/block/blk-crypto-fallback.c
> +++ b/block/blk-crypto-fallback.c
> @@ -179,10 +179,12 @@ static struct bio *blk_crypto_fallback_clone_bio(struct bio *bio_src)
>  	bio_for_each_segment(bv, bio_src, iter)
>  		bio->bi_io_vec[bio->bi_vcnt++] = bv;
>  
>  	bio_clone_blkg_association(bio, bio_src);
>  
> +	bio_clone_skip_dm_default_key(bio, bio_src);
> +
>  	return bio;
>  }
>  
>  static bool
>  blk_crypto_fallback_alloc_cipher_req(struct blk_crypto_keyslot *slot,
> diff --git a/include/linux/blk-crypto.h b/include/linux/blk-crypto.h
> index 5e5822c18ee41..f1f3d546c53e5 100644
> --- a/include/linux/blk-crypto.h
> +++ b/include/linux/blk-crypto.h
> @@ -110,10 +110,13 @@ static inline bool bio_has_crypt_ctx(struct bio *bio)
>  	return false;
>  }
>  
>  #endif /* CONFIG_BLK_INLINE_ENCRYPTION */
>  
> +static inline void bio_clone_skip_dm_default_key(struct bio *dst,
> +						 const struct bio *src);
> +
>  int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
>  /**
>   * bio_crypt_clone - clone bio encryption context
>   * @dst: destination bio
>   * @src: source bio
> @@ -125,11 +128,44 @@ int __bio_crypt_clone(struct bio *dst, struct bio *src, gfp_t gfp_mask);
>   *	   @gfp_mask doesn't include %__GFP_DIRECT_RECLAIM.
>   */
>  static inline int bio_crypt_clone(struct bio *dst, struct bio *src,
>  				  gfp_t gfp_mask)
>  {
> +	bio_clone_skip_dm_default_key(dst, src);
>  	if (bio_has_crypt_ctx(src))
>  		return __bio_crypt_clone(dst, src, gfp_mask);
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
> +static inline void bio_set_skip_dm_default_key(struct bio *bio)
> +{
> +	bio->bi_skip_dm_default_key = true;
> +}
> +
> +static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
> +{
> +	return bio->bi_skip_dm_default_key;
> +}
> +
> +static inline void bio_clone_skip_dm_default_key(struct bio *dst,
> +						 const struct bio *src)
> +{
> +	dst->bi_skip_dm_default_key = src->bi_skip_dm_default_key;
> +}
> +#else /* CONFIG_DM_DEFAULT_KEY */
> +static inline void bio_set_skip_dm_default_key(struct bio *bio)
> +{
> +}
> +
> +static inline bool bio_should_skip_dm_default_key(const struct bio *bio)
> +{
> +	return false;
> +}
> +
> +static inline void bio_clone_skip_dm_default_key(struct bio *dst,
> +						 const struct bio *src)
> +{
> +}
> +#endif /* !CONFIG_DM_DEFAULT_KEY */
> +
>  #endif /* __LINUX_BLK_CRYPTO_H */
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index dce7615c35e7e..2ee6a7e570796 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -247,10 +247,13 @@ struct bio {
>  #endif
>  #endif
>  
>  #ifdef CONFIG_BLK_INLINE_ENCRYPTION
>  	struct bio_crypt_ctx	*bi_crypt_context;
> +#if IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
> +	bool			bi_skip_dm_default_key;
> +#endif
>  #endif
>  
>  #if defined(CONFIG_BLK_DEV_INTEGRITY)
>  	struct bio_integrity_payload *bi_integrity; /* data integrity */
>  #endif
> -- 
> 2.47.0
> 

Re: [RFC PATCH 2/4] block: add the bi_skip_dm_default_key flag

[Eric Biggers]

On Mon, Oct 21, 2024 at 01:11:36PM +0200, Mikulas Patocka wrote:
> Hi
> 
> What about using the REQ_META flag (it is set on metadata bios and cleared 
> on data bios), instead of adding a new flag with the same meaning?
> 
> Mikulas

REQ_META is a hint and is not used for all metadata.

And while metadata is the main point, more precisely the goal is to encrypt
every block that isn't already encrypted.  That means that the contents of files
that are unencrypted at the filesystem layer are encrypted by dm-default-key
too.  So technically it's more than just metadata.

To avoid recurring "oops, we forgot to encrypt this" bugs, the right model is
really an opt-out flag, not opt-in.  And especially not opt-in via something
that is currently just a hint and is used as such.

- Eric

[RFC PATCH 3/4] dm-default-key: add target for filesystem metadata encryption

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add a new device-mapper target "dm-default-key" that is similar to
dm-crypt but has two main differences:

- It uses the blk-crypto API instead of the regular crypto API.  This
  allows it to take advantage of inline encryption hardware such as that
  commonly built into UFS host controllers.

- It supports a passthrough flag, which will be used by ext4 and f2fs to
  avoid double encryption of encrypted files.  In this setup,
  dm-default-key provides the "metadata encryption" layer.

The table syntax matches dm-crypt's, but for now only a stripped-down
set of parameters is supported.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 drivers/md/Kconfig          |  20 ++
 drivers/md/Makefile         |   1 +
 drivers/md/dm-default-key.c | 431 ++++++++++++++++++++++++++++++++++++
 3 files changed, 452 insertions(+)
 create mode 100644 drivers/md/dm-default-key.c

diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 1e9db8e4acdf6..a9f35a8efbade 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -268,10 +268,30 @@ config DM_CRYPT
 	  To compile this code as a module, choose M here: the module will
 	  be called dm-crypt.
 
 	  If unsure, say N.
 
+config DM_DEFAULT_KEY
+	tristate "Default-key target support"
+	depends on BLK_DEV_DM
+	depends on BLK_INLINE_ENCRYPTION
+	# dm-default-key doesn't require -o inlinecrypt, but it does currently
+	# rely on the inline encryption hooks being built into the kernel.
+	depends on FS_ENCRYPTION_INLINE_CRYPT
+	help
+	  This device-mapper target allows you to create a device that
+	  assigns a default encryption key to bios that aren't for the
+	  contents of an encrypted file.
+
+	  This ensures that all blocks on-disk will be encrypted with
+	  some key, without the performance hit of file contents being
+	  encrypted twice when fscrypt is used.
+
+	  It is only appropriate to use dm-default-key when key
+	  configuration is tightly controlled such that all fscrypt keys
+	  are at least as hard to compromise as the default key.
+
 config DM_SNAPSHOT
        tristate "Snapshot target"
        depends on BLK_DEV_DM
        select DM_BUFIO
 	help
diff --git a/drivers/md/Makefile b/drivers/md/Makefile
index 476a214e4bdc2..1dc4b20b506db 100644
--- a/drivers/md/Makefile
+++ b/drivers/md/Makefile
@@ -49,10 +49,11 @@ obj-$(CONFIG_BLK_DEV_DM)	+= dm-mod.o
 obj-$(CONFIG_BLK_DEV_DM_BUILTIN) += dm-builtin.o
 obj-$(CONFIG_DM_UNSTRIPED)	+= dm-unstripe.o
 obj-$(CONFIG_DM_BUFIO)		+= dm-bufio.o
 obj-$(CONFIG_DM_BIO_PRISON)	+= dm-bio-prison.o
 obj-$(CONFIG_DM_CRYPT)		+= dm-crypt.o
+obj-$(CONFIG_DM_DEFAULT_KEY)	+= dm-default-key.o
 obj-$(CONFIG_DM_DELAY)		+= dm-delay.o
 obj-$(CONFIG_DM_DUST)		+= dm-dust.o
 obj-$(CONFIG_DM_FLAKEY)		+= dm-flakey.o
 obj-$(CONFIG_DM_MULTIPATH)	+= dm-multipath.o dm-round-robin.o
 obj-$(CONFIG_DM_MULTIPATH_QL)	+= dm-queue-length.o
diff --git a/drivers/md/dm-default-key.c b/drivers/md/dm-default-key.c
new file mode 100644
index 0000000000000..f5533418b4456
--- /dev/null
+++ b/drivers/md/dm-default-key.c
@@ -0,0 +1,431 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Google LLC
+ */
+
+#include <linux/blk-crypto.h>
+#include <linux/device-mapper.h>
+#include <linux/module.h>
+
+#define DM_MSG_PREFIX		"default-key"
+
+static const struct dm_default_key_cipher {
+	const char *name;
+	enum blk_crypto_mode_num mode_num;
+	int key_size;
+} dm_default_key_ciphers[] = {
+	{
+		.name = "aes-xts-plain64",
+		.mode_num = BLK_ENCRYPTION_MODE_AES_256_XTS,
+		.key_size = 64,
+	}, {
+		.name = "xchacha12,aes-adiantum-plain64",
+		.mode_num = BLK_ENCRYPTION_MODE_ADIANTUM,
+		.key_size = 32,
+	},
+};
+
+/**
+ * struct dm_default_c - private data of a default-key target
+ * @dev: the underlying device
+ * @start: starting sector of the range of @dev which this target actually maps.
+ *	   For this purpose a "sector" is 512 bytes.
+ * @cipher_string: the name of the encryption algorithm being used
+ * @iv_offset: starting offset for IVs.  IVs are generated as if the target were
+ *	       preceded by @iv_offset 512-byte sectors.
+ * @sector_size: crypto sector size in bytes (usually 4096)
+ * @sector_bits: log2(sector_size)
+ * @key: the encryption key to use
+ * @max_dun: the maximum DUN that may be used (computed from other params)
+ */
+struct default_key_c {
+	struct dm_dev *dev;
+	sector_t start;
+	const char *cipher_string;
+	u64 iv_offset;
+	unsigned int sector_size;
+	unsigned int sector_bits;
+	struct blk_crypto_key key;
+	u64 max_dun;
+};
+
+static const struct dm_default_key_cipher *
+lookup_cipher(const char *cipher_string)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dm_default_key_ciphers); i++) {
+		if (strcmp(cipher_string, dm_default_key_ciphers[i].name) == 0)
+			return &dm_default_key_ciphers[i];
+	}
+	return NULL;
+}
+
+static void default_key_dtr(struct dm_target *ti)
+{
+	struct default_key_c *dkc = ti->private;
+
+	if (dkc->dev) {
+		if (dkc->key.size)
+			blk_crypto_evict_key(dkc->dev->bdev, &dkc->key);
+		dm_put_device(ti, dkc->dev);
+	}
+	kfree_sensitive(dkc->cipher_string);
+	kfree_sensitive(dkc);
+}
+
+static int default_key_ctr_optional(struct dm_target *ti,
+				    unsigned int argc, char **argv)
+{
+	struct default_key_c *dkc = ti->private;
+	struct dm_arg_set as;
+	static const struct dm_arg _args[] = {
+		{0, 3, "Invalid number of feature args"},
+	};
+	unsigned int opt_params;
+	const char *opt_string;
+	bool iv_large_sectors = false;
+	char dummy;
+	int err;
+
+	as.argc = argc;
+	as.argv = argv;
+
+	err = dm_read_arg_group(_args, &as, &opt_params, &ti->error);
+	if (err)
+		return err;
+
+	while (opt_params--) {
+		opt_string = dm_shift_arg(&as);
+		if (!opt_string) {
+			ti->error = "Not enough feature arguments";
+			return -EINVAL;
+		}
+		if (!strcmp(opt_string, "allow_discards")) {
+			ti->num_discard_bios = 1;
+		} else if (sscanf(opt_string, "sector_size:%u%c",
+				  &dkc->sector_size, &dummy) == 1) {
+			if (dkc->sector_size < SECTOR_SIZE ||
+			    dkc->sector_size > 4096 ||
+			    !is_power_of_2(dkc->sector_size)) {
+				ti->error = "Invalid sector_size";
+				return -EINVAL;
+			}
+		} else if (!strcmp(opt_string, "iv_large_sectors")) {
+			iv_large_sectors = true;
+		} else {
+			ti->error = "Invalid feature arguments";
+			return -EINVAL;
+		}
+	}
+
+	/* dm-default-key doesn't implement iv_large_sectors=false. */
+	if (dkc->sector_size != SECTOR_SIZE && !iv_large_sectors) {
+		ti->error = "iv_large_sectors must be specified";
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/*
+ * Construct a default-key mapping:
+ * <cipher> <key> <iv_offset> <dev_path> <start>
+ *
+ * This syntax matches dm-crypt's, but the set of supported functionality has
+ * been stripped down.
+ */
+static int default_key_ctr(struct dm_target *ti, unsigned int argc, char **argv)
+{
+	struct default_key_c *dkc;
+	const struct dm_default_key_cipher *cipher;
+	u8 raw_key[BLK_CRYPTO_MAX_KEY_SIZE];
+	unsigned int dun_bytes;
+	unsigned long long tmpll;
+	char dummy;
+	int err;
+
+	if (argc < 5) {
+		ti->error = "Not enough arguments";
+		return -EINVAL;
+	}
+
+	dkc = kzalloc(sizeof(*dkc), GFP_KERNEL);
+	if (!dkc) {
+		ti->error = "Out of memory";
+		return -ENOMEM;
+	}
+	ti->private = dkc;
+
+	/* <cipher> */
+	dkc->cipher_string = kstrdup(argv[0], GFP_KERNEL);
+	if (!dkc->cipher_string) {
+		ti->error = "Out of memory";
+		err = -ENOMEM;
+		goto bad;
+	}
+	cipher = lookup_cipher(dkc->cipher_string);
+	if (!cipher) {
+		ti->error = "Unsupported cipher";
+		err = -EINVAL;
+		goto bad;
+	}
+
+	/* <key> */
+	if (strlen(argv[1]) != 2 * cipher->key_size) {
+		ti->error = "Incorrect key size for cipher";
+		err = -EINVAL;
+		goto bad;
+	}
+	if (hex2bin(raw_key, argv[1], cipher->key_size) != 0) {
+		ti->error = "Malformed key string";
+		err = -EINVAL;
+		goto bad;
+	}
+
+	/* <iv_offset> */
+	if (sscanf(argv[2], "%llu%c", &dkc->iv_offset, &dummy) != 1) {
+		ti->error = "Invalid iv_offset sector";
+		err = -EINVAL;
+		goto bad;
+	}
+
+	/* <dev_path> */
+	err = dm_get_device(ti, argv[3], dm_table_get_mode(ti->table),
+			    &dkc->dev);
+	if (err) {
+		ti->error = "Device lookup failed";
+		goto bad;
+	}
+
+	/* <start> */
+	if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1 ||
+	    tmpll != (sector_t)tmpll) {
+		ti->error = "Invalid start sector";
+		err = -EINVAL;
+		goto bad;
+	}
+	dkc->start = tmpll;
+
+	if (bdev_is_zoned(dkc->dev->bdev)) {
+		/*
+		 * dm-default-key needs proper sector numbers because they
+		 * determine the IVs with which the data is encrypted.
+		 */
+		DMDEBUG("Zone append operations will be emulated");
+		ti->emulate_zone_append = true;
+	}
+
+	/* optional arguments */
+	dkc->sector_size = SECTOR_SIZE;
+	if (argc > 5) {
+		err = default_key_ctr_optional(ti, argc - 5, &argv[5]);
+		if (err)
+			goto bad;
+	}
+	dkc->sector_bits = ilog2(dkc->sector_size);
+	if (ti->len & ((dkc->sector_size >> SECTOR_SHIFT) - 1)) {
+		ti->error = "Device size is not a multiple of sector_size";
+		err = -EINVAL;
+		goto bad;
+	}
+
+	dkc->max_dun = (dkc->iv_offset + ti->len - 1) >>
+		       (dkc->sector_bits - SECTOR_SHIFT);
+	dun_bytes = DIV_ROUND_UP(fls64(dkc->max_dun), 8);
+
+	err = blk_crypto_init_key(&dkc->key, raw_key, cipher->mode_num,
+				  dun_bytes, dkc->sector_size);
+	if (err) {
+		ti->error = "Error initializing blk-crypto key";
+		goto bad;
+	}
+
+	err = blk_crypto_start_using_key(dkc->dev->bdev, &dkc->key);
+	if (err) {
+		ti->error = "Error starting to use blk-crypto";
+		goto bad;
+	}
+
+	ti->num_flush_bios = 1;
+
+	err = 0;
+	goto out;
+
+bad:
+	default_key_dtr(ti);
+out:
+	memzero_explicit(raw_key, sizeof(raw_key));
+	return err;
+}
+
+static int default_key_map(struct dm_target *ti, struct bio *bio)
+{
+	const struct default_key_c *dkc = ti->private;
+	sector_t sector_in_target;
+	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE] = {};
+
+	bio_set_dev(bio, dkc->dev->bdev);
+
+	/*
+	 * If the bio is a device-level request which doesn't target a specific
+	 * sector, there's nothing more to do.
+	 */
+	if (bio_sectors(bio) == 0)
+		return DM_MAPIO_REMAPPED;
+
+	/* Map the bio's sector to the underlying device. (512-byte sectors) */
+	sector_in_target = dm_target_offset(ti, bio->bi_iter.bi_sector);
+	bio->bi_iter.bi_sector = dkc->start + sector_in_target;
+
+	/*
+	 * If the bio should skip dm-default-key (i.e. if it's for an encrypted
+	 * file's contents), or if it doesn't have any data (e.g. if it's a
+	 * DISCARD request), there's nothing more to do.
+	 */
+	if (bio_should_skip_dm_default_key(bio) || !bio_has_data(bio))
+		return DM_MAPIO_REMAPPED;
+
+	/*
+	 * Else, dm-default-key needs to set this bio's encryption context.
+	 * It must not already have one.
+	 */
+	if (WARN_ON_ONCE(bio_has_crypt_ctx(bio)))
+		return DM_MAPIO_KILL;
+
+	/* Calculate the DUN and enforce data-unit (crypto sector) alignment. */
+	dun[0] = dkc->iv_offset + sector_in_target; /* 512-byte sectors */
+	if (dun[0] & ((dkc->sector_size >> SECTOR_SHIFT) - 1))
+		return DM_MAPIO_KILL;
+	dun[0] >>= dkc->sector_bits - SECTOR_SHIFT; /* crypto sectors */
+
+	/*
+	 * This check isn't necessary as we should have calculated max_dun
+	 * correctly, but be safe.
+	 */
+	if (WARN_ON_ONCE(dun[0] > dkc->max_dun))
+		return DM_MAPIO_KILL;
+
+	bio_crypt_set_ctx(bio, &dkc->key, dun, GFP_NOIO);
+
+	return DM_MAPIO_REMAPPED;
+}
+
+static void default_key_status(struct dm_target *ti, status_type_t type,
+			       unsigned int status_flags, char *result,
+			       unsigned int maxlen)
+{
+	const struct default_key_c *dkc = ti->private;
+	unsigned int sz = 0;
+	int num_feature_args = 0;
+
+	switch (type) {
+	case STATUSTYPE_INFO:
+	case STATUSTYPE_IMA:
+		result[0] = '\0';
+		break;
+
+	case STATUSTYPE_TABLE:
+		/* Omit the key for now. */
+		DMEMIT("%s - %llu %s %llu", dkc->cipher_string, dkc->iv_offset,
+		       dkc->dev->name, (unsigned long long)dkc->start);
+
+		num_feature_args += !!ti->num_discard_bios;
+		if (dkc->sector_size != SECTOR_SIZE)
+			num_feature_args += 2;
+		if (num_feature_args != 0) {
+			DMEMIT(" %d", num_feature_args);
+			if (ti->num_discard_bios)
+				DMEMIT(" allow_discards");
+			if (dkc->sector_size != SECTOR_SIZE) {
+				DMEMIT(" sector_size:%u", dkc->sector_size);
+				DMEMIT(" iv_large_sectors");
+			}
+		}
+		break;
+	}
+}
+
+static int default_key_prepare_ioctl(struct dm_target *ti,
+				     struct block_device **bdev)
+{
+	const struct default_key_c *dkc = ti->private;
+	const struct dm_dev *dev = dkc->dev;
+
+	*bdev = dev->bdev;
+
+	/* Only pass ioctls through if the device sizes match exactly. */
+	return dkc->start != 0 || ti->len != bdev_nr_sectors(dev->bdev);
+}
+
+static int default_key_iterate_devices(struct dm_target *ti,
+				       iterate_devices_callout_fn fn,
+				       void *data)
+{
+	const struct default_key_c *dkc = ti->private;
+
+	return fn(ti, dkc->dev, dkc->start, ti->len, data);
+}
+
+#ifdef CONFIG_BLK_DEV_ZONED
+static int default_key_report_zones(struct dm_target *ti,
+				    struct dm_report_zones_args *args,
+				    unsigned int nr_zones)
+{
+	const struct default_key_c *dkc = ti->private;
+
+	return dm_report_zones(dkc->dev->bdev, dkc->start,
+			dkc->start + dm_target_offset(ti, args->next_sector),
+			args, nr_zones);
+}
+#else
+#define default_key_report_zones NULL
+#endif
+
+static void default_key_io_hints(struct dm_target *ti,
+				 struct queue_limits *limits)
+{
+	const struct default_key_c *dkc = ti->private;
+	const unsigned int sector_size = dkc->sector_size;
+
+	limits->logical_block_size =
+		max_t(unsigned int, limits->logical_block_size, sector_size);
+	limits->physical_block_size =
+		max_t(unsigned int, limits->physical_block_size, sector_size);
+	limits->io_min = max_t(unsigned int, limits->io_min, sector_size);
+	limits->dma_alignment = limits->logical_block_size - 1;
+}
+
+static struct target_type default_key_target = {
+	.name			= "default-key",
+	.version		= {1, 0, 0},
+	.features		= DM_TARGET_PASSES_CRYPTO | DM_TARGET_ZONED_HM,
+	.module			= THIS_MODULE,
+	.ctr			= default_key_ctr,
+	.dtr			= default_key_dtr,
+	.map			= default_key_map,
+	.status			= default_key_status,
+	.prepare_ioctl		= default_key_prepare_ioctl,
+	.iterate_devices	= default_key_iterate_devices,
+	.report_zones		= default_key_report_zones,
+	.io_hints		= default_key_io_hints,
+};
+
+static int __init dm_default_key_init(void)
+{
+	return dm_register_target(&default_key_target);
+}
+
+static void __exit dm_default_key_exit(void)
+{
+	dm_unregister_target(&default_key_target);
+}
+
+module_init(dm_default_key_init);
+module_exit(dm_default_key_exit);
+
+MODULE_AUTHOR("Paul Lawrence <paullawrence@google.com>");
+MODULE_AUTHOR("Paul Crowley <paulcrowley@google.com>");
+MODULE_AUTHOR("Eric Biggers <ebiggers@google.com>");
+MODULE_DESCRIPTION(DM_NAME " target for encrypting filesystem metadata");
+MODULE_LICENSE("GPL");
-- 
2.47.0

[RFC PATCH 4/4] ext4,f2fs: support metadata encryption via dm-default-key

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Set the bi_skip_dm_default_key flag on bios that are targeting the
contents of an encrypted file and therefore should not be en/decrypted
by dm-default-key.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/inline_crypt.c | 14 +++++++++++++-
 fs/f2fs/data.c           |  6 +++++-
 include/linux/fscrypt.h  | 14 ++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 40de69860dcf9..b75c69c09500b 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -261,17 +261,22 @@ static void fscrypt_generate_dun(const struct fscrypt_inode_info *ci,
  *
  * Normally the bio should be newly allocated (i.e. no pages added yet), as
  * otherwise fscrypt_mergeable_bio() won't work as intended.
  *
  * The encryption context will be freed automatically when the bio is freed.
+ *
+ * This function also handles setting bi_skip_dm_default_key when needed.
  */
 void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
 			       u64 first_lblk, gfp_t gfp_mask)
 {
 	const struct fscrypt_inode_info *ci;
 	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
 
+	if (fscrypt_inode_should_skip_dm_default_key(inode))
+		bio_set_skip_dm_default_key(bio);
+
 	if (!fscrypt_inode_uses_inline_crypto(inode))
 		return;
 	ci = inode->i_crypt_info;
 
 	fscrypt_generate_dun(ci, first_lblk, dun);
@@ -342,20 +347,26 @@ EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
  *
  * This function isn't required in cases where crypto-mergeability is ensured in
  * another way, such as I/O targeting only a single file (and thus a single key)
  * combined with fscrypt_limit_io_blocks() to ensure DUN contiguity.
  *
+ * This function also returns false if the next part of the I/O would need to
+ * have a different value for the bi_skip_dm_default_key flag.
+ *
  * Return: true iff the I/O is mergeable
  */
 bool fscrypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 			   u64 next_lblk)
 {
 	const struct bio_crypt_ctx *bc = bio->bi_crypt_context;
 	u64 next_dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
 
 	if (!!bc != fscrypt_inode_uses_inline_crypto(inode))
 		return false;
+	if (bio_should_skip_dm_default_key(bio) !=
+	    fscrypt_inode_should_skip_dm_default_key(inode))
+		return false;
 	if (!bc)
 		return true;
 
 	/*
 	 * Comparing the key pointers is good enough, as all I/O for each key
@@ -385,11 +396,12 @@ bool fscrypt_mergeable_bio_bh(struct bio *bio,
 {
 	const struct inode *inode;
 	u64 next_lblk;
 
 	if (!bh_get_inode_and_lblk_num(next_bh, &inode, &next_lblk))
-		return !bio->bi_crypt_context;
+		return !bio->bi_crypt_context &&
+		       !bio_should_skip_dm_default_key(bio);
 
 	return fscrypt_mergeable_bio(bio, inode, next_lblk);
 }
 EXPORT_SYMBOL_GPL(fscrypt_mergeable_bio_bh);
 
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 94f7b084f6016..a413508210994 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -484,10 +484,12 @@ static void f2fs_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
 	 * The f2fs garbage collector sets ->encrypted_page when it wants to
 	 * read/write raw data without encryption.
 	 */
 	if (!fio || !fio->encrypted_page)
 		fscrypt_set_bio_crypt_ctx(bio, inode, first_idx, gfp_mask);
+	else if (fscrypt_inode_should_skip_dm_default_key(inode))
+		bio_set_skip_dm_default_key(bio);
 }
 
 static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 				     pgoff_t next_idx,
 				     const struct f2fs_io_info *fio)
@@ -495,11 +497,13 @@ static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
 	/*
 	 * The f2fs garbage collector sets ->encrypted_page when it wants to
 	 * read/write raw data without encryption.
 	 */
 	if (fio && fio->encrypted_page)
-		return !bio_has_crypt_ctx(bio);
+		return !bio_has_crypt_ctx(bio) &&
+			(bio_should_skip_dm_default_key(bio) ==
+			 fscrypt_inode_should_skip_dm_default_key(inode));
 
 	return fscrypt_mergeable_bio(bio, inode, next_idx);
 }
 
 void f2fs_submit_read_bio(struct f2fs_sb_info *sbi, struct bio *bio,
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 772f822dc6b82..eac1917db79a9 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -890,10 +890,24 @@ static inline u64 fscrypt_limit_io_blocks(const struct inode *inode, u64 lblk,
 {
 	return nr_blocks;
 }
 #endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
 
+#if IS_ENABLED(CONFIG_FS_ENCRYPTION) && IS_ENABLED(CONFIG_DM_DEFAULT_KEY)
+static inline bool
+fscrypt_inode_should_skip_dm_default_key(const struct inode *inode)
+{
+	return IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode);
+}
+#else
+static inline bool
+fscrypt_inode_should_skip_dm_default_key(const struct inode *inode)
+{
+	return false;
+}
+#endif
+
 /**
  * fscrypt_inode_uses_inline_crypto() - test whether an inode uses inline
  *					encryption
  * @inode: an inode. If encrypted, its key must be set up.
  *
-- 
2.47.0

Re: [RFC PATCH 0/4] dm-default-key: target for filesystem metadata encryption

[Mikulas Patocka]

On Fri, 18 Oct 2024, Eric Biggers wrote:

> This series adds "metadata encryption" support to ext4 and f2fs via a
> new device-mapper target dm-default-key.  dm-default-key encrypts all
> data on a block device that isn't already encrypted by the filesystem.
> 
> Except for the passthrough support, dm-default-key is basically the same
> as the proposed dm-inlinecrypt which omits that feature
> (https://lore.kernel.org/dm-devel/20241016232748.134211-1-ebiggers@kernel.org/).
> 
> I am sending this out for reference, as dm-default-key (which Android
> has been using for a while) hasn't previously been sent to the lists in
> full, and there has been interest in it.  However, my current impression
> is that this feature will need to be redesigned as a filesystem native
> feature in order to make it upstream.  If that is indeed the case, then
> IMO it would make sense to merge dm-inlinecrypt in the mean time instead
> (or add its functionality to dm-crypt) so that anyone who just wants
> "dm-crypt + inline encryption hardware" gets a solution for that.

I we merge dm-inlinecrypt, we can't remove it later because users will 
depend on it. I think it is not sensible to have two targets 
(dm-inlinecrypt and dm-default-key) that do almost the same thing.

I've got another idea - what about a new target "dm-metadata-switch" that 
will take two block devices as arguments and it will pass metadata bios to 
the first device and data bios to the second device - so that the logic 
to decide where the bio will go would be decoupled from the encryption. 
Then, you can put dm-crypt or dm-inlinecrypt underneath 
"dm-metadata-switch".

----------------------
|     filesystem     |
----------------------
          |
          V
----------------------
| dm-metadata-switch |
----------------------
      |           |
      V           |
------------      |
| dm-crypt |      |
------------      |
      |           |
      V           V
-------------------------
| physical block device |
-------------------------

Mikulas

Re: [RFC PATCH 0/4] dm-default-key: target for filesystem metadata encryption

[Eric Biggers]

On Mon, Oct 21, 2024 at 01:52:58PM +0200, Mikulas Patocka wrote:
> On Fri, 18 Oct 2024, Eric Biggers wrote:
> 
> > This series adds "metadata encryption" support to ext4 and f2fs via a
> > new device-mapper target dm-default-key.  dm-default-key encrypts all
> > data on a block device that isn't already encrypted by the filesystem.
> > 
> > Except for the passthrough support, dm-default-key is basically the same
> > as the proposed dm-inlinecrypt which omits that feature
> > (https://lore.kernel.org/dm-devel/20241016232748.134211-1-ebiggers@kernel.org/).
> > 
> > I am sending this out for reference, as dm-default-key (which Android
> > has been using for a while) hasn't previously been sent to the lists in
> > full, and there has been interest in it.  However, my current impression
> > is that this feature will need to be redesigned as a filesystem native
> > feature in order to make it upstream.  If that is indeed the case, then
> > IMO it would make sense to merge dm-inlinecrypt in the mean time instead
> > (or add its functionality to dm-crypt) so that anyone who just wants
> > "dm-crypt + inline encryption hardware" gets a solution for that.
> 
> I we merge dm-inlinecrypt, we can't remove it later because users will 
> depend on it. I think it is not sensible to have two targets 
> (dm-inlinecrypt and dm-default-key) that do almost the same thing.

The code would not need to be duplicated, though.  E.g. dm-default-key
functionality could be added as an enable_passthrough option to dm-inlinecrypt.
Or the same .c file could register both targets sharing most of the same code.

> I've got another idea - what about a new target "dm-metadata-switch" that 
> will take two block devices as arguments and it will pass metadata bios to 
> the first device and data bios to the second device - so that the logic 
> to decide where the bio will go would be decoupled from the encryption. 
> Then, you can put dm-crypt or dm-inlinecrypt underneath 
> "dm-metadata-switch".
> 
> ----------------------
> |     filesystem     |
> ----------------------
>           |
>           V
> ----------------------
> | dm-metadata-switch |
> ----------------------
>       |           |
>       V           |
> ------------      |
> | dm-crypt |      |
> ------------      |
>       |           |
>       V           V
> -------------------------
> | physical block device |
> -------------------------

Would this have any use case other than what dm-default-key does?

Keep in mind that dm-metadata-switch would have to pass through all sector
addresses unchanged.  So if you wanted to reuse this to actually put your
filesystem metadata on one disk and data on another, this wouldn't be very
effective at that, as both data and metadata would take up the full space.

- Eric