[RFC PATCH 0/3] xfstests: test adding filesystem-level fscrypt key via key_id

[RFC PATCH 0/3] xfstests: test adding filesystem-level fscrypt key via key_id

[Eric Biggers]

This series adds a test which tests adding a key to a filesystem's
fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
alternative to the normal method where the raw key is given directly.

I'm sending this out for comment, but it shouldn't be merged until the
corresponding kernel patch has reached mainline.  For more details, see
the kernel patch:
https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u

This test depends on an xfs_io patch which adds the '-k' option to the
'add_enckey' command, e.g.:

	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT

This test is skipped if the needed kernel or xfs_io support is absent.

This has been tested on ext4, f2fs, and ubifs.

To apply cleanly, my other xfstests patch series
"[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
must be applied first.

This series can also be retrieved from
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
tag "fscrypt-provisioning_2019-11-19".

Eric Biggers (3):
  common/rc: handle option with argument in _require_xfs_io_command()
  common/encrypt: move constant test key to common code
  generic: test adding filesystem-level fscrypt key via key_id

 common/encrypt        |  95 +++++++++++++++++++++----
 common/rc             |   2 +-
 tests/generic/580     |  17 ++---
 tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/806.out |  73 ++++++++++++++++++++
 tests/generic/group   |   1 +
 6 files changed, 316 insertions(+), 28 deletions(-)
 create mode 100644 tests/generic/806
 create mode 100644 tests/generic/806.out

-- 
2.24.0.432.g9d3f5f5b63-goog

[RFC PATCH 1/3] common/rc: handle option with argument in _require_xfs_io_command()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Fix _require_xfs_io_command() to handle options that take arguments when
the argument is shown in the help text.  E.g., it didn't work to run:

	_require_xfs_io_command "add_enckey" "-k"

because the relevant line of the help text is:

	-k KEY_ID -- ID of fscrypt-provisioning key containing the raw key

... but the grep command only matched "-k --", not "-k KEY_ID --".

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/rc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/rc b/common/rc
index b988e912..8d9479c0 100644
--- a/common/rc
+++ b/common/rc
@@ -2216,7 +2216,7 @@ _require_xfs_io_command()
 	[ -n "$param" ] || return
 
 	if [ -z "$param_checked" ]; then
-		$XFS_IO_PROG -c "help $command" | grep -q "^ $param --" || \
+		$XFS_IO_PROG -c "help $command" | grep -E -q "^ $param ([a-zA-Z_]+ )?--" || \
 			_notrun "xfs_io $command doesn't support $param"
 	else
 		# xfs_io could result in "command %c not supported" if it was
-- 
2.24.0.432.g9d3f5f5b63-goog

[RFC PATCH 2/3] common/encrypt: move constant test key to common code

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

For some encryption tests it's helpful to always use the same key so
that the test's output is always the same.

generic/580 already defines such a key, so move it into common/encrypt
so that other tests can use it too.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/encrypt    | 11 +++++++++++
 tests/generic/580 | 17 ++++-------------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/common/encrypt b/common/encrypt
index 2e9908ad..98a407ce 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -187,6 +187,17 @@ _scratch_mkfs_stable_inodes_encrypted()
 	esac
 }
 
+# For some tests it's helpful to always use the same key so that the test's
+# output is always the same.  For this purpose the following key can be used:
+TEST_RAW_KEY=
+for i in {1..64}; do
+	TEST_RAW_KEY+="\\x$(printf "%02x" $i)"
+done
+# Key descriptor: arbitrary value
+TEST_KEY_DESCRIPTOR="0000111122223333"
+# Key identifier: HKDF-SHA512(key=$TEST_RAW_KEY, salt="", info="fscrypt\0\x01")
+TEST_KEY_IDENTIFIER="69b2f6edeee720cce0577937eb8a6751"
+
 # Give the invoking shell a new session keyring.  This makes any keys we add to
 # the session keyring scoped to the lifetime of the test script.
 _new_session_keyring()
