Linux-FSCrypt Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/5] xfstests: test f2fs compression+encryption
@ 2020-10-01  0:25 Eric Biggers
  2020-10-01  0:25 ` [PATCH 1/5] fscrypt-crypt-util: clean up parsing --block-size and --inode-number Eric Biggers
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Eric Biggers @ 2020-10-01  0:25 UTC (permalink / raw)
  To: fstests
  Cc: linux-fscrypt, linux-f2fs-devel, Jaegeuk Kim, Chao Yu, Daeho Jeong

Add a test which verifies that encryption is done correctly when a file
on f2fs uses both compression and encryption at the same time.

Patches 1-4 add prerequisites for the test, while patch 5 adds the
actual test.  Patch 2 also fixes a bug which could cause the existing
test generic/602 to fail in extremely rare cases.  See the commit
messages for details.

The new test passes both with and without the inlinecrypt mount option.
It requires CONFIG_F2FS_FS_COMPRESSION=y.

I'd appreciate the f2fs developers taking a look.

Note, there is a quirk where the IVs in compressed files are off by one
from the "natural" values.  It's still secure, though it made the test
slightly harder to write.  I'm not sure how intentional this quirk was.

Eric Biggers (5):
  fscrypt-crypt-util: clean up parsing --block-size and --inode-number
  fscrypt-crypt-util: fix IV incrementing for --iv-ino-lblk-32
  fscrypt-crypt-util: add --block-number option
  common/f2fs: add _require_scratch_f2fs_compression()
  f2fs: verify ciphertext of compressed+encrypted file

 common/config            |   1 +
 common/f2fs              |  27 +++++
 src/fscrypt-crypt-util.c |  98 ++++++++++++------
 tests/f2fs/002           | 217 +++++++++++++++++++++++++++++++++++++++
 tests/f2fs/002.out       |  21 ++++
 tests/f2fs/group         |   1 +
 6 files changed, 334 insertions(+), 31 deletions(-)
 create mode 100644 common/f2fs
 create mode 100755 tests/f2fs/002
 create mode 100644 tests/f2fs/002.out

-- 
2.28.0


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

* [PATCH 1/5] fscrypt-crypt-util: clean up parsing --block-size and --inode-number
  2020-10-01  0:25 [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
@ 2020-10-01  0:25 ` Eric Biggers
  2020-10-01  0:25 ` [PATCH 2/5] fscrypt-crypt-util: fix IV incrementing for --iv-ino-lblk-32 Eric Biggers
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-10-01  0:25 UTC (permalink / raw)
  To: fstests
  Cc: linux-fscrypt, linux-f2fs-devel, Jaegeuk Kim, Chao Yu, Daeho Jeong

From: Eric Biggers <ebiggers@google.com>

For --block-size, check for strtoul() reporting an overflow.

For --inode-number, check for strtoull() reporting an overflow.

Also, move the check for 32-bit inode numbers into a more logical place
(the place where we check the encryption format-specific limitations).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 src/fscrypt-crypt-util.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/fscrypt-crypt-util.c b/src/fscrypt-crypt-util.c
index ce9da85d..d9189346 100644
--- a/src/fscrypt-crypt-util.c
+++ b/src/fscrypt-crypt-util.c
@@ -26,6 +26,7 @@
 #include <linux/types.h>
 #include <stdarg.h>
 #include <stdbool.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -1756,18 +1757,6 @@ static u8 parse_mode_number(const char *arg)
 	return num;
 }
 
-static u32 parse_inode_number(const char *arg)
-{
-	char *tmp;
-	unsigned long long num = strtoull(arg, &tmp, 10);
-
-	if (num <= 0 || *tmp)
-		die("Invalid inode number: %s", arg);
-	if ((u32)num != num)
-		die("Inode number %s is too large; must be 32-bit", arg);
-	return num;
-}
-
 struct key_and_iv_params {
 	u8 master_key[MAX_KEY_SIZE];
 	int master_key_size;
@@ -1777,7 +1766,7 @@ struct key_and_iv_params {
 	bool file_nonce_specified;
 	bool iv_ino_lblk_64;
 	bool iv_ino_lblk_32;
-	u32 inode_number;
+	u64 inode_number;
 	u8 fs_uuid[UUID_SIZE];
 	bool fs_uuid_specified;
 };
@@ -1842,6 +1831,8 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 			die("%s requires --inode-number", opt);
 		if (params->mode_num == 0)
 			die("%s requires --mode-num", opt);
+		if (params->inode_number > UINT32_MAX)
+			die("%s can't use --inode-number > UINT32_MAX", opt);
 	}
 
 	switch (params->kdf) {
@@ -1957,8 +1948,9 @@ int main(int argc, char *argv[])
 	while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
 		switch (c) {
 		case OPT_BLOCK_SIZE:
+			errno = 0;
 			block_size = strtoul(optarg, &tmp, 10);
-			if (block_size <= 0 || *tmp)
+			if (block_size <= 0 || *tmp || errno)
 				die("Invalid block size: %s", optarg);
 			break;
 		case OPT_DECRYPT:
@@ -1980,7 +1972,10 @@ int main(int argc, char *argv[])
 			usage(stdout);
 			return 0;
 		case OPT_INODE_NUMBER:
-			params.inode_number = parse_inode_number(optarg);
+			errno = 0;
+			params.inode_number = strtoull(optarg, &tmp, 10);
+			if (params.inode_number <= 0 || *tmp || errno)
+				die("Invalid inode number: %s", optarg);
 			break;
 		case OPT_IV_INO_LBLK_32:
 			params.iv_ino_lblk_32 = true;
-- 
2.28.0


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

* [PATCH 2/5] fscrypt-crypt-util: fix IV incrementing for --iv-ino-lblk-32
  2020-10-01  0:25 [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
  2020-10-01  0:25 ` [PATCH 1/5] fscrypt-crypt-util: clean up parsing --block-size and --inode-number Eric Biggers
