All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] common: Introduce crypt-style password support
@ 2021-05-10  6:19 Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 1/7] lib: add crypt subsystem Steffen Jaeckel
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 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.

A sandbox defconfig with password entry has been added. I'm not sure
whether this should be kept or not, it's just there as an example.

Cheers,
Steffen

Changes in v2:
Update Kconfig way of enabling, setting hashes etc.

Changes in v1:
Added unit-tests of crypt_compare()
Wrapped crypt functions to encapsulate errno

Steffen Jaeckel (7):
  lib: add crypt subsystem
  lib: wrap crypt API to hide errno usage
  common: integrate crypt-based passwords
  common: Rename macro appropriately
  cmd: allow disabling of timeout for password entry
  configs: add new values to bcm963158 defconfig
  configs: add new sandbox with crypt-based password

 common/Kconfig.boot                 |  45 +++-
 common/autoboot.c                   | 104 +++++++--
 configs/bcm963158_ram_defconfig     |   8 +
 configs/sandbox_cryptpass_defconfig | 296 +++++++++++++++++++++++
 include/crypt.h                     |  14 ++
 lib/Kconfig                         |   1 +
 lib/Makefile                        |   1 +
 lib/crypt/Kconfig                   |  28 +++
 lib/crypt/Makefile                  |  10 +
 lib/crypt/alg-sha256.h              |  11 +
 lib/crypt/alg-sha512.h              |  11 +
 lib/crypt/crypt-port.h              |  30 +++
 lib/crypt/crypt-sha256.c            | 335 ++++++++++++++++++++++++++
 lib/crypt/crypt-sha512.c            | 350 ++++++++++++++++++++++++++++
 lib/crypt/crypt.c                   |  76 ++++++
 test/Kconfig                        |   9 +
 test/lib/Makefile                   |   1 +
 test/lib/test_crypt.c               |  64 +++++
 18 files changed, 1373 insertions(+), 21 deletions(-)
 create mode 100644 configs/sandbox_cryptpass_defconfig
 create mode 100644 include/crypt.h
 create mode 100644 lib/crypt/Kconfig
 create mode 100644 lib/crypt/Makefile
 create mode 100644 lib/crypt/alg-sha256.h
 create mode 100644 lib/crypt/alg-sha512.h
 create mode 100644 lib/crypt/crypt-port.h
 create mode 100644 lib/crypt/crypt-sha256.c
 create mode 100644 lib/crypt/crypt-sha512.c
 create mode 100644 lib/crypt/crypt.c
 create mode 100644 test/lib/test_crypt.c

-- 
2.31.1

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 1/7] lib: add crypt subsystem
  2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
@ 2021-05-10  6:19 ` Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 2/7] lib: wrap crypt API to hide errno usage Steffen Jaeckel
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
Update Kconfig way of enabling, setting hashes etc.

Changes in v1:
Added unit-tests of crypt_compare()
Wrapped crypt functions to encapsulate errno

 include/crypt.h          |  13 ++
 lib/Kconfig              |   1 +
 lib/Makefile             |   1 +
 lib/crypt/Kconfig        |  28 ++++
 lib/crypt/Makefile       |  10 ++
 lib/crypt/alg-sha256.h   |  17 ++
 lib/crypt/alg-sha512.h   |  17 ++
 lib/crypt/crypt-port.h   |  28 ++++
 lib/crypt/crypt-sha256.c | 313 +++++++++++++++++++++++++++++++++++++
 lib/crypt/crypt-sha512.c | 328 +++++++++++++++++++++++++++++++++++++++
 lib/crypt/crypt.c        |  73 +++++++++
 test/Kconfig             |   9 ++
 test/lib/Makefile        |   1 +
 test/lib/test_crypt.c    |  44 ++++++
 14 files changed, 883 insertions(+)
 create mode 100644 include/crypt.h
 create mode 100644 lib/crypt/Kconfig
 create mode 100644 lib/crypt/Makefile
 create mode 100644 lib/crypt/alg-sha256.h
 create mode 100644 lib/crypt/alg-sha512.h
 create mode 100644 lib/crypt/crypt-port.h
 create mode 100644 lib/crypt/crypt-sha256.c
 create mode 100644 lib/crypt/crypt-sha512.c
 create mode 100644 lib/crypt/crypt.c
 create mode 100644 test/lib/test_crypt.c

diff --git a/include/crypt.h b/include/crypt.h
new file mode 100644
index 0000000000..e0be2832ff
--- /dev/null
+++ b/include/crypt.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (C) 2020 Steffen Jaeckel <jaeckel-floss@eyet-services.de> */
+
+/**
+ * Compare should with the processed passphrase.
+ *
+ * @should      The crypt-style string to compare against
+ * @passphrase  The plaintext passphrase
+ * @equal       Pointer to an int where the result is stored
+ *                 '0' = unequal
+ *                 '1' = equal
+ */
+void crypt_compare(const char *should, const char *passphrase, int *equal);
diff --git a/lib/Kconfig b/lib/Kconfig
index 6d2d41de30..c7c0b87ec7 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 6825671955..f0d91986b1 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_FIT_SIGNATURE) += hash-checksum.o
 obj-$(CONFIG_SHA1) += sha1.o
 obj-$(CONFIG_SHA256) += sha256.o
 obj-$(CONFIG_SHA512_ALGO) += sha512.o
+obj-$(CONFIG_CRYPT_PW) += crypt/
 
 obj-$(CONFIG_$(SPL_)ZLIB) += zlib/
 obj-$(CONFIG_$(SPL_)ZSTD) += zstd/
diff --git a/lib/crypt/Kconfig b/lib/crypt/Kconfig
new file mode 100644
index 0000000000..5495ae8d4c
--- /dev/null
+++ b/lib/crypt/Kconfig
@@ -0,0 +1,28 @@
+menuconfig CRYPT_PW
+	bool "Add crypt support for password-based unlock"
+	depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION
+	help
+	  Enable support for crypt-style hashed passphrases.
+	  This will then be used as the mechanism of choice to
+	  verify whether the entered password to unlock the
+	  console is correct or not.
+
+if CRYPT_PW
+
+config CRYPT_PW_SHA256
+	bool "Provide sha256crypt"
+	select SHA256
+	select SHA256_ALGO
+	help
+	  Enables support for the sha256crypt password-hashing algorithm.
+	  The prefix is "$5$".
+
+config CRYPT_PW_SHA512
+	bool "Provide sha512crypt"
+	select SHA512
+	select SHA512_ALGO
+	help
+	  Enables support for the sha512crypt password-hashing algorithm.
+	  The prefix is "$6$".
+
+endif
diff --git a/lib/crypt/Makefile b/lib/crypt/Makefile
new file mode 100644
index 0000000000..290231064c
--- /dev/null
+++ b/lib/crypt/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) 2013, Google Inc.
+#
+# (C) Copyright 2000-2007
+# Wolfgang Denk, DENX Software Engineering, wd 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));
+}
diff --git a/test/Kconfig b/test/Kconfig
index ab3ac54a1b..52eb1a7b53 100644
--- a/test/Kconfig
+++ b/test/Kconfig
@@ -38,6 +38,15 @@ config UT_LIB_ASN1
 	  Enables a test which exercises asn1 compiler and decoder function
 	  via various parsers.
 
+config UT_LIB_CRYPT
+	bool "Unit test for crypt-style password hashing"
+	default y
+	select CRYPT_PW
+	select CRYPT_PW_SHA256
+	select CRYPT_PW_SHA512
+	help
+	  Enables a test for the crypt-style password hash functions.
+
 config UT_LIB_RSA
 	bool "Unit test for rsa_verify() function"
 	depends on RSA
diff --git a/test/lib/Makefile b/test/lib/Makefile
index aa2e66bc7f..6fd0514251 100644
--- a/test/lib/Makefile
+++ b/test/lib/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_UT_LIB_ASN1) += asn1.o
 obj-$(CONFIG_UT_LIB_RSA) += rsa.o
 obj-$(CONFIG_AES) += test_aes.o
 obj-$(CONFIG_GETOPT) += getopt.o
+obj-$(CONFIG_UT_LIB_CRYPT) += test_crypt.o
diff --git a/test/lib/test_crypt.c b/test/lib/test_crypt.c
new file mode 100644
index 0000000000..277e4efed1
--- /dev/null
+++ b/test/lib/test_crypt.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Steffen Jaeckel
+ *
+ * Unit test for crypt-style password hashing
+ */
+
+#include <common.h>
+#include <test/lib.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+#include <crypt.h>
+
+/**
+ * lib_crypt() - unit test for crypt-style password hashing
+ *
+ * @uts:	unit test state
+ * Return:	0 = success, 1 = failure
+ */
+static int lib_crypt(struct unit_test_state *uts)
+{
+	int equals = 0;
+
+	if (IS_ENABLED(CONFIG_CRYPT_PW_SHA256)) {
+		crypt_compare(
+			"$5$rounds=640000$TM4lL4zXDG7F4aRX$JM7a9wmvodnA0WasjTztj6mxg.KVuk6doQ/eBhdcapB",
+			"password", &equals);
+		ut_assertf(equals == 1,
+			   "crypt-sha256 password hash didn't match\n");
+	}
+	equals = 0;
+	if (IS_ENABLED(CONFIG_CRYPT_PW_SHA512)) {
+		crypt_compare(
+			"$6$rounds=640000$fCTP1F0N5JLq2eND$z5EzK5KZJA9JnOaj5d1Gg/2v6VqFOQJ3bVekWuCPauabutBt/8qzV1exJnytUyhbq3H0bSBXtodwNbtGEi/Tm/",
+			"password", &equals);
+		ut_assertf(equals == 1,
+			   "crypt-sha512 password hash didn't match\n");
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+LIB_TEST(lib_crypt, 0);
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 2/7] lib: wrap crypt API to hide errno usage
  2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 1/7] lib: add crypt subsystem Steffen Jaeckel
@ 2021-05-10  6:19 ` Steffen Jaeckel
  2021-05-10 16:27   ` Simon Glass
  2021-05-10  6:19 ` [PATCH v2 3/7] common: integrate crypt-based passwords Steffen Jaeckel
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 UTC (permalink / raw)
  To: u-boot