diff --git a/tests/generic/580 b/tests/generic/580
index d0b0e9b3..283d6efa 100755
--- a/tests/generic/580
+++ b/tests/generic/580
@@ -43,21 +43,12 @@ _scratch_mount
 test_with_policy_version()
 {
 	local vers=$1
-	local raw_key=""
-	local i
-
-	for i in {1..64}; do
-		raw_key+="\\x$(printf "%02x" $i)"
-	done
 
 	if (( vers == 1 )); then
-		# Key descriptor: arbitrary value
-		local keyspec="0000111122223333"
+		local keyspec=$TEST_KEY_DESCRIPTOR
 		local add_enckey_args="-d $keyspec"
 	else
-		# Key identifier:
-		# HKDF-SHA512(key=raw_key, salt="", info="fscrypt\0\x01")
-		local keyspec="69b2f6edeee720cce0577937eb8a6751"
+		local keyspec=$TEST_KEY_IDENTIFIER
 		local add_enckey_args=""
 	fi
 
@@ -75,7 +66,7 @@ test_with_policy_version()
 	echo "# Getting encryption key status"
 	_enckey_status $SCRATCH_MNT $keyspec
 	echo "# Adding encryption key"
-	_add_enckey $SCRATCH_MNT "$raw_key" $add_enckey_args
+	_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" $add_enckey_args
 	echo "# Creating encrypted file"
 	echo contents > $dir/file
 	echo "# Getting encryption key status"
@@ -90,7 +81,7 @@ test_with_policy_version()
 
 	# Test removing key with a file open.
 	echo "# Re-adding encryption key"
-	_add_enckey $SCRATCH_MNT "$raw_key" $add_enckey_args
+	_add_enckey $SCRATCH_MNT "$TEST_RAW_KEY" $add_enckey_args
 	echo "# Creating another encrypted file"
 	echo foo > $dir/file2
 	echo "# Removing key while an encrypted file is open"
-- 
2.24.0.432.g9d3f5f5b63-goog

[RFC PATCH 3/3] generic: test adding filesystem-level fscrypt key via key_id

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add a test which tests adding a key to a filesystem's fscrypt keyring
via an "fscrypt-provisioning" keyring key.  This is an alternative to
the normal method where the raw key is given directly.

For more details, see the corresponding kernel patch:
https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u

This test depends on an xfs_io patch which adds the '-k' option to the
'add_enckey' command, e.g.:

	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT

This test is skipped if the needed kernel or xfs_io support is absent.

This has been tested on ext4, f2fs, and ubifs.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/encrypt        |  84 +++++++++++++++++++----
 tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/806.out |  73 ++++++++++++++++++++
 tests/generic/group   |   1 +
 4 files changed, 300 insertions(+), 14 deletions(-)
 create mode 100644 tests/generic/806
 create mode 100644 tests/generic/806.out

diff --git a/common/encrypt b/common/encrypt
index 98a407ce..cc7712c2 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -227,6 +227,28 @@ _generate_raw_encryption_key()
 	echo $raw
 }
 
+# Serialize an integer into a CPU-endian bytestring of the given length, and
+# print it as a string where each byte is hex-escaped.  For example,
+# `_num_to_hex 1000 4` == "\xe8\x03\x00\x00" if the CPU is little endian.
+_num_to_hex()
+{
+	local value=$1
+	local nbytes=$2
+	local i
+	local big_endian=$(echo -ne '\x11' | od -tx2 | head -1 | \
+			   cut -f2 -d' ' | cut -c1)
+
+	if (( big_endian )); then
+		for (( i = 0; i < nbytes; i++ )); do
+			printf '\\x%02x' $(((value >> (8*(nbytes-1-i))) & 0xff))
+		done
+	else
+		for (( i = 0; i < nbytes; i++ )); do
+			printf '\\x%02x' $(((value >> (8*i)) & 0xff))
+		done
+	fi
+}
+
 # Add the specified raw encryption key to the session keyring, using the
 # specified key descriptor.
 _add_session_encryption_key()
