From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 10 May 2021 13:45:14 -0700 Subject: [PATCH v2 3/7] common: integrate crypt-based passwords In-Reply-To: References: <20210510061916.3388626-1-jaeckel-floss@eyet-services.de> <20210510061916.3388626-4-jaeckel-floss@eyet-services.de> <665efe80-e61d-6750-ac0f-6801439c65df@eyet-services.de> <460701e4-3684-9de8-73fa-4229db6a089a@eyet-services.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Steffen, On Mon, 10 May 2021 at 13:37, Steffen Jaeckel wrote: > > Hi Simon > > On 5/10/21 10:24 PM, Simon Glass wrote: > > On Mon, 10 May 2021 at 14:06, Steffen Jaeckel > > wrote: > >> On 5/10/21 9:19 PM, Simon Glass wrote: > >>> On Mon, 10 May 2021 at 11:05, Steffen Jaeckel > >>> wrote: > >>>> On 5/10/21 6:27 PM, Simon Glass wrote: > >>>>> On Mon, 10 May 2021 at 00:19, Steffen Jaeckel > >>>>> wrote: > >>>>>> > >>>>>> Hook into the autoboot flow as an alternative to the existing > >>>>>> mechanisms. > >>>>>> > >>>>>> Signed-off-by: Steffen Jaeckel > >>>>>> --- > >>>>>> > >>>>>> (no changes since v1) > >>>>>> > >>>>>> common/Kconfig.boot | 37 ++++++++++++++++++--- > >>>>>> common/autoboot.c | 80 ++++++++++++++++++++++++++++++++++++++++----- > >>>>>> 2 files changed, 103 insertions(+), 14 deletions(-) > >>>>> > >>>>> Reviewed-by: Simon Glass > >>>>> > >>>>> 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); Yes, and then you can enable both in sandbox and potentially have a test for your code within the standard sandbox build. Regards, Simon