[PATCH 0/3] xfstests: fscrypt no-key name updates

[PATCH 0/3] xfstests: fscrypt no-key name updates

[Eric Biggers]

This series cleans up handling of no-key names in the encryption tests.

Eric Biggers (3):
  generic: update encryption tests to use term "no-key names"
  common/encrypt: add helper function for filtering no-key names
  common/encrypt: accept '-' character in no-key names

 common/encrypt        | 20 +++++++++++++++++++
 tests/generic/397     | 12 +++++------
 tests/generic/419     |  6 +++---
 tests/generic/419.out |  2 +-
 tests/generic/421     |  8 ++++----
 tests/generic/429     | 46 ++++++++++++++++++++-----------------------
 tests/generic/429.out | 22 ++++++++++-----------
 7 files changed, 66 insertions(+), 50 deletions(-)


base-commit: 623b4ea4082b8be71acaf261435d33fa4488993a
-- 
2.32.0

[PATCH 1/3] generic: update encryption tests to use term "no-key names"

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

To avoid ambiguity, don't use the terms "ciphertext names" or "encrypted
names" when we really mean "no-key names" (the names that the filesystem
shows when userspace lists an encrypted directory without the
directory's encryption key being present).  This aligns with changes
that have been made to the kernel source code and documentation
(https://lore.kernel.org/r/20200924042624.98439-1-ebiggers@kernel.org).

No change to the actual test logic.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 tests/generic/397     | 12 ++++++------
 tests/generic/419     |  4 ++--
 tests/generic/421     |  8 ++++----
 tests/generic/429     | 44 +++++++++++++++++++++----------------------
 tests/generic/429.out | 22 +++++++++++-----------
 5 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/tests/generic/397 b/tests/generic/397
index 56771a76..5ff65cd9 100755
--- a/tests/generic/397
+++ b/tests/generic/397
@@ -52,15 +52,15 @@ diff -r $SCRATCH_MNT/edir $SCRATCH_MNT/ref_dir
 #
 # Now try accessing the files without the encryption key.  It should still be
 # possible to list the directory and remove files.  But filenames should be
-# encrypted, and it should not be possible to read regular files or to create
-# new files or subdirectories.
+# presented in no-key form, and it should not be possible to read regular files
+# or to create new files or subdirectories.
 #
-# Note that we cannot simply use ls -R to verify the files because the encrypted
+# Note that we cannot simply use ls -R to verify the files because the no-key
 # filenames are unpredictable.  By design, the key used to encrypt a directory's
 # filenames is derived from the master key (the key in the keyring) and a nonce
-# generated by the kernel.  Hence, the encrypted filenames will be different
-# every time this test is run, even if we were to put a fixed key into the
-# keyring instead of a random one.  The same applies to symlink targets.
+# generated by the kernel.  Hence, the no-key filenames will be different every
+# time this test is run, even if we were to put a fixed key into the keyring
+# instead of a random one.  The same applies to symlink targets.
 #
 
 _unlink_session_encryption_key $keydesc
diff --git a/tests/generic/419 b/tests/generic/419
index cb55a5bb..61f9d605 100755
--- a/tests/generic/419
+++ b/tests/generic/419
@@ -37,8 +37,8 @@ echo b > $SCRATCH_MNT/edir/b
 _unlink_session_encryption_key $keydesc
 _scratch_cycle_mount
 
-# Note that because encrypted filenames are unpredictable, this needs to be
-# written in a way that does not assume any particular filenames.
+# Note that because no-key filenames are unpredictable, this needs to be written
+# in a way that does not assume any particular filenames.
 efile1=$(find $SCRATCH_MNT/edir -maxdepth 1 -type f | head -1)
 efile2=$(find $SCRATCH_MNT/edir -maxdepth 1 -type f | tail -1)
 mv $efile1 $efile2 |& _filter_scratch | sed 's|edir/[a-zA-Z0-9+,_]\+|edir/FILENAME|g'
diff --git a/tests/generic/421 b/tests/generic/421
index 1080e577..04462d17 100755
--- a/tests/generic/421
+++ b/tests/generic/421
@@ -59,10 +59,10 @@ keyid=$(_revoke_session_encryption_key $keydesc)
 # Now try to open the file again.  In buggy kernels this caused concurrent
 # readers to crash with a NULL pointer dereference during decryption.
 #
-# Note that the fix also made filenames stop "immediately" reverting to their
-# ciphertext on key revocation.  Therefore, the name of the file we're opening
-# here may be in either plaintext or ciphertext depending on the kernel version,
-# and ciphertext names are unpredictable anyway, so just use 'find' to find it.
+# Note that the fix also made filenames stop "immediately" reverting to no-key
+# names on key revocation.  Therefore, the name of the file we're opening here
+# may be in either plaintext or no-key form depending on the kernel version, and
+# no-key names are unpredictable anyway, so just use 'find' to find it.
 cat "$(find $dir -type f)" > /dev/null
 
 # Wait for readers to exit
diff --git a/tests/generic/429 b/tests/generic/429
index 034063d9..ba2281c9 100755
--- a/tests/generic/429
+++ b/tests/generic/429
@@ -4,12 +4,12 @@
 #
 # FS QA Test No. 429
 #
-# Test that encrypted dentries are revalidated after adding a key.
-# Regression test for:
+# Test that no-key dentries are revalidated after adding a key.  Regression test
+# for:
 #	28b4c263961c ("ext4 crypto: revalidate dentry after adding or removing the key")
 #
-# Furthermore, test that encrypted directories are *not* revalidated after
-# "revoking" a key.  This used to be done, but it was broken and was removed by:
+# Furthermore, test that no-key dentries are *not* revalidated after "revoking"
+# a key.  This used to be done, but it was broken and was removed by:
 #	1b53cf9815bb ("fscrypt: remove broken support for detecting keyring key revocation")
 #
 # Also test for a race condition bug in 28b4c263961c, fixed by:
@@ -43,18 +43,18 @@ _add_session_encryption_key $keydesc $raw_key
 _set_encpolicy $SCRATCH_MNT/edir $keydesc
 
 # Create two files in the directory: one whose name is valid in the base64
-# format used for encoding ciphertext filenames, and one whose name is not.  The
-# exact filenames *should* be irrelevant, but due to yet another bug, ->lookup()
-# in an encrypted directory without the key returned ERR_PTR(-ENOENT) rather
-# than NULL if the name was not valid ciphertext, causing a negative dentry to
-# not be created.  For the purpose of this test, we want at least one negative
-# dentry to be created, so just create both types of name.
+# format used in no-key filenames, and one whose name is not.  The exact
+# filenames *should* be irrelevant, but due to yet another bug, ->lookup() in an
+# encrypted directory without the key returned ERR_PTR(-ENOENT) rather than NULL
+# if the name was not a valid no-key name, causing a negative dentry to not be
+# created.  For the purpose of this test, we want at least one negative dentry
+# to be created, so just create both types of name.
 echo contents_@@@ > $SCRATCH_MNT/edir/@@@ # not valid base64
 echo contents_abcd > $SCRATCH_MNT/edir/abcd # valid base64
 
-filter_ciphertext_filenames()
+filter_nokey_filenames()
 {
-	_filter_scratch | sed 's|edir/[a-zA-Z0-9+,_]\+|edir/ENCRYPTED_NAME|g'
+	_filter_scratch | sed 's|edir/[a-zA-Z0-9+,_]\+|edir/NOKEY_NAME|g'
 }
 
 show_file_contents()
@@ -62,8 +62,8 @@ show_file_contents()
 	echo "--- Contents of files using plaintext names:"
 	cat $SCRATCH_MNT/edir/@@@ |& _filter_scratch
 	cat $SCRATCH_MNT/edir/abcd |& _filter_scratch
-	echo "--- Contents of files using ciphertext names:"
-	cat ${ciphertext_names[@]} |& filter_ciphertext_filenames
+	echo "--- Contents of files using no-key names:"
+	cat ${nokey_names[@]} |& filter_nokey_filenames
 }
 
 show_directory_with_key()
