linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Fscrypt support for casefolded encryption
@ 2020-01-07  2:33 Daniel Rosenberg
  2020-01-07  2:33 ` [PATCH v2 1/3] fscrypt: Add siphash and hash key for policy v2 Daniel Rosenberg
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Daniel Rosenberg @ 2020-01-07  2:33 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt
  Cc: linux-kernel, linux-fsdevel, Gabriel Krisman Bertazi,
	kernel-team, Daniel Rosenberg

These patches are to prepare fscrypt to support casefolding and
encryption at the same time. Other patches will add those to the
vfs, ext4 and f2fs. These patches are against fscrypt/master

Daniel Rosenberg (3):
  fscrypt: Add siphash and hash key for policy v2
  fscrypt: Don't allow v1 policies with casefolding
  fscrypt: Change format of no-key token

 fs/crypto/Kconfig           |   1 +
 fs/crypto/fname.c           | 232 ++++++++++++++++++++++++++++--------
 fs/crypto/fscrypt_private.h |   9 ++
 fs/crypto/keysetup.c        |  35 ++++--
 fs/crypto/policy.c          |  39 ++++++
 fs/inode.c                  |   7 ++
 include/linux/fscrypt.h     |  95 ++++-----------
 7 files changed, 284 insertions(+), 134 deletions(-)

-- 
2.24.1.735.g03f4e72817-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] fscrypt: Add siphash and hash key for policy v2
  2020-01-07  2:33 [PATCH v2 0/3] Fscrypt support for casefolded encryption Daniel Rosenberg
@ 2020-01-07  2:33 ` Daniel Rosenberg
  2020-01-07  4:02   ` Eric Biggers
  2020-01-07  2:33 ` [PATCH v2 2/3] fscrypt: Don't allow v1 policies with casefolding Daniel Rosenberg
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Daniel Rosenberg @ 2020-01-07  2:33 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt
  Cc: linux-kernel, linux-fsdevel, Gabriel Krisman Bertazi,
	kernel-team, Daniel Rosenberg

With encryption and casefolding, we cannot simply take the hash of the
ciphertext because of case insensitivity, and we can't take the hash of
the unencrypted name since that would leak information about the
encrypted name. Instead we can use siphash to compute a keyed hash of
the file names.

When a v2 policy is used on a directory, we derive a key for use with
siphash.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/crypto/fname.c           | 22 ++++++++++++++++++++++
 fs/crypto/fscrypt_private.h |  9 +++++++++
 fs/crypto/keysetup.c        | 32 +++++++++++++++++++++++---------
 include/linux/fscrypt.h     |  9 +++++++++
 4 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 3fd27e14ebdd..371e8f01d1c8 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -402,6 +402,28 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 }
 EXPORT_SYMBOL(fscrypt_setup_filename);
 
+/**
+ * fscrypt_fname_siphash() - Calculate the siphash for a file name
+ * @dir: the parent directory
+ * @name: the name of the file to get the siphash of
+ *
+ * Given a user-provided filename @name, this function calculates the siphash of
+ * that name using the directory's hash key.
+ *
+ * This assumes the directory uses a v2 policy, and the key is available.
+ *
+ * Return: the siphash of @name using the hash key of @dir
+ */
+u64 fscrypt_fname_siphash(const struct inode *dir, const struct qstr *name)
+{
+	struct fscrypt_info *ci = dir->i_crypt_info;
+
+	WARN_ON(!ci->ci_hash_key_initialized);
+
+	return siphash(name->name, name->len, &ci->ci_hash_key);
+}
+EXPORT_SYMBOL(fscrypt_fname_siphash);
+
 /*
  * Validate dentries in encrypted directories to make sure we aren't potentially
  * caching stale dentries after a key has been added.
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index b22e8decebed..8b37a5eebb57 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -12,6 +12,7 @@
 #define _FSCRYPT_PRIVATE_H
 
 #include <linux/fscrypt.h>
+#include <linux/siphash.h>
 #include <crypto/hash.h>
 
 #define CONST_STRLEN(str)	(sizeof(str) - 1)
@@ -188,6 +189,13 @@ struct fscrypt_info {
 	 */
 	struct fscrypt_direct_key *ci_direct_key;
 
+	/*
+	 * With v2 policies, this can be used with siphash
+	 * When the key has been set, ci_hash_key_initialized is set to true
+	 */
+	siphash_key_t ci_hash_key;
+	bool ci_hash_key_initialized;
+
 	/* The encryption policy used by this inode */
 	union fscrypt_policy ci_policy;
 
@@ -262,6 +270,7 @@ extern int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
 #define HKDF_CONTEXT_PER_FILE_KEY	2
 #define HKDF_CONTEXT_DIRECT_KEY		3
 #define HKDF_CONTEXT_IV_INO_LBLK_64_KEY	4
+#define HKDF_CONTEXT_FNAME_HASH_KEY     5
 
 extern int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
 			       const u8 *info, unsigned int infolen,
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 96074054bdbc..c1bd897c9310 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -189,7 +189,7 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
 		 * This ensures that the master key is consistently used only
 		 * for HKDF, avoiding key reuse issues.
 		 */
-		return setup_per_mode_key(ci, mk, mk->mk_direct_tfms,
+		err = setup_per_mode_key(ci, mk, mk->mk_direct_tfms,
 					  HKDF_CONTEXT_DIRECT_KEY, false);
 	} else if (ci->ci_policy.v2.flags &
 		   FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64) {
@@ -199,20 +199,34 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
 		 * the IVs.  This format is optimized for use with inline
 		 * encryption hardware compliant with the UFS or eMMC standards.
 		 */
-		return setup_per_mode_key(ci, mk, mk->mk_iv_ino_lblk_64_tfms,
+		err = setup_per_mode_key(ci, mk, mk->mk_iv_ino_lblk_64_tfms,
 					  HKDF_CONTEXT_IV_INO_LBLK_64_KEY,
 					  true);
+	} else {
+		err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
+					  HKDF_CONTEXT_PER_FILE_KEY,
+					  ci->ci_nonce,
+					  FS_KEY_DERIVATION_NONCE_SIZE,
+					  derived_key, ci->ci_mode->keysize);
+		if (err)
+			return err;
+
+		err = fscrypt_set_derived_key(ci, derived_key);
+		memzero_explicit(derived_key, ci->ci_mode->keysize);
 	}
-
-	err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
-				  HKDF_CONTEXT_PER_FILE_KEY,
-				  ci->ci_nonce, FS_KEY_DERIVATION_NONCE_SIZE,
-				  derived_key, ci->ci_mode->keysize);
 	if (err)
 		return err;
 
-	err = fscrypt_set_derived_key(ci, derived_key);
-	memzero_explicit(derived_key, ci->ci_mode->keysize);
+	if (S_ISDIR(ci->ci_inode->i_mode)) {
+		err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
+					  HKDF_CONTEXT_FNAME_HASH_KEY,
+					  ci->ci_nonce,
+					  FS_KEY_DERIVATION_NONCE_SIZE,
+					  (u8 *)&ci->ci_hash_key,
+					  sizeof(ci->ci_hash_key));
+		if (!err)
+			ci->ci_hash_key_initialized = true;
+	}
 	return err;
 }
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 6fe8d0f96a4a..1dfbed855bee 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -172,6 +172,8 @@ extern int fscrypt_fname_disk_to_usr(const struct inode *inode,
 				     u32 hash, u32 minor_hash,
 				     const struct fscrypt_str *iname,
 				     struct fscrypt_str *oname);
