tests: add comprehensive unit tests for is_valid_hash() function by ikerexxe · Pull Request #1529 · shadow-maint/shadow

ikerexxe · 2026-02-06T15:38:43Z

Add comprehensive unit testing for the is_valid_hash() function in lib/chkhash.c, which is critical for preventing invalid password hashes from being written to /etc/shadow.

The is_valid_hash() function had no unit test coverage despite being security-critical. Recent issues and PRs (#1483, #1520, #1486, #1505) highlighted the need for robust testing to prevent regressions that could lock users out of their accounts.

Finally, I've modified a misleading comment for yescrypt hash lenght.

ikerexxe · 2026-02-06T15:46:48Z

@AdamWill this is something that you asked for in one of the PRs that you opened when fixing hash validation. Pinging you just in case you'd like to take a look

AdamWill · 2026-02-06T16:51:56Z

Thanks a lot! This is great. I think it's missing checks of bare ! and *, right, which was one of the cases that needed fixing? But other than that it looks good, covers the points we've found so far, and is well structured so it'd be easy to extend for future cases.

Fix misleading comment that stated "43-char (minimum) hash" when the actual regex pattern requires exactly 43 characters. Update comment to accurately reflect the implementation behavior. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Alejandro Colomar <alx@kernel.org>

Introduce unit testing infrastructure for the `is_valid_hash()` function. Add yescrypt algorithm validation tests. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

Add comprehensive bcrypt algorithm validation tests covering all variants ($2a$, $2b$, $2x$, $2y$) with different cost factors. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

Add comprehensive SHA-512 algorithm validation tests covering rounds and salt cases. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

Add comprehensive SHA-256 algorithm validation tests covering rounds and salt cases. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

Add basic MD5 and DES algorithm validation tests. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

*, ! and empty strings are special valid cases for shadow's second field. Add test cases for them. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

Add comprehensive negative testing condition validation: - Invalid algorithm prefixes and hash length validation - Invalid delimiter handling - Invalid salt characters and rounds parameter testing Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

ikerexxe · 2026-02-09T08:37:27Z

Thanks a lot! This is great. I think it's missing checks of bare ! and *, right, which was one of the cases that needed fixing?

That's already covered in test_is_valid_hash_ok_special()

alejandro-colomar

Thanks! LGTM.