All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/8] common: Introduce crypt-style password support
@ 2021-07-07 23:09 Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 1/8] lib: add crypt subsystem Steffen Jaeckel
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Alexandru Gagniuc, Anastasiia Lukianenko,
	Andrii Anisov, Andy Shevchenko, Aswath Govindraju, Bin Meng,
	Christian Gmeiner, Da Xue, Heiko Schocher, Heinrich Schuchardt,
	Joel Peshkin, Joel Stanley, Klaus Heinrich Kiwi,
	Marek Szyprowski, Masahisa Kojima, Patrick Delaunay,
	Priyanka Jain, Sean Anderson, Siew Chin Lim, Simon Glass,
	Tero Kristo, Yuezhang.Mo


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 v4:
Fix depends for unit-tests
Take review comments into account
Add another test with `bootstopusesha256` unset

Changes in v3:
Add unit-tests for autoboot
Introduce `bootstopusesha256` to allow fallback to plain SHA256-based
hashing
Add AUTOBOOT_FLUSH_STDIN option
Drop the changes to bcm963158_ram_defconfig

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

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

Steffen Jaeckel (8):
  lib: add crypt subsystem
  lib: wrap crypt API to hide errno usage
  common: integrate crypt-based passwords
  common: Rename macro appropriately
  common: allow disabling of timeout for password entry
  common: add AUTOBOOT_FLUSH_STDIN option
  common: add support to fallback to plain SHA256
  test: add first autoboot unit tests

 common/Kconfig.boot         |  65 ++++++-
 common/autoboot.c           | 136 ++++++++++++--
 common/console.c            |   5 +
 configs/sandbox_defconfig   |  13 +-
 include/console.h           |  17 ++
 include/crypt.h             |  14 ++
 include/test/common.h       |  15 ++
 include/test/suites.h       |   1 +
 lib/Kconfig                 |   1 +
 lib/Makefile                |   1 +
 lib/crypt/Kconfig           |  28 +++
 lib/crypt/Makefile          |  10 ++
 lib/crypt/alg-sha256.h      |  11 ++
 lib/crypt/alg-sha512.h      |  11 ++
 lib/crypt/crypt-port.h      |  30 ++++
 lib/crypt/crypt-sha256.c    | 335 ++++++++++++++++++++++++++++++++++
 lib/crypt/crypt-sha512.c    | 350 ++++++++++++++++++++++++++++++++++++
 lib/crypt/crypt.c           |  76 ++++++++
 test/Kconfig                |  10 ++
 test/Makefile               |   1 +
 test/cmd_ut.c               |   1 +
 test/common/Makefile        |   3 +
 test/common/cmd_ut_common.c |  22 +++
 test/common/test_autoboot.c |  90 ++++++++++
 test/lib/Makefile           |   1 +
 test/lib/test_crypt.c       |  64 +++++++
 26 files changed, 1288 insertions(+), 23 deletions(-)
 create mode 100644 include/crypt.h
 create mode 100644 include/test/common.h
 create mode 100644 lib/crypt/Kconfig
 create mode 100644 lib/crypt/Makefile
 create mode 100644 lib/crypt/alg-sha256.h
 create mode 100644 lib/crypt/alg-sha512.h
 create mode 100644 lib/crypt/crypt-port.h
 create mode 100644 lib/crypt/crypt-sha256.c
 create mode 100644 lib/crypt/crypt-sha512.c
 create mode 100644 lib/crypt/crypt.c
 create mode 100644 test/common/Makefile
 create mode 100644 test/common/cmd_ut_common.c
 create mode 100644 test/common/test_autoboot.c
 create mode 100644 test/lib/test_crypt.c

-- 
2.32.0


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

* [PATCH v4 1/8] lib: add crypt subsystem
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  2021-07-08  3:56   ` Heiko Schocher
  2021-07-07 23:09 ` [PATCH v4 2/8] lib: wrap crypt API to hide errno usage Steffen Jaeckel
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Simon Glass, Alexandru Gagniuc,
	Anastasiia Lukianenko, Andrii Anisov, Aswath Govindraju,
	Bin Meng, Christian Gmeiner, Heinrich Schuchardt, Joel Stanley,
	Marek Szyprowski, Masahisa Kojima, Patrick Delaunay,
	Sean Anderson, Tero Kristo

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 v4:
Fix depends for unit-tests

