This patch series adds support for metadata encryption to F2FS using
blk-crypto.
Currently, F2FS supports native file based encryption (FBE) via fscrypt.
FBE encrypts the contents of files that reside in folders with encryption
policies, as well as their filenames, but all other file contents
and filesystem metadata is stored unencrypted. We'd like to have metadata
and the contents of non-FBE files encrypted too, to protect data like
file sizes, xattrs, locations, etc. which can be valuable in certain
contexts.
The simplest way to do metadata encryption would be to run the filesystem
over dm-crypt (set up to encrypt all bios with the metadata encryption
key). This would essentially encrypt file contents twice (once with the FBE
key and once with the metadata encryption key). On many android devices,
this is slower than we'd like, and also doesn't play well with inline
encryption engines (which only allow for one layer of encryption, so the
other layer must be done by the kernel crypto API).
Android currently has metadata encryption, and due to the drawbacks
listed above, doesn't use the above mentioned approach, and avoids
double encryption. Metadata encryption on android is currently
implemented using a new DM target (dm-default-key) that encrypts any
bio it receives that has data which has not previously been encrypted
(in practice, it checks for the presence of bio->bi_crypt_context, and
if it's missing, dm-default-key adds a bi_crypt_context to the bio with
the metadata encryption key that it was configured with). This works fine
as long as filesystems submit bios without bi_crypt_contexts for
filesystem metadata/unencrypted file contents, or submit bios with
bi_crypt_contexts for encrypted file contents. However, filesystems like
F2FS sometimes want to read the ciphertext of fscrypt encrypted data
contents (so F2FS will submit a bio without any bi_crypt_context, but
expects to receive ciphertext rather than the file contents decrypted
with the metadata encryption key). To address this issue, F2FS sets a flag
on the bio which essentially instructs dm-default-key not to add a
bi_crypt_context on that bio even though there isn't already one on it.
We'd like to try to come up with a metadata encryption solution that avoids
this layering violation.
The most natural solution that avoids double encryption and layering
violations is to let the filesystem take care of metadata encryption,
since the filesystem is what's responsible for knowing where the filesystem
metadata/unencrypted file contents/encrypted file contents are. This patch
series follows that approach, and adds support for metadata encryption to
F2FS and fscrypt.
Patch 1 replaces fscrypt_get_devices (which took an array of request_queues
and filled it up) with fscrypt_get_device, which takes a index of the
desired device and returns the device at that index (so the index passed
to fscrypt_get_device must be between 0 and (fscrypt_get_num_devices() - 1)
inclusive). This allows callers to avoid having to allocate an array to
pass to fscrypt_get_devices() when they only need to iterate through
each element in the array (and have no use for the array itself).
Patch 2 introduces some functions to fscrypt that help filesystems perform
metadata encryption. Any filesystem that wants to use metadata encryption
can call fscrypt_setup_metadata_encryption() with the super_block of the
filesystem, the encryption algorithm and the descriptor of the metadata
crypt key. The descriptor is looked up in the logon keyring of the
current session with "fscrypt:" as the prefix of the descriptor. The
metadata crypt key is not directly used for encryption - the actual
metadata encryption key is derived from this metadata key (refer to
fscrypt_setup_metadata_encryption() in fs/crypto/metadata_crypt.c for
details).
The patch also introduces fscrypt_metadata_crypt_bio() which an FS should
call on a bio that the FS wants metadata crypted. The function will add
an encryption context with the metadata encryption key set up by the call
to the above mentioned fscrypt_setup_metadata_encryption().
The patch also introduces fscrypt_metadata_crypt_prepare_all_devices().
Filesystems that use multiple devices should call this function once all
the underlying devices have been determined. An FS might only be able to
determine all the underlying devices after some initial processing that
might already require metadata en/decryption, which is why this function
is separate from fscrypt_setup_metadata_encryption().
Finally, the patch makes the metadata crypt key for the filesystem part
of the key derivation process for all fscrypt file content encryption
keys used with that filesystem - this way, the file content encryption
keys are at least as strong as the metadata encryption key. For more
details please refer to fscrypt_mix_in_metadata_key() in
fs/crypto/metadata_crypt.c
Patch 3 wires up F2FS with the functions introduced in Patch 2. F2FS
will encrypt every block (that's not being encrypted by some other
encryption key, e.g. a per-file key) with the metadata encryption key
except the superblock (and the redundant copy of the superblock). The DUN
of a block is the offset of the block from the start of the F2FS
filesystem.
Please refer to the commit message for why the superblock was excluded from
en/decryption, and other limitations. The superblock and its copy are
stored in plaintext on disk. The encryption algorithm used for metadata
encryption is stored within the superblock itself. Changes to the userspace
tools (that are required to test out metadata encryption with F2FS) are
also being sent out - I'll post a link as a reply to this mail once it's
out.
Changes v1 => v2:
- The metadata crypt key is no longer used directly for encryption. The
actual metadata encryption key is now derived from the metadata crypt key.
A key identifier is also derived from the metadata crypt key (and this
identifier is verified at FS mount time). The key identifier is stored
directly in the F2FS superblock, so there's no longer a need for any new
mount options.
- The metadata crypt key is now mixed into the key derivation process for
all subkeys derived from fscrypt master keys
- Make the metadata key payload in the keyring just the raw bytes of the
key (instead of having it represent a struct fscrypt_key)
- export some of the metadata_crypt.c functions, since F2FS can be built
as a module
- make FS_ENCRYPTION_METADATA depend on FS_ENCRYPTION_INLINE_CRYPT
- fscrypt_set_bio_crypt_ctx() calls fscrypt_metadata_crypt_bio()
directly, so filesystems only need to call fscrypt_set_bio_crypt_ctx()
- Cleanups and updated docs
Satya Tangirala (3):
fscrypt, f2fs: replace fscrypt_get_devices with fscrypt_get_device
fscrypt: Add metadata encryption support
f2fs: Add metadata encryption support
Documentation/filesystems/fscrypt.rst | 86 +++++++-
fs/crypto/Kconfig | 12 +
fs/crypto/Makefile | 1 +
fs/crypto/bio.c | 2 +-
fs/crypto/fscrypt_private.h | 46 ++++
fs/crypto/hkdf.c | 1 +
fs/crypto/inline_crypt.c | 52 ++---
fs/crypto/keyring.c | 4 +
fs/crypto/metadata_crypt.c | 303 ++++++++++++++++++++++++++
fs/ext4/readpage.c | 2 +-
fs/f2fs/data.c | 17 +-
fs/f2fs/f2fs.h | 2 +
fs/f2fs/super.c | 60 ++++-
include/linux/f2fs_fs.h | 7 +-
include/linux/fs.h | 10 +
include/linux/fscrypt.h | 50 ++++-
16 files changed, 586 insertions(+), 69 deletions(-)
create mode 100644 fs/crypto/metadata_crypt.c
--
2.29.2.729.g45daf8777d-goog
Introduces functions that help with metadata encryption.
In particular, we introduce:
fscrypt_setup_metadata_encryption() - filesystems should call this function
to set up metadata encryption on a super block with the encryption
algorithm (the desired FSCRYPT_MODE_*) and the key identifier of the
encryption key. fscrypt looks for a logon key with the specified key
identifier with prefix "fscrypt:". fscrypt will verify that the key
identifier matches the identifier that is derived using HKDF-512 with the
logon key as the keying material, no salt, and
"fscrypt\0|HKDF_CONTEXT_KEY_IDENTIFIER" as the info string.
fscrypt_free_metadata_encryption() - this function should be called when
the super block is being freed. It ensures that the metadata encryption key
is evicted, if necessary, from devices.
Filesystems should call fscrypt_set_bio_crypt_ctx() on any bio that needs
either metadata or file contents encryption. fscrypt will choose the
appropriate key (based on the inode argument) to use for encrypting the
bio.
Note that the filesystem (rather than fscrypt) controls precisely which
blocks are encrypted with the metadata encryption key and which blocks are
encrypted with other keys/not encrypted at all. fscrypt only provides some
convenience functions that ultimately help encrypt a bio with the metadata
encryption key when desired.
This feature relies on the blk-crypto framework to do the actual
encryption. As such, at least one of inline encryption hardware or the
blk-crypto-fallback needs to be present/enabled.
Signed-off-by: Satya Tangirala <[email protected]>
---
Documentation/filesystems/fscrypt.rst | 86 +++++++-
fs/crypto/Kconfig | 12 +
fs/crypto/Makefile | 1 +
fs/crypto/bio.c | 2 +-
fs/crypto/fscrypt_private.h | 46 ++++
fs/crypto/hkdf.c | 1 +
fs/crypto/inline_crypt.c | 33 +--
fs/crypto/keyring.c | 4 +
fs/crypto/metadata_crypt.c | 303 ++++++++++++++++++++++++++
fs/ext4/readpage.c | 2 +-
fs/f2fs/data.c | 2 +-
include/linux/fs.h | 10 +
include/linux/fscrypt.h | 46 +++-
13 files changed, 511 insertions(+), 37 deletions(-)
create mode 100644 fs/crypto/metadata_crypt.c
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 44b67ebd6e40..708164c413cc 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -27,8 +27,6 @@ at the block device level. This allows it to encrypt different files
with different keys and to have unencrypted files on the same
filesystem. This is useful for multi-user systems where each user's
data-at-rest needs to be cryptographically isolated from the others.
-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
@@ -47,6 +45,15 @@ userspace provides the key, all regular files, directories, and
symbolic links created in that directory tree are transparently
encrypted.
+fscrypt also has support for encrypting filesystem metadata, which
+can be used independently of file contents encryption. For any
+filesystem block that isn't part of an encrypted file's contents,
+the filesystem can ask fscrypt to encrypt it with the metadata encryption
+key set up ahead of time. In general, filesystems should always choose
+to encrypt a filesystem block with the metadata encryption key, if
+that block isn't already encrypted with another key (filesystems may
+choose to leave certain blocks, like the superblock, unencrypted).
+
Threat model
============
@@ -56,11 +63,17 @@ Offline attacks
Provided that userspace chooses a strong encryption key, fscrypt
protects the confidentiality of file contents and filenames in the
event of a single point-in-time permanent offline compromise of the
-block device content. fscrypt does not protect the confidentiality of
-non-filename metadata, e.g. file sizes, file permissions, file
-timestamps, and extended attributes. Also, the existence and location
-of holes (unallocated blocks which logically contain all zeroes) in
-files is not protected.
+block device content. fscrypt does not protect the existence and
+location of holes (unallocated blocks which logically contain all
+zeroes) in files.
+
+fscrypt protects the confidentiality of non-filename metadata, e.g.
+file sizes, file permissions, file timestamps, and extended attribute
+only when metadata encryption support is enabled for the filesystem,
+and the filesystem chooses to protect such information with the
+metadata encryption key. Presently only F2FS filesystems supports
+fscrypt metadata encryption. When metadata encryption is enabled, F2FS
+encrypts all not-otherwise-encrypted data in the filesystem with the metadata encryption key (except the filesystem superblock).
fscrypt is not guaranteed to protect confidentiality or authenticity
if an attacker is able to manipulate the filesystem offline prior to
@@ -90,6 +103,10 @@ After an encryption key has been added, fscrypt does not hide the
plaintext file contents or filenames from other users on the same
system. Instead, existing access control mechanisms such as file mode
bits, POSIX ACLs, LSMs, or namespaces should be used for this purpose.
+The above is also applicable for the metadata encryption key, if
+metadata encryption is enabled for the filesystem (once the key is added
+and the filesystem is mounted, fscrypt does not hide filesystem metadata
+from other users in the system).
(For the reasoning behind this, understand that while the key is
added, the confidentiality of the data, from the perspective of the
@@ -229,6 +246,15 @@ derived, the application-specific information string is the file's
nonce prefixed with "fscrypt\\0" and a context byte. Different
context bytes are used for other types of derived keys.
+For v2 encryption policies, if the filesystem has a metadata crypt key,
+the master key is first "mixed" with the metadata crypt key, generating
+a "mixed-metadata key", which is then used in place of the master key
+in the process described above. The "mixed-metadata key" is generated
+by using the metadata crypt key as the input keying material, and
+a context specific byte and the original master key as the
+application-specific information string with HKDF-SHA512 (refer to
+fscrypt_mix_in_metadata_key() for details).
+
HKDF-SHA512 is preferred to the original AES-128-ECB based KDF because
HKDF is more flexible, is nonreversible, and evenly distributes
entropy from the master key. HKDF is also standardized and widely
@@ -1024,8 +1050,16 @@ process-subscribed keyrings.
Access semantics
================
-With the key
-------------
+Without the metadata encryption key
+-----------------------------------
+
+If metadata encryption is enabled on a filesystem, it can't be mounted
+without the metadata encryption key, so no accesses are supported.
+The rest of this section assumes that the metadata encryption key is
+available.
+
+With the file encryption key
+----------------------------
With the encryption key, encrypted regular files, directories, and
symlinks behave very similarly to their unencrypted counterparts ---
@@ -1077,8 +1111,8 @@ astute users may notice some differences in behavior:
Note that mmap *is* supported. This is possible because the pagecache
for an encrypted file contains the plaintext, not the ciphertext.
-Without the key
----------------
+Without the file encryption key
+-------------------------------
Some filesystem operations may be performed on encrypted regular
files, directories, and symlinks even before their encryption key has
@@ -1255,6 +1289,36 @@ without the key is subject to change in the future. It is only meant
as a way to temporarily present valid filenames so that commands like
``rm -r`` work as expected on encrypted directories.
+Filesystem metadata encryption
+------------------------------
+
+fscrypt metadata encryption relies on FS_ENCRYPTION_INLINE_CRYPT, and
+in particular on the blk-crypto framework (so either
+hardware inline encryption support must be present, or
+blk-crypto-fallback must be enabled).
+
+fscrypt metadata encryption requires the filesystem to keep track of
+the required metadata encryption key's identifier, and pass it to
+fscrypt at mount time. fscrypt will search for a logon key with
+description "fscrypt:<metadata_key_ident>". If the key is found, it will
+derive the payload's identifier from the payload and check that it
+matches the given <metadata_key_ident>. The metadata key identifier is
+calculated as hkdf(key=metadata_crypt_key,
+info="fscrypt\\0"|HKDF_CONTEXT_KEY_IDENTIFIER), where | represents
+concatenation).
+
+Filesystems have complete control over which blocks are encrypted with
+the metadata encryption key (other than blocks that are encrypted with
+fscrypt file content encryption keys - those blocks can't be encrypted
+again with the metadata encryption key). In general, filesystems should
+encrypt (almost) all not-otherwise-encrypted blocks with the metadata
+encryption key.
+
+fscrypt will encrypt each filesystem block in a bio as a single data
+unit. Filesystems control the IV used to encrypt the first filesystem
+block in a bio. The IVs of the rest of the blocks in the bio are computed
+by blk-crypto (by incrementing the IV by one for each additional block).
+
Tests
=====
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index a5f5c30368a2..58b79d757608 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -30,3 +30,15 @@ config FS_ENCRYPTION_INLINE_CRYPT
depends on FS_ENCRYPTION && BLK_INLINE_ENCRYPTION
help
Enable fscrypt to use inline encryption hardware if available.
+
+config FS_ENCRYPTION_METADATA
+ bool "Enable metadata encryption with fscrypt"
+ depends on FS_ENCRYPTION_INLINE_CRYPT
+ help
+ Enable fscrypt to encrypt metadata. This allows filesystems
+ that support metadata encryption through fscrypt to mount
+ and use filesystem images formatted with metadata encryption
+ enabled. Such filesystem images generally have all
+ otherwise-non-encrypted data (like filesystem metadata,
+ and unencrypted files) encrypted with a metadata encryption
+ key instead.
\ No newline at end of file
diff --git a/fs/crypto/Makefile b/fs/crypto/Makefile
index 652c7180ec6d..8403c7956983 100644
--- a/fs/crypto/Makefile
+++ b/fs/crypto/Makefile
@@ -12,3 +12,4 @@ fscrypto-y := crypto.o \
fscrypto-$(CONFIG_BLOCK) += bio.o
fscrypto-$(CONFIG_FS_ENCRYPTION_INLINE_CRYPT) += inline_crypt.o
+fscrypto-$(CONFIG_FS_ENCRYPTION_METADATA) += metadata_crypt.o
\ No newline at end of file
diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
index b048a0e38516..d5c1dc38461b 100644
--- a/fs/crypto/bio.c
+++ b/fs/crypto/bio.c
@@ -59,7 +59,7 @@ static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode,
unsigned int bytes_this_page = blocks_this_page << blockbits;
if (num_pages == 0) {
- fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOFS);
+ fscrypt_set_bio_crypt_ctx(bio, 0, inode, lblk, GFP_NOFS);
bio_set_dev(bio, inode->i_sb->s_bdev);
bio->bi_iter.bi_sector =
pblk << (blockbits - SECTOR_SHIFT);
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 4f5806a3b73d..d13bb5e9d400 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -319,6 +319,8 @@ int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
#define HKDF_CONTEXT_DIRHASH_KEY 5 /* info=file_nonce */
#define HKDF_CONTEXT_IV_INO_LBLK_32_KEY 6 /* info=mode_num||fs_uuid */
#define HKDF_CONTEXT_INODE_HASH_KEY 7 /* info=<empty> */
+#define HKDF_CONTEXT_METADATA_ENC_KEY 8 /* info=<empty> */
+#define HKDF_CONTEXT_MIX_METADATA_KEY 9 /* info=raw_secret */
int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
const u8 *info, unsigned int infolen,
@@ -327,6 +329,25 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf);
/* inline_crypt.c */
+
+static inline int fscrypt_get_num_devices(struct super_block *sb)
+{
+ if (sb->s_cop->get_num_devices)
+ return sb->s_cop->get_num_devices(sb);
+ return 1;
+}
+
+static inline struct request_queue *fscrypt_get_device(struct super_block *sb,
+ unsigned int device_index)
+{
+ if (sb->s_cop->get_device)
+ return sb->s_cop->get_device(sb, device_index);
+ else if (WARN_ON_ONCE(device_index != 0))
+ return NULL;
+ else
+ return bdev_get_queue(sb->s_bdev);
+}
+
#ifdef CONFIG_FS_ENCRYPTION_INLINE_CRYPT
int fscrypt_select_encryption_impl(struct fscrypt_info *ci);
@@ -595,4 +616,29 @@ int fscrypt_policy_from_context(union fscrypt_policy *policy_u,
int ctx_size);
const union fscrypt_policy *fscrypt_policy_to_inherit(struct inode *dir);
+/* metadata_crypt.c */
+
+#ifdef CONFIG_FS_ENCRYPTION_METADATA
+
+int fscrypt_mix_in_metadata_key(struct super_block *sb,
+ struct fscrypt_master_key_secret *secret);
+
+void fscrypt_metadata_crypt_bio(struct bio *bio, u64 fsblk,
+ struct super_block *sb, gfp_t gfp_mask);
+
+#else /* !CONFIG_FS_ENCRYPTION_METADATA */
+
+static inline int
+fscrypt_mix_in_metadata_key(struct super_block *sb,
+ struct fscrypt_master_key_secret *secret)
+{
+ return 0;
+}
+
+static inline void fscrypt_metadata_crypt_bio(struct bio *bio, u64 fsblk,
+ struct super_block *sb,
+ gfp_t gfp_mask) { }
+
+#endif /* !CONFIG_FS_ENCRYPTION_METADATA */
+
#endif /* _FSCRYPT_PRIVATE_H */
diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c
index 0cba7928446d..61d1f0aa802e 100644
--- a/fs/crypto/hkdf.c
+++ b/fs/crypto/hkdf.c
@@ -174,4 +174,5 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf)
{
crypto_free_shash(hkdf->hmac_tfm);
+ hkdf->hmac_tfm = NULL;
}
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index 8234217b42f4..ebd1db1707e6 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -26,24 +26,6 @@ struct fscrypt_blk_crypto_key {
struct request_queue *devs[];
};
-static int fscrypt_get_num_devices(struct super_block *sb)
-{
- if (sb->s_cop->get_num_devices)
- return sb->s_cop->get_num_devices(sb);
- return 1;
-}
-
-static struct request_queue *fscrypt_get_device(struct super_block *sb,
- unsigned int device_index)
-{
- if (sb->s_cop->get_device)
- return sb->s_cop->get_device(sb, device_index);
- else if (WARN_ON_ONCE(device_index != 0))
- return NULL;
- else
- return bdev_get_queue(sb->s_bdev);
-}
-
static unsigned int fscrypt_get_dun_bytes(const struct fscrypt_info *ci)
{
struct super_block *sb = ci->ci_inode->i_sb;
@@ -221,6 +203,8 @@ static void fscrypt_generate_dun(const struct fscrypt_info *ci, u64 lblk_num,
/**
* fscrypt_set_bio_crypt_ctx() - prepare a file contents bio for inline crypto
* @bio: a bio which will eventually be submitted to the file
+ * @first_fsblk: the first FS block number in the I/O (only used if bio will be
+ * metadata crypted)
* @inode: the file's inode
* @first_lblk: the first file logical block number in the I/O
* @gfp_mask: memory allocation flags - these must be a waiting mask so that
@@ -234,12 +218,19 @@ static void fscrypt_generate_dun(const struct fscrypt_info *ci, u64 lblk_num,
*
* The encryption context will be freed automatically when the bio is freed.
*/
-void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
- u64 first_lblk, gfp_t gfp_mask)
+void fscrypt_set_bio_crypt_ctx(struct bio *bio, u64 first_fsblk,
+ const struct inode *inode, u64 first_lblk,
+ gfp_t gfp_mask)
{
const struct fscrypt_info *ci;
u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
+ if (!fscrypt_needs_contents_encryption(inode)) {
+ fscrypt_metadata_crypt_bio(bio, first_fsblk, inode->i_sb,
+ gfp_mask);
+ return;
+ }
+
if (!fscrypt_inode_uses_inline_crypto(inode))
return;
ci = inode->i_crypt_info;
@@ -291,7 +282,7 @@ void fscrypt_set_bio_crypt_ctx_bh(struct bio *bio,
u64 first_lblk;
if (bh_get_inode_and_lblk_num(first_bh, &inode, &first_lblk))
- fscrypt_set_bio_crypt_ctx(bio, inode, first_lblk, gfp_mask);
+ fscrypt_set_bio_crypt_ctx(bio, 0, inode, first_lblk, gfp_mask);
}
EXPORT_SYMBOL_GPL(fscrypt_set_bio_crypt_ctx_bh);
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 53cc552a7b8f..3a3bc9403882 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -483,6 +483,10 @@ static int add_master_key(struct super_block *sb,
int err;
if (key_spec->type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
+ err = fscrypt_mix_in_metadata_key(sb, secret);
+ if (err)
+ return err;
+
err = fscrypt_init_hkdf(&secret->hkdf, secret->raw,
secret->size);
if (err)
diff --git a/fs/crypto/metadata_crypt.c b/fs/crypto/metadata_crypt.c
new file mode 100644
index 000000000000..7e6044b21624
--- /dev/null
+++ b/fs/crypto/metadata_crypt.c
@@ -0,0 +1,303 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Metadata encryption support for fscrypt
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#include <keys/user-type.h>
+#include <linux/blk-crypto.h>
+#include <linux/blkdev.h>
+#include <linux/buffer_head.h>
+#include <linux/sched/mm.h>
+#include <linux/slab.h>
+
+#include "fscrypt_private.h"
+
+/**
+ * fscrypt_mix_in_metadata_key() - Mix in the metadata crypt key with an fscrypt
+ * master key
+ * @sb: The superblock whose metadata_crypt_key to mix in
+ * @secret: The secret that needs to be mixed with the metadata_crypt_key
+ *
+ * Replaces @secret->raw with hkdf(key=metadata_crypt_key,
+ * info=HKDF_CONTEXT_MIX_METADATA_KEY|@secret->raw). As such,
+ * @secret->raw is at least as strong as the metadata_crypt_key.
+ *
+ * Returns 0 on success and a negative value on error;
+ */
+int fscrypt_mix_in_metadata_key(struct super_block *sb,
+ struct fscrypt_master_key_secret *secret)
+{
+ u8 orig_key[FSCRYPT_MAX_KEY_SIZE];
+ int err;
+
+ if (WARN_ON(secret->size > sizeof(orig_key)))
+ return -EINVAL;
+
+ if (!sb->s_metadata_hkdf)
+ return 0;
+
+ memcpy(orig_key, secret->raw, secret->size);
+ err = fscrypt_hkdf_expand(sb->s_metadata_hkdf,
+ HKDF_CONTEXT_MIX_METADATA_KEY,
+ orig_key, secret->size,
+ secret->raw, secret->size);
+ memzero_explicit(orig_key, secret->size);
+ return err;
+}
+
+static int fscrypt_metadata_get_key_from_id(
+ u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
+ unsigned int keysize,
+ u8 raw_key[FSCRYPT_MAX_KEY_SIZE])
+{
+ char description[FSCRYPT_KEY_DESC_PREFIX_SIZE +
+ FSCRYPT_KEY_IDENTIFIER_SIZE * 2 + 1];
+ struct key *key;
+ const struct user_key_payload *ukp;
+ int err = -ENOKEY;
+
+ sprintf(description, "%s%*phN", FSCRYPT_KEY_DESC_PREFIX,
+ FSCRYPT_KEY_IDENTIFIER_SIZE, key_ident);
+
+ key = request_key(&key_type_logon, description, NULL);
+ if (IS_ERR(key)) {
+ fscrypt_warn(NULL, "Metadata key couldn't be found");
+ return PTR_ERR(key);
+ }
+
+ down_read(&key->sem);
+ ukp = user_key_payload_locked(key);
+
+ /* was the key revoked before we acquired its semaphore? */
+ if (!ukp) {
+ fscrypt_warn(NULL, "Metadata key was revoked");
+ goto out;
+ }
+
+ if (ukp->datalen != keysize) {
+ fscrypt_warn(NULL,
+ "key with description '%s' has incorrect length (got %u bytes, need %u bytes)",
+ key->description, ukp->datalen, keysize);
+ goto out;
+ }
+
+ memcpy(raw_key, ukp->data, keysize);
+ err = 0;
+
+out:
+ up_read(&key->sem);
+ key_put(key);
+
+ return err;
+}
+
+static bool fscrypt_metadata_mode_valid(int fscrypt_mode_num)
+{
+ return fscrypt_mode_num == FSCRYPT_MODE_AES_256_XTS ||
+ fscrypt_mode_num == FSCRYPT_MODE_AES_128_CBC ||
+ fscrypt_mode_num == FSCRYPT_MODE_ADIANTUM;
+}
+
+static int fscrypt_metadata_setup_hkdf(struct super_block *sb,
+ u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
+ unsigned int keysize)
+{
+ u8 raw_key[FSCRYPT_MAX_KEY_SIZE];
+ u8 key_ident_computed[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ int err;
+
+ err = fscrypt_metadata_get_key_from_id(key_ident, keysize, raw_key);
+ if (err)
+ return err;
+
+ sb->s_metadata_hkdf = kzalloc(sizeof(*sb->s_metadata_hkdf), GFP_KERNEL);
+ if (!sb->s_metadata_hkdf)
+ goto out_zero_key;
+
+ err = fscrypt_init_hkdf(sb->s_metadata_hkdf, raw_key, keysize);
+ if (err)
+ goto err_free_hkdf;
+
+ err = fscrypt_hkdf_expand(sb->s_metadata_hkdf,
+ HKDF_CONTEXT_KEY_IDENTIFIER, NULL, 0,
+ key_ident_computed,
+ FSCRYPT_KEY_IDENTIFIER_SIZE);
+ if (err)
+ goto err_destroy_hkdf;
+
+ if (memcmp(key_ident, key_ident_computed, FSCRYPT_KEY_IDENTIFIER_SIZE)) {
+ err = -EINVAL;
+ fscrypt_warn(NULL,
+ "Metadata encryption key did not have the correct key identifier. Rejecting the key.");
+ goto err_destroy_hkdf;
+ }
+
+ goto out_zero_key;
+
+err_destroy_hkdf:
+ fscrypt_destroy_hkdf(sb->s_metadata_hkdf);
+err_free_hkdf:
+ kfree(sb->s_metadata_hkdf);
+ sb->s_metadata_hkdf = NULL;
+out_zero_key:
+ memzero_explicit(raw_key, keysize);
+ return err;
+}
+
+/**
+ * fscrypt_setup_metadata_encryption() - prepare a super_block for metadata
+ * encryption
+ * @sb: The super_block to set up metadata encryption for
+ * @key_ident: The key identifier of a logon key to look for in the process
+ * subscribed keyrings.
+ * @fscrypt_mode_num: The FSCRYPT_MODE_* to use as the encryption algorithm.
+ * @fs_blk_bytes: The number of bytes required to represent fs block numbers.
+ *
+ * Return: 0 on success, negative number on error.
+ */
+int fscrypt_setup_metadata_encryption(struct super_block *sb,
+ u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
+ int fscrypt_mode_num,
+ unsigned int fs_blk_bytes)
+{
+ int err = 0;
+ struct fscrypt_mode *fscrypt_mode;
+ u8 derived_metadata_key[FSCRYPT_MAX_KEY_SIZE];
+
+ if (!fscrypt_metadata_mode_valid(fscrypt_mode_num)) {
+ fscrypt_warn(NULL, "Invalid fscrypt mode %d specified for metadata encryption.",
+ fscrypt_mode_num);
+ return -EOPNOTSUPP;
+ }
+
+ fscrypt_mode = &fscrypt_modes[fscrypt_mode_num];
+ fs_blk_bytes = DIV_ROUND_UP(fs_blk_bytes, 8);
+ if (fscrypt_mode->ivsize < fs_blk_bytes) {
+ fscrypt_warn(NULL, "The fscrypt mode only supports %d DUN bytes, but FS requires support for %d DUN bytes.",
+ fscrypt_mode->ivsize, fs_blk_bytes);
+ return -EOPNOTSUPP;
+ }
+
+ err = fscrypt_metadata_setup_hkdf(sb, key_ident, fscrypt_mode->keysize);
+ if (err)
+ return err;
+
+ err = fscrypt_hkdf_expand(sb->s_metadata_hkdf,
+ HKDF_CONTEXT_METADATA_ENC_KEY, NULL, 0,
+ derived_metadata_key, fscrypt_mode->keysize);
+ if (err)
+ goto err_free_metadata_encryption;
+
+ sb->s_metadata_key = kzalloc(sizeof(*sb->s_metadata_key), GFP_KERNEL);
+ if (!sb->s_metadata_key) {
+ err = -ENOMEM;
+ goto err_free_metadata_encryption;
+ }
+
+ err = blk_crypto_init_key(sb->s_metadata_key, derived_metadata_key,
+ fscrypt_mode->blk_crypto_mode,
+ fs_blk_bytes, sb->s_blocksize);
+ if (err)
+ goto err_free_metadata_encryption;
+
+ err = blk_crypto_start_using_key(sb->s_metadata_key,
+ bdev_get_queue(sb->s_bdev));
+ if (err)
+ goto err_free_metadata_encryption;
+
+ goto out;
+err_free_metadata_encryption:
+ fscrypt_free_metadata_encryption(sb);
+out:
+ memzero_explicit(derived_metadata_key, sizeof(derived_metadata_key));
+ return err;
+}
+EXPORT_SYMBOL_GPL(fscrypt_setup_metadata_encryption);
+
+/**
+ * fscrypt_metadata_crypt_prepare_all_devices() - prepare all devices used by
+ * the filesystem for metadata encryption.
+ * @sb: The super_block whose devices to prepare
+ *
+ * This function should be called when the filesystem has determined all its
+ * devices. This might happen only after some initial setup, which is why
+ * this is a separate function from fscrypt_setup_metadata_encryption().
+ *
+ * Return: 0 on success, negative on error.
+ */
+int fscrypt_metadata_crypt_prepare_all_devices(struct super_block *sb)
+{
+ int num_devices;
+ int i;
+ struct request_queue *q;
+
+ if (!sb->s_metadata_key)
+ return 0;
+
+ num_devices = fscrypt_get_num_devices(sb);
+ for (i = 0; i < num_devices; i++) {
+ q = fscrypt_get_device(sb, i);
+ if (!q || blk_crypto_start_using_key(sb->s_metadata_key, q))
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fscrypt_metadata_crypt_prepare_all_devices);
+
+/**
+ * fscrypt_free_metadata_encryption() - free metadata encryption fields in
+ * super_block.
+ * @sb: The super_block to free metatdata encryption fields from
+ */
+void fscrypt_free_metadata_encryption(struct super_block *sb)
+{
+ int num_devices;
+ int i;
+ struct request_queue *q;
+
+ if (!sb->s_metadata_hkdf)
+ return;
+
+ fscrypt_destroy_hkdf(sb->s_metadata_hkdf);
+ kfree(sb->s_metadata_hkdf);
+ sb->s_metadata_hkdf = NULL;
+
+ if (!sb->s_metadata_key)
+ return;
+
+ num_devices = fscrypt_get_num_devices(sb);
+
+ for (i = 0; i < num_devices; i++) {
+ q = fscrypt_get_device(sb, i);
+ if (WARN_ON(!q))
+ continue;
+ blk_crypto_evict_key(q, sb->s_metadata_key);
+ }
+
+ kfree_sensitive(sb->s_metadata_key);
+ sb->s_metadata_key = NULL;
+}
+EXPORT_SYMBOL_GPL(fscrypt_free_metadata_encryption);
+
+/**
+ * fscrypt_metadata_crypt_bio() - Add metadata encryption context to bio.
+ *
+ * @bio: The bio to add the encryption context to
+ * @fsblk: The block number within the filesystem at which this bio starts
+ * reading/writing data.
+ * @sb: The superblock of the filesystem
+ * @gfp_mask: gfp_mask for bio_crypt_context allocation
+ */
+void fscrypt_metadata_crypt_bio(struct bio *bio, u64 fsblk,
+ struct super_block *sb, gfp_t gfp_mask)
+{
+ u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE] = { 0 };
+
+ if (!sb->s_metadata_key)
+ return;
+
+ dun[0] = fsblk;
+ bio_crypt_set_ctx(bio, sb->s_metadata_key, dun, gfp_mask);
+}
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index f014c5e473a9..c473da5ee906 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -373,7 +373,7 @@ int ext4_mpage_readpages(struct inode *inode,
*/
bio = bio_alloc(GFP_KERNEL,
min_t(int, nr_pages, BIO_MAX_PAGES));
- fscrypt_set_bio_crypt_ctx(bio, inode, next_block,
+ fscrypt_set_bio_crypt_ctx(bio, 0, inode, next_block,
GFP_KERNEL);
ext4_set_bio_post_read_ctx(bio, inode, page->index);
bio_set_dev(bio, bdev);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index be4da52604ed..627164706029 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -470,7 +470,7 @@ static void f2fs_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
* read/write raw data without encryption.
*/
if (!fio || !fio->encrypted_page)
- fscrypt_set_bio_crypt_ctx(bio, inode, first_idx, gfp_mask);
+ fscrypt_set_bio_crypt_ctx(bio, 0, inode, first_idx, gfp_mask);
}
static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8667d0cdc71e..93a97cfa585b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -68,6 +68,7 @@ struct fsverity_info;
struct fsverity_operations;
struct fs_context;
struct fs_parameter_spec;
+struct fscrypt_hkdf;
extern void __init inode_init(void);
extern void __init inode_init_early(void);
@@ -1439,6 +1440,15 @@ struct super_block {
const struct fscrypt_operations *s_cop;
struct key *s_master_keys; /* master crypto keys in use */
#endif
+#ifdef CONFIG_FS_ENCRYPTION_METADATA
+ /* HKDF initialized with the raw metadata key */
+ struct fscrypt_hkdf *s_metadata_hkdf;
+ /*
+ * Encryption key derived from raw metadata key (this is the encryption
+ * key used to encrypt metadata)
+ */
+ struct blk_crypto_key *s_metadata_key;
+#endif
#ifdef CONFIG_FS_VERITY
const struct fsverity_operations *s_vop;
#endif
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index a7d1af4932aa..bed89de57a0c 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -553,7 +553,7 @@ static inline void fscrypt_set_ops(struct super_block *sb,
bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode);
-void fscrypt_set_bio_crypt_ctx(struct bio *bio,
+void fscrypt_set_bio_crypt_ctx(struct bio *bio, u64 first_fsblk,
const struct inode *inode, u64 first_lblk,
gfp_t gfp_mask);
@@ -574,7 +574,7 @@ static inline bool __fscrypt_inode_uses_inline_crypto(const struct inode *inode)
return false;
}
-static inline void fscrypt_set_bio_crypt_ctx(struct bio *bio,
+static inline void fscrypt_set_bio_crypt_ctx(struct bio *bio, u64 first_fsblk,
const struct inode *inode,
u64 first_lblk, gfp_t gfp_mask) { }
@@ -597,6 +597,48 @@ static inline bool fscrypt_mergeable_bio_bh(struct bio *bio,
}
#endif /* !CONFIG_FS_ENCRYPTION_INLINE_CRYPT */
+/* metadata_crypt.c */
+#ifdef CONFIG_FS_ENCRYPTION_METADATA
+
+int fscrypt_setup_metadata_encryption(struct super_block *sb,
+ u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
+ int fscrypt_mode_num,
+ unsigned int fs_blk_bytes);
+
+int fscrypt_metadata_crypt_prepare_all_devices(struct super_block *sb);
+
+void fscrypt_free_metadata_encryption(struct super_block *sb);
+
+static inline bool fscrypt_metadata_crypted(struct super_block *sb)
+{
+ return sb->s_metadata_key;
+}
+
+#else /* CONFIG_FS_ENCRYPTION_METADATA */
+
+static inline int fscrypt_setup_metadata_encryption(struct super_block *sb,
+ u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
+ int fscrypt_mode_num,
+ unsigned int fs_blk_bytes)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int
+fscrypt_metadata_crypt_prepare_all_devices(struct super_block *sb)
+{
+ return 0;
+}
+
+static inline void fscrypt_free_metadata_encryption(struct super_block *sb) { }
+
+static inline bool fscrypt_metadata_crypted(struct super_block *sb)
+{
+ return false;
+}
+
+#endif /* CONFIG_FS_ENCRYPTION_METADATA */
+
/**
* fscrypt_inode_uses_inline_crypto() - test whether an inode uses inline
* encryption
--
2.29.2.729.g45daf8777d-goog
Wire up metadata encryption support with the fscrypt metadata crypt
additions. Note that this feature relies on the blk-crypto framework
for encryption, and thus requires either hardware inline encryption
support or the blk-crypto-fallback.
Filesystems can be configured with metadata encryption support using the
f2fs userspace tools (mkfs.f2fs). Once formatted, F2FS filesystems with
metadata encryption can be mounted as long as the required key is present.
fscrypt looks for a logon key with the key descriptor=
fscrypt:<metadata_key_identifier>. The metadata_key_identifier is stored in
the filesystem superblock (and the userspace tools print the required
key descriptor).
Right now, the superblock of the filesystem is itself not encrypted. F2FS
reads the superblock using sb_bread, which uses the bd_inode of the block
device as the address space for any data it reads from the block device -
the data read under the bd_inode address space must be what is physically
present on disk (i.e. if the superblock is encrypted, then the ciphertext
of the superblock must be present in the page cache in the bd_inode's
address space), but f2fs requires that the superblock is decrypted by
blk-crypto, which would put the decrypted page contents into the page cache
instead. We could make f2fs read the superblock by submitting bios directly
with a separate address space, but we choose to just not encrypt the
superblock for now.
Not encrypting the superblock allows us to store the encryption algorithm
used for metadata encryption within the superblock itself, which simplifies
a few things. The userspace tools will store the encryption algorithm in
the superblock when formatting the FS.
Direct I/O with metadata encryption is also not supported for now.
Attempts to do direct I/O on a metadata encrypted F2FS filesystem will fall
back to using buffered I/O (just as attempts to do direct I/O on fscrypt
encrypted files also fall back to buffered I/O).
Signed-off-by: Satya Tangirala <[email protected]>
---
fs/f2fs/data.c | 17 ++++++++--------
fs/f2fs/f2fs.h | 2 ++
fs/f2fs/super.c | 44 +++++++++++++++++++++++++++++++++++++----
include/linux/f2fs_fs.h | 7 ++++++-
4 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 627164706029..4bb7d1dd2a18 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -460,8 +460,8 @@ static struct bio *__bio_alloc(struct f2fs_io_info *fio, int npages)
return bio;
}
-static void f2fs_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
- pgoff_t first_idx,
+static void f2fs_set_bio_crypt_ctx(struct bio *bio, block_t blk_addr,
+ const struct inode *inode, pgoff_t first_idx,
const struct f2fs_io_info *fio,
gfp_t gfp_mask)
{
@@ -470,7 +470,7 @@ static void f2fs_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
* read/write raw data without encryption.
*/
if (!fio || !fio->encrypted_page)
- fscrypt_set_bio_crypt_ctx(bio, 0, inode, first_idx, gfp_mask);
+ fscrypt_set_bio_crypt_ctx(bio, blk_addr, inode, first_idx, gfp_mask);
}
static bool f2fs_crypt_mergeable_bio(struct bio *bio, const struct inode *inode,
@@ -712,7 +712,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
/* Allocate a new bio */
bio = __bio_alloc(fio, 1);
- f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
+ f2fs_set_bio_crypt_ctx(bio, fio->new_blkaddr, fio->page->mapping->host,
fio->page->index, fio, GFP_NOIO);
if (bio_add_page(bio, page, PAGE_SIZE, 0) < PAGE_SIZE) {
@@ -918,7 +918,8 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
if (!bio) {
bio = __bio_alloc(fio, BIO_MAX_PAGES);
__attach_io_flag(fio);
- f2fs_set_bio_crypt_ctx(bio, fio->page->mapping->host,
+ f2fs_set_bio_crypt_ctx(bio, fio->new_blkaddr,
+ fio->page->mapping->host,
fio->page->index, fio, GFP_NOIO);
bio_set_op_attrs(bio, fio->op, fio->op_flags);
@@ -992,7 +993,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
goto skip;
}
io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
- f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
+ f2fs_set_bio_crypt_ctx(io->bio, fio->new_blkaddr,
+ fio->page->mapping->host,
bio_page->index, fio, GFP_NOIO);
io->fio = *fio;
}
@@ -1039,9 +1041,8 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
if (!bio)
return ERR_PTR(-ENOMEM);
- f2fs_set_bio_crypt_ctx(bio, inode, first_idx, NULL, GFP_NOFS);
-
f2fs_target_device(sbi, blkaddr, bio);
+ f2fs_set_bio_crypt_ctx(bio, blkaddr, inode, first_idx, NULL, GFP_NOFS);
bio->bi_end_io = f2fs_read_end_io;
bio_set_op_attrs(bio, REQ_OP_READ, op_flag);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index cb700d797296..af2c1f5136d9 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -4122,6 +4122,8 @@ static inline bool f2fs_force_buffered_io(struct inode *inode,
if (f2fs_post_read_required(inode))
return true;
+ if (fscrypt_metadata_crypted(sbi->sb))
+ return true;
if (f2fs_is_multi_device(sbi))
return true;
/*
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4872973d7a22..d817aa1cfc18 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -981,7 +981,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
return -EINVAL;
}
#endif
-
if (F2FS_IO_SIZE_BITS(sbi) && !f2fs_lfs_mode(sbi)) {
f2fs_err(sbi, "Should set mode=lfs with %uKB-sized IO",
F2FS_IO_SIZE_KB(sbi));
@@ -1268,6 +1267,8 @@ static void f2fs_put_super(struct super_block *sb)
iput(sbi->meta_inode);
sbi->meta_inode = NULL;
+ fscrypt_free_metadata_encryption(sb);
+
/*
* iput() can update stat information, if f2fs_write_checkpoint()
* above failed with error.
@@ -2533,6 +2534,9 @@ static int f2fs_get_num_devices(struct super_block *sb)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
+ if (!sbi)
+ return 0;
+
if (f2fs_is_multi_device(sbi))
return sbi->s_ndevs;
return 1;
@@ -2910,6 +2914,13 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
return -EFSCORRUPTED;
}
+ /* Check if FS has metadata encryption if kernel doesn't support it */
+#ifndef CONFIG_FS_ENCRYPTION_METADATA
+ if (raw_super->metadata_crypt_alg) {
+ f2fs_err(sbi, "Filesystem has metadata encryption but kernel support for it wasn't enabled");
+ return -EINVAL;
+ }
+#endif
/* check CP/SIT/NAT/SSA/MAIN_AREA area boundary */
if (sanity_check_area_boundary(sbi, bh))
return -EFSCORRUPTED;
@@ -3510,6 +3521,21 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
sizeof(raw_super->uuid));
default_options(sbi);
+
+#ifdef CONFIG_FS_ENCRYPTION
+ sb->s_cop = &f2fs_cryptops;
+#endif
+ if (sbi->raw_super->metadata_crypt_alg) {
+ err = fscrypt_setup_metadata_encryption(sb,
+ sbi->raw_super->metadata_crypt_key_ident,
+ le32_to_cpu(sbi->raw_super->metadata_crypt_alg),
+ sizeof(block_t));
+ if (err) {
+ f2fs_err(sbi, "Could not setup metadata encryption");
+ goto free_sb_buf;
+ }
+ }
+
/* parse mount options */
options = kstrdup((const char *)data, GFP_KERNEL);
if (data && !options) {
@@ -3544,9 +3570,6 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
#endif
sb->s_op = &f2fs_sops;
-#ifdef CONFIG_FS_ENCRYPTION
- sb->s_cop = &f2fs_cryptops;
-#endif
#ifdef CONFIG_FS_VERITY
sb->s_vop = &f2fs_verityops;
#endif
@@ -3658,6 +3681,12 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
goto free_devices;
}
+ err = fscrypt_metadata_crypt_prepare_all_devices(sb);
+ if (err) {
+ f2fs_err(sbi, "Failed to initialize metadata crypt on all devices");
+ goto free_devices;
+ }
+
err = f2fs_init_post_read_wq(sbi);
if (err) {
f2fs_err(sbi, "Failed to initialize post read workqueue");
@@ -3860,6 +3889,12 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
f2fs_notice(sbi, "Mounted with checkpoint version = %llx",
cur_cp_version(F2FS_CKPT(sbi)));
+ if (fscrypt_metadata_crypted(sb)) {
+ f2fs_notice(sbi, "Mounted with metadata key identifier = %s%*phN",
+ FSCRYPT_KEY_DESC_PREFIX,
+ FSCRYPT_KEY_IDENTIFIER_SIZE,
+ sbi->raw_super->metadata_crypt_key_ident);
+ }
f2fs_update_time(sbi, CP_TIME);
f2fs_update_time(sbi, REQ_TIME);
clear_sbi_flag(sbi, SBI_CP_DISABLED_QUICK);
@@ -3931,6 +3966,7 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
fscrypt_free_dummy_policy(&F2FS_OPTION(sbi).dummy_enc_policy);
kvfree(options);
free_sb_buf:
+ fscrypt_free_metadata_encryption(sb);
kfree(raw_super);
free_sbi:
if (sbi->s_chksum_driver)
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index a5dbb57a687f..34b2f6156694 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -10,6 +10,7 @@
#include <linux/pagemap.h>
#include <linux/types.h>
+#include <linux/fscrypt.h>
#define F2FS_SUPER_OFFSET 1024 /* byte-size offset */
#define F2FS_MIN_LOG_SECTOR_SIZE 9 /* 9 bits for 512 bytes */
@@ -115,7 +116,11 @@ struct f2fs_super_block {
__u8 hot_ext_count; /* # of hot file extension */
__le16 s_encoding; /* Filename charset encoding */
__le16 s_encoding_flags; /* Filename charset encoding flags */
- __u8 reserved[306]; /* valid reserved region */
+ /* The metadata encryption algorithm (FSCRYPT_MODE_*) */
+ __le32 metadata_crypt_alg;
+ /* The metadata encryption key identifier */
+ __u8 metadata_crypt_key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ __u8 reserved[286]; /* valid reserved region */
__le32 crc; /* checksum of superblock */
} __packed;
--
2.29.2.729.g45daf8777d-goog
The new function takes the super_block and the index of a device, and
returns the request_queue of the device at that index (whereas the old
function would take a pointer to an array of request_queues and fill them
all up). This allows callers to avoid allocating an array of request_queues
in some cases (when they don't need the array for anything else).
Signed-off-by: Satya Tangirala <[email protected]>
---
fs/crypto/inline_crypt.c | 33 ++++++++++++++-------------------
fs/f2fs/super.c | 16 ++++++++++------
include/linux/fscrypt.h | 4 ++--
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/fs/crypto/inline_crypt.c b/fs/crypto/inline_crypt.c
index c57bebfa48fe..8234217b42f4 100644
--- a/fs/crypto/inline_crypt.c
+++ b/fs/crypto/inline_crypt.c
@@ -33,13 +33,15 @@ static int fscrypt_get_num_devices(struct super_block *sb)
return 1;
}
-static void fscrypt_get_devices(struct super_block *sb, int num_devs,
- struct request_queue **devs)
+static struct request_queue *fscrypt_get_device(struct super_block *sb,
+ unsigned int device_index)
{
- if (num_devs == 1)
- devs[0] = bdev_get_queue(sb->s_bdev);
+ if (sb->s_cop->get_device)
+ return sb->s_cop->get_device(sb, device_index);
+ else if (WARN_ON_ONCE(device_index != 0))
+ return NULL;
else
- sb->s_cop->get_devices(sb, devs);
+ return bdev_get_queue(sb->s_bdev);
}
static unsigned int fscrypt_get_dun_bytes(const struct fscrypt_info *ci)
@@ -70,7 +72,7 @@ int fscrypt_select_encryption_impl(struct fscrypt_info *ci)
struct super_block *sb = inode->i_sb;
struct blk_crypto_config crypto_cfg;
int num_devs;
- struct request_queue **devs;
+ struct request_queue *dev;
int i;
/* The file must need contents encryption, not filenames encryption */
@@ -106,20 +108,14 @@ int fscrypt_select_encryption_impl(struct fscrypt_info *ci)
crypto_cfg.data_unit_size = sb->s_blocksize;
crypto_cfg.dun_bytes = fscrypt_get_dun_bytes(ci);
num_devs = fscrypt_get_num_devices(sb);
- devs = kmalloc_array(num_devs, sizeof(*devs), GFP_KERNEL);
- if (!devs)
- return -ENOMEM;
- fscrypt_get_devices(sb, num_devs, devs);
for (i = 0; i < num_devs; i++) {
- if (!blk_crypto_config_supported(devs[i], &crypto_cfg))
- goto out_free_devs;
+ dev = fscrypt_get_device(sb, i);
+ if (!dev || !blk_crypto_config_supported(dev, &crypto_cfg))
+ return 0;
}
ci->ci_inlinecrypt = true;
-out_free_devs:
- kfree(devs);
-
return 0;
}
@@ -140,9 +136,6 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
if (!blk_key)
return -ENOMEM;
- blk_key->num_devs = num_devs;
- fscrypt_get_devices(sb, num_devs, blk_key->devs);
-
err = blk_crypto_init_key(&blk_key->base, raw_key, crypto_mode,
fscrypt_get_dun_bytes(ci), sb->s_blocksize);
if (err) {
@@ -157,8 +150,10 @@ int fscrypt_prepare_inline_crypt_key(struct fscrypt_prepared_key *prep_key,
* aren't destroyed until after the filesystem was already unmounted
* (namely, the per-mode keys in struct fscrypt_master_key).
*/
+ blk_key->num_devs = num_devs;
for (i = 0; i < num_devs; i++) {
- if (!blk_get_queue(blk_key->devs[i])) {
+ blk_key->devs[i] = fscrypt_get_device(sb, i);
+ if (!blk_key->devs[i] || !blk_get_queue(blk_key->devs[i])) {
fscrypt_err(inode, "couldn't get request_queue");
err = -EAGAIN;
goto fail;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 00eff2f51807..4872973d7a22 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2538,14 +2538,18 @@ static int f2fs_get_num_devices(struct super_block *sb)
return 1;
}
-static void f2fs_get_devices(struct super_block *sb,
- struct request_queue **devs)
+static struct request_queue *f2fs_get_device(struct super_block *sb,
+ unsigned int device_index)
{
struct f2fs_sb_info *sbi = F2FS_SB(sb);
- int i;
- for (i = 0; i < sbi->s_ndevs; i++)
- devs[i] = bdev_get_queue(FDEV(i).bdev);
+ if (WARN_ON_ONCE(device_index >= f2fs_get_num_devices(sb)))
+ return NULL;
+
+ if (!f2fs_is_multi_device(sbi))
+ return bdev_get_queue(sb->s_bdev);
+
+ return bdev_get_queue(FDEV(device_index).bdev);
}
static const struct fscrypt_operations f2fs_cryptops = {
@@ -2558,7 +2562,7 @@ static const struct fscrypt_operations f2fs_cryptops = {
.has_stable_inodes = f2fs_has_stable_inodes,
.get_ino_and_lblk_bits = f2fs_get_ino_and_lblk_bits,
.get_num_devices = f2fs_get_num_devices,
- .get_devices = f2fs_get_devices,
+ .get_device = f2fs_get_device,
};
#endif
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index a8f7a43f031b..a7d1af4932aa 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -68,8 +68,8 @@ struct fscrypt_operations {
void (*get_ino_and_lblk_bits)(struct super_block *sb,
int *ino_bits_ret, int *lblk_bits_ret);
int (*get_num_devices)(struct super_block *sb);
- void (*get_devices)(struct super_block *sb,
- struct request_queue **devs);
+ struct request_queue *(*get_device)(struct super_block *sb,
+ unsigned int dev_index);
};
static inline struct fscrypt_info *fscrypt_get_info(const struct inode *inode)
--
2.29.2.729.g45daf8777d-goog
On Thu, Dec 17, 2020 at 03:04:32PM +0000, Satya Tangirala wrote:
> Changes to the userspace
> tools (that are required to test out metadata encryption with F2FS) are
> also being sent out - I'll post a link as a reply to this mail once it's
> out.
The userspace changes are at
https://lore.kernel.org/linux-f2fs-devel/[email protected]/
I'm not yet done with the xfstests that Eric asked for - I'll send them
out as soon as they're done.
On Thu, Dec 17, 2020 at 03:04:32PM +0000, Satya Tangirala wrote:
> This patch series adds support for metadata encryption to F2FS using
> blk-crypto.
Is there a companion patch series needed so that f2fstools can
check/repair a file system with metadata encryption enabled?
- Ted
On Thu, Dec 17, 2020 at 01:08:49PM -0500, Theodore Y. Ts'o wrote:
> On Thu, Dec 17, 2020 at 03:04:32PM +0000, Satya Tangirala wrote:
> > This patch series adds support for metadata encryption to F2FS using
> > blk-crypto.
>
> Is there a companion patch series needed so that f2fstools can
> check/repair a file system with metadata encryption enabled?
>
> - Ted
Yes! It's at
https://lore.kernel.org/linux-f2fs-devel/[email protected]/
On Thu, Dec 17, 2020 at 08:51:14PM +0000, Satya Tangirala wrote:
> On Thu, Dec 17, 2020 at 01:08:49PM -0500, Theodore Y. Ts'o wrote:
> > On Thu, Dec 17, 2020 at 03:04:32PM +0000, Satya Tangirala wrote:
> > > This patch series adds support for metadata encryption to F2FS using
> > > blk-crypto.
> >
> > Is there a companion patch series needed so that f2fstools can
> > check/repair a file system with metadata encryption enabled?
> >
> > - Ted
> Yes! It's at
> https://lore.kernel.org/linux-f2fs-devel/[email protected]/
Cool, I've been meaning to update f2fs-tools in Debian, and including
these patches will allow us to generate {kvm,gce,android}-xfstests
images with this support. I'm hoping to get to it sometime betweeen
Christmas and New Year's.
I guess there will need to be some additional work needed to create
the f2fs image with a fixed keys for a particular file system in
xfstests-bld, and then mounting and checking said image with the
appropriatre keys as well. Is that something you've put together?
Cheers,
- Ted
On Thu, Dec 17, 2020 at 06:34:27PM -0500, Theodore Y. Ts'o wrote:
> On Thu, Dec 17, 2020 at 08:51:14PM +0000, Satya Tangirala wrote:
> > On Thu, Dec 17, 2020 at 01:08:49PM -0500, Theodore Y. Ts'o wrote:
> > > On Thu, Dec 17, 2020 at 03:04:32PM +0000, Satya Tangirala wrote:
> > > > This patch series adds support for metadata encryption to F2FS using
> > > > blk-crypto.
> > >
> > > Is there a companion patch series needed so that f2fstools can
> > > check/repair a file system with metadata encryption enabled?
> > >
> > > - Ted
> > Yes! It's at
> > https://lore.kernel.org/linux-f2fs-devel/[email protected]/
>
> Cool, I've been meaning to update f2fs-tools in Debian, and including
> these patches will allow us to generate {kvm,gce,android}-xfstests
> images with this support. I'm hoping to get to it sometime betweeen
> Christmas and New Year's.
>
> I guess there will need to be some additional work needed to create
> the f2fs image with a fixed keys for a particular file system in
> xfstests-bld, and then mounting and checking said image with the
> appropriatre keys as well. Is that something you've put together?
>
I did put something together that sets up metadata encryption on the disks
used by kvm-xfstests. The main code changes were to add a fixed
metadata encryption key with keyctl, and export MKFS_OPTIONS with the
metadata encryption options.
The mkfs options are the only options that need direct modification because
the rest of the tools (fsck/dump etc.) automatically do the right thing if
the FS superblock has the metadata encryption options. But the rest of the
tools do need the metadata encryption key to be present, and some
xfstests/other parts of the harness code clear the keyrings directly, so I
had a few more hacky changes to re-add the keys when they're cleared.
Some more hacky changes were needed because some xfstests override
MKFS_OPTIONS. I'll be happy to send what I have to you/put it up somewhere.
I'll also try to clean up the code a little, but my knowledge of xfstests
is definitely limited so it might take a little while.
> Cheers,
>
> - Ted
I realized I didn't update fscrypt_mergeable_bio() to take metadata
encryption into account, so bios will be more fragmented than required.
I'll fix it in v3.
On Thu, Dec 17, 2020 at 03:04:34PM +0000, Satya Tangirala wrote:
> Introduces functions that help with metadata encryption.
>
> In particular, we introduce:
>
> fscrypt_setup_metadata_encryption() - filesystems should call this function
> to set up metadata encryption on a super block with the encryption
> algorithm (the desired FSCRYPT_MODE_*) and the key identifier of the
> encryption key.fscrypt looks for a logon key with the specified key
> identifier with prefix "fscrypt:". fscrypt will verify that the key
> identifier matches the identifier that is derived using HKDF-512 with the
> logon key as the keying material, no salt, and
> "fscrypt\0|HKDF_CONTEXT_KEY_IDENTIFIER" as the info string.
This describes *what* is done, but not *why*. The why is that we want to ensure
that wrong keys get rejected early on, before the filesystem is mounted.
Also, what happens if we want to add support for a KDF other than HKDF-SHA512 in
the future? With the existing fscrypt use of HKDF-SHA512, we can add support
for more KDFs by using one of the reserved fields in fscrypt_policy_v2 and
fscrypt_add_key_arg to indicate the KDF version. But as proposed, metadata
encryption has no reserved fields in the filesystem superblock (that are
strictly validated). It might be a good idea to add some reserved fields.
> Filesystems should call fscrypt_set_bio_crypt_ctx() on any bio that needs
> either metadata or file contents encryption. fscrypt will choose the
> appropriate key (based on the inode argument) to use for encrypting the
> bio.
Filesystem metadata doesn't necessarily have an inode associated with it. How
does that work?
> diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> index 44b67ebd6e40..708164c413cc 100644
> --- a/Documentation/filesystems/fscrypt.rst
> +++ b/Documentation/filesystems/fscrypt.rst
> @@ -27,8 +27,6 @@ at the block device level. This allows it to encrypt different files
> with different keys and to have unencrypted files on the same
> filesystem. This is useful for multi-user systems where each user's
> data-at-rest needs to be cryptographically isolated from the others.
> -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
> @@ -47,6 +45,15 @@ userspace provides the key, all regular files, directories, and
> symbolic links created in that directory tree are transparently
> encrypted.
>
> +fscrypt also has support for encrypting filesystem metadata, which
> +can be used independently of file contents encryption. For any
> +filesystem block that isn't part of an encrypted file's contents,
> +the filesystem can ask fscrypt to encrypt it with the metadata encryption
> +key set up ahead of time. In general, filesystems should always choose
> +to encrypt a filesystem block with the metadata encryption key, if
> +that block isn't already encrypted with another key (filesystems may
> +choose to leave certain blocks, like the superblock, unencrypted).
We need to be very careful how we describe the metadata encryption feature
because it's not really self-explanatory; people are going to wonder:
- What is meant by "metadata"?
- How does it relate to dm-crypt, and why not just use dm-crypt?
- How does it interact with the existing filenames encryption?
- How does it interact with unencrypted files?
We need to explain it properly so that all these questions are answered.
I recommend adding metadata encryption as a top-level section in the
documentation file (not just in the Introduction and Implementation Details
sections as this patch currently proposes) and adding a proper explanation of it
from a user's perspective. The Introduction should be kept brief.
> +fscrypt protects the confidentiality of non-filename metadata, e.g.
> +file sizes, file permissions, file timestamps, and extended attribute
> +only when metadata encryption support is enabled for the filesystem,
> +and the filesystem chooses to protect such information with the
> +metadata encryption key.
Do we actually anticipate that filesystems might support metadata encryption but
not encrypt these types of metadata? That would be weird, since the purpose of
metadata encryption is basically to ensure that everything gets encrypted.
> +For v2 encryption policies, if the filesystem has a metadata crypt key,
> +the master key is first "mixed" with the metadata crypt key, generating
> +a "mixed-metadata key", which is then used in place of the master key
> +in the process described above. The "mixed-metadata key" is generated
> +by using the metadata crypt key as the input keying material, and
> +a context specific byte and the original master key as the
> +application-specific information string with HKDF-SHA512 (refer to
> +fscrypt_mix_in_metadata_key() for details).
This explains *what* is done, but not *why*. The why is that we want to enforce
that the traditional fscrypt keys are at least as strong as the metadata
encryption key (if there is one).
> diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
> index a5f5c30368a2..58b79d757608 100644
> --- a/fs/crypto/Kconfig
> +++ b/fs/crypto/Kconfig
> @@ -30,3 +30,15 @@ config FS_ENCRYPTION_INLINE_CRYPT
> depends on FS_ENCRYPTION && BLK_INLINE_ENCRYPTION
> help
> Enable fscrypt to use inline encryption hardware if available.
> +
> +config FS_ENCRYPTION_METADATA
> + bool "Enable metadata encryption with fscrypt"
> + depends on FS_ENCRYPTION_INLINE_CRYPT
> + help
> + Enable fscrypt to encrypt metadata. This allows filesystems
> + that support metadata encryption through fscrypt to mount
> + and use filesystem images formatted with metadata encryption
> + enabled. Such filesystem images generally have all
> + otherwise-non-encrypted data (like filesystem metadata,
> + and unencrypted files) encrypted with a metadata encryption
> + key instead.
This could use some explanation of *why* someone would want to enable this.
Also, "Enable metadata encryption" => "Enable support for metadata encryption".
Otherwise people could think that they are enabling encryption just by setting
this kernel config option.
> diff --git a/fs/crypto/bio.c b/fs/crypto/bio.c
> index b048a0e38516..d5c1dc38461b 100644
> --- a/fs/crypto/bio.c
> +++ b/fs/crypto/bio.c
> @@ -59,7 +59,7 @@ static int fscrypt_zeroout_range_inline_crypt(const struct inode *inode,
> unsigned int bytes_this_page = blocks_this_page << blockbits;
>
> if (num_pages == 0) {
> - fscrypt_set_bio_crypt_ctx(bio, inode, lblk, GFP_NOFS);
> + fscrypt_set_bio_crypt_ctx(bio, 0, inode, lblk, GFP_NOFS);
Why is this passing 0 for first_fsblk instead of the filesystem block number?
> /**
> * fscrypt_set_bio_crypt_ctx() - prepare a file contents bio for inline crypto
> * @bio: a bio which will eventually be submitted to the file
> + * @first_fsblk: the first FS block number in the I/O (only used if bio will be
> + * metadata crypted)
> * @inode: the file's inode
> * @first_lblk: the first file logical block number in the I/O
> * @gfp_mask: memory allocation flags - these must be a waiting mask so that
> @@ -234,12 +218,19 @@ static void fscrypt_generate_dun(const struct fscrypt_info *ci, u64 lblk_num,
> *
> * The encryption context will be freed automatically when the bio is freed.
> */
> -void fscrypt_set_bio_crypt_ctx(struct bio *bio, const struct inode *inode,
> - u64 first_lblk, gfp_t gfp_mask)
> +void fscrypt_set_bio_crypt_ctx(struct bio *bio, u64 first_fsblk,
> + const struct inode *inode, u64 first_lblk,
> + gfp_t gfp_mask)
This is going to be confused with fscrypt_metadata_crypt_bio(). Also, the fact
that this now does "metadata encryption" for file contents is confusing. It
would be helpful to update the comments to make it very clear what is going on.
Also, does fscrypt_mergeable_bio() need to be updated too?
> +/**
> + * fscrypt_mix_in_metadata_key() - Mix in the metadata crypt key with an fscrypt
> + * master key
> + * @sb: The superblock whose metadata_crypt_key to mix in
> + * @secret: The secret that needs to be mixed with the metadata_crypt_key
> + *
> + * Replaces @secret->raw with hkdf(key=metadata_crypt_key,
> + * info=HKDF_CONTEXT_MIX_METADATA_KEY|@secret->raw). As such,
> + * @secret->raw is at least as strong as the metadata_crypt_key.
How about: "This makes @secret->raw at least as strong as the stronger of its
old value and the metadata_crypt_key."
> + *
> + * Returns 0 on success and a negative value on error;
> + */
This should be "Return: 0 on success and a negative value on error", otherwise
it isn't a valid kerneldoc comment. You can run
'scripts/kernel-doc -v -none FILE' to check the validity of kerneldoc comments.
> +
> +static int fscrypt_metadata_get_key_from_id(
> + u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
key_ident should be const.
> + unsigned int keysize,
> + u8 raw_key[FSCRYPT_MAX_KEY_SIZE])
Adding a 1-2 sentence description above this function (not necessarily a full
kerneldoc comment) would be helpful to understand what is going on.
> +static int fscrypt_metadata_setup_hkdf(struct super_block *sb,
> + u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
key_ident should be const.
> + unsigned int keysize)
Adding a 1-2 sentence description above this function (not necessarily a full
kerneldoc comment) would be helpful to understand what is going on.
> + sb->s_metadata_hkdf = kzalloc(sizeof(*sb->s_metadata_hkdf), GFP_KERNEL);
> + if (!sb->s_metadata_hkdf)
> + goto out_zero_key;
This is missing 'err = -ENOMEM' in the error path.
> + if (memcmp(key_ident, key_ident_computed, FSCRYPT_KEY_IDENTIFIER_SIZE)) {
> + err = -EINVAL;
> + fscrypt_warn(NULL,
> + "Metadata encryption key did not have the correct key identifier. Rejecting the key.");
> + goto err_destroy_hkdf;
> + }
It might make more sense to log something like "Incorrect metadata encryption
key provided.". The key identifiers being different is just how we determine
whether the keys are different; it's the latter that is actually important.
> +/**
> + * fscrypt_setup_metadata_encryption() - prepare a super_block for metadata
> + * encryption
> + * @sb: The super_block to set up metadata encryption for
> + * @key_ident: The key identifier of a logon key to look for in the process
> + * subscribed keyrings.
> + * @fscrypt_mode_num: The FSCRYPT_MODE_* to use as the encryption algorithm.
> + * @fs_blk_bytes: The number of bytes required to represent fs block numbers.
> + *
> + * Return: 0 on success, negative number on error.
Adding 1-2 more sentences of explanation above would be helpful.
> + */
> +int fscrypt_setup_metadata_encryption(struct super_block *sb,
> + u8 key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE],
key_ident should be const.
> + err = blk_crypto_init_key(sb->s_metadata_key, derived_metadata_key,
> + fscrypt_mode->blk_crypto_mode,
> + fs_blk_bytes, sb->s_blocksize);
> + if (err)
> + goto err_free_metadata_encryption;
> +
> + err = blk_crypto_start_using_key(sb->s_metadata_key,
> + bdev_get_queue(sb->s_bdev));
> + if (err)
> + goto err_free_metadata_encryption;
The calls to blk_crypto_start_using_key() also happen in
fscrypt_metadata_crypt_prepare_all_devices(). So why is this extra one needed?
> +/**
> + * fscrypt_metadata_crypt_prepare_all_devices() - prepare all devices used by
> + * the filesystem for metadata encryption.
prepare_all_devices => prepare_devices, to shorten this name a bit?
> +/**
> + * fscrypt_free_metadata_encryption() - free metadata encryption fields in
> + * super_block.
> + * @sb: The super_block to free metatdata encryption fields from
metatdata => metadata
> + */
> +void fscrypt_free_metadata_encryption(struct super_block *sb)
> +{
> + int num_devices;
> + int i;
> + struct request_queue *q;
> +
> + if (!sb->s_metadata_hkdf)
> + return;
> +
> + fscrypt_destroy_hkdf(sb->s_metadata_hkdf);
> + kfree(sb->s_metadata_hkdf);
It would be good to use kfree_sensitive() here, in the case the implementation
of fscrypt_hkdf changes in the future (though it's not needed currently).
> + sb->s_metadata_hkdf = NULL;
> +
> + if (!sb->s_metadata_key)
> + return;
> +
> + num_devices = fscrypt_get_num_devices(sb);
> +
> + for (i = 0; i < num_devices; i++) {
> + q = fscrypt_get_device(sb, i);
> + if (WARN_ON(!q))
> + continue;
> + blk_crypto_evict_key(q, sb->s_metadata_key);
> + }
> +
> + kfree_sensitive(sb->s_metadata_key);
> + sb->s_metadata_key = NULL;
> +}
> +EXPORT_SYMBOL_GPL(fscrypt_free_metadata_encryption);
This is assuming that s_metadata_hkdf and s_metadata_key got allocated in a
particular order. It would be better to just handle each field one at a time,
like:
void fscrypt_free_metadata_encryption(struct super_block *sb)
{
int num_devices;
int i;
struct request_queue *q;
if (sb->s_metadata_hkdf) {
fscrypt_destroy_hkdf(sb->s_metadata_hkdf);
kfree_sensitive(sb->s_metadata_hkdf);
sb->s_metadata_hkdf = NULL;
}
if (sb->s_metadata_key) {
num_devices = fscrypt_get_num_devices(sb);
for (i = 0; i < num_devices; i++) {
q = fscrypt_get_device(sb, i);
if (WARN_ON(!q))
continue;
blk_crypto_evict_key(q, sb->s_metadata_key);
}
kfree_sensitive(sb->s_metadata_key);
sb->s_metadata_key = NULL;
}
}
> +
> +/**
> + * fscrypt_metadata_crypt_bio() - Add metadata encryption context to bio.
> + *
> + * @bio: The bio to add the encryption context to
> + * @fsblk: The block number within the filesystem at which this bio starts
> + * reading/writing data.
> + * @sb: The superblock of the filesystem
> + * @gfp_mask: gfp_mask for bio_crypt_context allocation
> + */
It would be helpful to add an explanation of how this differs from
fscrypt_set_bio_crypt_ctx(), and how filesystems should decide which one to
call. Also maybe one or both of them needs a better name?
- Eric
On Thu, Dec 17, 2020 at 03:04:35PM +0000, Satya Tangirala wrote:
> Wire up metadata encryption support with the fscrypt metadata crypt
> additions. Note that this feature relies on the blk-crypto framework
> for encryption, and thus requires either hardware inline encryption
> support or the blk-crypto-fallback.
>
> Filesystems can be configured with metadata encryption support using the
> f2fs userspace tools (mkfs.f2fs). Once formatted, F2FS filesystems with
> metadata encryption can be mounted as long as the required key is present.
> fscrypt looks for a logon key with the key descriptor=
> fscrypt:<metadata_key_identifier>. The metadata_key_identifier is stored in
> the filesystem superblock (and the userspace tools print the required
> key descriptor).
>
> Right now, the superblock of the filesystem is itself not encrypted. F2FS
> reads the superblock using sb_bread, which uses the bd_inode of the block
> device as the address space for any data it reads from the block device -
> the data read under the bd_inode address space must be what is physically
> present on disk (i.e. if the superblock is encrypted, then the ciphertext
> of the superblock must be present in the page cache in the bd_inode's
> address space), but f2fs requires that the superblock is decrypted by
> blk-crypto, which would put the decrypted page contents into the page cache
> instead. We could make f2fs read the superblock by submitting bios directly
> with a separate address space, but we choose to just not encrypt the
> superblock for now.
>
> Not encrypting the superblock allows us to store the encryption algorithm
> used for metadata encryption within the superblock itself, which simplifies
> a few things. The userspace tools will store the encryption algorithm in
> the superblock when formatting the FS.
The explanation about why the superblock isn't encrypted seems a bit backwards.
It makes it sound like this decision was mainly an accident because of how f2fs
is currently implemented. But actually we need to leave the superblock
unencrypted anyway in order to keep the filesystem type and metadata encryption
options readable from disk, so that the filesystem can be mounted without
knowing the filesystem type and encryption options ahead of time -- right?
I.e. would anything actually be different if it was super easy to encrypt the
superblock in the kernel?
>
> + /* Check if FS has metadata encryption if kernel doesn't support it */
> +#ifndef CONFIG_FS_ENCRYPTION_METADATA
> + if (raw_super->metadata_crypt_alg) {
> + f2fs_err(sbi, "Filesystem has metadata encryption but kernel support for it wasn't enabled");
> + return -EINVAL;
> + }
> +#endif
This can use !IS_ENABLED(CONFIG_FS_ENCRYPTION_METADATA).
> + if (fscrypt_metadata_crypted(sb)) {
> + f2fs_notice(sbi, "Mounted with metadata key identifier = %s%*phN",
> + FSCRYPT_KEY_DESC_PREFIX,
> + FSCRYPT_KEY_IDENTIFIER_SIZE,
> + sbi->raw_super->metadata_crypt_key_ident);
> + }
Should this show the encryption algorithm too? Maybe:
"Metadata encryption enabled; algorithm=%s, key_identifier=%*phN"
Note that showing the "fscrypt:" key description prefix doesn't really add
anything, so I recommend leaving it out.
> + /* The metadata encryption algorithm (FSCRYPT_MODE_*) */
... or 0 if none.
> + __le32 metadata_crypt_alg;
> + /* The metadata encryption key identifier */
> + __u8 metadata_crypt_key_ident[FSCRYPT_KEY_IDENTIFIER_SIZE];
... (if metadata_crypt_alg != 0)
- Eric