From c6375343615ac20cc8bc8abd953093f7ebaaa401 Mon Sep 17 00:00:00 2001
From: Kaz Kylheku <kaz@kylheku.com>
Date: Sun, 3 Sep 2023 11:58:58 -0700
Subject: crypt: detect error tokens more weakly; drop some tests.

It has been reported by user cielesti that some of our
crypt tests fail on the Musl library.

Musl has some additional agorithms so it yields
a meaningful hash for a "$0$" salt, as well as for "$9$".

Musl uses "*" and "x" as error tokens rather than "*0"
and "*1". We need to change how we detect error tokens.

* sysif.c (crypt_wrap): Detect error tokens only by their
length: if a string emerges from crypt or crypt_r, whose
length is less than 13, it's an error token.

* tests/018/crypt.tl: Drop the tests that require :error
for salts "$0$" and "$9$", replacing them with a test
for a salt that is almost certainly invalid in all C libraries
on Linux.

* txr.1: Document that crypt throws an error exception and
under what circumstances (when the C library function does
what).
---
 sysif.c            | 13 +++++++++++--
 tests/018/crypt.tl |  3 +--
 txr.1              | 12 ++++++++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/sysif.c b/sysif.c
index d845c8ad..3b48af0b 100644
--- a/sysif.c
+++ b/sysif.c
@@ -2081,9 +2081,18 @@ static val crypt_wrap(val wkey, val wsalt)
   free(key);
   free(salt);
 
-  /* libxcrypt puts out two possible failure tokens "*0" or "*1".
+  /* libraries cannot agree on how to report unrecognized or bad hashes:
+   *
+   * - older glibc versions, other libraries return null
+   * - libxcrypt, integrated into newer glibc puts out two
+   *   possible failure tokens "*0" or "*1", documenting
+   *   that an error token starts with "*" and is less than 13
+   *   characters long.
+   * - musl uses "*" and "x", the latter being in the valid hash charset!
+   *
+   * let's go with: null or less than 13 chars means error.
    */
-  if (hash != 0 && strcmp(hash, "*0") != 0 && strcmp(hash, "*1") != 0) {
+  if (hash != 0 && memchr(hash, 0, 13) == 0) {
     val ret = string_utf8(hash);
 #if HAVE_CRYPT_R
     free(cd);
diff --git a/tests/018/crypt.tl b/tests/018/crypt.tl
index 33fd0ac5..7e68d6c7 100644
--- a/tests/018/crypt.tl
+++ b/tests/018/crypt.tl
@@ -11,8 +11,7 @@
 (if (eq :linux (os-symbol))
   (mtest
     (crypt "a" "b") :error
-    (crypt "a" "$0$") :error
-    (crypt "a" "$9$") :error
+    (crypt "a" "*$") :error
     (crypt "a" "$1$") "$1$$Ij31LCAysPM23KuPlm1wA/"
     (crypt "a" "$1$bcd$") "$1$bcd$cgz778Ks3pkbWfyW.CWae/"
     (crypt "a" "$5$") "$5$$QG6CCM7eJAxpUPcBpn0Z2K29NHtaI6Mk1fCpPrpjdj3"
diff --git a/txr.1 b/txr.1
index 2a772c76..eb20732e 100644
--- a/txr.1
+++ b/txr.1
@@ -74768,6 +74768,18 @@ that buffer. Where available, the
 .code crypt_r
 function is used which avoids static storage.
 
+Implementations of the C function vary in their error reporting.
+Some implementations return a null pointer for invalid salts,
+whereas others return valid "error token" strings which vary
+between implementations. To work consistently across numerous
+implementations, the \*(TL
+.code crypt
+function throws an
+.code error
+exception if the C library function returns either a null pointer,
+or a valid pointer to a string that is less than 13 characters long,
+regardless of its content.
+
 .SS* Unix Signal Handling
 
 On platforms where certain advanced features of POSIX signal handling are
-- 
cgit v1.2.3