* [xfstests PATCH 0/3] Fix test bugs related to fs.verity.require_signatures
@ 2022-11-04 6:47 Eric Biggers
2022-11-04 6:47 ` [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures Eric Biggers
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Eric Biggers @ 2022-11-04 6:47 UTC (permalink / raw)
To: fstests; +Cc: Andrey Albershteyn, linux-fscrypt
This is a replacement for Andrey's patchset
"generic/577: fix hash check and add metadata cleaning".
It handles some other things that were overlooked.
Eric Biggers (3):
common/verity: fix _fsv_have_hash_algorithm() with required signatures
generic/577: add missing file removal before empty file test
tests: fix some tests for systems with fs.verity.require_signatures=1
common/verity | 69 +++++++++++++++++++++++++++++++----------------
tests/btrfs/290 | 9 +++++++
tests/btrfs/291 | 2 ++
tests/generic/577 | 1 +
tests/generic/624 | 8 ++++++
tests/generic/692 | 8 ++++++
6 files changed, 74 insertions(+), 23 deletions(-)
base-commit: a75c5f50584e03ca7862ad51f48efd2d524d1dc5
--
2.38.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures
2022-11-04 6:47 [xfstests PATCH 0/3] Fix test bugs related to fs.verity.require_signatures Eric Biggers
@ 2022-11-04 6:47 ` Eric Biggers
2022-11-04 18:42 ` Andrey Albershteyn
2022-11-04 6:47 ` [xfstests PATCH 2/3] generic/577: add missing file removal before empty file test Eric Biggers
2022-11-04 6:47 ` [xfstests PATCH 3/3] tests: fix some tests for systems with fs.verity.require_signatures=1 Eric Biggers
2 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2022-11-04 6:47 UTC (permalink / raw)
To: fstests; +Cc: Andrey Albershteyn, linux-fscrypt
From: Eric Biggers <ebiggers@google.com>
_fsv_have_hash_algorithm() uses _fsv_enable() without a signature, so it
always fails when called while fs.verity.require_signatures=1. This
happens in generic/577, which tests file signing. This wasn't noticed
because it just made part of generic/577 always be skipped.
Fix this by making _fsv_have_hash_algorithm() temporarily set
fs.verity.require_signatures to 0.
Since the previous value needs to be restored afterwards, whether it is
0 or 1, also make some changes to the fs.verity.require_signatures
helper functions to allow the restoration of the previous value, rather
than the value that existed at the beginning of the test.
Finally, make a couple related cleanups: make _fsv_have_hash_algorithm()
always delete the file it works with, and also update the similar code
in _require_scratch_verity().
Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
common/verity | 69 ++++++++++++++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 23 deletions(-)
diff --git a/common/verity b/common/verity
index 65a39d3e..4a9c9872 100644
--- a/common/verity
+++ b/common/verity
@@ -44,12 +44,13 @@ _require_scratch_verity()
# doesn't work on ext3-style filesystems. So, try actually using it.
echo foo > $SCRATCH_MNT/tmpfile
_disable_fsverity_signatures
- if ! _fsv_enable $SCRATCH_MNT/tmpfile; then
- _restore_fsverity_signatures
+ _fsv_enable $SCRATCH_MNT/tmpfile
+ local status=$?
+ _restore_prev_fsverity_signatures
+ rm -f $SCRATCH_MNT/tmpfile
+ if (( $status != 0 )); then
_notrun "$FSTYP verity isn't usable by default with these mkfs options"
fi
- _restore_fsverity_signatures
- rm -f $SCRATCH_MNT/tmpfile
_scratch_unmount
@@ -104,30 +105,52 @@ _fsv_load_cert()
# Disable mandatory signatures for fs-verity files, if they are supported.
_disable_fsverity_signatures()
{
- if [ -e /proc/sys/fs/verity/require_signatures ]; then
- if [ -z "$FSVERITY_SIG_CTL_ORIG" ]; then
- FSVERITY_SIG_CTL_ORIG=$(</proc/sys/fs/verity/require_signatures)
- fi
- echo 0 > /proc/sys/fs/verity/require_signatures
- fi
+ _set_fsverity_require_signatures 0
}
# Enable mandatory signatures for fs-verity files.
# This assumes that _require_fsverity_builtin_signatures() was called.
_enable_fsverity_signatures()
{
- if [ -z "$FSVERITY_SIG_CTL_ORIG" ]; then
- FSVERITY_SIG_CTL_ORIG=$(</proc/sys/fs/verity/require_signatures)
- fi
- echo 1 > /proc/sys/fs/verity/require_signatures
+ _set_fsverity_require_signatures 1
}
-# Restore the original signature verification setting.
+# Restore the original value of fs.verity.require_signatures, i.e. the value it
+# had at the beginning of the test.
_restore_fsverity_signatures()
{
- if [ -n "$FSVERITY_SIG_CTL_ORIG" ]; then
- echo "$FSVERITY_SIG_CTL_ORIG" > /proc/sys/fs/verity/require_signatures
- fi
+ if [ -n "$FSVERITY_SIG_CTL_ORIG" ]; then
+ _set_fsverity_require_signatures "$FSVERITY_SIG_CTL_ORIG"
+ fi
+}
+
+# Restore the previous value of fs.verity.require_signatures, i.e. the value it
+# had just before it was last written to.
+_restore_prev_fsverity_signatures()
+{
+ if [ -n "$FSVERITY_SIG_CTL_PREV" ]; then
+ _set_fsverity_require_signatures "$FSVERITY_SIG_CTL_PREV"
+ fi
+}
+
+_set_fsverity_require_signatures()
+{
+ local newval=$1
+ if [ ! -e /proc/sys/fs/verity/require_signatures ]; then
+ # If the kernel doesn't support fs.verity.require_signatures,
+ # then trying to disable it is fine, but enabling it is not.
+ if [ "$newval" != 0 ]; then
+ # Forgot to call _require_fsverity_builtin_signatures().
+ _fail "fs.verity.require_signatures is missing"
+ fi
+ return
+ fi
+ local oldval=$(</proc/sys/fs/verity/require_signatures)
+ FSVERITY_SIG_CTL_PREV=$oldval
+ if [ -z "$FSVERITY_SIG_CTL_ORIG" ]; then
+ FSVERITY_SIG_CTL_ORIG=$oldval
+ fi
+ echo "$newval" > /proc/sys/fs/verity/require_signatures
}
# Require userspace and kernel support for 'fsverity dump_metadata'.
@@ -245,14 +268,14 @@ _fsv_have_hash_algorithm()
local hash_alg=$1
local test_file=$2
+ _disable_fsverity_signatures
rm -f $test_file
head -c 4096 /dev/zero > $test_file
- if ! _fsv_enable --hash-alg=$hash_alg $test_file &>> $seqres.full; then
- # no kernel support
- return 1
- fi
+ _fsv_enable --hash-alg=$hash_alg $test_file &>> $seqres.full
+ local status=$?
+ _restore_prev_fsverity_signatures
rm -f $test_file
- return 0
+ return $status
}
#
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [xfstests PATCH 2/3] generic/577: add missing file removal before empty file test
2022-11-04 6:47 [xfstests PATCH 0/3] Fix test bugs related to fs.verity.require_signatures Eric Biggers
2022-11-04 6:47 ` [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures Eric Biggers
@ 2022-11-04 6:47 ` Eric Biggers
2022-11-04 18:44 ` Andrey Albershteyn
2022-11-04 6:47 ` [xfstests PATCH 3/3] tests: fix some tests for systems with fs.verity.require_signatures=1 Eric Biggers
2 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2022-11-04 6:47 UTC (permalink / raw)
To: fstests; +Cc: Andrey Albershteyn, linux-fscrypt
From: Eric Biggers <ebiggers@google.com>
The fix for _fsv_have_hash_algorithm() exposed a bug where one of the
test cases in generic/577 isn't deleting the file from the previous test
case before it tries to write to it. That causes a failure, since due
to the fix for _fsv_have_hash_algorithm(), the file from the previous
test case now ends up with verity enabled and therefore cannot be
written to. Fix this by deleting the file.
Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
tests/generic/577 | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/generic/577 b/tests/generic/577
index 98c3888f..5f7e0573 100755
--- a/tests/generic/577
+++ b/tests/generic/577
@@ -121,6 +121,7 @@ if _fsv_have_hash_algorithm sha512 $fsv_file; then
fi
echo -e "\n# Testing empty file"
+rm -f $fsv_file
echo -n > $fsv_file
_fsv_sign $fsv_file $sigfile.emptyfile --key=$keyfile --cert=$certfile | \
_filter_scratch
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [xfstests PATCH 3/3] tests: fix some tests for systems with fs.verity.require_signatures=1
2022-11-04 6:47 [xfstests PATCH 0/3] Fix test bugs related to fs.verity.require_signatures Eric Biggers
2022-11-04 6:47 ` [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures Eric Biggers
2022-11-04 6:47 ` [xfstests PATCH 2/3] generic/577: add missing file removal before empty file test Eric Biggers
@ 2022-11-04 6:47 ` Eric Biggers
2022-11-04 18:48 ` Andrey Albershteyn
2 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2022-11-04 6:47 UTC (permalink / raw)
To: fstests; +Cc: Andrey Albershteyn, linux-fscrypt
From: Eric Biggers <ebiggers@google.com>
Some of the newer verity tests don't work properly on systems where
fs.verity.require_signatures is enabled, either because they forget to
disable it at the beginning of the test, or they forget to re-enable it
afterwards, or both. Fix this.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
tests/btrfs/290 | 9 +++++++++
tests/btrfs/291 | 2 ++
tests/generic/624 | 8 ++++++++
tests/generic/692 | 8 ++++++++
4 files changed, 27 insertions(+)
diff --git a/tests/btrfs/290 b/tests/btrfs/290
index b7254c5e..06a58f47 100755
--- a/tests/btrfs/290
+++ b/tests/btrfs/290
@@ -15,6 +15,14 @@ _begin_fstest auto quick verity
. ./common/filter
. ./common/verity
+# Override the default cleanup function.
+_cleanup()
+{
+ cd /
+ _restore_fsverity_signatures
+ rm -f $tmp.*
+}
+
# real QA test starts here
_supported_fs btrfs
_require_scratch_verity
@@ -24,6 +32,7 @@ _require_xfs_io_command "falloc"
_require_xfs_io_command "pread"
_require_xfs_io_command "pwrite"
_require_btrfs_corrupt_block
+_disable_fsverity_signatures
get_ino() {
local file=$1
diff --git a/tests/btrfs/291 b/tests/btrfs/291
index bbdd183d..c5947133 100755
--- a/tests/btrfs/291
+++ b/tests/btrfs/291
@@ -23,6 +23,7 @@ _cleanup()
rm -f $img
$LVM_PROG vgremove -f -y $vgname >>$seqres.full 2>&1
losetup -d $loop_dev >>$seqres.full 2>&1
+ _restore_fsverity_signatures
}
# Import common functions.
@@ -43,6 +44,7 @@ _require_command $LVM_PROG lvm
_require_scratch_verity
_require_btrfs_command inspect-internal dump-tree
_require_test_program "log-writes/replay-log"
+_disable_fsverity_signatures
sync_loop() {
i=$1
diff --git a/tests/generic/624 b/tests/generic/624
index 89fbf256..7c447289 100755
--- a/tests/generic/624
+++ b/tests/generic/624
@@ -10,6 +10,14 @@
. ./common/preamble
_begin_fstest auto quick verity
+# Override the default cleanup function.
+_cleanup()
+{
+ cd /
+ _restore_fsverity_signatures
+ rm -f $tmp.*
+}
+
. ./common/filter
. ./common/verity
diff --git a/tests/generic/692 b/tests/generic/692
index 0bb1fd33..d6da734b 100644
--- a/tests/generic/692
+++ b/tests/generic/692
@@ -15,6 +15,13 @@
. ./common/preamble
_begin_fstest auto quick verity
+# Override the default cleanup function.
+_cleanup()
+{
+ cd /
+ _restore_fsverity_signatures
+ rm -f $tmp.*
+}
# Import common functions.
. ./common/filter
@@ -26,6 +33,7 @@ _require_test
_require_math
_require_scratch_verity
_require_fsverity_max_file_size_limit
+_disable_fsverity_signatures
_scratch_mkfs_verity &>> $seqres.full
_scratch_mount
--
2.38.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures
2022-11-04 6:47 ` [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures Eric Biggers
@ 2022-11-04 18:42 ` Andrey Albershteyn
0 siblings, 0 replies; 7+ messages in thread
From: Andrey Albershteyn @ 2022-11-04 18:42 UTC (permalink / raw)
To: Eric Biggers; +Cc: fstests
On Thu, Nov 03, 2022 at 11:47:40PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> _fsv_have_hash_algorithm() uses _fsv_enable() without a signature, so it
> always fails when called while fs.verity.require_signatures=1. This
> happens in generic/577, which tests file signing. This wasn't noticed
> because it just made part of generic/577 always be skipped.
>
> Fix this by making _fsv_have_hash_algorithm() temporarily set
> fs.verity.require_signatures to 0.
>
> Since the previous value needs to be restored afterwards, whether it is
> 0 or 1, also make some changes to the fs.verity.require_signatures
> helper functions to allow the restoration of the previous value, rather
> than the value that existed at the beginning of the test.
>
> Finally, make a couple related cleanups: make _fsv_have_hash_algorithm()
> always delete the file it works with, and also update the similar code
> in _require_scratch_verity().
>
> Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> common/verity | 69 ++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 46 insertions(+), 23 deletions(-)
>
Looks good.
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
--
- Andrey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [xfstests PATCH 2/3] generic/577: add missing file removal before empty file test
2022-11-04 6:47 ` [xfstests PATCH 2/3] generic/577: add missing file removal before empty file test Eric Biggers
@ 2022-11-04 18:44 ` Andrey Albershteyn
0 siblings, 0 replies; 7+ messages in thread
From: Andrey Albershteyn @ 2022-11-04 18:44 UTC (permalink / raw)
To: Eric Biggers; +Cc: fstests
On Thu, Nov 03, 2022 at 11:47:41PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> The fix for _fsv_have_hash_algorithm() exposed a bug where one of the
> test cases in generic/577 isn't deleting the file from the previous test
> case before it tries to write to it. That causes a failure, since due
> to the fix for _fsv_have_hash_algorithm(), the file from the previous
> test case now ends up with verity enabled and therefore cannot be
> written to. Fix this by deleting the file.
>
> Reported-by: Andrey Albershteyn <aalbersh@redhat.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> tests/generic/577 | 1 +
> 1 file changed, 1 insertion(+)
>
Looks good.
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
--
- Andrey
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [xfstests PATCH 3/3] tests: fix some tests for systems with fs.verity.require_signatures=1
2022-11-04 6:47 ` [xfstests PATCH 3/3] tests: fix some tests for systems with fs.verity.require_signatures=1 Eric Biggers
@ 2022-11-04 18:48 ` Andrey Albershteyn
0 siblings, 0 replies; 7+ messages in thread
From: Andrey Albershteyn @ 2022-11-04 18:48 UTC (permalink / raw)
To: Eric Biggers; +Cc: fstests
On Thu, Nov 03, 2022 at 11:47:42PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> Some of the newer verity tests don't work properly on systems where
> fs.verity.require_signatures is enabled, either because they forget to
> disable it at the beginning of the test, or they forget to re-enable it
> afterwards, or both. Fix this.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> tests/btrfs/290 | 9 +++++++++
> tests/btrfs/291 | 2 ++
> tests/generic/624 | 8 ++++++++
> tests/generic/692 | 8 ++++++++
> 4 files changed, 27 insertions(+)
>
Looks good. I haven't run btrfs tests but change looks ok.
Reviewed-by: Andrey Albershteyn <aalbersh@redhat.com>
--
- Andrey
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-04 18:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04 6:47 [xfstests PATCH 0/3] Fix test bugs related to fs.verity.require_signatures Eric Biggers
2022-11-04 6:47 ` [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures Eric Biggers
2022-11-04 18:42 ` Andrey Albershteyn
2022-11-04 6:47 ` [xfstests PATCH 2/3] generic/577: add missing file removal before empty file test Eric Biggers
2022-11-04 18:44 ` Andrey Albershteyn
2022-11-04 6:47 ` [xfstests PATCH 3/3] tests: fix some tests for systems with fs.verity.require_signatures=1 Eric Biggers
2022-11-04 18:48 ` Andrey Albershteyn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.