[U-Boot] [PATCH] autoboot.c: Add feature to stop autobooting via SHA256 encrypted password

[Stefan Roese]

Hi Simon,

On 23.03.2015 21:28, Simon Glass wrote:
> Hi Stefan,
> 
> On 13 March 2015 at 01:15, Stefan Roese <sr at denx.de> wrote:
>> Hi Simon,
>>
>> On 13.03.2015 03:48, Simon Glass wrote:
>>>>>
>>>>> This patch adds the feature to only stop the autobooting, and therefor
>>>>> boot into the U-Boot prompt, when the input string / password matches
>>>>> a values that is encypted via a SHA256 hash and saved in the
>>>>> environment.
>>>>>
>>>>> This feature is enabled by defined these config options:
>>>>>        CONFIG_AUTOBOOT_KEYED
>>>>>        CONFIG_AUTOBOOT_STOP_STR_SHA256
>>>>>
>>>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>>
>>>>
>>>> This is certainly interesting but I think brings us back to a point
>>>> Simon made a long while back about needing to factor out this code
>>>> better.  Especially since this adds big long #if-#else-#endif blocks.
>>>> Can we re-do this so at least have some functions be called out instead?
>>>> Thanks!
>>>>
>>>
>>> Also if these CONFIG options are in Kconfig (as they should be) then we
>>> can use
>>>
>>> if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_SHA256))
>>>
>>> instead of #ifdef which may improve the code.
>>
>>
>> Right. I also thought about this. But the resulting code has all the
>> functionality extracted into 2 functions:
>>
>> #if defined(CONFIG_AUTOBOOT_STOP_STR_SHA256)
>> static int passwd_abort(uint64_t etime)
>> {
>>          const char *sha_env_str = getenv("bootstopkeysha256");
>>          ...
>> }
>> #else
>> static int passwd_abort(uint64_t etime)
>> {
>>          int abort = 0;
>>          ...
>> }
>> #endif
>>
>> And this function is now called unconditionally:
>>
>>          ...
>>          abort = passwd_abort(etime);
>>
>> So there is nothing here that could be simplified by using IS_ENABLED().
>>
>> I could of course just add this new config option to Kconfig. But with all
>> the other related options not in Kconfig (CONFIG_AUTOBOOT_KEYED,
>> CONFIG_AUTOBOOT_DELAY_STR...), this doesn't make much sense. So at some
>> point all those config options should be moved to Kconfig. Unfortunately I
>> don't have the time for this right now. But I'll add it to my list to do
>> this at a later time.
> 
> Well rather than adding more options, perhaps we should wait until we
> get this moved to Kconfig? It's not going to get any easier :-)

Right. And now I'm finally back at this task. To get this encrypted
password support into mainline. With Kconfig support of course this
time. ;)

Unfortunately I'm hitting a problem while moving some of the "old"
macros to Kconfig. Especially some strings like CONFIG_AUTOBOOT_PROMPT.
Here how this looks in some config headers:

#define CONFIG_AUTOBOOT_PROMPT         "autoboot in %d seconds\n", bootdelay

This does not work, as Kconfig truncates the string after the 2nd
'"'. Escaping this '"' using '\' also doesn't seem to work. Do you
or Masahiro have some experience with this kind of Kconfig macro
transition?

Thanks,
Stefan