Created attachment 151358 [details] work arond for login not found in bsdconfig usermgmt I tried to use bsdconfig usermgm to edit an existing entry on FreeBSD-current and when select a user received: XXX: Login not found. It seems this line in user.subr not quite right: f_quietly pw usershow -n "$input" -u -1 pw with negative number for -u return error: % pw usershow -n "oleg" -u -1 pw: -u expects a number % echo $? 64
This was caused by SVN r273787 (bapt) -- merged to stable/10 by SVN r274082 (bapt). Before SVN r273787, two things can be said: 1. There existed a bug wherein "-u not_a_number" would see "not_a_number" passed to atoi(3) returning zero, and since atoi(3) provides no error checking, this caused pw(8) to act on the root account (uid=0). 2. Using atoi(3), pw(8) supported negative values for "-u" flag. After SVN r273787, two things can be said: 1. The bug with "-u not_a_number" has been fixed, causing pw(8) to produce the fatal-error "-u expects a number". 2. You can no longer pass a negative number to the "-u" flag. The latter should be considered a regression because of the following use-case where it is contextually correct to pass a negative number to pw(8)'s "-u" flag. Because the "-n" flag of pw(8) can take a name OR a uid, there exists an edge-case where it becomes required to pass both the "-n" and "-u" flags to get expected results. To witness the edge-case, create two users in /etc/master.passwd (NB: The "nobody" entry should already exist on your standard FreeBSD system): nobody:*:65534:65534::0:0:Unprivileged user:/nonexistent:/sbin/nologin 65534:*:65533:65534::0:0:Unprivileged user:/nonexistent:/sbin/nologin Now execute the following pw(8) command: $ pw usershow -n 65534 You get the [expected] results: 65534:*:65533:65534::0:0:Unprivileged user:/nonexistent:/sbin/nologin NB: Because the user's name is "65534" But now, what if we pass a UID to the "-n" flag? $ pw usershow -n 65533 65534:*:65533:65534::0:0:Unprivileged user:/nonexistent:/sbin/nologin Depending on what your program needs, this may be the unexpected results. For example, let's say you have a program which does the following: 1. Gets a list of all users by-name 2. Displays a list of usernames 3. Allows the end-user to select a username 4. Validates that there is a user by that name before then going further It is the latter which is an issue: How to verify that there is an account by-name when the pw(8) usershow command will return false-positives for usernames that double as UID? This is where a work-around comes into play: $ pw usershow -n 65533 -u 0 pw: no such user `65533' It turns out that when given both "-n" and "-u", pw(8) will only return results for name-only matches. What's more, it doesn't matter what you use for the UID: $ pw usershow -n 65533 -u 65533 pw: no such user `65533' However, this syntax is very confusing. At first-glance, someone reading the code "pw usershow -n NAME -u NUMBER" will immediately think that the results will consist of accounts where both NAME and NUMBER match. It is not obvious that the above syntax causes a NAME-only search (ignoring whatever value is provided to "-u"). By using a negative number as the UID, it becomes more clear that we (the code) is doing a NAME-only search: $ pw usershow -n 65533 -u -1 pw: no such user `65533' NB: That's the response given pre-r273787. Post-r273787 the above reports: $ pw usershow -n ANYTHING -u -1 pw: -u expects a number Since the principal problem in solving related-PR 90114 was that there was no error-checking for number-conversion(s), the proper solution (instead of using strspn(3) to check for non-numbers in the argument) is to switch to a number-conversion function that supports error-checking (e.g., strtoll(3)). That being said, the change is NOT as simple as switching strspn(3) for strtoll(3) and adding error-checking. We need to make sure long-UID support (UID 0 thru 4294967295) is retained.
Another reason to support negative numbers for the "-u" flag is for referencing high-bit unsigned integer values. For example, in stable/9 I can still do this: pw usershow -u -1 fatapple:*:4294967295:65534::0:0:UID Test:/:/usr/sbin/nologin So, now there's two reasons why I want negative numbers back.
Feel free to add support for negative number as long as you do not introduce a regression on what r273787 which was fixing a harmful bug
(In reply to Devin Teske from comment #2) Maybe it makes sense to describe negative number in pw(8) to prevent regression in the future ?
(In reply to olevole from comment #4) Absolutely! (re: update pw(8) to document negative numbers) Since neither r273787 (head) nor r274082 (stable/10) have made it into a release, an update to the manual while fixing the behavior (to support negative numbers) would be good (considering pw(8) in all previous releases supports negative numbers as it currently stands).
A commit references this bug: Author: bapt Date: Sat Jan 24 19:13:05 UTC 2015 New revision: 277652 URL: https://svnweb.freebsd.org/changeset/base/277652 Log: Allow negative numbers in -u and -g options PR: 196514 MFC after: 1 week Changes: head/usr.sbin/pw/pw_group.c head/usr.sbin/pw/pw_user.c head/usr.sbin/pw/tests/Makefile head/usr.sbin/pw/tests/pw_groupshow.sh head/usr.sbin/pw/tests/pw_usershow.sh
Discussed on IRC (EFNet #bsdports -- where interested parties were dwelling). bdrewery, mandree, bapt, jgh, and I discussed the underlying issue(s) and believe we've come up with a viable solution that reduces the hackery in-play here. NB: Hackery being that I'm passing both -n NAME and -u IGNORED_UID to get pw(8) to treat NAME as a name-only versus what happens when you omit -u when using -n (say, for example, in a usershow command; read: when -u UID is present at the same time as -n NAME, NAME is no longer treated as "name _or_ uid" but instead "name only"). The same goes for a comparable "groupshow" (-n GROUP -g IGNORED_GID to get GROUP to be treated as groupname only and never treat it as a GID for matching). Because we don't want to break any existing scripts which may rely on the functionality of NAME passed to -n to be treated as an account name or account ID, we feel the best approach is to introduce a new flag which alters the way -n works. A cursory glance at the manual page for pw(8) (in recent HEAD) reveals that -z is a yet unused flag. I suggest the following syntax for performing queries on account name only: pw usershow -zn NAME pw groupshow -zn NAME The -z flag would cause NAME to be used as a name ONLY.
Plan is to (in order): 1. Apply interim fix to bsdconfig for upcoming 10.2-R 2. Push https://reviews.freebsd.org/D2700 NB: Paving way for next step 3. Add a `-z' flag to pw(8) 4. Apply change(s) to bsdconfig to use new `-z' flag of pw(8) 5. Close this bug
A commit references this bug: Author: dteske Date: Fri Jun 19 21:32:21 UTC 2015 New revision: 284609 URL: https://svnweb.freebsd.org/changeset/base/284609 Log: Interim fix for "Login not found" error. PR: bin/196514 MFC after: 3 days X-MFC-to: stable/10 Changes: head/usr.sbin/bsdconfig/usermgmt/share/group.subr head/usr.sbin/bsdconfig/usermgmt/share/user.subr
A commit references this bug: Author: dteske Date: Tue Jun 23 04:17:13 UTC 2015 New revision: 284716 URL: https://svnweb.freebsd.org/changeset/base/284716 Log: MFC r284609: Interim fix for "Login not found" error. PR: bin/196514 Changes: _U stable/10/ stable/10/usr.sbin/bsdconfig/usermgmt/share/group.subr stable/10/usr.sbin/bsdconfig/usermgmt/share/user.subr