From 40faae610d203bc103c9e8017e5ab932835f5cec Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Mon, 4 Nov 2024 17:34:49 -0500 Subject: [PATCH 1/3] ext/standard/tests/strings/crypt_sha256.phpt: fix on musl Among other things, this test tries to run too few and too many rounds of SHA256. In both cases, it is expecting an error, but that behavior depends on the implementation: * PHP's own implementation raises an error in either case * libxcrypt raises an error in either case * Older versions of glibc would clamp the number of rounds to a valid amount (newer versions don't have libcrypt) * Musl libc clamps values that are too low, but raises an error for values that are too high If PHP is built with --with-external-libcrypt, the musl implementation above can be used. Even if libxcrypt is installed, PHP will notice that no additional -lfoo flags are needed to use the crypt implementation in musl. To pass on such a system, we must not test for the "too few rounds" behavior. --- ext/standard/tests/strings/crypt_sha256.phpt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/ext/standard/tests/strings/crypt_sha256.phpt b/ext/standard/tests/strings/crypt_sha256.phpt index 095e8f1691336..324248294be72 100644 --- a/ext/standard/tests/strings/crypt_sha256.phpt +++ b/ext/standard/tests/strings/crypt_sha256.phpt @@ -39,12 +39,10 @@ $tests = array( 'a short string', '$5$rounds=123456$asaltof16chars..$gP3VQ/6X7UUEW3HkBn2w1/Ptq2jxPyzV/cZKmF/wJvD' ), + + // The "too many rounds" behavior depends on the crypt() + // implementation, but for now everyone agrees on what to do. 8 => array( - '$5$rounds=10$roundstoolow', - 'the number of rounds is too low', - '*0' - ), - 9 => array( '$5$rounds=1000000000$roundstoohigh', 'the number of rounds is too high', '*0' From 9c7df556fa1ec947be8d671bc51ae53376b23799 Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Mon, 4 Nov 2024 18:47:46 -0500 Subject: [PATCH 2/3] ext/standard/tests/crypt/des_fallback_invalid_salt.phpt: less valid salt Musl's crypt() implementation of DES tries to handle invalid salts and can make this test fail because it returns an answer and not an error. Even musl however will reject a salt with a ':' in it, so we can make this test cross-platform by supplying an even less valid salt. --- ext/standard/tests/crypt/des_fallback_invalid_salt.phpt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ext/standard/tests/crypt/des_fallback_invalid_salt.phpt b/ext/standard/tests/crypt/des_fallback_invalid_salt.phpt index b0797657d80a2..8b00c81bbd1ce 100644 --- a/ext/standard/tests/crypt/des_fallback_invalid_salt.phpt +++ b/ext/standard/tests/crypt/des_fallback_invalid_salt.phpt @@ -3,8 +3,11 @@ Test DES with invalid fallback --FILE-- --EXPECT-- From f32fe6a56d07bbabc364962400f5702f5c72b22c Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Mon, 4 Nov 2024 18:50:07 -0500 Subject: [PATCH 3/3] ext/standard/crypt.c: handle musl failure tokens Musl's crypt() returns "*" to indicate failure in contrast with the "*0" returned by PHP/libxcrypt. This causes test failures, but more importantly, is a pretty silly thing to expect the user to know. This commit catches the musl value and turns it into "*0". --- ext/standard/crypt.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ext/standard/crypt.c b/ext/standard/crypt.c index 14eb6cda9735a..54687f6cdf307 100644 --- a/ext/standard/crypt.c +++ b/ext/standard/crypt.c @@ -177,7 +177,19 @@ PHPAPI zend_string *php_crypt(const char *password, const int pass_len, const ch if (!crypt_res || (salt[0] == '*' && salt[1] == '0')) { return NULL; - } else { + } + else if (!strcmp(crypt_res, "*")) { + /* Musl crypt() uses "*" as a failure token rather + * than the "*0" that libxcrypt/PHP use. Our test + * suite in particular looks for "*0" in a few places, + * and it would be annoying to handle both values + * explicitly. It seems wise to abstract this detail + * from the end user: if it's annoying for us, imagine + * how annoying it would be in end-user code; not that + * anyone would think of it. */ + return NULL; + } + else { result = zend_string_init(crypt_res, strlen(crypt_res), 0); return result; }