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

[Heinrich Schuchardt]

On 10/19/22 15:17, Simon Glass wrote:
> 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.

 From other projects I am used to differentiate between unit tests and
integration tests.

An integration test is what checks that all the parts work together
while a unit test checks a single functionality works as designed.

A description can be found in
https://circleci.com/blog/unit-testing-vs-integration-testing/#c-consent-modal

For an integration test this Python script is fine (except for the menu
navigation.

As I said in my last mail additional tests on a lower level may be
added. But unit tests cannot replace an integration test.

Best regards

Heinrich

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