From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Fri, 20 Nov 2020 17:56:04 +0100 Subject: [PATCH] Add optional salt to AUTOBOOT_STOP_STR_SHA256 In-Reply-To: <20201120014114.48895-1-jp933255@xl-irv-13.lvn.broadcom.net> References: <20201120014114.48895-1-jp933255@xl-irv-13.lvn.broadcom.net> Message-ID: <0704a7b8-b3ca-76e6-ec31-489b874af8d6@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 11/20/20 2:41 AM, Joel Peshkin wrote: > From: Joel Peshkin > > Adds an optional SALT value to AUTOBOOT_STOP_STR_SHA256. If a string > followed by a ":" is prepended to the sha256, the portion to the left > of the colon will be used as a salt and the password will be appended > to the salt before the sha256 is computed and compared. > > Signed-off-by: Joel Peshkin > --- > common/Kconfig.boot | 5 ++++- > common/autoboot.c | 10 +++++++++- > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/common/Kconfig.boot b/common/Kconfig.boot > index 3f6d9c1..8a98672 100644 > --- a/common/Kconfig.boot > +++ b/common/Kconfig.boot > @@ -819,7 +819,10 @@ config AUTOBOOT_STOP_STR_SHA256 > This option adds the feature to only stop the autobooting, > and therefore 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. > + a SHA256 hash and saved in the environment variable > + "bootstopkeysha256". If the value in that variable > + includes a ":", the portion prior to the ":" will be treated > + as a salt value. > > config AUTOBOOT_USE_MENUKEY > bool "Allow a specify key to run a menu from the environment" > diff --git a/common/autoboot.c b/common/autoboot.c > index e628baf..0c4e6ff 100644 > --- a/common/autoboot.c > +++ b/common/autoboot.c > @@ -80,6 +80,7 @@ static int passwd_abort_sha256(uint64_t etime) > u8 sha_env[SHA256_SUM_LEN]; > u8 *sha; > char *presskey; > + char *c; > const char *algo_name = "sha256"; > u_int presskey_len = 0; > int abort = 0; > @@ -89,6 +90,14 @@ static int passwd_abort_sha256(uint64_t etime) > if (sha_env_str == NULL) > sha_env_str = AUTOBOOT_STOP_STR_SHA256; > > + presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR); > + c = strstr(sha_env_str, ":"); > + if (c) { > + /* preload presskey with salt */ > + memcpy(presskey, sha_env_str, c - sha_env_str); Dear Joel, thank you for your contribution helping to fend of password attacks using lookup-tables for hashes. Please, safeguard against c - sha_env_str > MAX_DELAY_STOP_STR to avoid a possible buffer overflow. We have #define MAX_DELAY_STOP_STR 32 Shouldn't this value be enlarged to encompass a salt with 256 bits of randomness (matching the SHA256 algorithm)? If you encode 6 bits of entropy in each character, you need 43 characters for the salt and 43 characters for the password. > + presskey_len += c - sha_env_str; This would be more readable: presskey_len = c - sha_env_str; Best regards Heinrich > + sha_env_str = c + 1; > + } > /* > * Generate the binary value from the environment hash value > * so that we can compare this value with the computed hash > @@ -100,7 +109,6 @@ static int passwd_abort_sha256(uint64_t etime) > return 0; > } > > - presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR); > sha = malloc_cache_aligned(SHA256_SUM_LEN); > size = SHA256_SUM_LEN; > /* >