On Mon, Feb 14, 2022 at 03:36:06PM +0900, AKASHI Takahiro wrote:
On Mon, Feb 14, 2022 at 08:18:03AM +0200, Ilias Apalodimas wrote:
On Mon, Feb 14, 2022 at 10:50:08AM +0900, AKASHI Takahiro wrote:
Ilias,
On Fri, Feb 11, 2022 at 09:37:50AM +0200, Ilias Apalodimas wrote:
The previous patch is changing U-Boot's behavior wrt certificate based binary authentication. Specifically an image who's digest of a certificate is found in dbx is now rejected. Fix the test accordingly and add another one testing signatures in reverse order
Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org
changes since RFC:
- Added another test cases checking signature hashes in reverse order
test/py/tests/test_efi_secboot/test_signed.py | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_efi_secboot/test_signed.py b/test/py/tests/test_efi_secboot/test_signed.py index 0aee34479f55..cc9396a11d48 100644 --- a/test/py/tests/test_efi_secboot/test_signed.py +++ b/test/py/tests/test_efi_secboot/test_signed.py @@ -186,7 +186,7 @@ class TestEfiSignedImage(object): assert 'Hello, world!' in ''.join(output)
with u_boot_console.log.section('Test Case 5c'):
# Test Case 5c, not rejected if one of signatures (digest of
# Test Case 5c, rejected if one of signatures (digest of # certificate) is revoked output = u_boot_console.run_command_list([ 'fatload host 0:1 4000000 dbx_hash.auth',@@ -195,7 +195,8 @@ class TestEfiSignedImage(object): output = u_boot_console.run_command_list([ 'efidebug boot next 1', 'efidebug test bootmgr'])
assert 'Hello, world!' in ''.join(output)
assert '\'HELLO\' failed' in ''.join(output)assert 'efi_start_image() returned: 26' in ''.join(output) with u_boot_console.log.section('Test Case 5d'): # Test Case 5d, rejected if both of signatures are revoked@@ -209,6 +210,31 @@ class TestEfiSignedImage(object): assert ''HELLO' failed' in ''.join(output) assert 'efi_start_image() returned: 26' in ''.join(output)
# Try rejection in reverse order."Reverse order" of what?
Of the test right above
Please specify the signature database, I guess "dbx"?
u_boot_console.restart_uboot()I don't think we need 'restart' here. I added it in each test function (not test case), IIRC, because we didn't have file-based non-volatile variables at that time.
You do. dbx already holds dbx_hash.auth and dbx1_hash.auth (in that order) at that point. The point is cleaning up dbx and testing against dbx1_hash.
Why not simply overwrite "dbx" variable? Without "-a", "env set -e" does it if it is properly signed with KEK.
I am not sure you've understood the bug yet. If I did that, db's 1sts entry would still be there. The whole point is insert dbx1_hash first. The easiest way to do this is on an empty database, instead of starting overwriting and cleaning variables. Why is rebooting even a problem?
with u_boot_console.log.section('Test Case 5e'):# Test Case 5e, authenticated even if only one of signatures# is verified. Same as before but reject dbx_hash1.auth onlyPlease specify what test case "before" means.
The test that run right before that
Please add a particular test case number to avoid any ambiguity. I believe that a test case description should be easy enough to understand and convey no ambiguity especially if there is some subtle difference between cases.
This is exactly the test case right above with dbx1_auth inserted first. I think it's fine under the current test.
output = u_boot_console.run_command_list(['host bind 0 %s' % disk_img,'fatload host 0:1 4000000 db.auth','setenv -e -nv -bs -rt -at -i 4000000:$filesize db','fatload host 0:1 4000000 KEK.auth','setenv -e -nv -bs -rt -at -i 4000000:$filesize KEK','fatload host 0:1 4000000 PK.auth','setenv -e -nv -bs -rt -at -i 4000000:$filesize PK','fatload host 0:1 4000000 db1.auth','setenv -e -nv -bs -rt -at -a -i 4000000:$filesize db','fatload host 0:1 4000000 dbx_hash1.auth','setenv -e -nv -bs -rt -at -i 4000000:$filesize dbx'])Now "db" has db.auth and db1.auth in this order and 'dbx" has dbx_hash1.auth. Is this what you intend to test?
Yes. The patchset solved 2 bugs. One was not rejecting the image when a single dbx entry was found. The second was that depending on the order the image was signed and the keys inserted into dbx, the code could reject or accept the image.
Which part of "dbx" (or "db"?) is in a reverse order?
the first tests add dbx_hash -> dbx1_hash, while the second purges the dbx database and adds dbx1_hash to test against.
Regards /Ilias
-Takahiro Akashi
-Takahiro Akashi
assert 'Failed to set EFI variable' not in ''.join(output)output = u_boot_console.run_command_list(['efidebug boot add -b 1 HELLO host 0:1 /helloworld.efi.signed_2sigs -s ""','efidebug boot next 1','efidebug test bootmgr'])assert '\'HELLO\' failed' in ''.join(output)assert 'efi_start_image() returned: 26' in ''.join(output)- def test_efi_signed_image_auth6(self, u_boot_console, efi_boot_env): """ Test Case 6 - using digest of signed image in database
-- 2.32.0
Regards /Ilias