In order to prevent using the global errno, replace it with a static
version and create a wrapper function which returns the error value.

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
---

(no changes since v1)

 include/crypt.h          |  3 ++-
 lib/crypt/alg-sha256.h   |  6 ------
 lib/crypt/alg-sha512.h   |  6 ------
 lib/crypt/crypt-port.h   | 18 ++++++++++--------
 lib/crypt/crypt-sha256.c | 26 ++++++++++++++++++++++++--
 lib/crypt/crypt-sha512.c | 26 ++++++++++++++++++++++++--
 lib/crypt/crypt.c        | 25 ++++++++++++++-----------
 test/lib/test_crypt.c    | 24 ++++++++++++++++++++++--
 8 files changed, 96 insertions(+), 38 deletions(-)

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 1/7] lib: add crypt subsystem Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 2/7] lib: wrap crypt API to hide errno usage Steffen Jaeckel
@ 2021-05-10  6:19 ` Steffen Jaeckel
  2021-05-10 16:27   ` Simon Glass
  2021-05-10  6:19 ` [PATCH v2 4/7] common: Rename macro appropriately Steffen Jaeckel
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 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>
---

(no changes since v1)

 common/Kconfig.boot | 37 ++++++++++++++++++---
 common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 103 insertions(+), 14 deletions(-)

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 4/7] common: Rename macro appropriately
  2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (2 preceding siblings ...)
  2021-05-10  6:19 ` [PATCH v2 3/7] common: integrate crypt-based passwords Steffen Jaeckel
@ 2021-05-10  6:19 ` Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 5/7] cmd: allow disabling of timeout for password entry Steffen Jaeckel
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/autoboot.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/common/autoboot.c b/common/autoboot.c
index d52f88dd54..e03c387a1f 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -27,7 +27,7 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define MAX_DELAY_STOP_STR 64
+#define DELAY_STOP_STR_MAX_LENGTH 64
 
 #ifndef DEBUG_BOOTKEYS
 #define DEBUG_BOOTKEYS 0
@@ -68,7 +68,7 @@ static int menukey;
 static int passwd_abort_crypt(uint64_t etime)
 {
 	const char *crypt_env_str = env_get("bootstopkeycrypt");
-	char presskey[MAX_DELAY_STOP_STR];
+	char presskey[DELAY_STOP_STR_MAX_LENGTH];
 	u_int presskey_len = 0;
 	int abort = 0;
 	int err;
@@ -149,9 +149,9 @@ static int passwd_abort_sha256(uint64_t etime)
 	if (sha_env_str == NULL)
 		sha_env_str = CONFIG_AUTOBOOT_STOP_STR_SHA256;
 
-	presskey = malloc_cache_aligned(MAX_DELAY_STOP_STR);
+	presskey = malloc_cache_aligned(DELAY_STOP_STR_MAX_LENGTH);
 	c = strstr(sha_env_str, ":");
-	if (c && (c - sha_env_str < MAX_DELAY_STOP_STR)) {
+	if (c && (c - sha_env_str < DELAY_STOP_STR_MAX_LENGTH)) {
 		/* preload presskey with salt */
 		memcpy(presskey, sha_env_str, c - sha_env_str);
 		presskey_len = c - sha_env_str;
@@ -178,7 +178,7 @@ static int passwd_abort_sha256(uint64_t etime)
 	do {
 		if (tstc()) {
 			/* Check for input string overflow */
-			if (presskey_len >= MAX_DELAY_STOP_STR) {
+			if (presskey_len >= DELAY_STOP_STR_MAX_LENGTH) {
 				free(presskey);
 				free(sha);
 				return 0;
@@ -222,7 +222,7 @@ static int passwd_abort_key(uint64_t etime)
 		{ .str = env_get("bootstopkey"),   .retry = 0 },
 	};
 
-	char presskey[MAX_DELAY_STOP_STR];
+	char presskey[DELAY_STOP_STR_MAX_LENGTH];
 	int presskey_len = 0;
 	int presskey_max = 0;
 	int i;
@@ -239,8 +239,8 @@ static int passwd_abort_key(uint64_t etime)
 	for (i = 0; i < sizeof(delaykey) / sizeof(delaykey[0]); i++) {
 		delaykey[i].len = delaykey[i].str == NULL ?
 				    0 : strlen(delaykey[i].str);
-		delaykey[i].len = delaykey[i].len > MAX_DELAY_STOP_STR ?
-				    MAX_DELAY_STOP_STR : delaykey[i].len;
+		delaykey[i].len = delaykey[i].len > DELAY_STOP_STR_MAX_LENGTH ?
+				    DELAY_STOP_STR_MAX_LENGTH : delaykey[i].len;
 
 		presskey_max = presskey_max > delaykey[i].len ?
 				    presskey_max : delaykey[i].len;
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 5/7] cmd: allow disabling of timeout for password entry
  2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (3 preceding siblings ...)
  2021-05-10  6:19 ` [PATCH v2 4/7] common: Rename macro appropriately Steffen Jaeckel
