[PATCH v3 0/8] common: Introduce crypt-style password support

This patchset introduces support for crypt-style passwords to unlock the console in autoboot mode.
The implementation of crypt-sha256 and crypt-sha512 originate from libxcrypt at https://github.com/besser82/libxcrypt.git Version v4.4.17 Git commit hash 6b110bc
I didn't re-format those two files to make diffing to the original versions from libxcrypt easier, which leads to a huge load of checkpatch.pl warnings&errors. Please advise on whether they should be re-formatted or can be kept as is.
The remaining warnings from checkpatch.pl are intentional resp. open for discussion.
A sandbox defconfig with password entry has been added. I'm not sure whether this should be kept or not, it's just there as an example.
Cheers, Steffen
Changes in v3: Add unit-tests for autoboot Introduce `bootstopusesha256` to allow fallback to plain SHA256-based hashing Add AUTOBOOT_FLUSH_STDIN option Drop the changes to bcm963158_ram_defconfig
Changes in v2: Update Kconfig way of enabling, setting hashes etc.
Changes in v1: Added unit-tests of crypt_compare() Wrapped crypt functions to encapsulate errno
Steffen Jaeckel (8): lib: add crypt subsystem lib: wrap crypt API to hide errno usage common: integrate crypt-based passwords common: Rename macro appropriately common: allow disabling of timeout for password entry common: add AUTOBOOT_FLUSH_STDIN option common: add support to fallback to plain SHA256 test: add first autoboot unit tests
common/Kconfig.boot | 65 ++++++- common/autoboot.c | 135 ++++++++++++-- common/console.c | 5 + configs/sandbox_defconfig | 11 ++ include/console.h | 17 ++ include/crypt.h | 14 ++ include/test/common.h | 15 ++ include/test/suites.h | 1 + lib/Kconfig | 1 + lib/Makefile | 1 + lib/crypt/Kconfig | 28 +++ lib/crypt/Makefile | 10 ++ lib/crypt/alg-sha256.h | 11 ++ lib/crypt/alg-sha512.h | 11 ++ lib/crypt/crypt-port.h | 30 ++++ lib/crypt/crypt-sha256.c | 335 ++++++++++++++++++++++++++++++++++ lib/crypt/crypt-sha512.c | 350 ++++++++++++++++++++++++++++++++++++ lib/crypt/crypt.c | 76 ++++++++ test/Kconfig | 9 + test/Makefile | 1 + test/cmd_ut.c | 1 + test/common/Makefile | 3 + test/common/cmd_ut_common.c | 22 +++ test/common/test_autoboot.c | 90 ++++++++++ test/lib/Makefile | 1 + test/lib/test_crypt.c | 64 +++++++ 26 files changed, 1285 insertions(+), 22 deletions(-) create mode 100644 include/crypt.h create mode 100644 include/test/common.h create mode 100644 lib/crypt/Kconfig create mode 100644 lib/crypt/Makefile create mode 100644 lib/crypt/alg-sha256.h create mode 100644 lib/crypt/alg-sha512.h create mode 100644 lib/crypt/crypt-port.h create mode 100644 lib/crypt/crypt-sha256.c create mode 100644 lib/crypt/crypt-sha512.c create mode 100644 lib/crypt/crypt.c create mode 100644 test/common/Makefile create mode 100644 test/common/cmd_ut_common.c create mode 100644 test/common/test_autoboot.c create mode 100644 test/lib/test_crypt.c

