This series adds a test that verifies the on-disk format of encrypted
files that use a crypto data unit size that differs from the filesystem
block size. This tests the functionality that was introduced in Linux
6.7 by kernel commit 5b1188847180 ("fscrypt: support crypto data unit
size less than filesystem block size").
This depends on the xfsprogs patch
"xfs_io/encrypt: support specifying crypto data unit size"
(https://lore.kernel.org/r/[email protected])
which adds the '-s' option to the set_encpolicy command of xfs_io.
As usual, the test skips itself when any prerequisite isn't met.
I've tested the new test on both ext4 and f2fs.
Changed in v2:
- Updated the cover letter, commit message, and a comment to reflect
that the kernel commit that added this feature was merged in 6.7.
- Rebased onto latest for-next branch of xfstests.
Eric Biggers (4):
fscrypt-crypt-util: rename block to data unit
common/rc: fix _require_xfs_io_command with digits in argument
common/encrypt: support custom data unit size
generic: add test for custom crypto data unit size
common/encrypt | 42 +++++++++++++-----
common/rc | 2 +-
src/fscrypt-crypt-util.c | 93 ++++++++++++++++++++--------------------
tests/f2fs/002 | 6 +--
tests/generic/900 | 29 +++++++++++++
tests/generic/900.out | 11 +++++
6 files changed, 123 insertions(+), 60 deletions(-)
create mode 100755 tests/generic/900
create mode 100644 tests/generic/900.out
base-commit: b9e1a88f8198ac02f3b82fe3b127d4e14f4a97b7
--
2.42.1
From: Eric Biggers <[email protected]>
Make _require_scratch_encryption() and
_require_encryption_policy_support() support the new '-s' option to
set_encpolicy to specify a custom value of log2_data_unit_size.
Likewise, make _verify_ciphertext_for_encryption_policy() accept an
argument "log2_dusize=*" to cause it to use the specified data unit size
for the test and verify that the file contents are encrypted as expected
for that data unit size.
Signed-off-by: Eric Biggers <[email protected]>
---
common/encrypt | 38 ++++++++++++++++++++++++++++++--------
1 file changed, 30 insertions(+), 8 deletions(-)
diff --git a/common/encrypt b/common/encrypt
index 5688745c..d90a566a 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -1,32 +1,41 @@
##/bin/bash
# SPDX-License-Identifier: GPL-2.0
# Copyright (c) 2016 Google, Inc. All Rights Reserved.
#
# Functions for setting up and testing file encryption
#
# _require_scratch_encryption [-c CONTENTS_MODE] [-n FILENAMES_MODE]
# [-f POLICY_FLAGS] [-v POLICY_VERSION]
+# [-s LOG2_DUSIZE]
#
# Require encryption support on the scratch device.
#
# This checks for support for the default type of encryption policy (v1 with
# AES-256-XTS and AES-256-CTS). Options can be specified to also require
# support for a different type of encryption policy.
#
_require_scratch_encryption()
{
- _require_scratch
+ local arg
+ _require_scratch
_require_xfs_io_command "set_encpolicy"
+ for arg; do
+ if [ "$arg" = "-s" ]; then
+ # -s option was added later. Make sure it's available.
+ _require_xfs_io_command "set_encpolicy" "-s"
+ fi
+ done
+
# The 'test_dummy_encryption' mount option interferes with trying to use
# encryption for real, even if we are just trying to get/set policies
# and never put any keys in the keyring. So skip the real encryption
# tests if the 'test_dummy_encryption' mount option was specified.
_exclude_scratch_mount_option "test_dummy_encryption"
# Make a filesystem on the scratch device with the encryption feature
# enabled. If this fails then probably the userspace tools (e.g.
# e2fsprogs or f2fs-tools) are too old to understand encryption.
if ! _scratch_mkfs_encrypted &>>$seqres.full; then
@@ -67,35 +76,35 @@ _require_scratch_encryption()
_require_encryption_policy_support()
{
local mnt=$1
local dir=$mnt/tmpdir
local set_encpolicy_args=""
local policy_flags=0
local policy_version=1
local c
OPTIND=2
- while getopts "c:n:f:v:" c; do
+ while getopts "c:n:f:s:v:" c; do
case $c in
- c|n)
+ c|n|s)
set_encpolicy_args+=" -$c $OPTARG"
;;
f)
set_encpolicy_args+=" -$c $OPTARG"
policy_flags=$OPTARG
;;
v)
set_encpolicy_args+=" -$c $OPTARG"
policy_version=$OPTARG
;;
*)
- _fail "Unrecognized option '$c'"
+ _fail "${FUNCNAME[0]}: unrecognized option '$c'"
;;
esac
done
set_encpolicy_args=${set_encpolicy_args# }
echo "Checking whether kernel supports encryption policy: $set_encpolicy_args" \
>> $seqres.full
if (( policy_flags & (FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64 |
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32) )); then
@@ -756,28 +765,27 @@ _do_verify_ciphertext_for_encryption_policy()
# Now unmount the filesystem and verify the ciphertext we just wrote.
_scratch_unmount
echo "Verifying encrypted file contents" >> $seqres.full
for f in "${test_contents_files[@]}"; do
read -r src inode blocklist <<< "$f"
nonce=$(_get_encryption_nonce $SCRATCH_DEV $inode)
_dump_ciphertext_blocks $SCRATCH_DEV $blocklist > $tmp.actual_contents
$crypt_contents_cmd $contents_encryption_mode $raw_key_hex \
- --file-nonce=$nonce --data-unit-size=$blocksize \
- --inode-number=$inode < $src > $tmp.expected_contents
+ --file-nonce=$nonce --inode-number=$inode \
+ < $src > $tmp.expected_contents
if ! cmp $tmp.expected_contents $tmp.actual_contents; then
_fail "Expected encrypted contents != actual encrypted contents. File: $f"
fi
$crypt_contents_cmd $contents_encryption_mode $raw_key_hex \
- --decrypt --file-nonce=$nonce \
- --data-unit-size=$blocksize --inode-number=$inode \
+ --decrypt --file-nonce=$nonce --inode-number=$inode \
< $tmp.actual_contents > $tmp.decrypted_contents
if ! cmp $src $tmp.decrypted_contents; then
_fail "Contents decryption sanity check failed. File: $f"
fi
done
echo "Verifying encrypted file names" >> $seqres.full
for f in "${test_filenames_files[@]}"; do
read -r name inode dir_inode padding <<< "$f"
nonce=$(_get_encryption_nonce $SCRATCH_DEV $dir_inode)
@@ -837,28 +845,30 @@ _fscrypt_mode_name_to_num()
# policy of the specified type is used.
#
# The first two parameters are the contents and filenames encryption modes to
# test. The following optional parameters are also accepted to further modify
# the type of encryption policy that is tested:
#
# 'v2': test a v2 encryption policy
# 'direct': test the DIRECT_KEY policy flag
# 'iv_ino_lblk_64': test the IV_INO_LBLK_64 policy flag
# 'iv_ino_lblk_32': test the IV_INO_LBLK_32 policy flag
+# 'log2_dusize=N': test the log2_data_unit_size field
#
_verify_ciphertext_for_encryption_policy()
{
local contents_encryption_mode=$1
local filenames_encryption_mode=$2
local opt
local policy_version=1
local policy_flags=0
+ local log2_dusize=0
local set_encpolicy_args=""
local crypt_util_args=""
local crypt_util_contents_args=""
local crypt_util_filename_args=""
local expected_identifier
shift 2
for opt; do
case "$opt" in
v2)
@@ -870,30 +880,36 @@ _verify_ciphertext_for_encryption_policy()
_fail "For direct key mode, contents and filenames modes must match"
fi
(( policy_flags |= FSCRYPT_POLICY_FLAG_DIRECT_KEY ))
;;
iv_ino_lblk_64)
(( policy_flags |= FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64 ))
;;
iv_ino_lblk_32)
(( policy_flags |= FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32 ))
;;
+ log2_dusize=*)
+ log2_dusize=$(echo "$opt" | sed 's/^log2_dusize=//')
+ ;;
*)
_fail "Unknown option '$opt' passed to ${FUNCNAME[0]}"
;;
esac
done
local contents_mode_num=$(_fscrypt_mode_name_to_num $contents_encryption_mode)
local filenames_mode_num=$(_fscrypt_mode_name_to_num $filenames_encryption_mode)
set_encpolicy_args+=" -c $contents_mode_num"
set_encpolicy_args+=" -n $filenames_mode_num"
+ if (( log2_dusize != 0 )); then
+ set_encpolicy_args+=" -s $log2_dusize"
+ fi
crypt_util_contents_args+=" --mode-num=$contents_mode_num"
crypt_util_filename_args+=" --mode-num=$filenames_mode_num"
if (( policy_version > 1 )); then
set_encpolicy_args+=" -v 2"
crypt_util_args+=" --kdf=HKDF-SHA512"
if (( policy_flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY )); then
crypt_util_args+=" --direct-key"
elif (( policy_flags & FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64 )); then
crypt_util_args+=" --iv-ino-lblk-64"
@@ -923,20 +939,26 @@ _verify_ciphertext_for_encryption_policy()
echo "Creating encryption-capable filesystem" >> $seqres.full
if (( policy_flags & (FSCRYPT_POLICY_FLAG_IV_INO_LBLK_64 |
FSCRYPT_POLICY_FLAG_IV_INO_LBLK_32) )); then
_scratch_mkfs_stable_inodes_encrypted &>> $seqres.full
else
_scratch_mkfs_encrypted &>> $seqres.full
fi
_scratch_mount
+ if (( log2_dusize != 0 )); then
+ crypt_util_contents_args+=" --data-unit-size=$((1 << log2_dusize))"
+ else
+ crypt_util_contents_args+=" --data-unit-size=$(_get_block_size $SCRATCH_MNT)"
+ fi
+
crypt_util_args+=" --fs-uuid=$(blkid -s UUID -o value $SCRATCH_DEV | tr -d -)"
crypt_util_contents_args+="$crypt_util_args"
crypt_util_filename_args+="$crypt_util_args"
echo "Generating encryption key" >> $seqres.full
local raw_key=$(_generate_raw_encryption_key)
if (( policy_version > 1 )); then
local keyspec=$(_add_enckey $SCRATCH_MNT "$raw_key" \
| awk '{print $NF}')
--
2.42.1
From: Eric Biggers <[email protected]>
Add a test that verifies the on-disk format of encrypted files that use
a crypto data unit size that differs from the filesystem block size.
This tests the functionality that was introduced in Linux 6.7 by kernel
commit 5b1188847180 ("fscrypt: support crypto data unit size less than
filesystem block size").
This depends on the xfsprogs patch
"xfs_io/encrypt: support specifying crypto data unit size"
(https://lore.kernel.org/r/[email protected])
which adds the '-s' option to the set_encpolicy command of xfs_io.
As usual, the test skips itself when any prerequisite isn't met.
Signed-off-by: Eric Biggers <[email protected]>
---
tests/generic/900 | 29 +++++++++++++++++++++++++++++
tests/generic/900.out | 11 +++++++++++
2 files changed, 40 insertions(+)
create mode 100755 tests/generic/900
create mode 100644 tests/generic/900.out
diff --git a/tests/generic/900 b/tests/generic/900
new file mode 100755
index 00000000..8d1b5766
--- /dev/null
+++ b/tests/generic/900
@@ -0,0 +1,29 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2023 Google LLC
+#
+# FS QA Test No. generic/900
+#
+# Verify the on-disk format of encrypted files that use a crypto data unit size
+# that differs from the filesystem block size. This tests the functionality
+# that was introduced in Linux 6.7 by kernel commit 5b1188847180
+# ("fscrypt: support crypto data unit size less than filesystem block size").
+#
+. ./common/preamble
+_begin_fstest auto quick encrypt
+
+. ./common/filter
+. ./common/encrypt
+
+_supported_fs generic
+
+# For now, just test 512-byte and 1024-byte data units. Filesystems accept
+# power-of-2 sizes between 512 and the filesystem block size, inclusively.
+# Testing 512 and 1024 ensures this test will run for any FS block size >= 1024
+# (provided that the filesystem supports sub-block data units at all).
+_verify_ciphertext_for_encryption_policy AES-256-XTS AES-256-CTS-CBC v2 log2_dusize=9
+_verify_ciphertext_for_encryption_policy AES-256-XTS AES-256-CTS-CBC v2 log2_dusize=10
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/900.out b/tests/generic/900.out
new file mode 100644
index 00000000..3259f08c
--- /dev/null
+++ b/tests/generic/900.out
@@ -0,0 +1,11 @@
+QA output created by 900
+
+Verifying ciphertext with parameters:
+ contents_encryption_mode: AES-256-XTS
+ filenames_encryption_mode: AES-256-CTS-CBC
+ options: v2 log2_dusize=9
+
+Verifying ciphertext with parameters:
+ contents_encryption_mode: AES-256-XTS
+ filenames_encryption_mode: AES-256-CTS-CBC
+ options: v2 log2_dusize=10
--
2.42.1
From: Eric Biggers <[email protected]>
'_require_xfs_io_command set_encpolicy -s' does not work as expected
because the following in the output of 'xfs_io -c "help set_encpolicy"':
-s LOG2_DUSIZE -- log2 of data unit size
... does not match the regex:
"^ -s ([a-zA-Z_]+ )?--"
... because the 2 in the argument name LOG2_DUSIZE is not matched. Fix
the regex to support digits in the argument name.
Signed-off-by: Eric Biggers <[email protected]>
---
common/rc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/rc b/common/rc
index cc92fe06..dab672d8 100644
--- a/common/rc
+++ b/common/rc
@@ -2719,21 +2719,21 @@ _require_xfs_io_command()
echo $testio | grep -q "foreign file active" && \
_notrun "xfs_io $command $param_checked not supported on $FSTYP"
echo $testio | grep -q "Function not implemented" && \
_notrun "xfs_io $command $param_checked support is missing (missing syscall?)"
echo $testio | grep -q "unknown flag" && \
_notrun "xfs_io $command $param_checked support is missing (unknown flag)"
[ -n "$param" ] || return
if [ -z "$param_checked" ]; then
- $XFS_IO_PROG -c "help $command" | grep -E -q "^ $param ([a-zA-Z_]+ )?--" || \
+ $XFS_IO_PROG -c "help $command" | grep -E -q "^ $param ([a-zA-Z0-9_]+ )?--" || \
_notrun "xfs_io $command doesn't support $param"
else
# xfs_io could result in "command %c not supported" if it was
# built on kernels not supporting pwritev2() calls
echo $testio | grep -q "\(invalid option\|not supported\)" && \
_notrun "xfs_io $command doesn't support $param"
fi
# On XFS, ioctl(FSSETXATTR)(called by xfs_io -c "chattr") maskes off unsupported
# or invalid flags silently so need to check these flags by extra ioctl(FSGETXATTR)
--
2.42.1
On Tue, Nov 21, 2023 at 02:39:05PM -0800, Eric Biggers wrote:
> This series adds a test that verifies the on-disk format of encrypted
> files that use a crypto data unit size that differs from the filesystem
> block size. This tests the functionality that was introduced in Linux
> 6.7 by kernel commit 5b1188847180 ("fscrypt: support crypto data unit
> size less than filesystem block size").
Hi Zorro, can you consider applying this series? It's been out for review for
3 months, so I don't think reviews are going to come in. The prerequisite
xfsprogs patch is already present on the for-next branch of xfsprogs.
Thanks!
- Eric
On Wed, Jan 10, 2024 at 07:54:44PM -0800, Eric Biggers wrote:
> On Tue, Nov 21, 2023 at 02:39:05PM -0800, Eric Biggers wrote:
> > This series adds a test that verifies the on-disk format of encrypted
> > files that use a crypto data unit size that differs from the filesystem
> > block size. This tests the functionality that was introduced in Linux
> > 6.7 by kernel commit 5b1188847180 ("fscrypt: support crypto data unit
> > size less than filesystem block size").
>
> Hi Zorro, can you consider applying this series? It's been out for review for
> 3 months, so I don't think reviews are going to come in. The prerequisite
> xfsprogs patch is already present on the for-next branch of xfsprogs.
Sure Eric, I thought about this patchset last week, I saw that xfsprogs
patchset has been merge. I'll give this patchset a basic test, to make
sure it won't break xfstests, then merge it.
Thanks, and sorry this late.
Zorro
>
> Thanks!
>
> - Eric
>
On Tue, Nov 21, 2023 at 02:39:09PM -0800, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Add a test that verifies the on-disk format of encrypted files that use
> a crypto data unit size that differs from the filesystem block size.
> This tests the functionality that was introduced in Linux 6.7 by kernel
> commit 5b1188847180 ("fscrypt: support crypto data unit size less than
> filesystem block size").
>
> This depends on the xfsprogs patch
> "xfs_io/encrypt: support specifying crypto data unit size"
> (https://lore.kernel.org/r/[email protected])
> which adds the '-s' option to the set_encpolicy command of xfs_io.
>
> As usual, the test skips itself when any prerequisite isn't met.
>
> Signed-off-by: Eric Biggers <[email protected]>
> ---
> tests/generic/900 | 29 +++++++++++++++++++++++++++++
> tests/generic/900.out | 11 +++++++++++
> 2 files changed, 40 insertions(+)
> create mode 100755 tests/generic/900
> create mode 100644 tests/generic/900.out
>
> diff --git a/tests/generic/900 b/tests/generic/900
> new file mode 100755
> index 00000000..8d1b5766
> --- /dev/null
> +++ b/tests/generic/900
> @@ -0,0 +1,29 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright 2023 Google LLC
> +#
> +# FS QA Test No. generic/900
> +#
> +# Verify the on-disk format of encrypted files that use a crypto data unit size
> +# that differs from the filesystem block size. This tests the functionality
> +# that was introduced in Linux 6.7 by kernel commit 5b1188847180
> +# ("fscrypt: support crypto data unit size less than filesystem block size").
I'll write this part as:
_wants_kernel_commit 5b1188847180
"fscrypt: support crypto data unit size less than filesystem block size"
when I merge it.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick encrypt
> +
> +. ./common/filter
> +. ./common/encrypt
> +
> +_supported_fs generic
> +
> +# For now, just test 512-byte and 1024-byte data units. Filesystems accept
> +# power-of-2 sizes between 512 and the filesystem block size, inclusively.
> +# Testing 512 and 1024 ensures this test will run for any FS block size >= 1024
> +# (provided that the filesystem supports sub-block data units at all).
> +_verify_ciphertext_for_encryption_policy AES-256-XTS AES-256-CTS-CBC v2 log2_dusize=9
Oh, all _require_scratch_... things are in this helper. I was wondering
why it doesn't need to _require_scratch :)
This patchset looks good to me, tested passed on latest upstream kernel.
Thanks for this upgrade. Feel free to ping me, if your patchset be blocked
long time.
Reviewed-by: Zorro Lang <[email protected]>
> +_verify_ciphertext_for_encryption_policy AES-256-XTS AES-256-CTS-CBC v2 log2_dusize=10
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/900.out b/tests/generic/900.out
> new file mode 100644
> index 00000000..3259f08c
> --- /dev/null
> +++ b/tests/generic/900.out
> @@ -0,0 +1,11 @@
> +QA output created by 900
> +
> +Verifying ciphertext with parameters:
> + contents_encryption_mode: AES-256-XTS
> + filenames_encryption_mode: AES-256-CTS-CBC
> + options: v2 log2_dusize=9
> +
> +Verifying ciphertext with parameters:
> + contents_encryption_mode: AES-256-XTS
> + filenames_encryption_mode: AES-256-CTS-CBC
> + options: v2 log2_dusize=10
> --
> 2.42.1
>
>