@ 2021-05-10  6:19 ` Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 6/7] configs: add new values to bcm963158 defconfig Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 7/7] configs: add new sandbox with crypt-based password Steffen Jaeckel
  6 siblings, 0 replies; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 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>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/Kconfig.boot |  8 ++++++++
 common/autoboot.c   | 12 +++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index f81a44b23e..f837224a8d 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -860,6 +860,14 @@ config AUTOBOOT_KEYED_CTRLC
 	  Setting this variable	provides an escape sequence from the
 	  limited "password" strings.
 
+config AUTOBOOT_NEVER_TIMEOUT
+	bool "Make the password entry never time-out"
+	depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION && CRYPT_PW
+	help
+	  This option removes the timeout from the password entry
+	  when the user first presses the <Enter> key before entering
+	  any other character.
+
 config AUTOBOOT_STOP_STR_ENABLE
 	bool "Enable fixed string to stop autobooting"
 	depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION
diff --git a/common/autoboot.c b/common/autoboot.c
index e03c387a1f..50ab9281e7 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -62,6 +62,10 @@ static int menukey;
  * or
  *   the config value CONFIG_AUTOBOOT_STOP_STR_CRYPT
  *
+ * In case the config value CONFIG_AUTOBOOT_NEVER_TIMEOUT has been enabled
+ * this function never times out if the user presses the <Enter> key
+ * before starting to enter the password.
+ *
  * @etime: Timeout value ticks (stop when get_ticks() reachs this)
  * @return 0 if autoboot should continue, 1 if it should stop
  */
@@ -71,6 +75,7 @@ static int passwd_abort_crypt(uint64_t etime)
 	char presskey[DELAY_STOP_STR_MAX_LENGTH];
 	u_int presskey_len = 0;
 	int abort = 0;
+	int never_timeout = 0;
 	int err;
 
 	if (IS_ENABLED(CONFIG_AUTOBOOT_STOP_STR_ENABLE) && !crypt_env_str)
@@ -90,6 +95,11 @@ static int passwd_abort_crypt(uint64_t etime)
 
 			if ((presskey[presskey_len] == '\r') ||
 			    (presskey[presskey_len] == '\n')) {
+				if (IS_ENABLED(CONFIG_AUTOBOOT_NEVER_TIMEOUT) &&
+				    !presskey_len) {
+					never_timeout = 1;
+					continue;
+				}
 				presskey[presskey_len] = '\0';
 				err = crypt_compare(crypt_env_str, presskey,
 						    &abort);
@@ -103,7 +113,7 @@ static int passwd_abort_crypt(uint64_t etime)
 				presskey_len++;
 			}
 		}
-	} while (get_ticks() <= etime);
+	} while (never_timeout || get_ticks() <= etime);
 
 	return abort;
 }
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 6/7] configs: add new values to bcm963158 defconfig
  2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (4 preceding siblings ...)
  2021-05-10  6:19 ` [PATCH v2 5/7] cmd: allow disabling of timeout for password entry Steffen Jaeckel
@ 2021-05-10  6:19 ` Steffen Jaeckel
  2021-05-10  6:19 ` [PATCH v2 7/7] configs: add new sandbox with crypt-based password Steffen Jaeckel
  6 siblings, 0 replies; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 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>
---