@@ -75,21 +75,21 @@ show_directory_with_key()
 
 # View the directory without the encryption key.  The plaintext names shouldn't
 # exist, but 'cat' each to verify this, which also should create negative
-# dentries.  The ciphertext names are unpredictable by design, but verify that
-# the correct number of them are listed by readdir, and save them for later.
+# dentries.  The no-key names are unpredictable by design, but verify that the
+# correct number of them are listed by readdir, and save them for later.
 echo
 echo "***** Without encryption key *****"
 _unlink_session_encryption_key $keydesc
 _scratch_cycle_mount
 echo "--- Directory listing:"
-ciphertext_names=( $(find $SCRATCH_MNT/edir -mindepth 1 | sort) )
-printf '%s\n' "${ciphertext_names[@]}" | filter_ciphertext_filenames
+nokey_names=( $(find $SCRATCH_MNT/edir -mindepth 1 | sort) )
+printf '%s\n' "${nokey_names[@]}" | filter_nokey_filenames
 show_file_contents
 
 # Without remounting or dropping caches, add the encryption key and view the
-# directory again.  Now the plaintext names should all be there, and the
-# ciphertext names should be gone.  Make sure to 'cat' all the names to test for
-# stale dentries.
+# directory again.  Now the plaintext names should all be there, and the no-key
+# names should be gone.  Make sure to 'cat' all the names to test for stale
+# dentries.
 echo
 echo "***** With encryption key *****"
 _add_session_encryption_key $keydesc $raw_key
