On 2023-09-02 21:23, cielesti wrote:
> Hello, i am trying to upgrade the TXR package in Alpine Linux, which is currently at version 278.
>
> Alpine Linux uses musl libc, and on TXR 291, i encounter some issues with failing tests in "tests/018/crypt.tl".
>
> It seems 3 tests that are supposed to produce errors do not do so:
>
> 1) (crypt "a" "b") returns "*"
> 2) (crypt "a" "$0$") returns "$0UqSJBCvZvN."
> 3) (crypt "a" "$9$") returns "$9SxH5/IxLdbI"
Hi cielesti.
I've debugged Musl issues before; mostly by learning about them
by spying on distro activity, not due to anyone contacting the
TXR project. I've spun up VMs with Alpine to drill into things.
The first two test cases above are bogus. The focus should be to
test whether the crypt wrapper is working and doing the right
things, but in those tests are really just confirming that the
underlying library does *not* support certain algorithms.
One such test would be good, but using some garbage hash
that nobody is likely to treat as a valid algorithm header.
That batch of tests is only run on Linux, so it only has to work
with Linux C libraries.
About the first one, it looks like libraries are making a mess
of the error reporting rather that agreeing to do the same thing.
libxcrypt actually specifies that the error token starts with *
and is less than 13 characters long. I wrote the check for its
specific actual tokens which are *0 and *1. Even if I followed
their documented rule, though the above test case would pass,
the function would still return musl's "x" error token rather than
throw.
I'm going to change the code such that if a string is returned
that is less than 13 characters long, it's an error indication.
There won't be any check regarding its content, such as whether
it contains invalid characters like *. Even if it consists entirely
of valid salt/hash characters, it will be treated as erroneous.
How does this work for you? Not necessarily my final patch:
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"
(text/plain)