lib/chkname.c: Put limits for LOGIN_NAME_MAX and sysconf(_SC_LOGIN_NAME_MAX)

[alejandro-colomar]

GNU Hurd recommends having no system limits.  When a program needs a
limit, because it needs to validate user input, it is recommended that
each program defines its own limit macros.  The rationale is that this
avoids hard-coded limits in ABIs, which cannot be modified ever.

However, that doesn't mean that programs should have no limits at all.
We use this limit for validating user input, and so we shouldn't allow
anything just because the system doesn't want to set a limit.

So, when sysconf(2) returns -1, either due to an error or due to a claim
for no limits, we must fall back to the LOGIN_NAME_MAX value.  And if
the system doesn't define that value, we must define it ourselves (we're
more or less free to choose any value, so let's pick the one that glibc
provides nowadays).

Fixes: 6a1f45d (2024-02-04: "lib/chkname.c: Support unlimited user name lengths")
Closes: #1166
Cc: @zeha
Cc: @stoeckmann
Cc: @sthibaul

---

Revisions:

v1b

• Remove credits to @zeha
• Update copyright.
• Mention in commit message explicitly that GNU Hurd doesn't define LOGIN_NAME_MAX. [@zeha]

$ git range-diff shadow/master gh/hurd hurd 
1:  1d87e944 ! 1:  f2698953 lib/chkname.c: login_name_max_size(): Put limits for LOGIN_NAME_MAX and sysconf(_SC_LOGIN_NAME_MAX)
    @@ Metadata
      ## Commit message ##
         lib/chkname.c: login_name_max_size(): Put limits for LOGIN_NAME_MAX and sysconf(_SC_LOGIN_NAME_MAX)
     
    -    GNU Hurd recommends having no system limits.  When a program needs a
    -    limit, because it needs to validate user input, it is recommended that
    -    each program defines its own limit macros.  The rationale is that this
    -    avoids hard-coded limits in ABIs, which cannot be modified ever.
    +    GNU Hurd doesn't define LOGIN_NAME_MAX.  GNU Hurd recommends having no
    +    system limits.  When a program needs a limit, because it needs to
    +    validate user input, it is recommended that each program defines its own
    +    limit macros.  The rationale is that this avoids hard-coded limits in
    +    ABIs, which cannot be modified ever.
     
         However, that doesn't mean that programs should have no limits at all.
         We use this limit for validating user input, and so we shouldn't allow
    @@ Commit message
     
         Fixes: 6a1f45d932c8 (2024-02-04; "lib/chkname.c: Support unlimited user name lengths")
         Closes: <https://github.com/shadow-maint/shadow/issues/1166>
    -    Co-developed-by: Chris Hofstaedtler <[email protected]>
    +    Cc: Chris Hofstaedtler <[email protected]>
         Cc: Tobias Stoeckmann <[email protected]>
         Cc: Samuel Thibault <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/chkname.c ##
    +@@
    + // SPDX-FileCopyrightText: 1996-2000, Marek Michałkiewicz
    + // SPDX-FileCopyrightText: 2001-2005, Tomasz Kłoczko
    + // SPDX-FileCopyrightText: 2005-2008, Nicolas François
    +-// SPDX-FileCopyrightText: 2023-2024, Alejandro Colomar <[email protected]>
    ++// SPDX-FileCopyrightText: 2023-2025, Alejandro Colomar <[email protected]>
    + // SPDX-License-Identifier: BSD-3-Clause
    + 
    + 
     @@
      #include <limits.h>
      #include <stdbool.h>

v1c

• Reviewed-by: @sthibaul , @stoeckmann , @ikerexxe

$ git range-diff shadow/master gh/hurd hurd 
1:  f2698953 ! 1:  87b8f5a3 lib/chkname.c: login_name_max_size(): Put limits for LOGIN_NAME_MAX and sysconf(_SC_LOGIN_NAME_MAX)
    @@ Commit message
         Fixes: 6a1f45d932c8 (2024-02-04; "lib/chkname.c: Support unlimited user name lengths")
         Closes: <https://github.com/shadow-maint/shadow/issues/1166>
         Cc: Chris Hofstaedtler <[email protected]>
    -    Cc: Tobias Stoeckmann <[email protected]>
    -    Cc: Samuel Thibault <[email protected]>
    +    Reviewed-by: Samuel Thibault <[email protected]>
    +    Reviewed-by: Tobias Stoeckmann <[email protected]>
    +    Reviewed-by: Iker Pedrosa <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
     
      ## lib/chkname.c ##

[alejandro-colomar]

@zeha , I put a Co-developed-by because I based my patch on your Debian patch. However, if you think it differs too much from it and want that tag removed, just let me know.

If you want to keep it, feel free to sign the patch by sending a Signed-off-by.

[zeha]

Yeah I don't think I want to take any credit for this.

BTW: do you want a comment next to the ifdef hinting that this is mostly for Hurd?

[alejandro-colomar]

Yeah I don't think I want to take any credit for this.

Okay. I've removed that line, and put a Cc one instead.

BTW: do you want a comment next to the ifdef hinting that this is mostly for Hurd?

I don't know. I tend to prefer to keep those in the commit message. A git-blame(1) on any of those three lines will bring up this commit. I've edited the commit message to be more explicit about it.

[alejandro-colomar]

@zeha , @sthibaul , @stoeckmann

Can you please review and (if appropriate) approve the PR?

[alejandro-colomar]

Thanks for the reviews!

[ikerexxe]

LGTM. Thanks for taking care of it.