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

[Ilias Apalodimas]

Akashi-san,

Here's an example of what was not working in case it helps you understand

[...]
> > >
> > > >
> > > > When you said "in a reversed order" in your commit, I expected that either
> > > >  1.the image(helloworld.efi) has two signatures in a reversed order, or
> > > >        (You hinted this possibility in our chat yesterday.)
> > > >  2."db" has "db1.auth" and "db.auth" in this order, or
> > > >  3."dbx" has "dbx_hash1.auth" and "dbx_hash.auth" in this order
> > > > in this context, but your change didn't do neither.
> > > >
> > > > You intended (3). Right?
> > >
> > > Yes, however inserting dbx_hash.auth right after dbx_hash1.auth didnt work
> > > for me.  There's something date related which prevents us from adding both
> > > of the sha256 hashes of the certs in reverse order.
> >
> > I don't know why we can't do that.
>
> There's a security vioilation reported if you try to insert dbx_hash
> after dbx_hash1,  I assumed it's date related but didn't have time to
> check it.  Adding dbx_hash1 alone is enough to test the order though.
>
> >
> > > However I think that
> > > inserting dbx_hash1.auth is enough for the test purpose.  The whole point
> > > was to verify the change of the first patch, were a binary gets rejected on
> > > ony dbx match.
> >
> > In your commit message for the first one, you said,
> > "The rejection depends on the order that the image was signed
> > and the order the certificates are read (and checked) in db."
> >
> > In your new test case (5e), you mentioned "reverse order."
> >
> > That kind of things confused me (and probably others as well) regarding
> > what this test case is meant for.
> > # Again, appropriate description about test cases is very much crucial
> > # for reviewing test scenario.
>
> You either have to revert the signing order of the binary or the
> sha256 hashes of certs that are inserted in dbx to test the order.

If you have an image signed with 2 certs

sbverify --list tmp/Image.signed.signed
signature 1
image signature issuers:
 - /CN=apalos DB2
image signature certificates:
 - subject: /CN=apalos DB2
   issuer:  /CN=apalos DB2
signature 2
image signature issuers:
 - /CN=apalos DB
image signature certificates:
 - subject: /CN=apalos DB
   issuer:  /CN=apalos DB

and add both of the certificates in DB and and 'apalos DB2' in dbx,
without this patch the image is allowed to run.  However if you add
'apalos DB' the image gets rejected.   What this patch does is reject
the image properly if any of the certificates match.

Regards
/Ilias
>
> >
> > > >
> > > > > The
> > > > > easiest way to do this is on an empty database, instead of starting
> > > > > overwriting and cleaning variables.  Why is rebooting even a problem?
> > > >
> > > > If "dbx" is a matter, the easiest way is to simply overwrite that variable.
> > > > (Apparently we don't need any cleanup.)
> > > >
> > >
> > > Ah sure, I can test that and send a patch along with some more test cases I
> > > got in mind.
> >
> >
> > Anyhow, I'm looking forward for more test cases here :)
> >
> > -Takahiro Akashi
> >
> > > > >
> > > > > > > >
> > > > > > > > > +        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
> > > > > >
> > > > > > 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.
> > > >
> > > > See my comment above.
> > > >
> > > > >
> > > > > >
> > > > > > > >
> > > > > > > > > +            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.
> > > >
> > > > See my comment above.
> > > >
> > > > -Takahiro Akashi
> > > >
> > > > > 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