@@ -237,12 +259,12 @@ _add_session_encryption_key()
 	#
 	# Add the key to the session keyring.  The required structure is:
 	#
-	#	#define FS_MAX_KEY_SIZE 64
+	#	#define FSCRYPT_MAX_KEY_SIZE 64
 	#	struct fscrypt_key {
-	#		u32 mode;
-	#		u8 raw[FS_MAX_KEY_SIZE];
-	#		u32 size;
-	#	} __packed;
+	#		__u32 mode;
+	#		__u8 raw[FSCRYPT_MAX_KEY_SIZE];
+	#		__u32 size;
+	#	};
 	#
 	# The kernel ignores 'mode' but requires that 'size' be 64.
 	#
@@ -253,15 +275,8 @@ _add_session_encryption_key()
 	# nice to use the common key prefix, but for now use the filesystem-
 	# specific prefix to make it possible to test older kernels...
 	#
-	local big_endian=$(echo -ne '\x11' | od -tx2 | head -1 | \
-			   cut -f2 -d' ' | cut -c1 )
-	if (( big_endian )); then
-		local mode='\x00\x00\x00\x00'
-		local size='\x00\x00\x00\x40'
-	else
-		local mode='\x00\x00\x00\x00'
-		local size='\x40\x00\x00\x00'
-	fi
+	local mode=$(_num_to_hex 0 4)
+	local size=$(_num_to_hex 64 4)
 	echo -n -e "${mode}${raw}${size}" |
 		$KEYCTL_PROG padd logon $FSTYP:$keydesc @s >>$seqres.full
 }
@@ -389,6 +404,44 @@ _user_do_enckey_status()
 	_user_do "$XFS_IO_PROG -c \"enckey_status $* $keyspec\" \"$mnt\""
 }
 
+# Require support for adding a key to a filesystem's fscrypt keyring via an
+# "fscrypt-provisioning" keyring key.
+_require_add_enckey_by_key_id()
+{
+	local mnt=$1
+
+	# Userspace support
+	_require_xfs_io_command "add_enckey" "-k"
+
+	# Kernel support
+	if $XFS_IO_PROG -c "$add_enckey -k 12345 $mnt" \
+		|& grep -q 'Invalid argument'; then
+		_notrun "Kernel doesn't support key_id parameter to FS_IOC_ADD_ENCRYPTION_KEY"
+	fi
+}
+
+# Add a key of type "fscrypt-provisioning" to the session keyring and print the
+# resulting key ID.
+_add_fscrypt_provisioning_key()
+{
+	local desc=$1
+	local type=$2
+	local raw=$3
+
+	# The format of the key payload must be:
+	#
+	#	struct fscrypt_provisioning_key_payload {
+	#		__u32 type;
+	#		__u32 __reserved;
+	#		__u8 raw[];
+	#	};
+	#
+	local type_hex=$(_num_to_hex $type 4)
+	local reserved=$(_num_to_hex 0 4)
+	echo -n -e "${type_hex}${reserved}${raw}" |
+		$KEYCTL_PROG padd fscrypt-provisioning "$desc" @s
+}
+
 # Retrieve the encryption nonce of the given inode as a hex string.  The nonce
 # was randomly generated by the filesystem and isn't exposed directly to
 # userspace.  But it can be read using the filesystem's debugging tools.
@@ -717,6 +770,9 @@ FSCRYPT_MODE_ADIANTUM=9
 FSCRYPT_POLICY_FLAG_DIRECT_KEY=0x04
 FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64=0x08
 
+FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR=1
+FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER=2
+
 _fscrypt_mode_name_to_num()
 {
 	local name=$1
diff --git a/tests/generic/806 b/tests/generic/806
new file mode 100644
index 00000000..29c09858
--- /dev/null
+++ b/tests/generic/806
@@ -0,0 +1,156 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2019 Google LLC
+#
+# FS QA Test generic/806
+#
+# Test adding a key to a filesystem's fscrypt keyring via an
+# "fscrypt-provisioning" keyring key.  This is an alternative to the normal
+# method where the raw key is given directly.
+#
+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.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/encrypt
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch_encryption -v 2
+_require_command "$KEYCTL_PROG" keyctl
+
+_new_session_keyring
+_scratch_mkfs_encrypted &>> $seqres.full
+_scratch_mount
+_require_add_enckey_by_key_id $SCRATCH_MNT
+
+test_with_policy_version()
+{
+	local vers=$1
+	local raw_key=$TEST_RAW_KEY
+	local dir=$SCRATCH_MNT/dir
+	local keyid
+
+	echo
+	echo "# =========================="
+	echo "# Test with policy version $vers"
+	echo "# =========================="
+
+	case $vers in
+	1)
+		local key_type=$FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR
+		local keyspec=$TEST_KEY_DESCRIPTOR
+		local add_enckey_args="-d $TEST_KEY_DESCRIPTOR"
+		;;
+	2)
+		local key_type=$FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER
+		local keyspec=$TEST_KEY_IDENTIFIER
+		local add_enckey_args=""
+		;;
+	*)
+		_fail "Unknown policy version: $vers"
+		;;
+	esac
+
+	# First add the key in the regular way (raw key given directly), create
+	# an encrypted file with some contents, and remove the key.  After this,
+	# the encrypted file should no longer be readable.
+
+	echo -e "\n# Adding key to filesystem"
+	_add_enckey $SCRATCH_MNT "$raw_key" $add_enckey_args
+
+	echo -e "\n# Creating encrypted file"
+	mkdir $dir
+	_set_encpolicy $dir $keyspec
+	echo "contents" > $dir/file
+
+	echo -e "\n# Removing key from filesystem"
+	_rm_enckey $SCRATCH_MNT $keyspec
+	cat $dir/file |& _filter_scratch
+
+	# Now we should be able to add the key back via an fscrypt-provisioning
+	# key which contains the raw key, instead of providing the raw key
+	# directly.  After this, the encrypted file should be readable again.
+
+	echo -e "\n# Adding fscrypt-provisioning key"
+	keyid=$(_add_fscrypt_provisioning_key $keyspec $key_type "$raw_key")
+
+	echo -e "\n# Adding key to filesystem via fscrypt-provisioning key"
+	$XFS_IO_PROG -c "add_enckey -k $keyid $add_enckey_args" $SCRATCH_MNT
+
+	echo -e "\n# Reading encrypted file"
+	cat $dir/file
+
+	echo -e "\n# Cleaning up"
+	rm -rf $dir
+	_scratch_cycle_mount	# Clear all keys
+}
+
+# Test with both v1 and v2 encryption policies.
+test_with_policy_version 1
+test_with_policy_version 2
+
+# Now test that invalid fscrypt-provisioning keys can't be created, that
+# fscrypt-provisioning keys can't be read back by userspace, and that the
+# filesystem only accepts properly matching fscrypt-provisioning keys.
+echo
+echo "# ================"
+echo "# Validation tests"
+echo "# ================"
+
+echo -e "\n# Adding an invalid fscrypt-provisioning key fails"
+echo "# ... bad type"
+_add_fscrypt_provisioning_key desc 0 "$TEST_RAW_KEY"
+echo "# ... bad type"
+_add_fscrypt_provisioning_key desc 10000 "$TEST_RAW_KEY"
+echo "# ... raw key too small"
+_add_fscrypt_provisioning_key desc $FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR ""
+echo "# ... raw key too large"
+_add_fscrypt_provisioning_key desc $FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR \
+	"$TEST_RAW_KEY$TEST_RAW_KEY"
+
+echo -e "\n# keyctl_read() doesn't work on fscrypt-provisioning keys"
+keyid=$(_add_fscrypt_provisioning_key desc $FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR \
+	"$TEST_RAW_KEY")
+$KEYCTL_PROG read $keyid
+$KEYCTL_PROG unlink $keyid @s
+
+echo -e "\n# Only keys with the correct fscrypt_provisioning_key_payload::type field can be added"
+echo "# ... keyring key is v1, filesystem wants v2 key"
+keyid=$(_add_fscrypt_provisioning_key desc $FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR \
+	"$TEST_RAW_KEY")
+$XFS_IO_PROG -c "add_enckey -k $keyid" $SCRATCH_MNT
+$KEYCTL_PROG unlink $keyid @s
+
+echo "# ... keyring key is v2, filesystem wants v1 key"
+keyid=$(_add_fscrypt_provisioning_key desc $FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER \
+	"$TEST_RAW_KEY")
+$XFS_IO_PROG -c "add_enckey -k $keyid -d $TEST_KEY_DESCRIPTOR" $SCRATCH_MNT
+$KEYCTL_PROG unlink $keyid @s
+
+echo -e "\n# Only keys of type fscrypt-provisioning can be added"
+keyid=$(head -c 64 /dev/urandom | $KEYCTL_PROG padd logon foo:desc @s)
+$XFS_IO_PROG -c "add_enckey -k $keyid" $SCRATCH_MNT
+$KEYCTL_PROG unlink $keyid @s
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/806.out b/tests/generic/806.out
new file mode 100644
index 00000000..b7795f9a
--- /dev/null
+++ b/tests/generic/806.out
@@ -0,0 +1,73 @@
+QA output created by 806
+
+# ==========================
+# Test with policy version 1
+# ==========================
+
+# Adding key to filesystem
+Added encryption key with descriptor 0000111122223333
+
+# Creating encrypted file
+
+# Removing key from filesystem
+Removed encryption key with descriptor 0000111122223333
+cat: SCRATCH_MNT/dir/file: No such file or directory
+
+# Adding fscrypt-provisioning key
+
+# Adding key to filesystem via fscrypt-provisioning key
+Added encryption key with descriptor 0000111122223333
+
+# Reading encrypted file
+contents
+
+# Cleaning up
+
+# ==========================
+# Test with policy version 2
+# ==========================
+
+# Adding key to filesystem
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+
+# Creating encrypted file
+
+# Removing key from filesystem
+Removed encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+cat: SCRATCH_MNT/dir/file: No such file or directory
+
+# Adding fscrypt-provisioning key
+
+# Adding key to filesystem via fscrypt-provisioning key
+Added encryption key with identifier 69b2f6edeee720cce0577937eb8a6751
+
+# Reading encrypted file
+contents
+
+# Cleaning up
+
+# ================
+# Validation tests
+# ================
+
+# Adding an invalid fscrypt-provisioning key fails
+# ... bad type
+add_key: Invalid argument
+# ... bad type
+add_key: Invalid argument
+# ... raw key too small
+add_key: Invalid argument
+# ... raw key too large
+add_key: Invalid argument
+
+# keyctl_read() doesn't work on fscrypt-provisioning keys
+keyctl_read_alloc: Operation not supported
+
+# Only keys with the correct fscrypt_provisioning_key_payload::type field can be added
+# ... keyring key is v1, filesystem wants v2 key
+Error adding encryption key: Key was rejected by service
+# ... keyring key is v2, filesystem wants v1 key
+Error adding encryption key: Key was rejected by service
+
+# Only keys of type fscrypt-provisioning can be added
+Error adding encryption key: Key was rejected by service
diff --git a/tests/generic/group b/tests/generic/group
index 02e4cc45..ad571ea1 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -589,3 +589,4 @@
 584 auto quick encrypt
 585 auto rename
 805 auto quick encrypt