@ 2020-10-01  0:25 ` Eric Biggers
  2020-10-01  0:25 ` [PATCH 3/5] fscrypt-crypt-util: add --block-number option Eric Biggers
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-10-01  0:25 UTC (permalink / raw)
  To: fstests
  Cc: linux-fscrypt, linux-f2fs-devel, Jaegeuk Kim, Chao Yu, Daeho Jeong

From: Eric Biggers <ebiggers@google.com>

fscrypt-crypt-util treats the "block number" part of the IV as 64-bit
when incrementing it.  That's wrong for --iv-ino-lblk-32 and
--iv-ino-lblk-64, as in those cases the block number should be 32-bit.

Fix this by using the correct length for the block number.

For --iv-ino-lblk-64 this doesn't actually matter, since in that case
the block number starts at 0 and never exceeds UINT32_MAX.

But for --iv-ino-lblk-32, the hashed inode number gets added to the
original block number to produce the IV block number, which can later
wrap around from UINT32_MAX to 0.  As a result, this change fixes
generic/602, though since the wraparound case isn't specifically tested
currently, the chance of failure was extremely small.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 src/fscrypt-crypt-util.c | 54 ++++++++++++++++++++++++++++------------
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/src/fscrypt-crypt-util.c b/src/fscrypt-crypt-util.c
index d9189346..5c065116 100644
--- a/src/fscrypt-crypt-util.c
+++ b/src/fscrypt-crypt-util.c
@@ -1692,16 +1692,32 @@ static const struct fscrypt_cipher *find_fscrypt_cipher(const char *name)
 	return NULL;
 }
 
-struct fscrypt_iv {
-	union {
-		__le64 block_num;
-		u8 bytes[32];
+union fscrypt_iv {
+	/* usual IV format */
+	struct {
+		/* logical block number within the file */
+		__le64 block_number;
+
+		/* per-file nonce; only set in DIRECT_KEY mode */
+		u8 nonce[FILE_NONCE_SIZE];
+	};
+	/* IV format for IV_INO_LBLK_* modes */
+	struct {
+		/*
+		 * IV_INO_LBLK_64: logical block number within the file
+		 * IV_INO_LBLK_32: hashed inode number + logical block number
+		 *		   within the file, mod 2^32
+		 */
+		__le32 block_number32;
+
+		/* IV_INO_LBLK_64: inode number */
+		__le32 inode_number;
 	};
 };
 
 static void crypt_loop(const struct fscrypt_cipher *cipher, const u8 *key,
-		       struct fscrypt_iv *iv, bool decrypting,
-		       size_t block_size, size_t padding)
+		       union fscrypt_iv *iv, bool decrypting,
+		       size_t block_size, size_t padding, bool is_bnum_32bit)
 {
 	u8 *buf = xmalloc(block_size);
 	size_t res;
@@ -1718,13 +1734,18 @@ static void crypt_loop(const struct fscrypt_cipher *cipher, const u8 *key,
 		memset(&buf[res], 0, crypt_len - res);
 
 		if (decrypting)
-			cipher->decrypt(key, iv->bytes, buf, buf, crypt_len);
+			cipher->decrypt(key, (u8 *)iv, buf, buf, crypt_len);
 		else
-			cipher->encrypt(key, iv->bytes, buf, buf, crypt_len);
+			cipher->encrypt(key, (u8 *)iv, buf, buf, crypt_len);
 
 		full_write(STDOUT_FILENO, buf, crypt_len);
 
-		iv->block_num = cpu_to_le64(le64_to_cpu(iv->block_num) + 1);
+		if (is_bnum_32bit)
+			iv->block_number32 = cpu_to_le32(
+					le32_to_cpu(iv->block_number32) + 1);
+		else
+			iv->block_number = cpu_to_le64(
+					le64_to_cpu(iv->block_number) + 1);
 	}
 	free(buf);
 }
@@ -1806,7 +1827,7 @@ static u32 hash_inode_number(const struct key_and_iv_params *params)
  */
 static void get_key_and_iv(const struct key_and_iv_params *params,
 			   u8 *real_key, size_t real_key_size,
-			   struct fscrypt_iv *iv)
+			   union fscrypt_iv *iv)
 {
 	bool file_nonce_in_iv = false;
 	struct aes_key aes_key;
@@ -1860,14 +1881,14 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 			info[infolen++] = params->mode_num;
 			memcpy(&info[infolen], params->fs_uuid, UUID_SIZE);
 			infolen += UUID_SIZE;
-			put_unaligned_le32(params->inode_number, &iv->bytes[4]);
+			iv->inode_number = cpu_to_le32(params->inode_number);
 		} else if (params->iv_ino_lblk_32) {
 			info[infolen++] = HKDF_CONTEXT_IV_INO_LBLK_32_KEY;
 			info[infolen++] = params->mode_num;
 			memcpy(&info[infolen], params->fs_uuid, UUID_SIZE);
 			infolen += UUID_SIZE;
-			put_unaligned_le32(hash_inode_number(params),
-					   iv->bytes);
+			iv->block_number32 =
+				cpu_to_le32(hash_inode_number(params));
 		} else if (params->mode_num != 0) {
 			info[infolen++] = HKDF_CONTEXT_DIRECT_KEY;
 			info[infolen++] = params->mode_num;
@@ -1888,7 +1909,7 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 	}
 
 	if (file_nonce_in_iv && params->file_nonce_specified)
