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

please consider giving a dedicated exit value for "invalid user name" #1103

Open
zeha opened this issue Oct 29, 2024 · 10 comments · May be fixed by #1141
Open

please consider giving a dedicated exit value for "invalid user name" #1103

zeha opened this issue Oct 29, 2024 · 10 comments · May be fixed by #1141

Comments

@zeha
Copy link
Contributor

zeha commented Oct 29, 2024

Debian has a wrapper around useradd called adduser, which controls a lot of the default policy on Debian etc. It also has its own idea on which usernames are allowable. The adduser maintainer requested the following feature, IMO a sensible request.
I hope upstream is open for this feature.

Quoting the feature request in full:

recently, useradd has grown funcitonality to check user names for
validity, see #1074306 [1]. This kind of clashes with adduser doing the same
thing, and has made testing adduser a bit harder. But still, the efforts
of staying sane upsetream-wise and close to upstream Debian-wise are
appreciated.

I would like to have adduser give more clear error messages like "the
user name is fine with adduser but useradd doesnt like it", and that
would be way easier if useradd's error message "invaid user name" woud
be backed with an appropriate, exclusive exit code.

I understand that this is unlikely to show up in useradd next week, so I
will be tweaking adduser's tests to handle those failures more
graefully, but I have just commented the tests so that we can re-enable
them once there is a more reliable method to detect this kind of useradd
error programatically. Please keep this bug report posted about your and
upstream's decisions.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1074306

@alejandro-colomar
Copy link
Collaborator

I think adduser(8) should not attempt to check the validity of names. Why not patch useradd(8) to do the checks you want?

@zeha
Copy link
Contributor Author

zeha commented Dec 5, 2024

I think adduser(8) should not attempt to check the validity of names. Why not patch useradd(8) to do the checks you want?

adduser should still be able to tell somehow that useradd deemed the name to be bad. A dedicated exit code would be a nice way to indicate that.

@alejandro-colomar
Copy link
Collaborator

alejandro-colomar commented Dec 5, 2024

adduser should still be able to tell somehow that useradd deemed the name to be bad.

Could you clarify why adduser(8) needs to be able to tell that useradd(8) deemed the name too bad? Why is it not enough to let useradd(8) fail and print to stderr, and then just fail from adduser(8) too?

What does adduser(8) do different when useradd(8) fails due to ENOMEM, from when it fails due to badname?

alx@devuan:~/src/shadow/shadow/master$ ./src/useradd /
useradd: invalid user name '/': use --badname to ignore
alx@devuan:~/src/shadow/shadow/master$ echo $?
3

@Zugschlus
Copy link

adduser might want to be friendly and give a friendly error message. "use --badname" is misleading since adduser's options are named differently.

@alejandro-colomar
Copy link
Collaborator

adduser might want to be friendly and give a friendly error message. "use --badname" is misleading since adduser's options are named differently.

How about adding a --badname option for compatibility, as an alias to whatever option you have?

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 5, 2024
…ames

Wrappers like adduser(8) want to do their own stuff if the login name is
bad.  For that, they need to be able to differentiate such an error.

Closes: <shadow-maint#1103>
Suggested-by: Chris Hofstaedtler <zeha@debian.org>
Cc: Marc 'Zugschlus' Haber <mh+githubvisible@zugschlus.de>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@Zugschlus
Copy link

Thank you very much for considering this on your side. Do you want me to contribute docs? At least the man page needs adaption.

@alejandro-colomar
Copy link
Collaborator

Thank you very much for considering this on your side.

You're welcome. :-)

Do you want me to contribute docs? At least the man page needs adaption.

Sure, feel free to send a patch/PR to my branch here: https://github.com/alejandro-colomar/shadow/tree/badname. I can then squash it in the PR (crediting you, of course).

@zeha
Copy link
Contributor Author

zeha commented Dec 5, 2024

@Zugschlus I assume you'll take it from here, ok?

@Zugschlus
Copy link

@Zugschlus I assume you'll take it from here, ok?

Depends on what else there is to do.

@zeha
Copy link
Contributor Author

zeha commented Dec 5, 2024

@Zugschlus I assume you'll take it from here, ok?

Depends on what else there is to do.

You said something about contributing docs. I guess you'll check if the implementation does what you wanted, incl. for groupadd. I'll deal with the Debian side once it's in master, or when we want to run with a draft patch. Feel free to ping me tho!

alejandro-colomar added a commit to alejandro-colomar/shadow that referenced this issue Dec 5, 2024
…ames

Wrappers like adduser(8) want to do their own stuff if the login name is
bad.  For that, they need to be able to differentiate such an error.

Closes: <shadow-maint#1103>
Suggested-by: Chris Hofstaedtler <zeha@debian.org>
Cc: Marc 'Zugschlus' 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 6, 2024
…ames

Wrappers like adduser(8) want to do their own stuff if the login name is
bad.  For that, they need to be able to differentiate such an error.

Closes: <shadow-maint#1103>
Suggested-by: Chris Hofstaedtler <zeha@debian.org>
Cc: Marc 'Zugschlus' 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
…ames

Wrappers like adduser(8) want to do their own stuff if the login name is
bad.  For that, they need to be able to differentiate such an error.

Closes: <shadow-maint#1103>
Suggested-by: Chris Hofstaedtler <zeha@debian.org>
Cc: Marc 'Zugschlus' 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