[PATCH] Add optional salt to AUTOBOOT_STOP_STR_SHA256

[Heinrich Schuchardt]

On 11/20/20 2:41 AM, Joel Peshkin wrote:
> From: Joel Peshkin <joel.peshkin at broadcom.com>
>
> 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 <joel.peshkin at broadcom.com>
> ---
>   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;
>   	/*
>