[PATCH v2 3/7] common: integrate crypt-based passwords

[Simon Glass]

Hi Steffen,

On Tue, 11 May 2021 at 12:30, Steffen Jaeckel
<jaeckel-floss at eyet-services.de> wrote:
>
> Hi Simon,
>
> On 5/11/21 5:27 PM, Simon Glass wrote:
> >>
> >> [snip]
> >>
> >>>> diff --git a/common/autoboot.c b/common/autoboot.c
> >>>> index 50ab9281e7..6f55abe388 100644
> >>>> --- a/common/autoboot.c
> >>>> +++ b/common/autoboot.c
> >>>> @@ -316,3 +316,4 @@ static int abortboot_key_sequence(int bootdelay)
> >>>>         if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
> >>>> -               if (IS_ENABLED(CONFIG_CRYPT_PW))
> >>>> +               if (IS_ENABLED(CONFIG_CRYPT_PW) &&
> >>>> +                   env_get_yesno("bootstopusesha256") != 1)
> >>>>                         abort = passwd_abort_crypt(etime);
> >>>
> >>> Yes, and then you can enable both in sandbox and potentially have a
> >>> test for your code within the standard sandbox build.
> >>
> >> What kind of tests do you want to have added? Python based or C based ones?
> >>
> >> TBH I don't see an easy way (yet) to add more tests than the ones I
> >> already added, as enabling AUTOBOOT_KEYED (which is required for both,
> >> crypt and sha256) would change the startup behavior of the sandbox...
> >
> > Here is my idea...we have console monitoring, like this:
> >
> > console_record_reset();
> > run_command("acpi dump rdst", 0);
> > ut_assert_nextline("Table 'RDST' not found");
> > ut_assert_console_end();
> >
> > What is needed is the ability to inject console input. We have
> > gd->console_in (in console.c) but there is currently no function to
> > add input to it. Something similar to console_record_puts() is needed,
> > perhaps called console_write_in(), which does a
> > membuff_put(&gd->console_in, ...) with some input data (the hash).
> > That way the input can be read by sandbox.
> >
> > Then I think you could write a test like this:
> >
> > console_record_reset();
> > console_write_in(hash_string, strlen(hash_string));
> > ut_assertok(autoboot_command(""));
> > ut_assert_nextline("whatever indicates success");
> > ut_assert_console_end();
>
> OK, that sounds fine, with the only problem that there's no way to
> enable the necessary features without also having them enabled in the
> autoboot flow!?
>
> i.e. instead of having a single keypress to enable the console of the
> sandbox, one would always have to enter the password, or am I missing
> something?

Yes I see. Perhaps have a function to indicate whether it should be
enabled at runtime? For sandbox it can return false?

You could add a sandbox command-line flag to make it skip this, I
suppose. An environment variable is another option.

We often have little tweaks with tests to get them to do what we want.

Regards,
Simon