Takashi Iwai 9fdfde
From 40cfb8e68aa073f03c25bc33022a8dbb29fdc777 Mon Sep 17 00:00:00 2001
Takashi Iwai 9fdfde
From: Eric Biggers <ebiggers@google.com>
Takashi Iwai 9fdfde
Date: Mon, 20 Sep 2021 20:03:03 -0700
Takashi Iwai 9fdfde
Subject: [PATCH] fscrypt: allow 256-bit master keys with AES-256-XTS
Takashi Iwai 9fdfde
Git-commit: 7f595d6a6cdc336834552069a2e0a4f6d4756ddf
Takashi Iwai 9fdfde
Patch-mainline: v5.16-rc1
Takashi Iwai 9fdfde
References: stable-5.14.19
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
[ Upstream commit 7f595d6a6cdc336834552069a2e0a4f6d4756ddf ]
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
fscrypt currently requires a 512-bit master key when AES-256-XTS is
Takashi Iwai 9fdfde
used, since AES-256-XTS keys are 512-bit and fscrypt requires that the
Takashi Iwai 9fdfde
master key be at least as long any key that will be derived from it.
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
However, this is overly strict because AES-256-XTS doesn't actually have
Takashi Iwai 9fdfde
a 512-bit security strength, but rather 256-bit.  The fact that XTS
Takashi Iwai 9fdfde
takes twice the expected key size is a quirk of the XTS mode.  It is
Takashi Iwai 9fdfde
sufficient to use 256 bits of entropy for AES-256-XTS, provided that it
Takashi Iwai 9fdfde
is first properly expanded into a 512-bit key, which HKDF-SHA512 does.
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
Therefore, relax the check of the master key size to use the security
Takashi Iwai 9fdfde
strength of the derived key rather than the size of the derived key
Takashi Iwai 9fdfde
(except for v1 encryption policies, which don't use HKDF).
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
Besides making things more flexible for userspace, this is needed in
Takashi Iwai 9fdfde
order for the use of a KDF which only takes a 256-bit key to be
Takashi Iwai 9fdfde
introduced into the fscrypt key hierarchy.  This will happen with
Takashi Iwai 9fdfde
hardware-wrapped keys support, as all known hardware which supports that
Takashi Iwai 9fdfde
feature uses an SP800-108 KDF using AES-256-CMAC, so the wrapped keys
Takashi Iwai 9fdfde
are wrapped 256-bit AES keys.  Moreover, there is interest in fscrypt
Takashi Iwai 9fdfde
supporting the same type of AES-256-CMAC based KDF in software as an
Takashi Iwai 9fdfde
alternative to HKDF-SHA512.  There is no security problem with such
Takashi Iwai 9fdfde
features, so fix the key length check to work properly with them.
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
Reviewed-by: Paul Crowley <paulcrowley@google.com>
Takashi Iwai 9fdfde
Link: https://lore.kernel.org/r/20210921030303.5598-1-ebiggers@kernel.org
Takashi Iwai 9fdfde
Signed-off-by: Eric Biggers <ebiggers@google.com>
Takashi Iwai 9fdfde
Signed-off-by: Sasha Levin <sashal@kernel.org>
Takashi Iwai 9fdfde
Acked-by: Takashi Iwai <tiwai@suse.de>
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
---
Takashi Iwai 9fdfde
 Documentation/filesystems/fscrypt.rst | 10 ++---
Takashi Iwai 9fdfde
 fs/crypto/fscrypt_private.h           |  5 ++-
Takashi Iwai 9fdfde
 fs/crypto/hkdf.c                      | 11 ++++--
Takashi Iwai 9fdfde
 fs/crypto/keysetup.c                  | 57 +++++++++++++++++++++------
Takashi Iwai 9fdfde
 4 files changed, 61 insertions(+), 22 deletions(-)
Takashi Iwai 9fdfde
Takashi Iwai 9fdfde
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
Takashi Iwai 9fdfde
index 44b67ebd6e40..936fae06db77 100644
Takashi Iwai 9fdfde
--- a/Documentation/filesystems/fscrypt.rst
Takashi Iwai 9fdfde
+++ b/Documentation/filesystems/fscrypt.rst
Takashi Iwai 9fdfde
@@ -176,11 +176,11 @@ Master Keys
Takashi Iwai 9fdfde
 
Takashi Iwai 9fdfde
 Each encrypted directory tree is protected by a *master key*.  Master
Takashi Iwai 9fdfde
 keys can be up to 64 bytes long, and must be at least as long as the
Takashi Iwai 9fdfde
-greater of the key length needed by the contents and filenames
Takashi Iwai 9fdfde
-encryption modes being used.  For example, if AES-256-XTS is used for
Takashi Iwai 9fdfde
-contents encryption, the master key must be 64 bytes (512 bits).  Note
Takashi Iwai 9fdfde
-that the XTS mode is defined to require a key twice as long as that
Takashi Iwai 9fdfde
-required by the underlying block cipher.
Takashi Iwai 9fdfde
+greater of the security strength of the contents and filenames
Takashi Iwai 9fdfde
+encryption modes being used.  For example, if any AES-256 mode is
Takashi Iwai 9fdfde
+used, the master key must be at least 256 bits, i.e. 32 bytes.  A
Takashi Iwai 9fdfde
+stricter requirement applies if the key is used by a v1 encryption
Takashi Iwai 9fdfde
+policy and AES-256-XTS is used; such keys must be 64 bytes.
Takashi Iwai 9fdfde
 
Takashi Iwai 9fdfde
 To "unlock" an encrypted directory tree, userspace must provide the
Takashi Iwai 9fdfde
 appropriate master key.  There can be any number of master keys, each
Takashi Iwai 9fdfde
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
Takashi Iwai 9fdfde
index 3fa965eb3336..cb25ef0cdf1f 100644
Takashi Iwai 9fdfde
--- a/fs/crypto/fscrypt_private.h
Takashi Iwai 9fdfde
+++ b/fs/crypto/fscrypt_private.h
Takashi Iwai 9fdfde
@@ -549,8 +549,9 @@ int __init fscrypt_init_keyring(void);
Takashi Iwai 9fdfde
 struct fscrypt_mode {
Takashi Iwai 9fdfde
 	const char *friendly_name;
Takashi Iwai 9fdfde
 	const char *cipher_str;
Takashi Iwai 9fdfde
-	int keysize;
Takashi Iwai 9fdfde
-	int ivsize;
Takashi Iwai 9fdfde
+	int keysize;		/* key size in bytes */
Takashi Iwai 9fdfde
+	int security_strength;	/* security strength in bytes */
Takashi Iwai 9fdfde
+	int ivsize;		/* IV size in bytes */
Takashi Iwai 9fdfde
 	int logged_impl_name;
Takashi Iwai 9fdfde
 	enum blk_crypto_mode_num blk_crypto_mode;
Takashi Iwai 9fdfde
 };
Takashi Iwai 9fdfde
diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c
Takashi Iwai 9fdfde
index e0ec21055505..7607d18b35fc 100644
Takashi Iwai 9fdfde
--- a/fs/crypto/hkdf.c
Takashi Iwai 9fdfde
+++ b/fs/crypto/hkdf.c
Takashi Iwai 9fdfde
@@ -16,9 +16,14 @@
Takashi Iwai 9fdfde
 
Takashi Iwai 9fdfde
 /*
Takashi Iwai 9fdfde
  * HKDF supports any unkeyed cryptographic hash algorithm, but fscrypt uses
Takashi Iwai 9fdfde
- * SHA-512 because it is reasonably secure and efficient; and since it produces
Takashi Iwai 9fdfde
- * a 64-byte digest, deriving an AES-256-XTS key preserves all 64 bytes of
Takashi Iwai 9fdfde
- * entropy from the master key and requires only one iteration of HKDF-Expand.
Takashi Iwai 9fdfde
+ * SHA-512 because it is well-established, secure, and reasonably efficient.
Takashi Iwai 9fdfde
+ *
Takashi Iwai 9fdfde
+ * HKDF-SHA256 was also considered, as its 256-bit security strength would be
Takashi Iwai 9fdfde
+ * sufficient here.  A 512-bit security strength is "nice to have", though.
Takashi Iwai 9fdfde
+ * Also, on 64-bit CPUs, SHA-512 is usually just as fast as SHA-256.  In the
Takashi Iwai 9fdfde
+ * common case of deriving an AES-256-XTS key (512 bits), that can result in
Takashi Iwai 9fdfde
+ * HKDF-SHA512 being much faster than HKDF-SHA256, as the longer digest size of
Takashi Iwai 9fdfde
+ * SHA-512 causes HKDF-Expand to only need to do one iteration rather than two.
Takashi Iwai 9fdfde
  */
Takashi Iwai 9fdfde
 #define HKDF_HMAC_ALG		"hmac(sha512)"
Takashi Iwai 9fdfde
 #define HKDF_HASHLEN		SHA512_DIGEST_SIZE
Takashi Iwai 9fdfde
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
Takashi Iwai 9fdfde
index bca9c6658a7c..89cd533a88bf 100644
Takashi Iwai 9fdfde
--- a/fs/crypto/keysetup.c
Takashi Iwai 9fdfde
+++ b/fs/crypto/keysetup.c
Takashi Iwai 9fdfde
@@ -19,6 +19,7 @@ struct fscrypt_mode fscrypt_modes[] = {
Takashi Iwai 9fdfde
 		.friendly_name = "AES-256-XTS",
Takashi Iwai 9fdfde
 		.cipher_str = "xts(aes)",
Takashi Iwai 9fdfde
 		.keysize = 64,
Takashi Iwai 9fdfde
+		.security_strength = 32,
Takashi Iwai 9fdfde
 		.ivsize = 16,
Takashi Iwai 9fdfde
 		.blk_crypto_mode = BLK_ENCRYPTION_MODE_AES_256_XTS,
Takashi Iwai 9fdfde
 	},
Takashi Iwai 9fdfde
@@ -26,12 +27,14 @@ struct fscrypt_mode fscrypt_modes[] = {
Takashi Iwai 9fdfde
 		.friendly_name = "AES-256-CTS-CBC",
Takashi Iwai 9fdfde
 		.cipher_str = "cts(cbc(aes))",
Takashi Iwai 9fdfde
 		.keysize = 32,
Takashi Iwai 9fdfde
+		.security_strength = 32,
Takashi Iwai 9fdfde
 		.ivsize = 16,
Takashi Iwai 9fdfde
 	},
Takashi Iwai 9fdfde
 	[FSCRYPT_MODE_AES_128_CBC] = {
Takashi Iwai 9fdfde
 		.friendly_name = "AES-128-CBC-ESSIV",
Takashi Iwai 9fdfde
 		.cipher_str = "essiv(cbc(aes),sha256)",
Takashi Iwai 9fdfde
 		.keysize = 16,
Takashi Iwai 9fdfde
+		.security_strength = 16,
Takashi Iwai 9fdfde
 		.ivsize = 16,
Takashi Iwai 9fdfde
 		.blk_crypto_mode = BLK_ENCRYPTION_MODE_AES_128_CBC_ESSIV,
Takashi Iwai 9fdfde
 	},
Takashi Iwai 9fdfde
@@ -39,12 +42,14 @@ struct fscrypt_mode fscrypt_modes[] = {
Takashi Iwai 9fdfde
 		.friendly_name = "AES-128-CTS-CBC",
Takashi Iwai 9fdfde
 		.cipher_str = "cts(cbc(aes))",
Takashi Iwai 9fdfde
 		.keysize = 16,
Takashi Iwai 9fdfde
+		.security_strength = 16,
Takashi Iwai 9fdfde
 		.ivsize = 16,
Takashi Iwai 9fdfde
 	},
Takashi Iwai 9fdfde
 	[FSCRYPT_MODE_ADIANTUM] = {
Takashi Iwai 9fdfde
 		.friendly_name = "Adiantum",
Takashi Iwai 9fdfde
 		.cipher_str = "adiantum(xchacha12,aes)",
Takashi Iwai 9fdfde
 		.keysize = 32,
Takashi Iwai 9fdfde
+		.security_strength = 32,
Takashi Iwai 9fdfde
 		.ivsize = 32,
Takashi Iwai 9fdfde
 		.blk_crypto_mode = BLK_ENCRYPTION_MODE_ADIANTUM,
Takashi Iwai 9fdfde
 	},
Takashi Iwai 9fdfde
@@ -357,6 +362,45 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
Takashi Iwai 9fdfde
 	return 0;
Takashi Iwai 9fdfde
 }
