Re: [PATCH 2/2] test/py: efi_secboot: adjust secure boot tests to code changes

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

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

> 
> > +        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.

> 
> > +        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 only
> 
> Please specify what test case "before" means.

The test that run right before that

> 
> > +            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.

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