[PATCH] Resize configuration to match MD5 for bg_setenv tests

[Earl Chew]

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 <earl...@yahoo.com>
---
.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
+
- 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
- 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 $?
+
+ 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*.* ]]
}
--
2.39.1

[Jan Kiszka]

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 <earl...@yahoo.com>
> ---
> .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?

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?

Jan

--
Siemens AG, Technology
Linux Expert Center

[Earl Chew]

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

[Jan Kiszka]

On 14.08.23 16:32, 'Earl Chew' via EFI Boot Guard wrote:
> 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.

Ok, now I see. But then why not use the defaultsize from configure.ac as
targetsize?

>
> 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?

Makes sense now. I'm fine with extracting from config.h, but please
provide a hint if that file is not found, rather than likely exploding.
So far, it was fine to run the tests outside of the build folder, and
people may be surprised if things silently fail now.

[Earl Chew]

Jan,

>> The check against configure.ac is to ensure that the MD5 checksum
>> computed against ENV_MEM_USERVARS=131072 is plausibly valid.
>
> Ok, now I see. But then why not use the defaultsize from configure.ac as
> targetsize?

Hmm ... yes, that's possible. The main downside that I see is that
the golden MD5 checksums are tied to not only the content, but also
the size of the uservars area.

I thought having the 131072 specified explicitly in the helps make that
relationship more obvious.

What would you prefer?

> Makes sense now. I'm fine with extracting from config.h, but please
> provide a hint if that file is not found, rather than likely exploding.

All right.

So you mean, provide a hint, and fail that step of the test, right?

Earl

[Jan Kiszka]

On 15.08.23 17:36, 'Earl Chew' via EFI Boot Guard wrote:
> Jan,
>

>>> The check against configure.ac is to ensure that the MD5 checksum
>>> computed against ENV_MEM_USERVARS=131072 is plausibly valid.
>>
>> Ok, now I see. But then why not use the defaultsize from configure.ac as
>> targetsize?
>
> Hmm ... yes, that's possible. The main downside that I see is that
> the golden MD5 checksums are tied to not only the content, but also
> the size of the uservars area.
>
> I thought having the 131072 specified explicitly in the helps make that
> relationship more obvious.
>
> What would you prefer?

OK, a larger default size would require to re-calculate the CRC to that.
That will indeed make things even more complicated. Then let's keep
things as you suggested.

>
>> Makes sense now. I'm fine with extracting from config.h, but please
>> provide a hint if that file is not found, rather than likely exploding.
>
> All right.
>
> So you mean, provide a hint, and fail that step of the test, right?

Yes. To my understanding, we will get a weird behaviour right now if
config.h is not found.

[Earl Chew]

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 <earl...@yahoo.com>
---

.github/workflows/main.yaml | 4 +-
tests/bg_setenv.bats | 78 +++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 38c2b66..23ec0e0 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -44,7 +44,7 @@ jobs:
sudo apt-get update
sudo apt-get install --no-install-recommends \
autoconf-archive gcc-multilib gnu-efi libz-dev libpci-dev check \
- bats
+ bats libarchive-zip-perl
- name: Install i386 dependencies

if: ${{ matrix.target == 'i386' }}
run: |

@@ -126,8 +126,8 @@ jobs:

../configure
make check -j $(nproc)
sudo make install

+ time bats --tap ../tests
popd >/dev/null
- time bats --tap tests

- name: Build i386
if: ${{ matrix.target == 'i386' }}
run: |
diff --git a/tests/bg_setenv.bats b/tests/bg_setenv.bats

index 6d0249b..9d8ae16 100755
--- a/tests/bg_setenv.bats
+++ b/tests/bg_setenv.bats
@@ -34,6 +34,76 @@ create_sample_bgenv() {

+ grep -qF "ENV_MEM_USERVARS=$targetsize" "$BATS_TEST_DIRNAME"/../configure.ac || return $?

+
+ local envsize
+ envsize=$(

+ awk '/ ENV_MEM_USERVARS / { print $3 }' config.h || {
+ echo Expected to find config.h in $PWD >&2
+ exit 1
+ }
+ ) || return $?
+

@@ -51,6 +121,8 @@ ustate: 0 (OK)

user variables:
foo = bar" ]]

+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^6ad1dd1d98209a03d7b4fc2d2f16f9ec\s*.* ]]
}

@@ -62,6 +134,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 +169,8 @@ ustate: 0 (OK)

user variables:" ]]

+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^15bc40c9feae99cc879cfc55e0132caa\s*.* ]]
}

@@ -118,6 +194,8 @@ ustate: 0 (OK)

user variables:
foo = bar" ]]

+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^a24b154a48e1f33b79b87e0fa5eff8a1\s*.* ]]
}

--
2.39.1

[Jan Kiszka]

Hmm, did you test again? Or am I doing something wrong?

