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

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

[Steffen Jaeckel]

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

[PATCH 1/5] lib: add crypt subsystem

[Steffen Jaeckel]

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

[PATCH 2/5] common: integrate crypt-based passwords

[Steffen Jaeckel]

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

[PATCH 3/5] common: Rename macro appropriately

[Steffen Jaeckel]

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

[PATCH 4/5] cmd: allow disabling of timeout for password entry

[Steffen Jaeckel]

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

[PATCH 5/5] configs: add new values to bcm963158 defconfig

[Steffen Jaeckel]

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

[PATCH 3/5] common: Rename macro appropriately

[Simon Glass]

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>

[PATCH 1/5] lib: add crypt subsystem

[Simon Glass]

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

[PATCH 2/5] common: integrate crypt-based passwords

[Simon Glass]

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

[PATCH 4/5] cmd: allow disabling of timeout for password entry

[Simon Glass]

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

[PATCH 1/5] lib: add crypt subsystem

[Steffen Jaeckel]

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.

[PATCH 2/5] common: integrate crypt-based passwords

[Steffen Jaeckel]

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
> 

[PATCH 4/5] cmd: allow disabling of timeout for password entry

[Steffen Jaeckel]

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

[PATCH 1/5] lib: add crypt subsystem

[Simon Glass]

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

[PATCH 2/5] common: integrate crypt-based passwords

[Simon Glass]

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