@@ -103,7 +103,7 @@ rm -rf $SCRATCH_MNT/edir/dir
 
 # Now open the files to pin them in the inode cache (needed to make the test
 # reliable), then revoke the encryption key.  This should no longer cause the
-# files to be presented in ciphertext form immediately.
+# files to be presented in no-key form immediately.
 echo
 echo "***** After key revocation *****"
 (
diff --git a/tests/generic/429.out b/tests/generic/429.out
index bebc4865..68c2590d 100644
--- a/tests/generic/429.out
+++ b/tests/generic/429.out
@@ -2,14 +2,14 @@ QA output created by 429
 
 ***** Without encryption key *****
 --- Directory listing:
-SCRATCH_MNT/edir/ENCRYPTED_NAME
-SCRATCH_MNT/edir/ENCRYPTED_NAME
+SCRATCH_MNT/edir/NOKEY_NAME
+SCRATCH_MNT/edir/NOKEY_NAME
 --- Contents of files using plaintext names:
 cat: SCRATCH_MNT/edir/@@@: No such file or directory
 cat: SCRATCH_MNT/edir/abcd: No such file or directory
---- Contents of files using ciphertext names:
-cat: SCRATCH_MNT/edir/ENCRYPTED_NAME: Required key not available
-cat: SCRATCH_MNT/edir/ENCRYPTED_NAME: Required key not available
+--- Contents of files using no-key names:
+cat: SCRATCH_MNT/edir/NOKEY_NAME: Required key not available
+cat: SCRATCH_MNT/edir/NOKEY_NAME: Required key not available
 
 ***** With encryption key *****
 --- Directory listing:
@@ -18,9 +18,9 @@ SCRATCH_MNT/edir/abcd
 --- Contents of files using plaintext names:
 contents_@@@
 contents_abcd
---- Contents of files using ciphertext names:
-cat: SCRATCH_MNT/edir/ENCRYPTED_NAME: No such file or directory
-cat: SCRATCH_MNT/edir/ENCRYPTED_NAME: No such file or directory
+--- Contents of files using no-key names:
+cat: SCRATCH_MNT/edir/NOKEY_NAME: No such file or directory
+cat: SCRATCH_MNT/edir/NOKEY_NAME: No such file or directory
 
 ***** Race conditions *****
 t_encrypted_d_revalidate finished
@@ -32,6 +32,6 @@ SCRATCH_MNT/edir/abcd
 --- Contents of files using plaintext names:
 contents_@@@
 contents_abcd
---- Contents of files using ciphertext names:
-cat: SCRATCH_MNT/edir/ENCRYPTED_NAME: No such file or directory
-cat: SCRATCH_MNT/edir/ENCRYPTED_NAME: No such file or directory
+--- Contents of files using no-key names:
+cat: SCRATCH_MNT/edir/NOKEY_NAME: No such file or directory
+cat: SCRATCH_MNT/edir/NOKEY_NAME: No such file or directory
-- 
2.32.0

[PATCH 2/3] common/encrypt: add helper function for filtering no-key names

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add a helper function _filter_nokey_filenames() which replaces no-key
filenames with "NOKEY_NAME".  Use it in generic/419 and generic/429.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/encrypt        | 17 +++++++++++++++++
 tests/generic/419     |  2 +-
 tests/generic/419.out |  2 +-
 tests/generic/429     | 10 +++-------
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/common/encrypt b/common/encrypt
index c4cc2d83..766a6d81 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -920,3 +920,20 @@ _verify_ciphertext_for_encryption_policy()
 		"$crypt_util_contents_args" \
 		"$crypt_util_filename_args"
 }