(no changes since v1)

 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.31.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 7/7] configs: add new sandbox with crypt-based password
  2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (5 preceding siblings ...)
  2021-05-10  6:19 ` [PATCH v2 6/7] configs: add new values to bcm963158 defconfig Steffen Jaeckel
@ 2021-05-10  6:19 ` Steffen Jaeckel
  2021-05-10 16:28   ` Simon Glass
  6 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10  6:19 UTC (permalink / raw)
  To: u-boot

This is a copy of the regular sandbox with crypt-based password entry
enabled.

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
---

(no changes since v1)

 configs/sandbox_cryptpass_defconfig | 296 ++++++++++++++++++++++++++++
 1 file changed, 296 insertions(+)
 create mode 100644 configs/sandbox_cryptpass_defconfig

diff --git a/configs/sandbox_cryptpass_defconfig b/configs/sandbox_cryptpass_defconfig
new file mode 100644
index 0000000000..d0c8bb6b59
--- /dev/null
+++ b/configs/sandbox_cryptpass_defconfig
@@ -0,0 +1,296 @@
+CONFIG_SYS_TEXT_BASE=0
+CONFIG_NR_DRAM_BANKS=1
+CONFIG_SYS_MEMTEST_START=0x00100000
+CONFIG_SYS_MEMTEST_END=0x00101000
+CONFIG_ENV_SIZE=0x2000
+CONFIG_PRE_CON_BUF_ADDR=0xf0000
+CONFIG_BOOTSTAGE_STASH_ADDR=0x0
+CONFIG_DEFAULT_DEVICE_TREE="sandbox"
+CONFIG_DEBUG_UART=y
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_FIT=y
+CONFIG_FIT_SIGNATURE=y
+CONFIG_FIT_ENABLE_RSASSA_PSS_SUPPORT=y
+CONFIG_FIT_CIPHER=y
+CONFIG_FIT_VERBOSE=y
+CONFIG_BOOTSTAGE=y
+CONFIG_BOOTSTAGE_REPORT=y
+CONFIG_BOOTSTAGE_FDT=y
+CONFIG_BOOTSTAGE_STASH=y
+CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
+CONFIG_CONSOLE_RECORD=y
+CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
+CONFIG_PRE_CONSOLE_BUFFER=y
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_MISC_INIT_F=y
+CONFIG_STACKPROTECTOR=y
+CONFIG_ANDROID_AB=y
+CONFIG_CMD_CPU=y
+CONFIG_CMD_LICENSE=y
+CONFIG_CMD_BOOTZ=y
+CONFIG_CMD_BOOTEFI_HELLO=y
+CONFIG_CMD_ABOOTIMG=y
+# CONFIG_CMD_ELF is not set
+CONFIG_CMD_ASKENV=y
+CONFIG_CMD_GREPENV=y
+CONFIG_CMD_ERASEENV=y
+CONFIG_CMD_ENV_CALLBACK=y
+CONFIG_CMD_ENV_FLAGS=y
+CONFIG_CMD_NVEDIT_EFI=y
+CONFIG_CMD_NVEDIT_INFO=y
+CONFIG_CMD_NVEDIT_LOAD=y
+CONFIG_CMD_NVEDIT_SELECT=y
+CONFIG_LOOPW=y
+CONFIG_CMD_MD5SUM=y
+CONFIG_CMD_MEMINFO=y
+CONFIG_CMD_MEM_SEARCH=y
+CONFIG_CMD_MX_CYCLIC=y
+CONFIG_CMD_MEMTEST=y
+CONFIG_CMD_BIND=y
+CONFIG_CMD_DEMO=y
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_PWM=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_GPT_RENAME=y
+CONFIG_CMD_IDE=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_LSBLK=y
+CONFIG_CMD_MUX=y
+CONFIG_CMD_OSD=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_READ=y
+CONFIG_CMD_REMOTEPROC=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_AXI=y
+CONFIG_CMD_AB_SELECT=y
+CONFIG_BOOTP_DNS2=y
+CONFIG_CMD_PCAP=y
+CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_TFTPSRV=y
+CONFIG_CMD_RARP=y
+CONFIG_CMD_CDP=y
+CONFIG_CMD_SNTP=y
+CONFIG_CMD_DNS=y
+CONFIG_CMD_LINK_LOCAL=y
+CONFIG_CMD_ETHSW=y
+CONFIG_CMD_BMP=y
+CONFIG_CMD_BOOTCOUNT=y
+CONFIG_CMD_EFIDEBUG=y
+CONFIG_CMD_RTC=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_TIMER=y
+CONFIG_CMD_SOUND=y
+CONFIG_CMD_QFW=y
+CONFIG_CMD_PSTORE=y
+CONFIG_CMD_PSTORE_MEM_ADDR=0x3000000
+CONFIG_CMD_BOOTSTAGE=y
+CONFIG_CMD_PMIC=y
+CONFIG_CMD_REGULATOR=y
+CONFIG_CMD_AES=y
+CONFIG_CMD_TPM=y
+CONFIG_CMD_TPM_TEST=y
+CONFIG_CMD_BTRFS=y
+CONFIG_CMD_CBFS=y
+CONFIG_CMD_CRAMFS=y
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_SQUASHFS=y
+CONFIG_CMD_MTDPARTS=y
+CONFIG_CMD_STACKPROTECTOR_TEST=y
+CONFIG_MAC_PARTITION=y
+CONFIG_AMIGA_PARTITION=y
+CONFIG_OF_CONTROL=y
+CONFIG_OF_LIVE=y
+CONFIG_OF_HOSTFILE=y
+CONFIG_ENV_IS_NOWHERE=y
+CONFIG_ENV_IS_IN_EXT4=y
+CONFIG_ENV_EXT4_INTERFACE="host"
+CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
+CONFIG_BOOTP_SEND_HOSTNAME=y
+CONFIG_NETCONSOLE=y
+CONFIG_IP_DEFRAG=y
+CONFIG_DM_DMA=y
+CONFIG_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_DEVRES=y
+CONFIG_DEBUG_DEVRES=y
+CONFIG_SIMPLE_PM_BUS=y
+CONFIG_ADC=y
+CONFIG_ADC_SANDBOX=y
+CONFIG_AXI=y
+CONFIG_AXI_SANDBOX=y
+CONFIG_BOOTCOUNT_LIMIT=y
+CONFIG_DM_BOOTCOUNT=y
+CONFIG_DM_BOOTCOUNT_RTC=y
+CONFIG_DM_BOOTCOUNT_I2C_EEPROM=y
+CONFIG_BUTTON=y
+CONFIG_BUTTON_ADC=y
+CONFIG_BUTTON_GPIO=y
+CONFIG_CLK=y
+CONFIG_CLK_COMPOSITE_CCF=y
+CONFIG_CLK_SCMI=y
+CONFIG_SANDBOX_CLK_CCF=y
+CONFIG_CPU=y
+CONFIG_DM_DEMO=y
+CONFIG_DM_DEMO_SIMPLE=y
+CONFIG_DM_DEMO_SHAPE=y
+CONFIG_DFU_SF=y
+CONFIG_DMA=y
+CONFIG_DMA_CHANNELS=y
+CONFIG_SANDBOX_DMA=y
+CONFIG_FASTBOOT_FLASH=y
+CONFIG_FASTBOOT_FLASH_MMC_DEV=0
+CONFIG_GPIO_HOG=y
+CONFIG_DM_GPIO_LOOKUP_LABEL=y
+CONFIG_PM8916_GPIO=y
+CONFIG_SANDBOX_GPIO=y
+CONFIG_DM_HWSPINLOCK=y
+CONFIG_HWSPINLOCK_SANDBOX=y
+CONFIG_I2C_CROS_EC_TUNNEL=y
+CONFIG_I2C_CROS_EC_LDO=y
+CONFIG_DM_I2C_GPIO=y
+CONFIG_SYS_I2C_SANDBOX=y
+CONFIG_I2C_MUX=y
+CONFIG_SPL_I2C_MUX=y
+CONFIG_I2C_ARB_GPIO_CHALLENGE=y
+CONFIG_CROS_EC_KEYB=y
+CONFIG_I8042_KEYB=y
+CONFIG_LED=y
+CONFIG_LED_BLINK=y
+CONFIG_LED_GPIO=y
+CONFIG_DM_MAILBOX=y
+CONFIG_SANDBOX_MBOX=y
+CONFIG_MISC=y
+CONFIG_CROS_EC=y
+CONFIG_CROS_EC_I2C=y
+CONFIG_CROS_EC_LPC=y
+CONFIG_CROS_EC_SANDBOX=y
+CONFIG_CROS_EC_SPI=y
+CONFIG_P2SB=y
+CONFIG_PWRSEQ=y
+CONFIG_SPL_PWRSEQ=y
+CONFIG_I2C_EEPROM=y
+CONFIG_MMC_PCI=y
+CONFIG_MMC_SANDBOX=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MTD=y
+CONFIG_SPI_FLASH_SANDBOX=y
+CONFIG_SPI_FLASH_ATMEL=y
+CONFIG_SPI_FLASH_EON=y
+CONFIG_SPI_FLASH_GIGADEVICE=y
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_SPI_FLASH_SPANSION=y
+CONFIG_SPI_FLASH_STMICRO=y
+CONFIG_SPI_FLASH_SST=y
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_MULTIPLEXER=y
+CONFIG_MUX_MMIO=y
+CONFIG_DM_ETH=y
+CONFIG_NVME=y
+CONFIG_PCI=y
+CONFIG_DM_PCI=y
+CONFIG_DM_PCI_COMPAT=y
+CONFIG_PCI_REGION_MULTI_ENTRY=y
+CONFIG_PCI_SANDBOX=y
+CONFIG_PHY=y
+CONFIG_PHY_SANDBOX=y
+CONFIG_PINCTRL=y
+CONFIG_PINCONF=y
+CONFIG_PINCTRL_SANDBOX=y
+CONFIG_PINCTRL_SINGLE=y
+CONFIG_POWER_DOMAIN=y
+CONFIG_SANDBOX_POWER_DOMAIN=y
+CONFIG_DM_PMIC=y
+CONFIG_PMIC_ACT8846=y
+CONFIG_DM_PMIC_PFUZE100=y
+CONFIG_DM_PMIC_MAX77686=y
+CONFIG_DM_PMIC_MC34708=y
+CONFIG_PMIC_PM8916=y
+CONFIG_PMIC_RK8XX=y
+CONFIG_PMIC_S2MPS11=y
+CONFIG_DM_PMIC_SANDBOX=y
+CONFIG_PMIC_S5M8767=y
+CONFIG_PMIC_TPS65090=y
+CONFIG_DM_REGULATOR=y
+CONFIG_REGULATOR_ACT8846=y
+CONFIG_DM_REGULATOR_PFUZE100=y
+CONFIG_DM_REGULATOR_MAX77686=y
+CONFIG_DM_REGULATOR_FIXED=y
+CONFIG_REGULATOR_RK8XX=y
+CONFIG_REGULATOR_S5M8767=y
+CONFIG_DM_REGULATOR_SANDBOX=y
+CONFIG_REGULATOR_TPS65090=y
+CONFIG_DM_REGULATOR_SCMI=y
+CONFIG_DM_PWM=y
+CONFIG_PWM_SANDBOX=y
+CONFIG_RAM=y
+CONFIG_REMOTEPROC_SANDBOX=y
+CONFIG_DM_RESET=y
+CONFIG_SANDBOX_RESET=y
+CONFIG_RESET_SYSCON=y
+CONFIG_RESET_SCMI=y
+CONFIG_DM_RNG=y
+CONFIG_DM_RTC=y
+CONFIG_RTC_RV8803=y
+CONFIG_SANDBOX_SERIAL=y
+CONFIG_SMEM=y
+CONFIG_SANDBOX_SMEM=y
+CONFIG_SOUND=y
+CONFIG_SOUND_DA7219=y
+CONFIG_SOUND_MAX98357A=y
+CONFIG_SOUND_SANDBOX=y
+CONFIG_SOC_DEVICE=y
+CONFIG_SANDBOX_SPI=y
+CONFIG_SPMI=y
+CONFIG_SPMI_SANDBOX=y
+CONFIG_SYSINFO=y
+CONFIG_SYSINFO_SANDBOX=y
+CONFIG_SYSRESET=y
+CONFIG_TIMER=y
+CONFIG_TIMER_EARLY=y
+CONFIG_SANDBOX_TIMER=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_EMUL=y
+CONFIG_USB_KEYBOARD=y
+CONFIG_DM_VIDEO=y
+CONFIG_VIDEO_COPY=y
+CONFIG_CONSOLE_ROTATION=y
+CONFIG_CONSOLE_TRUETYPE=y
+CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y
+CONFIG_VIDEO_SANDBOX_SDL=y
+CONFIG_VIDEO_DSI_HOST_SANDBOX=y
+CONFIG_OSD=y
+CONFIG_SANDBOX_OSD=y
+CONFIG_SPLASH_SCREEN_ALIGN=y
+CONFIG_VIDEO_BMP_RLE8=y
+CONFIG_W1=y
+CONFIG_W1_GPIO=y
+CONFIG_W1_EEPROM=y
+CONFIG_W1_EEPROM_SANDBOX=y
+CONFIG_WDT=y
+CONFIG_WDT_SANDBOX=y
+CONFIG_FS_CBFS=y
+CONFIG_FS_CRAMFS=y
+CONFIG_CMD_DHRYSTONE=y
+CONFIG_TPM=y
+CONFIG_LZ4=y
+CONFIG_ERRNO_STR=y
+CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
+CONFIG_EFI_CAPSULE_ON_DISK=y
+CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
+CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
+CONFIG_EFI_SECURE_BOOT=y
+CONFIG_TEST_FDTDEC=y
+CONFIG_CRYPT_PW=y
+CONFIG_CRYPT_PW_SHA256=y
+CONFIG_AUTOBOOT_KEYED=y
+CONFIG_AUTOBOOT_PROMPT="Enter password in %d seconds to stop autoboot\n"
+CONFIG_AUTOBOOT_ENCRYPTION=y
+CONFIG_AUTOBOOT_STOP_STR_ENABLE=y
+# default password "password"
+CONFIG_AUTOBOOT_STOP_STR_CRYPT="$5$rounds=640000$TM4lL4zXDG7F4aRX$JM7a9wmvodnA0WasjTztj6mxg.KVuk6doQ/eBhdcapB"
+CONFIG_AUTOBOOT_NEVER_TIMEOUT=y
+CONFIG_UNIT_TEST=y
+CONFIG_UT_TIME=y
+CONFIG_UT_DM=y
-- 
2.31.1

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 2/7] lib: wrap crypt API to hide errno usage
  2021-05-10  6:19 ` [PATCH v2 2/7] lib: wrap crypt API to hide errno usage Steffen Jaeckel