Changes in v3:
Add unit-tests for autoboot
Introduce `bootstopusesha256` to allow fallback to plain SHA256-based
hashing
Add AUTOBOOT_FLUSH_STDIN option
Drop the changes to bcm963158_ram_defconfig

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

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

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


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

* [PATCH v4 2/8] lib: wrap crypt API to hide errno usage
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 1/8] lib: add crypt subsystem Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  2021-07-08  3:58   ` Heiko Schocher
  2021-07-07 23:09 ` [PATCH v4 3/8] common: integrate crypt-based passwords Steffen Jaeckel
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot; +Cc: Steffen Jaeckel, Simon Glass

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

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

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

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


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

* [PATCH v4 3/8] common: integrate crypt-based passwords
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 1/8] lib: add crypt subsystem Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 2/8] lib: wrap crypt API to hide errno usage Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  2021-07-08  4:00   ` Heiko Schocher
  2021-07-07 23:09 ` [PATCH v4 4/8] common: Rename macro appropriately Steffen Jaeckel
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Simon Glass, Alexandru Gagniuc, Bin Meng,
	Da Xue, Heiko Schocher, Heinrich Schuchardt, Joel Peshkin,
	Klaus Heinrich Kiwi, Masahisa Kojima, Siew Chin Lim, Yuezhang.Mo

Hook into the autoboot flow as an alternative to the existing
mechanisms.

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/Kconfig.boot | 38 ++++++++++++++++++---
 common/autoboot.c   | 81 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 105 insertions(+), 14 deletions(-)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 89a3161f1f..410d117cdd 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -811,10 +811,17 @@ config AUTOBOOT_ENCRYPTION
 	depends on AUTOBOOT_KEYED
 	help
 	  This option allows a string to be entered into U-Boot to stop the
-	  autoboot. The string itself is hashed and compared against the hash
-	  in the environment variable 'bootstopkeysha256'. If it matches then
-	  boot stops and a command-line prompt is presented.
-
+	  autoboot.
+	  The behavior depends whether CONFIG_CRYPT_PW from lib is enabled
+	  or not.
+	  In case CONFIG_CRYPT_PW is enabled, the string will be forwarded
+	  to the crypt-based functionality and be compared against the
+	  string in the environment variable 'bootstopkeycrypt'.
+	  In case CONFIG_CRYPT_PW is disabled the string itself is hashed
+	  and compared against the hash in the environment variable
+	  'bootstopkeysha256'.
+	  If it matches in either case then boot stops and
+	  a command-line prompt is presented.
 	  This provides a way to ship a secure production device which can also
 	  be accessed at the U-Boot command line.
 
@@ -852,9 +859,30 @@ config AUTOBOOT_KEYED_CTRLC
 	  Setting this variable	provides an escape sequence from the
 	  limited "password" strings.
 
+config AUTOBOOT_STOP_STR_ENABLE
+	bool "Enable fixed string to stop autobooting"
+	depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION
+	help
+	  This option enables the feature to add a fixed stop
+	  string that is defined at compile time.
+	  In every case it will be tried to load the stop
+	  string from the environment.
+	  In case this is enabled and there is no stop string
+	  in the environment, this will be used as default value.
+
+config AUTOBOOT_STOP_STR_CRYPT
+	string "Stop autobooting via crypt-hashed password"
+	depends on AUTOBOOT_STOP_STR_ENABLE && CRYPT_PW
+	help
+	  This option adds the feature to only stop the autobooting,
+	  and therefore boot into the U-Boot prompt, when the input
+	  string / password matches a values that is hashed via
+	  one of the supported crypt-style password hashing options
+	  and saved in the environment variable "bootstopkeycrypt".
+
 config AUTOBOOT_STOP_STR_SHA256
 	string "Stop autobooting via SHA256 encrypted password"
