Re: [PATCH] Add optional salt to AUTOBOOT_STOP_STR_SHA256

On 11/20/20 2:41 AM, Joel Peshkin wrote:

From: Joel Peshkin joel.peshkin@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@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; /*