@ 2021-05-10 16:27   ` Simon Glass
  2021-05-10 17:05     ` Steffen Jaeckel
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2021-05-10 16:27 UTC (permalink / raw)
  To: u-boot

On Mon, 10 May 2021 at 00:19, Steffen Jaeckel
<jaeckel-floss@eyet-services.de> wrote:
>
> In order to prevent using the global errno, replace it with a static
> version and create a wrapper function which returns the error value.
>
> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
> ---
>
> (no changes since v1)
>
>  include/crypt.h          |  3 ++-
>  lib/crypt/alg-sha256.h   |  6 ------
>  lib/crypt/alg-sha512.h   |  6 ------
>  lib/crypt/crypt-port.h   | 18 ++++++++++--------
>  lib/crypt/crypt-sha256.c | 26 ++++++++++++++++++++++++--
>  lib/crypt/crypt-sha512.c | 26 ++++++++++++++++++++++++--
>  lib/crypt/crypt.c        | 25 ++++++++++++++-----------
>  test/lib/test_crypt.c    | 24 ++++++++++++++++++++++--
>  8 files changed, 96 insertions(+), 38 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10  6:19 ` [PATCH v2 3/7] common: integrate crypt-based passwords Steffen Jaeckel
@ 2021-05-10 16:27   ` Simon Glass
  2021-05-10 17:05     ` Steffen Jaeckel
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2021-05-10 16:27 UTC (permalink / raw)
  To: u-boot

On Mon, 10 May 2021 at 00:19, 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>
> ---
>
> (no changes since v1)
>
>  common/Kconfig.boot | 37 ++++++++++++++++++---
>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 103 insertions(+), 14 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But I think you'll need to allow both to be enabled.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 7/7] configs: add new sandbox with crypt-based password
  2021-05-10  6:19 ` [PATCH v2 7/7] configs: add new sandbox with crypt-based password Steffen Jaeckel
@ 2021-05-10 16:28   ` Simon Glass
  2021-05-10 17:04     ` Steffen Jaeckel
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2021-05-10 16:28 UTC (permalink / raw)
  To: u-boot

Hi Steffen,

On Mon, 10 May 2021 at 00:19, Steffen Jaeckel
<jaeckel-floss@eyet-services.de> wrote:
>
> This is a copy of the regular sandbox with crypt-based password entry
> enabled.
>
> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
> ---
>
> (no changes since v1)
>
>  configs/sandbox_cryptpass_defconfig | 296 ++++++++++++++++++++++++++++
>  1 file changed, 296 insertions(+)
>  create mode 100644 configs/sandbox_cryptpass_defconfig

Is this so you can add a test? In that case I think you need to adjust things...

For sandbox you need to be able to build in both options, so your
if/else approach won't work.

You could control it with an env var perhaps, when both are enabled.
Then it is easy enough to select the required setup with sandbox.

Regards,
Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 7/7] configs: add new sandbox with crypt-based password
  2021-05-10 16:28   ` Simon Glass
@ 2021-05-10 17:04     ` Steffen Jaeckel
  0 siblings, 0 replies; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10 17:04 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 5/10/21 6:28 PM, Simon Glass wrote:
> Hi Steffen,
> 
> On Mon, 10 May 2021 at 00:19, Steffen Jaeckel
> <jaeckel-floss@eyet-services.de> wrote:
>>
>> This is a copy of the regular sandbox with crypt-based password entry
>> enabled.
>>
>> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
>> ---
>>
>> (no changes since v1)
>>
>>  configs/sandbox_cryptpass_defconfig | 296 ++++++++++++++++++++++++++++
>>  1 file changed, 296 insertions(+)
>>  create mode 100644 configs/sandbox_cryptpass_defconfig
> 
> Is this so you can add a test? In that case I think you need to adjust things...
> 
> For sandbox you need to be able to build in both options, so your
> if/else approach won't work.
> 
> You could control it with an env var perhaps, when both are enabled.
> Then it is easy enough to select the required setup with sandbox.

as mentioned in the cover letter, I guess this commit could also be
removed. I'm going to make a v3 without it.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10 16:27   ` Simon Glass
@ 2021-05-10 17:05     ` Steffen Jaeckel
  2021-05-10 19:19       ` Simon Glass
  0 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10 17:05 UTC (permalink / raw)
  To: u-boot



On 5/10/21 6:27 PM, Simon Glass wrote:
> On Mon, 10 May 2021 at 00:19, 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>
>> ---
>>
>> (no changes since v1)
>>
>>  common/Kconfig.boot | 37 ++++++++++++++++++---
>>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
>>  2 files changed, 103 insertions(+), 14 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> But I think you'll need to allow both to be enabled.

