2020-11-18 06:43:22

by Daniel Rosenberg

[permalink] [raw]
Subject: [PATCH v3 0/3] Add support for Encryption and Casefolding in F2FS

These patches are on top of the torvalds tree.

F2FS currently supports casefolding and encryption, but not at
the same time. These patches aim to rectify that. In a later follow up,
this will be added for Ext4 as well.

The f2fs-tools changes have already been applied.

Since both fscrypt and casefolding require their own dentry operations,
I've moved the responsibility of setting the dentry operations from fscrypt
to the filesystems and provided helper functions that should work for most
cases.

These are a follow-up to the previously sent patch set
"[PATCH v12 0/4] Prepare for upcoming Casefolding/Encryption patches"

v2:
Simplified generic dentry_op function
Passed through errors in f2fs_match_ci_name

v3:
Split some long lines
Cleaned up some code
Made some comments clearer
Fixed bug in v2 error passing

Daniel Rosenberg (3):
libfs: Add generic function for setting dentry_ops
fscrypt: Have filesystems handle their d_ops
f2fs: Handle casefolding with Encryption

fs/crypto/fname.c | 4 --
fs/crypto/hooks.c | 1 -
fs/ext4/dir.c | 7 ---
fs/ext4/ext4.h | 4 --
fs/ext4/namei.c | 1 +
fs/ext4/super.c | 5 --
fs/f2fs/dir.c | 105 +++++++++++++++++++++++++++++-----------
fs/f2fs/f2fs.h | 11 ++---
fs/f2fs/hash.c | 11 ++++-
fs/f2fs/inline.c | 4 ++
fs/f2fs/namei.c | 1 +
fs/f2fs/recovery.c | 12 ++++-
fs/f2fs/super.c | 7 ---
fs/libfs.c | 70 +++++++++++++++++++++++++++
fs/ubifs/dir.c | 1 +
include/linux/fs.h | 1 +
include/linux/fscrypt.h | 7 ++-
17 files changed, 185 insertions(+), 67 deletions(-)


base-commit: 0fa8ee0d9ab95c9350b8b84574824d9a384a9f7d
--
2.29.2.454.gaff20da3a2-goog


2020-11-18 06:43:22

by Daniel Rosenberg

[permalink] [raw]
Subject: [PATCH v3 1/3] libfs: Add generic function for setting dentry_ops

This adds a function to set dentry operations at lookup time that will
work for both encrypted filenames and casefolded filenames.

A filesystem that supports both features simultaneously can use this
function during lookup preparations to set up its dentry operations once
fscrypt no longer does that itself.

Currently the casefolding dentry operation are always set if the
filesystem defines an encoding because the features is toggleable on
empty directories. Unlike in the encryption case, the dentry operations
used come from the parent. Since we don't know what set of functions
we'll eventually need, and cannot change them later, we enable the
casefolding operations if the filesystem supports them at all.

By splitting out the various cases, we support as few dentry operations
as we can get away with, maximizing compatibility with overlayfs, which
will not function if a filesystem supports certain dentry_operations.

Signed-off-by: Daniel Rosenberg <[email protected]>
---
fs/libfs.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 1 +
2 files changed, 71 insertions(+)