-	depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION
+	depends on AUTOBOOT_STOP_STR_ENABLE
 	help
 	  This option adds the feature to only stop the autobooting,
 	  and therefore boot into the U-Boot prompt, when the input
diff --git a/common/autoboot.c b/common/autoboot.c
index b42148c729..107fb1533d 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,10 +39,11 @@ 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_AUTOBOOT_USE_MENUKEY
@@ -50,6 +52,63 @@ static int menukey;
 #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 +148,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 +304,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");
 
@@ -394,4 +457,4 @@ void autoboot_command(const char *s)
 		if (s)
 			run_command_list(s, -1, 0);
 	}
-}
+}
\ No newline at end of file
-- 
2.32.0


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

* [PATCH v4 4/8] common: Rename macro appropriately
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (2 preceding siblings ...)
  2021-07-07 23:09 ` [PATCH v4 3/8] common: integrate crypt-based passwords Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  2021-07-08  4:04   ` Heiko Schocher
  2021-07-07 23:09 ` [PATCH v4 5/8] common: allow disabling of timeout for password entry Steffen Jaeckel
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Simon Glass, Da Xue, Heiko Schocher,
	Heinrich Schuchardt, Joel Peshkin, Yuezhang.Mo

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 107fb1533d..832ef7c78c 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
@@ -69,7 +69,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;
@@ -150,9 +150,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;
@@ -179,7 +179,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;
@@ -223,7 +223,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;
@@ -240,8 +240,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.32.0


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

* [PATCH v4 5/8] common: allow disabling of timeout for password entry
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (3 preceding siblings ...)
  2021-07-07 23:09 ` [PATCH v4 4/8] common: Rename macro appropriately Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 6/8] common: add AUTOBOOT_FLUSH_STDIN option Steffen Jaeckel
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Simon Glass, Alexandru Gagniuc, Bin Meng,
	Da Xue, Heiko Schocher, Heinrich Schuchardt, Joel Peshkin,
	Klaus Heinrich Kiwi, Masahisa Kojima, Patrick Delaunay,
	Siew Chin Lim, Yuezhang.Mo

In case a user has to enter a complicated password it is sometimes
desireable to give the user more time than the default timeout.
Enabling this feature will disable the timeout entirely in case the user
presses the <Enter> key before entering any other character.

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

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

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index 410d117cdd..e70dcf17cb 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -859,6 +859,14 @@ config AUTOBOOT_KEYED_CTRLC
 	  Setting this variable	provides an escape sequence from the
 	  limited "password" strings.
 
+config AUTOBOOT_NEVER_TIMEOUT
+	bool "Make the password entry never time-out"
+	depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION && CRYPT_PW
+	help
+	  This option removes the timeout from the password entry
+	  when the user first presses the <Enter> key before entering
+	  any other character.
+
 config AUTOBOOT_STOP_STR_ENABLE
 	bool "Enable fixed string to stop autobooting"
 	depends on AUTOBOOT_KEYED && AUTOBOOT_ENCRYPTION
diff --git a/common/autoboot.c b/common/autoboot.c
index 832ef7c78c..2564ef8a56 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -63,6 +63,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
  */
@@ -72,6 +76,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)
@@ -91,6 +96,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);
@@ -104,7 +114,7 @@ static int passwd_abort_crypt(uint64_t etime)
 				presskey_len++;
 			}
 		}
-	} while (get_ticks() <= etime);
+	} while (never_timeout || get_ticks() <= etime);
 
 	return abort;
 }
-- 
2.32.0


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

