Skip to contentSkip to conversation

fscrypt: support for inline-encryption-optimized policies

PATCH In review Eric Biggers 6 participants 18 messages Oct 21, 2019 – Oct 24, 2019

Subsystems (5)
lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

cover fscrypt: support for inline-encryption-optimized policies

Hello,

In preparation for adding inline encryption support to fscrypt, this patchset adds a new fscrypt policy flag which modifies the encryption to be optimized for inline encryption hardware compliant with the UFS standard --- specifically, to use a smaller number of keys while still using at most 64 IV bits. This required adding the inode number to the IVs. For ext4, this precludes filesystem shrinking, so I've also added a compat feature which will prevent the filesystem from being shrunk.

I've separated this from the full "Inline Encryption Support" patchset
(https://lkml.kernel.org/linux-fsdevel/[email protected]/)
to avoid conflating an implementation (inline encryption) with a new on-disk format (INLINE_CRYPT_OPTIMIZED). This patchset purely adds support for INLINE_CRYPT_OPTIMIZED policies to fscrypt, but implements them using the existing filesystem layer crypto.

We're planning to make the *implementation* (filesystem layer or inline crypto) be controlled by a mount option '-o inlinecrypt'.

This patchset applies to fscrypt.git#master and can also be retrieved from
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=inline-crypt-optimized-v1

A ciphertext verification test for INLINE_CRYPT_OPTIMIZED policies can be found at
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git/log/?h=inline-encryption

Work-in-progress patches for the inline encryption implementation of both INLINE_CRYPT_OPTIMIZED and regular policies can be found at
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=inline-encryption-wip

Eric Biggers (3):
fscrypt: add support for inline-encryption-optimized policies
ext4: add support for INLINE_CRYPT_OPTIMIZED encryption policies
f2fs: add support for INLINE_CRYPT_OPTIMIZED encryption policies

 Documentation/filesystems/fscrypt.rst | 53 +++++++++++++++++++--------
 fs/crypto/crypto.c                    | 11 +++++-
 fs/crypto/fscrypt_private.h           | 20 +++++++---
 fs/crypto/keyring.c                   |  6 ++-
 fs/crypto/keysetup.c                  | 47 +++++++++++++++++++-----
 fs/crypto/policy.c                    | 42 ++++++++++++++++++++-
 fs/ext4/ext4.h                        |  2 +
 fs/ext4/super.c                       | 14 +++++++
 fs/f2fs/super.c                       | 26 ++++++++++---
 include/linux/fscrypt.h               |  3 ++
 include/uapi/linux/fscrypt.h          | 15 ++++----
 11 files changed, 191 insertions(+), 48 deletions(-)
Signature · 1 line
2.23.0.866.gb869b98d4c-goog
author maintainer
#1 lore raw

PATCH 1/3 fscrypt: add support for inline-encryption-optimized policies

From: Eric Biggers <[email protected]>

Some inline encryption hardware has only a small number of keyslots, which would make it inefficient to use the traditional fscrypt per-file keys. The existing DIRECT_KEY encryption policy flag doesn't solve this because it assumes that file contents and names are encrypted by the same algorithm and that IVs are at least 24 bytes.

Therefore, add a new encryption policy flag INLINE_CRYPT_OPTIMIZED which causes the encryption to modified as follows:

- The key for file contents encryption is derived from the values (master_key, mode_num, filesystem_uuid). The per-file nonce is not included, so many files may share the same contents encryption key.

- The IV for encrypting each block of file contents is built as (inode_number << 32) | file_logical_block_num.

Including the inode number in the IVs ensures that data in different files is encrypted differently, despite per-file keys not being used. Limiting the inode and block numbers to 32 bits and putting the block number in the low bits is needed to be compatible with inline encryption hardware which only supports specifying a 64-bit data unit number which is auto-incremented; this is what the UFS and EMMC standards support.

This IV generation method places extra constraints on filesystems: inode numbers must be stable, which may preclude filesystem shrinking; and inode and file logical block numbers can be at most 32-bit. Therefore, only allow INLINE_CRYPT_OPTIMIZED policies on filesystems that declare that they meet these constraints.

Note that INLINE_CRYPT_OPTIMIZED is an on-disk format, not an implementation; it doesn't require the use of inline encryption. This patch adds support for it using the existing filesystem layer encryption. A later patch will add inline encryption support.

---
 Documentation/filesystems/fscrypt.rst | 53 +++++++++++++++++++--------
 fs/crypto/crypto.c                    | 11 +++++-
 fs/crypto/fscrypt_private.h           | 20 +++++++---
 fs/crypto/keyring.c                   |  6 ++-
 fs/crypto/keysetup.c                  | 47 +++++++++++++++++++-----
 fs/crypto/policy.c                    | 42 ++++++++++++++++++++-
 include/linux/fscrypt.h               |  3 ++
 include/uapi/linux/fscrypt.h          | 15 ++++----
 8 files changed, 155 insertions(+), 42 deletions(-)
Documentation/filesystems/fscrypt.rst +38 -15
index 6ec459be3de16..d059185f79ba9 100644
@@ -257,12 +257,13 @@ the master keys may be wrapped in userspace, e.g. as is done by the
 `fscrypt <https://github.com/google/fscrypt>`_ tool.
 
 Including the inode number in the IVs was considered.  However, it was
-rejected as it would have prevented ext4 filesystems from being
-resized, and by itself still wouldn't have been sufficient to prevent
-the same key from being directly reused for both XTS and CTS-CBC.
+rejected (except for INLINE_CRYPT_OPTIMIZED policies) as it would have
+prevented ext4 filesystems from being resized, and by itself still
+wouldn't have been sufficient to prevent the same key from being
+directly reused for both XTS and CTS-CBC.
 
-DIRECT_KEY and per-mode keys
-----------------------------
+DIRECT_KEY policies
+-------------------
 
 The Adiantum encryption mode (see `Encryption modes and usage`_) is
 suitable for both contents and filenames encryption, and it accepts
@@ -285,6 +286,22 @@ IV.  Moreover:
   key derived using the KDF.  Users may use the same master key for
   other v2 encryption policies.
 
+INLINE_CRYPT_OPTIMIZED policies
+-------------------------------
+
+When FSCRYPT_POLICY_FLAG_INLINE_CRYPT_OPTIMIZED is set in an
+encryption policy, file contents are encrypted using a key derived
+from the master key, encryption mode number, and filesystem UUID.  In
+this case, many files may share the same key.  To still encrypt
+different files' data differently, inode numbers are included in the
+IVs.  Consequently, shrinking the filesystem may not be allowed.
+
+This format is optimized for use with inline encryption hardware that
+supports only a small number of keyslots and allows specifying only 8
+IV bytes, e.g. the crypto capabilities in the UFSHCI v2.1+ standard.
+
+This flag has no effect on filenames encryption.
+
 Key identifiers
 ---------------
 
@@ -342,11 +359,14 @@ a little endian number, except that:
   is encrypted with AES-256 where the AES-256 key is the SHA-256 hash
   of the file's data encryption key.
 
-- In the "direct key" configuration (FSCRYPT_POLICY_FLAG_DIRECT_KEY
-  set in the fscrypt_policy), the file's nonce is also appended to the
-  IV.  Currently this is only allowed with the Adiantum encryption
+- With `DIRECT_KEY policies`_, the file's nonce is also appended to
+  the IV.  Currently this is only allowed with the Adiantum encryption
   mode.
 
+- With `INLINE_CRYPT_OPTIMIZED policies`_, the block number is limited
+  to 32 bits and is placed in bits 0-31 of the IV.  The inode number
+  (which is also limited to 32 bits) is placed in bits 32-63.
+
 Filenames encryption
 --------------------
 
@@ -432,12 +452,15 @@ This structure must be initialized as follows:
   (1) for ``contents_encryption_mode`` and FSCRYPT_MODE_AES_256_CTS
   (4) for ``filenames_encryption_mode``.
 
-- ``flags`` must contain a value from ``<linux/fscrypt.h>`` which
-  identifies the amount of NUL-padding to use when encrypting
-  filenames.  If unsure, use FSCRYPT_POLICY_FLAGS_PAD_32 (0x3).
-  Additionally, if the encryption modes are both
-  FSCRYPT_MODE_ADIANTUM, this can contain
-  FSCRYPT_POLICY_FLAG_DIRECT_KEY; see `DIRECT_KEY and per-mode keys`_.
+- ``flags`` contains optional flags from ``<linux/fscrypt.h>``:
+
+  - FSCRYPT_POLICY_FLAGS_PAD_*: The amount of NUL padding to use when
+    encrypting filenames.  If unsure, use FSCRYPT_POLICY_FLAGS_PAD_32
+    (0x3).
+  - FSCRYPT_POLICY_FLAG_DIRECT_KEY: See `DIRECT_KEY policies`_.
+  - FSCRYPT_POLICY_FLAG_INLINE_CRYPT_OPTIMIZED: See
+    `INLINE_CRYPT_OPTIMIZED policies`_.  This is mutually exclusive
+    with DIRECT_KEY and is not supported on v1 policies.
 
 - For v2 encryption policies, ``__reserved`` must be zeroed.
 
@@ -1090,7 +1113,7 @@ policy structs (see `Setting an encryption policy`_), except that the
 context structs also contain a nonce.  The nonce is randomly generated
 by the kernel and is used as KDF input or as a tweak to cause
 different files to be encrypted differently; see `Per-file keys`_ and
-`DIRECT_KEY and per-mode keys`_.
+`DIRECT_KEY policies`_.
 
 Data path changes
 -----------------
fs/crypto/crypto.c +9 -2
index ced8ad9f2d019..6be16837accdd 100644
@@ -73,11 +73,18 @@ EXPORT_SYMBOL(fscrypt_free_bounce_page);
 void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
 			 const struct fscrypt_info *ci)
 {
+	u8 flags = fscrypt_policy_flags(&ci->ci_policy);
+
 	memset(iv, 0, ci->ci_mode->ivsize);
-	iv->lblk_num = cpu_to_le64(lblk_num);
 
-	if (fscrypt_is_direct_key_policy(&ci->ci_policy))
+	if ((flags & FSCRYPT_POLICY_FLAG_INLINE_CRYPT_OPTIMIZED) &&
+	    S_ISREG(ci->ci_inode->i_mode)) {
+		WARN_ON_ONCE((u32)lblk_num != lblk_num);
+		lblk_num |= (u64)ci->ci_inode->i_ino << 32;
+	} else if (flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY) {
 		memcpy(iv->nonce, ci->ci_nonce, FS_KEY_DERIVATION_NONCE_SIZE);
+	}
+	iv->lblk_num = cpu_to_le64(lblk_num);
 }
 
 /* Encrypt or decrypt a single filesystem block of file contents */