+
+# Replace no-key filenames in the given directory with "NOKEY_NAME".
+#
+# No-key filenames are the filenames that the filesystem shows when userspace
+# lists an encrypted directory without the directory's encryption key being
+# present.  These will differ on every run of the test, even when using the same
+# encryption key, hence the need for this filtering in some cases.
+#
+# Note, this may replace "regular" names too, as technically we can only tell
+# whether a name is definitely a regular name, or either a regular or no-key
+# name.  A directory will only contain one type of name at a time, though.
+_filter_nokey_filenames()
+{
+	local dir=$1
+
+	sed "s|${dir}${dir:+/}[A-Za-z0-9+,_]\+|${dir}${dir:+/}NOKEY_NAME|g"
+}
diff --git a/tests/generic/419 b/tests/generic/419
index 61f9d605..6be7865c 100755
--- a/tests/generic/419
+++ b/tests/generic/419
@@ -41,7 +41,7 @@ _scratch_cycle_mount
 # in a way that does not assume any particular filenames.
 efile1=$(find $SCRATCH_MNT/edir -maxdepth 1 -type f | head -1)
 efile2=$(find $SCRATCH_MNT/edir -maxdepth 1 -type f | tail -1)
-mv $efile1 $efile2 |& _filter_scratch | sed 's|edir/[a-zA-Z0-9+,_]\+|edir/FILENAME|g'
+mv $efile1 $efile2 |& _filter_scratch | _filter_nokey_filenames edir
 $here/src/renameat2 -x $efile1 $efile2
 
 # success, all done
diff --git a/tests/generic/419.out b/tests/generic/419.out
index 1bba10fd..67bb7fb1 100644
--- a/tests/generic/419.out
+++ b/tests/generic/419.out
@@ -1,3 +1,3 @@
 QA output created by 419
-mv: cannot move 'SCRATCH_MNT/edir/FILENAME' to 'SCRATCH_MNT/edir/FILENAME': Required key not available
+mv: cannot move 'SCRATCH_MNT/edir/NOKEY_NAME' to 'SCRATCH_MNT/edir/NOKEY_NAME': Required key not available
 Required key not available
diff --git a/tests/generic/429 b/tests/generic/429
index ba2281c9..558e93ca 100755
--- a/tests/generic/429
+++ b/tests/generic/429
@@ -52,18 +52,13 @@ _set_encpolicy $SCRATCH_MNT/edir $keydesc
 echo contents_@@@ > $SCRATCH_MNT/edir/@@@ # not valid base64
 echo contents_abcd > $SCRATCH_MNT/edir/abcd # valid base64
 
-filter_nokey_filenames()
-{
-	_filter_scratch | sed 's|edir/[a-zA-Z0-9+,_]\+|edir/NOKEY_NAME|g'
-}
-
 show_file_contents()
 {
 	echo "--- Contents of files using plaintext names:"
 	cat $SCRATCH_MNT/edir/@@@ |& _filter_scratch
 	cat $SCRATCH_MNT/edir/abcd |& _filter_scratch
 	echo "--- Contents of files using no-key names:"
-	cat ${nokey_names[@]} |& filter_nokey_filenames
+	cat ${nokey_names[@]} |& _filter_scratch | _filter_nokey_filenames edir
 }
 
 show_directory_with_key()
@@ -83,7 +78,8 @@ _unlink_session_encryption_key $keydesc
 _scratch_cycle_mount
 echo "--- Directory listing:"
 nokey_names=( $(find $SCRATCH_MNT/edir -mindepth 1 | sort) )
-printf '%s\n' "${nokey_names[@]}" | filter_nokey_filenames
+printf '%s\n' "${nokey_names[@]}" | \
+	_filter_scratch | _filter_nokey_filenames edir
 show_file_contents
 
 # Without remounting or dropping caches, add the encryption key and view the
-- 
2.32.0

[PATCH 3/3] common/encrypt: accept '-' character in no-key names

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add the '-' character to the regex that generic/{419,429} use to match
no-key filenames.  This is needed to prevent these tests from failing
after the kernel is changed to use a more standard variant of Base64
(https://lkml.kernel.org/r/20210718000125.59701-1-ebiggers@kernel.org).

Note that despite breaking these tests, the kernel change is not
expected to break any real users, as the fscrypt no-key name encoding
has always been considered an implementation detail.  So it is
appropriate to just update these tests.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 common/encrypt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/encrypt b/common/encrypt
index 766a6d81..f90c4ef0 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -935,5 +935,8 @@ _filter_nokey_filenames()
 {
 	local dir=$1
 
-	sed "s|${dir}${dir:+/}[A-Za-z0-9+,_]\+|${dir}${dir:+/}NOKEY_NAME|g"
+	# The no-key name format is a filesystem implementation detail that has
+	# varied slightly over time.  Just look for names that consist entirely
+	# of characters that have ever been used in such names.
+	sed "s|${dir}${dir:+/}[A-Za-z0-9+,_-]\+|${dir}${dir:+/}NOKEY_NAME|g"
 }
-- 
2.32.0