[PATCH v3 6/6] test: add test for eficonfig secure boot key management

[Simon Glass]

iHi Heinrich,

On Fri, 14 Oct 2022 at 22:43, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 10/15/22 03:10, Simon Glass wrote:
> > Hi Ilias,
> >
> > On Fri, 14 Oct 2022 at 09:59, Ilias Apalodimas
> > <ilias.apalodimas at linaro.org> wrote:
> >>
> >> Hi Simon,
> >>
> >> On Fri, 14 Oct 2022 at 18:56, Simon Glass <sjg at chromium.org> wrote:
> >>>
> >>> Hi,
> >>>
> >>> On Fri, 14 Oct 2022 at 00:58, Masahisa Kojima
> >>> <masahisa.kojima at linaro.org> wrote:
> >>>>
> >>>> Provide a unit test for the eficonfig secure boot key
> >>>> management menu.
> >>>>
> >>>> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> >>>> ---
> >>>> No change since v2
> >>>>
> >>>> newly created in v2
> >>>>
> >>>>   test/py/tests/test_eficonfig/conftest.py      |  84 +++-
> >>>>   test/py/tests/test_eficonfig/defs.py          |  14 +
> >>>>   .../test_eficonfig/test_eficonfig_sbkey.py    | 472 ++++++++++++++++++
> >>>>   3 files changed, 568 insertions(+), 2 deletions(-)
> >>>>   create mode 100644 test/py/tests/test_eficonfig/defs.py
> >>>>   create mode 100644 test/py/tests/test_eficonfig/test_eficonfig_sbkey.py
> >>>
> >>> Please can this test be in C? Also, using down-arrow to select menus
> >>> is brittle. Add a function to select the one you want, e.g. by name.
> >>>
> >>
> >> Is there a very specific reason why we should do stuff like that in C?
> >
> > Yes, see here.
> >
> >>   Python is way easier to extend and test in our case.
> >
> > In what way? It seems a lot more complicated, plus the brittle nature
> > of this test suggests it will be a hassle to maintain.
> >
> > https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html#python-or-c
> >
> > There is a pending update here too:
> >
> > https://patchwork.ozlabs.org/project/uboot/patch/20221013122927.636867-15-sjg@chromium.org/
>
> The discussion touches different aspects:
>
> ** What does it take to make a GUI easily testable? **
>
> Relying on cursor movement for testing is fragile. This is why GUIs
> often assign to each executable element the following:
>
> * Access key, e.g  <CTRL><S>
>    A key combination that when entered will have the the same
>    effect as selecting the GUI item
> * Access string, '@SAVE'
>    A string that when typed into the command field will have
>    the same effect as selecting the GUI item
>
> Let's look at U-Boot's menu entry definition:
>
> struct bootmenu_entry {
>      unsigned short int num;      // unique number 0 .. MAX_COUNT
>      char key[3];                 // key identifier of number
>      char *title;                 // title of entry
>      char *command;               // hush command of entry
>      enum boot_type type;         // boot type of entry
>      u16 bootorder;               // order for each boot type
>      struct bootmenu_data *menu;  // this bootmenu
>      struct bootmenu_entry *next; // next menu entry (num+1)
> }
>
> Our structure lacks an accessor element that can be used to select a
> menu item without using cursor actions.
>
> Compound keystrokes like <CTRL><F6> are send as multiple bytes on the
> console, e.g. 1b 5b 31 37 3b 35 7e.
>
> We may define a field shortcut of type char *. If the string is received
> by the menu loop, let it activate the matching menu entry. Let cursor
> actions (up, down, enter, space '+', '-') interrupt matching the
> shortcut string.
>
> Instead we could also use a convention for the title:
>
> If a letter in the title is preceded by '&', this is the shortcut key.
> This letter will be shown highlighted in the menu and the ampersand will
> not be shown.
>
> This is probably easier to implement.
>
> Adding the shortcut facility will allow both for easier testing and
> faster navigation.
>
> ** Choice of programming language **
>
> Several aspects control the choice of the programming language for tests:
>
> - Testing single library functions is only possible in C.
> - Checking contents of internal structures is only possible in C.
> - Testing the U-Boot's CLI is easily accessible in our Python framework.
> - Preparation of complex test data is easier to do in Python.
> - Mixed language tests should be avoided if not strictly necessary.
>    It is much easier to maintain a single code source for a test.
>
> ** What to do for secure boot key management? **
>
> Secure boot key management requires complex preparation which fits well
> into out Python testing framework.
>
> Once we provide shortcut keys to U-Boot menus the entries will be easily
> accessible from Python.
>
> As we want to avoid complexity due to mixed language tests we should
> stick to Python for testing key management at the user level.

The test setup should be in Python (and needs to be) but the actual
test should be in C, in general.

>From what I can tell these tests should be split up, one for each test case.

Using keyboard shortcuts for menus would certainly help, but you still
have the dependency between the menu code in C and the Python code
that uses it. The complexity of this dependency is quite significant.

Can we not design this all for easier testing? The functionality
should be broken down a little more so we can change the configs in a
test (e.g. using the eficonfig command), then check that booting does
what we expect.

Overall I feel that this 'black box' testing is not a good approach
and we should try to test the components. It is OK to have a
happy-path test but here we are just writing hundreds of lines of
Python to get a very high-level signal about correctness.

>
> Additional tests in C for library functions managing keys and verifying
> signatures would improve our code coverage.
>
> Best regards
>
> Heinrich

Regards,
Simon