fs/crypto/fscrypt_private.h +15 -5
index dacf8fcbac3be..3428488612b84 100644
@@ -163,6 +163,9 @@ struct fscrypt_info {
 	/* The actual crypto transform used for encryption and decryption */
 	struct crypto_skcipher *ci_ctfm;
 
+	/* True if the key should be freed when this fscrypt_info is freed */
+	bool ci_owns_key;
+
 	/*
 	 * Encryption mode used for this inode.  It corresponds to either the
 	 * contents or filenames encryption mode, depending on the inode type.
@@ -279,9 +282,10 @@ extern int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
  * outputs are unique and cryptographically isolated, i.e. knowledge of one
  * output doesn't reveal another.
  */
-#define HKDF_CONTEXT_KEY_IDENTIFIER	1
-#define HKDF_CONTEXT_PER_FILE_KEY	2
-#define HKDF_CONTEXT_PER_MODE_KEY	3
+#define HKDF_CONTEXT_KEY_IDENTIFIER		1
+#define HKDF_CONTEXT_PER_FILE_KEY		2
+#define HKDF_CONTEXT_DIRECT_KEY			3
+#define HKDF_CONTEXT_INLINE_CRYPT_OPTIMIZED_KEY	4
 
 extern int fscrypt_hkdf_expand(struct fscrypt_hkdf *hkdf, u8 context,
 			       const u8 *info, unsigned int infolen,
@@ -378,8 +382,14 @@ struct fscrypt_master_key {
 	struct list_head	mk_decrypted_inodes;
 	spinlock_t		mk_decrypted_inodes_lock;
 
-	/* Per-mode tfms for DIRECT_KEY policies, allocated on-demand */
-	struct crypto_skcipher	*mk_mode_keys[__FSCRYPT_MODE_MAX + 1];
+	/* Crypto API transforms for DIRECT_KEY policies, allocated on-demand */
+	struct crypto_skcipher	*mk_direct_tfms[__FSCRYPT_MODE_MAX + 1];
+
+	/*
+	 * Crypto API transforms for filesystem-layer implementation of
+	 * INLINE_CRYPT_OPTIMIZED policies, allocated on-demand.
+	 */
+	struct crypto_skcipher	*mk_inline_crypt_optimized_tfms[__FSCRYPT_MODE_MAX + 1];
 
 } __randomize_layout;
 
fs/crypto/keyring.c +4 -2
index c34fa7c61b43b..6ca3364b2c968 100644
@@ -43,8 +43,10 @@ static void free_master_key(struct fscrypt_master_key *mk)
 
 	wipe_master_key_secret(&mk->mk_secret);
 
-	for (i = 0; i < ARRAY_SIZE(mk->mk_mode_keys); i++)
-		crypto_free_skcipher(mk->mk_mode_keys[i]);
+	for (i = 0; i <= __FSCRYPT_MODE_MAX; i++) {
+		crypto_free_skcipher(mk->mk_direct_tfms[i]);
+		crypto_free_skcipher(mk->mk_inline_crypt_optimized_tfms[i]);
+	}
 
 	key_put(mk->mk_users);
 	kzfree(mk);
fs/crypto/keysetup.c +37 -10

92 lines not on this page · raw message

fs/crypto/policy.c +41 -1

60 lines not on this page · raw message

include/linux/fscrypt.h +3 -0
index 04f5ed6284454..1a7bffe78ed56 100644
@@ -61,6 +61,9 @@ struct fscrypt_operations {
 	bool (*dummy_context)(struct inode *);
 	bool (*empty_dir)(struct inode *);
 	unsigned int max_namelen;
+	bool (*has_stable_inodes)(struct super_block *sb);
+	void (*get_ino_and_lblk_bits)(struct super_block *sb,
+				      int *ino_bits_ret, int *lblk_bits_ret);
 };
 
 static inline bool fscrypt_has_encryption_key(const struct inode *inode)
include/uapi/linux/fscrypt.h +8 -7
index 39ccfe9311c38..6a8fd8ff2d2ac 100644
@@ -11,13 +11,14 @@
 #include <linux/types.h>
 
 /* Encryption policy flags */
-#define FSCRYPT_POLICY_FLAGS_PAD_4		0x00
-#define FSCRYPT_POLICY_FLAGS_PAD_8		0x01
-#define FSCRYPT_POLICY_FLAGS_PAD_16		0x02
-#define FSCRYPT_POLICY_FLAGS_PAD_32		0x03
-#define FSCRYPT_POLICY_FLAGS_PAD_MASK		0x03
-#define FSCRYPT_POLICY_FLAG_DIRECT_KEY		0x04
-#define FSCRYPT_POLICY_FLAGS_VALID		0x07
+#define FSCRYPT_POLICY_FLAGS_PAD_4			0x00
+#define FSCRYPT_POLICY_FLAGS_PAD_8			0x01
+#define FSCRYPT_POLICY_FLAGS_PAD_16			0x02
+#define FSCRYPT_POLICY_FLAGS_PAD_32			0x03
+#define FSCRYPT_POLICY_FLAGS_PAD_MASK			0x03
+#define FSCRYPT_POLICY_FLAG_DIRECT_KEY			0x04
+#define FSCRYPT_POLICY_FLAG_INLINE_CRYPT_OPTIMIZED	0x08
+#define FSCRYPT_POLICY_FLAGS_VALID			0x0F
 
 /* Encryption algorithms */
 #define FSCRYPT_MODE_AES_256_XTS		1
Signature · 1 line
2.23.0.866.gb869b98d4c-goog
#2 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 18 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Some inline encryption hardware has only a small number of keyslots, which would make it inefficient to use the traditional fscrypt per-file keys. The existing DIRECT_KEY encryption policy flag doesn't solve this because it assumes that file contents and names are encrypted by the same algorithm and that IVs are at least 24 bytes.

Therefore, add a new encryption policy flag INLINE_CRYPT_OPTIMIZED which causes the encryption to modified as follows:

- The key for file contents encryption is derived from the values (master_key, mode_num, filesystem_uuid). The per-file nonce is not included, so many files may share the same contents encryption key.

- The IV for encrypting each block of file contents is built as (inode_number << 32) | file_logical_block_num.

Quoted 6 lines

Including the inode number in the IVs ensures that data in different files is encrypted differently, despite per-file keys not being used. Limiting the inode and block numbers to 32 bits and putting the block number in the low bits is needed to be compatible with inline encryption hardware which only supports specifying a 64-bit data unit number which is auto-incremented; this is what the UFS and EMMC standards support.

These 32 bit size limits seem arbitrary and rules out implementing this on larger filesystems. Why not just hash the 64 bit inode, file offset and block numbers into a single 64 bit value? It is still unique enough for the stated use (i.e. unique IV for each file block) but it doesn't limit what filesystem configurations can actually make use of this functionality....

Cheers,

Dave.

Signature · 2 lines
Dave Chinner
[email protected]
author maintainer
#3 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 26 lines · Dave Chinner
Quoted 18 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Some inline encryption hardware has only a small number of keyslots, which would make it inefficient to use the traditional fscrypt per-file keys. The existing DIRECT_KEY encryption policy flag doesn't solve this because it assumes that file contents and names are encrypted by the same algorithm and that IVs are at least 24 bytes.

Therefore, add a new encryption policy flag INLINE_CRYPT_OPTIMIZED which causes the encryption to modified as follows:

- The key for file contents encryption is derived from the values (master_key, mode_num, filesystem_uuid). The per-file nonce is not included, so many files may share the same contents encryption key.

- The IV for encrypting each block of file contents is built as (inode_number << 32) | file_logical_block_num.

Quoted 6 lines

Including the inode number in the IVs ensures that data in different files is encrypted differently, despite per-file keys not being used. Limiting the inode and block numbers to 32 bits and putting the block number in the low bits is needed to be compatible with inline encryption hardware which only supports specifying a 64-bit data unit number which is auto-incremented; this is what the UFS and EMMC standards support.

Quoted 7 lines

These 32 bit size limits seem arbitrary and rules out implementing this on larger filesystems. Why not just hash the 64 bit inode, file offset and block numbers into a single 64 bit value? It is still unique enough for the stated use (i.e. unique IV for each file block) but it doesn't limit what filesystem configurations can actually make use of this functionality....

That won't work because we need consecutive file blocks to have consecutive IVs as often as possible. The crypto support in the UFS and EMMC standards takes only a single 64-bit "data unit number" (DUN) per request, which the hardware uses as the first 64 bits of the IV and automatically increments for each data unit (i.e. for each filesystem block, in this case).

If every block had some random DUN, we'd have to submit a separate bio for every single block. And they wouldn't be mergable, so each one would cause a separate disk request. That would be really terrible for performance.

Also, a 64 bit hash value isn't sufficiently safe against hash collisions.

An alternative which would work nicely on ext4 and xfs (if xfs supported fscrypt) would be to pass the physical block number as the DUN. However, that wouldn't work at all on f2fs because f2fs moves data blocks around. And since most people who want to use this are using f2fs, f2fs support is essential.

Also keep in mind that the proposed format can still be used on a specific filesystem instance with fewer than 2^32 inodes and blocks, even if that type of filesystem can support more inodes and blocks in general.

- Eric

maintainer
#4 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 5 lines · Eric Biggers

That won't work because we need consecutive file blocks to have consecutive IVs as often as possible. The crypto support in the UFS and EMMC standards takes only a single 64-bit "data unit number" (DUN) per request, which the hardware uses as the first 64 bits of the IV and automatically increments for each data unit (i.e. for each filesystem block, in this case).

It seems very likely that for systems that are using UFS and eMMC (which are overwhelming lower-end devices --- e.g., embedded and mobile handsets) 32-bit inode and logical block numbers will be just fine.

If and when we actually get inline crypto support for server-class systems, hopefully they will support 128-bit DUN's, and/or they will have sufficiently fast key load times such that we can use per-file keying.

Quoted 4 lines

An alternative which would work nicely on ext4 and xfs (if xfs supported fscrypt) would be to pass the physical block number as the DUN. However, that wouldn't work at all on f2fs because f2fs moves data blocks around. And since most people who want to use this are using f2fs, f2fs support is essential.

And that is something fscrypt supports already, so if people really did want to use 64-bit logical block numbers, they could do that, at the cost of giving up the ability to shrink the file system (which XFS doesn't support anyway....)

- Ted

_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

author maintainer
#5 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 9 lines · Theodore Y. Ts'o
Quoted 4 lines

An alternative which would work nicely on ext4 and xfs (if xfs supported fscrypt) would be to pass the physical block number as the DUN. However, that wouldn't work at all on f2fs because f2fs moves data blocks around. And since most people who want to use this are using f2fs, f2fs support is essential.

And that is something fscrypt supports already, so if people really did want to use 64-bit logical block numbers, they could do that, at the cost of giving up the ability to shrink the file system (which XFS doesn't support anyway....)

I was talking about the physical block number (offset from the start of the filesystem -- ext4_fsblk_t on ext4), not the file logical block number (offset in the file data -- ext4_lblk_t on ext4). fscrypt doesn't currently support using the physical block number.

- Eric

#6 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

In reply to Theodore Y. Ts'o
Quoted 4 lines · Theodore Y. Ts'o

If and when we actually get inline crypto support for server-class systems, hopefully they will support 128-bit DUN's, and/or they will have sufficiently fast key load times such that we can use per-file keying.

NVMe is working on a key per I/O feature. So at very least the naming of this option should be "crappy_underwhelming_embedded_inline_crypto"

maintainer
#7 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Theodore Y. Ts'o

If and when we actually get inline crypto support for server-class systems, hopefully they will support 128-bit DUN's, and/or they will have sufficiently fast key load times such that we can use per-file keying.

NVMe is working on a key per I/O feature. So at very least the naming of this option should be "crappy_underwhelming_embedded_inline_crypto"

If and when the vaporware shows up in real hardware, and assuming that fscrypt is useful for this hardware, we can name it "super_duper_fancy_inline_crypto". :-)

Remember that fscrypt only encrypts the data and the file name. It doesn't encrypt the metadata. It has very specific use cases for Android and ChromeOS where you have multiple users that need to use different keys, and in the case of ChromeOS, we want to be able to efficiently use the space so that while user A is logged in, we can delete files in user B's cache directory without user B's keys being present. (This is why we can't use fixed per-user partitions with dm-crypt; that solution was considered and rejected before we started work on fscrypt.)

If you aren't working under tight space and cost constraints, it's actually better to encrypt the whole partition, so that all of the metadata can be protected. fscrypt is deployed in millions and millions of devices, and is solving real world problems. However, it never claimed to be the only way to address encryption in the storage stack --- and it's not at all clear fscrypt is the way that makes the most amount of sense for NVMe devices. So let's cross that bridge when we get to it.

Cheers,

- Ted

_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

#8 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 3 lines

If and when the vaporware shows up in real hardware, and assuming that fscrypt is useful for this hardware, we can name it "super_duper_fancy_inline_crypto". :-)

I think you are entirely missing the point. The point is that naming the option someting related to inline encryption is fundamentally wrong. It is related to a limitation of existing inline crypto engines, not related to the fudamental model. And all the other rambling below don't matter either.

author maintainer
#9 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 10 lines · Christoph Hellwig
Quoted 3 lines

If and when the vaporware shows up in real hardware, and assuming that fscrypt is useful for this hardware, we can name it "super_duper_fancy_inline_crypto". :-)