Add the basic functionality required to support the standard crypt format. The files crypt-sha256.c and crypt-sha512.c originate from libxcrypt and their formatting is therefor retained. The integration is done via a crypt_compare() function in crypt.c.
``` libxcrypt $ git describe --long --always --all tags/v4.4.17-0-g6b110bc ```
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v3: Add unit-tests for autoboot Introduce `bootstopusesha256` to allow fallback to plain SHA256-based hashing Add AUTOBOOT_FLUSH_STDIN option Drop the changes to bcm963158_ram_defconfig
Changes in v2: Update Kconfig way of enabling, setting hashes etc.
Changes in v1: Added unit-tests of crypt_compare() Wrapped crypt functions to encapsulate errno
include/crypt.h | 13 ++ lib/Kconfig | 1 + lib/Makefile | 1 + lib/crypt/Kconfig | 28 ++++ lib/crypt/Makefile | 10 ++ lib/crypt/alg-sha256.h | 17 ++ lib/crypt/alg-sha512.h | 17 ++ lib/crypt/crypt-port.h | 28 ++++ lib/crypt/crypt-sha256.c | 313 +++++++++++++++++++++++++++++++++++++ lib/crypt/crypt-sha512.c | 328 +++++++++++++++++++++++++++++++++++++++ lib/crypt/crypt.c | 73 +++++++++ test/Kconfig | 9 ++ test/lib/Makefile | 1 + test/lib/test_crypt.c | 44 ++++++ 14 files changed, 883 insertions(+) create mode 100644 include/crypt.h create mode 100644 lib/crypt/Kconfig create mode 100644 lib/crypt/Makefile create mode 100644 lib/crypt/alg-sha256.h create mode 100644 lib/crypt/alg-sha512.h create mode 100644 lib/crypt/crypt-port.h create mode 100644 lib/crypt/crypt-sha256.c create mode 100644 lib/crypt/crypt-sha512.c create mode 100644 lib/crypt/crypt.c create mode 100644 test/lib/test_crypt.c
diff --git a/include/crypt.h b/include/crypt.h new file mode 100644 index 0000000000..e0be2832ff --- /dev/null +++ b/include/crypt.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Copyright (C) 2020 Steffen Jaeckel jaeckel-floss@eyet-services.de */ + +/** + * Compare should with the processed passphrase. + * + * @should The crypt-style string to compare against + * @passphrase The plaintext passphrase + * @equal Pointer to an int where the result is stored + * '0' = unequal + * '1' = equal + */ +void crypt_compare(const char *should, const char *passphrase, int *equal); diff --git a/lib/Kconfig b/lib/Kconfig index 15019d2c65..ea32d9a00c 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -297,6 +297,7 @@ config AES
source lib/rsa/Kconfig source lib/crypto/Kconfig +source lib/crypt/Kconfig
config TPM bool "Trusted Platform Module (TPM) Support" diff --git a/lib/Makefile b/lib/Makefile index b4795a62a0..a3cf7894b4 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -65,6 +65,7 @@ obj-$(CONFIG_HASH) += hash-checksum.o obj-$(CONFIG_SHA1) += sha1.o obj-$(CONFIG_SHA256) += sha256.o obj-$(CONFIG_SHA512_ALGO) += sha512.o +obj-$(CONFIG_CRYPT_PW) += crypt/
obj-$(CONFIG_$(SPL_)ZLIB) += zlib/ obj-$(CONFIG_$(SPL_)ZSTD) += zstd/ diff --git a/lib/crypt/Kconfig b/lib/crypt/Kconfig new file mode 100644 index 0000000000..5495ae8d4c --- /dev/null +++ b/lib/crypt/Kconfig @@ -0,0 +1,28 @@ +menuconfig CRYPT_PW + bool "Add crypt support for password-based unlock" + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION + help + Enable support for crypt-style hashed passphrases. + This will then be used as the mechanism of choice to + verify whether the entered password to unlock the + console is correct or not. + +if CRYPT_PW + +config CRYPT_PW_SHA256 + bool "Provide sha256crypt" + select SHA256 + select SHA256_ALGO + help + Enables support for the sha256crypt password-hashing algorithm. + The prefix is "$5$". + +config CRYPT_PW_SHA512 + bool "Provide sha512crypt" + select SHA512 + select SHA512_ALGO + help + Enables support for the sha512crypt password-hashing algorithm. + The prefix is "$6$". + +endif diff --git a/lib/crypt/Makefile b/lib/crypt/Makefile new file mode 100644 index 0000000000..290231064c --- /dev/null +++ b/lib/crypt/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (c) 2013, Google Inc. +# +# (C) Copyright 2000-2007 +# Wolfgang Denk, DENX Software Engineering, wd@denx.de. + +obj-$(CONFIG_CRYPT_PW) += crypt.o +obj-$(CONFIG_CRYPT_PW_SHA256) += crypt-sha256.o +obj-$(CONFIG_CRYPT_PW_SHA512) += crypt-sha512.o diff --git a/lib/crypt/alg-sha256.h b/lib/crypt/alg-sha256.h new file mode 100644 index 0000000000..e4b29c9f31 --- /dev/null +++ b/lib/crypt/alg-sha256.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Copyright (C) 2020 Steffen Jaeckel jaeckel-floss@eyet-services.de */ + +#ifndef USE_HOSTCC +#include "common.h" +#else +#include <string.h> +#endif + +#include "u-boot/sha256.h" + +#define INCLUDE_sha256crypt 1 + +#define SHA256_CTX sha256_context +#define SHA256_Init sha256_starts +#define SHA256_Update(c, i, l) sha256_update(c, (const void *)i, l) +#define SHA256_Final(b, c) sha256_finish(c, b) diff --git a/lib/crypt/alg-sha512.h b/lib/crypt/alg-sha512.h new file mode 100644 index 0000000000..93b6109fae --- /dev/null +++ b/lib/crypt/alg-sha512.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Copyright (C) 2020 Steffen Jaeckel jaeckel-floss@eyet-services.de */ + +#ifndef USE_HOSTCC +#include "common.h" +#else +#include <string.h> +#endif + +#include "u-boot/sha512.h" + +#define INCLUDE_sha512crypt 1 + +#define SHA512_CTX sha512_context +#define SHA512_Init sha512_starts +#define SHA512_Update(c, i, l) sha512_update(c, (const void *)i, l) +#define SHA512_Final(b, c) sha512_finish(c, b) diff --git a/lib/crypt/crypt-port.h b/lib/crypt/crypt-port.h new file mode 100644 index 0000000000..680ffe9349 --- /dev/null +++ b/lib/crypt/crypt-port.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Copyright (C) 2020 Steffen Jaeckel jaeckel-floss@eyet-services.de */ + +#include <linux/types.h> +#include <vsprintf.h> + +#define NO_GENSALT +#define CRYPT_OUTPUT_SIZE 384 +#define ALG_SPECIFIC_SIZE 8192 + +#define ARG_UNUSED(x) (x) + +#define static_assert(a, b) _Static_assert(a, b) + +#define strtoul(cp, endp, base) simple_strtoul(cp, endp, base) + +extern const unsigned char ascii64[65]; + +#define b64t ((const char *)ascii64) + +void crypt_sha256crypt_rn(const char *phrase, size_t phr_size, + const char *setting, size_t ARG_UNUSED(set_size), + uint8_t *output, size_t out_size, void *scratch, + size_t scr_size); +void crypt_sha512crypt_rn(const char *phrase, size_t phr_size, + const char *setting, size_t ARG_UNUSED(set_size), + uint8_t *output, size_t out_size, void *scratch, + size_t scr_size); diff --git a/lib/crypt/crypt-sha256.c b/lib/crypt/crypt-sha256.c new file mode 100644 index 0000000000..37127d41e1 --- /dev/null +++ b/lib/crypt/crypt-sha256.c @@ -0,0 +1,313 @@ +/* One way encryption based on the SHA256-based Unix crypt implementation. + * + * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1]. + * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018. + * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018. + * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020. + * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2020. + * To the extent possible under law, the named authors have waived all + * copyright and related or neighboring rights to this work. + * + * See https://creativecommons.org/publicdomain/zero/1.0/ for further + * details. + * + * This file is a modified except from [2], lines 648 up to 909. + * + * [1] https://www.akkadia.org/drepper/sha-crypt.html + * [2] https://www.akkadia.org/drepper/SHA-crypt.txt + */ + +#include "crypt-port.h" +#include "alg-sha256.h" + +#include <errno.h> +#include <stdio.h> +#include <stdlib.h> + +#if INCLUDE_sha256crypt + +/* Define our magic string to mark salt for SHA256 "encryption" + replacement. */ +static const char sha256_salt_prefix[] = "$5$"; + +/* Prefix for optional rounds specification. */ +static const char sha256_rounds_prefix[] = "rounds="; + +/* Maximum salt string length. */ +#define SALT_LEN_MAX 16 +/* Default number of rounds if not explicitly specified. */ +#define ROUNDS_DEFAULT 5000 +/* Minimum number of rounds. */ +#define ROUNDS_MIN 1000 +/* Maximum number of rounds. */ +#define ROUNDS_MAX 999999999 + +/* The maximum possible length of a SHA256-hashed password string, + including the terminating NUL character. Prefix (including its NUL) + + rounds tag ("rounds=$" = "rounds=\0") + strlen(ROUNDS_MAX) + + salt (up to SALT_LEN_MAX chars) + '$' + hash (43 chars). */ + +#define LENGTH_OF_NUMBER(n) (sizeof #n - 1) + +#define SHA256_HASH_LENGTH \ + (sizeof (sha256_salt_prefix) + sizeof (sha256_rounds_prefix) + \ + LENGTH_OF_NUMBER (ROUNDS_MAX) + SALT_LEN_MAX + 1 + 43) + +static_assert (SHA256_HASH_LENGTH <= CRYPT_OUTPUT_SIZE, + "CRYPT_OUTPUT_SIZE is too small for SHA256"); + +/* A sha256_buffer holds all of the sensitive intermediate data. */ +struct sha256_buffer +{ + SHA256_CTX ctx; + uint8_t result[32]; + uint8_t p_bytes[32]; + uint8_t s_bytes[32]; +}; + +static_assert (sizeof (struct sha256_buffer) <= ALG_SPECIFIC_SIZE, + "ALG_SPECIFIC_SIZE is too small for SHA256"); + + +/* Feed CTX with LEN bytes of a virtual byte sequence consisting of + BLOCK repeated over and over indefinitely. */ +static void +SHA256_Update_recycled (SHA256_CTX *ctx, + unsigned char block[32], size_t len) +{ + size_t cnt; + for (cnt = len; cnt >= 32; cnt -= 32) + SHA256_Update (ctx, block, 32); + SHA256_Update (ctx, block, cnt); +} + +void +crypt_sha256crypt_rn (const char *phrase, size_t phr_size, + const char *setting, size_t ARG_UNUSED (set_size), + uint8_t *output, size_t out_size, + void *scratch, size_t scr_size) +{ + /* This shouldn't ever happen, but... */ + if (out_size < SHA256_HASH_LENGTH + || scr_size < sizeof (struct sha256_buffer)) + { + errno = ERANGE; + return; + } + + struct sha256_buffer *buf = scratch; + SHA256_CTX *ctx = &buf->ctx; + uint8_t *result = buf->result; + uint8_t *p_bytes = buf->p_bytes; + uint8_t *s_bytes = buf->s_bytes; + char *cp = (char *)output; + const char *salt = setting; + + size_t salt_size; + size_t cnt; + /* Default number of rounds. */ + size_t rounds = ROUNDS_DEFAULT; + bool rounds_custom = false; + + /* Find beginning of salt string. The prefix should normally always + be present. Just in case it is not. */ + if (strncmp (sha256_salt_prefix, salt, sizeof (sha256_salt_prefix) - 1) == 0) + /* Skip salt prefix. */ + salt += sizeof (sha256_salt_prefix) - 1; + + if (strncmp (salt, sha256_rounds_prefix, sizeof (sha256_rounds_prefix) - 1) + == 0) + { + const char *num = salt + sizeof (sha256_rounds_prefix) - 1; + /* Do not allow an explicit setting of zero rounds, nor of the + default number of rounds, nor leading zeroes on the rounds. */ + if (!(*num >= '1' && *num <= '9')) + { + errno = EINVAL; + return; + } + + errno = 0; + char *endp; + rounds = strtoul (num, &endp, 10); + if (endp == num || *endp != '$' + || rounds < ROUNDS_MIN + || rounds > ROUNDS_MAX + || errno) + { + errno = EINVAL; + return; + } + salt = endp + 1; + rounds_custom = true; + } + + /* The salt ends at the next '$' or the end of the string. + Ensure ':' does not appear in the salt (it is used as a separator in /etc/passwd). + Also check for '\n', as in /etc/passwd the whole parameters of the user data must + be on a single line. */ + salt_size = strcspn (salt, "$:\n"); + if (!(salt[salt_size] == '$' || !salt[salt_size])) + { + errno = EINVAL; + return; + } + + /* Ensure we do not use more salt than SALT_LEN_MAX. */ + if (salt_size > SALT_LEN_MAX) + salt_size = SALT_LEN_MAX; + + /* Compute alternate SHA256 sum with input PHRASE, SALT, and PHRASE. The + final result will be added to the first context. */ + SHA256_Init (ctx); + + /* Add phrase. */ + SHA256_Update (ctx, phrase, phr_size); + + /* Add salt. */ + SHA256_Update (ctx, salt, salt_size); + + /* Add phrase again. */ + SHA256_Update (ctx, phrase, phr_size); + + /* Now get result of this (32 bytes). */ + SHA256_Final (result, ctx); + + /* Prepare for the real work. */ + SHA256_Init (ctx); + + /* Add the phrase string. */ + SHA256_Update (ctx, phrase, phr_size); + + /* The last part is the salt string. This must be at most 8 + characters and it ends at the first `$' character (for + compatibility with existing implementations). */ + SHA256_Update (ctx, salt, salt_size); + + /* Add for any character in the phrase one byte of the alternate sum. */ + for (cnt = phr_size; cnt > 32; cnt -= 32) + SHA256_Update (ctx, result, 32); + SHA256_Update (ctx, result, cnt); + + /* Take the binary representation of the length of the phrase and for every + 1 add the alternate sum, for every 0 the phrase. */ + for (cnt = phr_size; cnt > 0; cnt >>= 1) + if ((cnt & 1) != 0) + SHA256_Update (ctx, result, 32); + else + SHA256_Update (ctx, phrase, phr_size); + + /* Create intermediate result. */ + SHA256_Final (result, ctx); + + /* Start computation of P byte sequence. */ + SHA256_Init (ctx); + + /* For every character in the password add the entire password. */ + for (cnt = 0; cnt < phr_size; ++cnt) + SHA256_Update (ctx, phrase, phr_size); + + /* Finish the digest. */ + SHA256_Final (p_bytes, ctx); + + /* Start computation of S byte sequence. */ + SHA256_Init (ctx); + + /* For every character in the password add the entire password. */ + for (cnt = 0; cnt < (size_t) 16 + (size_t) result[0]; ++cnt) + SHA256_Update (ctx, salt, salt_size); + + /* Finish the digest. */ + SHA256_Final (s_bytes, ctx); + + /* Repeatedly run the collected hash value through SHA256 to burn + CPU cycles. */ + for (cnt = 0; cnt < rounds; ++cnt) + { + /* New context. */ + SHA256_Init (ctx); + + /* Add phrase or last result. */ + if ((cnt & 1) != 0) + SHA256_Update_recycled (ctx, p_bytes, phr_size); + else + SHA256_Update (ctx, result, 32); + + /* Add salt for numbers not divisible by 3. */ + if (cnt % 3 != 0) + SHA256_Update_recycled (ctx, s_bytes, salt_size); + + /* Add phrase for numbers not divisible by 7. */ + if (cnt % 7 != 0) + SHA256_Update_recycled (ctx, p_bytes, phr_size); + + /* Add phrase or last result. */ + if ((cnt & 1) != 0) + SHA256_Update (ctx, result, 32); + else + SHA256_Update_recycled (ctx, p_bytes, phr_size); + + /* Create intermediate result. */ + SHA256_Final (result, ctx); + } + + /* Now we can construct the result string. It consists of four + parts, one of which is optional. We already know that there + is sufficient space at CP for the longest possible result string. */ + memcpy (cp, sha256_salt_prefix, sizeof (sha256_salt_prefix) - 1); + cp += sizeof (sha256_salt_prefix) - 1; + + if (rounds_custom) + { + int n = snprintf (cp, + SHA256_HASH_LENGTH - (sizeof (sha256_salt_prefix) - 1), + "%s%zu$", sha256_rounds_prefix, rounds); + cp += n; + } + + memcpy (cp, salt, salt_size); + cp += salt_size; + *cp++ = '$'; + +#define b64_from_24bit(B2, B1, B0, N) \ + do { \ + unsigned int w = ((((unsigned int)(B2)) << 16) | \ + (((unsigned int)(B1)) << 8) | \ + ((unsigned int)(B0))); \ + int n = (N); \ + while (n-- > 0) \ + { \ + *cp++ = b64t[w & 0x3f]; \ + w >>= 6; \ + } \ + } while (0) + + b64_from_24bit (result[0], result[10], result[20], 4); + b64_from_24bit (result[21], result[1], result[11], 4); + b64_from_24bit (result[12], result[22], result[2], 4); + b64_from_24bit (result[3], result[13], result[23], 4); + b64_from_24bit (result[24], result[4], result[14], 4); + b64_from_24bit (result[15], result[25], result[5], 4); + b64_from_24bit (result[6], result[16], result[26], 4); + b64_from_24bit (result[27], result[7], result[17], 4); + b64_from_24bit (result[18], result[28], result[8], 4); + b64_from_24bit (result[9], result[19], result[29], 4); + b64_from_24bit (0, result[31], result[30], 3); + + *cp = '\0'; +} + +#ifndef NO_GENSALT + +void +gensalt_sha256crypt_rn (unsigned long count, + const uint8_t *rbytes, size_t nrbytes, + uint8_t *output, size_t output_size) +{ + gensalt_sha_rn ('5', SALT_LEN_MAX, ROUNDS_DEFAULT, ROUNDS_MIN, ROUNDS_MAX, + count, rbytes, nrbytes, output, output_size); +} + +#endif + +#endif diff --git a/lib/crypt/crypt-sha512.c b/lib/crypt/crypt-sha512.c new file mode 100644 index 0000000000..3616019445 --- /dev/null +++ b/lib/crypt/crypt-sha512.c @@ -0,0 +1,328 @@ +/* One way encryption based on the SHA512-based Unix crypt implementation. + * + * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1]. + * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018. + * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018. + * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020. + * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2020. + * To the extent possible under law, the named authors have waived all + * copyright and related or neighboring rights to this work. + * + * See https://creativecommons.org/publicdomain/zero/1.0/ for further + * details. + * + * This file is a modified except from [2], lines 1403 up to 1676. + * + * [1] https://www.akkadia.org/drepper/sha-crypt.html + * [2] https://www.akkadia.org/drepper/SHA-crypt.txt + */ + +#include "crypt-port.h" +#include "alg-sha512.h" + +#include <errno.h> +#include <stdio.h> +#include <stdlib.h> + +#if INCLUDE_sha512crypt + +/* Define our magic string to mark salt for SHA512 "encryption" + replacement. */ +static const char sha512_salt_prefix[] = "$6$"; + +/* Prefix for optional rounds specification. */ +static const char sha512_rounds_prefix[] = "rounds="; + +/* Maximum salt string length. */ +#define SALT_LEN_MAX 16 +/* Default number of rounds if not explicitly specified. */ +#define ROUNDS_DEFAULT 5000 +/* Minimum number of rounds. */ +#define ROUNDS_MIN 1000 +/* Maximum number of rounds. */ +#define ROUNDS_MAX 999999999 + +/* The maximum possible length of a SHA512-hashed password string, + including the terminating NUL character. Prefix (including its NUL) + + rounds tag ("rounds=$" = "rounds=\0") + strlen(ROUNDS_MAX) + + salt (up to SALT_LEN_MAX chars) + '$' + hash (86 chars). */ + +#define LENGTH_OF_NUMBER(n) (sizeof #n - 1) + +#define SHA512_HASH_LENGTH \ + (sizeof (sha512_salt_prefix) + sizeof (sha512_rounds_prefix) + \ + LENGTH_OF_NUMBER (ROUNDS_MAX) + SALT_LEN_MAX + 1 + 86) + +static_assert (SHA512_HASH_LENGTH <= CRYPT_OUTPUT_SIZE, + "CRYPT_OUTPUT_SIZE is too small for SHA512"); + +/* A sha512_buffer holds all of the sensitive intermediate data. */ +struct sha512_buffer +{ + SHA512_CTX ctx; + uint8_t result[64]; + uint8_t p_bytes[64]; + uint8_t s_bytes[64]; +}; + +static_assert (sizeof (struct sha512_buffer) <= ALG_SPECIFIC_SIZE, + "ALG_SPECIFIC_SIZE is too small for SHA512"); + + +/* Subroutine of _xcrypt_crypt_sha512crypt_rn: Feed CTX with LEN bytes of a + virtual byte sequence consisting of BLOCK repeated over and over + indefinitely. */ +static void +sha512_process_recycled_bytes (unsigned char block[64], size_t len, + SHA512_CTX *ctx) +{ + size_t cnt; + for (cnt = len; cnt >= 64; cnt -= 64) + SHA512_Update (ctx, block, 64); + SHA512_Update (ctx, block, cnt); +} + +void +crypt_sha512crypt_rn (const char *phrase, size_t phr_size, + const char *setting, size_t ARG_UNUSED (set_size), + uint8_t *output, size_t out_size, + void *scratch, size_t scr_size) +{ + /* This shouldn't ever happen, but... */ + if (out_size < SHA512_HASH_LENGTH + || scr_size < sizeof (struct sha512_buffer)) + { + errno = ERANGE; + return; + } + + struct sha512_buffer *buf = scratch; + SHA512_CTX *ctx = &buf->ctx; + uint8_t *result = buf->result; + uint8_t *p_bytes = buf->p_bytes; + uint8_t *s_bytes = buf->s_bytes; + char *cp = (char *)output; + const char *salt = setting; + + size_t salt_size; + size_t cnt; + /* Default number of rounds. */ + size_t rounds = ROUNDS_DEFAULT; + bool rounds_custom = false; + + /* Find beginning of salt string. The prefix should normally always + be present. Just in case it is not. */ + if (strncmp (sha512_salt_prefix, salt, sizeof (sha512_salt_prefix) - 1) == 0) + /* Skip salt prefix. */ + salt += sizeof (sha512_salt_prefix) - 1; + + if (strncmp (salt, sha512_rounds_prefix, sizeof (sha512_rounds_prefix) - 1) + == 0) + { + const char *num = salt + sizeof (sha512_rounds_prefix) - 1; + /* Do not allow an explicit setting of zero rounds, nor of the + default number of rounds, nor leading zeroes on the rounds. */ + if (!(*num >= '1' && *num <= '9')) + { + errno = EINVAL; + return; + } + + errno = 0; + char *endp; + rounds = strtoul (num, &endp, 10); + if (endp == num || *endp != '$' + || rounds < ROUNDS_MIN + || rounds > ROUNDS_MAX + || errno) + { + errno = EINVAL; + return; + } + salt = endp + 1; + rounds_custom = true; + } + + /* The salt ends at the next '$' or the end of the string. + Ensure ':' does not appear in the salt (it is used as a separator in /etc/passwd). + Also check for '\n', as in /etc/passwd the whole parameters of the user data must + be on a single line. */ + salt_size = strcspn (salt, "$:\n"); + if (!(salt[salt_size] == '$' || !salt[salt_size])) + { + errno = EINVAL; + return; + } + + /* Ensure we do not use more salt than SALT_LEN_MAX. */ + if (salt_size > SALT_LEN_MAX) + salt_size = SALT_LEN_MAX; + + /* Compute alternate SHA512 sum with input PHRASE, SALT, and PHRASE. The + final result will be added to the first context. */ + SHA512_Init (ctx); + + /* Add phrase. */ + SHA512_Update (ctx, phrase, phr_size); + + /* Add salt. */ + SHA512_Update (ctx, salt, salt_size); + + /* Add phrase again. */ + SHA512_Update (ctx, phrase, phr_size); + + /* Now get result of this (64 bytes) and add it to the other + context. */ + SHA512_Final (result, ctx); + + /* Prepare for the real work. */ + SHA512_Init (ctx); + + /* Add the phrase string. */ + SHA512_Update (ctx, phrase, phr_size); + + /* The last part is the salt string. This must be at most 8 + characters and it ends at the first `$' character (for + compatibility with existing implementations). */ + SHA512_Update (ctx, salt, salt_size); + + /* Add for any character in the phrase one byte of the alternate sum. */ + for (cnt = phr_size; cnt > 64; cnt -= 64) + SHA512_Update (ctx, result, 64); + SHA512_Update (ctx, result, cnt); + + /* Take the binary representation of the length of the phrase and for every + 1 add the alternate sum, for every 0 the phrase. */ + for (cnt = phr_size; cnt > 0; cnt >>= 1) + if ((cnt & 1) != 0) + SHA512_Update (ctx, result, 64); + else + SHA512_Update (ctx, phrase, phr_size); + + /* Create intermediate result. */ + SHA512_Final (result, ctx); + + /* Start computation of P byte sequence. */ + SHA512_Init (ctx); + + /* For every character in the password add the entire password. */ + for (cnt = 0; cnt < phr_size; ++cnt) + SHA512_Update (ctx, phrase, phr_size); + + /* Finish the digest. */ + SHA512_Final (p_bytes, ctx); + + /* Start computation of S byte sequence. */ + SHA512_Init (ctx); + + /* For every character in the password add the entire password. */ + for (cnt = 0; cnt < (size_t) 16 + (size_t) result[0]; ++cnt) + SHA512_Update (ctx, salt, salt_size); + + /* Finish the digest. */ + SHA512_Final (s_bytes, ctx); + + /* Repeatedly run the collected hash value through SHA512 to burn + CPU cycles. */ + for (cnt = 0; cnt < rounds; ++cnt) + { + /* New context. */ + SHA512_Init (ctx); + + /* Add phrase or last result. */ + if ((cnt & 1) != 0) + sha512_process_recycled_bytes (p_bytes, phr_size, ctx); + else + SHA512_Update (ctx, result, 64); + + /* Add salt for numbers not divisible by 3. */ + if (cnt % 3 != 0) + sha512_process_recycled_bytes (s_bytes, salt_size, ctx); + + /* Add phrase for numbers not divisible by 7. */ + if (cnt % 7 != 0) + sha512_process_recycled_bytes (p_bytes, phr_size, ctx); + + /* Add phrase or last result. */ + if ((cnt & 1) != 0) + SHA512_Update (ctx, result, 64); + else + sha512_process_recycled_bytes (p_bytes, phr_size, ctx); + + /* Create intermediate result. */ + SHA512_Final (result, ctx); + } + + /* Now we can construct the result string. It consists of four + parts, one of which is optional. We already know that buflen is + at least sha512_hash_length, therefore none of the string bashing + below can overflow the buffer. */ + + memcpy (cp, sha512_salt_prefix, sizeof (sha512_salt_prefix) - 1); + cp += sizeof (sha512_salt_prefix) - 1; + + if (rounds_custom) + { + int n = snprintf (cp, + SHA512_HASH_LENGTH - (sizeof (sha512_salt_prefix) - 1), + "%s%zu$", sha512_rounds_prefix, rounds); + cp += n; + } + + memcpy (cp, salt, salt_size); + cp += salt_size; + *cp++ = '$'; + +#define b64_from_24bit(B2, B1, B0, N) \ + do { \ + unsigned int w = ((((unsigned int)(B2)) << 16) | \ + (((unsigned int)(B1)) << 8) | \ + ((unsigned int)(B0))); \ + int n = (N); \ + while (n-- > 0) \ + { \ + *cp++ = b64t[w & 0x3f]; \ + w >>= 6; \ + } \ + } while (0) + + b64_from_24bit (result[0], result[21], result[42], 4); + b64_from_24bit (result[22], result[43], result[1], 4); + b64_from_24bit (result[44], result[2], result[23], 4); + b64_from_24bit (result[3], result[24], result[45], 4); + b64_from_24bit (result[25], result[46], result[4], 4); + b64_from_24bit (result[47], result[5], result[26], 4); + b64_from_24bit (result[6], result[27], result[48], 4); + b64_from_24bit (result[28], result[49], result[7], 4); + b64_from_24bit (result[50], result[8], result[29], 4); + b64_from_24bit (result[9], result[30], result[51], 4); + b64_from_24bit (result[31], result[52], result[10], 4); + b64_from_24bit (result[53], result[11], result[32], 4); + b64_from_24bit (result[12], result[33], result[54], 4); + b64_from_24bit (result[34], result[55], result[13], 4); + b64_from_24bit (result[56], result[14], result[35], 4); + b64_from_24bit (result[15], result[36], result[57], 4); + b64_from_24bit (result[37], result[58], result[16], 4); + b64_from_24bit (result[59], result[17], result[38], 4); + b64_from_24bit (result[18], result[39], result[60], 4); + b64_from_24bit (result[40], result[61], result[19], 4); + b64_from_24bit (result[62], result[20], result[41], 4); + b64_from_24bit (0, 0, result[63], 2); + + *cp = '\0'; +} + +#ifndef NO_GENSALT + +void +gensalt_sha512crypt_rn (unsigned long count, + const uint8_t *rbytes, size_t nrbytes, + uint8_t *output, size_t output_size) +{ + gensalt_sha_rn ('6', SALT_LEN_MAX, ROUNDS_DEFAULT, ROUNDS_MIN, ROUNDS_MAX, + count, rbytes, nrbytes, output, output_size); +} + +#endif + +#endif diff --git a/lib/crypt/crypt.c b/lib/crypt/crypt.c new file mode 100644 index 0000000000..4ec6079768 --- /dev/null +++ b/lib/crypt/crypt.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Copyright (C) 2020 Steffen Jaeckel jaeckel-floss@eyet-services.de */ + +#include <common.h> +#include <crypt.h> +#include "crypt-port.h" + +typedef void (*crypt_fn)(const char *, size_t, const char *, size_t, uint8_t *, + size_t, void *, size_t); + +const unsigned char ascii64[65] = + "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; + +static void equals_constant_time(const void *a_, const void *b_, size_t len, + int *equal) +{ + u8 ret = 0; + const u8 *a = a_, *b = b_; + int i; + + for (i = 0; i < len; i++) + ret |= a[i] ^ b[i]; + + ret |= ret >> 4; + ret |= ret >> 2; + ret |= ret >> 1; + ret &= 1; + + *equal = ret ^ 1; +} + +void crypt_compare(const char *should, const char *passphrase, int *equal) +{ + u8 output[CRYPT_OUTPUT_SIZE], scratch[ALG_SPECIFIC_SIZE]; + size_t n; + struct { + const char *prefix; + crypt_fn crypt; + } crypt_algos[] = { +#if defined(CONFIG_CRYPT_PW_SHA256) + { "$5$", crypt_sha256crypt_rn }, +#endif +#if defined(CONFIG_CRYPT_PW_SHA512) + { "$6$", crypt_sha512crypt_rn }, +#endif + { NULL, NULL } + }; + + *equal = 0; + + for (n = 0; n < ARRAY_SIZE(crypt_algos); ++n) { + if (!crypt_algos[n].prefix) + continue; + if (strncmp(should, crypt_algos[n].prefix, 3) == 0) + break; + } + + if (n >= ARRAY_SIZE(crypt_algos)) + return; + + crypt_algos[n].crypt(passphrase, strlen(passphrase), should, 0, output, + sizeof(output), scratch, sizeof(scratch)); + + /* early return on error, nothing really happened inside the crypt() function */ + if (errno == ERANGE || errno == EINVAL) + return; + + equals_constant_time(should, output, strlen((const char *)output), + equal); + + memset(scratch, 0, sizeof(scratch)); + memset(output, 0, sizeof(output)); +} diff --git a/test/Kconfig b/test/Kconfig index ab3ac54a1b..52eb1a7b53 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -38,6 +38,15 @@ config UT_LIB_ASN1 Enables a test which exercises asn1 compiler and decoder function via various parsers.
+config UT_LIB_CRYPT + bool "Unit test for crypt-style password hashing" + default y + select CRYPT_PW + select CRYPT_PW_SHA256 + select CRYPT_PW_SHA512 + help + Enables a test for the crypt-style password hash functions. + config UT_LIB_RSA bool "Unit test for rsa_verify() function" depends on RSA diff --git a/test/lib/Makefile b/test/lib/Makefile index aa2e66bc7f..6fd0514251 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_UT_LIB_ASN1) += asn1.o obj-$(CONFIG_UT_LIB_RSA) += rsa.o obj-$(CONFIG_AES) += test_aes.o obj-$(CONFIG_GETOPT) += getopt.o +obj-$(CONFIG_UT_LIB_CRYPT) += test_crypt.o diff --git a/test/lib/test_crypt.c b/test/lib/test_crypt.c new file mode 100644 index 0000000000..277e4efed1 --- /dev/null +++ b/test/lib/test_crypt.c @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2021 Steffen Jaeckel + * + * Unit test for crypt-style password hashing + */ + +#include <common.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h> + +#include <crypt.h> + +/** + * lib_crypt() - unit test for crypt-style password hashing + * + * @uts: unit test state + * Return: 0 = success, 1 = failure + */ +static int lib_crypt(struct unit_test_state *uts) +{ + int equals = 0; + + if (IS_ENABLED(CONFIG_CRYPT_PW_SHA256)) { + crypt_compare( + "$5$rounds=640000$TM4lL4zXDG7F4aRX$JM7a9wmvodnA0WasjTztj6mxg.KVuk6doQ/eBhdcapB", + "password", &equals); + ut_assertf(equals == 1, + "crypt-sha256 password hash didn't match\n"); + } + equals = 0; + if (IS_ENABLED(CONFIG_CRYPT_PW_SHA512)) { + crypt_compare( + "$6$rounds=640000$fCTP1F0N5JLq2eND$z5EzK5KZJA9JnOaj5d1Gg/2v6VqFOQJ3bVekWuCPauabutBt/8qzV1exJnytUyhbq3H0bSBXtodwNbtGEi/Tm/", + "password", &equals); + ut_assertf(equals == 1, + "crypt-sha512 password hash didn't match\n"); + } + + return CMD_RET_SUCCESS; +} + +LIB_TEST(lib_crypt, 0);