Sorry, but what exactly do you mean?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 2/7] lib: wrap crypt API to hide errno usage
  2021-05-10 16:27   ` Simon Glass
@ 2021-05-10 17:05     ` Steffen Jaeckel
  0 siblings, 0 replies; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10 17:05 UTC (permalink / raw)
  To: u-boot

On 5/10/21 6:27 PM, Simon Glass wrote:
> On Mon, 10 May 2021 at 00:19, Steffen Jaeckel
> <jaeckel-floss@eyet-services.de> wrote:
>>
>> In order to prevent using the global errno, replace it with a static
>> version and create a wrapper function which returns the error value.
>>
>> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
>> ---
>>
>> (no changes since v1)
>>
>>  include/crypt.h          |  3 ++-
>>  lib/crypt/alg-sha256.h   |  6 ------
>>  lib/crypt/alg-sha512.h   |  6 ------
>>  lib/crypt/crypt-port.h   | 18 ++++++++++--------
>>  lib/crypt/crypt-sha256.c | 26 ++++++++++++++++++++++++--
>>  lib/crypt/crypt-sha512.c | 26 ++++++++++++++++++++++++--
>>  lib/crypt/crypt.c        | 25 ++++++++++++++-----------
>>  test/lib/test_crypt.c    | 24 ++++++++++++++++++++++--
>>  8 files changed, 96 insertions(+), 38 deletions(-)
>>
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Thx!

Cheers
Steffen

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10 17:05     ` Steffen Jaeckel
@ 2021-05-10 19:19       ` Simon Glass
  2021-05-10 20:05         ` Steffen Jaeckel
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2021-05-10 19:19 UTC (permalink / raw)
  To: u-boot

Hi Steffen,

On Mon, 10 May 2021 at 11:05, Steffen Jaeckel
<jaeckel-floss@eyet-services.de> wrote:
>
>
>
> On 5/10/21 6:27 PM, Simon Glass wrote:
> > On Mon, 10 May 2021 at 00:19, 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>
> >> ---
> >>
> >> (no changes since v1)
> >>
> >>  common/Kconfig.boot | 37 ++++++++++++++++++---
> >>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
> >>  2 files changed, 103 insertions(+), 14 deletions(-)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > But I think you'll need to allow both to be enabled.
>
> Sorry, but what exactly do you mean?

I mean the ability to enable both crypt and the sha options rather
than just one at a time.

Regards,
Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10 19:19       ` Simon Glass
@ 2021-05-10 20:05         ` Steffen Jaeckel
  2021-05-10 20:24           ` Simon Glass
  0 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10 20:05 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 5/10/21 9:19 PM, Simon Glass wrote:
> On Mon, 10 May 2021 at 11:05, Steffen Jaeckel
> <jaeckel-floss@eyet-services.de> wrote:
>> On 5/10/21 6:27 PM, Simon Glass wrote:
>>> On Mon, 10 May 2021 at 00:19, 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>
>>>> ---
>>>>
>>>> (no changes since v1)
>>>>
>>>>  common/Kconfig.boot | 37 ++++++++++++++++++---
>>>>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
>>>>  2 files changed, 103 insertions(+), 14 deletions(-)
>>>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>> But I think you'll need to allow both to be enabled.
>>
>> Sorry, but what exactly do you mean?
> 
> I mean the ability to enable both crypt and the sha options rather
> than just one at a time.

You have a point there. Even though this approach should IMO become the
recommended way to store passwords, one could imagine that support for
both approaches in parallel could be needed, e.g. in a transition period.

The biggest problem I see is that the passwd_abort_{crypt,sha256}()
functions consume the serial input. I fear that an implementation that
supports both would need to have a painful amount of special case
handling in order to not break the expected behavior of the existing
sha256 implementation.

Supporting both in a backwards compatible way would make the
implementation a lot more complex and therefor I'd prefer to leave it as is.

Cheers
Steffen

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10 20:05         ` Steffen Jaeckel
@ 2021-05-10 20:24           ` Simon Glass
  2021-05-10 20:36             ` Steffen Jaeckel
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2021-05-10 20:24 UTC (permalink / raw)
  To: u-boot

Hi Steffen,

On Mon, 10 May 2021 at 14:06, Steffen Jaeckel
<jaeckel-floss@eyet-services.de> wrote:
>
> Hi Simon,
>
> On 5/10/21 9:19 PM, Simon Glass wrote:
> > On Mon, 10 May 2021 at 11:05, Steffen Jaeckel
> > <jaeckel-floss@eyet-services.de> wrote:
> >> On 5/10/21 6:27 PM, Simon Glass wrote:
> >>> On Mon, 10 May 2021 at 00:19, 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>
> >>>> ---
> >>>>
> >>>> (no changes since v1)
> >>>>
> >>>>  common/Kconfig.boot | 37 ++++++++++++++++++---
> >>>>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
> >>>>  2 files changed, 103 insertions(+), 14 deletions(-)
> >>>
> >>> Reviewed-by: Simon Glass <sjg@chromium.org>
> >>>
> >>> But I think you'll need to allow both to be enabled.
> >>
> >> Sorry, but what exactly do you mean?
> >
> > I mean the ability to enable both crypt and the sha options rather
> > than just one at a time.
>
> You have a point there. Even though this approach should IMO become the
> recommended way to store passwords, one could imagine that support for
> both approaches in parallel could be needed, e.g. in a transition period.
>
> The biggest problem I see is that the passwd_abort_{crypt,sha256}()
> functions consume the serial input. I fear that an implementation that
> supports both would need to have a painful amount of special case
> handling in order to not break the expected behavior of the existing
> sha256 implementation.
>
> Supporting both in a backwards compatible way would make the
> implementation a lot more complex and therefor I'd prefer to leave it as is.

I don't quite mean that. Only one should be used at once. But would it
be possible to support both at runtime, so that (at runtime) you check
an env var to decide which is active?

Regards,
Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10 20:24           ` Simon Glass
@ 2021-05-10 20:36             ` Steffen Jaeckel
  2021-05-10 20:45               ` Simon Glass
  0 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-10 20:36 UTC (permalink / raw)
  To: u-boot

Hi Simon

On 5/10/21 10:24 PM, Simon Glass wrote:
> On Mon, 10 May 2021 at 14:06, Steffen Jaeckel
> <jaeckel-floss@eyet-services.de> wrote:
>> On 5/10/21 9:19 PM, Simon Glass wrote:
>>> On Mon, 10 May 2021 at 11:05, Steffen Jaeckel
>>> <jaeckel-floss@eyet-services.de> wrote:
>>>> On 5/10/21 6:27 PM, Simon Glass wrote:
>>>>> On Mon, 10 May 2021 at 00:19, 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>
>>>>>> ---
>>>>>>
>>>>>> (no changes since v1)
>>>>>>
>>>>>>  common/Kconfig.boot | 37 ++++++++++++++++++---
>>>>>>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
>>>>>>  2 files changed, 103 insertions(+), 14 deletions(-)
>>>>>
>>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>>>
>>>>> But I think you'll need to allow both to be enabled.
>>>>
>>>> Sorry, but what exactly do you mean?
>>>
>>> I mean the ability to enable both crypt and the sha options rather
>>> than just one at a time.
>>
>> You have a point there. Even though this approach should IMO become the
>> recommended way to store passwords, one could imagine that support for
>> both approaches in parallel could be needed, e.g. in a transition period.
>>
>> The biggest problem I see is that the passwd_abort_{crypt,sha256}()
>> functions consume the serial input. I fear that an implementation that
>> supports both would need to have a painful amount of special case
>> handling in order to not break the expected behavior of the existing
>> sha256 implementation.
>>
>> Supporting both in a backwards compatible way would make the
>> implementation a lot more complex and therefor I'd prefer to leave it as is.
> 
> I don't quite mean that. Only one should be used at once. But would it
> be possible to support both at runtime, so that (at runtime) you check
> an env var to decide which is active?