I think you are entirely missing the point. The point is that naming the option someting related to inline encryption is fundamentally wrong. It is related to a limitation of existing inline crypto engines, not related to the fudamental model. And all the other rambling below don't matter either.

Would you be happy with something that more directly describes the change the flag makes, like FSCRYPT_POLICY_FLAG_CONTENTS_IV_INO_LBLK_64? I.e., the IVs for contents encryption are 64-bit and contain the inode and logical block numbers.

Actually, we could use the same key derivation and IV generation for directories and symlinks too, which would result in just FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64. (lblk is 0 when encrypting a filename.)

Although, in general it would be nice to name the settings in ways that are easier for people not intimately familiar with the crypto to understand...

- Eric

#10 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 2 lines · Eric Biggers

Would you be happy with something that more directly describes the change the flag makes

Yes.

Quoted 6 lines

, like FSCRYPT_POLICY_FLAG_CONTENTS_IV_INO_LBLK_64? I.e., the IVs for contents encryption are 64-bit and contain the inode and logical block numbers.

Actually, we could use the same key derivation and IV generation for directories and symlinks too, which would result in just FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64. (lblk is 0 when encrypting a filename.)

I think not making it crazy verbose is a helpful, but at the same time it should be somewhat descriptive.

Quoted 2 lines

