* [PATCH 0/5] common: Introduce crypt-style password support
@ 2021-04-12 22:15 Steffen Jaeckel
2021-04-12 22:15 ` [PATCH 1/5] lib: add crypt subsystem Steffen Jaeckel
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Steffen Jaeckel @ 2021-04-12 22:15 UTC (permalink / raw)
To: u-boot
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.
Cheers,
Steffen
Steffen Jaeckel (5):
lib: add crypt subsystem
common: integrate crypt-based passwords
common: Rename macro appropriately
cmd: allow disabling of timeout for password entry
configs: add new values to bcm963158 defconfig
cmd/Kconfig | 8 +
common/Kconfig.boot | 23 ++-
common/autoboot.c | 87 +++++++--
configs/bcm963158_ram_defconfig | 8 +
include/crypt.h | 13 ++
lib/Kconfig | 1 +
lib/Makefile | 1 +
lib/crypt/Kconfig | 29 +++
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 +++++++
15 files changed, 936 insertions(+), 20 deletions(-)
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
--
2.30.1
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/5] lib: add crypt subsystem 2021-04-12 22:15 [PATCH 0/5] common: Introduce crypt-style password support Steffen Jaeckel @ 2021-04-12 22:15 ` Steffen Jaeckel 2021-04-21 7:14 ` Simon Glass 2021-04-12 22:15 ` [PATCH 2/5] common: integrate crypt-based passwords Steffen Jaeckel ` (3 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-12 22:15 UTC (permalink / raw) To: u-boot 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> --- include/crypt.h | 13 ++ lib/Kconfig | 1 + lib/Makefile | 1 + lib/crypt/Kconfig | 29 ++++ 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 +++++++++ 11 files changed, 830 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 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 80ff2443cb..99a4e1a5a7 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 c42d4e1233..ab78f32eb1 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -64,6 +64,7 @@ obj-$(CONFIG_$(SPL_)RSA) += rsa/ 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..6f828cefd6 --- /dev/null +++ b/lib/crypt/Kconfig @@ -0,0 +1,29 @@ +config CRYPT_PW + bool "Add crypt support for password-based unlock" + 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. + To make it fully functional, one has also to enable + CONFIG_AUTOBOOT_KEYED and CONFIG_AUTOBOOT_ENCRYPTION + +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 at 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@redhat.com> in 2007 [1]. + * Modified by Zack Weinberg <zackw@panix.com> in 2017, 2018. + * Composed by Bj?rn Esser <besser82@fedoraproject.org> in 2018. + * Modified by Bj?rn Esser <besser82@fedoraproject.org> in 2020. + * Modified by Steffen Jaeckel <jaeckel-floss@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@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@redhat.com> in 2007 [1]. + * Modified by Zack Weinberg <zackw@panix.com> in 2017, 2018. + * Composed by Bj?rn Esser <besser82@fedoraproject.org> in 2018. + * Modified by Bj?rn Esser <besser82@fedoraproject.org> in 2020. + * Modified by Steffen Jaeckel <jaeckel-floss@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 + @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)); +} -- 2.30.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 1/5] lib: add crypt subsystem 2021-04-12 22:15 ` [PATCH 1/5] lib: add crypt subsystem Steffen Jaeckel @ 2021-04-21 7:14 ` Simon Glass 2021-04-21 8:21 ` Steffen Jaeckel 0 siblings, 1 reply; 15+ messages in thread From: Simon Glass @ 2021-04-21 7:14 UTC (permalink / raw) To: u-boot On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel <jaeckel-floss@eyet-services.de> wrote: > > 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> > --- > > include/crypt.h | 13 ++ > lib/Kconfig | 1 + > lib/Makefile | 1 + > lib/crypt/Kconfig | 29 ++++ > 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 +++++++++ > 11 files changed, 830 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 This seems to use errno - is that necessary? Also are there any simple unit tests we could usefully bring over? Regards, Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] lib: add crypt subsystem 2021-04-21 7:14 ` Simon Glass @ 2021-04-21 8:21 ` Steffen Jaeckel 2021-04-22 23:55 ` Simon Glass 0 siblings, 1 reply; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-21 8:21 UTC (permalink / raw) To: u-boot Hi Simon, thanks for taking the time to review. On 4/21/21 9:14 AM, Simon Glass wrote: > On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel > <jaeckel-floss@eyet-services.de> wrote: >> >> 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> >> --- >> >> include/crypt.h | 13 ++ >> lib/Kconfig | 1 + >> lib/Makefile | 1 + >> lib/crypt/Kconfig | 29 ++++ >> 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 +++++++++ >> 11 files changed, 830 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 > > This seems to use errno - is that necessary? Also are there any simple > unit tests we could usefully bring over? Regarding errno - that's the way how libxcrypt works internally, I'm not sure whether we should really touch this ... the default crypt_alg_rn() function has a void return type, so either we have to keep using errno or we have to change the return type... Regarding unit tests - good idea, I'll have a look. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/5] lib: add crypt subsystem 2021-04-21 8:21 ` Steffen Jaeckel @ 2021-04-22 23:55 ` Simon Glass 0 siblings, 0 replies; 15+ messages in thread From: Simon Glass @ 2021-04-22 23:55 UTC (permalink / raw) To: u-boot Hi Steffen, On Wed, 21 Apr 2021 at 20:21, Steffen Jaeckel <jaeckel-floss@eyet-services.de> wrote: > > Hi Simon, > > thanks for taking the time to review. > > On 4/21/21 9:14 AM, Simon Glass wrote: > > On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel > > <jaeckel-floss@eyet-services.de> wrote: > >> > >> 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> > >> --- > >> > >> include/crypt.h | 13 ++ > >> lib/Kconfig | 1 + > >> lib/Makefile | 1 + > >> lib/crypt/Kconfig | 29 ++++ > >> 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 +++++++++ > >> 11 files changed, 830 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 > > > > This seems to use errno - is that necessary? Also are there any simple > > unit tests we could usefully bring over? > > Regarding errno - that's the way how libxcrypt works internally, I'm not > sure whether we should really touch this ... the default crypt_alg_rn() > function has a void return type, so either we have to keep using errno > or we have to change the return type... Well you could add a wrapper function for U-Boot which returns errno, then make errno a static int in the library. > > Regarding unit tests - good idea, I'll have a look. Regards, Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] common: integrate crypt-based passwords 2021-04-12 22:15 [PATCH 0/5] common: Introduce crypt-style password support Steffen Jaeckel 2021-04-12 22:15 ` [PATCH 1/5] lib: add crypt subsystem Steffen Jaeckel @ 2021-04-12 22:15 ` Steffen Jaeckel 2021-04-21 7:14 ` Simon Glass 2021-04-12 22:15 ` [PATCH 3/5] common: Rename macro appropriately Steffen Jaeckel ` (2 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-12 22:15 UTC (permalink / raw) To: u-boot Hook into the autoboot flow as an alternative to the existing mechanisms. Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de> --- common/Kconfig.boot | 23 +++++++++++++--- common/autoboot.c | 67 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 77 insertions(+), 13 deletions(-) diff --git a/common/Kconfig.boot b/common/Kconfig.boot index 9c335f4f8c..59fec48c5d 100644 --- a/common/Kconfig.boot +++ b/common/Kconfig.boot @@ -802,10 +802,16 @@ 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 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. @@ -843,6 +849,15 @@ config AUTOBOOT_KEYED_CTRLC Setting this variable provides an escape sequence from the limited "password" strings. +config AUTOBOOT_STOP_STR_CRYPT + string "Stop autobooting via crypt-hashed password" + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION + 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 support crypt options and saved in the environment. + config AUTOBOOT_STOP_STR_SHA256 string "Stop autobooting via SHA256 encrypted password" depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION diff --git a/common/autoboot.c b/common/autoboot.c index 0bb08e7a4c..732a01d0e5 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,62 @@ 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_ENCRYPTION) +#if defined(CONFIG_CRYPT_PW) && defined(CONFIG_AUTOBOOT_STOP_STR_CRYPT) +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_CRYPT +#define HAS_STOP_STR_CRYPT 1 +#elif defined(CONFIG_AUTOBOOT_STOP_STR_SHA256) +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_SHA256 +#endif +#endif +#if !defined(AUTOBOOT_STOP_STR_ENC) +#define AUTOBOOT_STOP_STR_ENC "" #endif - #ifdef CONFIG_USE_AUTOBOOT_MENUKEY #define AUTOBOOT_MENUKEY CONFIG_USE_AUTOBOOT_MENUKEY #else #define AUTOBOOT_MENUKEY 0 #endif +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; + + if (IS_ENABLED(HAS_STOP_STR_CRYPT) && !crypt_env_str) + crypt_env_str = AUTOBOOT_STOP_STR_ENC; + + 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 >= MAX_DELAY_STOP_STR) + return 0; + + presskey[presskey_len] = getchar(); + + if ((presskey[presskey_len] == '\r') || + (presskey[presskey_len] == '\n')) { + presskey[presskey_len] = '\0'; + crypt_compare(crypt_env_str, presskey, &abort); + /* 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 +134,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 = AUTOBOOT_STOP_STR_ENC; presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR); c = strstr(sha_env_str, ":"); @@ -245,10 +290,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"); -- 2.30.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/5] common: integrate crypt-based passwords 2021-04-12 22:15 ` [PATCH 2/5] common: integrate crypt-based passwords Steffen Jaeckel @ 2021-04-21 7:14 ` Simon Glass 2021-04-21 8:55 ` Steffen Jaeckel 0 siblings, 1 reply; 15+ messages in thread From: Simon Glass @ 2021-04-21 7:14 UTC (permalink / raw) To: u-boot Hi Steffen, On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel <jaeckel-floss@eyet-services.de> wrote: > > Hook into the autoboot flow as an alternative to the existing > mechanisms. > > Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de> > --- > > common/Kconfig.boot | 23 +++++++++++++--- > common/autoboot.c | 67 +++++++++++++++++++++++++++++++++++++++------ > 2 files changed, 77 insertions(+), 13 deletions(-) > > diff --git a/common/Kconfig.boot b/common/Kconfig.boot > index 9c335f4f8c..59fec48c5d 100644 > --- a/common/Kconfig.boot > +++ b/common/Kconfig.boot > @@ -802,10 +802,16 @@ 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 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. > > @@ -843,6 +849,15 @@ config AUTOBOOT_KEYED_CTRLC > Setting this variable provides an escape sequence from the > limited "password" strings. > > +config AUTOBOOT_STOP_STR_CRYPT > + string "Stop autobooting via crypt-hashed password" > + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION > + 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 support crypt options and saved in the environment. > + > config AUTOBOOT_STOP_STR_SHA256 > string "Stop autobooting via SHA256 encrypted password" > depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION > diff --git a/common/autoboot.c b/common/autoboot.c > index 0bb08e7a4c..732a01d0e5 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,62 @@ 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_ENCRYPTION) > +#if defined(CONFIG_CRYPT_PW) && defined(CONFIG_AUTOBOOT_STOP_STR_CRYPT) > +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_CRYPT > +#define HAS_STOP_STR_CRYPT 1 > +#elif defined(CONFIG_AUTOBOOT_STOP_STR_SHA256) > +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_SHA256 > +#endif > +#endif > +#if !defined(AUTOBOOT_STOP_STR_ENC) > +#define AUTOBOOT_STOP_STR_ENC "" > #endif I wonder if we actually need all this now that things are in Kconfig? Can we use IS_ENABLED() in the code instead? > - > #ifdef CONFIG_USE_AUTOBOOT_MENUKEY > #define AUTOBOOT_MENUKEY CONFIG_USE_AUTOBOOT_MENUKEY > #else > #define AUTOBOOT_MENUKEY 0 > #endif > > +static int passwd_abort_crypt(uint64_t etime) Please add function comment Also unsigned long if you can...if you need 64-bit on 32-bit machines it is u64...but why? That is a very large number of milliseconds! > +{ > + const char *crypt_env_str = env_get("bootstopkeycrypt"); > + char presskey[MAX_DELAY_STOP_STR]; > + u_int presskey_len = 0; uint Can you drop the presskey_ prefix on these. There is no other kind of key, so just 'key' and 'len' (or num_keys) is good enough. > + int abort = 0; > + > + if (IS_ENABLED(HAS_STOP_STR_CRYPT) && !crypt_env_str) We normally use IS_ENABLED with a CONFIG_.... > + crypt_env_str = AUTOBOOT_STOP_STR_ENC; > + > + if (!crypt_env_str) > + return 0; > + > + /* > + * We expect the stop-string to be newline terminated. > + */ /* ... */ is the single-line comment format > + do { > + if (tstc()) { > + /* Check for input string overflow */ > + if (presskey_len >= MAX_DELAY_STOP_STR) > + return 0; > + > + presskey[presskey_len] = getchar(); > + > + if ((presskey[presskey_len] == '\r') || > + (presskey[presskey_len] == '\n')) { > + presskey[presskey_len] = '\0'; > + crypt_compare(crypt_env_str, presskey, &abort); > + /* 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 +134,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 = AUTOBOOT_STOP_STR_ENC; > > presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR); > c = strstr(sha_env_str, ":"); > @@ -245,10 +290,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"); > > -- > 2.30.1 > Regards, SImon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] common: integrate crypt-based passwords 2021-04-21 7:14 ` Simon Glass @ 2021-04-21 8:55 ` Steffen Jaeckel 2021-04-29 16:10 ` Simon Glass 0 siblings, 1 reply; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-21 8:55 UTC (permalink / raw) To: u-boot On 4/21/21 9:14 AM, Simon Glass wrote: > Hi Steffen, > > On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel > <jaeckel-floss@eyet-services.de> wrote: >> >> Hook into the autoboot flow as an alternative to the existing >> mechanisms. >> >> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de> >> --- >> >> common/Kconfig.boot | 23 +++++++++++++--- >> common/autoboot.c | 67 +++++++++++++++++++++++++++++++++++++++------ >> 2 files changed, 77 insertions(+), 13 deletions(-) >> >> diff --git a/common/Kconfig.boot b/common/Kconfig.boot >> index 9c335f4f8c..59fec48c5d 100644 >> --- a/common/Kconfig.boot >> +++ b/common/Kconfig.boot >> @@ -802,10 +802,16 @@ 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 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. >> >> @@ -843,6 +849,15 @@ config AUTOBOOT_KEYED_CTRLC >> Setting this variable provides an escape sequence from the >> limited "password" strings. >> >> +config AUTOBOOT_STOP_STR_CRYPT >> + string "Stop autobooting via crypt-hashed password" >> + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION >> + 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 support crypt options and saved in the environment. >> + >> config AUTOBOOT_STOP_STR_SHA256 >> string "Stop autobooting via SHA256 encrypted password" >> depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION >> diff --git a/common/autoboot.c b/common/autoboot.c >> index 0bb08e7a4c..732a01d0e5 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,62 @@ 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_ENCRYPTION) >> +#if defined(CONFIG_CRYPT_PW) && defined(CONFIG_AUTOBOOT_STOP_STR_CRYPT) >> +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_CRYPT >> +#define HAS_STOP_STR_CRYPT 1 >> +#elif defined(CONFIG_AUTOBOOT_STOP_STR_SHA256) >> +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_SHA256 >> +#endif >> +#endif >> +#if !defined(AUTOBOOT_STOP_STR_ENC) >> +#define AUTOBOOT_STOP_STR_ENC "" >> #endif > > I wonder if we actually need all this now that things are in Kconfig? > Can we use IS_ENABLED() in the code instead? The problem here is that CONFIG_AUTOBOOT_STOP_STR_{CRYPT,SHA256} are strings which can't be checked with IS_ENABLED(). >> - >> #ifdef CONFIG_USE_AUTOBOOT_MENUKEY >> #define AUTOBOOT_MENUKEY CONFIG_USE_AUTOBOOT_MENUKEY >> #else >> #define AUTOBOOT_MENUKEY 0 >> #endif >> >> +static int passwd_abort_crypt(uint64_t etime) > > Please add function comment OK > Also unsigned long if you can...if you need 64-bit on 32-bit machines > it is u64...but why? That is a very large number of milliseconds! I've simply c&p'ed the prototype of the existing functions :) static int passwd_abort_sha256(uint64_t etime) static int passwd_abort_key(uint64_t etime) >> +{ >> + const char *crypt_env_str = env_get("bootstopkeycrypt"); >> + char presskey[MAX_DELAY_STOP_STR]; >> + u_int presskey_len = 0; > > uint > > Can you drop the presskey_ prefix on these. There is no other kind of > key, so just 'key' and 'len' (or num_keys) is good enough. same as above, this is a c&p from passwd_abort_sha256() I had started by including my functionality into passwd_abort_sha256(), when I realized that it won't work like that I decided to c&p the entire implementation... should I still change both? those variable names and the uint64_t in the arguments? > >> + int abort = 0; >> + >> + if (IS_ENABLED(HAS_STOP_STR_CRYPT) && !crypt_env_str) > > We normally use IS_ENABLED with a CONFIG_.... I don't see a different way than either this one or introducing a separate enable switch in Kconfig (which would in turn then either require different behavior for enabling CONFIG_AUTOBOOT_STOP_STR_SHA256 vs CONFIG_AUTOBOOT_STOP_STR_CRYPT or we'd have to break the existing flow of CONFIG_AUTOBOOT_STOP_STR_SHA256). > >> + crypt_env_str = AUTOBOOT_STOP_STR_ENC; >> + >> + if (!crypt_env_str) >> + return 0; >> + >> + /* >> + * We expect the stop-string to be newline terminated. >> + */ > > /* ... */ is the single-line comment format OK > >> + do { >> + if (tstc()) { >> + /* Check for input string overflow */ >> + if (presskey_len >= MAX_DELAY_STOP_STR) >> + return 0; >> + >> + presskey[presskey_len] = getchar(); >> + >> + if ((presskey[presskey_len] == '\r') || >> + (presskey[presskey_len] == '\n')) { >> + presskey[presskey_len] = '\0'; >> + crypt_compare(crypt_env_str, presskey, &abort); >> + /* 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 +134,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 = AUTOBOOT_STOP_STR_ENC; >> >> presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR); >> c = strstr(sha_env_str, ":"); >> @@ -245,10 +290,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"); >> >> -- >> 2.30.1 >> > > Regards, > SImon > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/5] common: integrate crypt-based passwords 2021-04-21 8:55 ` Steffen Jaeckel @ 2021-04-29 16:10 ` Simon Glass 0 siblings, 0 replies; 15+ messages in thread From: Simon Glass @ 2021-04-29 16:10 UTC (permalink / raw) To: u-boot Hi Steffen, On Wed, 21 Apr 2021 at 01:55, Steffen Jaeckel <jaeckel-floss@eyet-services.de> wrote: > > On 4/21/21 9:14 AM, Simon Glass wrote: > > Hi Steffen, > > > > On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel > > <jaeckel-floss@eyet-services.de> wrote: > >> > >> Hook into the autoboot flow as an alternative to the existing > >> mechanisms. > >> > >> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de> > >> --- > >> > >> common/Kconfig.boot | 23 +++++++++++++--- > >> common/autoboot.c | 67 +++++++++++++++++++++++++++++++++++++++------ > >> 2 files changed, 77 insertions(+), 13 deletions(-) > >> > >> diff --git a/common/Kconfig.boot b/common/Kconfig.boot > >> index 9c335f4f8c..59fec48c5d 100644 > >> --- a/common/Kconfig.boot > >> +++ b/common/Kconfig.boot > >> @@ -802,10 +802,16 @@ 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 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. > >> > >> @@ -843,6 +849,15 @@ config AUTOBOOT_KEYED_CTRLC > >> Setting this variable provides an escape sequence from the > >> limited "password" strings. > >> > >> +config AUTOBOOT_STOP_STR_CRYPT > >> + string "Stop autobooting via crypt-hashed password" > >> + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION > >> + 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 support crypt options and saved in the environment. > >> + > >> config AUTOBOOT_STOP_STR_SHA256 > >> string "Stop autobooting via SHA256 encrypted password" > >> depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION > >> diff --git a/common/autoboot.c b/common/autoboot.c > >> index 0bb08e7a4c..732a01d0e5 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,62 @@ 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_ENCRYPTION) > >> +#if defined(CONFIG_CRYPT_PW) && defined(CONFIG_AUTOBOOT_STOP_STR_CRYPT) > >> +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_CRYPT > >> +#define HAS_STOP_STR_CRYPT 1 > >> +#elif defined(CONFIG_AUTOBOOT_STOP_STR_SHA256) > >> +#define AUTOBOOT_STOP_STR_ENC CONFIG_AUTOBOOT_STOP_STR_SHA256 > >> +#endif > >> +#endif > >> +#if !defined(AUTOBOOT_STOP_STR_ENC) > >> +#define AUTOBOOT_STOP_STR_ENC "" > >> #endif > > > > I wonder if we actually need all this now that things are in Kconfig? > > Can we use IS_ENABLED() in the code instead? > > The problem here is that CONFIG_AUTOBOOT_STOP_STR_{CRYPT,SHA256} are > strings which can't be checked with IS_ENABLED(). OK...then they should be split into a CONFIG that enables the feature and one that sets the value. > > > >> - > >> #ifdef CONFIG_USE_AUTOBOOT_MENUKEY > >> #define AUTOBOOT_MENUKEY CONFIG_USE_AUTOBOOT_MENUKEY > >> #else > >> #define AUTOBOOT_MENUKEY 0 > >> #endif > >> > >> +static int passwd_abort_crypt(uint64_t etime) > > > > Please add function comment > > OK > > > > Also unsigned long if you can...if you need 64-bit on 32-bit machines > > it is u64...but why? That is a very large number of milliseconds! > > I've simply c&p'ed the prototype of the existing functions :) > static int passwd_abort_sha256(uint64_t etime) > static int passwd_abort_key(uint64_t etime) OK but it still seems wrong to me so I don't think we should copy it. Perhaps clean up the existing code? > > > >> +{ > >> + const char *crypt_env_str = env_get("bootstopkeycrypt"); > >> + char presskey[MAX_DELAY_STOP_STR]; > >> + u_int presskey_len = 0; > > > > uint > > > > Can you drop the presskey_ prefix on these. There is no other kind of > > key, so just 'key' and 'len' (or num_keys) is good enough. > > same as above, this is a c&p from passwd_abort_sha256() > > I had started by including my functionality into passwd_abort_sha256(), > when I realized that it won't work like that I decided to c&p the entire > implementation... > > should I still change both? those variable names and the uint64_t in the > arguments? Yes you can change the existing code in a separate patch. > > > > > >> + int abort = 0; > >> + > >> + if (IS_ENABLED(HAS_STOP_STR_CRYPT) && !crypt_env_str) > > > > We normally use IS_ENABLED with a CONFIG_.... > > I don't see a different way than either this one or introducing a > separate enable switch in Kconfig (which would in turn then either > require different behavior for enabling CONFIG_AUTOBOOT_STOP_STR_SHA256 > vs CONFIG_AUTOBOOT_STOP_STR_CRYPT or we'd have to break the existing > flow of CONFIG_AUTOBOOT_STOP_STR_SHA256). My point was just that you should use HAS_STOP_STR_CRYPT , not IS_ENABLED(HAS_STOP_STR_CRYPT) [...] Regards, Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/5] common: Rename macro appropriately 2021-04-12 22:15 [PATCH 0/5] common: Introduce crypt-style password support Steffen Jaeckel 2021-04-12 22:15 ` [PATCH 1/5] lib: add crypt subsystem Steffen Jaeckel 2021-04-12 22:15 ` [PATCH 2/5] common: integrate crypt-based passwords Steffen Jaeckel @ 2021-04-12 22:15 ` Steffen Jaeckel 2021-04-14 19:38 ` Simon Glass 2021-04-12 22:15 ` [PATCH 4/5] cmd: allow disabling of timeout for password entry Steffen Jaeckel 2021-04-12 22:15 ` [PATCH 5/5] configs: add new values to bcm963158 defconfig Steffen Jaeckel 4 siblings, 1 reply; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-12 22:15 UTC (permalink / raw) To: u-boot 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> --- common/autoboot.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/common/autoboot.c b/common/autoboot.c index 732a01d0e5..5bda3da7b1 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 @@ -59,7 +59,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; @@ -75,7 +75,7 @@ static int passwd_abort_crypt(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) return 0; presskey[presskey_len] = getchar(); @@ -136,9 +136,9 @@ static int passwd_abort_sha256(uint64_t etime) if (sha_env_str == NULL) sha_env_str = AUTOBOOT_STOP_STR_ENC; - 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; @@ -165,7 +165,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; @@ -209,7 +209,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; @@ -226,8 +226,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; -- 2.30.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/5] common: Rename macro appropriately 2021-04-12 22:15 ` [PATCH 3/5] common: Rename macro appropriately Steffen Jaeckel @ 2021-04-14 19:38 ` Simon Glass 0 siblings, 0 replies; 15+ messages in thread From: Simon Glass @ 2021-04-14 19:38 UTC (permalink / raw) To: u-boot On Mon, 12 Apr 2021 at 23:16, Steffen Jaeckel <jaeckel-floss@eyet-services.de> wrote: > > 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> > --- > > common/autoboot.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] cmd: allow disabling of timeout for password entry 2021-04-12 22:15 [PATCH 0/5] common: Introduce crypt-style password support Steffen Jaeckel ` (2 preceding siblings ...) 2021-04-12 22:15 ` [PATCH 3/5] common: Rename macro appropriately Steffen Jaeckel @ 2021-04-12 22:15 ` Steffen Jaeckel 2021-04-21 7:14 ` Simon Glass 2021-04-12 22:15 ` [PATCH 5/5] configs: add new values to bcm963158 defconfig Steffen Jaeckel 4 siblings, 1 reply; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-12 22:15 UTC (permalink / raw) To: u-boot 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> --- cmd/Kconfig | 8 ++++++++ common/autoboot.c | 8 +++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cmd/Kconfig b/cmd/Kconfig index c735e81b37..03c07d0f32 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -177,6 +177,14 @@ config CMD_SBI help Display information about the SBI implementation. +config AUTOBOOT_NEVER_TIMEOUT + bool "Make the password entry never time-out" + depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION + help + This option removes the timeout from the password entry + when the user first presses the <Enter> key before entering + any other character. + endmenu menu "Boot commands" diff --git a/common/autoboot.c b/common/autoboot.c index 5bda3da7b1..467333db9d 100644 --- a/common/autoboot.c +++ b/common/autoboot.c @@ -62,6 +62,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; if (IS_ENABLED(HAS_STOP_STR_CRYPT) && !crypt_env_str) crypt_env_str = AUTOBOOT_STOP_STR_ENC; @@ -82,6 +83,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'; crypt_compare(crypt_env_str, presskey, &abort); /* you had one chance */ @@ -90,7 +96,7 @@ static int passwd_abort_crypt(uint64_t etime) presskey_len++; } } - } while (get_ticks() <= etime); + } while (never_timeout || get_ticks() <= etime); return abort; } -- 2.30.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 4/5] cmd: allow disabling of timeout for password entry 2021-04-12 22:15 ` [PATCH 4/5] cmd: allow disabling of timeout for password entry Steffen Jaeckel @ 2021-04-21 7:14 ` Simon Glass 2021-04-21 14:09 ` Steffen Jaeckel 0 siblings, 1 reply; 15+ messages in thread From: Simon Glass @ 2021-04-21 7:14 UTC (permalink / raw) To: u-boot On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel <jaeckel-floss@eyet-services.de> wrote: > > 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> > --- > > cmd/Kconfig | 8 ++++++++ > common/autoboot.c | 8 +++++++- > 2 files changed, 15 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org> I wonder if we could have some unit tests for this? - Simon ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/5] cmd: allow disabling of timeout for password entry 2021-04-21 7:14 ` Simon Glass @ 2021-04-21 14:09 ` Steffen Jaeckel 0 siblings, 0 replies; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-21 14:09 UTC (permalink / raw) To: u-boot On 4/21/21 9:14 AM, Simon Glass wrote: > On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel > <jaeckel-floss@eyet-services.de> wrote: >> >> 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> >> --- >> >> cmd/Kconfig | 8 ++++++++ >> common/autoboot.c | 8 +++++++- >> 2 files changed, 15 insertions(+), 1 deletion(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > I wonder if we could have some unit tests for this? As there are no tests yet for the autoboot part and I don't really feel comfortable adding one, I guess this has to wait ... FYI: I've implemented the unit tests & your change requests but I'm waiting with a re-spin until we've sorted out the open points of Patch 2. Cheers Steffen ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/5] configs: add new values to bcm963158 defconfig 2021-04-12 22:15 [PATCH 0/5] common: Introduce crypt-style password support Steffen Jaeckel ` (3 preceding siblings ...) 2021-04-12 22:15 ` [PATCH 4/5] cmd: allow disabling of timeout for password entry Steffen Jaeckel @ 2021-04-12 22:15 ` Steffen Jaeckel 4 siblings, 0 replies; 15+ messages in thread From: Steffen Jaeckel @ 2021-04-12 22:15 UTC (permalink / raw) To: u-boot In order to have at least one defconfig that enables all those newly added values. Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de> --- configs/bcm963158_ram_defconfig | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/configs/bcm963158_ram_defconfig b/configs/bcm963158_ram_defconfig index 0be1e0981a..7e1e9b639d 100644 --- a/configs/bcm963158_ram_defconfig +++ b/configs/bcm963158_ram_defconfig @@ -54,4 +54,12 @@ CONFIG_BCM63XX_HSSPI=y CONFIG_SYSRESET=y CONFIG_SYSRESET_WATCHDOG=y CONFIG_WDT_BCM6345=y +CONFIG_CRYPT_PW=y +CONFIG_CRYPT_PW_SHA256=y +CONFIG_AUTOBOOT_KEYED=y +CONFIG_AUTOBOOT_PROMPT="Enter password in %d seconds to stop autoboot\n" +CONFIG_AUTOBOOT_ENCRYPTION=y +# default password "password" +CONFIG_AUTOBOOT_STOP_STR_CRYPT="$5$rounds=640000$TM4lL4zXDG7F4aRX$JM7a9wmvodnA0WasjTztj6mxg.KVuk6doQ/eBhdcapB" +CONFIG_AUTOBOOT_NEVER_TIMEOUT=y # CONFIG_GENERATE_SMBIOS_TABLE is not set -- 2.30.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-04-29 16:10 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-12 22:15 [PATCH 0/5] common: Introduce crypt-style password support Steffen Jaeckel 2021-04-12 22:15 ` [PATCH 1/5] lib: add crypt subsystem Steffen Jaeckel 2021-04-21 7:14 ` Simon Glass 2021-04-21 8:21 ` Steffen Jaeckel 2021-04-22 23:55 ` Simon Glass 2021-04-12 22:15 ` [PATCH 2/5] common: integrate crypt-based passwords Steffen Jaeckel 2021-04-21 7:14 ` Simon Glass 2021-04-21 8:55 ` Steffen Jaeckel 2021-04-29 16:10 ` Simon Glass 2021-04-12 22:15 ` [PATCH 3/5] common: Rename macro appropriately Steffen Jaeckel 2021-04-14 19:38 ` Simon Glass 2021-04-12 22:15 ` [PATCH 4/5] cmd: allow disabling of timeout for password entry Steffen Jaeckel 2021-04-21 7:14 ` Simon Glass 2021-04-21 14:09 ` Steffen Jaeckel 2021-04-12 22:15 ` [PATCH 5/5] configs: add new values to bcm963158 defconfig Steffen Jaeckel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.