oh okay, that's easier :)

maybe something like this?

diff --git a/common/autoboot.c b/common/autoboot.c
index 50ab9281e7..6f55abe388 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -316,3 +316,4 @@ static int abortboot_key_sequence(int bootdelay)
        if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
-               if (IS_ENABLED(CONFIG_CRYPT_PW))
+               if (IS_ENABLED(CONFIG_CRYPT_PW) &&
+                   env_get_yesno("bootstopusesha256") != 1)
                        abort = passwd_abort_crypt(etime);

Cheers
Steffen

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10 20:36             ` Steffen Jaeckel
@ 2021-05-10 20:45               ` Simon Glass
  2021-05-11 15:02                 ` Steffen Jaeckel
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2021-05-10 20:45 UTC (permalink / raw)
  To: u-boot

Hi Steffen,

On Mon, 10 May 2021 at 13:37, Steffen Jaeckel
<jaeckel-floss@eyet-services.de> wrote:
>
> Hi Simon
>
> On 5/10/21 10:24 PM, Simon Glass wrote:
> > On Mon, 10 May 2021 at 14:06, Steffen Jaeckel
> > <jaeckel-floss@eyet-services.de> wrote:
> >> On 5/10/21 9:19 PM, Simon Glass wrote:
> >>> On Mon, 10 May 2021 at 11:05, Steffen Jaeckel
> >>> <jaeckel-floss@eyet-services.de> wrote:
> >>>> On 5/10/21 6:27 PM, Simon Glass wrote:
> >>>>> On Mon, 10 May 2021 at 00:19, 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>
> >>>>>> ---
> >>>>>>
> >>>>>> (no changes since v1)
> >>>>>>
> >>>>>>  common/Kconfig.boot | 37 ++++++++++++++++++---
> >>>>>>  common/autoboot.c   | 80 ++++++++++++++++++++++++++++++++++++++++-----
> >>>>>>  2 files changed, 103 insertions(+), 14 deletions(-)
> >>>>>
> >>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
> >>>>>
> >>>>> But I think you'll need to allow both to be enabled.
> >>>>
> >>>> Sorry, but what exactly do you mean?
> >>>
> >>> I mean the ability to enable both crypt and the sha options rather
> >>> than just one at a time.
> >>
> >> You have a point there. Even though this approach should IMO become the
> >> recommended way to store passwords, one could imagine that support for
> >> both approaches in parallel could be needed, e.g. in a transition period.
> >>
> >> The biggest problem I see is that the passwd_abort_{crypt,sha256}()
> >> functions consume the serial input. I fear that an implementation that
> >> supports both would need to have a painful amount of special case
> >> handling in order to not break the expected behavior of the existing
> >> sha256 implementation.
> >>
> >> Supporting both in a backwards compatible way would make the
> >> implementation a lot more complex and therefor I'd prefer to leave it as is.
> >
> > I don't quite mean that. Only one should be used at once. But would it
> > be possible to support both at runtime, so that (at runtime) you check
> > an env var to decide which is active?
>
> oh okay, that's easier :)
>
> maybe something like this?
>
> diff --git a/common/autoboot.c b/common/autoboot.c
> index 50ab9281e7..6f55abe388 100644
> --- a/common/autoboot.c
> +++ b/common/autoboot.c
> @@ -316,3 +316,4 @@ static int abortboot_key_sequence(int bootdelay)
>         if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
> -               if (IS_ENABLED(CONFIG_CRYPT_PW))
> +               if (IS_ENABLED(CONFIG_CRYPT_PW) &&
> +                   env_get_yesno("bootstopusesha256") != 1)
>                         abort = passwd_abort_crypt(etime);

Yes, and then you can enable both in sandbox and potentially have a
test for your code within the standard sandbox build.

Regards,
Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-10 20:45               ` Simon Glass
@ 2021-05-11 15:02                 ` Steffen Jaeckel
  2021-05-11 15:27                   ` Simon Glass
  0 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-11 15:02 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 5/10/21 10:45 PM, Simon Glass wrote:
> On Mon, 10 May 2021 at 13:37, Steffen Jaeckel
> <jaeckel-floss@eyet-services.de> wrote:

[snip]

>> diff --git a/common/autoboot.c b/common/autoboot.c
>> index 50ab9281e7..6f55abe388 100644
>> --- a/common/autoboot.c
>> +++ b/common/autoboot.c
>> @@ -316,3 +316,4 @@ static int abortboot_key_sequence(int bootdelay)
>>         if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
>> -               if (IS_ENABLED(CONFIG_CRYPT_PW))
>> +               if (IS_ENABLED(CONFIG_CRYPT_PW) &&
>> +                   env_get_yesno("bootstopusesha256") != 1)
>>                         abort = passwd_abort_crypt(etime);
> 
> Yes, and then you can enable both in sandbox and potentially have a
> test for your code within the standard sandbox build.

What kind of tests do you want to have added? Python based or C based ones?

TBH I don't see an easy way (yet) to add more tests than the ones I
already added, as enabling AUTOBOOT_KEYED (which is required for both,
crypt and sha256) would change the startup behavior of the sandbox...


Cheers
Steffen

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-11 15:02                 ` Steffen Jaeckel
@ 2021-05-11 15:27                   ` Simon Glass
  2021-05-11 18:29                     ` Steffen Jaeckel
  0 siblings, 1 reply; 23+ messages in thread
From: Simon Glass @ 2021-05-11 15:27 UTC (permalink / raw)
  To: u-boot

Hi Steffen,

On Tue, 11 May 2021 at 09:02, Steffen Jaeckel
<jaeckel-floss@eyet-services.de> wrote:
>
> Hi Simon,
>
> On 5/10/21 10:45 PM, Simon Glass wrote:
> > On Mon, 10 May 2021 at 13:37, Steffen Jaeckel
> > <jaeckel-floss@eyet-services.de> wrote:
>
> [snip]
>
> >> diff --git a/common/autoboot.c b/common/autoboot.c
> >> index 50ab9281e7..6f55abe388 100644
> >> --- a/common/autoboot.c
> >> +++ b/common/autoboot.c
> >> @@ -316,3 +316,4 @@ static int abortboot_key_sequence(int bootdelay)
> >>         if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
> >> -               if (IS_ENABLED(CONFIG_CRYPT_PW))
> >> +               if (IS_ENABLED(CONFIG_CRYPT_PW) &&
> >> +                   env_get_yesno("bootstopusesha256") != 1)
> >>                         abort = passwd_abort_crypt(etime);
> >
> > Yes, and then you can enable both in sandbox and potentially have a
> > test for your code within the standard sandbox build.
>
> What kind of tests do you want to have added? Python based or C based ones?
>
> TBH I don't see an easy way (yet) to add more tests than the ones I
> already added, as enabling AUTOBOOT_KEYED (which is required for both,
> crypt and sha256) would change the startup behavior of the sandbox...

Here is my idea...we have console monitoring, like this:

console_record_reset();
run_command("acpi dump rdst", 0);
ut_assert_nextline("Table 'RDST' not found");
ut_assert_console_end();

What is needed is the ability to inject console input. We have
gd->console_in (in console.c) but there is currently no function to
add input to it. Something similar to console_record_puts() is needed,
perhaps called console_write_in(), which does a
membuff_put(&gd->console_in, ...) with some input data (the hash).
That way the input can be read by sandbox.

Then I think you could write a test like this:

console_record_reset();
console_write_in(hash_string, strlen(hash_string));
ut_assertok(autoboot_command(""));
ut_assert_nextline("whatever indicates success");
ut_assert_console_end();

Regards,
SImon

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-11 15:27                   ` Simon Glass
@ 2021-05-11 18:29                     ` Steffen Jaeckel
  2021-05-12 16:17                       ` Simon Glass
  0 siblings, 1 reply; 23+ messages in thread