# ./configure --with-mem-uservars=262144
# make
# bats tests/
bg_setenv.bats
✓ ensure BGENV.DAT backwards compatbility
✗ create an empty BGENV.DAT
(in test file tests/bg_setenv.bats, line 150)
`user variables:" ]]' failed
-rw-r--r-- 1 jan users 132100 Aug 16 14:54 /tmp/bats-run-OIAll8/test/2/BGENV.DAT
668966655
\xff\x9e\xdf\x27
✓ modify BGENV, discard existing values
✓ modify BGENV, preserve existing values
✓ bg_printenv ustate
✓ bg_printenv with all fields is the same as omitting fields
✓ bg_printenv ustate raw
✓ bg_printenv multiple fields raw

8 tests, 1 failure

[Earl Chew]

Jan,

> Hmm, did you test again? Or am I doing something wrong?

I did test:

> $ ../../../local/bin/bats tests

> ✓ ensure BGENV.DAT backwards compatbility
> ✓ create an empty BGENV.DAT

> ✓ modify BGENV, discard existing values
> ✓ modify BGENV, preserve existing values
> ✓ bg_printenv ustate
> ✓ bg_printenv with all fields is the same as omitting fields
> ✓ bg_printenv ustate raw
> ✓ bg_printenv multiple fields raw
>

> 8 tests, 0 failures

The problem is my test configuration uses a smaller uservars size,
and I failed to check with a larger uservars size.

I've corrected the problem in v3 of the patch.

Earl

[Earl Chew]

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 <earl...@yahoo.com>
---
.github/workflows/main.yaml | 4 +-

tests/bg_setenv.bats | 83 +++++++++++++++++++++++++++++++++++--
2 files changed, 82 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/main.yaml b/.github/workflows/main.yaml
index 38c2b66..23ec0e0 100644
--- a/.github/workflows/main.yaml
+++ b/.github/workflows/main.yaml
@@ -44,7 +44,7 @@ jobs:
sudo apt-get update
sudo apt-get install --no-install-recommends \
autoconf-archive gcc-multilib gnu-efi libz-dev libpci-dev check \
- bats
+ bats libarchive-zip-perl
- name: Install i386 dependencies
if: ${{ matrix.target == 'i386' }}
run: |
@@ -126,8 +126,8 @@ jobs:
../configure
make check -j $(nproc)
sudo make install
+ time bats --tap ../tests
popd >/dev/null
- time bats --tap tests
- name: Build i386
if: ${{ matrix.target == 'i386' }}
run: |
diff --git a/tests/bg_setenv.bats b/tests/bg_setenv.bats

index 6d0249b..38cf03f 100755
--- a/tests/bg_setenv.bats
+++ b/tests/bg_setenv.bats
@@ -34,6 +34,75 @@ create_sample_bgenv() {

+ envsize=$(awk '/ ENV_MEM_USERVARS / { print $3 }' config.h) &&
+ [ -n "$envsize" ] || {
+ echo Expected to find ENV_MEM_USERVARS in $PWD/config.h >&2

+ exit 1
+ }
+

@@ -51,6 +120,8 @@ ustate: 0 (OK)

user variables:
foo = bar" ]]

+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^6ad1dd1d98209a03d7b4fc2d2f16f9ec\s*.* ]]
}

@@ -62,9 +133,6 @@ foo = bar" ]]

run bg_setenv -f "$envfile"
[[ "$output" = "Output written to $envfile." ]]

- run md5sum "$envfile"
- [[ "$output" =~ ^441b49e907a117d2fe1dc1d69d8ea1b0\s*.* ]]
-
run bg_printenv -f "$envfile"
[[ "$output" = "Values:
in_progress: no
@@ -75,6 +143,11 @@ watchdog timeout: 0 seconds

ustate: 0 (OK)

user variables:" ]]
+

+ resize_envfile "$envfile" 131072
+

+ run md5sum "$envfile"
+ [[ "$output" =~ ^441b49e907a117d2fe1dc1d69d8ea1b0\s*.* ]]
}

@test "modify BGENV, discard existing values" {
@@ -95,6 +168,8 @@ ustate: 0 (OK)

user variables:" ]]

+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^15bc40c9feae99cc879cfc55e0132caa\s*.* ]]
}

@@ -118,6 +193,8 @@ ustate: 0 (OK)

user variables:
foo = bar" ]]

+ resize_envfile "$envfile" 131072
+
run md5sum "$envfile"
[[ "$output" =~ ^a24b154a48e1f33b79b87e0fa5eff8a1\s*.* ]]
}

--
2.39.1

[Jan Kiszka]

Thanks, applied.

[JEMS EBERHARD HORBEL]

DIRECT SENDER IS HERE LETS DEAL.

JENS EBERHARD



MT103/202 DIRECT WIRE TRANSFER
PAYPAL TRANSFER
CASHAPP TRANSFER
ZELLE TRANSFER
TRANSFER WISE
WESTERN UNION TRANSFER
BITCOIN FLASHING 
BANK ACCOUNT LOADING/FLASHING
IBAN TO IBAN TRANSFER
MONEYGRAM TRANSFER
SLBC PROVIDER
CREDIT CARD TOP UP
SEPA TRANSFER
WIRE TRANSFER
GLOBALPAY INC US

Thanks.


NOTE; ONLY SERIOUS / RELIABLE RECEIVERS CAN CONTACT.

DM ME ON WHATSAPP FOR A SERIOUS DEAL.

+447405129573