-		memcpy(&iv->bytes[8], params->file_nonce, FILE_NONCE_SIZE);
+		memcpy(iv->nonce, params->file_nonce, FILE_NONCE_SIZE);
 }
 
 enum {
@@ -1928,7 +1949,7 @@ int main(int argc, char *argv[])
 	size_t padding = 0;
 	const struct fscrypt_cipher *cipher;
 	u8 real_key[MAX_KEY_SIZE];
-	struct fscrypt_iv iv;
+	union fscrypt_iv iv;
 	char *tmp;
 	int c;
 
@@ -2025,6 +2046,7 @@ int main(int argc, char *argv[])
 
 	get_key_and_iv(&params, real_key, cipher->keysize, &iv);
 
-	crypt_loop(cipher, real_key, &iv, decrypting, block_size, padding);
+	crypt_loop(cipher, real_key, &iv, decrypting, block_size, padding,
+		   params.iv_ino_lblk_64 || params.iv_ino_lblk_32);
 	return 0;
 }
-- 
2.28.0


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

* [PATCH 3/5] fscrypt-crypt-util: add --block-number option
  2020-10-01  0:25 [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
  2020-10-01  0:25 ` [PATCH 1/5] fscrypt-crypt-util: clean up parsing --block-size and --inode-number Eric Biggers
  2020-10-01  0:25 ` [PATCH 2/5] fscrypt-crypt-util: fix IV incrementing for --iv-ino-lblk-32 Eric Biggers
@ 2020-10-01  0:25 ` Eric Biggers
  2020-10-01  0:25 ` [PATCH 4/5] common/f2fs: add _require_scratch_f2fs_compression() Eric Biggers
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-10-01  0:25 UTC (permalink / raw)
  To: fstests
  Cc: linux-fscrypt, linux-f2fs-devel, Jaegeuk Kim, Chao Yu, Daeho Jeong

From: Eric Biggers <ebiggers@google.com>

Currently fscrypt-crypt-util assumes that the number of the first block
encrypted/decrypted is 0.  I.e., it replicates either contents
encryption from the start of a file, or encryption of a filename.

However, to easily test compression+encryption on f2fs, we need the
ability to specify a different starting block number.

Add a --block-number option which does this.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 src/fscrypt-crypt-util.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/fscrypt-crypt-util.c b/src/fscrypt-crypt-util.c
index 5c065116..26698d7a 100644
--- a/src/fscrypt-crypt-util.c
+++ b/src/fscrypt-crypt-util.c
@@ -59,6 +59,8 @@ static void usage(FILE *fp)
 "WARNING: this program is only meant for testing, not for \"real\" use!\n"
 "\n"
 "Options:\n"
+"  --block-number=BNUM         Starting block number for IV generation.\n"
+"                                Default: 0\n"
 "  --block-size=BLOCK_SIZE     Encrypt each BLOCK_SIZE bytes independently.\n"
 "                                Default: 4096 bytes\n"
 "  --decrypt                   Decrypt instead of encrypt\n"
@@ -1787,6 +1789,7 @@ struct key_and_iv_params {
 	bool file_nonce_specified;
 	bool iv_ino_lblk_64;
 	bool iv_ino_lblk_32;
+	u64 block_number;
 	u64 inode_number;
 	u8 fs_uuid[UUID_SIZE];
 	bool fs_uuid_specified;
@@ -1839,6 +1842,9 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 
 	memset(iv, 0, sizeof(*iv));
 
+	/* Overridden later for iv_ino_lblk_{64,32} */
+	iv->block_number = cpu_to_le64(params->block_number);
+
 	if (params->iv_ino_lblk_64 || params->iv_ino_lblk_32) {
 		const char *opt = params->iv_ino_lblk_64 ? "--iv-ino-lblk-64" :
 							   "--iv-ino-lblk-32";
@@ -1852,6 +1858,8 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 			die("%s requires --inode-number", opt);
 		if (params->mode_num == 0)
 			die("%s requires --mode-num", opt);
+		if (params->block_number > UINT32_MAX)
+			die("%s can't use --block-number > UINT32_MAX", opt);
 		if (params->inode_number > UINT32_MAX)
 			die("%s can't use --inode-number > UINT32_MAX", opt);
 	}
@@ -1881,6 +1889,7 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 			info[infolen++] = params->mode_num;
 			memcpy(&info[infolen], params->fs_uuid, UUID_SIZE);
 			infolen += UUID_SIZE;
+			iv->block_number32 = cpu_to_le32(params->block_number);
 			iv->inode_number = cpu_to_le32(params->inode_number);
 		} else if (params->iv_ino_lblk_32) {
 			info[infolen++] = HKDF_CONTEXT_IV_INO_LBLK_32_KEY;
@@ -1888,7 +1897,9 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 			memcpy(&info[infolen], params->fs_uuid, UUID_SIZE);
 			infolen += UUID_SIZE;
 			iv->block_number32 =
-				cpu_to_le32(hash_inode_number(params));
+				cpu_to_le32(hash_inode_number(params) +
+					    params->block_number);
+			iv->inode_number = 0;
 		} else if (params->mode_num != 0) {
 			info[infolen++] = HKDF_CONTEXT_DIRECT_KEY;
 			info[infolen++] = params->mode_num;
@@ -1913,6 +1924,7 @@ static void get_key_and_iv(const struct key_and_iv_params *params,
 }
 
 enum {
+	OPT_BLOCK_NUMBER,
 	OPT_BLOCK_SIZE,
 	OPT_DECRYPT,
 	OPT_FILE_NONCE,
@@ -1927,6 +1939,7 @@ enum {
 };
 
 static const struct option longopts[] = {
+	{ "block-number",    required_argument, NULL, OPT_BLOCK_NUMBER },
 	{ "block-size",      required_argument, NULL, OPT_BLOCK_SIZE },
 	{ "decrypt",         no_argument,       NULL, OPT_DECRYPT },
 	{ "file-nonce",      required_argument, NULL, OPT_FILE_NONCE },
@@ -1968,6 +1981,12 @@ int main(int argc, char *argv[])
 
 	while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
 		switch (c) {
+		case OPT_BLOCK_NUMBER:
+			errno = 0;
+			params.block_number = strtoull(optarg, &tmp, 10);
+			if (*tmp || errno)
+				die("Invalid block number: %s", optarg);
+			break;
 		case OPT_BLOCK_SIZE:
 			errno = 0;
 			block_size = strtoul(optarg, &tmp, 10);
-- 
2.28.0


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

* [PATCH 4/5] common/f2fs: add _require_scratch_f2fs_compression()
  2020-10-01  0:25 [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
                   ` (2 preceding siblings ...)
  2020-10-01  0:25 ` [PATCH 3/5] fscrypt-crypt-util: add --block-number option Eric Biggers
@ 2020-10-01  0:25 ` Eric Biggers
  2020-10-01  0:25 ` [PATCH 5/5] f2fs: verify ciphertext of compressed+encrypted file Eric Biggers
  2020-10-07  3:48 ` [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
  5 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-10-01  0:25 UTC (permalink / raw)
  To: fstests
  Cc: linux-fscrypt, linux-f2fs-devel, Jaegeuk Kim, Chao Yu, Daeho Jeong

From: Eric Biggers <ebiggers@google.com>

Create the file common/f2fs, which will contain f2fs-specific utilities.

Then add a function _require_scratch_f2fs_compression(), which checks
for f2fs compression support on the scratch filesystem.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/f2fs | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 common/f2fs

diff --git a/common/f2fs b/common/f2fs
new file mode 100644
index 00000000..1b39d8ce
--- /dev/null
+++ b/common/f2fs
@@ -0,0 +1,27 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+# Copyright 2020 Google LLC
+
+# Require f2fs compression support on the scratch filesystem.
+# Optionally, check for support for a specific compression algorithm.
+_require_scratch_f2fs_compression()
+{
+	local algorithm=$1
+
+	_require_scratch
+
+	if [ ! -e /sys/fs/f2fs/features/compression ]; then
+		_notrun "Kernel doesn't support f2fs compression"
+	fi
+	# Note: '-O compression' is only accepted when used in
+	# combination with extra_attr.
+	if ! _scratch_mkfs -O compression,extra_attr >> $seqres.full; then
+		_notrun "f2fs-tools doesn't support compression"
+	fi
+	if [ -n "$algorithm" ]; then
+		if ! _scratch_mount "-o compress_algorithm=$algorithm"; then
+			_notrun "Kernel doesn't support $algorithm compression algorithm for f2fs"
+		fi
+		_scratch_unmount
+	fi
+}
-- 
2.28.0


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

* [PATCH 5/5] f2fs: verify ciphertext of compressed+encrypted file
  2020-10-01  0:25 [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
                   ` (3 preceding siblings ...)
  2020-10-01  0:25 ` [PATCH 4/5] common/f2fs: add _require_scratch_f2fs_compression() Eric Biggers
@ 2020-10-01  0:25 ` Eric Biggers
  2020-10-07  3:48 ` [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
  5 siblings, 0 replies; 8+ messages in thread
From: Eric Biggers @ 2020-10-01  0:25 UTC (permalink / raw)
  To: fstests
  Cc: linux-fscrypt, linux-f2fs-devel, Jaegeuk Kim, Chao Yu, Daeho Jeong

From: Eric Biggers <ebiggers@google.com>

In Linux v5.6, f2fs added support for per-file compression.  f2fs
compression can be used in combination with the existing f2fs encryption
support (a.k.a. fscrypt), in which case the compressed data is encrypted
rather than the uncompressed data.

We need to verify that the encryption is actually being done as expected
in this case.  So add a test which verifies it.

For now this is a f2fs-specific test.  It's possible that ext4 will
implement compression in the same way as f2fs (in which case this could
be made a generic test), but for now there are no plans for that.

This complements the existing ciphertext verification tests, e.g.
generic/548, which don't handle compression.  Encryption+compression has
some unique considerations, so it requires its own test.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/config      |   1 +
 tests/f2fs/002     | 217 +++++++++++++++++++++++++++++++++++++++++++++
 tests/f2fs/002.out |  21 +++++
 tests/f2fs/group   |   1 +
 4 files changed, 240 insertions(+)
 create mode 100755 tests/f2fs/002
 create mode 100644 tests/f2fs/002.out

diff --git a/common/config b/common/config
index 3de87ea7..285b7d1f 100644
--- a/common/config
+++ b/common/config
@@ -202,6 +202,7 @@ export GETRICHACL_PROG="$(type -P getrichacl)"
 export SETRICHACL_PROG="$(type -P setrichacl)"
 export KEYCTL_PROG="$(type -P keyctl)"
 export XZ_PROG="$(type -P xz)"
+export LZ4_PROG="$(type -P lz4)"
 export FLOCK_PROG="$(type -P flock)"
 export LDD_PROG="$(type -P ldd)"
 export TIMEOUT_PROG="$(type -P timeout)"
diff --git a/tests/f2fs/002 b/tests/f2fs/002
new file mode 100755
index 00000000..9e6cb102
--- /dev/null
+++ b/tests/f2fs/002
@@ -0,0 +1,217 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+# Copyright 2020 Google LLC
+#
+# FS QA Test No. f2fs/002
+#
+# Test that when a file is both compressed and encrypted, the encryption is done
+# correctly.  I.e., the correct ciphertext is written to disk.
+#
+# f2fs compression behaves as follows: the original data of a compressed file is
+# divided into equal-sized clusters.  The cluster size is configurable, but it
+# must be a power-of-2 multiple of the filesystem block size.  If the file size
+# isn't a multiple of the cluster size, then the final cluster is "partial" and
+# holds the remainder modulo the cluster size.  Each cluster is compressed
+# independently.  Each cluster is stored compressed if it isn't partial and
+# compression would save at least 1 block, otherwise it is stored uncompressed.
+#
+# If the file is also encrypted, then the data is encrypted after compression
+# (or decrypted before decompression).  In a compressed cluster, the block
+# numbers used in the IVs for encryption start at logical_block_number + 1 and
+# increment from there.  E.g. if the first three clusters each compressed 8
+# blocks to 6 blocks, then the IVs used will be 1..6, 9..14, 17..22.
+# In comparison, uncompressed clusters would use 0..7, 8..15, 16..23.
+#
+# This test verifies that the encryption is actually being done in the expected
+# way.  This is critical, since the f2fs filesystem implementation uses
+# significantly different code for I/O to/from compressed files, and bugs (say,
+# a bug that caused the encryption to be skipped) may not otherwise be detected.
+#
+# To do this test, we create a file that is both compressed and encrypted,
+# retrieve its raw data from disk, decrypt it, decompress it, and compare the
+# result to the original file.  We can't do it the other way around (compress
+# and encrypt the original data, and compare it to the on-disk data) because
+# compression can produce many different outputs from the same input.  E.g. the
+# lz4 command-line tool may not give the same output as the kernel's lz4
+# implementation, even though both outputs will decompress to the original data.
+#
+# f2fs supports multiple compression algorithms, but the choice of compression
+# algorithm shouldn't make a difference for the purpose of this test.  So we
+# just test LZ4.
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+. ./common/rc
+. ./common/filter
+. ./common/f2fs
+. ./common/encrypt
+
+rm -f $seqres.full
+
+_supported_fs f2fs
+
+# Prerequisites to create a file that is both encrypted and LZ4-compressed
+_require_scratch_encryption -v 2
+_require_scratch_f2fs_compression lz4
+_require_command "$CHATTR_PROG" chattr
+
+# Prerequisites to verify the ciphertext of the file
+_require_get_encryption_nonce_support
+_require_xfs_io_command "fiemap" # for _get_ciphertext_block_list()
+_require_test_program "fscrypt-crypt-util"
+_require_command "$LZ4_PROG" lz4
+
+# Test parameters
+compress_log_size=4
+num_compressible_clusters=5
+num_incompressible_clusters=2
+
+echo -e "\n# Creating filesystem that supports encryption and compression"
+_scratch_mkfs -O encrypt,compression,extra_attr >> $seqres.full
+_scratch_mount "-o compress_algorithm=lz4,compress_log_size=$compress_log_size"
+
+dir=$SCRATCH_MNT/dir
+file=$dir/file
+block_size=$(_get_block_size $SCRATCH_MNT)
+cluster_blocks=$((1 << compress_log_size))
+cluster_bytes=$((cluster_blocks * block_size))
+num_compressible_blocks=$((num_compressible_clusters * cluster_blocks))
+num_compressible_bytes=$((num_compressible_clusters * cluster_bytes))
+
+echo -e "\n# Creating directory"
+mkdir $dir
+
+echo -e "\n# Enabling encryption on the directory"
+_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" >> $seqres.full
+_set_encpolicy $dir $TEST_KEY_IDENTIFIER
+
+echo -e "\n# Enabling compression on the directory"
+$CHATTR_PROG +c $dir
+
+echo -e "\n# Creating compressed+encrypted file"
+for (( i = 0; i < num_compressible_clusters; i++ )); do
+	# Fill each compressible cluster with 2 blocks of zeroes, then the rest
+	# random data.  This should make the compression save 1 block.  (Not 2,
+	# due to overhead.)  We don't want the data to be *too* compressible,
+	# since we want to see the encryption IVs increment within each cluster.
+	head -c $(( 2 * block_size )) /dev/zero
+	head -c $(( (cluster_blocks - 2) * block_size )) /dev/urandom
+done > $tmp.orig_data
+# Also append some incompressible clusters, just in case there is some problem
+# that affects only uncompressed data in a compressed file.
+head -c $(( num_incompressible_clusters * cluster_bytes )) /dev/urandom \
+	>> $tmp.orig_data
+# Also append a compressible partial cluster at the end, just in case there is
+# some problem specific to partial clusters at EOF.  However, the current
+# behavior of f2fs compression is that partial clusters are never compressed.
+head -c $(( cluster_bytes - block_size )) /dev/zero >> $tmp.orig_data
+
+cp $tmp.orig_data $file
+inode=$(stat -c %i $file)
+
+# Get the list of blocks that contain the file's raw data.
+#
+# This is a hack, because the only API to get this information is fiemap, which
+# doesn't directly support compression as it assumes a 1:1 mapping between
+# logical blocks and physical blocks.
+#
+# But as we have no other option, we use fiemap anyway.  We rely on some f2fs
+# implementation details which make it work well enough in practice for the
+# purpose of this test:
+#
+#   - f2fs writes the blocks of each compressed cluster contiguously.
+#   - fiemap on a f2fs file gives an extent for each compressed cluster,
+#     with length equal to its uncompressed size.
+#
+# Note that for each compressed cluster, there will be some extra blocks
+# appended which aren't actually part of the file.  But it's simplest to just
+# read these anyway and ignore them when actually doing the decompression.
+blocklist=$(_get_ciphertext_block_list $file)
+
+_scratch_unmount
+
+echo -e "\n# Getting file's encryption nonce"
+nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
+
+echo -e "\n# Dumping the file's raw data"
+_dump_ciphertext_blocks $SCRATCH_DEV $blocklist > $tmp.raw
+
+echo -e "\n# Decrypting the file's data"
+TEST_RAW_KEY_HEX=$(echo "$TEST_RAW_KEY" | tr -d '\\x')
+decrypt_blocks()
+{
+	$here/src/fscrypt-crypt-util "$@"			\
+		--decrypt					\
+		--block-size=$block_size			\
+		--file-nonce=$nonce				\
+		--kdf=HKDF-SHA512				\
+		AES-256-XTS					\
+		$TEST_RAW_KEY_HEX
+}
+head -c $num_compressible_bytes $tmp.raw \
+	| decrypt_blocks --block-number=1 > $tmp.decrypted
+dd if=$tmp.raw bs=$cluster_bytes skip=$num_compressible_clusters status=none \
+	| decrypt_blocks --block-number=$num_compressible_blocks \
+	>> $tmp.decrypted
+
+# Decompress the compressed clusters using the lz4 command-line tool.
+#
+# Each f2fs compressed cluster begins with a 24-byte header, starting with the
+# compressed size in bytes (excluding the header) as a __le32.  The header is
+# followed by the actual compressed data; for LZ4, that means an LZ4 block.
+#
+# Unfortunately, the lz4 command-line tool only deals with LZ4 *frames*
+# (https://github.com/lz4/lz4/blob/master/doc/lz4_Frame_format.md) and can't
+# decompress LZ4 blocks directly.  So we have to extract the LZ4 block, then
+# wrap it with a minimal LZ4 frame.
+
+decompress_cluster()
+{
+	if (( $(stat -c %s "$1") < 24 )); then
+		_fail "Invalid compressed cluster (truncated)"
+	fi
+	compressed_size=$(od -td4 -N4 -An --endian=little $1 | awk '{print $1}')
+	if (( compressed_size <= 0 )); then
+		_fail "Invalid compressed cluster (bad compressed size)"
+	fi
+	(
+		echo -e -n '\x04\x22\x4d\x18' # LZ4 frame magic number
+		echo -e -n '\x40\x70\xdf'     # LZ4 frame descriptor
+		head -c 4 "$1"                # Compressed block size
+		dd if="$1" skip=24 iflag=skip_bytes bs=$compressed_size \
+			count=1 status=none
+		echo -e -n '\x00\x00\x00\x00' # Next block size (none)
+	) | $LZ4_PROG -d
+}
+
+echo -e "\n# Decompressing the file's data"
+for (( i = 0; i < num_compressible_clusters; i++ )); do
+	dd if=$tmp.decrypted bs=$cluster_bytes skip=$i count=1 status=none \
+		of=$tmp.cluster
+	decompress_cluster $tmp.cluster >> $tmp.uncompressed_data
+done
+# Append the incompressible clusters and the final partial cluster,
+# neither of which should have been compressed.
+dd if=$tmp.decrypted bs=$cluster_bytes skip=$num_compressible_clusters \
+	status=none >> $tmp.uncompressed_data
+
+# Finally do the actual test.  The data we got after decryption+decompression
+# should match the original file contents.
+echo -e "\n# Comparing to original data"
+cmp $tmp.uncompressed_data $tmp.orig_data
+
+status=0
+exit
diff --git a/tests/f2fs/002.out b/tests/f2fs/002.out
new file mode 100644
index 00000000..bc7bf9b3
--- /dev/null
+++ b/tests/f2fs/002.out
@@ -0,0 +1,21 @@
+QA output created by 002
+
+# Creating filesystem that supports encryption and compression
+
+# Creating directory
+
+# Enabling encryption on the directory
+
+# Enabling compression on the directory
+
+# Creating compressed+encrypted file
+
+# Getting file's encryption nonce
+
+# Dumping the file's raw data
+
+# Decrypting the file's data
+
+# Decompressing the file's data
+
+# Comparing to original data
diff --git a/tests/f2fs/group b/tests/f2fs/group
index daba9a30..7cd42fe4 100644
--- a/tests/f2fs/group
+++ b/tests/f2fs/group
@@ -4,3 +4,4 @@
 # - comment line before each group is "new" description
 #
 001 auto quick rw
+002 auto quick rw encrypt compress
-- 
2.28.0


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

* Re: [PATCH 0/5] xfstests: test f2fs compression+encryption
  2020-10-01  0:25 [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
                   ` (4 preceding siblings ...)
  2020-10-01  0:25 ` [PATCH 5/5] f2fs: verify ciphertext of compressed+encrypted file Eric Biggers
@ 2020-10-07  3:48 ` Eric Biggers
  2020-10-07  4:27   ` [f2fs-dev] " Daeho Jeong
  5 siblings, 1 reply; 8+ messages in thread
From: Eric Biggers @ 2020-10-07  3:48 UTC (permalink / raw)
  To: Jaegeuk Kim, Chao Yu, Daeho Jeong
  Cc: fstests, linux-fscrypt, linux-f2fs-devel

On Wed, Sep 30, 2020 at 05:25:02PM -0700, Eric Biggers wrote:
> Add a test which verifies that encryption is done correctly when a file
> on f2fs uses both compression and encryption at the same time.
> 
> Patches 1-4 add prerequisites for the test, while patch 5 adds the
> actual test.  Patch 2 also fixes a bug which could cause the existing
> test generic/602 to fail in extremely rare cases.  See the commit
> messages for details.
> 
> The new test passes both with and without the inlinecrypt mount option.
> It requires CONFIG_F2FS_FS_COMPRESSION=y.
> 
> I'd appreciate the f2fs developers taking a look.
> 
> Note, there is a quirk where the IVs in compressed files are off by one
> from the "natural" values.  It's still secure, though it made the test
> slightly harder to write.  I'm not sure how intentional this quirk was.
> 
> Eric Biggers (5):
>   fscrypt-crypt-util: clean up parsing --block-size and --inode-number
>   fscrypt-crypt-util: fix IV incrementing for --iv-ino-lblk-32
>   fscrypt-crypt-util: add --block-number option
>   common/f2fs: add _require_scratch_f2fs_compression()
>   f2fs: verify ciphertext of compressed+encrypted file

Jaegeuk, Chao, Daeho: any comments on this?

- Eric

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

* Re: [f2fs-dev] [PATCH 0/5] xfstests: test f2fs compression+encryption
  2020-10-07  3:48 ` [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
@ 2020-10-07  4:27   ` Daeho Jeong
  0 siblings, 0 replies; 8+ messages in thread
From: Daeho Jeong @ 2020-10-07  4:27 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Jaegeuk Kim, Chao Yu, Daeho Jeong, linux-fscrypt, fstests,
	linux-f2fs-devel

Sorry for the late reply. We had a long holiday last week.
The patch looks good to me~

Thanks,

2020년 10월 7일 (수) 오후 12:49, Eric Biggers <ebiggers@kernel.org>님이 작성:
>
> On Wed, Sep 30, 2020 at 05:25:02PM -0700, Eric Biggers wrote:
> > Add a test which verifies that encryption is done correctly when a file
> > on f2fs uses both compression and encryption at the same time.
> >
> > Patches 1-4 add prerequisites for the test, while patch 5 adds the
> > actual test.  Patch 2 also fixes a bug which could cause the existing
> > test generic/602 to fail in extremely rare cases.  See the commit
> > messages for details.
> >
> > The new test passes both with and without the inlinecrypt mount option.
> > It requires CONFIG_F2FS_FS_COMPRESSION=y.
> >
> > I'd appreciate the f2fs developers taking a look.
> >
> > Note, there is a quirk where the IVs in compressed files are off by one
> > from the "natural" values.  It's still secure, though it made the test
> > slightly harder to write.  I'm not sure how intentional this quirk was.
> >
> > Eric Biggers (5):
> >   fscrypt-crypt-util: clean up parsing --block-size and --inode-number
> >   fscrypt-crypt-util: fix IV incrementing for --iv-ino-lblk-32
> >   fscrypt-crypt-util: add --block-number option
> >   common/f2fs: add _require_scratch_f2fs_compression()
> >   f2fs: verify ciphertext of compressed+encrypted file
>
> Jaegeuk, Chao, Daeho: any comments on this?
>
> - Eric
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01  0:25 [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
2020-10-01  0:25 ` [PATCH 1/5] fscrypt-crypt-util: clean up parsing --block-size and --inode-number Eric Biggers
2020-10-01  0:25 ` [PATCH 2/5] fscrypt-crypt-util: fix IV incrementing for --iv-ino-lblk-32 Eric Biggers
2020-10-01  0:25 ` [PATCH 3/5] fscrypt-crypt-util: add --block-number option Eric Biggers
2020-10-01  0:25 ` [PATCH 4/5] common/f2fs: add _require_scratch_f2fs_compression() Eric Biggers
2020-10-01  0:25 ` [PATCH 5/5] f2fs: verify ciphertext of compressed+encrypted file Eric Biggers
2020-10-07  3:48 ` [PATCH 0/5] xfstests: test f2fs compression+encryption Eric Biggers
2020-10-07  4:27   ` [f2fs-dev] " Daeho Jeong

Linux-FSCrypt Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fscrypt/0 linux-fscrypt/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fscrypt linux-fscrypt/ https://lore.kernel.org/linux-fscrypt \
		linux-fscrypt@vger.kernel.org
	public-inbox-index linux-fscrypt

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fscrypt


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git