Although, in general it would be nice to name the settings in ways that are easier for people not intimately familiar with the crypto to understand...

For the andoid case the actual users won't ever really see it, and if you set up the thing yourself it probably helps a lot to try to understand what your are doing.

#11 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

Quoted 2 lines · Christoph Hellwig

I think not making it crazy verbose is a helpful, but at the same time it should be somewhat descriptive.

What would your suggested name be?

#12 lore raw

Re: [PATCH 1/3] fscrypt: add support for inline-encryption-optimized policies

In reply to Eric Biggers
Quoted 3 lines · Eric Biggers

An alternative which would work nicely on ext4 and xfs (if xfs supported fscrypt) would be to pass the physical block number as the DUN. However, that wouldn't work at all on f2fs because f2fs moves data blocks around.

XFS can also move data blocks around. Even ext4 can do that for limited cases (defrag).

author maintainer
#13 lore raw

PATCH 2/3 ext4: add support for INLINE_CRYPT_OPTIMIZED encryption policies

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

INLINE_CRYPT_OPTIMIZED encryption policies have special requirements from the filesystem:

- Inode numbers must never change, even if the filesystem is resized
- Inode numbers must be <= 32 bits
- File logical block numbers must be <= 32 bits

ext4 has 32-bit inode and file logical block numbers. However, resize2fs can re-number inodes when shrinking an ext4 filesystem.