* [PATCH v4 6/8] common: add AUTOBOOT_FLUSH_STDIN option
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (4 preceding siblings ...)
  2021-07-07 23:09 ` [PATCH v4 5/8] common: allow disabling of timeout for password entry Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 7/8] common: add support to fallback to plain SHA256 Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 8/8] test: add first autoboot unit tests Steffen Jaeckel
  7 siblings, 0 replies; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Simon Glass, Alexandru Gagniuc, Bin Meng,
	Da Xue, Heiko Schocher, Heinrich Schuchardt, Joel Peshkin,
	Klaus Heinrich Kiwi, Masahisa Kojima, Patrick Delaunay,
	Siew Chin Lim, Yuezhang.Mo

The key-sequence based unlock mechanisms are sensitive to junk symbols
that could have been sent to stdin and are still waiting to be retrieved.
Enabling this option will read all symbols off stdin before displaying the
autoboot prompt (and starting to read the password from stdin).

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 common/Kconfig.boot |  9 +++++++++
 common/autoboot.c   | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index e70dcf17cb..d19bc32836 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -790,6 +790,15 @@ config AUTOBOOT_KEYED
 	  U-Boot automatic booting process and bring the device
 	  to the U-Boot prompt for user input.
 
+config AUTOBOOT_FLUSH_STDIN
+	bool "Enable flushing stdin before starting to read the password"
+	depends on AUTOBOOT_KEYED && !SANDBOX
+	help
+	  When this option is enabled stdin buffer will be flushed before
+	  starting to read the password.
+	  This can't be enabled for the sandbox as flushing stdin would
+	  break the autoboot unit tests.
+
 config AUTOBOOT_PROMPT
 	string "Autoboot stop prompt"
 	depends on AUTOBOOT_KEYED
diff --git a/common/autoboot.c b/common/autoboot.c
index 2564ef8a56..35ef526c42 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -297,6 +297,15 @@ static int passwd_abort_key(uint64_t etime)
 	return abort;
 }
 
+/**
+ * flush_stdin() - drops all pending characters from stdin
+ */
+static void flush_stdin(void)
+{
+	while (tstc())
+		(void)getchar();
+}
+
 /***************************************************************************
  * Watch for 'delay' seconds for autoboot stop or autoboot delay string.
  * returns: 0 -  no key string, allow autoboot 1 - got key string, abort
@@ -306,6 +315,8 @@ static int abortboot_key_sequence(int bootdelay)
 	int abort;
 	uint64_t etime = endtick(bootdelay);
 
+	if (IS_ENABLED(CONFIG_AUTOBOOT_FLUSH_STDIN))
+		flush_stdin();
 #  ifdef CONFIG_AUTOBOOT_PROMPT
 	/*
 	 * CONFIG_AUTOBOOT_PROMPT includes the %d for all boards.
-- 
2.32.0


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

* [PATCH v4 7/8] common: add support to fallback to plain SHA256
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (5 preceding siblings ...)
  2021-07-07 23:09 ` [PATCH v4 6/8] common: add AUTOBOOT_FLUSH_STDIN option Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  2021-07-07 23:09 ` [PATCH v4 8/8] test: add first autoboot unit tests Steffen Jaeckel
  7 siblings, 0 replies; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Simon Glass, Alexandru Gagniuc, Bin Meng,
	Da Xue, Heiko Schocher, Heinrich Schuchardt, Joel Peshkin,
	Klaus Heinrich Kiwi, Patrick Delaunay, Siew Chin Lim,
	Yuezhang.Mo

In case crypt-based hashing is enabled this will be the default mechanism
that is used. If a user wants to have support for both, the environment
variable `bootstopusesha256` can be set to `true` to allow plain SHA256
based hashing of the password.

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v4:
Take review comments into account

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

diff --git a/common/Kconfig.boot b/common/Kconfig.boot
index d19bc32836..764656720a 100644
--- a/common/Kconfig.boot
+++ b/common/Kconfig.boot
@@ -834,6 +834,14 @@ config AUTOBOOT_ENCRYPTION
 	  This provides a way to ship a secure production device which can also
 	  be accessed at the U-Boot command line.
 
+config AUTOBOOT_SHA256_FALLBACK
+	bool "Allow fallback from crypt-hashed password to sha256"
+	depends on AUTOBOOT_ENCRYPTION && CRYPT_PW
+	help
+	  This option adds support to fall back from crypt-hashed
+	  passwords to checking a SHA256 hashed password in case the
+	  'bootstopusesha256' environment variable is set to 'true'.
+
 config AUTOBOOT_DELAY_STR
 	string "Delay autobooting via specific input key / string"
 	depends on AUTOBOOT_KEYED && !AUTOBOOT_ENCRYPTION
diff --git a/common/autoboot.c b/common/autoboot.c
index 35ef526c42..8b9e9aa878 100644
--- a/common/autoboot.c
+++ b/common/autoboot.c
@@ -306,6 +306,26 @@ static void flush_stdin(void)
 		(void)getchar();
 }
 
+/**
+ * fallback_to_sha256() - check whether we should fall back to sha256
+ *                        password checking
+ *
+ * This checks for the environment variable `bootstopusesha256` in case
+ * sha256-fallback has been enabled via the config setting
+ * `AUTOBOOT_SHA256_FALLBACK`.
+ *
+ * @return `false` if we must not fall-back, `true` if plain sha256 should be tried
+ */
+static bool fallback_to_sha256(void)
+{
+	if (IS_ENABLED(CONFIG_AUTOBOOT_SHA256_FALLBACK))
+		return env_get_yesno("bootstopusesha256") == 1;
+	else if (IS_ENABLED(CONFIG_CRYPT_PW))
+		return false;
+	else
+		return true;
+}
+
 /***************************************************************************
  * Watch for 'delay' seconds for autoboot stop or autoboot delay string.
  * returns: 0 -  no key string, allow autoboot 1 - got key string, abort
@@ -326,7 +346,7 @@ static int abortboot_key_sequence(int bootdelay)
 #  endif
 
 	if (IS_ENABLED(CONFIG_AUTOBOOT_ENCRYPTION)) {
-		if (IS_ENABLED(CONFIG_CRYPT_PW))
+		if (IS_ENABLED(CONFIG_CRYPT_PW) && !fallback_to_sha256())
 			abort = passwd_abort_crypt(etime);
 		else
 			abort = passwd_abort_sha256(etime);
-- 
2.32.0


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

* [PATCH v4 8/8] test: add first autoboot unit tests
  2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
                   ` (6 preceding siblings ...)
  2021-07-07 23:09 ` [PATCH v4 7/8] common: add support to fallback to plain SHA256 Steffen Jaeckel
@ 2021-07-07 23:09 ` Steffen Jaeckel
  7 siblings, 0 replies; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-07 23:09 UTC (permalink / raw)
  To: u-boot
  Cc: Steffen Jaeckel, Simon Glass, Alexandru Gagniuc, Andy Shevchenko,
	Bin Meng, Heinrich Schuchardt, Joel Peshkin, Klaus Heinrich Kiwi,
	Patrick Delaunay, Priyanka Jain, Siew Chin Lim

This adds tests for the crypt-based and plain SHA256-based password hashing
algorithms in the autoboot flow.

Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

Changes in v4:
Add another test with `bootstopusesha256` unset

 common/Kconfig.boot         |  2 +-
 common/console.c            |  5 +++
 configs/sandbox_defconfig   | 13 +++++-
 include/console.h           | 17 +++++++
 include/test/common.h       | 15 +++++++
 include/test/suites.h       |  1 +
 test/Makefile               |  1 +
 test/cmd_ut.c               |  1 +
 test/common/Makefile        |  3 ++
 test/common/cmd_ut_common.c | 22 +++++++++
 test/common/test_autoboot.c | 90 +++++++++++++++++++++++++++++++++++++
 11 files changed, 168 insertions(+), 2 deletions(-)
 create mode 100644 include/test/common.h
 create mode 100644 test/common/Makefile
 create mode 100644 test/common/cmd_ut_common.c
 create mode 100644 test/common/test_autoboot.c

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


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

* Re: [PATCH v4 1/8] lib: add crypt subsystem
  2021-07-07 23:09 ` [PATCH v4 1/8] lib: add crypt subsystem Steffen Jaeckel
@ 2021-07-08  3:56   ` Heiko Schocher
  2021-07-08 11:43     ` Steffen Jaeckel
  0 siblings, 1 reply; 15+ messages in thread
From: Heiko Schocher @ 2021-07-08  3:56 UTC (permalink / raw)
  To: Steffen Jaeckel, u-boot
  Cc: Simon Glass, Alexandru Gagniuc, Anastasiia Lukianenko,
	Andrii Anisov, Aswath Govindraju, Bin Meng, Christian Gmeiner,
	Heinrich Schuchardt, Joel Stanley, Marek Szyprowski,
	Masahisa Kojima, Patrick Delaunay, Sean Anderson, Tero Kristo

Hello Steffen,

On 08.07.21 01:09, Steffen Jaeckel wrote:
> Add the basic functionality required to support the standard crypt
> format.
> The files crypt-sha256.c and crypt-sha512.c originate from libxcrypt and
> their formatting is therefor retained.
> The integration is done via a crypt_compare() function in crypt.c.
> 
> ```
> libxcrypt $ git describe --long --always --all
> tags/v4.4.17-0-g6b110bc
> ```
> 
> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v4:
> Fix depends for unit-tests
> 
> Changes in v3:
> Add unit-tests for autoboot
> Introduce `bootstopusesha256` to allow fallback to plain SHA256-based
> hashing
> Add AUTOBOOT_FLUSH_STDIN option
> Drop the changes to bcm963158_ram_defconfig
> 
> Changes in v2:
> Update Kconfig way of enabling, setting hashes etc.
> 
> Changes in v1:
> Added unit-tests of crypt_compare()
> Wrapped crypt functions to encapsulate errno

Reviewed-by: Heiko Schocher <hs@denx.de>

Nitpicks below...

>  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             |  10 ++
>  test/lib/Makefile        |   1 +
>  test/lib/test_crypt.c    |  44 ++++++
>  14 files changed, 884 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/lib/crypt/Makefile b/lib/crypt/Makefile
> new file mode 100644
> index 0000000000..290231064c
> --- /dev/null
> +++ b/lib/crypt/Makefile
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2013, Google Inc.
> +#
> +# (C) Copyright 2000-2007
> +# Wolfgang Denk, DENX Software Engineering, wd@denx.de.

Hmm... this is a new file ... I think you should add
only your Copyright and drop the others... ?

> +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/crypt-sha256.c b/lib/crypt/crypt-sha256.c
> new file mode 100644
> index 0000000000..37127d41e1
> --- /dev/null
> +++ b/lib/crypt/crypt-sha256.c
> @@ -0,0 +1,313 @@
> +/* One way encryption based on the SHA256-based Unix crypt implementation.
> + *
> + * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1].
> + * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018.
> + * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018.
> + * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020.
> + * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2020.
> + * To the extent possible under law, the named authors have waived all
> + * copyright and related or neighboring rights to this work.
> + *
> + * See https://creativecommons.org/publicdomain/zero/1.0/ for further
> + * details.
> + *
> + * This file is a modified except from [2], lines 648 up to 909.
> + *
> + * [1]  https://www.akkadia.org/drepper/sha-crypt.html
> + * [2]  https://www.akkadia.org/drepper/SHA-crypt.txt
> + */

I miss here the SPDX license identifier... also some hint, from which
exact version this code is from ...


> diff --git a/lib/crypt/crypt-sha512.c b/lib/crypt/crypt-sha512.c
> new file mode 100644
> index 0000000000..3616019445
> --- /dev/null
> +++ b/lib/crypt/crypt-sha512.c
> @@ -0,0 +1,328 @@
> +/* One way encryption based on the SHA512-based Unix crypt implementation.
> + *
> + * Written by Ulrich Drepper <drepper at redhat.com> in 2007 [1].
> + * Modified by Zack Weinberg <zackw at panix.com> in 2017, 2018.
> + * Composed by Björn Esser <besser82 at fedoraproject.org> in 2018.
> + * Modified by Björn Esser <besser82 at fedoraproject.org> in 2020.
> + * Modified by Steffen Jaeckel <jaeckel-floss at eyet-services.de> in 2020.
> + * To the extent possible under law, the named authors have waived all
> + * copyright and related or neighboring rights to this work.
> + *
> + * See https://creativecommons.org/publicdomain/zero/1.0/ for further
> + * details.
> + *
> + * This file is a modified except from [2], lines 1403 up to 1676.
> + *
> + * [1]  https://www.akkadia.org/drepper/sha-crypt.html
> + * [2]  https://www.akkadia.org/drepper/SHA-crypt.txt
> + */

same here


Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

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

* Re: [PATCH v4 2/8] lib: wrap crypt API to hide errno usage
  2021-07-07 23:09 ` [PATCH v4 2/8] lib: wrap crypt API to hide errno usage Steffen Jaeckel
@ 2021-07-08  3:58   ` Heiko Schocher
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Schocher @ 2021-07-08  3:58 UTC (permalink / raw)
  To: Steffen Jaeckel, u-boot; +Cc: Simon Glass

Hello Steffen,

On 08.07.21 01:09, Steffen Jaeckel 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>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  include/crypt.h          |  3 ++-
>  lib/crypt/alg-sha256.h   |  6 ------
>  lib/crypt/alg-sha512.h   |  6 ------
>  lib/crypt/crypt-port.h   | 18 ++++++++++--------
>  lib/crypt/crypt-sha256.c | 26 ++++++++++++++++++++++++--
>  lib/crypt/crypt-sha512.c | 26 ++++++++++++++++++++++++--
>  lib/crypt/crypt.c        | 25 ++++++++++++++-----------
>  test/lib/test_crypt.c    | 24 ++++++++++++++++++++++--
>  8 files changed, 96 insertions(+), 38 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

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

* Re: [PATCH v4 3/8] common: integrate crypt-based passwords
  2021-07-07 23:09 ` [PATCH v4 3/8] common: integrate crypt-based passwords Steffen Jaeckel
@ 2021-07-08  4:00   ` Heiko Schocher
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Schocher @ 2021-07-08  4:00 UTC (permalink / raw)
  To: Steffen Jaeckel, u-boot
  Cc: Simon Glass, Alexandru Gagniuc, Bin Meng, Da Xue,
	Heinrich Schuchardt, Joel Peshkin, Klaus Heinrich Kiwi,
	Masahisa Kojima, Siew Chin Lim, Yuezhang.Mo

Hello Steffen,

On 08.07.21 01:09, Steffen Jaeckel wrote:
> Hook into the autoboot flow as an alternative to the existing
> mechanisms.
> 
> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  common/Kconfig.boot | 38 ++++++++++++++++++---
>  common/autoboot.c   | 81 ++++++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 105 insertions(+), 14 deletions(-)

Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

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

* Re: [PATCH v4 4/8] common: Rename macro appropriately
  2021-07-07 23:09 ` [PATCH v4 4/8] common: Rename macro appropriately Steffen Jaeckel
@ 2021-07-08  4:04   ` Heiko Schocher
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Schocher @ 2021-07-08  4:04 UTC (permalink / raw)
  To: Steffen Jaeckel, u-boot
  Cc: Simon Glass, Da Xue, Heinrich Schuchardt, Joel Peshkin, Yuezhang.Mo

Hello Steffen,

On 08.07.21 01:09, Steffen Jaeckel wrote:
> While doing code-review internally this got nitpicked by 2 reviewers, so
> I decided to include this here.
> 
> Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  common/autoboot.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)


Reviewed-by: Heiko Schocher <hs@denx.de>

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

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

* Re: [PATCH v4 1/8] lib: add crypt subsystem
  2021-07-08  3:56   ` Heiko Schocher
@ 2021-07-08 11:43     ` Steffen Jaeckel
  2021-07-08 11:50       ` Heiko Schocher
  0 siblings, 1 reply; 15+ messages in thread
From: Steffen Jaeckel @ 2021-07-08 11:43 UTC (permalink / raw)
  To: hs, u-boot
  Cc: Simon Glass, Alexandru Gagniuc, Anastasiia Lukianenko,
	Andrii Anisov, Aswath Govindraju, Bin Meng, Christian Gmeiner,
	Heinrich Schuchardt, Joel Stanley, Marek Szyprowski,
	Masahisa Kojima, Patrick Delaunay, Sean Anderson, Tero Kristo

Hi Heiko,

thanks for the review!

On 7/8/21 5:56 AM, Heiko Schocher wrote:
> [...]
> 
>> diff --git a/lib/crypt/Makefile b/lib/crypt/Makefile
>> new file mode 100644
>> index 0000000000..290231064c
>> --- /dev/null
>> +++ b/lib/crypt/Makefile
>> @@ -0,0 +1,10 @@
>> +# SPDX-License-Identifier: GPL-2.0+
>> +#
>> +# Copyright (c) 2013, Google Inc.
>> +#
>> +# (C) Copyright 2000-2007
>> +# Wolfgang Denk, DENX Software Engineering, wd@denx.de.
> 
> Hmm... this is a new file ... I think you should add
> only your Copyright and drop the others... ?

thx, done


> [...]
> 
> I miss here the SPDX license identifier... also some hint, from which
> exact version this code is from ...

I've added this info in the next commit, this contains the vanilla
version. Is this ok?


Cheers,
Steffen

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

* Re: [PATCH v4 1/8] lib: add crypt subsystem
  2021-07-08 11:43     ` Steffen Jaeckel
@ 2021-07-08 11:50       ` Heiko Schocher
  0 siblings, 0 replies; 15+ messages in thread
From: Heiko Schocher @ 2021-07-08 11:50 UTC (permalink / raw)
  To: Steffen Jaeckel, u-boot
  Cc: Simon Glass, Alexandru Gagniuc, Anastasiia Lukianenko,
	Andrii Anisov, Aswath Govindraju, Bin Meng, Christian Gmeiner,
	Heinrich Schuchardt, Joel Stanley, Marek Szyprowski,
	Masahisa Kojima, Patrick Delaunay, Sean Anderson, Tero Kristo

Hello Steffen,

On 08.07.21 13:43, Steffen Jaeckel wrote:
> Hi Heiko,
> 
> thanks for the review!
> 
> On 7/8/21 5:56 AM, Heiko Schocher wrote:
>> [...]
>>
>>> diff --git a/lib/crypt/Makefile b/lib/crypt/Makefile
>>> new file mode 100644
>>> index 0000000000..290231064c
>>> --- /dev/null
>>> +++ b/lib/crypt/Makefile
>>> @@ -0,0 +1,10 @@
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +#
>>> +# Copyright (c) 2013, Google Inc.
>>> +#
>>> +# (C) Copyright 2000-2007
>>> +# Wolfgang Denk, DENX Software Engineering, wd@denx.de.
>>
>> Hmm... this is a new file ... I think you should add
>> only your Copyright and drop the others... ?
> 
> thx, done
> 
> 
>> [...]
>>
>> I miss here the SPDX license identifier... also some hint, from which
>> exact version this code is from ...
> 
> I've added this info in the next commit, this contains the vanilla
> version. Is this ok?

Yes from my side, thanks!

bye,
Heiko
> 
> 
> Cheers,
> Steffen
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de

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

end of thread, other threads:[~2021-07-08 11:50 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 23:09 [PATCH v4 0/8] common: Introduce crypt-style password support Steffen Jaeckel
2021-07-07 23:09 ` [PATCH v4 1/8] lib: add crypt subsystem Steffen Jaeckel
2021-07-08  3:56   ` Heiko Schocher
2021-07-08 11:43     ` Steffen Jaeckel
2021-07-08 11:50       ` Heiko Schocher
2021-07-07 23:09 ` [PATCH v4 2/8] lib: wrap crypt API to hide errno usage Steffen Jaeckel
2021-07-08  3:58   ` Heiko Schocher
2021-07-07 23:09 ` [PATCH v4 3/8] common: integrate crypt-based passwords Steffen Jaeckel
2021-07-08  4:00   ` Heiko Schocher
2021-07-07 23:09 ` [PATCH v4 4/8] common: Rename macro appropriately Steffen Jaeckel
2021-07-08  4:04   ` Heiko Schocher
2021-07-07 23:09 ` [PATCH v4 5/8] common: allow disabling of timeout for password entry Steffen Jaeckel
2021-07-07 23:09 ` [PATCH v4 6/8] common: add AUTOBOOT_FLUSH_STDIN option Steffen Jaeckel
2021-07-07 23:09 ` [PATCH v4 7/8] common: add support to fallback to plain SHA256 Steffen Jaeckel
2021-07-07 23:09 ` [PATCH v4 8/8] test: add first autoboot unit tests 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.