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

[Steffen Jaeckel]

Hi Simon

On 5/10/21 10:24 PM, Simon Glass wrote:
> On Mon, 10 May 2021 at 14:06, Steffen Jaeckel
> <jaeckel-floss at eyet-services.de> wrote:
>> On 5/10/21 9:19 PM, Simon Glass wrote:
>>> On Mon, 10 May 2021 at 11:05, Steffen Jaeckel
>>> <jaeckel-floss at eyet-services.de> wrote:
>>>> On 5/10/21 6:27 PM, Simon Glass wrote:
>>>>> On Mon, 10 May 2021 at 00:19, Steffen Jaeckel
>>>>> <jaeckel-floss at eyet-services.de> wrote:
>>>>>>
>>>>>> Hook into the autoboot flow as an alternative to the existing
>>>>>> mechanisms.
>>>>>>
>>>>>> Signed-off-by: Steffen Jaeckel <jaeckel-floss at eyet-services.de>
>>>>>> ---
>>>>>>
>>>>>> (no changes since v1)
>>>>>>
>>>>>>  common/Kconfig.boot | 37 ++++++++++++++++++---
>>>>>>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
>>>>>>  2 files changed, 103 insertions(+), 14 deletions(-)
>>>>>
>>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>>>
>>>>> But I think you'll need to allow both to be enabled.
>>>>
>>>> Sorry, but what exactly do you mean?
>>>
>>> I mean the ability to enable both crypt and the sha options rather
>>> than just one at a time.
>>
>> You have a point there. Even though this approach should IMO become the
>> recommended way to store passwords, one could imagine that support for
>> both approaches in parallel could be needed, e.g. in a transition period.
>>
>> The biggest problem I see is that the passwd_abort_{crypt,sha256}()
>> functions consume the serial input. I fear that an implementation that
>> supports both would need to have a painful amount of special case
>> handling in order to not break the expected behavior of the existing
>> sha256 implementation.
>>
>> Supporting both in a backwards compatible way would make the
>> implementation a lot more complex and therefor I'd prefer to leave it as is.
> 
> I don't quite mean that. Only one should be used at once. But would it
> be possible to support both at runtime, so that (at runtime) you check
> an env var to decide which is active?

oh okay, that's easier :)

maybe something like this?

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);

Cheers
Steffen