In order to prevent using the global errno, replace it with a static version and create a wrapper function which returns the error value.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
include/crypt.h | 3 ++- lib/crypt/alg-sha256.h | 6 ------ lib/crypt/alg-sha512.h | 6 ------ lib/crypt/crypt-port.h | 18 ++++++++++-------- lib/crypt/crypt-sha256.c | 26 ++++++++++++++++++++++++-- lib/crypt/crypt-sha512.c | 26 ++++++++++++++++++++++++-- lib/crypt/crypt.c | 25 ++++++++++++++----------- test/lib/test_crypt.c | 24 ++++++++++++++++++++++-- 8 files changed, 96 insertions(+), 38 deletions(-)
diff --git a/include/crypt.h b/include/crypt.h index e0be2832ff..f18a1705d4 100644 --- a/include/crypt.h +++ b/include/crypt.h @@ -9,5 +9,6 @@ * @equal Pointer to an int where the result is stored * '0' = unequal * '1' = equal + * @return 0 on success, error code of errno else */ -void crypt_compare(const char *should, const char *passphrase, int *equal); +int crypt_compare(const char *should, const char *passphrase, int *equal); diff --git a/lib/crypt/alg-sha256.h b/lib/crypt/alg-sha256.h index e4b29c9f31..62e7b9d5c0 100644 --- a/lib/crypt/alg-sha256.h +++ b/lib/crypt/alg-sha256.h @@ -1,12 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /* Copyright (C) 2020 Steffen Jaeckel jaeckel-floss@eyet-services.de */
-#ifndef USE_HOSTCC -#include "common.h" -#else -#include <string.h> -#endif - #include "u-boot/sha256.h"
#define INCLUDE_sha256crypt 1 diff --git a/lib/crypt/alg-sha512.h b/lib/crypt/alg-sha512.h index 93b6109fae..47e45730cc 100644 --- a/lib/crypt/alg-sha512.h +++ b/lib/crypt/alg-sha512.h @@ -1,12 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ /* Copyright (C) 2020 Steffen Jaeckel jaeckel-floss@eyet-services.de */
-#ifndef USE_HOSTCC -#include "common.h" -#else -#include <string.h> -#endif - #include "u-boot/sha512.h"
#define INCLUDE_sha512crypt 1 diff --git a/lib/crypt/crypt-port.h b/lib/crypt/crypt-port.h index 680ffe9349..6b9542d75b 100644 --- a/lib/crypt/crypt-port.h +++ b/lib/crypt/crypt-port.h @@ -18,11 +18,13 @@ extern const unsigned char ascii64[65];
#define b64t ((const char *)ascii64)
-void crypt_sha256crypt_rn(const char *phrase, size_t phr_size, - const char *setting, size_t ARG_UNUSED(set_size), - uint8_t *output, size_t out_size, void *scratch, - size_t scr_size); -void crypt_sha512crypt_rn(const char *phrase, size_t phr_size, - const char *setting, size_t ARG_UNUSED(set_size), - uint8_t *output, size_t out_size, void *scratch, - size_t scr_size); +int crypt_sha256crypt_rn_wrapped(const char *phrase, size_t phr_size, + const char *setting, + size_t ARG_UNUSED(set_size), uint8_t *output, + size_t out_size, void *scratch, + size_t scr_size); +int crypt_sha512crypt_rn_wrapped(const char *phrase, size_t phr_size, + const char *setting, + size_t ARG_UNUSED(set_size), uint8_t *output, + size_t out_size, void *scratch, + size_t scr_size); diff --git a/lib/crypt/crypt-sha256.c b/lib/crypt/crypt-sha256.c index 37127d41e1..e1c1eff060 100644 --- a/lib/crypt/crypt-sha256.c +++ b/lib/crypt/crypt-sha256.c @@ -1,10 +1,13 @@ +// SPDX-License-Identifier: CC0-1.0 /* One way encryption based on the SHA256-based Unix crypt implementation. * * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1]. * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018. * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018. * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020. - * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2020. + * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2021 + * for U-Boot, instead of using the global errno to use a static one + * inside this file. * To the extent possible under law, the named authors have waived all * copyright and related or neighboring rights to this work. * @@ -20,7 +23,7 @@ #include "crypt-port.h" #include "alg-sha256.h"
-#include <errno.h> +#include <linux/errno.h> #include <stdio.h> #include <stdlib.h>
@@ -69,6 +72,25 @@ static_assert (sizeof (struct sha256_buffer) <= ALG_SPECIFIC_SIZE, "ALG_SPECIFIC_SIZE is too small for SHA256");
+/* Use this instead of including errno.h */ +static int errno; + +void crypt_sha256crypt_rn(const char *phrase, size_t phr_size, + const char *setting, size_t ARG_UNUSED(set_size), + uint8_t *output, size_t out_size, void *scratch, + size_t scr_size); + +int crypt_sha256crypt_rn_wrapped(const char *phrase, size_t phr_size, + const char *setting, size_t set_size, + u8 *output, size_t out_size, void *scratch, + size_t scr_size) +{ + errno = 0; + crypt_sha256crypt_rn(phrase, phr_size, setting, set_size, output, + out_size, scratch, scr_size); + return -errno; +} + /* Feed CTX with LEN bytes of a virtual byte sequence consisting of BLOCK repeated over and over indefinitely. */ static void diff --git a/lib/crypt/crypt-sha512.c b/lib/crypt/crypt-sha512.c index 3616019445..6f5be3b460 100644 --- a/lib/crypt/crypt-sha512.c +++ b/lib/crypt/crypt-sha512.c @@ -1,10 +1,13 @@ +// SPDX-License-Identifier: CC0-1.0 /* One way encryption based on the SHA512-based Unix crypt implementation. * * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1]. * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018. * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018. * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020. - * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2020. + * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2021 + * for U-Boot, instead of using the global errno to use a static one + * inside this file. * To the extent possible under law, the named authors have waived all * copyright and related or neighboring rights to this work. * @@ -20,7 +23,7 @@ #include "crypt-port.h" #include "alg-sha512.h"
-#include <errno.h> +#include <linux/errno.h> #include <stdio.h> #include <stdlib.h>
@@ -69,6 +72,25 @@ static_assert (sizeof (struct sha512_buffer) <= ALG_SPECIFIC_SIZE, "ALG_SPECIFIC_SIZE is too small for SHA512");
+/* Use this instead of including errno.h */ +static int errno; + +void crypt_sha512crypt_rn(const char *phrase, size_t phr_size, + const char *setting, size_t ARG_UNUSED(set_size), + uint8_t *output, size_t out_size, void *scratch, + size_t scr_size); + +int crypt_sha512crypt_rn_wrapped(const char *phrase, size_t phr_size, + const char *setting, size_t set_size, + u8 *output, size_t out_size, void *scratch, + size_t scr_size) +{ + errno = 0; + crypt_sha512crypt_rn(phrase, phr_size, setting, set_size, output, + out_size, scratch, scr_size); + return -errno; +} + /* Subroutine of _xcrypt_crypt_sha512crypt_rn: Feed CTX with LEN bytes of a virtual byte sequence consisting of BLOCK repeated over and over indefinitely. */ diff --git a/lib/crypt/crypt.c b/lib/crypt/crypt.c index 4ec6079768..247c34b2a9 100644 --- a/lib/crypt/crypt.c +++ b/lib/crypt/crypt.c @@ -5,8 +5,8 @@ #include <crypt.h> #include "crypt-port.h"
-typedef void (*crypt_fn)(const char *, size_t, const char *, size_t, uint8_t *, - size_t, void *, size_t); +typedef int (*crypt_fn)(const char *, size_t, const char *, size_t, uint8_t *, + size_t, void *, size_t);
const unsigned char ascii64[65] = "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"; @@ -29,19 +29,20 @@ static void equals_constant_time(const void *a_, const void *b_, size_t len, *equal = ret ^ 1; }
-void crypt_compare(const char *should, const char *passphrase, int *equal) +int crypt_compare(const char *should, const char *passphrase, int *equal) { u8 output[CRYPT_OUTPUT_SIZE], scratch[ALG_SPECIFIC_SIZE]; size_t n; + int err; struct { const char *prefix; crypt_fn crypt; } crypt_algos[] = { #if defined(CONFIG_CRYPT_PW_SHA256) - { "$5$", crypt_sha256crypt_rn }, + { "$5$", crypt_sha256crypt_rn_wrapped }, #endif #if defined(CONFIG_CRYPT_PW_SHA512) - { "$6$", crypt_sha512crypt_rn }, + { "$6$", crypt_sha512crypt_rn_wrapped }, #endif { NULL, NULL } }; @@ -56,18 +57,20 @@ void crypt_compare(const char *should, const char *passphrase, int *equal) }
if (n >= ARRAY_SIZE(crypt_algos)) - return; - - crypt_algos[n].crypt(passphrase, strlen(passphrase), should, 0, output, - sizeof(output), scratch, sizeof(scratch)); + return -EINVAL;
+ err = crypt_algos[n].crypt(passphrase, strlen(passphrase), should, 0, + output, sizeof(output), scratch, + sizeof(scratch)); /* early return on error, nothing really happened inside the crypt() function */ - if (errno == ERANGE || errno == EINVAL) - return; + if (err) + return err;
equals_constant_time(should, output, strlen((const char *)output), equal);
memset(scratch, 0, sizeof(scratch)); memset(output, 0, sizeof(output)); + + return 0; } diff --git a/test/lib/test_crypt.c b/test/lib/test_crypt.c index 277e4efed1..fb21edf974 100644 --- a/test/lib/test_crypt.c +++ b/test/lib/test_crypt.c @@ -21,19 +21,39 @@ static int lib_crypt(struct unit_test_state *uts) { int equals = 0; + int err; + + err = crypt_compare("", "password", &equals); + ut_assertf(err != 0, "crypt_compare successful but should not\n"); + ut_assertf(equals != 1, + "crypt_compare password hash matched but should not\n");
if (IS_ENABLED(CONFIG_CRYPT_PW_SHA256)) { - crypt_compare( + err = crypt_compare("$5$", "password", &equals); + ut_assertf(err == 0, "crypt-sha256 not successful\n"); + ut_assertf( + equals != 1, + "crypt-sha256 password hash matched but should not\n"); + + err = crypt_compare( "$5$rounds=640000$TM4lL4zXDG7F4aRX$JM7a9wmvodnA0WasjTztj6mxg.KVuk6doQ/eBhdcapB", "password", &equals); + ut_assertf(err == 0, "crypt-sha256 failed: %d\n", err); ut_assertf(equals == 1, "crypt-sha256 password hash didn't match\n"); } equals = 0; if (IS_ENABLED(CONFIG_CRYPT_PW_SHA512)) { - crypt_compare( + err = crypt_compare("$6$", "password", &equals); + ut_assertf(err == 0, "crypt-sha512 not successful\n"); + ut_assertf( + equals != 1, + "crypt-sha512 password hash matched but should not\n"); + + err = crypt_compare( "$6$rounds=640000$fCTP1F0N5JLq2eND$z5EzK5KZJA9JnOaj5d1Gg/2v6VqFOQJ3bVekWuCPauabutBt/8qzV1exJnytUyhbq3H0bSBXtodwNbtGEi/Tm/", "password", &equals); + ut_assertf(err == 0, "crypt-sha512 failed: %d\n", err); ut_assertf(equals == 1, "crypt-sha512 password hash didn't match\n"); }

Hook into the autoboot flow as an alternative to the existing mechanisms.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/Kconfig.boot | 38 ++++++++++++++++++--- common/autoboot.c | 80 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 104 insertions(+), 14 deletions(-)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 89a3161f1f..410d117cdd 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -811,10 +811,17 @@ config AUTOBOOT_ENCRYPTION depends on AUTOBOOT_KEYED help This option allows a string to be entered into U-Boot to stop the - autoboot. The string itself is hashed and compared against the hash - in the environment variable 'bootstopkeysha256'. If it matches then - boot stops and a command-line prompt is presented. - + autoboot. + The behavior depends whether CONFIG_CRYPT_PW from lib is enabled + or not. + In case CONFIG_CRYPT_PW is enabled, the string will be forwarded + to the crypt-based functionality and be compared against the + string in the environment variable 'bootstopkeycrypt'. + In case CONFIG_CRYPT_PW is disabled the string itself is hashed + and compared against the hash in the environment variable + 'bootstopkeysha256'. + If it matches in either case then boot stops and + a command-line prompt is presented. This provides a way to ship a secure production device which can also be accessed at the U-Boot command line.
@@ -852,9 +859,30 @@ config AUTOBOOT_KEYED_CTRLC Setting this variable provides an escape sequence from the limited "password" strings.
+config AUTOBOOT_STOP_STR_ENABLE + bool "Enable fixed string to stop autobooting" + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION + help + This option enables the feature to add a fixed stop + string that is defined at compile time. + In every case it will be tried to load the stop + string from the environment. + In case this is enabled and there is no stop string + in the environment, this will be used as default value. + +config AUTOBOOT_STOP_STR_CRYPT + string "Stop autobooting via crypt-hashed password" + depends on AUTOBOOT_STOP_STR_ENABLE && CRYPT_PW + help + 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 hashed via + one of the supported crypt-style password hashing options + and saved in the environment variable "bootstopkeycrypt". + config AUTOBOOT_STOP_STR_SHA256 string "Stop autobooting via SHA256 encrypted password" - depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION + depends on AUTOBOOT_STOP_STR_ENABLE help This option adds the feature to only stop the autobooting, and therefore boot into the U-Boot prompt, when the input diff --git a/common/autoboot.c b/common/autoboot.c index 0bb08e7a4c..d52f88dd54 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -23,6 +23,7 @@ #include <linux/delay.h> #include <u-boot/sha256.h> #include <bootcount.h> +#include <crypt.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -38,18 +39,75 @@ DECLARE_GLOBAL_DATA_PTR; static int stored_bootdelay; static int menukey;
-#ifdef CONFIG_AUTOBOOT_ENCRYPTION -#define AUTOBOOT_STOP_STR_SHA256 CONFIG_AUTOBOOT_STOP_STR_SHA256 -#else -#define AUTOBOOT_STOP_STR_SHA256 "" +#if !defined(CONFIG_AUTOBOOT_STOP_STR_CRYPT) +#define CONFIG_AUTOBOOT_STOP_STR_CRYPT "" +#endif +#if !defined(CONFIG_AUTOBOOT_STOP_STR_SHA256) +#define CONFIG_AUTOBOOT_STOP_STR_SHA256 "" #endif - #ifdef CONFIG_USE_AUTOBOOT_MENUKEY #define AUTOBOOT_MENUKEY CONFIG_USE_AUTOBOOT_MENUKEY #else #define AUTOBOOT_MENUKEY 0 #endif
+/** + * passwd_abort_crypt() - check for a crypt-style hashed key sequence to abort booting + * + * This checks for the user entering a password within a given time. + * + * The entered password is hashed via one of the crypt-style hash methods + * and compared to the pre-defined value from either + * the environment variable "bootstopkeycrypt" + * or + * the config value CONFIG_AUTOBOOT_STOP_STR_CRYPT + * + * @etime: Timeout value ticks (stop when get_ticks() reachs this) + * @return 0 if autoboot should continue, 1 if it should stop + */ +static int passwd_abort_crypt(uint64_t etime) +{ + const char *crypt_env_str = env_get("bootstopkeycrypt"); + char presskey[MAX_DELAY_STOP_STR]; + u_int presskey_len = 0; + int abort = 0; + int err; + + if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_ENABLE) && !crypt_env_str) + crypt_env_str = CONFIG_AUTOBOOT_STOP_STR_CRYPT; + + if (!crypt_env_str) + return 0; + + /* We expect the stop-string to be newline-terminated */ + do { + if (tstc()) { + /* Check for input string overflow */ + if (presskey_len >= sizeof(presskey)) + return 0; + + presskey[presskey_len] = getchar(); + + if ((presskey[presskey_len] == '\r') || + (presskey[presskey_len] == '\n')) { + presskey[presskey_len] = '\0'; + err = crypt_compare(crypt_env_str, presskey, + &abort); + if (err) + debug_bootkeys( + "crypt_compare() failed with: %s\n", + errno_str(err)); + /* you had one chance */ + break; + } else { + presskey_len++; + } + } + } while (get_ticks() <= etime); + + return abort; +} + /* * Use a "constant-length" time compare function for this * hash compare: @@ -89,7 +147,7 @@ static int passwd_abort_sha256(uint64_t etime) int ret;
if (sha_env_str == NULL) - sha_env_str = AUTOBOOT_STOP_STR_SHA256; + sha_env_str = CONFIG_AUTOBOOT_STOP_STR_SHA256;
presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR); c = strstr(sha_env_str, ":"); @@ -245,10 +303,14 @@ static int abortboot_key_sequence(int bootdelay) printf(CONFIG_AUTOBOOT_PROMPT, bootdelay); # endif
- if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) - abort = passwd_abort_sha256(etime); - else + if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) { + if (IS_ENABLED(CONFIG_CRYPT_PW)) + abort = passwd_abort_crypt(etime); + else + abort = passwd_abort_sha256(etime); + } else { abort = passwd_abort_key(etime); + } if (!abort) debug_bootkeys("key timeout\n");

While doing code-review internally this got nitpicked by 2 reviewers, so I decided to include this here.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/autoboot.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/autoboot.c b/common/autoboot.c index d52f88dd54..e03c387a1f 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -27,7 +27,7 @@
DECLARE_GLOBAL_DATA_PTR;
-#define MAX_DELAY_STOP_STR 64 +#define DELAY_STOP_STR_MAX_LENGTH 64
#ifndef DEBUG_BOOTKEYS #define DEBUG_BOOTKEYS 0 @@ -68,7 +68,7 @@ static int menukey; static int passwd_abort_crypt(uint64_t etime) { const char *crypt_env_str = env_get("bootstopkeycrypt"); - char presskey[MAX_DELAY_STOP_STR]; + char presskey[DELAY_STOP_STR_MAX_LENGTH]; u_int presskey_len = 0; int abort = 0; int err; @@ -149,9 +149,9 @@ static int passwd_abort_sha256(uint64_t etime) if (sha_env_str == NULL) sha_env_str = CONFIG_AUTOBOOT_STOP_STR_SHA256;
- presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR); + presskey = malloc_cache_aligned(DELAY_STOP_STR_MAX_LENGTH); c = strstr(sha_env_str, ":"); - if (c && (c - sha_env_str < MAX_DELAY_STOP_STR)) { + if (c && (c - sha_env_str < DELAY_STOP_STR_MAX_LENGTH)) { /* preload presskey with salt */ memcpy(presskey, sha_env_str, c - sha_env_str); presskey_len = c - sha_env_str; @@ -178,7 +178,7 @@ static int passwd_abort_sha256(uint64_t etime) do { if (tstc()) { /* Check for input string overflow */ - if (presskey_len >= MAX_DELAY_STOP_STR) { + if (presskey_len >= DELAY_STOP_STR_MAX_LENGTH) { free(presskey); free(sha); return 0; @@ -222,7 +222,7 @@ static int passwd_abort_key(uint64_t etime) { .str = env_get("bootstopkey"), .retry = 0 }, };
- char presskey[MAX_DELAY_STOP_STR]; + char presskey[DELAY_STOP_STR_MAX_LENGTH]; int presskey_len = 0; int presskey_max = 0; int i; @@ -239,8 +239,8 @@ static int passwd_abort_key(uint64_t etime) for (i = 0; i < sizeof(delaykey) / sizeof(delaykey[0]); i++) { delaykey[i].len = delaykey[i].str == NULL ? 0 : strlen(delaykey[i].str); - delaykey[i].len = delaykey[i].len > MAX_DELAY_STOP_STR ? - MAX_DELAY_STOP_STR : delaykey[i].len; + delaykey[i].len = delaykey[i].len > DELAY_STOP_STR_MAX_LENGTH ? + DELAY_STOP_STR_MAX_LENGTH : delaykey[i].len;
presskey_max = presskey_max > delaykey[i].len ? presskey_max : delaykey[i].len;

In case a user has to enter a complicated password it is sometimes desireable to give the user more time than the default timeout. Enabling this feature will disable the timeout entirely in case the user presses the <Enter> key before entering any other character.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
common/Kconfig.boot | 8 ++++++++ common/autoboot.c | 12 +++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 410d117cdd..e70dcf17cb 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -859,6 +859,14 @@ config AUTOBOOT_KEYED_CTRLC Setting this variable provides an escape sequence from the limited "password" strings.
+config AUTOBOOT_NEVER_TIMEOUT + bool "Make the password entry never time-out" + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION && CRYPT_PW + help + This option removes the timeout from the password entry + when the user first presses the <Enter> key before entering + any other character. + config AUTOBOOT_STOP_STR_ENABLE bool "Enable fixed string to stop autobooting" depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION diff --git a/common/autoboot.c b/common/autoboot.c index e03c387a1f..50ab9281e7 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -62,6 +62,10 @@ static int menukey; * or * the config value CONFIG_AUTOBOOT_STOP_STR_CRYPT * + * In case the config value CONFIG_AUTOBOOT_NEVER_TIMEOUT has been enabled + * this function never times out if the user presses the <Enter> key + * before starting to enter the password. + * * @etime: Timeout value ticks (stop when get_ticks() reachs this) * @return 0 if autoboot should continue, 1 if it should stop */ @@ -71,6 +75,7 @@ static int passwd_abort_crypt(uint64_t etime) char presskey[DELAY_STOP_STR_MAX_LENGTH]; u_int presskey_len = 0; int abort = 0; + int never_timeout = 0; int err;
if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_ENABLE) && !crypt_env_str) @@ -90,6 +95,11 @@ static int passwd_abort_crypt(uint64_t etime)
if ((presskey[presskey_len] == '\r') || (presskey[presskey_len] == '\n')) { + if (IS_ENABLED(CONFIG_AUTOBOOT_NEVER_TIMEOUT) && + !presskey_len) { + never_timeout = 1; + continue; + } presskey[presskey_len] = '\0'; err = crypt_compare(crypt_env_str, presskey, &abort); @@ -103,7 +113,7 @@ static int passwd_abort_crypt(uint64_t etime) presskey_len++; } } - } while (get_ticks() <= etime); + } while (never_timeout || get_ticks() <= etime);
return abort; }