However, many people who want to use inline encryption don't care about filesystem shrinking. They'd be fine with a solution that just prevents the filesystem from being shrunk.

Therefore, add a new feature flag EXT4_FEATURE_COMPAT_STABLE_INODES that will do exactly that. Then wire up the fscrypt_operations to expose this flag to fs/crypto/, so that it allows INLINE_CRYPT_OPTIMIZED policies when this flag is set.

---
 fs/ext4/ext4.h  |  2 ++
 fs/ext4/super.c | 14 ++++++++++++++
 2 files changed, 16 insertions(+)
fs/ext4/ext4.h +2 -0
index 03db3e71676ce..b3a2cc7c0252f 100644
@@ -1678,6 +1678,7 @@ static inline bool ext4_verity_in_progress(struct inode *inode)
 #define EXT4_FEATURE_COMPAT_RESIZE_INODE	0x0010
 #define EXT4_FEATURE_COMPAT_DIR_INDEX		0x0020
 #define EXT4_FEATURE_COMPAT_SPARSE_SUPER2	0x0200
+#define EXT4_FEATURE_COMPAT_STABLE_INODES	0x0800
 
 #define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER	0x0001
 #define EXT4_FEATURE_RO_COMPAT_LARGE_FILE	0x0002
@@ -1779,6 +1780,7 @@ EXT4_FEATURE_COMPAT_FUNCS(xattr,		EXT_ATTR)
 EXT4_FEATURE_COMPAT_FUNCS(resize_inode,		RESIZE_INODE)
 EXT4_FEATURE_COMPAT_FUNCS(dir_index,		DIR_INDEX)
 EXT4_FEATURE_COMPAT_FUNCS(sparse_super2,	SPARSE_SUPER2)