+806 auto quick encrypt
-- 
2.24.0.432.g9d3f5f5b63-goog

Re: [RFC PATCH 3/3] generic: test adding filesystem-level fscrypt key via key_id

[Eric Biggers]

On Tue, Nov 19, 2019 at 02:31:30PM -0800, Eric Biggers wrote:
> +# Require support for adding a key to a filesystem's fscrypt keyring via an
> +# "fscrypt-provisioning" keyring key.
> +_require_add_enckey_by_key_id()
> +{
> +	local mnt=$1
> +
> +	# Userspace support
> +	_require_xfs_io_command "add_enckey" "-k"
> +
> +	# Kernel support
> +	if $XFS_IO_PROG -c "$add_enckey -k 12345 $mnt" \
> +		|& grep -q 'Invalid argument'; then
> +		_notrun "Kernel doesn't support key_id parameter to FS_IOC_ADD_ENCRYPTION_KEY"
> +	fi
> +}

There's a bug here that makes the test not be skipped when it should.
It should say:

	if $XFS_IO_PROG -c "add_enckey -k 12345" "$mnt" \

I'll fix this in the next version.

- Eric

Re: [RFC PATCH 0/3] xfstests: test adding filesystem-level fscrypt key via key_id

[Jarkko Sakkinen]

On Tue, Nov 19, 2019 at 02:31:27PM -0800, Eric Biggers wrote:
> This series adds a test which tests adding a key to a filesystem's
> fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
> alternative to the normal method where the raw key is given directly.
> 
> I'm sending this out for comment, but it shouldn't be merged until the
> corresponding kernel patch has reached mainline.  For more details, see
> the kernel patch:
> https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u
> 
> This test depends on an xfs_io patch which adds the '-k' option to the
> 'add_enckey' command, e.g.:
> 
> 	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT
> 
> This test is skipped if the needed kernel or xfs_io support is absent.
> 
> This has been tested on ext4, f2fs, and ubifs.
> 
> To apply cleanly, my other xfstests patch series
> "[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
> must be applied first.
> 
> This series can also be retrieved from
> https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
> tag "fscrypt-provisioning_2019-11-19".
> 
> Eric Biggers (3):
>   common/rc: handle option with argument in _require_xfs_io_command()
>   common/encrypt: move constant test key to common code
>   generic: test adding filesystem-level fscrypt key via key_id
> 
>  common/encrypt        |  95 +++++++++++++++++++++----
>  common/rc             |   2 +-
>  tests/generic/580     |  17 ++---
>  tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/806.out |  73 ++++++++++++++++++++
>  tests/generic/group   |   1 +
>  6 files changed, 316 insertions(+), 28 deletions(-)
>  create mode 100644 tests/generic/806
>  create mode 100644 tests/generic/806.out
> 
> -- 
> 2.24.0.432.g9d3f5f5b63-goog
> 

