Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we remove --badname? #1149

Open
alejandro-colomar opened this issue Dec 8, 2024 · 9 comments · May be fixed by #1151
Open

Should we remove --badname? #1149

alejandro-colomar opened this issue Dec 8, 2024 · 9 comments · May be fixed by #1151

Comments

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Dec 8, 2024

https://lwn.net/Articles/1001215/#Comments

That should be a reminder of the dangers.

My opinion is now stronger that names shall be limited to ASCII. I'm not happy allowing distros to lessen the requirements.

If one needs to manually fix an old system with such users, they can still manually edit the shadow files via coreutils tools. The files are text files, and so they can be fixed easily.

For continued use, I would like to ban any bad names.

How about removing --badname in 4.18?

Cc: @hallyn , @ikerexxe, @thesamesam , @jubalh , @zeha , @Zugschlus, @rbalint

See also: https://dwheeler.com/essays/fixing-unix-linux-filenames.html

@ikerexxe
Copy link
Collaborator

ikerexxe commented Dec 9, 2024

I would also feel more comfortable if we eliminated that option.

From what I see in the history, #121 was opened to re-add a way to bypass the username checks. That ticket mentions https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874, where it is discussed that Debian lost that ability because shadow removed it previously. So, shadow has had a way of bypassing username checks for some time already.

Before proceeding to remove the option I would like to understand the reasons why Debian still insists on having that option, and whether they would welcome its removal.

@alejandro-colomar
Copy link
Collaborator Author

I would also feel more comfortable if we eliminated that option.

From what I see in the history, #121 was opened to re-add a way to bypass the username checks. That ticket mentions https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874, where it is discussed that Debian lost that ability because shadow removed it previously. So, shadow has had a way of bypassing username checks for some time already.

Before proceeding to remove the option I would like to understand the reasons why Debian still insists on having that option, and whether they would welcome its removal.

They're discussing about internationalization of usernames.