+EXT4_FEATURE_COMPAT_FUNCS(stable_inodes,	STABLE_INODES)
 
 EXT4_FEATURE_RO_COMPAT_FUNCS(sparse_super,	SPARSE_SUPER)
 EXT4_FEATURE_RO_COMPAT_FUNCS(large_file,	LARGE_FILE)
fs/ext4/super.c +14 -0
index dd654e53ba3d9..b3cbf8622eab6 100644
@@ -1345,6 +1345,18 @@ static bool ext4_dummy_context(struct inode *inode)
 	return DUMMY_ENCRYPTION_ENABLED(EXT4_SB(inode->i_sb));
 }
 
+static bool ext4_has_stable_inodes(struct super_block *sb)
+{
+	return ext4_has_feature_stable_inodes(sb);
+}
+
+static void ext4_get_ino_and_lblk_bits(struct super_block *sb,
+				       int *ino_bits_ret, int *lblk_bits_ret)
+{
+	*ino_bits_ret = 8 * sizeof(EXT4_SB(sb)->s_es->s_inodes_count);
+	*lblk_bits_ret = 8 * sizeof(ext4_lblk_t);
+}
+
 static const struct fscrypt_operations ext4_cryptops = {
 	.key_prefix		= "ext4:",
 	.get_context		= ext4_get_context,
@@ -1352,6 +1364,8 @@ static const struct fscrypt_operations ext4_cryptops = {
 	.dummy_context		= ext4_dummy_context,
 	.empty_dir		= ext4_empty_dir,
 	.max_namelen		= EXT4_NAME_LEN,
+	.has_stable_inodes	= ext4_has_stable_inodes,
+	.get_ino_and_lblk_bits	= ext4_get_ino_and_lblk_bits,
 };
 #endif
 
Signature · 1 line
2.23.0.866.gb869b98d4c-goog
maintainer Reviewed-by on v1 2/3
#14 lore raw

Re: [PATCH 2/3] ext4: add support for INLINE_CRYPT_OPTIMIZED encryption policies

Quoted 8 lines · Eric Biggers

From: Eric Biggers <[email protected]>

INLINE_CRYPT_OPTIMIZED encryption policies have special requirements from the filesystem:

- Inode numbers must never change, even if the filesystem is resized
- Inode numbers must be <= 32 bits
- File logical block numbers must be <= 32 bits

You need to guarantee more than this; you also need to guarantee that the logical block number may not change. Fortunately, because the original per-file key scheme used a logical block tweak, we've prohibited this already, and we didn't relax this restriction for files encrpyted using DIRECT_KEY. So it's a requirement which we already meet, but we should document this requirement explicitly --- both here and also in Documentations/filesystems/fscrypt.rst.

Otherwise, looks good. Feel free to add:

- Ted

_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

author maintainer
#15 lore raw

Re: [PATCH 2/3] ext4: add support for INLINE_CRYPT_OPTIMIZED encryption policies

Quoted 18 lines · Theodore Y. Ts'o
Quoted 8 lines · Eric Biggers

From: Eric Biggers <[email protected]>

INLINE_CRYPT_OPTIMIZED encryption policies have special requirements from the filesystem:

- Inode numbers must never change, even if the filesystem is resized
- Inode numbers must be <= 32 bits
- File logical block numbers must be <= 32 bits

You need to guarantee more than this; you also need to guarantee that the logical block number may not change. Fortunately, because the original per-file key scheme used a logical block tweak, we've prohibited this already, and we didn't relax this restriction for files encrpyted using DIRECT_KEY. So it's a requirement which we already meet, but we should document this requirement explicitly --- both here and also in Documentations/filesystems/fscrypt.rst.

Quoted 4 lines

Otherwise, looks good. Feel free to add:

This is meant to list the requirements over the current policies. If we wanted to list all requirements on filesystems to support any fscrypt policy at all, we'd also have to list a lot of other things like that the filesystem must implement all the fscrypt_operations, must call all the needed hooks, must support encrypted filenames and symlinks, etc...

I'll change the beginning of this commit message to "INLINE_CRYPT_OPTIMIZED encryption policies have special requirements from the filesystem, in comparison to the current encryption policies:"

... and in the previous patch I'll add a note in the "Contents encryption" section of Documentation/filesystems/fscrypt.rst that the use of the file logical block number means that filesystems must not allow operations that would change it.

- Eric

author maintainer
#16 lore raw

PATCH 3/3 f2fs: add support for INLINE_CRYPT_OPTIMIZED encryption policies

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

f2fs inode numbers are stable across filesystem resizing, and f2fs inode and file logical block numbers are always 32-bit. So f2fs can always support INLINE_CRYPT_OPTIMIZED encryption policies. Wire up the needed fscrypt_operations to declare support.

---
 fs/f2fs/super.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
fs/f2fs/super.c +20 -6
index 1443cee158633..851ac95229263 100644
@@ -2308,13 +2308,27 @@ static bool f2fs_dummy_context(struct inode *inode)
 	return DUMMY_ENCRYPTION_ENABLED(F2FS_I_SB(inode));
 }
 
