| Message ID | 20221211070704.341481-10-ebiggers@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfstests: update verity tests for non-4K block and page size | expand |
On Sat, Dec 10, 2022 at 11:07:02PM -0800, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > Instead of only testing 4K Merkle tree blocks, test FSV_BLOCK_SIZE, and > also other sizes if they happen to be supported. This allows this test > to run in cases where it couldn't before and improves test coverage in > cases where it did run before. > > This required reworking the test to compute the expected digests using > the 'fsverity digest' command, instead of relying on .out file > comparisons. (There isn't much downside to this, since comparison to > known-good file digests already happens in generic/575. So if both the > kernel and fsverity-utils were to be broken in the same way, generic/575 > would detect it. generic/624 serves a different purpose.) > > Signed-off-by: Eric Biggers <ebiggers@google.com> > --- > common/verity | 11 ++++ > tests/generic/624 | 119 ++++++++++++++++++++++++++++++------------ > tests/generic/624.out | 15 ++---- > 3 files changed, 101 insertions(+), 44 deletions(-) > > diff --git a/common/verity b/common/verity > index b88e839b..36a0f7d1 100644 > --- a/common/verity > +++ b/common/verity > @@ -263,6 +263,17 @@ _fsv_measure() > $FSVERITY_PROG measure "$@" | awk '{print $1}' > } > > +_fsv_digest() > +{ > + local args=("$@") > + # If the caller didn't explicitly specify a Merkle tree block size, then > + # use FSV_BLOCK_SIZE. > + if ! [[ " $*" =~ " --block-size" ]]; then > + args+=("--block-size=$FSV_BLOCK_SIZE") > + fi > + $FSVERITY_PROG digest "${args[@]}" | awk '{print $1}' > +} > + > _fsv_sign() > { > local args=("$@") > diff --git a/tests/generic/624 b/tests/generic/624 > index 7c447289..87a1e9d2 100755 > --- a/tests/generic/624 > +++ b/tests/generic/624 > @@ -24,48 +24,99 @@ _cleanup() > _supported_fs generic > _require_scratch_verity > _disable_fsverity_signatures > -# For the output of this test to always be the same, it has to use a specific > -# Merkle tree block size. > -if [ $FSV_BLOCK_SIZE != 4096 ]; then > - _notrun "4096-byte verity block size not supported on this platform" > -fi > +fsv_orig_file=$SCRATCH_MNT/file > +fsv_file=$SCRATCH_MNT/file.fsv > > _scratch_mkfs_verity &>> $seqres.full > _scratch_mount > - > -echo -e "\n# Creating a verity file" > -fsv_file=$SCRATCH_MNT/file > -# Always use the same file contents, so that the output of the test is always > -# the same. Also use a file that is large enough to have multiple Merkle tree > -# levels, so that the test verifies that the blocks are returned in the expected > -# order. A 1 MB file with SHA-256 and a Merkle tree block size of 4096 will > -# have 3 Merkle tree blocks (3*4096 bytes): two at level 0 and one at level 1. > -head -c 1000000 /dev/zero > $fsv_file > -merkle_tree_size=$((3 * FSV_BLOCK_SIZE)) > -fsverity_descriptor_size=256 > -_fsv_enable $fsv_file --salt=abcd > +_fsv_create_enable_file $fsv_file > _require_fsverity_dump_metadata $fsv_file > -_fsv_measure $fsv_file > > -echo -e "\n# Dumping Merkle tree" > -_fsv_dump_merkle_tree $fsv_file | sha256sum > +# Test FS_IOC_READ_VERITY_METADATA on a file that uses the given Merkle tree > +# block size. > +test_block_size() > +{ > + local block_size=$1 > + local digest_size=32 # assuming SHA-256 > + local i > + > + # Create the file. Make the file size big enough to result in multiple > + # Merkle tree levels being needed. The following expression computes a > + # file size that needs 2 blocks at level 0, and thus 1 block at level 1. > + local file_size=$((block_size * (2 * (block_size / digest_size)))) > + head -c $file_size /dev/zero > $fsv_orig_file > + local tree_params=("--salt=abcd" "--block-size=$block_size") > + cp $fsv_orig_file $fsv_file > + _fsv_enable $fsv_file "${tree_params[@]}" > + > + # Use the 'fsverity digest' command to compute the expected Merkle tree, > + # descriptor, and file digest. > + # > + # Ideally we'd just hard-code expected values into the .out file and > + # echo the actual values. That doesn't quite work here, since all these > + # values depend on the Merkle tree block size, and the Merkle tree block > + # sizes that are supported (and thus get tested here) vary. Therefore, > + # we calculate the expected values in userspace with the help of > + # 'fsverity digest', then do explicit comparisons with them. This works > + # fine as long as fsverity-utils and the kernel don't get broken in the > + # same way, in which case generic/575 should detect the problem anyway. > + local expected_file_digest=$(_fsv_digest $fsv_orig_file \ > + "${tree_params[@]}" \ > + --out-merkle-tree=$tmp.merkle_tree.expected \ > + --out-descriptor=$tmp.descriptor.expected) > + local merkle_tree_size=$(_get_filesize $tmp.merkle_tree.expected) > + local descriptor_size=$(_get_filesize $tmp.descriptor.expected) > > -echo -e "\n# Dumping Merkle tree (in chunks)" > -# The above test may get the whole tree in one read, so also try reading it in > -# chunks. > -for (( i = 0; i < merkle_tree_size; i += 997 )); do > - _fsv_dump_merkle_tree $fsv_file --offset=$i --length=997 > -done | sha256sum > + # 'fsverity measure' should return the expected file digest. > + local actual_file_digest=$(_fsv_measure $fsv_file) > + if [ "$actual_file_digest" != "$expected_file_digest" ]; then > + echo "Measure returned $actual_file_digest but expected $expected_file_digest" > + fi > > -echo -e "\n# Dumping descriptor" > -# Note that the hash that is printed here should be the same hash that was > -# printed by _fsv_measure above. > -_fsv_dump_descriptor $fsv_file | sha256sum > + # Test dumping the Merkle tree. > + _fsv_dump_merkle_tree $fsv_file > $tmp.merkle_tree.actual > + if ! cmp $tmp.merkle_tree.expected $tmp.merkle_tree.actual; then > + echo "Dumped Merkle tree didn't match" > + fi > + > + # Test dumping the Merkle tree in chunks. > + for (( i = 0; i < merkle_tree_size; i += 997 )); do > + _fsv_dump_merkle_tree $fsv_file --offset=$i --length=997 > + done > $tmp.merkle_tree.actual > + if ! cmp $tmp.merkle_tree.expected $tmp.merkle_tree.actual; then > + echo "Dumped Merkle tree (in chunks) didn't match" > + fi > + > + # Test dumping the descriptor. > + _fsv_dump_descriptor $fsv_file > $tmp.descriptor.actual > + if ! cmp $tmp.descriptor.expected $tmp.descriptor.actual; then > + echo "Dumped descriptor didn't match" > + fi > + > + # Test dumping the descriptor in chunks. > + for (( i = 0; i < descriptor_size; i += 13 )); do > + _fsv_dump_descriptor $fsv_file --offset=$i --length=13 > + done > $tmp.descriptor.actual > + if ! cmp $tmp.descriptor.expected $tmp.descriptor.actual; then > + echo "Dumped descriptor (in chunks) didn't match" > + fi > +} > > -echo -e "\n# Dumping descriptor (in chunks)" > -for (( i = 0; i < fsverity_descriptor_size; i += 13 )); do > - _fsv_dump_descriptor $fsv_file --offset=$i --length=13 > -done | sha256sum > +# Always test FSV_BLOCK_SIZE. Also test some other block sizes if they happen > +# to be supported. > +_fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=FSV_BLOCK_SIZE" > +test_block_size $FSV_BLOCK_SIZE > +for block_size in 1024 4096 16384 65536; do > + _fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=$block_size" > + if (( block_size == FSV_BLOCK_SIZE )); then > + continue > + fi > + if ! _fsv_can_enable $fsv_file --block-size=$block_size; then > + echo "block_size=$block_size is unsupported" >> $seqres.full > + continue If a block size isn't supported, e.g. 1024. Then this case trys to skip that test, but it'll break golden image, due to the .out file contains each line of: Testing FS_IOC_READ_VERITY_METADATA with block_size=1024/4096/16384/65536 Do you expect that failure, or we shouldn't fail on that? Thanks, Zorro > + fi > + test_block_size $block_size > +done > > # success, all done > status=0 > diff --git a/tests/generic/624.out b/tests/generic/624.out > index 912826d3..97d5691a 100644 > --- a/tests/generic/624.out > +++ b/tests/generic/624.out > @@ -1,16 +1,11 @@ > QA output created by 624 > > -# Creating a verity file > -sha256:11e4f886bf2d70a6ef3a8b6ce8e8c62c9e5d3263208b9f120ae46791f124be73 > +# Testing FS_IOC_READ_VERITY_METADATA with block_size=FSV_BLOCK_SIZE > > -# Dumping Merkle tree > -db88cdad554734cd648a1bfbb5be7f86646c54397847aab0b3f42a28829fed17 - > +# Testing FS_IOC_READ_VERITY_METADATA with block_size=1024 > > -# Dumping Merkle tree (in chunks) > -db88cdad554734cd648a1bfbb5be7f86646c54397847aab0b3f42a28829fed17 - > +# Testing FS_IOC_READ_VERITY_METADATA with block_size=4096 > > -# Dumping descriptor > -11e4f886bf2d70a6ef3a8b6ce8e8c62c9e5d3263208b9f120ae46791f124be73 - > +# Testing FS_IOC_READ_VERITY_METADATA with block_size=16384 > > -# Dumping descriptor (in chunks) > -11e4f886bf2d70a6ef3a8b6ce8e8c62c9e5d3263208b9f120ae46791f124be73 - > +# Testing FS_IOC_READ_VERITY_METADATA with block_size=65536 > -- > 2.38.1 >
On Tue, Dec 20, 2022 at 02:56:04PM +0800, Zorro Lang wrote: > > +# Always test FSV_BLOCK_SIZE. Also test some other block sizes if they happen > > +# to be supported. > > +_fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=FSV_BLOCK_SIZE" > > +test_block_size $FSV_BLOCK_SIZE > > +for block_size in 1024 4096 16384 65536; do > > + _fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=$block_size" > > + if (( block_size == FSV_BLOCK_SIZE )); then > > + continue > > + fi > > + if ! _fsv_can_enable $fsv_file --block-size=$block_size; then > > + echo "block_size=$block_size is unsupported" >> $seqres.full > > + continue > > If a block size isn't supported, e.g. 1024. Then this case trys to skip that > test, but it'll break golden image, due to the .out file contains each line > of: > Testing FS_IOC_READ_VERITY_METADATA with block_size=1024/4096/16384/65536 > > Do you expect that failure, or we shouldn't fail on that? Actually it doesn't fail, since "Testing FS_IOC_READ_VERITY_METADATA with block_size=$block_size" is printed unconditionally, and "block_size=$block_size is unsupported" is only printed to $seqres.full. To avoid this confusion, how about I change "Testing FS_IOC_READ_VERITY_METADATA with block_size=$block_size" to "Testing block_size=$block_size if supported"? Or do you have another suggestion? - Eric
On Tue, Dec 20, 2022 at 12:00:37AM -0800, Eric Biggers wrote: > On Tue, Dec 20, 2022 at 02:56:04PM +0800, Zorro Lang wrote: > > > +# Always test FSV_BLOCK_SIZE. Also test some other block sizes if they happen > > > +# to be supported. > > > +_fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=FSV_BLOCK_SIZE" > > > +test_block_size $FSV_BLOCK_SIZE > > > +for block_size in 1024 4096 16384 65536; do > > > + _fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=$block_size" > > > + if (( block_size == FSV_BLOCK_SIZE )); then > > > + continue > > > + fi > > > + if ! _fsv_can_enable $fsv_file --block-size=$block_size; then > > > + echo "block_size=$block_size is unsupported" >> $seqres.full > > > + continue > > > > If a block size isn't supported, e.g. 1024. Then this case trys to skip that > > test, but it'll break golden image, due to the .out file contains each line > > of: > > Testing FS_IOC_READ_VERITY_METADATA with block_size=1024/4096/16384/65536 > > > > Do you expect that failure, or we shouldn't fail on that? > > Actually it doesn't fail, since "Testing FS_IOC_READ_VERITY_METADATA with > block_size=$block_size" is printed unconditionally, and > "block_size=$block_size is unsupported" is only printed to $seqres.full. Oh, you're right. > > To avoid this confusion, how about I change "Testing FS_IOC_READ_VERITY_METADATA > with block_size=$block_size" to "Testing block_size=$block_size if supported"? > Or do you have another suggestion? That's fine, I think current "output" is good to me. If there's not objection from linux-fscrypt@, I'll merge this patchset. Thanks for this update. Thanks, Zorro > > - Eric >
diff --git a/common/verity b/common/verity index b88e839b..36a0f7d1 100644 --- a/common/verity +++ b/common/verity @@ -263,6 +263,17 @@ _fsv_measure() $FSVERITY_PROG measure "$@" | awk '{print $1}' } +_fsv_digest() +{ + local args=("$@") + # If the caller didn't explicitly specify a Merkle tree block size, then + # use FSV_BLOCK_SIZE. + if ! [[ " $*" =~ " --block-size" ]]; then + args+=("--block-size=$FSV_BLOCK_SIZE") + fi + $FSVERITY_PROG digest "${args[@]}" | awk '{print $1}' +} + _fsv_sign() { local args=("$@") diff --git a/tests/generic/624 b/tests/generic/624 index 7c447289..87a1e9d2 100755 --- a/tests/generic/624 +++ b/tests/generic/624 @@ -24,48 +24,99 @@ _cleanup() _supported_fs generic _require_scratch_verity _disable_fsverity_signatures -# For the output of this test to always be the same, it has to use a specific -# Merkle tree block size. -if [ $FSV_BLOCK_SIZE != 4096 ]; then - _notrun "4096-byte verity block size not supported on this platform" -fi +fsv_orig_file=$SCRATCH_MNT/file +fsv_file=$SCRATCH_MNT/file.fsv _scratch_mkfs_verity &>> $seqres.full _scratch_mount - -echo -e "\n# Creating a verity file" -fsv_file=$SCRATCH_MNT/file -# Always use the same file contents, so that the output of the test is always -# the same. Also use a file that is large enough to have multiple Merkle tree -# levels, so that the test verifies that the blocks are returned in the expected -# order. A 1 MB file with SHA-256 and a Merkle tree block size of 4096 will -# have 3 Merkle tree blocks (3*4096 bytes): two at level 0 and one at level 1. -head -c 1000000 /dev/zero > $fsv_file -merkle_tree_size=$((3 * FSV_BLOCK_SIZE)) -fsverity_descriptor_size=256 -_fsv_enable $fsv_file --salt=abcd +_fsv_create_enable_file $fsv_file _require_fsverity_dump_metadata $fsv_file -_fsv_measure $fsv_file -echo -e "\n# Dumping Merkle tree" -_fsv_dump_merkle_tree $fsv_file | sha256sum +# Test FS_IOC_READ_VERITY_METADATA on a file that uses the given Merkle tree +# block size. +test_block_size() +{ + local block_size=$1 + local digest_size=32 # assuming SHA-256 + local i + + # Create the file. Make the file size big enough to result in multiple + # Merkle tree levels being needed. The following expression computes a + # file size that needs 2 blocks at level 0, and thus 1 block at level 1. + local file_size=$((block_size * (2 * (block_size / digest_size)))) + head -c $file_size /dev/zero > $fsv_orig_file + local tree_params=("--salt=abcd" "--block-size=$block_size") + cp $fsv_orig_file $fsv_file + _fsv_enable $fsv_file "${tree_params[@]}" + + # Use the 'fsverity digest' command to compute the expected Merkle tree, + # descriptor, and file digest. + # + # Ideally we'd just hard-code expected values into the .out file and + # echo the actual values. That doesn't quite work here, since all these + # values depend on the Merkle tree block size, and the Merkle tree block + # sizes that are supported (and thus get tested here) vary. Therefore, + # we calculate the expected values in userspace with the help of + # 'fsverity digest', then do explicit comparisons with them. This works + # fine as long as fsverity-utils and the kernel don't get broken in the + # same way, in which case generic/575 should detect the problem anyway. + local expected_file_digest=$(_fsv_digest $fsv_orig_file \ + "${tree_params[@]}" \ + --out-merkle-tree=$tmp.merkle_tree.expected \ + --out-descriptor=$tmp.descriptor.expected) + local merkle_tree_size=$(_get_filesize $tmp.merkle_tree.expected) + local descriptor_size=$(_get_filesize $tmp.descriptor.expected) -echo -e "\n# Dumping Merkle tree (in chunks)" -# The above test may get the whole tree in one read, so also try reading it in -# chunks. -for (( i = 0; i < merkle_tree_size; i += 997 )); do - _fsv_dump_merkle_tree $fsv_file --offset=$i --length=997 -done | sha256sum + # 'fsverity measure' should return the expected file digest. + local actual_file_digest=$(_fsv_measure $fsv_file) + if [ "$actual_file_digest" != "$expected_file_digest" ]; then + echo "Measure returned $actual_file_digest but expected $expected_file_digest" + fi -echo -e "\n# Dumping descriptor" -# Note that the hash that is printed here should be the same hash that was -# printed by _fsv_measure above. -_fsv_dump_descriptor $fsv_file | sha256sum + # Test dumping the Merkle tree. + _fsv_dump_merkle_tree $fsv_file > $tmp.merkle_tree.actual + if ! cmp $tmp.merkle_tree.expected $tmp.merkle_tree.actual; then + echo "Dumped Merkle tree didn't match" + fi + + # Test dumping the Merkle tree in chunks. + for (( i = 0; i < merkle_tree_size; i += 997 )); do + _fsv_dump_merkle_tree $fsv_file --offset=$i --length=997 + done > $tmp.merkle_tree.actual + if ! cmp $tmp.merkle_tree.expected $tmp.merkle_tree.actual; then + echo "Dumped Merkle tree (in chunks) didn't match" + fi + + # Test dumping the descriptor. + _fsv_dump_descriptor $fsv_file > $tmp.descriptor.actual + if ! cmp $tmp.descriptor.expected $tmp.descriptor.actual; then + echo "Dumped descriptor didn't match" + fi + + # Test dumping the descriptor in chunks. + for (( i = 0; i < descriptor_size; i += 13 )); do + _fsv_dump_descriptor $fsv_file --offset=$i --length=13 + done > $tmp.descriptor.actual + if ! cmp $tmp.descriptor.expected $tmp.descriptor.actual; then + echo "Dumped descriptor (in chunks) didn't match" + fi +} -echo -e "\n# Dumping descriptor (in chunks)" -for (( i = 0; i < fsverity_descriptor_size; i += 13 )); do - _fsv_dump_descriptor $fsv_file --offset=$i --length=13 -done | sha256sum +# Always test FSV_BLOCK_SIZE. Also test some other block sizes if they happen +# to be supported. +_fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=FSV_BLOCK_SIZE" +test_block_size $FSV_BLOCK_SIZE +for block_size in 1024 4096 16384 65536; do + _fsv_scratch_begin_subtest "Testing FS_IOC_READ_VERITY_METADATA with block_size=$block_size" + if (( block_size == FSV_BLOCK_SIZE )); then + continue + fi + if ! _fsv_can_enable $fsv_file --block-size=$block_size; then + echo "block_size=$block_size is unsupported" >> $seqres.full + continue + fi + test_block_size $block_size +done # success, all done status=0 diff --git a/tests/generic/624.out b/tests/generic/624.out index 912826d3..97d5691a 100644 --- a/tests/generic/624.out +++ b/tests/generic/624.out @@ -1,16 +1,11 @@ QA output created by 624 -# Creating a verity file -sha256:11e4f886bf2d70a6ef3a8b6ce8e8c62c9e5d3263208b9f120ae46791f124be73 +# Testing FS_IOC_READ_VERITY_METADATA with block_size=FSV_BLOCK_SIZE -# Dumping Merkle tree -db88cdad554734cd648a1bfbb5be7f86646c54397847aab0b3f42a28829fed17 - +# Testing FS_IOC_READ_VERITY_METADATA with block_size=1024 -# Dumping Merkle tree (in chunks) -db88cdad554734cd648a1bfbb5be7f86646c54397847aab0b3f42a28829fed17 - +# Testing FS_IOC_READ_VERITY_METADATA with block_size=4096 -# Dumping descriptor -11e4f886bf2d70a6ef3a8b6ce8e8c62c9e5d3263208b9f120ae46791f124be73 - +# Testing FS_IOC_READ_VERITY_METADATA with block_size=16384 -# Dumping descriptor (in chunks) -11e4f886bf2d70a6ef3a8b6ce8e8c62c9e5d3263208b9f120ae46791f124be73 - +# Testing FS_IOC_READ_VERITY_METADATA with block_size=65536