From: Steffen Jaeckel @ 2021-05-11 18:29 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 5/11/21 5:27 PM, Simon Glass wrote:
>>
>> [snip]
>>
>>>> diff --git a/common/autoboot.c b/common/autoboot.c
>>>> index 50ab9281e7..6f55abe388 100644
>>>> --- a/common/autoboot.c
>>>> +++ b/common/autoboot.c
>>>> @@ -316,3 +316,4 @@ static int abortboot_key_sequence(int bootdelay)
>>>>         if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
>>>> -               if (IS_ENABLED(CONFIG_CRYPT_PW))
>>>> +               if (IS_ENABLED(CONFIG_CRYPT_PW) &&
>>>> +                   env_get_yesno("bootstopusesha256") != 1)
>>>>                         abort = passwd_abort_crypt(etime);
>>>
>>> Yes, and then you can enable both in sandbox and potentially have a
>>> test for your code within the standard sandbox build.
>>
>> What kind of tests do you want to have added? Python based or C based ones?
>>
>> TBH I don't see an easy way (yet) to add more tests than the ones I
>> already added, as enabling AUTOBOOT_KEYED (which is required for both,
>> crypt and sha256) would change the startup behavior of the sandbox...
> 
> Here is my idea...we have console monitoring, like this:
> 
> console_record_reset();
> run_command("acpi dump rdst", 0);
> ut_assert_nextline("Table 'RDST' not found");
> ut_assert_console_end();
> 
> What is needed is the ability to inject console input. We have
> gd->console_in (in console.c) but there is currently no function to
> add input to it. Something similar to console_record_puts() is needed,
> perhaps called console_write_in(), which does a
> membuff_put(&gd->console_in, ...) with some input data (the hash).
> That way the input can be read by sandbox.
> 
> Then I think you could write a test like this:
> 
> console_record_reset();
> console_write_in(hash_string, strlen(hash_string));
> ut_assertok(autoboot_command(""));
> ut_assert_nextline("whatever indicates success");
> ut_assert_console_end();

OK, that sounds fine, with the only problem that there's no way to
enable the necessary features without also having them enabled in the
autoboot flow!?
i.e. instead of having a single keypress to enable the console of the
sandbox, one would always have to enter the password, or am I missing
something?

Cheers
Steffen

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 3/7] common: integrate crypt-based passwords
  2021-05-11 18:29                     ` Steffen Jaeckel
@ 2021-05-12 16:17                       ` Simon Glass
  0 siblings, 0 replies; 23+ messages in thread
From: Simon Glass @ 2021-05-12 16:17 UTC (permalink / raw)
  To: u-boot

Hi Steffen,

On Tue, 11 May 2021 at 12:30, Steffen Jaeckel
<jaeckel-floss@eyet-services.de> wrote:
>
> Hi Simon,
>
> On 5/11/21 5:27 PM, Simon Glass wrote:
> >>
> >> [snip]
> >>
> >>>> diff --git a/common/autoboot.c b/common/autoboot.c
> >>>> index 50ab9281e7..6f55abe388 100644
> >>>> --- a/common/autoboot.c
> >>>> +++ b/common/autoboot.c
> >>>> @@ -316,3 +316,4 @@ static int abortboot_key_sequence(int bootdelay)
> >>>>         if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
> >>>> -               if (IS_ENABLED(CONFIG_CRYPT_PW))
> >>>> +               if (IS_ENABLED(CONFIG_CRYPT_PW) &&
> >>>> +                   env_get_yesno("bootstopusesha256") != 1)
> >>>>                         abort = passwd_abort_crypt(etime);
> >>>
> >>> Yes, and then you can enable both in sandbox and potentially have a
> >>> test for your code within the standard sandbox build.
> >>
> >> What kind of tests do you want to have added? Python based or C based ones?
> >>
> >> TBH I don't see an easy way (yet) to add more tests than the ones I
> >> already added, as enabling AUTOBOOT_KEYED (which is required for both,
> >> crypt and sha256) would change the startup behavior of the sandbox...
> >
> > Here is my idea...we have console monitoring, like this:
> >
> > console_record_reset();
> > run_command("acpi dump rdst", 0);
> > ut_assert_nextline("Table 'RDST' not found");
> > ut_assert_console_end();
> >
> > What is needed is the ability to inject console input. We have
> > gd->console_in (in console.c) but there is currently no function to
> > add input to it. Something similar to console_record_puts() is needed,
> > perhaps called console_write_in(), which does a
> > membuff_put(&gd->console_in, ...) with some input data (the hash).
> > That way the input can be read by sandbox.
> >
> > Then I think you could write a test like this:
> >
> > console_record_reset();
> > console_write_in(hash_string, strlen(hash_string));
> > ut_assertok(autoboot_command(""));
> > ut_assert_nextline("whatever indicates success");
> > ut_assert_console_end();
>
> OK, that sounds fine, with the only problem that there's no way to
> enable the necessary features without also having them enabled in the
> autoboot flow!?
>
> i.e. instead of having a single keypress to enable the console of the
> sandbox, one would always have to enter the password, or am I missing
> something?

Yes I see. Perhaps have a function to indicate whether it should be
enabled at runtime? For sandbox it can return false?

You could add a sandbox command-line flag to make it skip this, I
suppose. An environment variable is another option.

We often have little tweaks with tests to get them to do what we want.

Regards,
Simon

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-05-12 16:17 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10  6:19 [PATCH v2 0/7] common: Introduce crypt-style password support Steffen Jaeckel
2021-05-10  6:19 ` [PATCH v2 1/7] lib: add crypt subsystem Steffen Jaeckel
2021-05-10  6:19 ` [PATCH v2 2/7] lib: wrap crypt API to hide errno usage Steffen Jaeckel
2021-05-10 16:27   ` Simon Glass
2021-05-10 17:05     ` Steffen Jaeckel
2021-05-10  6:19 ` [PATCH v2 3/7] common: integrate crypt-based passwords Steffen Jaeckel
2021-05-10 16:27   ` Simon Glass
2021-05-10 17:05     ` Steffen Jaeckel
2021-05-10 19:19       ` Simon Glass
2021-05-10 20:05         ` Steffen Jaeckel
2021-05-10 20:24           ` Simon Glass
2021-05-10 20:36             ` Steffen Jaeckel
2021-05-10 20:45               ` Simon Glass
2021-05-11 15:02                 ` Steffen Jaeckel
2021-05-11 15:27                   ` Simon Glass
2021-05-11 18:29                     ` Steffen Jaeckel
2021-05-12 16:17                       ` Simon Glass
2021-05-10  6:19 ` [PATCH v2 4/7] common: Rename macro appropriately Steffen Jaeckel
2021-05-10  6:19 ` [PATCH v2 5/7] cmd: allow disabling of timeout for password entry Steffen Jaeckel
2021-05-10  6:19 ` [PATCH v2 6/7] configs: add new values to bcm963158 defconfig Steffen Jaeckel
2021-05-10  6:19 ` [PATCH v2 7/7] configs: add new sandbox with crypt-based password Steffen Jaeckel
2021-05-10 16:28   ` Simon Glass
2021-05-10 17:04     ` 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.