+static bool f2fs_has_stable_inodes(struct super_block *sb)
+{
+	return true;
+}
+
+static void f2fs_get_ino_and_lblk_bits(struct super_block *sb,
+				       int *ino_bits_ret, int *lblk_bits_ret)
+{
+	*ino_bits_ret = 8 * sizeof(nid_t);
+	*lblk_bits_ret = 8 * sizeof(block_t);
+}
+
 static const struct fscrypt_operations f2fs_cryptops = {
-	.key_prefix	= "f2fs:",
-	.get_context	= f2fs_get_context,
-	.set_context	= f2fs_set_context,
-	.dummy_context	= f2fs_dummy_context,
-	.empty_dir	= f2fs_empty_dir,
-	.max_namelen	= F2FS_NAME_LEN,
+	.key_prefix		= "f2fs:",
+	.get_context		= f2fs_get_context,
+	.set_context		= f2fs_set_context,
+	.dummy_context		= f2fs_dummy_context,
+	.empty_dir		= f2fs_empty_dir,
+	.max_namelen		= F2FS_NAME_LEN,
+	.has_stable_inodes	= f2fs_has_stable_inodes,
+	.get_ino_and_lblk_bits	= f2fs_get_ino_and_lblk_bits,
 };
 #endif
 
Signature · 1 line
2.23.0.866.gb869b98d4c-goog
maintainer Acked-by on v1 3/3
#17 lore raw