+extern u64 fscrypt_fname_siphash(const struct inode *dir,
+				 const struct qstr *name);
 
 #define FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE	32
 
@@ -468,6 +470,13 @@ static inline int fscrypt_fname_disk_to_usr(const struct inode *inode,
 	return -EOPNOTSUPP;
 }
 
+static inline u64 fscrypt_fname_siphash(const struct inode *dir,
+					const struct qstr *name)
+{
+	WARN_ON_ONCE(1);
+	return 0;
+}
+
 static inline bool fscrypt_match_name(const struct fscrypt_name *fname,
 				      const u8 *de_name, u32 de_name_len)
 {
-- 
2.24.1.735.g03f4e72817-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] fscrypt: Don't allow v1 policies with casefolding
  2020-01-07  2:33 [PATCH v2 0/3] Fscrypt support for casefolded encryption Daniel Rosenberg
  2020-01-07  2:33 ` [PATCH v2 1/3] fscrypt: Add siphash and hash key for policy v2 Daniel Rosenberg
@ 2020-01-07  2:33 ` Daniel Rosenberg
  2020-01-07  3:35   ` Eric Biggers
  2020-01-07  2:33 ` [PATCH v2 3/3] fscrypt: Change format of no-key token Daniel Rosenberg
  2020-01-07  3:26 ` [PATCH v2 0/3] Fscrypt support for casefolded encryption Eric Biggers
  3 siblings, 1 reply; 8+ messages in thread
From: Daniel Rosenberg @ 2020-01-07  2:33 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt
  Cc: linux-kernel, linux-fsdevel, Gabriel Krisman Bertazi,
	kernel-team, Daniel Rosenberg

Casefolding currently requires a derived key for computing the siphash.
This is available for v2 policies, but not v1, so we disallow it for v1.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/crypto/keysetup.c    |  7 ++++---
 fs/crypto/policy.c      | 39 +++++++++++++++++++++++++++++++++++++++
 fs/inode.c              |  7 +++++++
 include/linux/fscrypt.h | 11 +++++++++++
 4 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index c1bd897c9310..7445ab76e0b3 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -224,10 +224,11 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
 					  FS_KEY_DERIVATION_NONCE_SIZE,
 					  (u8 *)&ci->ci_hash_key,
 					  sizeof(ci->ci_hash_key));
-		if (!err)
-			ci->ci_hash_key_initialized = true;
+		if (err)
+			return err;
+		ci->ci_hash_key_initialized = true;
 	}
-	return err;
+	return 0;
 }
 
 /*
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index f1cff83c151a..9e937cfa732c 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -124,6 +124,12 @@ static bool fscrypt_supported_v1_policy(const struct fscrypt_policy_v1 *policy,
 					policy->filenames_encryption_mode))
 		return false;
 
+	if (IS_CASEFOLDED(inode)) {
+		fscrypt_warn(inode,
+			     "v1 policy does not support casefolded directories");
+		return false;
+	}
+
 	return true;
 }
 
@@ -579,3 +585,36 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
 	return preload ? fscrypt_get_encryption_info(child): 0;
 }
 EXPORT_SYMBOL(fscrypt_inherit_context);
+
+static int fscrypt_set_casefolding_allowed(struct inode *inode)
+{
+	union fscrypt_policy policy;
+	int err = fscrypt_get_policy(inode, &policy);
+
+	if (err)
+		return err;
+
+	if (policy.version != FSCRYPT_POLICY_V2)
+		return -EINVAL;
+
+	return 0;
+}
+
+int fscrypt_ioc_setflags_prepare(struct inode *inode,
+				 unsigned int oldflags,
+				 unsigned int flags)
+{
+	int err;
+
+	/*
+	 * When a directory is encrypted, the CASEFOLD flag can only be turned
+	 * on if the fscrypt policy supports it.
+	 */
+	if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) {
+		err = fscrypt_set_casefolding_allowed(inode);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
diff --git a/fs/inode.c b/fs/inode.c
index 96d62d97694e..77f3e6e2e934 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -20,6 +20,7 @@
 #include <linux/ratelimit.h>
 #include <linux/list_lru.h>
 #include <linux/iversion.h>
+#include <linux/fscrypt.h>
 #include <trace/events/writeback.h>
 #include "internal.h"
 
@@ -2242,6 +2243,8 @@ EXPORT_SYMBOL(current_time);
 int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags,
 			     unsigned int flags)
 {
+	int err;
+
 	/*
 	 * The IMMUTABLE and APPEND_ONLY flags can only be changed by
 	 * the relevant capability.
@@ -2252,6 +2255,10 @@ int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags,
 	    !capable(CAP_LINUX_IMMUTABLE))
 		return -EPERM;
 
+	err = fscrypt_ioc_setflags_prepare(inode, oldflags, flags);
+	if (err)
+		return err;
+
 	return 0;
 }
 EXPORT_SYMBOL(vfs_ioc_setflags_prepare);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 1dfbed855bee..2c292f19c6b9 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -142,6 +142,10 @@ extern int fscrypt_ioctl_get_policy_ex(struct file *, void __user *);
 extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
 extern int fscrypt_inherit_context(struct inode *, struct inode *,
 					void *, bool);
+extern int fscrypt_ioc_setflags_prepare(struct inode *inode,
+					unsigned int oldflags,
+					unsigned int flags);
+
 /* keyring.c */
 extern void fscrypt_sb_free(struct super_block *sb);
 extern int fscrypt_ioctl_add_key(struct file *filp, void __user *arg);
@@ -383,6 +387,13 @@ static inline int fscrypt_inherit_context(struct inode *parent,
 	return -EOPNOTSUPP;
 }
 
+static inline int fscrypt_ioc_setflags_prepare(struct inode *inode,
+					       unsigned int oldflags,
+					       unsigned int flags)
+{
+	return 0;
+}
+
 /* keyring.c */
 static inline void fscrypt_sb_free(struct super_block *sb)
 {
-- 
2.24.1.735.g03f4e72817-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] fscrypt: Change format of no-key token
  2020-01-07  2:33 [PATCH v2 0/3] Fscrypt support for casefolded encryption Daniel Rosenberg
  2020-01-07  2:33 ` [PATCH v2 1/3] fscrypt: Add siphash and hash key for policy v2 Daniel Rosenberg
  2020-01-07  2:33 ` [PATCH v2 2/3] fscrypt: Don't allow v1 policies with casefolding Daniel Rosenberg
@ 2020-01-07  2:33 ` Daniel Rosenberg
  2020-01-08 22:07   ` Eric Biggers
  2020-01-07  3:26 ` [PATCH v2 0/3] Fscrypt support for casefolded encryption Eric Biggers
  3 siblings, 1 reply; 8+ messages in thread
