Jan,
> Just augment the line that installs bats & Co.
All right.
> For which new statement do we need that dep? I'm not spotting it yet.
This is required to provide /usr/bin/crc32.
>> + time bats --tap ../tests
>> popd >/dev/null
>> - time bats --tap tests
>
> Why that?
The configuration artifacts are located in build/ which was created
as part of the github workflow, but user workflows are free to use
whatever directory they like or even build in the source tree.
See my reply below about configure.ac.
This change assumes that running the BATS test from the directory
containing the build artifacts is a reasonable thing to do.
>> + grep -qF "ENV_MEM_USERVARS=$targetsize"
"$BATS_TEST_DIRNAME"/../configure.ac || return $?
>
> You probably wanted to check config.h or config.log. The above will
> always bail out because the default is your targetsize. But that will
> mean having the path to the build dir still handy.
The check against configure.ac is to ensure that the MD5 checksum
computed against ENV_MEM_USERVARS=131072 is plausibly valid.
configure.ac: ENV_MEM_USERVARS=131072
Maybe renaming $targetsize to $defaultsize would be helpful.
The check against config.h that follows this is to determine the size
selected by the user at configuration time. This determines the actual
present size of the uservars array:
+ local envsize
+ envsize=$(awk '/ ENV_MEM_USERVARS / { print $3 }' config.h)
> How about checking the target file size against the $targetsize?
Given that the size intended by the user is extracted from config.h,
I think using that to confirm the present size would be preferred, since
the size should match the configured value.
Including such a test would make the BATS test a bit more brittle
since the file also includes other parts of the BG_ENVDATA structure.
Do you think that's acceptable?
Earl
On 2023-08-14 12:26 a.m., Jan Kiszka wrote:
On 14.08.23 08:18, 'Earl Chew' via EFI Boot Guard wrote:
Using --with-mem-uservars causes a mismatch with the precomputed
MD5 checksum. To support this use case, first verify the CRC32,
resize the configuration to match the default expected by the
precomputed MD5 checksum, then recompute a matching CRC32.
Signed-off-by: Earl Chew <[email protected]>
---
.github/workflows/main.yaml | 7 +++-
tests/bg_setenv.bats | 73 +++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 1 deletion(-)
diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 38c2b66..4b34ebe 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -114,6 +114,11 @@ jobs:
sudo apt-get update
sudo apt-get install --no-install-recommends cppcheck
+ - name: Install test dependencies
+ run: |
+ sudo apt-get update
+ sudo apt-get install --no-install-recommends libarchive-zip-perl
+
Just augment the line that installs bats & Co.
For which new statement do we need that dep? I'm not spotting it yet.
- name: Prepare build
run: |
autoreconf -fi
@@ -126,8 +131,8 @@ jobs:
../configure
make check -j $(nproc)
sudo make install
+ time bats --tap ../tests
popd >/dev/null
- time bats --tap tests
Why that?
- name: Build i386
if: ${{ matrix.target == 'i386' }}
run: |
diff --git a/tests/bg_setenv.bats b/tests/bg_setenv.bats
index 6d0249b..5802ea5 100755
--- a/tests/bg_setenv.bats
+++ b/tests/bg_setenv.bats
@@ -34,6 +34,71 @@ create_sample_bgenv() {
--revision=1
}
+verify_envfile() {
+ local envfile=$1 ; shift
+
+ local envsize
+ envsize=$(wc -c < "$envfile")
+
+ local stored
+ stored=$(od -tx4 -j $((envsize - 4)) < "$envfile" | awk 'NF>1 {print $2}')
+
+ local computed
+ computed=$(crc32 <(head -c $((envsize - 4)) < "$envfile"))
+
+ [ -n "$stored$computed" -a "$stored" = "$computed" ]
+}
+
+resize_envfile() {
+ local envfile=$1 ; shift
+ local targetsize=$1 ; shift
+
+ # Before resizing, verify that the contents are uncorrupted to
+ # avoid truncating the mismatching checksum.
+
+ verify_envfile "$envfile" || return $?
+
+ # Only resize if the target size matches the default size
+ # named in the configuration. This is primarily a sanity check
+ # because the default size is also encoded in the MD5 checksum
+ # validation.
+
+ grep -qF "ENV_MEM_USERVARS=$targetsize"
"$BATS_TEST_DIRNAME"/../configure.ac || return $?
You probably wanted to check config.h or config.log. The above will
always bail out because the default is your targetsize. But that will
mean having the path to the build dir still handy.
How about checking the target file size against the $targetsize?
+
+ local envsize
+ envsize=$(awk '/ ENV_MEM_USERVARS / { print $3 }' config.h)
+
+ # Compute the difference between the actual size, and the target
+ # size. This will be used to expand or contract the payload to match.
+
+ local deltasize=$((targetsize - envsize))
+ [[ $deltasize -lt 0 ]] || deltasize=+$deltasize
+
+ # Remove the existing checksum, and then adjust the length
+ # to reach the target sizes.
+
+ truncate -s -4 "$envfile" && truncate -s "$deltasize" "$envfile"
+
+ ls -l "$envfile" >&2
+
+ # Install a checksum that matches the content included or
+ # excluded to meet the target size.
+
+ local computed
+ computed=$(( 0x$(crc32 <(cat "$envfile") ) ))
+ echo "$computed" >&2
+
+ local byte0=$(printf %02x $(( (computed >> 0) & 0xff)) )
+ local byte1=$(printf %02x $(( (computed >> 8) & 0xff)) )
+ local byte2=$(printf %02x $(( (computed >> 16) & 0xff)) )
+ local byte3=$(printf %02x $(( (computed >> 24) & 0xff)) )
+
+ printf "%s" "\x$byte0\x$byte1\x$byte2\x$byte3" >&2
+ printf "%b" "\x$byte0\x$byte1\x$byte2\x$byte3" >> "$envfile"
+
+ verify_envfile "$envfile"
+}
+
@test "ensure BGENV.DAT backwards compatbility" {
local envfile
envfile="$BATS_TEST_TMPDIR/BGENV.DAT"
@@ -51,6 +116,8 @@ ustate: 0 (OK)
user variables:
foo = bar" ]]
+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^6ad1dd1d98209a03d7b4fc2d2f16f9ec\s*.* ]]
}
@@ -62,6 +129,8 @@ foo = bar" ]]
run bg_setenv -f "$envfile"
[[ "$output" = "Output written to $envfile." ]]
+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^441b49e907a117d2fe1dc1d69d8ea1b0\s*.* ]]
@@ -95,6 +164,8 @@ ustate: 0 (OK)
user variables:" ]]
+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^15bc40c9feae99cc879cfc55e0132caa\s*.* ]]
}
@@ -118,6 +189,8 @@ ustate: 0 (OK)
user variables:
foo = bar" ]]
+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^a24b154a48e1f33b79b87e0fa5eff8a1\s*.* ]]
}
Jan
--
You received this message because you are subscribed to the Google Groups "EFI Boot
Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/efibootguard-dev/6c6ddcd1-7af3-476b-94ce-c499b5fb93a8%40yahoo.com.
