[PATCH 2/5] common: integrate crypt-based passwords
Simon Glass
sjg at chromium.org
Wed Apr 21 09:14:44 CEST 2021
Hi Steffen,
On Tue, 13 Apr 2021 at 10:16, Steffen Jaeckel
<jaeckel-floss at eyet-services.de> wrote:
>
> Hook into the autoboot flow as an alternative to the existing
> mechanisms.
>
> Signed-off-by: Steffen Jaeckel <jaeckel-floss at 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
More information about the U-Boot
mailing list