Skip to contentSkip to conversation

generic/397: test renaming encrypted files without key

PATCH Superseded Eric Biggers 2 participants 3 messages Mar 14, 2017 – Mar 15, 2017

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 generic/397: test renaming encrypted files without key

From: Eric Biggers <[email protected]>

Update generic/397 to test another behavior when accessing encrypted files without the key: renames should be forbidden, even though they may be possible cryptographically. Test both a regular rename and a cross rename. (It happens that generic/398 also covers the cross rename case, but it's primarily for a different reason.)

Cc Theodore Ts'o <[email protected]>
Cc Jaegeuk Kim <[email protected]>
Cc Richard Weinberger <[email protected]>
---
 tests/generic/397     | 13 +++++++++++++
 tests/generic/397.out |  2 ++
 2 files changed, 15 insertions(+)
tests/generic/397 +13 -0
index 7077d048..0d3ab4c1 100755
@@ -46,6 +46,7 @@ _cleanup()
 . ./common/rc
 . ./common/filter
 . ./common/encrypt
+. ./common/renameat2
 
 # remove previous $seqres.full before test
 rm -f $seqres.full
@@ -56,6 +57,7 @@ _supported_os Linux
 _require_scratch_encryption
 _require_xfs_io_command "set_encpolicy"
 _require_command "$KEYCTL_PROG" keyctl
+_requires_renameat2
 
 _new_session_keyring
 
@@ -135,6 +137,17 @@ mkdir $SCRATCH_MNT/edir/0123456789abcdef |& filter_create_errors | _filter_scrat
 ln -s foo $SCRATCH_MNT/edir/newlink |& filter_create_errors | _filter_scratch
 ln -s foo $SCRATCH_MNT/edir/0123456789abcdef |& filter_create_errors | _filter_scratch
 
+# Try to rename files in the encrypted directory.  This should fail with ENOKEY.
+# As noted above, encrypted filenames are unpredictable, so this needs to be
+# written in a way that does not assume any particular filenames.
+# Regression test for:
+#	173b8439e1ba ("ext4: don't allow encrypted operations without keys")
+#	363fa4e078cb ("f2fs: don't allow encrypted operations without keys")
+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'
+src/renameat2 -x $efile1 $efile2
+
 # Delete the encrypted directory (should succeed)
 rm -r $SCRATCH_MNT/edir
 stat $SCRATCH_MNT/edir |& _filter_scratch
tests/generic/397.out +2 -0
index 2f55c5d6..3cf57aab 100644
@@ -10,4 +10,6 @@ mkdir: cannot create directory 'SCRATCH_MNT/edir/newdir': Required key not avail
 mkdir: cannot create directory 'SCRATCH_MNT/edir/0123456789abcdef': Required key not available
 ln: failed to create symbolic link 'SCRATCH_MNT/edir/newlink': Required key not available
 ln: failed to create symbolic link 'SCRATCH_MNT/edir/0123456789abcdef': Required key not available
+mv: cannot move 'SCRATCH_MNT/edir/FILENAME' to 'SCRATCH_MNT/edir/FILENAME': Required key not available
+Required key not available
 stat: cannot stat 'SCRATCH_MNT/edir': No such file or directory
Signature · 1 line
2.12.0.367.g23dc2f6d3c-goog
#1 lore raw
Quoted 8 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Update generic/397 to test another behavior when accessing encrypted files without the key: renames should be forbidden, even though they may be possible cryptographically. Test both a regular rename and a cross rename. (It happens that generic/398 also covers the cross rename case, but it's primarily for a different reason.)

Quoted 4 lines
Cc Theodore Ts'o <[email protected]>
Cc Jaegeuk Kim <[email protected]>
Cc Richard Weinberger <[email protected]>

We usually don't add new tests to existing tests, expecially targeted regression tests, this could make test start failing and make people think it's a new regression.

Would you mind writing a new case for it?

Thanks,
Eryu

#2 lore raw
Quoted 14 lines · Eryu Guan
Quoted 8 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Update generic/397 to test another behavior when accessing encrypted files without the key: renames should be forbidden, even though they may be possible cryptographically. Test both a regular rename and a cross rename. (It happens that generic/398 also covers the cross rename case, but it's primarily for a different reason.)

Quoted 4 lines
Cc Theodore Ts'o <[email protected]>
Cc Jaegeuk Kim <[email protected]>
Cc Richard Weinberger <[email protected]>
Quoted 8 lines

We usually don't add new tests to existing tests, expecially targeted regression tests, this could make test start failing and make people think it's a new regression.

Would you mind writing a new case for it?

Thanks,
Eryu

I felt that this fit well into the existing test, but yes I'll add a new one if that's preferable. Thanks!

- Eric