I'm newbie with fscrypt so I started by encrypting a directory without
the new feature

sudo tune2fs -O encrypt /dev/sda2
sudo fscrypt setup /
fscrypt encrypt foo

Worked.

Generally speaking I'd appreciate a usage example like here to the
commit message:

https://lwn.net/Articles/692514/

Is this doable?

I might consider trying out the XFS test suite some day but right now it
would be first nice to smoke test the feature quickly.

I think for this patch that would actually be mostly sufficient testing.

/Jarkko

Re: [RFC PATCH 0/3] xfstests: test adding filesystem-level fscrypt key via key_id

[Jarkko Sakkinen]

On Wed, Nov 27, 2019 at 10:45:36PM +0200, Jarkko Sakkinen wrote:
> On Tue, Nov 19, 2019 at 02:31:27PM -0800, Eric Biggers wrote:
> > This series adds a test which tests adding a key to a filesystem's
> > fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
> > alternative to the normal method where the raw key is given directly.
> > 
> > I'm sending this out for comment, but it shouldn't be merged until the
> > corresponding kernel patch has reached mainline.  For more details, see
> > the kernel patch:
> > https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u
> > 
> > This test depends on an xfs_io patch which adds the '-k' option to the
> > 'add_enckey' command, e.g.:
> > 
> > 	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT
> > 
> > This test is skipped if the needed kernel or xfs_io support is absent.
> > 
> > This has been tested on ext4, f2fs, and ubifs.
> > 
> > To apply cleanly, my other xfstests patch series
> > "[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
> > must be applied first.
> > 
> > This series can also be retrieved from
> > https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
> > tag "fscrypt-provisioning_2019-11-19".
> > 
> > Eric Biggers (3):
> >   common/rc: handle option with argument in _require_xfs_io_command()
> >   common/encrypt: move constant test key to common code
> >   generic: test adding filesystem-level fscrypt key via key_id
> > 
> >  common/encrypt        |  95 +++++++++++++++++++++----
> >  common/rc             |   2 +-
> >  tests/generic/580     |  17 ++---
> >  tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/806.out |  73 ++++++++++++++++++++
> >  tests/generic/group   |   1 +
> >  6 files changed, 316 insertions(+), 28 deletions(-)
> >  create mode 100644 tests/generic/806
> >  create mode 100644 tests/generic/806.out
> > 
> > -- 
> > 2.24.0.432.g9d3f5f5b63-goog
> > 
> 
> I'm newbie with fscrypt so I started by encrypting a directory without
> the new feature
> 
> sudo tune2fs -O encrypt /dev/sda2
> sudo fscrypt setup /
> fscrypt encrypt foo
> 
> Worked.
> 
> Generally speaking I'd appreciate a usage example like here to the
> commit message:
> 
> https://lwn.net/Articles/692514/
> 
> Is this doable?
> 
> I might consider trying out the XFS test suite some day but right now it
> would be first nice to smoke test the feature quickly.
> 
> I think for this patch that would actually be mostly sufficient testing.