From: Daniel Rosenberg @ 2020-01-07  2:33 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt
  Cc: linux-kernel, linux-fsdevel, Gabriel Krisman Bertazi,
	kernel-team, Daniel Rosenberg

Fscrypt supplies a no-key token in place of file names when the name is
encrypted and the key is not present. In the current scheme, the no-key
token is the base64 encoded ciphertext of the name, unless the name is
longer than a certain amount, after which it uses an alternative scheme
which includes the directory hash of the file name and an abbreviated
form of the ciphertext. Encrypted and casefolded names always require a
dirtree hash, since their values cannot be generated without the key.

In the new format, we always base64 encode the same structure. For names
that are less than 149 characters, we concatenate the provided hash and
ciphertext. If the name is longer than 149 characters, we also include
the sha256 of the remaining parts of the name. We then base64 encode the
resulting data to get a representation of the name that is at most 252
characters long, with a very low collision rate. We avoid needing to
compute the sha256 apart from in the case of a very long filename, and
then only need to compute the sha256 of possible matches if their
ciphertext is also longer than 149.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fs/crypto/Kconfig       |   1 +
 fs/crypto/fname.c       | 210 ++++++++++++++++++++++++++++++----------
 include/linux/fscrypt.h |  75 +-------------
 3 files changed, 162 insertions(+), 124 deletions(-)

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index 02df95b44331..8046d7c7a3e9 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -21,5 +21,6 @@ config FS_ENCRYPTION_ALGS
 	select CRYPTO_CTS
 	select CRYPTO_ECB
 	select CRYPTO_HMAC
+	select CRYPTO_SHA256
 	select CRYPTO_SHA512
 	select CRYPTO_XTS
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 371e8f01d1c8..4006ffd59ffa 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -13,9 +13,69 @@
 
 #include <linux/namei.h>
 #include <linux/scatterlist.h>
+#include <crypto/hash.h>
+#include <crypto/sha.h>
 #include <crypto/skcipher.h>
 #include "fscrypt_private.h"
 
+/**
+ * fscrypt_nokey_name - identifier for on-disk filenames when key is not present
+ *
+ * When userspace lists an encrypted directory without access to the key, we
+ * must present them with a unique identifier for the file. base64 encoding will
+ * expand the space, so we use this format to avoid most collisions.
+ *
+ * Filesystems may rely on the hash being present to look up a file on disk.
+ * For filenames that are both casefolded and encrypted, it is not possible to
+ * calculate the hash without the key. Additionally, if the ciphertext is longer
+ * than what we can base64 encode, we cannot generate the hash from the partial
+ * name. For simplicity, we always store the hash at the front of the name,
+ * followed by the first 149 bytes of the ciphertext, and then the sha256 of the
+ * remainder of the name if the ciphertext was longer than 149 bytes. For the
+ * usual case of relatively short filenames, this allows us to avoid needing to
+ * compute the sha256. This results in an encoded name that is at most 252 bytes
+ * long.
+ */
+
+#define FSCRYPT_FNAME_UNDIGESTED_SIZE 149
+struct fscrypt_nokey_name {
+	u32 dirtree_hash[2];
+	u8 bytes[FSCRYPT_FNAME_UNDIGESTED_SIZE];
+	u8 sha256[SHA256_DIGEST_SIZE];
+};
+
+static struct crypto_shash *sha256_hash_tfm;
+
+static int fscrypt_do_sha256(unsigned char *result,
+	     const u8 *data, unsigned int data_len)
+{
+	struct crypto_shash *tfm = READ_ONCE(sha256_hash_tfm);
+
+	if (unlikely(!tfm)) {
+		struct crypto_shash *prev_tfm;
+
+		tfm = crypto_alloc_shash("sha256", 0, 0);
+		if (IS_ERR(tfm)) {
+			fscrypt_err(NULL,
+				    "Error allocating SHA-256 transform: %ld",
+				    PTR_ERR(tfm));
+			return PTR_ERR(tfm);
+		}
+		prev_tfm = cmpxchg(&sha256_hash_tfm, NULL, tfm);
+		if (prev_tfm) {
+			crypto_free_shash(tfm);
+			tfm = prev_tfm;
+		}
+	}
+	{
+		SHASH_DESC_ON_STACK(desc, tfm);
+
+		desc->tfm = tfm;
+
+		return crypto_shash_digest(desc, data, data_len, result);
+	}
+}
+
 static inline bool fscrypt_is_dot_dotdot(const struct qstr *str)
 {
 	if (str->len == 1 && str->name[0] == '.')
@@ -208,8 +268,7 @@ int fscrypt_fname_alloc_buffer(const struct inode *inode,
 			       struct fscrypt_str *crypto_str)
 {
 	const u32 max_encoded_len =
-		max_t(u32, BASE64_CHARS(FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE),
-		      1 + BASE64_CHARS(sizeof(struct fscrypt_digested_name)));
+		      BASE64_CHARS(sizeof(struct fscrypt_nokey_name));
 	u32 max_presented_len;
 
 	max_presented_len = max(max_encoded_len, max_encrypted_len);
@@ -242,9 +301,9 @@ EXPORT_SYMBOL(fscrypt_fname_free_buffer);
  *
  * The caller must have allocated sufficient memory for the @oname string.
  *
- * If the key is available, we'll decrypt the disk name; otherwise, we'll encode
- * it for presentation.  Short names are directly base64-encoded, while long
- * names are encoded in fscrypt_digested_name format.
+ * If the key is available, we'll decrypt the disk name;
+ * otherwise, we'll encode it for presentation in fscrypt_nokey_name format.
+ * See struct fscrypt_nokey_name for details.
  *
  * Return: 0 on success, -errno on failure
  */
@@ -254,7 +313,9 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
 			      struct fscrypt_str *oname)
 {
 	const struct qstr qname = FSTR_TO_QSTR(iname);
-	struct fscrypt_digested_name digested_name;
+	struct fscrypt_nokey_name nokey_name;
+	u32 size;
+	int err = 0;
 
 	if (fscrypt_is_dot_dotdot(&qname)) {
 		oname->name[0] = '.';
@@ -269,25 +330,29 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
 	if (fscrypt_has_encryption_key(inode))
 		return fname_decrypt(inode, iname, oname);
 
-	if (iname->len <= FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE) {
-		oname->len = base64_encode(iname->name, iname->len,
-					   oname->name);
-		return 0;
-	}
 	if (hash) {
-		digested_name.hash = hash;
-		digested_name.minor_hash = minor_hash;
+		nokey_name.dirtree_hash[0] = hash;
+		nokey_name.dirtree_hash[1] = minor_hash;
 	} else {
-		digested_name.hash = 0;
-		digested_name.minor_hash = 0;
+		nokey_name.dirtree_hash[0] = 0;
+		nokey_name.dirtree_hash[1] = 0;
 	}
-	memcpy(digested_name.digest,
-	       FSCRYPT_FNAME_DIGEST(iname->name, iname->len),
-	       FSCRYPT_FNAME_DIGEST_SIZE);
-	oname->name[0] = '_';
-	oname->len = 1 + base64_encode((const u8 *)&digested_name,
-				       sizeof(digested_name), oname->name + 1);
-	return 0;
+	if (iname->len <= FSCRYPT_FNAME_UNDIGESTED_SIZE) {
+		memcpy(nokey_name.bytes, iname->name, iname->len);
+		size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]);
+	} else {
+		memcpy(nokey_name.bytes, iname->name,
+		       FSCRYPT_FNAME_UNDIGESTED_SIZE);
+		/* compute sha256 of remaining name */
+		err = fscrypt_do_sha256(nokey_name.sha256,
+				&iname->name[FSCRYPT_FNAME_UNDIGESTED_SIZE],
+				iname->len - FSCRYPT_FNAME_UNDIGESTED_SIZE);
+		if (err)
+			return err;
+		size = offsetofend(struct fscrypt_nokey_name, sha256);
+	}
+	oname->len = base64_encode((const u8 *)&nokey_name, size, oname->name);
+	return err;
 }
 EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
 