diff --git a/fs/libfs.c b/fs/libfs.c
index fc34361c1489..babef1f7b50e 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -1449,4 +1449,74 @@ int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str)
return 0;
}
EXPORT_SYMBOL(generic_ci_d_hash);
+
+static const struct dentry_operations generic_ci_dentry_ops = {
+ .d_hash = generic_ci_d_hash,
+ .d_compare = generic_ci_d_compare,
+};
+#endif
+
+#ifdef CONFIG_FS_ENCRYPTION
+static const struct dentry_operations generic_encrypted_dentry_ops = {
+ .d_revalidate = fscrypt_d_revalidate,
+};
+#endif
+
+#if IS_ENABLED(CONFIG_UNICODE) && IS_ENABLED(CONFIG_FS_ENCRYPTION)
+static const struct dentry_operations generic_encrypted_ci_dentry_ops = {
+ .d_hash = generic_ci_d_hash,
+ .d_compare = generic_ci_d_compare,
+ .d_revalidate = fscrypt_d_revalidate,
+};
+#endif
+
+/**
+ * generic_set_encrypted_ci_d_ops - helper for setting d_ops for given dentry
+ * @dentry: dentry to set ops on
+ *
+ * Casefolded directories need d_hash and d_compare set, so that the dentries
+ * contained in them are handled case-insensitively. Note that these operations
+ * are needed on the parent directory rather than on the dentries in it, and
+ * while the casefolding flag can be toggled on and off on an empty directory,
+ * dentry_operations can't be changed later. As a result, if the filesystem has
+ * casefolding support enabled at all, we have to give all dentries the
+ * casefolding operations even if their inode doesn't have the casefolding flag
+ * currently (and thus the casefolding ops would be no-ops for now).
+ *
+ * Encryption works differently in that the only dentry operation it needs is
+ * d_revalidate, which it only needs on dentries that have the no-key name flag.
+ * The no-key flag can't be set "later", so we don't have to worry about that.
+ *
+ * Finally, to maximize compatibility with overlayfs (which isn't compatible
+ * with certain dentry operations) and to avoid taking an unnecessary
+ * performance hit, we use custom dentry_operations for each possible
+ * combination rather than always installing all operations.
+ */
+void generic_set_encrypted_ci_d_ops(struct dentry *dentry)
+{
+#ifdef CONFIG_FS_ENCRYPTION
+ bool needs_encrypt_ops = dentry->d_flags & DCACHE_NOKEY_NAME;
+#endif
+#ifdef CONFIG_UNICODE
+ bool needs_ci_ops = dentry->d_sb->s_encoding;
+#endif
+#if defined(CONFIG_FS_ENCRYPTION) && defined(CONFIG_UNICODE)
+ if (needs_encrypt_ops && needs_ci_ops) {
+ d_set_d_op(dentry, &generic_encrypted_ci_dentry_ops);
+ return;
+ }
#endif
+#ifdef CONFIG_FS_ENCRYPTION
+ if (needs_encrypt_ops) {
+ d_set_d_op(dentry, &generic_encrypted_dentry_ops);
+ return;
+ }
+#endif
+#ifdef CONFIG_UNICODE
+ if (needs_ci_ops) {
+ d_set_d_op(dentry, &generic_ci_dentry_ops);
+ return;
+ }
+#endif
+}
+EXPORT_SYMBOL(generic_set_encrypted_ci_d_ops);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8667d0cdc71e..11345e66353b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3202,6 +3202,7 @@ extern int generic_ci_d_hash(const struct dentry *dentry, struct qstr *str);
extern int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
const char *str, const struct qstr *name);
#endif
+extern void generic_set_encrypted_ci_d_ops(struct dentry *dentry);