The thread started in November here:
https://lists.debian.org/debian-devel/2024/11/msg00250.html.
And continued in December here:
https://lists.debian.org/debian-devel/2024/12/msg00012.html
(for some reason, their archive software for the mailing list can't follow from one month to the other, and you need a link to both).

Some people are in favor, some are against.

@alejandro-colomar
Copy link
Collaborator Author

LWN has also covered it on the news (behind a paywall): https://lwn.net/Articles/1000485/

@ikerexxe
Copy link
Collaborator

ikerexxe commented Dec 9, 2024

Yes, I already read those before replying to this ticket. That's why I think they should make their mind and we agree on a shared path for everybody.

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 9, 2024
Closes: <shadow-maint#1149>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@alejandro-colomar alejandro-colomar linked a pull request Dec 9, 2024 that will close this issue
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 9, 2024
Closes: <shadow-maint#1149>
Link: <https://www.wired.com/story/null-license-plate-landed-one-hacker-ticket-hell/>
Link: <https://xkcd.com/327/>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 9, 2024
lib/, man/, src/: Do not allow bad names

Closes: <shadow-maint#1149>
Link: <https://www.wired.com/story/null-license-plate-landed-one-hacker-ticket-hell/>
Link: <https://xkcd.com/327/>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@zeha
Copy link
Contributor

zeha commented Dec 9, 2024

I haven't fully looked at the proposed changes, but here are a few thoughts.

Two things in src:shadow are at play, and were maybe mixed up in the LWN discussion

  1. badname allows turning off the name check
  2. the allowed characters for "good" names

With this in mind, I'll note that the badname flag is probably mostly useless on Debian. For historic reasons we allow almost all characters in "good" names in shadow.
adduser has another set of restrictions on good names. This set is admin-configurable, so tighter restrictions in shadow/useradd break that.

Then, there is a general question on what the restrictions on "good" names should be. I stand by my observation and thought where we should go:

  1. Right now, some subset of ascii is safe. Some programs break if this is not followed.
  2. Programs already break if the gecos field is not strictly ascii. In Debian the username restriction is already very weak. Both these things mean that in practice the admin must deal with this somehow. Additionally, useradd/groupadd are m not the only software pieces modifying the shadow database files.
  3. I strongly believe that restricting to ascii as a design goal for the future is wrong. It's probably okay to keep the status quo and as an interim step. But the future shall become utf8.

Regardless of all that, for Debian we have to come up with a transition plan, if the badname flag shall go away. At least adduser needs to change and maybe deprecate admin-configurable restrictions already in Debian trixie (not much time left), and we need to check all package maintscripts if rhey use the flag, and maybe other stuff.

/cc @Zugschlus

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 9, 2024

This set is admin-configurable, so tighter restrictions in shadow/useradd break that.

I wish to not allow admins to configure this. It should just be forbidden.

Then, there is a general question on what the restrictions on "good" names should be. I stand by my observation and thought where we should go:

1. Right now, some subset of ascii is safe. Some programs break if this is not followed.

Aka, the POSIX recommended way.

2. Programs already break if the gecos field is not strictly ascii. In Debian the username restriction is already very weak. Both these things mean that in practice the admin must deal with this somehow. Additionally, useradd/groupadd are m not the only software pieces modifying the shadow database files.

Hmmm, I hope none of shadow.git programs break with those. gecos should be free-form. If there's any bug, please report.

3. I strongly believe that restricting to ascii as a design goal for the future is wrong. It's probably okay to keep the status quo and as an interim step. But the future shall become utf8.

I strongly oppose. And I say this while both my mother tongue (Valencian) and my second native language (Spanish) contain non-ASCII characters. But I think they should not be allowed in usernames, nor in anything meaningful to the system. That's just a path for disaster.

And why limit to UTF8? Names may be composed of sounds not representable by UTF8. UTF8 might be offensive to such names. https://www.youtube.com/watch?v=hNoS2BU6bbQ And should we allow Little Bobby Tables using its full name as a user name?

@alejandro-colomar
Copy link
Collaborator Author

As a data point: My mother's name includes a white space (Maria Teresa). Should whitespace be allowed? I guess no, but what's your opinion?

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 9, 2024
lib/, man/, src/: Do not allow bad names

Closes: <shadow-maint#1149>
Link: <https://www.wired.com/story/null-license-plate-landed-one-hacker-ticket-hell/>
Link: <https://xkcd.com/327/>
Link: <https://www.youtube.com/watch?v=hNoS2BU6bbQ>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 17, 2024
lib/, man/, src/: Do not allow bad names

Closes: <shadow-maint#1149>
Link: <https://www.wired.com/story/null-license-plate-landed-one-hacker-ticket-hell/>
Link: <https://xkcd.com/327/>
Link: <https://www.youtube.com/watch?v=hNoS2BU6bbQ>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@zeha
Copy link
Contributor

zeha commented Dec 22, 2024

Personal hot take:

  • no whitespace
  • no control characters
  • no slashes, no backslashes
  • no field separators used in files written by shadow (:, new line, ...)
  • not purely numeric
  • cannot start with dot
  • names should be normalized to one utf8 form (probably: require the name is already normalized, otherwise error out)
  • most utf8 characters should work

And why limit to UTF8? Names may be composed of sounds not representable by UTF8.

Because UTF8 is the (encoding of the) character set we now have on Linux, and if a name is not representable in that, UTF8/Unicode needs to be fixed.

@alejandro-colomar
Copy link
Collaborator Author

alejandro-colomar commented Dec 23, 2024

Personal hot take:

* no whitespace

* no control characters

* no slashes, no backslashes

* no field separators used in files written by shadow (:, new line, ...)

* not purely numeric

* cannot start with dot

* names should be normalized to one utf8 form (probably: require the name is already normalized, otherwise error out)

* most utf8 characters should work

How about this?

I will restrict --badname so that even if --badname is passed, the following are rejected:

""
"."
".."

  • Anything containing ',' or ':'.
  • A purely numeric name.

None of those can possibly work with our code, since they will cause a bug in one place or another. Unconditionally rejecting those might result in a safer --badname, which is also closer to having partial support for UTF-8.

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 23, 2024
lib/, man/, src/: Do not allow bad names

Closes: <shadow-maint#1149>
Link: <https://www.wired.com/story/null-license-plate-landed-one-hacker-ticket-hell/>
Link: <https://xkcd.com/327/>
Link: <https://www.youtube.com/watch?v=hNoS2BU6bbQ>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 24, 2024
lib/, man/, src/: Do not allow bad names

Closes: <shadow-maint#1149>
Link: <https://www.wired.com/story/null-license-plate-landed-one-hacker-ticket-hell/>
Link: <https://xkcd.com/327/>
Link: <https://www.youtube.com/watch?v=hNoS2BU6bbQ>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 26, 2024
lib/, man/, src/: Do not allow bad names

Closes: <shadow-maint#1149>
Link: <https://www.wired.com/story/null-license-plate-landed-one-hacker-ticket-hell/>
Link: <https://xkcd.com/327/>
Link: <https://www.youtube.com/watch?v=hNoS2BU6bbQ>
Link: <https://lwn.net/Articles/1001215/>
Link: <https://dwheeler.com/essays/fixing-unix-linux-filenames.html>
Link: <shadow-maint#121>
Link: <https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874>
Link: <https://lists.debian.org/debian-devel/2024/11/msg00250.html>
Link: <https://lists.debian.org/debian-devel/2024/12/msg00012.html>
Link: <https://lwn.net/Articles/1000485/>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Sam James <sam@gentoo.org>
Cc: Michael Vetter <jubalh@iodoru.org>
Cc: Chris Hofstaedtler <zeha@debian.org>
Cc: Balint Reczey <rbalint@debian.org>
Cc: Marc Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants