From 0e98d17e77a024a16fc15589b2b21bb6196d4567 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 8 Sep 2020 14:52:13 +0200
Subject: [PATCH 01/12] Add a helper function that does make_salt+crypt_r

No functional change.
---
 src/firstboot/firstboot.c       | 17 +++++------------
 src/home/homectl-fido2.c        | 16 ++++------------
 src/home/homectl-pkcs11.c       | 16 ++++------------
 src/home/homectl-recovery-key.c | 13 +++----------
 src/home/homework.c             |  1 +
 src/home/user-record-util.c     | 13 +++----------
 src/shared/libcrypt-util.c      | 25 +++++++++++++++++++++++++
 src/shared/libcrypt-util.h      |  2 +-
 8 files changed, 46 insertions(+), 57 deletions(-)

diff --git a/src/firstboot/firstboot.c b/src/firstboot/firstboot.c
index f8499a6ffd..3109f9cdfc 100644
--- a/src/firstboot/firstboot.c
+++ b/src/firstboot/firstboot.c
@@ -802,7 +802,7 @@ static int write_root_shadow(const char *shadow_path, const char *hashed_passwor
 
 static int process_root_args(void) {
         _cleanup_close_ int lock = -1;
-        struct crypt_data cd = {};
+        _cleanup_(erase_and_freep) char *_hashed_password = NULL;
         const char *password, *hashed_password;
         const char *etc_passwd, *etc_shadow;
         int r;
@@ -866,20 +866,13 @@ static int process_root_args(void) {
                 password = "x";
                 hashed_password = arg_root_password;
         } else if (arg_root_password) {
-                _cleanup_free_ char *salt = NULL;
-                /* hashed_password points inside cd after crypt_r returns so cd has function scope. */
+                r = hash_password(arg_root_password, &_hashed_password);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to hash password: %m");
 
                 password = "x";
+                hashed_password = _hashed_password;
 
-                r = make_salt(&salt);
-                if (r < 0)
-                        return log_error_errno(r, "Failed to get salt: %m");
-
-                errno = 0;
-                hashed_password = crypt_r(arg_root_password, salt, &cd);
-                if (!hashed_password)
-                        return log_error_errno(errno == 0 ? SYNTHETIC_ERRNO(EINVAL) : errno,
-                                        "Failed to encrypt password: %m");
         } else if (arg_delete_root_password)
                 password = hashed_password = "";
         else
diff --git a/src/home/homectl-fido2.c b/src/home/homectl-fido2.c
index b7b2c1a3b5..b9092df18c 100644
--- a/src/home/homectl-fido2.c
+++ b/src/home/homectl-fido2.c
@@ -70,31 +70,23 @@ static int add_fido2_salt(
                 size_t secret_size) {
 
         _cleanup_(json_variant_unrefp) JsonVariant *l = NULL, *w = NULL, *e = NULL;
-        _cleanup_(erase_and_freep) char *base64_encoded = NULL;
-        _cleanup_free_ char *unix_salt = NULL;
-        struct crypt_data cd = {};
-        char *k;
+        _cleanup_(erase_and_freep) char *base64_encoded = NULL, *hashed = NULL;
         int r;
 
-        r = make_salt(&unix_salt);
-        if (r < 0)
-                return log_error_errno(r, "Failed to generate salt: %m");
-
         /* Before using UNIX hashing on the supplied key we base64 encode it, since crypt_r() and friends
          * expect a NUL terminated string, and we use a binary key */
         r = base64mem(secret, secret_size, &base64_encoded);
         if (r < 0)
                 return log_error_errno(r, "Failed to base64 encode secret key: %m");
 
-        errno = 0;
-        k = crypt_r(base64_encoded, unix_salt, &cd);
-        if (!k)
+        r = hash_password(base64_encoded, &hashed);
+        if (r < 0)
                 return log_error_errno(errno_or_else(EINVAL), "Failed to UNIX hash secret key: %m");
 
         r = json_build(&e, JSON_BUILD_OBJECT(
                                        JSON_BUILD_PAIR("credential", JSON_BUILD_BASE64(cid, cid_size)),
                                        JSON_BUILD_PAIR("salt", JSON_BUILD_BASE64(fido2_salt, fido2_salt_size)),
-                                       JSON_BUILD_PAIR("hashedPassword", JSON_BUILD_STRING(k))));
+                                       JSON_BUILD_PAIR("hashedPassword", JSON_BUILD_STRING(hashed))));
         if (r < 0)
                 return log_error_errno(r, "Failed to build FIDO2 salt JSON key object: %m");
 
diff --git a/src/home/homectl-pkcs11.c b/src/home/homectl-pkcs11.c
index f4253ed7bf..21c9b9a6a3 100644
--- a/src/home/homectl-pkcs11.c
+++ b/src/home/homectl-pkcs11.c
@@ -134,10 +134,7 @@ static int add_pkcs11_encrypted_key(
                 const void *decrypted_key, size_t decrypted_key_size) {
 
         _cleanup_(json_variant_unrefp) JsonVariant *l = NULL, *w = NULL, *e = NULL;
-        _cleanup_(erase_and_freep) char *base64_encoded = NULL;
-        _cleanup_free_ char *salt = NULL;
-        struct crypt_data cd = {};
-        char *k;
+        _cleanup_(erase_and_freep) char *base64_encoded = NULL, *hashed = NULL;
         int r;
 
         assert(v);
@@ -147,25 +144,20 @@ static int add_pkcs11_encrypted_key(
         assert(decrypted_key);
         assert(decrypted_key_size > 0);
 
-        r = make_salt(&salt);
-        if (r < 0)
-                return log_error_errno(r, "Failed to generate salt: %m");
-
         /* Before using UNIX hashing on the supplied key we base64 encode it, since crypt_r() and friends
          * expect a NUL terminated string, and we use a binary key */
         r = base64mem(decrypted_key, decrypted_key_size, &base64_encoded);
         if (r < 0)
                 return log_error_errno(r, "Failed to base64 encode secret key: %m");
 
-        errno = 0;
-        k = crypt_r(base64_encoded, salt, &cd);
-        if (!k)
+        r = hash_password(base64_encoded, &hashed);
+        if (r < 0)
                 return log_error_errno(errno_or_else(EINVAL), "Failed to UNIX hash secret key: %m");
 
         r = json_build(&e, JSON_BUILD_OBJECT(
                                        JSON_BUILD_PAIR("uri", JSON_BUILD_STRING(uri)),
                                        JSON_BUILD_PAIR("data", JSON_BUILD_BASE64(encrypted_key, encrypted_key_size)),
-                                       JSON_BUILD_PAIR("hashedPassword", JSON_BUILD_STRING(k))));
+                                       JSON_BUILD_PAIR("hashedPassword", JSON_BUILD_STRING(hashed))));
         if (r < 0)
                 return log_error_errno(r, "Failed to build encrypted JSON key object: %m");
 
diff --git a/src/home/homectl-recovery-key.c b/src/home/homectl-recovery-key.c
index 9d7f345f1e..c63d3415f4 100644
--- a/src/home/homectl-recovery-key.c
+++ b/src/home/homectl-recovery-key.c
@@ -183,9 +183,7 @@ static int print_qr_code(const char *secret) {
 }
 
 int identity_add_recovery_key(JsonVariant **v) {
-        _cleanup_(erase_and_freep) char *unix_salt = NULL, *password = NULL;
-        struct crypt_data cd = {};
-        char *k;
+        _cleanup_(erase_and_freep) char *password = NULL, *hashed = NULL;
         int r;
 
         assert(v);
@@ -196,17 +194,12 @@ int identity_add_recovery_key(JsonVariant **v) {
                 return r;
 
         /* Let's UNIX hash it */
-        r = make_salt(&unix_salt);
+        r = hash_password(password, &hashed);
         if (r < 0)
-                return log_error_errno(r, "Failed to generate salt: %m");
-
-        errno = 0;
-        k = crypt_r(password, unix_salt, &cd);
-        if (!k)
                 return log_error_errno(errno_or_else(EINVAL), "Failed to UNIX hash secret key: %m");
 
         /* Let's now add the "privileged" version of the recovery key */
-        r = add_privileged(v, k);
+        r = add_privileged(v, hashed);
         if (r < 0)
                 return r;
 
diff --git a/src/home/homework.c b/src/home/homework.c
index 594c4a05bb..986ce2b3f0 100644
--- a/src/home/homework.c
+++ b/src/home/homework.c
@@ -17,6 +17,7 @@
 #include "homework-mount.h"
 #include "homework-pkcs11.h"
 #include "homework.h"
+#include "libcrypt-util.h"
 #include "main-func.h"
 #include "memory-util.h"
 #include "missing_magic.h"
diff --git a/src/home/user-record-util.c b/src/home/user-record-util.c
index 0bbe44ce26..6928427730 100644
--- a/src/home/user-record-util.c
+++ b/src/home/user-record-util.c
@@ -806,20 +806,13 @@ int user_record_make_hashed_password(UserRecord *h, char **secret, bool extend)
         }
 
         STRV_FOREACH(i, secret) {
-                _cleanup_free_ char *salt = NULL;
-                struct crypt_data cd = {};
-                char *k;
+                _cleanup_(erase_and_freep) char *hashed = NULL;
 
-                r = make_salt(&salt);
+                r = hash_password(*i, &hashed);
                 if (r < 0)
                         return r;
 
-                errno = 0;
-                k = crypt_r(*i, salt, &cd);
-                if (!k)
-                        return errno_or_else(EINVAL);
-
-                r = strv_extend(&np, k);
+                r = strv_consume(&np, TAKE_PTR(hashed));
                 if (r < 0)
                         return r;
         }
diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index bf6605508a..d19bcf1d8a 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -4,6 +4,7 @@
 #include <stdlib.h>
 
 #include "alloc-util.h"
+#include "errno-util.h"
 #include "libcrypt-util.h"
 #include "log.h"
 #include "macro.h"
@@ -74,6 +75,30 @@ int make_salt(char **ret) {
 #endif
 }
 
+int hash_password(const char *password, char **ret) {
+        _cleanup_free_ char *salt = NULL;
+        char *p;
+        struct crypt_data cd = {};
+        int r;
+
+        r = make_salt(&salt);
+        if (r < 0)
+                return log_debug_errno(r, "Failed to generate salt: %m");
+
+        errno = 0;
+        p = crypt_r(password, salt, &cd);
+        if (!p)
+                return log_debug_errno(errno_or_else(SYNTHETIC_ERRNO(EINVAL)),
+                                       "crypt_r() failed: %m");
+
+        p = strdup(p);
+        if (!p)
+                return -ENOMEM;
+
+        *ret = p;
+        return 0;
+}
+
 bool looks_like_hashed_password(const char *s) {
         /* Returns false if the specified string is certainly not a hashed UNIX password. crypt(5) lists
          * various hashing methods. We only reject (return false) strings which are documented to have
diff --git a/src/shared/libcrypt-util.h b/src/shared/libcrypt-util.h
index 8a860ceb0d..b10be2f7d2 100644
--- a/src/shared/libcrypt-util.h
+++ b/src/shared/libcrypt-util.h
@@ -18,5 +18,5 @@
 #include <stdlib.h>
 
 int make_salt(char **ret);
-
+int hash_password(const char *password, char **ret);
 bool looks_like_hashed_password(const char *s);

From a937ce2d8545568c1de3ba0370fd621ffcbdd8ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 8 Sep 2020 15:13:44 +0200
Subject: [PATCH 02/12] shared/libcrypt-util: use libcrypt_ra()

This lets the libc/xcrypt allocate as much storage area as it needs.
Should fix #16965:

testsuite-46.sh[74]: ==74==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7f3e972e1080 at pc 0x7f3e9be8deed bp 0x7ffce4f28530 sp 0x7ffce4f27ce0
testsuite-46.sh[74]: WRITE of size 131232 at 0x7f3e972e1080 thread T0
testsuite-46.sh[74]:     #0 0x7f3e9be8deec  (/usr/lib/clang/10.0.1/lib/linux/libclang_rt.asan-x86_64.so+0x9feec)
testsuite-46.sh[74]:     #1 0x559cd05a6412 in user_record_make_hashed_password /systemd-meson-build/../build/src/home/user-record-util.c:818:21
testsuite-46.sh[74]:     #2 0x559cd058fb03 in create_home /systemd-meson-build/../build/src/home/homectl.c:1112:29
testsuite-46.sh[74]:     #3 0x7f3e9b5b3058 in dispatch_verb /systemd-meson-build/../build/src/shared/verbs.c:103:24
testsuite-46.sh[74]:     #4 0x559cd058c101 in run /systemd-meson-build/../build/src/home/homectl.c:3325:16
testsuite-46.sh[74]:     #5 0x559cd058c00a in main /systemd-meson-build/../build/src/home/homectl.c:3328:1
testsuite-46.sh[74]:     #6 0x7f3e9a88b151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
testsuite-46.sh[74]:     #7 0x559cd0583e7d in _start (/usr/bin/homectl+0x24e7d)
testsuite-46.sh[74]: Address 0x7f3e972e1080 is located in stack of thread T0 at offset 32896 in frame
testsuite-46.sh[74]:     #0 0x559cd05a60df in user_record_make_hashed_password /systemd-meson-build/../build/src/home/user-record-util.c:789
testsuite-46.sh[74]:   This frame has 6 object(s):
testsuite-46.sh[74]:     [32, 40) 'priv' (line 790)
testsuite-46.sh[74]:     [64, 72) 'np' (line 791)
testsuite-46.sh[74]:     [96, 104) 'salt' (line 809)
testsuite-46.sh[74]:     [128, 32896) 'cd' (line 810)
testsuite-46.sh[74]:     [33152, 33168) '.compoundliteral' <== Memory access at offset 32896 partially underflows this variable
testsuite-46.sh[74]:     [33184, 33192) 'new_array' (line 832) <== Memory access at offset 32896 partially underflows this variable
testsuite-46.sh[74]: HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
testsuite-46.sh[74]:       (longjmp and C++ exceptions *are* supported)
testsuite-46.sh[74]: SUMMARY: AddressSanitizer: stack-buffer-overflow (/usr/lib/clang/10.0.1/lib/linux/libclang_rt.asan-x86_64.so+0x9feec)

It seems 'struct crypt_data' is 32896 bytes, but libclang_rt wants more, at least 33168?
---
 src/shared/libcrypt-util.c    | 13 ++++++++-----
 src/shared/libcrypt-util.h    |  5 ++++-
 src/test/meson.build          |  4 ++++
 src/test/test-libcrypt-util.c | 35 +++++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 6 deletions(-)
 create mode 100644 src/test/test-libcrypt-util.c

diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index d19bcf1d8a..cb12cd65f3 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -8,6 +8,7 @@
 #include "libcrypt-util.h"
 #include "log.h"
 #include "macro.h"
+#include "memory-util.h"
 #include "missing_stdlib.h"
 #include "random-util.h"
 #include "string-util.h"
@@ -75,21 +76,23 @@ int make_salt(char **ret) {
 #endif
 }
 
-int hash_password(const char *password, char **ret) {
+int hash_password_full(const char *password, void **cd_data, int *cd_size, char **ret) {
         _cleanup_free_ char *salt = NULL;
+        _cleanup_(erase_and_freep) void *_cd_data = NULL;
         char *p;
-        struct crypt_data cd = {};
-        int r;
+        int r, _cd_size = 0;
+
+        assert(!!cd_data == !!cd_size);
 
         r = make_salt(&salt);
         if (r < 0)
                 return log_debug_errno(r, "Failed to generate salt: %m");
 
         errno = 0;
-        p = crypt_r(password, salt, &cd);
+        p = crypt_ra(password, salt, cd_data ?: &_cd_data, cd_size ?: &_cd_size);
         if (!p)
                 return log_debug_errno(errno_or_else(SYNTHETIC_ERRNO(EINVAL)),
-                                       "crypt_r() failed: %m");
+                                       "crypt_ra() failed: %m");
 
         p = strdup(p);
         if (!p)
diff --git a/src/shared/libcrypt-util.h b/src/shared/libcrypt-util.h
index b10be2f7d2..2f8c352ab3 100644
--- a/src/shared/libcrypt-util.h
+++ b/src/shared/libcrypt-util.h
@@ -18,5 +18,8 @@
 #include <stdlib.h>
 
 int make_salt(char **ret);
-int hash_password(const char *password, char **ret);
+int hash_password_full(const char *password, void **cd_data, int *cd_size, char **ret);
+static inline int hash_password(const char *password, char **ret) {
+        return hash_password_full(password, NULL, NULL, ret);
+}
 bool looks_like_hashed_password(const char *s);
diff --git a/src/test/meson.build b/src/test/meson.build
index 70450ea1c5..3e84e5a0d3 100644
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -301,6 +301,10 @@ tests += [
          [],
          []],
 
+        [['src/test/test-libcrypt-util.c'],
+         [],
+         []],
+
         [['src/test/test-offline-passwd.c',
           'src/shared/offline-passwd.c',
           'src/shared/offline-passwd.h'],
diff --git a/src/test/test-libcrypt-util.c b/src/test/test-libcrypt-util.c
new file mode 100644
index 0000000000..94bd4172ea
--- /dev/null
+++ b/src/test/test-libcrypt-util.c
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: LGPL-2.1+ */
+
+#include "strv.h"
+#include "tests.h"
+#include "libcrypt-util.h"
+
+static void test_hash_password_full(void) {
+        log_info("/* %s */", __func__);
+
+        _cleanup_free_ void *cd_data = NULL;
+        const char *i;
+        int cd_size = 0;
+
+        log_info("sizeof(struct crypt_data): %zu bytes", sizeof(struct crypt_data));
+
+        for (unsigned c = 0; c < 2; c++)
+                FOREACH_STRING(i, "abc123", "password", "s3cr3t") {
+                        _cleanup_free_ char *hashed;
+
+                        if (c == 0)
+                                assert_se(hash_password_full(i, &cd_data, &cd_size, &hashed) == 0);
+                        else
+                                assert_se(hash_password_full(i, NULL, NULL, &hashed) == 0);
+                        log_debug("\"%s\" → \"%s\"", i, hashed);
+                        log_info("crypt_r[a] buffer size: %i bytes", cd_size);
+                }
+}
+
+int main(int argc, char *argv[]) {
+        test_setup_logging(LOG_DEBUG);
+
+        test_hash_password_full();
+
+        return 0;
+}

From 2ae297fe0d8b0736ee16b3be47ef62e17b4afe4e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 8 Sep 2020 15:21:21 +0200
Subject: [PATCH 03/12] Move test_password_{one,many} to libcrypt-util.c

They are only used under src/home/, but I want to add tests in test-libcrypt-util.c.
And the functions are almost trivial, so I think it is OK to move them to shared.
---
 src/home/home-util.c             | 33 --------------------------------
 src/home/home-util.h             |  3 ---
 src/home/user-record-pwquality.c |  1 +
 src/shared/libcrypt-util.c       | 32 +++++++++++++++++++++++++++++++
 src/shared/libcrypt-util.h       |  2 ++
 5 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/src/home/home-util.c b/src/home/home-util.c
index 3fd57639f8..8e28e3ab76 100644
--- a/src/home/home-util.c
+++ b/src/home/home-util.c
@@ -1,7 +1,6 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
 #include "dns-domain.h"
-#include "errno-util.h"
 #include "home-util.h"
 #include "libcrypt-util.h"
 #include "memory-util.h"
@@ -134,35 +133,3 @@ int bus_message_append_secret(sd_bus_message *m, UserRecord *secret) {
 
         return sd_bus_message_append(m, "s", formatted);
 }
-
-int test_password_one(const char *hashed_password, const char *password) {
-        struct crypt_data cc = {};
-        const char *k;
-        bool b;
-
-        errno = 0;
-        k = crypt_r(password, hashed_password, &cc);
-        if (!k) {
-                explicit_bzero_safe(&cc, sizeof(cc));
-                return errno_or_else(EINVAL);
-        }
-
-        b = streq(k, hashed_password);
-        explicit_bzero_safe(&cc, sizeof(cc));
-        return b;
-}
-
-int test_password_many(char **hashed_password, const char *password) {
-        char **hpw;
-        int r;
-
-        STRV_FOREACH(hpw, hashed_password) {
-                r = test_password_one(*hpw, password);
-                if (r < 0)
-                        return r;
-                if (r > 0)
-                        return true;
-        }
-
-        return false;
-}
diff --git a/src/home/home-util.h b/src/home/home-util.h
index 6161d4c3d0..73602e4f8e 100644
--- a/src/home/home-util.h
+++ b/src/home/home-util.h
@@ -21,6 +21,3 @@ int bus_message_append_secret(sd_bus_message *m, UserRecord *secret);
 /* Many of our operations might be slow due to crypto, fsck, recursive chown() and so on. For these
  * operations permit a *very* long timeout */
 #define HOME_SLOW_BUS_CALL_TIMEOUT_USEC (2*USEC_PER_MINUTE)
-
-int test_password_one(const char *hashed_password, const char *password);
-int test_password_many(char **hashed_password, const char *password);
diff --git a/src/home/user-record-pwquality.c b/src/home/user-record-pwquality.c
index a5d632c772..08d7dc0169 100644
--- a/src/home/user-record-pwquality.c
+++ b/src/home/user-record-pwquality.c
@@ -3,6 +3,7 @@
 #include "bus-common-errors.h"
 #include "errno-util.h"
 #include "home-util.h"
+#include "libcrypt-util.h"
 #include "pwquality-util.h"
 #include "strv.h"
 #include "user-record-pwquality.h"
diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index cb12cd65f3..1faf683d71 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -117,3 +117,35 @@ bool looks_like_hashed_password(const char *s) {
 
         return !STR_IN_SET(s, "x", "*");
 }
+
+int test_password_one(const char *hashed_password, const char *password) {
+        struct crypt_data cc = {};
+        const char *k;
+        bool b;
+
+        errno = 0;
+        k = crypt_r(password, hashed_password, &cc);
+        if (!k) {
+                explicit_bzero_safe(&cc, sizeof(cc));
+                return errno_or_else(EINVAL);
+        }
+
+        b = streq(k, hashed_password);
+        explicit_bzero_safe(&cc, sizeof(cc));
+        return b;
+}
+
+int test_password_many(char **hashed_password, const char *password) {
+        char **hpw;
+        int r;
+
+        STRV_FOREACH(hpw, hashed_password) {
+                r = test_password_one(*hpw, password);
+                if (r < 0)
+                        return r;
+                if (r > 0)
+                        return true;
+        }
+
+        return false;
+}
diff --git a/src/shared/libcrypt-util.h b/src/shared/libcrypt-util.h
index 2f8c352ab3..5be4e64a52 100644
--- a/src/shared/libcrypt-util.h
+++ b/src/shared/libcrypt-util.h
@@ -23,3 +23,5 @@ static inline int hash_password(const char *password, char **ret) {
         return hash_password_full(password, NULL, NULL, ret);
 }
 bool looks_like_hashed_password(const char *s);
+int test_password_one(const char *hashed_password, const char *password);
+int test_password_many(char **hashed_password, const char *password);

From 6016b4b0ba6a5d47533b47f7f944116425594f41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 8 Sep 2020 15:28:21 +0200
Subject: [PATCH 04/12] Add reciprocal test for password hashing and checking

---
 src/test/test-libcrypt-util.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/test/test-libcrypt-util.c b/src/test/test-libcrypt-util.c
index 94bd4172ea..5c4958ed72 100644
--- a/src/test/test-libcrypt-util.c
+++ b/src/test/test-libcrypt-util.c
@@ -14,7 +14,7 @@ static void test_hash_password_full(void) {
         log_info("sizeof(struct crypt_data): %zu bytes", sizeof(struct crypt_data));
 
         for (unsigned c = 0; c < 2; c++)
-                FOREACH_STRING(i, "abc123", "password", "s3cr3t") {
+                FOREACH_STRING(i, "abc123", "h⸿sło") {
                         _cleanup_free_ char *hashed;
 
                         if (c == 0)
@@ -23,6 +23,21 @@ static void test_hash_password_full(void) {
                                 assert_se(hash_password_full(i, NULL, NULL, &hashed) == 0);
                         log_debug("\"%s\" → \"%s\"", i, hashed);
                         log_info("crypt_r[a] buffer size: %i bytes", cd_size);
+
+                        assert_se(test_password_one(hashed, i) == true);
+                        assert_se(test_password_one(i, hashed) <= 0); /* We get an error for non-utf8 */
+                        assert_se(test_password_one(hashed, "foobar") == false);
+                        assert_se(test_password_many(STRV_MAKE(hashed), i) == true);
+                        assert_se(test_password_many(STRV_MAKE(hashed), "foobar") == false);
+                        assert_se(test_password_many(STRV_MAKE(hashed, hashed, hashed), "foobar") == false);
+                        assert_se(test_password_many(STRV_MAKE("$y$j9T$dlCXwkX0GC5L6B8Gf.4PN/$VCyEH",
+                                                               hashed,
+                                                               "$y$j9T$SAayASazWZIQeJd9AS02m/$"),
+                                                     i) == true);
+                        assert_se(test_password_many(STRV_MAKE("$y$j9T$dlCXwkX0GC5L6B8Gf.4PN/$VCyEH",
+                                                               hashed,
+                                                               "$y$j9T$SAayASazWZIQeJd9AS02m/$"),
+                                                     "") == false);
                 }
 }
 

From 999b49c818f5ae28d24fe7a551542e945130dcb3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 8 Sep 2020 16:27:47 +0200
Subject: [PATCH 05/12] Make test_password_{one,many} also use crypt_ra()

---
 src/shared/libcrypt-util.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index 1faf683d71..a2f5f06bcc 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -119,20 +119,16 @@ bool looks_like_hashed_password(const char *s) {
 }
 
 int test_password_one(const char *hashed_password, const char *password) {
-        struct crypt_data cc = {};
+        _cleanup_(erase_and_freep) void *cd_data = NULL;
+        int cd_size = 0;
         const char *k;
-        bool b;
 
         errno = 0;
-        k = crypt_r(password, hashed_password, &cc);
-        if (!k) {
-                explicit_bzero_safe(&cc, sizeof(cc));
+        k = crypt_ra(password, hashed_password, &cd_data, &cd_size);
+        if (!k)
                 return errno_or_else(EINVAL);
-        }
 
-        b = streq(k, hashed_password);
-        explicit_bzero_safe(&cc, sizeof(cc));
-        return b;
+        return streq(k, hashed_password);
 }
 
 int test_password_many(char **hashed_password, const char *password) {

From 83764f8d002afaef1492909bc4b00821eacab38f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 8 Sep 2020 18:26:24 +0200
Subject: [PATCH 06/12] shared/libcrypt-util: include fewer headers

Now that we wrap crypt_r/ra uses, we can include the header only in libcrypt-util.c.
---
 src/shared/libcrypt-util.c    | 15 +++++++++++++++
 src/shared/libcrypt-util.h    | 14 --------------
 src/test/test-libcrypt-util.c |  6 ++++++
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index a2f5f06bcc..ca40c02c4e 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -1,5 +1,20 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
+#if HAVE_CRYPT_H
+/* libxcrypt is a replacement for glibc's libcrypt, and libcrypt might be
+ * removed from glibc at some point. As part of the removal, defines for
+ * crypt(3) are dropped from unistd.h, and we must include crypt.h instead.
+ *
+ * Newer versions of glibc (v2.0+) already ship crypt.h with a definition
+ * of crypt(3) as well, so we simply include it if it is present.  MariaDB,
+ * MySQL, PostgreSQL, Perl and some other wide-spread packages do it the
+ * same way since ages without any problems.
+ */
+#  include <crypt.h>
+#else
+#  include <unistd.h>
+#endif
+
 #include <errno.h>
 #include <stdlib.h>
 
diff --git a/src/shared/libcrypt-util.h b/src/shared/libcrypt-util.h
index 5be4e64a52..924a35d3e1 100644
--- a/src/shared/libcrypt-util.h
+++ b/src/shared/libcrypt-util.h
@@ -1,21 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 #pragma once
 
-#if HAVE_CRYPT_H
-/* libxcrypt is a replacement for glibc's libcrypt, and libcrypt might be
- * removed from glibc at some point. As part of the removal, defines for
- * crypt(3) are dropped from unistd.h, and we must include crypt.h instead.
- *
- * Newer versions of glibc (v2.0+) already ship crypt.h with a definition
- * of crypt(3) as well, so we simply include it if it is present.  MariaDB,
- * MySQL, PostgreSQL, Perl and some other wide-spread packages do it the
- * same way since ages without any problems.
- */
-#include <crypt.h>
-#endif
-
 #include <stdbool.h>
-#include <stdlib.h>
 
 int make_salt(char **ret);
 int hash_password_full(const char *password, void **cd_data, int *cd_size, char **ret);
diff --git a/src/test/test-libcrypt-util.c b/src/test/test-libcrypt-util.c
index 5c4958ed72..90abfa7b81 100644
--- a/src/test/test-libcrypt-util.c
+++ b/src/test/test-libcrypt-util.c
@@ -1,5 +1,11 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
+#if HAVE_CRYPT_H
+#  include <crypt.h>
+#else
+#  include <unistd.h>
+#endif
+
 #include "strv.h"
 #include "tests.h"
 #include "libcrypt-util.h"

From e8644a414a9953a4f7a3b966519dee6fcf66eff0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 8 Sep 2020 20:02:31 +0200
Subject: [PATCH 07/12] meson: test if we have libcrypt_ra

We always seem to have either libcrypt_r and not the other two, or all
three. So the fallback for libcrypt_ra needs to be based on libcrypt_r.
---
 meson.build | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/meson.build b/meson.build
index ab0d7da1e9..203aa63c1a 100644
--- a/meson.build
+++ b/meson.build
@@ -881,6 +881,16 @@ libm = cc.find_library('m')
 libdl = cc.find_library('dl')
 libcrypt = cc.find_library('crypt')
 
+crypt_header = conf.get('HAVE_CRYPT_H') == 1 ? \
+               '''#include <crypt.h>''' : '''#include <unistd.h>'''
+foreach ident : [
+        ['crypt_ra',          crypt_header]]
+
+        have = cc.has_function(ident[0], prefix : ident[1], args : '-D_GNU_SOURCE',
+                               dependencies : libcrypt)
+        conf.set10('HAVE_' + ident[0].to_upper(), have)
+endforeach
+
 libcap = dependency('libcap', required : false)
 if not libcap.found()
         # Compat with Ubuntu 14.04 which ships libcap w/o .pc file

From 2b49f0ca83279cdf1199df33ee794d0e026a08d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 10 Sep 2020 18:21:41 +0200
Subject: [PATCH 08/12] Check for crypt_gensalt_ra() instead of relying on
 libxcrypt presence

Since the loop to check various xcrypt functions is already in place,
adding one more is cheap. And it is nicer to check for the function
directly. People like to backport things, so we might get lucky even
without having libxcrypt.
---
 meson.build                | 3 ++-
 src/shared/libcrypt-util.c | 9 ++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/meson.build b/meson.build
index 203aa63c1a..f75c9b2cfb 100644
--- a/meson.build
+++ b/meson.build
@@ -884,7 +884,8 @@ libcrypt = cc.find_library('crypt')
 crypt_header = conf.get('HAVE_CRYPT_H') == 1 ? \
                '''#include <crypt.h>''' : '''#include <unistd.h>'''
 foreach ident : [
-        ['crypt_ra',          crypt_header]]
+        ['crypt_ra',          crypt_header],
+        ['crypt_gensalt_ra',  crypt_header]]
 
         have = cc.has_function(ident[0], prefix : ident[1], args : '-D_GNU_SOURCE',
                                dependencies : libcrypt)
diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index ca40c02c4e..de0cfced6d 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -31,12 +31,12 @@
 
 int make_salt(char **ret) {
 
-#ifdef XCRYPT_VERSION_MAJOR
+#if HAVE_CRYPT_GENSALT_RA
         const char *e;
         char *salt;
 
-        /* If we have libxcrypt we default to the "preferred method" (i.e. usually yescrypt), and generate it
-         * with crypt_gensalt_ra(). */
+        /* If we have crypt_gensalt_ra() we default to the "preferred method" (i.e. usually yescrypt).
+         * crypt_gensalt_ra() is usually provided by libxcrypt. */
 
         e = secure_getenv("SYSTEMD_CRYPT_PREFIX");
         if (!e)
@@ -51,8 +51,7 @@ int make_salt(char **ret) {
         *ret = salt;
         return 0;
 #else
-        /* If libxcrypt is not used, we use SHA512 and generate the salt on our own since crypt_gensalt_ra()
-         * is not available. */
+        /* If crypt_gensalt_ra() is not available, we use SHA512 and generate the salt on our own. */
 
         static const char table[] =
                 "abcdefghijklmnopqrstuvwxyz"

From 9de324c3c919f20fd49e1d25579f5a66cac0eaa0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Thu, 10 Sep 2020 18:30:22 +0200
Subject: [PATCH 09/12] shared/libcrypt-util: add fallback for crypt_ra()

Following the style in missing_syscall.h, we use a non-conflicting name
for the function and use a macro to map to the real name to the replacement.
---
 src/shared/libcrypt-util.c | 45 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index de0cfced6d..66f345ee5f 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -69,6 +69,8 @@ int make_salt(char **ret) {
 
         assert_cc(sizeof(table) == 64U + 1U);
 
+        log_debug("Generating fallback salt for hash prefix: $6$");
+
         /* Insist on the best randomness by setting RANDOM_BLOCK, this is about keeping passwords secret after all. */
         r = genuine_random_bytes(raw, sizeof(raw), RANDOM_BLOCK);
         if (r < 0)
@@ -90,6 +92,47 @@ int make_salt(char **ret) {
 #endif
 }
 
+#if HAVE_CRYPT_RA
+#  define CRYPT_RA_NAME "crypt_ra"
+#else
+#  define CRYPT_RA_NAME "crypt_r"
+
+/* Provide a poor man's fallback that uses a fixed size buffer. */
+
+static char* systemd_crypt_ra(const char *phrase, const char *setting, void **data, int *size) {
+        assert(data);
+        assert(size);
+
+        /* We allocate the buffer because crypt(3) says: struct crypt_data may be quite large (32kB in this
+         * implementation of libcrypt; over 128kB in some other implementations). This is large enough that
+         * it may be unwise to allocate it on the stack. */
+
+        if (!*data) {
+                *data = new0(struct crypt_data, 1);
+                if (!*data) {
+                        errno = -ENOMEM;
+                        return NULL;
+                }
+
+                *size = (int) (sizeof(struct crypt_data));
+        }
+
+        char *t = crypt_r(phrase, setting, *data);
+        if (!t)
+                return NULL;
+
+        /* crypt_r may return a pointer to an invalid hashed password on error. Our callers expect NULL on
+         * error, so let's just return that. */
+        if (t[0] == '*')
+                return NULL;
+
+        return t;
+}
+
+#define crypt_ra systemd_crypt_ra
+
+#endif
+
 int hash_password_full(const char *password, void **cd_data, int *cd_size, char **ret) {
         _cleanup_free_ char *salt = NULL;
         _cleanup_(erase_and_freep) void *_cd_data = NULL;
@@ -106,7 +149,7 @@ int hash_password_full(const char *password, void **cd_data, int *cd_size, char
         p = crypt_ra(password, salt, cd_data ?: &_cd_data, cd_size ?: &_cd_size);
         if (!p)
                 return log_debug_errno(errno_or_else(SYNTHETIC_ERRNO(EINVAL)),
-                                       "crypt_ra() failed: %m");
+                                       CRYPT_RA_NAME "() failed: %m");
 
         p = strdup(p);
         if (!p)

From 35e22827a9b74321e5ad13c5b9c9b7cff1fb0b96 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 11 Sep 2020 08:27:43 +0200
Subject: [PATCH 10/12] shared/libcrypt-util: do not refuse passwords if some
 other hash is unsupported

---
 src/shared/libcrypt-util.c    | 8 ++++++--
 src/test/test-libcrypt-util.c | 8 ++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/shared/libcrypt-util.c b/src/shared/libcrypt-util.c
index 66f345ee5f..c5d98671bd 100644
--- a/src/shared/libcrypt-util.c
+++ b/src/shared/libcrypt-util.c
@@ -182,8 +182,12 @@ int test_password_one(const char *hashed_password, const char *password) {
 
         errno = 0;
         k = crypt_ra(password, hashed_password, &cd_data, &cd_size);
-        if (!k)
-                return errno_or_else(EINVAL);
+        if (!k) {
+                if (errno == ENOMEM)
+                        return -ENOMEM;
+                /* Unknown or unavailable hashing method or string too short */
+                return 0;
+        }
 
         return streq(k, hashed_password);
 }
diff --git a/src/test/test-libcrypt-util.c b/src/test/test-libcrypt-util.c
index 90abfa7b81..1c570784f4 100644
--- a/src/test/test-libcrypt-util.c
+++ b/src/test/test-libcrypt-util.c
@@ -40,10 +40,18 @@ static void test_hash_password_full(void) {
                                                                hashed,
                                                                "$y$j9T$SAayASazWZIQeJd9AS02m/$"),
                                                      i) == true);
+                        assert_se(test_password_many(STRV_MAKE("$W$j9T$dlCXwkX0GC5L6B8Gf.4PN/$VCyEH", /* no such method exists... */
+                                                               hashed,
+                                                               "$y$j9T$SAayASazWZIQeJd9AS02m/$"),
+                                                     i) == true);
                         assert_se(test_password_many(STRV_MAKE("$y$j9T$dlCXwkX0GC5L6B8Gf.4PN/$VCyEH",
                                                                hashed,
                                                                "$y$j9T$SAayASazWZIQeJd9AS02m/$"),
                                                      "") == false);
+                        assert_se(test_password_many(STRV_MAKE("$W$j9T$dlCXwkX0GC5L6B8Gf.4PN/$VCyEH", /* no such method exists... */
+                                                               hashed,
+                                                               "$y$j9T$SAayASazWZIQeJd9AS02m/$"),
+                                                     "") == false);
                 }
 }
 

From bd99297bd3e78f6b06a588243df71db3553d0e5c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 15 Sep 2020 09:18:31 +0200
Subject: [PATCH 11/12] test-libcrypt-util: skip test on ppc64 with no xcrypt

I'm tired of trying to figure this out.
---
 src/test/test-libcrypt-util.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/test/test-libcrypt-util.c b/src/test/test-libcrypt-util.c
index 1c570784f4..89ff76e84c 100644
--- a/src/test/test-libcrypt-util.c
+++ b/src/test/test-libcrypt-util.c
@@ -58,6 +58,10 @@ static void test_hash_password_full(void) {
 int main(int argc, char *argv[]) {
         test_setup_logging(LOG_DEBUG);
 
+#if defined(__powerpc__) && !defined(XCRYPT_VERSION_MAJOR)
+        return log_tests_skipped("crypt_r() causes a buffer overflow on ppc64el, see https://github.com/systemd/systemd/pull/16981#issuecomment-691203787");
+#endif
+
         test_hash_password_full();
 
         return 0;

From 5d3fe6f78d8b92356b910ea11dbd64501d87c485 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 15 Sep 2020 09:30:39 +0200
Subject: [PATCH 12/12] test-libcrypt-util: before doing anything check what
 methods are available

On centos7 ci:

--- test-libcrypt-util begin ---
Found container virtualization none.
/* test_hash_password */
ew3bU1.hoKk4o: yes
$1$gc5rWpTB$wK1aul1PyBn9AX1z93stk1: no
$2b$12$BlqcGkB/7BFvNMXKGxDea.5/8D6FTny.cbNcHW/tqcrcyo6ZJd8u2: no
$5$lGhDrcrao9zb5oIK$05KlOVG3ocknx/ThreqXE/gk.XzFFBMTksc4t2CPDUD: no
$6$c7wB/3GiRk0VHf7e$zXJ7hN0aLZapE.iO4mn/oHu6.prsXTUG/5k1AxpgR85ELolyAcaIGRgzfwJs3isTChMDBjnthZyaMCfCNxo9I.: no
$y$j9T$$9cKOWsAm4m97WiYk61lPPibZpy3oaGPIbsL4koRe/XD: no
---
 src/test/test-libcrypt-util.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/test/test-libcrypt-util.c b/src/test/test-libcrypt-util.c
index 89ff76e84c..58b83b6866 100644
--- a/src/test/test-libcrypt-util.c
+++ b/src/test/test-libcrypt-util.c
@@ -10,6 +10,37 @@
 #include "tests.h"
 #include "libcrypt-util.h"
 
+static int test_hash_password(void) {
+        log_info("/* %s */", __func__);
+
+        /* As a warmup exercise, check if we can hash passwords. */
+
+        bool have_sane_hash = false;
+        const char *hash;
+
+        FOREACH_STRING(hash,
+                       "ew3bU1.hoKk4o",
+                       "$1$gc5rWpTB$wK1aul1PyBn9AX1z93stk1",
+                       "$2b$12$BlqcGkB/7BFvNMXKGxDea.5/8D6FTny.cbNcHW/tqcrcyo6ZJd8u2",
+                       "$5$lGhDrcrao9zb5oIK$05KlOVG3ocknx/ThreqXE/gk.XzFFBMTksc4t2CPDUD",
+                       "$6$c7wB/3GiRk0VHf7e$zXJ7hN0aLZapE.iO4mn/oHu6.prsXTUG/5k1AxpgR85ELolyAcaIGRgzfwJs3isTChMDBjnthZyaMCfCNxo9I.",
+                       "$y$j9T$$9cKOWsAm4m97WiYk61lPPibZpy3oaGPIbsL4koRe/XD") {
+                int b;
+
+                b = test_password_one(hash, "ppp");
+                log_info("%s: %s", hash, yes_no(b));
+#if defined(XCRYPT_VERSION_MAJOR)
+                /* xcrypt is supposed to always implement all methods. */
+                assert_se(b);
+#endif
+
+                if (b && IN_SET(hash[1], '6', 'y'))
+                        have_sane_hash = true;
+        }
+
+        return have_sane_hash;
+}
+
 static void test_hash_password_full(void) {
         log_info("/* %s */", __func__);
 
@@ -62,6 +93,9 @@ int main(int argc, char *argv[]) {
         return log_tests_skipped("crypt_r() causes a buffer overflow on ppc64el, see https://github.com/systemd/systemd/pull/16981#issuecomment-691203787");
 #endif
 
+        if (!test_hash_password())
+                return log_tests_skipped("crypt doesn't support yescrypt or sha512crypt");
+
         test_hash_password_full();
 
         return 0;