All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: fstests@vger.kernel.org
Cc: Andrey Albershteyn <aalbersh@redhat.com>, linux-fscrypt@vger.kernel.org
Subject: [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures
Date: Thu,  3 Nov 2022 23:47:40 -0700	[thread overview]
Message-ID: <20221104064742.167326-2-ebiggers@kernel.org> (raw)
In-Reply-To: <20221104064742.167326-1-ebiggers@kernel.org>

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


  reply	other threads:[~2022-11-04  6:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2022-11-04 18:42   ` [xfstests PATCH 1/3] common/verity: fix _fsv_have_hash_algorithm() with required signatures 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221104064742.167326-2-ebiggers@kernel.org \
    --to=ebiggers@kernel.org \
    --cc=aalbersh@redhat.com \
    --cc=fstests@vger.kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.