Re: [PATCH 3/3] f2fs: add support for INLINE_CRYPT_OPTIMIZED encryption policies

Quoted 8 lines · Eric Biggers

From: Eric Biggers <[email protected]>

f2fs inode numbers are stable across filesystem resizing, and f2fs inode and file logical block numbers are always 32-bit. So f2fs can always support INLINE_CRYPT_OPTIMIZED encryption policies. Wire up the needed fscrypt_operations to declare support.

Quoted 44 lines
---
 fs/f2fs/super.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)
fs/f2fs/super.c +20 -6
index 1443cee158633..851ac95229263 100644
@@ -2308,13 +2308,27 @@ static bool f2fs_dummy_context(struct inode *inode)
 	return DUMMY_ENCRYPTION_ENABLED(F2FS_I_SB(inode));
 }
 
+static bool f2fs_has_stable_inodes(struct super_block *sb)
+{
+	return true;
+}
+
+static void f2fs_get_ino_and_lblk_bits(struct super_block *sb,
+				       int *ino_bits_ret, int *lblk_bits_ret)
+{
+	*ino_bits_ret = 8 * sizeof(nid_t);
+	*lblk_bits_ret = 8 * sizeof(block_t);
+}
+
 static const struct fscrypt_operations f2fs_cryptops = {
-	.key_prefix	= "f2fs:",
-	.get_context	= f2fs_get_context,
-	.set_context	= f2fs_set_context,
-	.dummy_context	= f2fs_dummy_context,
-	.empty_dir	= f2fs_empty_dir,
-	.max_namelen	= F2FS_NAME_LEN,
+	.key_prefix		= "f2fs:",
+	.get_context		= f2fs_get_context,
+	.set_context		= f2fs_set_context,
+	.dummy_context		= f2fs_dummy_context,
+	.empty_dir		= f2fs_empty_dir,
+	.max_namelen		= F2FS_NAME_LEN,
+	.has_stable_inodes	= f2fs_has_stable_inodes,
+	.get_ino_and_lblk_bits	= f2fs_get_ino_and_lblk_bits,
 };
 #endif
 
Signature · 1 line
2.23.0.866.gb869b98d4c-goog