#ifdef CONFIG_MIGRATION
extern int buffer_migrate_page(struct address_space *,
--
2.29.2.454.gaff20da3a2-goog

2020-11-18 06:44:33

by Daniel Rosenberg

[permalink] [raw]
Subject: [PATCH v3 3/3] f2fs: Handle casefolding with Encryption

Expand f2fs's casefolding support to include encrypted directories. To
index casefolded+encrypted directories, we use the SipHash of the
casefolded name, keyed by a key derived from the directory's fscrypt
master key. This ensures that the dirhash doesn't leak information
about the plaintext filenames.

Encryption keys are unavailable during roll-forward recovery, so we
can't compute the dirhash when recovering a new dentry in an encrypted +
casefolded directory. To avoid having to force a checkpoint when a new
file is fsync'ed, store the dirhash on-disk appended to i_name.

This patch incorporates work by Eric Biggers <[email protected]>
and Jaegeuk Kim <[email protected]>.

Co-developed-by: Eric Biggers <[email protected]>
Signed-off-by: Eric Biggers <[email protected]>
Signed-off-by: Daniel Rosenberg <[email protected]>
---
fs/f2fs/dir.c | 98 +++++++++++++++++++++++++++++++++++-----------
fs/f2fs/f2fs.h | 8 ++--
fs/f2fs/hash.c | 11 +++++-
fs/f2fs/inline.c | 4 ++
fs/f2fs/recovery.c | 12 +++++-
fs/f2fs/super.c | 6 ---
6 files changed, 106 insertions(+), 33 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 71fdf5076461..82b58d1f80eb 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -5,6 +5,7 @@
* Copyright (c) 2012 Samsung Electronics Co., Ltd.
* http://www.samsung.com/
*/
+#include <asm/unaligned.h>
#include <linux/fs.h>
#include <linux/f2fs_fs.h>
#include <linux/sched/signal.h>
@@ -206,30 +207,55 @@ static struct f2fs_dir_entry *find_in_block(struct inode *dir,
/*
* Test whether a case-insensitive directory entry matches the filename
* being searched for.
+ *
+ * Returns 1 for a match, 0 for no match, and -errno on an error.
*/
-static bool f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
+static int f2fs_match_ci_name(const struct inode *dir, const struct qstr *name,
const u8 *de_name, u32 de_name_len)
{
const struct super_block *sb = dir->i_sb;
const struct unicode_map *um = sb->s_encoding;
+ struct fscrypt_str decrypted_name = FSTR_INIT(NULL, de_name_len);
struct qstr entry = QSTR_INIT(de_name, de_name_len);
int res;

+ if (IS_ENCRYPTED(dir)) {
+ const struct fscrypt_str encrypted_name =
+ FSTR_INIT((u8 *)de_name, de_name_len);
+
+ if (WARN_ON_ONCE(!fscrypt_has_encryption_key(dir)))
+ return -EINVAL;
+
+ decrypted_name.name = kmalloc(de_name_len, GFP_KERNEL);
+ if (!decrypted_name.name)
+ return -ENOMEM;
+ res = fscrypt_fname_disk_to_usr(dir, 0, 0, &encrypted_name,
+ &decrypted_name);
+ if (res < 0)
+ goto out;
+ entry.name = decrypted_name.name;
+ entry.len = decrypted_name.len;
+ }
+
res = utf8_strncasecmp_folded(um, name, &entry);
- if (res < 0) {
- /*
- * In strict mode, ignore invalid names. In non-strict mode,
- * fall back to treating them as opaque byte sequences.
- */
- if (sb_has_strict_encoding(sb) || name->len != entry.len)
- return false;
- return !memcmp(name->name, entry.name, name->len);
+ /*
+ * In strict mode, ignore invalid names. In non-strict mode,
+ * fall back to treating them as opaque byte sequences.
+ */
+ if (res < 0 && !sb_has_strict_encoding(sb)) {
+ res = name->len == entry.len &&
+ memcmp(name->name, entry.name, name->len) == 0;
+ } else {
+ /* utf8_strncasecmp_folded returns 0 on match */
+ res = (res == 0);
}
- return res == 0;
+out:
+ kfree(decrypted_name.name);
+ return res;
}
#endif /* CONFIG_UNICODE */

-static inline bool f2fs_match_name(const struct inode *dir,
+static inline int f2fs_match_name(const struct inode *dir,
const struct f2fs_filename *fname,
const u8 *de_name, u32 de_name_len)
{
@@ -256,6 +282,7 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
struct f2fs_dir_entry *de;
unsigned long bit_pos = 0;
int max_len = 0;
+ int res = 0;

if (max_slots)
*max_slots = 0;
@@ -273,10 +300,15 @@ struct f2fs_dir_entry *f2fs_find_target_dentry(const struct f2fs_dentry_ptr *d,
continue;
}

- if (de->hash_code == fname->hash &&
- f2fs_match_name(d->inode, fname, d->filename[bit_pos],
- le16_to_cpu(de->name_len)))
- goto found;
+ if (de->hash_code == fname->hash) {
+ res = f2fs_match_name(d->inode, fname,
+ d->filename[bit_pos],
+ le16_to_cpu(de->name_len));
+ if (res < 0)
+ return ERR_PTR(res);
+ if (res)
+ goto found;
+ }

if (max_slots && max_len > *max_slots)
*max_slots = max_len;
@@ -326,7 +358,11 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
}

de = find_in_block(dir, dentry_page, fname, &max_slots);
- if (de) {
+ if (IS_ERR(de)) {
+ *res_page = ERR_CAST(de);
+ de = NULL;
+ break;
+ } else if (de) {
*res_page = dentry_page;
break;
}
@@ -448,17 +484,39 @@ void f2fs_set_link(struct inode *dir, struct f2fs_dir_entry *de,
f2fs_put_page(page, 1);
}

-static void init_dent_inode(const struct f2fs_filename *fname,
+static void init_dent_inode(struct inode *dir, struct inode *inode,
+ const struct f2fs_filename *fname,
struct page *ipage)
{
struct f2fs_inode *ri;

+ if (!fname) /* tmpfile case? */
+ return;
+
f2fs_wait_on_page_writeback(ipage, NODE, true, true);

/* copy name info. to this inode page */
ri = F2FS_INODE(ipage);
ri->i_namelen = cpu_to_le32(fname->disk_name.len);
memcpy(ri->i_name, fname->disk_name.name, fname->disk_name.len);
+ if (IS_ENCRYPTED(dir)) {
+ file_set_enc_name(inode);
+ /*
+ * Roll-forward recovery doesn't have encryption keys available,
+ * so it can't compute the dirhash for encrypted+casefolded
+ * filenames. Append it to i_name if possible. Else, disable
+ * roll-forward recovery of the dentry (i.e., make fsync'ing the
+ * file force a checkpoint) by setting LOST_PINO.
+ */
+ if (IS_CASEFOLDED(dir)) {
+ if (fname->disk_name.len + sizeof(f2fs_hash_t) <=
+ F2FS_NAME_LEN)
+ put_unaligned(fname->hash, (f2fs_hash_t *)
+ &ri->i_name[fname->disk_name.len]);
+ else
+ file_lost_pino(inode);
+ }
+ }
set_page_dirty(ipage);
}

@@ -541,11 +599,7 @@ struct page *f2fs_init_inode_metadata(struct inode *inode, struct inode *dir,
return page;
}

- if (fname) {
- init_dent_inode(fname, page);
- if (IS_ENCRYPTED(dir))
- file_set_enc_name(inode);
- }
+ init_dent_inode(dir, inode, fname, page);

/*
* This file should be checkpointed during fsync.
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 62b4f31d30e2..878308736761 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -533,9 +533,11 @@ struct f2fs_filename {
#ifdef CONFIG_UNICODE
/*
* For casefolded directories: the casefolded name, but it's left NULL
- * if the original name is not valid Unicode or if the filesystem is
- * doing an internal operation where usr_fname is also NULL. In these
- * cases we fall back to treating the name as an opaque byte sequence.
+ * if the original name is not valid Unicode, if the directory is both
+ * casefolded and encrypted and its encryption key is unavailable, or if
+ * the filesystem is doing an internal operation where usr_fname is also
+ * NULL. In all these cases we fall back to treating the name as an
+ * opaque byte sequence.
*/
struct fscrypt_str cf_name;
#endif
diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
index de841aaf3c43..e3beac546c63 100644
--- a/fs/f2fs/hash.c
+++ b/fs/f2fs/hash.c
@@ -111,7 +111,9 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
* If the casefolded name is provided, hash it instead of the
* on-disk name. If the casefolded name is *not* provided, that
* should only be because the name wasn't valid Unicode, so fall
- * back to treating the name as an opaque byte sequence.
+ * back to treating the name as an opaque byte sequence. Note
+ * that to handle encrypted directories, the fallback must use
+ * usr_fname (plaintext) rather than disk_name (ciphertext).
*/
WARN_ON_ONCE(!fname->usr_fname->name);
if (fname->cf_name.name) {
@@ -121,6 +123,13 @@ void f2fs_hash_filename(const struct inode *dir, struct f2fs_filename *fname)
name = fname->usr_fname->name;
len = fname->usr_fname->len;
}
+ if (IS_ENCRYPTED(dir)) {
+ struct qstr tmp = QSTR_INIT(name, len);
+
+ fname->hash =
+ cpu_to_le32(fscrypt_fname_siphash(dir, &tmp));
+ return;
+ }
}
#endif
fname->hash = cpu_to_le32(TEA_hash_name(name, len));
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 70384e31788d..92e9852d316a 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -332,6 +332,10 @@ struct f2fs_dir_entry *f2fs_find_in_inline_dir(struct inode *dir,
make_dentry_ptr_inline(dir, &d, inline_dentry);
de = f2fs_find_target_dentry(&d, fname, NULL);
unlock_page(ipage);
+ if (IS_ERR(de)) {
+ *res_page = ERR_CAST(de);
+ de = NULL;
+ }
if (de)
*res_page = ipage;
else
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 4f12ade6410a..0947d36af1a8 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -5,6 +5,7 @@
* Copyright (c) 2012 Samsung Electronics Co., Ltd.
* http://www.samsung.com/
*/
+#include <asm/unaligned.h>
#include <linux/fs.h>
#include <linux/f2fs_fs.h>
#include "f2fs.h"
@@ -128,7 +129,16 @@ static int init_recovered_filename(const struct inode *dir,
}

/* Compute the hash of the filename */
- if (IS_CASEFOLDED(dir)) {
+ if (IS_ENCRYPTED(dir) && IS_CASEFOLDED(dir)) {
+ /*
+ * In this case the hash isn't computable without the key, so it
+ * was saved on-disk.
+ */
+ if (fname->disk_name.len + sizeof(f2fs_hash_t) > F2FS_NAME_LEN)
+ return -EINVAL;
+ fname->hash = get_unaligned((f2fs_hash_t *)
+ &raw_inode->i_name[fname->disk_name.len]);
+ } else if (IS_CASEFOLDED(dir)) {
err = f2fs_init_casefolded_name(dir, fname);
if (err)
return err;
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f51d52591c99..42293b7ceaf2 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3399,12 +3399,6 @@ static int f2fs_setup_casefold(struct f2fs_sb_info *sbi)
struct unicode_map *encoding;
__u16 encoding_flags;

- if (f2fs_sb_has_encrypt(sbi)) {
- f2fs_err(sbi,
- "Can't mount with encoding and encryption");
- return -EINVAL;
- }
-
if (f2fs_sb_read_encoding(sbi->raw_super, &encoding_info,
&encoding_flags)) {
f2fs_err(sbi,
--
2.29.2.454.gaff20da3a2-goog

2020-11-18 18:33:48

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] libfs: Add generic function for setting dentry_ops

On Wed, Nov 18, 2020 at 06:42:43AM +0000, Daniel Rosenberg wrote:
> +#if IS_ENABLED(CONFIG_UNICODE) && IS_ENABLED(CONFIG_FS_ENCRYPTION)
> +static const struct dentry_operations generic_encrypted_ci_dentry_ops = {
> + .d_hash = generic_ci_d_hash,
> + .d_compare = generic_ci_d_compare,
> + .d_revalidate = fscrypt_d_revalidate,
> +};
> +#endif

One nit: it would be good to change the #if condition above to:

#if defined(CONFIG_FS_ENCRYPTION) && defined(CONFIG_UNICODE)

... to make it identical to the #if condition later on:

> +#if defined(CONFIG_FS_ENCRYPTION) && defined(CONFIG_UNICODE)
> + if (needs_encrypt_ops && needs_ci_ops) {
> + d_set_d_op(dentry, &generic_encrypted_ci_dentry_ops);
> + return;
> + }
> #endif

It doesn't actually matter, but it's nice to keep things consistent.

Otherwise, please feel free to add:

Reviewed-by: Eric Biggers <[email protected]>

- Eric

2020-11-18 19:06:35

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] f2fs: Handle casefolding with Encryption

On Wed, Nov 18, 2020 at 06:42:45AM +0000, Daniel Rosenberg wrote:
> Expand f2fs's casefolding support to include encrypted directories. To
> index casefolded+encrypted directories, we use the SipHash of the
> casefolded name, keyed by a key derived from the directory's fscrypt
> master key. This ensures that the dirhash doesn't leak information
> about the plaintext filenames.
>
> Encryption keys are unavailable during roll-forward recovery, so we
> can't compute the dirhash when recovering a new dentry in an encrypted +
> casefolded directory. To avoid having to force a checkpoint when a new
> file is fsync'ed, store the dirhash on-disk appended to i_name.
>
> This patch incorporates work by Eric Biggers <[email protected]>
> and Jaegeuk Kim <[email protected]>.
>
> Co-developed-by: Eric Biggers <[email protected]>
> Signed-off-by: Eric Biggers <[email protected]>
> Signed-off-by: Daniel Rosenberg <[email protected]>

Looks good. If it's needed (some may claim it's not needed because I have a
Co-developed-by), you can add:

Reviewed-by: Eric Biggers <[email protected]>

- Eric