search

shadow-maint / shadow

Upstream shadow tree
Other
290 stars 228 forks source link

Passwd length #953

Closed thalman closed 5 months ago

thalman commented 5 months ago

This PR addresses two issues:

First commit unifies the password length limit between agetpass.c and passwd.c.

The second one introduces checking the length of provided password.

alejandro-colomar commented 5 months ago

Thanks for doing this!

I see we have several macros that seem very related:

$ grep -rho 'PASS[A-Z_]*MAX[A-Z_]*' src/ lib* | sort | uniq | grep -v PASS_MAX_DAYS
PASSWD_ENTRY_MAX_LENGTH
PASS_MAX
PASS_MAX_LEN

Do any of those mean the same thing? I suspect we have different maximum lengths in different places, and we should unify them.

alejandro-colomar commented 5 months ago

https://github.com/shadow-maint/shadow/pull/953/commits/1a36ac7121dbf665414f7b42df621b8e69da0916 ("src/passwd.c: inconsistent password length limit ")

Reviewed-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar commented 5 months ago

https://github.com/shadow-maint/shadow/pull/953/commits/6a0c380705f02d9d85e156ad52f8bf61d815dd45 ("src/passwd.c: check password length upper limit")

Reviewed-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar commented 5 months ago

Thanks for doing this!

I see we have several macros that seem very related:

$ grep -rho 'PASS[A-Z_]*MAX[A-Z_]*' src/ lib* | sort | uniq | grep -v PASS_MAX_DAYS
PASSWD_ENTRY_MAX_LENGTH
PASS_MAX
PASS_MAX_LEN

Do any of those mean the same thing? I suspect we have different maximum lengths in different places, and we should unify them.

Let's keep this for a different PR. I think this one is good as is.

thalman commented 5 months ago

Thanks for doing this! I see we have several macros that seem very related:

$ grep -rho 'PASS[A-Z_]*MAX[A-Z_]*' src/ lib* | sort | uniq | grep -v PASS_MAX_DAYS
PASSWD_ENTRY_MAX_LENGTH
PASS_MAX
PASS_MAX_LEN

Do any of those mean the same thing? I suspect we have different maximum lengths in different places, and we should unify them.

Let's keep this for a different PR. I think this one is good as is.

I think that they have different meaning. If I understand correctly PASSWD_ENTRY_MAX_LENGTH is the whole record from /etc/passwd. PASS_MAX_LEN is from system configuration and PASS_MAX is our internal limit. But I might be wrong.