The key-sequence based unlock mechanisms are sensitive to junk symbols that could have been sent to stdin and are still waiting to be retrieved. Enabling this option will read all symbols off stdin before displaying the autoboot prompt (and starting to read the password from stdin).
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de ---
(no changes since v1)
common/Kconfig.boot | 9 +++++++++ common/autoboot.c | 11 +++++++++++ 2 files changed, 20 insertions(+)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index e70dcf17cb..d19bc32836 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -790,6 +790,15 @@ config AUTOBOOT_KEYED U-Boot automatic booting process and bring the device to the U-Boot prompt for user input.
+config AUTOBOOT_FLUSH_STDIN + bool "Enable flushing stdin before starting to read the password" + depends on AUTOBOOT_KEYED && !SANDBOX + help + When this option is enabled stdin buffer will be flushed before + starting to read the password. + This can't be enabled for the sandbox as flushing stdin would + break the autoboot unit tests. + config AUTOBOOT_PROMPT string "Autoboot stop prompt" depends on AUTOBOOT_KEYED diff --git a/common/autoboot.c b/common/autoboot.c index 50ab9281e7..1eeabf0b1a 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -296,6 +296,15 @@ static int passwd_abort_key(uint64_t etime) return abort; }
+/** + * flush_stdin() - drops all pending characters from stdin + */ +static void flush_stdin(void) +{ + while (tstc()) + (void)getchar(); +} + /*************************************************************************** * Watch for 'delay' seconds for autoboot stop or autoboot delay string. * returns: 0 - no key string, allow autoboot 1 - got key string, abort @@ -305,6 +314,8 @@ static int abortboot_key_sequence(int bootdelay) int abort; uint64_t etime = endtick(bootdelay);
+ if (IS_ENABLED(CONFIG_AUTOBOOT_FLUSH_STDIN)) + flush_stdin(); # ifdef CONFIG_AUTOBOOT_PROMPT /* * CONFIG_AUTOBOOT_PROMPT includes the %d for all boards.

On Mon, 21 Jun 2021 at 14:24, Steffen Jaeckel jaeckel-floss@eyet-services.de wrote:
The key-sequence based unlock mechanisms are sensitive to junk symbols that could have been sent to stdin and are still waiting to be retrieved. Enabling this option will read all symbols off stdin before displaying the autoboot prompt (and starting to read the password from stdin).
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de
(no changes since v1)
common/Kconfig.boot | 9 +++++++++ common/autoboot.c | 11 +++++++++++ 2 files changed, 20 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

In case crypt-based hashing is enabled this will be the default mechanism that is used. If a user wants to have support for both, the environment variable `bootstopusesha256` can be set to `true` to allow plain SHA256 based hashing of the password.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de ---
(no changes since v1)
common/Kconfig.boot | 8 ++++++++ common/autoboot.c | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index d19bc32836..b04a6c98e5 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -834,6 +834,14 @@ config AUTOBOOT_ENCRYPTION This provides a way to ship a secure production device which can also be accessed at the U-Boot command line.
+config AUTOBOOT_SHA256_FALLBACK_ENABLE + bool "Allow fallback from crypt-hashed password to sha256" + depends on AUTOBOOT_ENCRYPTION && CRYPT_PW + help + This option adds support to fall back from crypt-hashed + passwords to checking a SHA256 hashed password in case the + 'bootstopusesha256' environment variable is set to 'true'. + config AUTOBOOT_DELAY_STR string "Delay autobooting via specific input key / string" depends on AUTOBOOT_KEYED && !AUTOBOOT_ENCRYPTION diff --git a/common/autoboot.c b/common/autoboot.c index 1eeabf0b1a..c6f550b8a7 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -305,6 +305,26 @@ static void flush_stdin(void) (void)getchar(); }
+/** + * fallback_to_sha256() - check whether we should fall back to sha256 + * password checking + * + * This checks for the environment variable `bootstopusesha256` in case + * sha256-fallback has been enabled via the config setting + * `AUTOBOOT_SHA256_FALLBACK_ENABLE`. + * + * @return 0 if we must not fall-back, 1 if plain sha256 should be tried + */ +static int fallback_to_sha256(void) +{ + if (IS_ENABLED(CONFIG_AUTOBOOT_SHA256_FALLBACK_ENABLE)) + return env_get_yesno("bootstopusesha256") == 1; + else if (IS_ENABLED(CONFIG_CRYPT_PW)) + return 0; + else + return 1; +} + /*************************************************************************** * Watch for 'delay' seconds for autoboot stop or autoboot delay string. * returns: 0 - no key string, allow autoboot 1 - got key string, abort @@ -325,7 +345,7 @@ static int abortboot_key_sequence(int bootdelay) # endif
if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) { - if (IS_ENABLED(CONFIG_CRYPT_PW)) + if (IS_ENABLED(CONFIG_CRYPT_PW) && !fallback_to_sha256()) abort = passwd_abort_crypt(etime); else abort = passwd_abort_sha256(etime);

On Mon, 21 Jun 2021 at 14:24, Steffen Jaeckel jaeckel-floss@eyet-services.de wrote:
In case crypt-based hashing is enabled this will be the default mechanism that is used. If a user wants to have support for both, the environment variable `bootstopusesha256` can be set to `true` to allow plain SHA256 based hashing of the password.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de
(no changes since v1)
common/Kconfig.boot | 8 ++++++++ common/autoboot.c | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
nits below
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index d19bc32836..b04a6c98e5 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -834,6 +834,14 @@ config AUTOBOOT_ENCRYPTION This provides a way to ship a secure production device which can also be accessed at the U-Boot command line.
+config AUTOBOOT_SHA256_FALLBACK_ENABLE
Drop _ENABLE
bool "Allow fallback from crypt-hashed password to sha256"
depends on AUTOBOOT_ENCRYPTION && CRYPT_PW
help
This option adds support to fall back from crypt-hashed
passwords to checking a SHA256 hashed password in case the
'bootstopusesha256' environment variable is set to 'true'.
config AUTOBOOT_DELAY_STR string "Delay autobooting via specific input key / string" depends on AUTOBOOT_KEYED && !AUTOBOOT_ENCRYPTION diff --git a/common/autoboot.c b/common/autoboot.c index 1eeabf0b1a..c6f550b8a7 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -305,6 +305,26 @@ static void flush_stdin(void) (void)getchar(); }
+/**
- fallback_to_sha256() - check whether we should fall back to sha256
password checking
- This checks for the environment variable `bootstopusesha256` in case
- sha256-fallback has been enabled via the config setting
- `AUTOBOOT_SHA256_FALLBACK_ENABLE`.
- @return 0 if we must not fall-back, 1 if plain sha256 should be tried
false / true
- */
+static int fallback_to_sha256(void)
bool
+{
if (IS_ENABLED(CONFIG_AUTOBOOT_SHA256_FALLBACK_ENABLE))
return env_get_yesno("bootstopusesha256") == 1;
else if (IS_ENABLED(CONFIG_CRYPT_PW))
return 0;
false
else
return 1;
true
+}
/***************************************************************************
- Watch for 'delay' seconds for autoboot stop or autoboot delay string.
- returns: 0 - no key string, allow autoboot 1 - got key string, abort
@@ -325,7 +345,7 @@ static int abortboot_key_sequence(int bootdelay) # endif
if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
if (IS_ENABLED(CONFIG_CRYPT_PW))
if (IS_ENABLED(CONFIG_CRYPT_PW) && !fallback_to_sha256()) abort = passwd_abort_crypt(etime); else abort = passwd_abort_sha256(etime);
-- 2.31.1
Regards, Simon

This adds tests for the crypt-based and plain SHA256-based password hashing algorithms in the autoboot flow.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de ---
(no changes since v1)
common/Kconfig.boot | 2 +- common/console.c | 5 +++ configs/sandbox_defconfig | 11 +++++ include/console.h | 17 +++++++ include/test/common.h | 15 +++++++ include/test/suites.h | 1 + test/Makefile | 1 + test/cmd_ut.c | 1 + test/common/Makefile | 3 ++ test/common/cmd_ut_common.c | 22 +++++++++ test/common/test_autoboot.c | 90 +++++++++++++++++++++++++++++++++++++ 11 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 include/test/common.h create mode 100644 test/common/Makefile create mode 100644 test/common/cmd_ut_common.c create mode 100644 test/common/test_autoboot.c
diff --git a/common/Kconfig.boot b/common/Kconfig.boot index b04a6c98e5..308d952066 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -906,7 +906,7 @@ config AUTOBOOT_STOP_STR_CRYPT and saved in the environment variable "bootstopkeycrypt".
config AUTOBOOT_STOP_STR_SHA256 - string "Stop autobooting via SHA256 encrypted password" + string "Stop autobooting via SHA256 hashed password" depends on AUTOBOOT_STOP_STR_ENABLE help This option adds the feature to only stop the autobooting, diff --git a/common/console.c b/common/console.c index 561cdf36a7..9d5060d5ce 100644 --- a/common/console.c +++ b/common/console.c @@ -763,6 +763,11 @@ int console_record_avail(void) return membuff_avail((struct membuff *)&gd->console_out); }
+int console_in_puts(const char *str) +{ + return membuff_put((struct membuff *)&gd->console_in, str, strlen(str)); +} + #endif
/* test if ctrl-c was pressed */ diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index bdbf714e2b..1cd4f491aa 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -284,6 +284,17 @@ CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y CONFIG_EFI_SECURE_BOOT=y CONFIG_TEST_FDTDEC=y +CONFIG_CRYPT_PW=y +CONFIG_CRYPT_PW_SHA256=y +CONFIG_CRYPT_PW_SHA512=y +CONFIG_AUTOBOOT_KEYED=y +CONFIG_AUTOBOOT_PROMPT="Enter password "a" in %d seconds to stop autoboot\n" +CONFIG_AUTOBOOT_ENCRYPTION=y +CONFIG_AUTOBOOT_STOP_STR_ENABLE=y +# default password "a" +CONFIG_AUTOBOOT_STOP_STR_CRYPT="$5$rounds=640000$HrpE65IkB8CM5nCL$BKT3QdF98Bo8fJpTr9tjZLZQyzqPASBY20xuK5Rent9" +CONFIG_AUTOBOOT_NEVER_TIMEOUT=y +CONFIG_AUTOBOOT_SHA256_FALLBACK_ENABLE=y CONFIG_UNIT_TEST=y CONFIG_UT_TIME=y CONFIG_UT_DM=y diff --git a/include/console.h b/include/console.h index 7e628c0cf8..6be045ddcc 100644 --- a/include/console.h +++ b/include/console.h @@ -82,6 +82,17 @@ int console_record_readline(char *str, int maxlen); * @return available bytes (0 if empty) */ int console_record_avail(void); + +/** + * console_in_puts() - Write a string to the console input buffer + * + * This writes the given string to the console_in buffer which will then be + * returned if a function calls e.g. `getc()` + * + * @str: the string to write + * @return the number of bytes added + */ +int console_in_puts(const char *str); #else static inline int console_record_init(void) { @@ -113,6 +124,12 @@ static inline int console_record_avail(void) return 0; }
+static inline int console_in_puts(const char *str) +{ + /* There is never anything written */ + return 0; +} + #endif /* !CONFIG_CONSOLE_RECORD */
/** diff --git a/include/test/common.h b/include/test/common.h new file mode 100644 index 0000000000..81260d06ad --- /dev/null +++ b/include/test/common.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2019 Heinrich Schuchardt xypron.glpk@gmx.de + * Copyright (c) 2021 Steffen Jaeckel jaeckel-floss@eyet-services.de + */ + +#ifndef __TEST_COMMON_H__ +#define __TEST_COMMON_H__ + +#include <test/test.h> + +/* Declare a new common function test */ +#define COMMON_TEST(_name, _flags) UNIT_TEST(_name, _flags, common_test) + +#endif /* __TEST_COMMON_H__ */ diff --git a/include/test/suites.h b/include/test/suites.h index f5d8e139ce..3765bf9ffd 100644 --- a/include/test/suites.h +++ b/include/test/suites.h @@ -31,6 +31,7 @@ int do_ut_addrmap(struct cmd_tbl *cmdtp, int flag, int argc, int do_ut_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_bloblist(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); +int do_ut_common(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_compression(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); int do_ut_dm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); diff --git a/test/Makefile b/test/Makefile index a26e915e05..afc7864a54 100644 --- a/test/Makefile +++ b/test/Makefile @@ -20,6 +20,7 @@ obj-$(CONFIG_UT_TIME) += time_ut.o obj-y += ut.o
ifeq ($(CONFIG_SPL_BUILD),) +obj-$(CONFIG_UNIT_TEST) += common/ obj-$(CONFIG_UNIT_TEST) += lib/ obj-y += log/ obj-$(CONFIG_$(SPL_)UT_UNICODE) += unicode_ut.o diff --git a/test/cmd_ut.c b/test/cmd_ut.c index b9c166045d..cf6c24b509 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -28,6 +28,7 @@ int cmd_ut_category(const char *name, const char *prefix,
static struct cmd_tbl cmd_ut_sub[] = { U_BOOT_CMD_MKENT(all, CONFIG_SYS_MAXARGS, 1, do_ut_all, "", ""), + U_BOOT_CMD_MKENT(common, CONFIG_SYS_MAXARGS, 1, do_ut_common, "", ""), #if defined(CONFIG_UT_DM) U_BOOT_CMD_MKENT(dm, CONFIG_SYS_MAXARGS, 1, do_ut_dm, "", ""), #endif diff --git a/test/common/Makefile b/test/common/Makefile new file mode 100644 index 0000000000..24c9145dcc --- /dev/null +++ b/test/common/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0+ +obj-y += cmd_ut_common.o +obj-$(CONFIG_AUTOBOOT) += test_autoboot.o diff --git a/test/common/cmd_ut_common.c b/test/common/cmd_ut_common.c new file mode 100644 index 0000000000..2c0267801b --- /dev/null +++ b/test/common/cmd_ut_common.c @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2019 Heinrich Schuchardt xypron.glpk@gmx.de + * Copyright (c) 2021 Steffen Jaeckel jaeckel-floss@eyet-services.de + * + * Unit tests for common functions + */ + +#include <common.h> +#include <command.h> +#include <test/common.h> +#include <test/suites.h> +#include <test/ut.h> + +int do_ut_common(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{ + struct unit_test *tests = UNIT_TEST_SUITE_START(common_test); + const int n_ents = UNIT_TEST_SUITE_COUNT(common_test); + + return cmd_ut_category("common", "common_test_", tests, n_ents, argc, + argv); +} diff --git a/test/common/test_autoboot.c b/test/common/test_autoboot.c new file mode 100644 index 0000000000..8dc555f760 --- /dev/null +++ b/test/common/test_autoboot.c @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2021 Steffen Jaeckel + * + * Unit tests for autoboot functionality + */ + +#include <autoboot.h> +#include <common.h> +#include <test/common.h> +#include <test/test.h> +#include <test/ut.h> + +#include <crypt.h> + +static int check_for_input(struct unit_test_state *uts, const char *in, + bool correct) +{ + /* The bootdelay is set to 1 second in test_autoboot() */ + const char *autoboot_prompt = + "Enter password "a" in 1 seconds to stop autoboot"; + + console_record_reset_enable(); + console_in_puts(in); + autoboot_command("echo Autoboot password unlock not successful"); + ut_assert_nextline(autoboot_prompt); + if (!correct) { + ut_assert_nextline("Autoboot password unlock not successful"); + } + ut_assert_console_end(); + return 0; +} + +/** + * test_autoboot() - unit test for autoboot + * + * @uts: unit test state + * Return: 0 = success, 1 = failure + */ +static int test_autoboot(struct unit_test_state *uts) +{ + /* make sure that the bootdelay is set to something, + * otherwise the called functions will time out + */ + ut_assertok(env_set("bootdelay", "1")); + bootdelay_process(); + + /* unset all relevant environment variables */ + env_set("bootstopusesha256", NULL); + env_set("bootstopkeycrypt", NULL); + env_set("bootstopkeysha256", NULL); + + if (IS_ENABLED(CONFIG_CRYPT_PW_SHA256)) { + /* test the default password from CONFIG_AUTOBOOT_STOP_STR_CRYPT */ + ut_assertok(check_for_input(uts, "a\n", true)); + + /* verify that the `bootstopusesha256` variable is treated correctly */ + ut_assertok(env_set("bootstopusesha256", "false")); + /* test a password from the `bootstopkeycrypt` environment variable */ + ut_assertok(env_set( + "bootstopkeycrypt", + "$5$rounds=640000$ycgRgpnRq4lmu.eb$aZ6YJWdklvyLML13w7mEHMHJnJOux6aptnp6VlsR5a9")); + + ut_assertok(check_for_input(uts, "test\n", true)); + } + + if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) { + /* test the `bootstopusesha256` and `bootstopkeysha256` features */ + ut_assertok(env_set("bootstopusesha256", "true")); + ut_assertok(env_set( + "bootstopkeysha256", + "edeaaff3f1774ad2888673770c6d64097e391bc362d7d6fb34982ddf0efd18cb")); + + ut_assertok(check_for_input(uts, "abc\n", true)); + + ut_assertok(env_set( + "bootstopkeysha256", + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad")); + + ut_assertok(check_for_input(uts, "abc", true)); + + ut_assertok(check_for_input(uts, "abc\n", true)); + + ut_assertok(check_for_input(uts, "abd", false)); + } + + return CMD_RET_SUCCESS; +} + +COMMON_TEST(test_autoboot, 0);

On Mon, 21 Jun 2021 at 14:25, Steffen Jaeckel jaeckel-floss@eyet-services.de wrote:
This adds tests for the crypt-based and plain SHA256-based password hashing algorithms in the autoboot flow.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de
(no changes since v1)
common/Kconfig.boot | 2 +- common/console.c | 5 +++ configs/sandbox_defconfig | 11 +++++ include/console.h | 17 +++++++ include/test/common.h | 15 +++++++ include/test/suites.h | 1 + test/Makefile | 1 + test/cmd_ut.c | 1 + test/common/Makefile | 3 ++ test/common/cmd_ut_common.c | 22 +++++++++ test/common/test_autoboot.c | 90 +++++++++++++++++++++++++++++++++++++ 11 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 include/test/common.h create mode 100644 test/common/Makefile create mode 100644 test/common/cmd_ut_common.c create mode 100644 test/common/test_autoboot.c
Reviewed-by: Simon Glass sjg@chromium.org
but please make sure patman does not give you checkpatch warnings. I see one, I think.

Hi Simon,
thanks again for the review!
On 6/26/21 8:32 PM, Simon Glass wrote:
On Mon, 21 Jun 2021 at 14:25, Steffen Jaeckel jaeckel-floss@eyet-services.de wrote:
This adds tests for the crypt-based and plain SHA256-based password hashing algorithms in the autoboot flow.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de
(no changes since v1)
common/Kconfig.boot | 2 +- common/console.c | 5 +++ configs/sandbox_defconfig | 11 +++++ include/console.h | 17 +++++++ include/test/common.h | 15 +++++++ include/test/suites.h | 1 + test/Makefile | 1 + test/cmd_ut.c | 1 + test/common/Makefile | 3 ++ test/common/cmd_ut_common.c | 22 +++++++++ test/common/test_autoboot.c | 90 +++++++++++++++++++++++++++++++++++++ 11 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 include/test/common.h create mode 100644 test/common/Makefile create mode 100644 test/common/cmd_ut_common.c create mode 100644 test/common/test_autoboot.c
Reviewed-by: Simon Glass sjg@chromium.org
but please make sure patman does not give you checkpatch warnings. I see one, I think.
I see two potential candidates
test/common/test_autoboot.c:27: warning: braces {} are not necessary for single statement blocks test/common/test_autoboot.c:62: warning: line length of 106 exceeds 100 columns
Which one are you referring to?
I've fixed the braces one.
The line length one if changed leads to another warning "warning: quoted string split across lines".
Cheers Steffen

Hi Steffen,
On Wed, 7 Jul 2021 at 04:39, Steffen Jaeckel jaeckel-floss@eyet-services.de wrote:
Hi Simon,
thanks again for the review!
On 6/26/21 8:32 PM, Simon Glass wrote:
On Mon, 21 Jun 2021 at 14:25, Steffen Jaeckel jaeckel-floss@eyet-services.de wrote:
This adds tests for the crypt-based and plain SHA256-based password hashing algorithms in the autoboot flow.
Signed-off-by: Steffen Jaeckel jaeckel-floss@eyet-services.de
(no changes since v1)
common/Kconfig.boot | 2 +- common/console.c | 5 +++ configs/sandbox_defconfig | 11 +++++ include/console.h | 17 +++++++ include/test/common.h | 15 +++++++ include/test/suites.h | 1 + test/Makefile | 1 + test/cmd_ut.c | 1 + test/common/Makefile | 3 ++ test/common/cmd_ut_common.c | 22 +++++++++ test/common/test_autoboot.c | 90 +++++++++++++++++++++++++++++++++++++ 11 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 include/test/common.h create mode 100644 test/common/Makefile create mode 100644 test/common/cmd_ut_common.c create mode 100644 test/common/test_autoboot.c
Reviewed-by: Simon Glass sjg@chromium.org
but please make sure patman does not give you checkpatch warnings. I see one, I think.
I see two potential candidates
test/common/test_autoboot.c:27: warning: braces {} are not necessary for single statement blocks test/common/test_autoboot.c:62: warning: line length of 106 exceeds 100 columns
Which one are you referring to?
The first one, I think.
I've fixed the braces one.
The line length one if changed leads to another warning "warning: quoted string split across lines".
Yes we prefer to have long lines than split strings.
Regards, Simon
participants (2)
-
Simon Glass
-
Steffen Jaeckel