dslm4515 / Musl-LFS

Linux From Scratch using Musl as Libc
GNU General Public License v3.0
170 stars 21 forks source link

Mismatch between SHA_ROUNDS_MAX in shadow and musl #79

Open Strahinja opened 2 years ago

Strahinja commented 2 years ago

Regarding the https://github.com/dslm4515/Musl-LFS/blob/eb647866a11fb81c419bf7cba06324f05b4dd833/doc/3-Chroot/022-Shadow#L13 there is another hidden potential mismatch between shadow and musl.

Currently, the SHA_ROUNDS_MAX in shadow and musl differ. The one in musl 1.2.3 is defined as https://git.musl-libc.org/cgit/musl/tree/src/crypt/crypt_sha512.c?h=v1.2.3#n196

/* key limit is not part of the original design, added for DoS protection.
 * rounds limit has been lowered (versus the reference/spec), also for DoS
 * protection. runtime is O(klen^2 + klen*rounds) */
#define KEY_MAX 256
#define SALT_MAX 16
#define ROUNDS_DEFAULT 5000
#define ROUNDS_MIN 1000
#define ROUNDS_MAX 9999999

(7 digits), whereas the one expected by shadow 4.11.1 is https://github.com/shadow-maint/shadow/blob/1bf5868e3378aef0f36ba3490852709d79729419/libmisc/salt.c#L63

#define SHA_ROUNDS_MAX 999999999

(9 digits), which means that shadow can potentially request the larger number of rounds than supported by musl.

dslm4515 commented 2 years ago

Per the LFS 11.0 for shadow 4.9:

Fix a simple programming error by modifying a file with following command: sed -e "224s/rounds/min_rounds/" -i libmisc/salt.c

in LFS 11.1, shadow is updated to 4.11.1 and the suggested fix above was omitted. closed issue

But LFS, is based on Glibc. I haven’t checked if Glibc limits the max as well or if it matches shadow. As reference, I check Alpine Linux (and sometimes Void Linux) for any suggested patches or modifications. So far I have not seen any patches. If there is, I probably didn’t see it by mistake.

So far, I haven’t seen this cause any issues on my daily driver system.

Strahinja commented 2 years ago

I haven’t checked if Glibc limits the max as well or if it matches shadow. As reference, I check Alpine Linux (and sometimes Void Linux) for any suggested patches or modifications. So far I have not seen any patches. If there is, I probably didn’t see it by mistake.

glibc matches shadow with 9 digits:

https://github.com/bminor/glibc/blob/b92a49359f33a461db080a33940d73f47c756126/crypt/sha512-crypt.c#L91

So far, I haven’t seen this cause any issues on my daily driver system.

Nevertheless, the potential is there. What would happen if the number of rounds was passed greater than 9999999 (7 digits) is that musl would silently fallback to returning a DES-encrypted string, making shadow print out error about SHA512 "not being supported?", when it actually is. I believe this is either not being considered by musl developers, or left to the user's configuration of userland programs such as those from shadow.

On my system, I used sed to apply musl's limit to shadow as well, because it seems saner to me.

dslm4515 commented 2 years ago

Oh okay. I thank you for the explanation as… I wasn’t sure where to start researching.

then yes, I agree on applying the same limit in Musl Libc on shadow.

Strahinja commented 2 years ago

As reference, this is the exact command I used:

sed -i -e 's/^\(#define SHA_ROUNDS_MAX 9999999\)99/\1/' libmisc/salt.c