Sorry, this was meant for the kernel patch :-/

/Jarkko

Re: [RFC PATCH 0/3] xfstests: test adding filesystem-level fscrypt key via key_id

[Eric Biggers]

On Wed, Nov 27, 2019 at 10:45:36PM +0200, Jarkko Sakkinen wrote:
> On Tue, Nov 19, 2019 at 02:31:27PM -0800, Eric Biggers wrote:
> > This series adds a test which tests adding a key to a filesystem's
> > fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
> > alternative to the normal method where the raw key is given directly.
> > 
> > I'm sending this out for comment, but it shouldn't be merged until the
> > corresponding kernel patch has reached mainline.  For more details, see
> > the kernel patch:
> > https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u
> > 
> > This test depends on an xfs_io patch which adds the '-k' option to the
> > 'add_enckey' command, e.g.:
> > 
> > 	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT
> > 
> > This test is skipped if the needed kernel or xfs_io support is absent.
> > 
> > This has been tested on ext4, f2fs, and ubifs.
> > 
> > To apply cleanly, my other xfstests patch series
> > "[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
> > must be applied first.
> > 
> > This series can also be retrieved from
> > https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
> > tag "fscrypt-provisioning_2019-11-19".
> > 
> > Eric Biggers (3):
> >   common/rc: handle option with argument in _require_xfs_io_command()
> >   common/encrypt: move constant test key to common code
> >   generic: test adding filesystem-level fscrypt key via key_id
> > 
> >  common/encrypt        |  95 +++++++++++++++++++++----
> >  common/rc             |   2 +-
> >  tests/generic/580     |  17 ++---
> >  tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/806.out |  73 ++++++++++++++++++++
> >  tests/generic/group   |   1 +
> >  6 files changed, 316 insertions(+), 28 deletions(-)
> >  create mode 100644 tests/generic/806
> >  create mode 100644 tests/generic/806.out
> > 
> > -- 
> > 2.24.0.432.g9d3f5f5b63-goog
> > 
> 
> I'm newbie with fscrypt so I started by encrypting a directory without
> the new feature
> 
> sudo tune2fs -O encrypt /dev/sda2
> sudo fscrypt setup /
> fscrypt encrypt foo
> 
> Worked.
> 
> Generally speaking I'd appreciate a usage example like here to the
> commit message:
> 
> https://lwn.net/Articles/692514/
> 
> Is this doable?
> 
> I might consider trying out the XFS test suite some day but right now it
> would be first nice to smoke test the feature quickly.
> 
> I think for this patch that would actually be mostly sufficient testing.
> 