Takashi Iwai 9fdfde
 
Takashi Iwai 9fdfde
+/*
Takashi Iwai 9fdfde
+ * Check whether the size of the given master key (@mk) is appropriate for the
Takashi Iwai 9fdfde
+ * encryption settings which a particular file will use (@ci).
Takashi Iwai 9fdfde
+ *
Takashi Iwai 9fdfde
+ * If the file uses a v1 encryption policy, then the master key must be at least
Takashi Iwai 9fdfde
+ * as long as the derived key, as this is a requirement of the v1 KDF.
Takashi Iwai 9fdfde
+ *
Takashi Iwai 9fdfde
+ * Otherwise, the KDF can accept any size key, so we enforce a slightly looser
Takashi Iwai 9fdfde
+ * requirement: we require that the size of the master key be at least the
Takashi Iwai 9fdfde
+ * maximum security strength of any algorithm whose key will be derived from it
Takashi Iwai 9fdfde
+ * (but in practice we only need to consider @ci->ci_mode, since any other
Takashi Iwai 9fdfde
+ * possible subkeys such as DIRHASH and INODE_HASH will never increase the
Takashi Iwai 9fdfde
+ * required key size over @ci->ci_mode).  This allows AES-256-XTS keys to be
Takashi Iwai 9fdfde
+ * derived from a 256-bit master key, which is cryptographically sufficient,
Takashi Iwai 9fdfde
+ * rather than requiring a 512-bit master key which is unnecessarily long.  (We
Takashi Iwai 9fdfde
+ * still allow 512-bit master keys if the user chooses to use them, though.)
Takashi Iwai 9fdfde
+ */
Takashi Iwai 9fdfde
+static bool fscrypt_valid_master_key_size(const struct fscrypt_master_key *mk,
Takashi Iwai 9fdfde
+					  const struct fscrypt_info *ci)
Takashi Iwai 9fdfde
+{
Takashi Iwai 9fdfde
+	unsigned int min_keysize;
Takashi Iwai 9fdfde
+
Takashi Iwai 9fdfde
+	if (ci->ci_policy.version == FSCRYPT_POLICY_V1)
Takashi Iwai 9fdfde
+		min_keysize = ci->ci_mode->keysize;
Takashi Iwai 9fdfde
+	else
Takashi Iwai 9fdfde
+		min_keysize = ci->ci_mode->security_strength;
Takashi Iwai 9fdfde
+
Takashi Iwai 9fdfde
+	if (mk->mk_secret.size < min_keysize) {
Takashi Iwai 9fdfde
+		fscrypt_warn(NULL,
Takashi Iwai 9fdfde
+			     "key with %s %*phN is too short (got %u bytes, need %u+ bytes)",
Takashi Iwai 9fdfde
+			     master_key_spec_type(&mk->mk_spec),
Takashi Iwai 9fdfde
+			     master_key_spec_len(&mk->mk_spec),
Takashi Iwai 9fdfde
+			     (u8 *)&mk->mk_spec.u,
Takashi Iwai 9fdfde
+			     mk->mk_secret.size, min_keysize);
Takashi Iwai 9fdfde
+		return false;
Takashi Iwai 9fdfde
+	}
Takashi Iwai 9fdfde
+	return true;
Takashi Iwai 9fdfde
+}
Takashi Iwai 9fdfde
+
Takashi Iwai 9fdfde
 /*
Takashi Iwai 9fdfde
  * Find the master key, then set up the inode's actual encryption key.
Takashi Iwai 9fdfde
  *
Takashi Iwai 9fdfde
@@ -422,18 +466,7 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
Takashi Iwai 9fdfde
 		goto out_release_key;
Takashi Iwai 9fdfde
 	}
Takashi Iwai 9fdfde
 
Takashi Iwai 9fdfde
-	/*
Takashi Iwai 9fdfde
-	 * Require that the master key be at least as long as the derived key.
Takashi Iwai 9fdfde
-	 * Otherwise, the derived key cannot possibly contain as much entropy as
Takashi Iwai 9fdfde
-	 * that required by the encryption mode it will be used for.  For v1
Takashi Iwai 9fdfde
-	 * policies it's also required for the KDF to work at all.
Takashi Iwai 9fdfde
-	 */
Takashi Iwai 9fdfde
-	if (mk->mk_secret.size < ci->ci_mode->keysize) {
Takashi Iwai 9fdfde
-		fscrypt_warn(NULL,
Takashi Iwai 9fdfde
-			     "key with %s %*phN is too short (got %u bytes, need %u+ bytes)",
Takashi Iwai 9fdfde
-			     master_key_spec_type(&mk_spec),
Takashi Iwai 9fdfde
-			     master_key_spec_len(&mk_spec), (u8 *)&mk_spec.u,
Takashi Iwai 9fdfde
-			     mk->mk_secret.size, ci->ci_mode->keysize);
Takashi Iwai 9fdfde
+	if (!fscrypt_valid_master_key_size(mk, ci)) {
Takashi Iwai 9fdfde
 		err = -ENOKEY;
Takashi Iwai 9fdfde
 		goto out_release_key;
Takashi Iwai 9fdfde
 	}
Takashi Iwai 9fdfde
-- 
Takashi Iwai 9fdfde
2.26.2
Takashi Iwai 9fdfde