@@ -307,8 +372,7 @@ EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
  * get the disk_name.
  *
  * Else, for keyless @lookup operations, @iname is the presented ciphertext, so
- * we decode it to get either the ciphertext disk_name (for short names) or the
- * fscrypt_digested_name (for long names).  Non-@lookup operations will be
+ * we decode it to get the fscrypt_nokey_name. Non-@lookup operations will be
  * impossible in this case, so we fail them with ENOKEY.
  *
  * If successful, fscrypt_free_filename() must be called later to clean up.
@@ -318,8 +382,8 @@ EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
 int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 			      int lookup, struct fscrypt_name *fname)
 {
+	struct fscrypt_nokey_name *nokey_name;
 	int ret;
-	int digested;
 
 	memset(fname, 0, sizeof(struct fscrypt_name));
 	fname->usr_fname = iname;
@@ -359,41 +423,29 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 	 * We don't have the key and we are doing a lookup; decode the
 	 * user-supplied name
 	 */
-	if (iname->name[0] == '_') {
-		if (iname->len !=
-		    1 + BASE64_CHARS(sizeof(struct fscrypt_digested_name)))
-			return -ENOENT;
-		digested = 1;
-	} else {
-		if (iname->len >
-		    BASE64_CHARS(FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE))
-			return -ENOENT;
-		digested = 0;
-	}
 
 	fname->crypto_buf.name =
-		kmalloc(max_t(size_t, FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE,
-			      sizeof(struct fscrypt_digested_name)),
-			GFP_KERNEL);
+			kmalloc(sizeof(struct fscrypt_nokey_name), GFP_KERNEL);
 	if (fname->crypto_buf.name == NULL)
 		return -ENOMEM;
 
-	ret = base64_decode(iname->name + digested, iname->len - digested,
-			    fname->crypto_buf.name);
-	if (ret < 0) {
+	if (iname->len > BASE64_CHARS(sizeof(struct fscrypt_nokey_name))) {
 		ret = -ENOENT;
 		goto errout;
 	}
-	fname->crypto_buf.len = ret;
-	if (digested) {
-		const struct fscrypt_digested_name *n =
-			(const void *)fname->crypto_buf.name;
-		fname->hash = n->hash;
-		fname->minor_hash = n->minor_hash;
-	} else {
-		fname->disk_name.name = fname->crypto_buf.name;
-		fname->disk_name.len = fname->crypto_buf.len;
+	ret = base64_decode(iname->name, iname->len, fname->crypto_buf.name);
+	if ((int)ret < offsetof(struct fscrypt_nokey_name, bytes[1]) ||
+	    (ret > offsetof(struct fscrypt_nokey_name, sha256) &&
+	     ret != offsetofend(struct fscrypt_nokey_name, sha256))) {
+		ret = -ENOENT;
+		goto errout;
 	}
+
+	nokey_name = (void *)fname->crypto_buf.name;
+	fname->crypto_buf.len = ret;
+
+	fname->hash = nokey_name->dirtree_hash[0];
+	fname->minor_hash = nokey_name->dirtree_hash[1];
 	return 0;
 
 errout:
@@ -402,6 +454,62 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 }
 EXPORT_SYMBOL(fscrypt_setup_filename);
 
+/**
+ * fscrypt_match_name() - test whether the given name matches a directory entry
+ * @fname: the name being searched for
+ * @de_name: the name from the directory entry
+ * @de_name_len: the length of @de_name in bytes
+ *
+ * Normally @fname->disk_name will be set, and in that case we simply compare
+ * that to the name stored in the directory entry.  The only exception is that
+ * if we don't have the key for an encrypted directory we'll instead need to
+ * match against the fscrypt_nokey_name.
+ *
+ * Return: %true if the name matches, otherwise %false.
+ */
+bool fscrypt_match_name(const struct fscrypt_name *fname,
+				      const u8 *de_name, u32 de_name_len)
+{
+	BUILD_BUG_ON(BASE64_CHARS(offsetofend(struct fscrypt_nokey_name,
+					      sha256)) > NAME_MAX);
+	if (unlikely(!fname->disk_name.name)) {
+		const struct fscrypt_nokey_name *n =
+			(const void *)fname->crypto_buf.name;
+
+		if (fname->crypto_buf.len ==
+			    offsetofend(struct fscrypt_nokey_name, sha256)) {
+			u8 sha256[SHA256_DIGEST_SIZE];
+
+			if (de_name_len <= FSCRYPT_FNAME_UNDIGESTED_SIZE)
+				return false;
+			if (memcmp(de_name, n->bytes,
+				   FSCRYPT_FNAME_UNDIGESTED_SIZE) != 0)
+				return false;
+			fscrypt_do_sha256(sha256,
+				&de_name[FSCRYPT_FNAME_UNDIGESTED_SIZE],
+				de_name_len - FSCRYPT_FNAME_UNDIGESTED_SIZE);
+			if (memcmp(sha256, n->sha256, sizeof(sha256)) != 0)
+				return false;
+		} else {
+			u32 len = fname->crypto_buf.len -
+				offsetof(struct fscrypt_nokey_name, bytes);
+
+			if (de_name_len != len)
+				return false;
+
+			if (memcmp(de_name, n->bytes, len) != 0)
+				return false;
+		}
+
+		return true;
+	}
+
+	if (de_name_len != fname->disk_name.len)
+		return false;
+	return !memcmp(de_name, fname->disk_name.name, fname->disk_name.len);
+}
+EXPORT_SYMBOL(fscrypt_match_name);
+
 /**
  * fscrypt_fname_siphash() - Calculate the siphash for a file name
  * @dir: the parent directory
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 2c292f19c6b9..14a727759a81 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -179,79 +179,8 @@ extern int fscrypt_fname_disk_to_usr(const struct inode *inode,
 extern u64 fscrypt_fname_siphash(const struct inode *dir,
 				 const struct qstr *name);
 
-#define FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE	32
-
-/* Extracts the second-to-last ciphertext block; see explanation below */
-#define FSCRYPT_FNAME_DIGEST(name, len)	\
-	((name) + round_down((len) - FS_CRYPTO_BLOCK_SIZE - 1, \
-			     FS_CRYPTO_BLOCK_SIZE))
-
-#define FSCRYPT_FNAME_DIGEST_SIZE	FS_CRYPTO_BLOCK_SIZE
-
-/**
- * fscrypt_digested_name - alternate identifier for an on-disk filename
- *
- * When userspace lists an encrypted directory without access to the key,
- * filenames whose ciphertext is longer than FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE
- * bytes are shown in this abbreviated form (base64-encoded) rather than as the
- * full ciphertext (base64-encoded).  This is necessary to allow supporting
- * filenames up to NAME_MAX bytes, since base64 encoding expands the length.
- *
- * To make it possible for filesystems to still find the correct directory entry
- * despite not knowing the full on-disk name, we encode any filesystem-specific
- * 'hash' and/or 'minor_hash' which the filesystem may need for its lookups,
- * followed by the second-to-last ciphertext block of the filename.  Due to the
- * use of the CBC-CTS encryption mode, the second-to-last ciphertext block
- * depends on the full plaintext.  (Note that ciphertext stealing causes the
- * last two blocks to appear "flipped".)  This makes accidental collisions very
- * unlikely: just a 1 in 2^128 chance for two filenames to collide even if they
- * share the same filesystem-specific hashes.
- *
- * However, this scheme isn't immune to intentional collisions, which can be
- * created by anyone able to create arbitrary plaintext filenames and view them
- * without the key.  Making the "digest" be a real cryptographic hash like
- * SHA-256 over the full ciphertext would prevent this, although it would be
- * less efficient and harder to implement, especially since the filesystem would
- * need to calculate it for each directory entry examined during a search.
- */
-struct fscrypt_digested_name {
-	u32 hash;
-	u32 minor_hash;
-	u8 digest[FSCRYPT_FNAME_DIGEST_SIZE];
-};
-
-/**
- * fscrypt_match_name() - test whether the given name matches a directory entry
- * @fname: the name being searched for
- * @de_name: the name from the directory entry
- * @de_name_len: the length of @de_name in bytes
- *
- * Normally @fname->disk_name will be set, and in that case we simply compare
- * that to the name stored in the directory entry.  The only exception is that
- * if we don't have the key for an encrypted directory and a filename in it is
- * very long, then we won't have the full disk_name and we'll instead need to
- * match against the fscrypt_digested_name.
- *
- * Return: %true if the name matches, otherwise %false.
- */
-static inline bool fscrypt_match_name(const struct fscrypt_name *fname,
-				      const u8 *de_name, u32 de_name_len)
-{
-	if (unlikely(!fname->disk_name.name)) {
-		const struct fscrypt_digested_name *n =
-			(const void *)fname->crypto_buf.name;
-		if (WARN_ON_ONCE(fname->usr_fname->name[0] != '_'))
-			return false;
-		if (de_name_len <= FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE)
-			return false;
-		return !memcmp(FSCRYPT_FNAME_DIGEST(de_name, de_name_len),
-			       n->digest, FSCRYPT_FNAME_DIGEST_SIZE);
-	}
-
-	if (de_name_len != fname->disk_name.len)
-		return false;
-	return !memcmp(de_name, fname->disk_name.name, fname->disk_name.len);
-}
+extern bool fscrypt_match_name(const struct fscrypt_name *fname,
+				      const u8 *de_name, u32 de_name_len);
 
 /* bio.c */
 extern void fscrypt_decrypt_bio(struct bio *);
-- 
2.24.1.735.g03f4e72817-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] Fscrypt support for casefolded encryption
  2020-01-07  2:33 [PATCH v2 0/3] Fscrypt support for casefolded encryption Daniel Rosenberg
                   ` (2 preceding siblings ...)
  2020-01-07  2:33 ` [PATCH v2 3/3] fscrypt: Change format of no-key token Daniel Rosenberg
@ 2020-01-07  3:26 ` Eric Biggers
  3 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-01-07  3:26 UTC (permalink / raw)
  To: Daniel Rosenberg
  Cc: linux-fscrypt, linux-kernel, linux-fsdevel,
	Gabriel Krisman Bertazi, kernel-team

On Mon, Jan 06, 2020 at 06:33:20PM -0800, Daniel Rosenberg wrote:
> These patches are to prepare fscrypt to support casefolding and
> encryption at the same time. Other patches will add those to the
> vfs, ext4 and f2fs. These patches are against fscrypt/master
> 
> Daniel Rosenberg (3):
>   fscrypt: Add siphash and hash key for policy v2
>   fscrypt: Don't allow v1 policies with casefolding
>   fscrypt: Change format of no-key token
> 

I think you should send out the full series again so that people have the needed
context when reviewing it.  It can still be on top of fscrypt.git#master if
that's easiest.  We can consider taking these three patches for 5.6 before the
fs/ext4/ and fs/f2fs/ parts in order to help avoid merge conflicts between git
trees, but that doesn't mean you can't send out the full series.

- Eric

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] fscrypt: Don't allow v1 policies with casefolding
  2020-01-07  2:33 ` [PATCH v2 2/3] fscrypt: Don't allow v1 policies with casefolding Daniel Rosenberg
@ 2020-01-07  3:35   ` Eric Biggers
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-01-07  3:35 UTC (permalink / raw)
  To: Daniel Rosenberg
  Cc: linux-fscrypt, linux-kernel, linux-fsdevel,
	Gabriel Krisman Bertazi, kernel-team

On Mon, Jan 06, 2020 at 06:33:22PM -0800, Daniel Rosenberg wrote:
> Casefolding currently requires a derived key for computing the siphash.
> This is available for v2 policies, but not v1, so we disallow it for v1.
> 
> Signed-off-by: Daniel Rosenberg <drosen@google.com>
> ---
>  fs/crypto/keysetup.c    |  7 ++++---
>  fs/crypto/policy.c      | 39 +++++++++++++++++++++++++++++++++++++++
>  fs/inode.c              |  7 +++++++
>  include/linux/fscrypt.h | 11 +++++++++++
>  4 files changed, 61 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index c1bd897c9310..7445ab76e0b3 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -224,10 +224,11 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
>  					  FS_KEY_DERIVATION_NONCE_SIZE,
>  					  (u8 *)&ci->ci_hash_key,
>  					  sizeof(ci->ci_hash_key));
> -		if (!err)
> -			ci->ci_hash_key_initialized = true;
> +		if (err)
> +			return err;
> +		ci->ci_hash_key_initialized = true;
>  	}
> -	return err;
> +	return 0;
>  }

This part should be folded into patch 1.

>  
>  /*
> diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
> index f1cff83c151a..9e937cfa732c 100644
> --- a/fs/crypto/policy.c
> +++ b/fs/crypto/policy.c
> @@ -124,6 +124,12 @@ static bool fscrypt_supported_v1_policy(const struct fscrypt_policy_v1 *policy,
>  					policy->filenames_encryption_mode))
>  		return false;
>  
> +	if (IS_CASEFOLDED(inode)) {
> +		fscrypt_warn(inode,
> +			     "v1 policy does not support casefolded directories");
> +		return false;
> +	}
> +
>  	return true;
>  }
>  
> @@ -579,3 +585,36 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
>  	return preload ? fscrypt_get_encryption_info(child): 0;
>  }
>  EXPORT_SYMBOL(fscrypt_inherit_context);
> +
> +static int fscrypt_set_casefolding_allowed(struct inode *inode)
> +{
> +	union fscrypt_policy policy;
> +	int err = fscrypt_get_policy(inode, &policy);
> +
> +	if (err)
> +		return err;
> +
> +	if (policy.version != FSCRYPT_POLICY_V2)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +int fscrypt_ioc_setflags_prepare(struct inode *inode,
> +				 unsigned int oldflags,
> +				 unsigned int flags)
> +{
> +	int err;
> +
> +	/*
> +	 * When a directory is encrypted, the CASEFOLD flag can only be turned
> +	 * on if the fscrypt policy supports it.
> +	 */
> +	if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) {
> +		err = fscrypt_set_casefolding_allowed(inode);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}

There's not really any point to the fscrypt_set_casefolding_allowed() function.
It can just be folded into fscrypt_ioc_setflags_prepare():

int fscrypt_ioc_setflags_prepare(struct inode *inode,
				 unsigned int oldflags,
				 unsigned int flags)
{
	union fscrypt_policy policy;
	int err;

	/*
	 * When a directory is encrypted, the CASEFOLD flag can only be turned
	 * on if the fscrypt policy supports it.
	 */
	if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) {
		err = fscrypt_get_policy(inode, &policy);
		if (err)
			return err;
		if (policy.version != FSCRYPT_POLICY_V2)
			return -EINVAL;
	}

	return 0;
}

> @@ -2242,6 +2243,8 @@ EXPORT_SYMBOL(current_time);
>  int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags,
>  			     unsigned int flags)
>  {
> +	int err;
> +
>  	/*
>  	 * The IMMUTABLE and APPEND_ONLY flags can only be changed by
>  	 * the relevant capability.
> @@ -2252,6 +2255,10 @@ int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags,
>  	    !capable(CAP_LINUX_IMMUTABLE))
>  		return -EPERM;
>  
> +	err = fscrypt_ioc_setflags_prepare(inode, oldflags, flags);
> +	if (err)
> +		return err;
> +
>  	return 0;
>  }

Can just do 'return fscrypt_ioc_setflags_prepare(inode, oldflags, flags);'

- Eric

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] fscrypt: Add siphash and hash key for policy v2
  2020-01-07  2:33 ` [PATCH v2 1/3] fscrypt: Add siphash and hash key for policy v2 Daniel Rosenberg
@ 2020-01-07  4:02   ` Eric Biggers
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-01-07  4:02 UTC (permalink / raw)
  To: Daniel Rosenberg
  Cc: linux-fscrypt, linux-kernel, linux-fsdevel,
	Gabriel Krisman Bertazi, kernel-team

On Mon, Jan 06, 2020 at 06:33:21PM -0800, Daniel Rosenberg wrote:
> With encryption and casefolding, we cannot simply take the hash of the
> ciphertext because of case insensitivity, and we can't take the hash of
> the unencrypted name since that would leak information about the
> encrypted name. Instead we can use siphash to compute a keyed hash of
> the file names.
> 
> When a v2 policy is used on a directory, we derive a key for use with
> siphash.
> 
> Signed-off-by: Daniel Rosenberg <drosen@google.com>
> ---
>  fs/crypto/fname.c           | 22 ++++++++++++++++++++++
>  fs/crypto/fscrypt_private.h |  9 +++++++++
>  fs/crypto/keysetup.c        | 32 +++++++++++++++++++++++---------
>  include/linux/fscrypt.h     |  9 +++++++++
>  4 files changed, 63 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 3fd27e14ebdd..371e8f01d1c8 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -402,6 +402,28 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
>  }
>  EXPORT_SYMBOL(fscrypt_setup_filename);
>  
> +/**
> + * fscrypt_fname_siphash() - Calculate the siphash for a file name
> + * @dir: the parent directory
> + * @name: the name of the file to get the siphash of
> + *
> + * Given a user-provided filename @name, this function calculates the siphash of
> + * that name using the directory's hash key.
> + *
> + * This assumes the directory uses a v2 policy, and the key is available.
> + *
> + * Return: the siphash of @name using the hash key of @dir
> + */
> +u64 fscrypt_fname_siphash(const struct inode *dir, const struct qstr *name)
> +{
> +	struct fscrypt_info *ci = dir->i_crypt_info;
> +
> +	WARN_ON(!ci->ci_hash_key_initialized);
> +
> +	return siphash(name->name, name->len, &ci->ci_hash_key);
> +}
> +EXPORT_SYMBOL(fscrypt_fname_siphash);
> +
>  /*
>   * Validate dentries in encrypted directories to make sure we aren't potentially
>   * caching stale dentries after a key has been added.
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index b22e8decebed..8b37a5eebb57 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -12,6 +12,7 @@
>  #define _FSCRYPT_PRIVATE_H
>  
>  #include <linux/fscrypt.h>
> +#include <linux/siphash.h>
>  #include <crypto/hash.h>
>  
>  #define CONST_STRLEN(str)	(sizeof(str) - 1)
> @@ -188,6 +189,13 @@ struct fscrypt_info {
>  	 */
>  	struct fscrypt_direct_key *ci_direct_key;
>  
> +	/*
> +	 * With v2 policies, this can be used with siphash
> +	 * When the key has been set, ci_hash_key_initialized is set to true
> +	 */
> +	siphash_key_t ci_hash_key;
> +	bool ci_hash_key_initialized;
> +
>  	/* The encryption policy used by this inode */
>  	union fscrypt_policy ci_policy;
>  
> @@ -262,6 +270,7 @@ extern int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
>  #define HKDF_CONTEXT_PER_FILE_KEY	2
>  #define HKDF_CONTEXT_DIRECT_KEY		3
>  #define HKDF_CONTEXT_IV_INO_LBLK_64_KEY	4
> +#define HKDF_CONTEXT_FNAME_HASH_KEY     5
>  
>  extern int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
>  			       const u8 *info, unsigned int infolen,
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 96074054bdbc..c1bd897c9310 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -189,7 +189,7 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
>  		 * This ensures that the master key is consistently used only
>  		 * for HKDF, avoiding key reuse issues.
>  		 */
> -		return setup_per_mode_key(ci, mk, mk->mk_direct_tfms,
> +		err = setup_per_mode_key(ci, mk, mk->mk_direct_tfms,
>  					  HKDF_CONTEXT_DIRECT_KEY, false);
>  	} else if (ci->ci_policy.v2.flags &
>  		   FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64) {
> @@ -199,20 +199,34 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
>  		 * the IVs.  This format is optimized for use with inline
>  		 * encryption hardware compliant with the UFS or eMMC standards.
>  		 */
> -		return setup_per_mode_key(ci, mk, mk->mk_iv_ino_lblk_64_tfms,
> +		err = setup_per_mode_key(ci, mk, mk->mk_iv_ino_lblk_64_tfms,
>  					  HKDF_CONTEXT_IV_INO_LBLK_64_KEY,
>  					  true);
> +	} else {
> +		err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
> +					  HKDF_CONTEXT_PER_FILE_KEY,
> +					  ci->ci_nonce,
> +					  FS_KEY_DERIVATION_NONCE_SIZE,
> +					  derived_key, ci->ci_mode->keysize);
> +		if (err)
> +			return err;
> +
> +		err = fscrypt_set_derived_key(ci, derived_key);
> +		memzero_explicit(derived_key, ci->ci_mode->keysize);
>  	}
> -
> -	err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
> -				  HKDF_CONTEXT_PER_FILE_KEY,
> -				  ci->ci_nonce, FS_KEY_DERIVATION_NONCE_SIZE,
> -				  derived_key, ci->ci_mode->keysize);
>  	if (err)
>  		return err;
>  
> -	err = fscrypt_set_derived_key(ci, derived_key);
> -	memzero_explicit(derived_key, ci->ci_mode->keysize);
> +	if (S_ISDIR(ci->ci_inode->i_mode)) {
> +		err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
> +					  HKDF_CONTEXT_FNAME_HASH_KEY,
> +					  ci->ci_nonce,
> +					  FS_KEY_DERIVATION_NONCE_SIZE,
> +					  (u8 *)&ci->ci_hash_key,
> +					  sizeof(ci->ci_hash_key));
> +		if (!err)
> +			ci->ci_hash_key_initialized = true;
> +	}
>  	return err;
>  }
>  

This is deriving a SipHash key for every directory, even ones which won't use it
at all.  Since deriving a SipHash key is just as expensive as deriving an
encryption key, this doubles the time needed to setup every directory's key(s)
-- or even much more than doubles it, in the case of per-mode encryption keys.

It's really important that we keep fscrypt overhead as low as possible.
How feasible would it be to only derive the SipHash key when needed?
I.e., check IS_CASEFOLDED() here, and derive the SipHash key if needed in
fscrypt_ioc_setflags_prepare()?  Is the problem that safely getting access to
the master key again is hard?

- Eric

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 3/3] fscrypt: Change format of no-key token
  2020-01-07  2:33 ` [PATCH v2 3/3] fscrypt: Change format of no-key token Daniel Rosenberg
@ 2020-01-08 22:07   ` Eric Biggers
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-01-08 22:07 UTC (permalink / raw)
  To: Daniel Rosenberg
  Cc: linux-fscrypt, linux-kernel, linux-fsdevel,
	Gabriel Krisman Bertazi, kernel-team

A few more nits:

On Mon, Jan 06, 2020 at 06:33:23PM -0800, Daniel Rosenberg wrote:
> +static int fscrypt_do_sha256(unsigned char *result,
> +	     const u8 *data, unsigned int data_len)

Use 'u8 *' instead of 'unsigned char *', and then this fits on one line.

I'd probably also put 'result' last since it's an output parameter, and that
also matches the crypto interfaces.

> @@ -307,8 +372,7 @@ EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
>   * get the disk_name.
>   *
>   * Else, for keyless @lookup operations, @iname is the presented ciphertext, so
> - * we decode it to get either the ciphertext disk_name (for short names) or the
> - * fscrypt_digested_name (for long names).  Non-@lookup operations will be
> + * we decode it to get the fscrypt_nokey_name. Non-@lookup operations will be
>   * impossible in this case, so we fail them with ENOKEY.
>   *
>   * If successful, fscrypt_free_filename() must be called later to clean up.
> @@ -318,8 +382,8 @@ EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
>  int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
>  			      int lookup, struct fscrypt_name *fname)
>  {
> +	struct fscrypt_nokey_name *nokey_name;

This can be 'const'.

>  	int ret;
> -	int digested;
>  
>  	memset(fname, 0, sizeof(struct fscrypt_name));
>  	fname->usr_fname = iname;
> @@ -359,41 +423,29 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
>  	 * We don't have the key and we are doing a lookup; decode the
>  	 * user-supplied name
>  	 */
> -	if (iname->name[0] == '_') {
> -		if (iname->len !=
> -		    1 + BASE64_CHARS(sizeof(struct fscrypt_digested_name)))
> -			return -ENOENT;
> -		digested = 1;
> -	} else {
> -		if (iname->len >
> -		    BASE64_CHARS(FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE))
> -			return -ENOENT;
> -		digested = 0;
> -	}
>  
>  	fname->crypto_buf.name =
> -		kmalloc(max_t(size_t, FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE,
> -			      sizeof(struct fscrypt_digested_name)),
> -			GFP_KERNEL);
> +			kmalloc(sizeof(struct fscrypt_nokey_name), GFP_KERNEL);
>  	if (fname->crypto_buf.name == NULL)
>  		return -ENOMEM;
>  
> -	ret = base64_decode(iname->name + digested, iname->len - digested,
> -			    fname->crypto_buf.name);
> -	if (ret < 0) {
> +	if (iname->len > BASE64_CHARS(sizeof(struct fscrypt_nokey_name))) {
>  		ret = -ENOENT;
>  		goto errout;
>  	}
> -	fname->crypto_buf.len = ret;
> -	if (digested) {
> -		const struct fscrypt_digested_name *n =
> -			(const void *)fname->crypto_buf.name;
> -		fname->hash = n->hash;
> -		fname->minor_hash = n->minor_hash;
> -	} else {
> -		fname->disk_name.name = fname->crypto_buf.name;
> -		fname->disk_name.len = fname->crypto_buf.len;
> +	ret = base64_decode(iname->name, iname->len, fname->crypto_buf.name);
> +	if ((int)ret < offsetof(struct fscrypt_nokey_name, bytes[1]) ||
> +	    (ret > offsetof(struct fscrypt_nokey_name, sha256) &&
> +	     ret != offsetofend(struct fscrypt_nokey_name, sha256))) {
> +		ret = -ENOENT;
> +		goto errout;
>  	}
> +
> +	nokey_name = (void *)fname->crypto_buf.name;
> +	fname->crypto_buf.len = ret;
> +
> +	fname->hash = nokey_name->dirtree_hash[0];
> +	fname->minor_hash = nokey_name->dirtree_hash[1];
>  	return 0;
>  
>  errout:
> @@ -402,6 +454,62 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
>  }
>  EXPORT_SYMBOL(fscrypt_setup_filename);
>  
> +/**
> + * fscrypt_match_name() - test whether the given name matches a directory entry
> + * @fname: the name being searched for
> + * @de_name: the name from the directory entry
> + * @de_name_len: the length of @de_name in bytes
> + *
> + * Normally @fname->disk_name will be set, and in that case we simply compare
> + * that to the name stored in the directory entry.  The only exception is that
> + * if we don't have the key for an encrypted directory we'll instead need to
> + * match against the fscrypt_nokey_name.
> + *
> + * Return: %true if the name matches, otherwise %false.
> + */
> +bool fscrypt_match_name(const struct fscrypt_name *fname,
> +				      const u8 *de_name, u32 de_name_len)

Align the continuation line:

bool fscrypt_match_name(const struct fscrypt_name *fname,
                        const u8 *de_name, u32 de_name_len)

> +	if (unlikely(!fname->disk_name.name)) {
> +		const struct fscrypt_nokey_name *n =
> +			(const void *)fname->crypto_buf.name;
> +
> +		if (fname->crypto_buf.len ==
> +			    offsetofend(struct fscrypt_nokey_name, sha256)) {
> +			u8 sha256[SHA256_DIGEST_SIZE];
> +
> +			if (de_name_len <= FSCRYPT_FNAME_UNDIGESTED_SIZE)
> +				return false;
> +			if (memcmp(de_name, n->bytes,
> +				   FSCRYPT_FNAME_UNDIGESTED_SIZE) != 0)
> +				return false;
> +			fscrypt_do_sha256(sha256,
> +				&de_name[FSCRYPT_FNAME_UNDIGESTED_SIZE],
> +				de_name_len - FSCRYPT_FNAME_UNDIGESTED_SIZE);
> +			if (memcmp(sha256, n->sha256, sizeof(sha256)) != 0)
> +				return false;

Should check the return value of fscrypt_do_sha256().  I guess for now just
return false if it fails.  It would be nice if the sha256 tfm were preallocated
when the directory was opened, or alternatively the sha256 library interface
were used, so that this couldn't fail.  But just returning false should be fine
for now...

> +			u32 len = fname->crypto_buf.len -
> +				offsetof(struct fscrypt_nokey_name, bytes);
> +
> +			if (de_name_len != len)
> +				return false;
> +
> +			if (memcmp(de_name, n->bytes, len) != 0)
> +				return false;
> +		}
> +
> +		return true;
> +	}
> +
> +	if (de_name_len != fname->disk_name.len)
> +		return false;
> +	return !memcmp(de_name, fname->disk_name.name, fname->disk_name.len);
> +}
> +EXPORT_SYMBOL(fscrypt_match_name);
> +
>  /**
>   * fscrypt_fname_siphash() - Calculate the siphash for a file name
>   * @dir: the parent directory
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 2c292f19c6b9..14a727759a81 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -179,79 +179,8 @@ extern int fscrypt_fname_disk_to_usr(const struct inode *inode,
>  extern u64 fscrypt_fname_siphash(const struct inode *dir,
>  				 const struct qstr *name);
>  
> -#define FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE	32
> -
> -/* Extracts the second-to-last ciphertext block; see explanation below */
> -#define FSCRYPT_FNAME_DIGEST(name, len)	\
> -	((name) + round_down((len) - FS_CRYPTO_BLOCK_SIZE - 1, \
> -			     FS_CRYPTO_BLOCK_SIZE))
> -
> -#define FSCRYPT_FNAME_DIGEST_SIZE	FS_CRYPTO_BLOCK_SIZE
> -
> -/**
> - * fscrypt_digested_name - alternate identifier for an on-disk filename
> - *
> - * When userspace lists an encrypted directory without access to the key,
> - * filenames whose ciphertext is longer than FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE
> - * bytes are shown in this abbreviated form (base64-encoded) rather than as the
> - * full ciphertext (base64-encoded).  This is necessary to allow supporting
> - * filenames up to NAME_MAX bytes, since base64 encoding expands the length.
> - *
> - * To make it possible for filesystems to still find the correct directory entry
> - * despite not knowing the full on-disk name, we encode any filesystem-specific
> - * 'hash' and/or 'minor_hash' which the filesystem may need for its lookups,
> - * followed by the second-to-last ciphertext block of the filename.  Due to the
> - * use of the CBC-CTS encryption mode, the second-to-last ciphertext block
> - * depends on the full plaintext.  (Note that ciphertext stealing causes the
> - * last two blocks to appear "flipped".)  This makes accidental collisions very
> - * unlikely: just a 1 in 2^128 chance for two filenames to collide even if they
> - * share the same filesystem-specific hashes.
> - *
> - * However, this scheme isn't immune to intentional collisions, which can be
> - * created by anyone able to create arbitrary plaintext filenames and view them
> - * without the key.  Making the "digest" be a real cryptographic hash like
> - * SHA-256 over the full ciphertext would prevent this, although it would be
> - * less efficient and harder to implement, especially since the filesystem would
> - * need to calculate it for each directory entry examined during a search.
> - */
> -struct fscrypt_digested_name {
> -	u32 hash;
> -	u32 minor_hash;
> -	u8 digest[FSCRYPT_FNAME_DIGEST_SIZE];
> -};
> -
> -/**
> - * fscrypt_match_name() - test whether the given name matches a directory entry
> - * @fname: the name being searched for
> - * @de_name: the name from the directory entry
> - * @de_name_len: the length of @de_name in bytes
> - *
> - * Normally @fname->disk_name will be set, and in that case we simply compare
> - * that to the name stored in the directory entry.  The only exception is that
> - * if we don't have the key for an encrypted directory and a filename in it is
> - * very long, then we won't have the full disk_name and we'll instead need to
> - * match against the fscrypt_digested_name.
> - *
> - * Return: %true if the name matches, otherwise %false.
> - */
> -static inline bool fscrypt_match_name(const struct fscrypt_name *fname,
> -				      const u8 *de_name, u32 de_name_len)
> -{
> -	if (unlikely(!fname->disk_name.name)) {
> -		const struct fscrypt_digested_name *n =
> -			(const void *)fname->crypto_buf.name;
> -		if (WARN_ON_ONCE(fname->usr_fname->name[0] != '_'))
> -			return false;
> -		if (de_name_len <= FSCRYPT_FNAME_MAX_UNDIGESTED_SIZE)
> -			return false;
> -		return !memcmp(FSCRYPT_FNAME_DIGEST(de_name, de_name_len),
> -			       n->digest, FSCRYPT_FNAME_DIGEST_SIZE);
> -	}
> -
> -	if (de_name_len != fname->disk_name.len)
> -		return false;
> -	return !memcmp(de_name, fname->disk_name.name, fname->disk_name.len);
> -}
> +extern bool fscrypt_match_name(const struct fscrypt_name *fname,
> +				      const u8 *de_name, u32 de_name_len);

Align the continuation line:

extern bool fscrypt_match_name(const struct fscrypt_name *fname,
                               const u8 *de_name, u32 de_name_len);

Also, this should be moved above fscrypt_fname_siphash() in order to match their
order in the .c file.

- Eric

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-01-08 22:07 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-07  2:33 [PATCH v2 0/3] Fscrypt support for casefolded encryption Daniel Rosenberg
2020-01-07  2:33 ` [PATCH v2 1/3] fscrypt: Add siphash and hash key for policy v2 Daniel Rosenberg
2020-01-07  4:02   ` Eric Biggers
2020-01-07  2:33 ` [PATCH v2 2/3] fscrypt: Don't allow v1 policies with casefolding Daniel Rosenberg
2020-01-07  3:35   ` Eric Biggers
2020-01-07  2:33 ` [PATCH v2 3/3] fscrypt: Change format of no-key token Daniel Rosenberg
2020-01-08 22:07   ` Eric Biggers
2020-01-07  3:26 ` [PATCH v2 0/3] Fscrypt support for casefolded encryption Eric Biggers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).