You could manually do what the xfstest does, which is more or less the following
(requires xfs_io patched with https://patchwork.kernel.org/patch/11252795/):

	mkfs.ext4 -O encrypt /dev/vdc
	mount /dev/vdc /vdc
	keyctl new_session
	payload='\x02\x00\x00\x00\x00\x00\x00\x00'
	for i in {1..64}; do
		payload+="\\x$(printf "%02x" $i)"
	done
	keyid=$(echo -ne "$payload" | keyctl padd fscrypt-provisioning desc @s)
	keyspec=$(xfs_io -c "add_enckey -k $keyid" /vdc | awk '{print $NF}')
	mkdir /vdc/edir
	xfs_io -c "set_encpolicy $keyspec" /vdc/edir
	echo contents > /vdc/edir/file

I'm not yet planning to use this feature in the 'fscrypt' userspace tool
https://github.com/google/fscrypt.  The user who actually wants this feature
(Android) isn't using the 'fscrypt' tool, but rather is using custom code.

Also, the 'fscrypt' tool isn't meant for testing or exposing every corner of the
fscrypt kernel API.  Rather, it's meant to be a user-friendly tool for creating
encrypted directories on "regular" Linux distros, supporting lots of higher
level userspace features like passphrase hashing with Argon2, multiple
protectors, and auto-unlocking directories with PAM.

So for now, the important thing is to have an xfstest that fully tests this new
API.  For that, like the other fscrypt tests, I'm using keyctl and xfs_io to
access the kernel API directly.

Later, if people really need the 'fscrypt' tool to do something that requires
this feature, we can add it.  This would likely take the form of a user-friendly
option to 'fscrypt unlock' rather than a direct translation of the kernel API.

- Eric

Re: [RFC PATCH 0/3] xfstests: test adding filesystem-level fscrypt key via key_id

[Jarkko Sakkinen]

On Wed, Nov 27, 2019 at 02:57:59PM -0800, Eric Biggers wrote:
> You could manually do what the xfstest does, which is more or less the following
> (requires xfs_io patched with https://patchwork.kernel.org/patch/11252795/):

I postpone testing/reviewing this patch up until its depedencies are in
the mainline.

I'll add these to my tree as soon as we have addressed a critical bug
in tpm_tis:

1. KEYS: remove CONFIG_KEYS_COMPAT
2. KEYS: asymmetric: return ENOMEM if akcipher_request_alloc() fails

Just mentioning that I haven't forgotten them.

/Jarkko

Re: [RFC PATCH 0/3] xfstests: test adding filesystem-level fscrypt key via key_id

[Eric Biggers]

Hi Jarkko,

On Wed, Dec 11, 2019 at 11:50:19AM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 27, 2019 at 02:57:59PM -0800, Eric Biggers wrote:
> > You could manually do what the xfstest does, which is more or less the following
> > (requires xfs_io patched with https://patchwork.kernel.org/patch/11252795/):
> 
> I postpone testing/reviewing this patch up until its depedencies are in
> the mainline.
> 
> I'll add these to my tree as soon as we have addressed a critical bug
> in tpm_tis:
> 
> 1. KEYS: remove CONFIG_KEYS_COMPAT
> 2. KEYS: asymmetric: return ENOMEM if akcipher_request_alloc() fails
> 
> Just mentioning that I haven't forgotten them.
> 

xfstests and xfsprogs are developed separately from the kernel, and their
maintainers don't apply patches that depend on non-mainlined features.  So
unless there are objections to the kernel patch [1], in a couple weeks I'll
apply it to the fscrypt tree for 5.6, and then once it's in mainline I'll resend
the patches for the test.  I've simply sent the test out early as an RFC, in
case it helps reviewing the kernel patch or in case there are early comments.

Again, while you're certainly welcome to manually test the kernel patch, it's
more important that we have test coverage of it in xfstests.

[1] https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/

- Eric