GNOME Bugzilla – Bug 724193
utmp.h and UT_NAMESIZE are not portable
Last modified: 2018-02-01 15:33:24 UTC
um-utils.c in the user-accounts panel makes use of this header and this constant The value from Linux is 32. I think it would make sense to fallback to this as a reasonable constant if we can't find the header.
Created attachment 268940 [details] [review] user-accounts: use POSIX for max login name length Use the POSIX-specified way of determining the maximum length of a valid username on this system.
The way this works is that the system will define LOGIN_NAME_MAX if the value is statically known. If it is not defined, then you're supposed to use sysconf(). I think in all cases where the value is statically defined, sysconf() will return that value. I've seen some advice around, however (HP-UX) that you should avoid the static path and always use the dynamic one, presumably because some people were hard-depending on the static macro existing and they were afraid to remove it, but wanted to dynamically change the max username length in the future. I'm not really sure if we should care about that, but on the other hand, it would simplify the code a bit to always use the dynamic route (at the cost of an extremely small performance hit). Your call...
Review of attachment 268940 [details] [review]: looks fine to me
Attachment 268940 [details] pushed as 07d24bb - user-accounts: use POSIX for max login name length
It doesn't work. glibc's LOGIN_NAME_MAX is 256, but useradd limits us to 32. See bug #766401.
I think the initial reasoning in comment #2 was faulty. According to comments in /usr/include/bits/local_lim.h, LOGIN_NAME_MAX is the "Minimum guaranteed maximum value" for the sysconf value.
shadow-utils does this: /* Maximum length of usernames */ #ifdef HAVE_UTMPX_H # include <utmpx.h> # define USER_NAME_MAX_LENGTH (sizeof (((struct utmpx *)NULL)->ut_user)) #else # include <utmp.h> # ifdef HAVE_STRUCT_UTMP_UT_USER # define USER_NAME_MAX_LENGTH (sizeof (((struct utmp *)NULL)->ut_user)) # else # ifdef HAVE_STRUCT_UTMP_UT_NAME # define USER_NAME_MAX_LENGTH (sizeof (((struct utmp *)NULL)->ut_name)) # else # define USER_NAME_MAX_LENGTH 32 # endif # endif #endif All of the above evaluate to 32 with glibc. It makes sense. Even if the kernel allows larger usernames, it doesn't make sense to create usernames too large to be tracked by utmp. Note that while struct utmp and utmp.h are nonstandard, struct utmpx and utmpx.h are both specified by POSIX, but in XSI (i.e. they're optional). There is no specified constant for the length of the struct, hence the nasty workaround to find the size, rather than relying on nonstandard UT_NAMESIZE.
Created attachment 328033 [details] [review] user-accounts: Fix definition of MAXNAMELEN
I'm not sure this makes sense, as it implies that LOGIN_NAME_MAX is useless for all practical purposes, but I don't see any other way to make this reliable.
All of these ancient interfaces (utmp, wtmp, utmpx...) are really shitty, broken and inconsistent.
Note we're shipping this in Endless, because we don't expose usernames at all and can't have account creation fail due to invalid usernames.
Review of attachment 328033 [details] [review]: Seems definitely better than the current state. utmpx.h should be portable on POSIX compliant, so probably ok... Just it needs to be rebased on master. Please add also some commit description.
Created attachment 367736 [details] [review] user-accounts: Fix definition of MAXNAMELEN Currently we get MAXNAMELEN from glibc's LOGIN_NAME_MAX, if available, and sysconf otherwise. But the maximum username length supported by glibc and the kernel is actually way larger than the maximum length that actually works in practice. On Linux, anything larger than 32 characters is not going to fit into utmp, and will therefore be rejected by useradd. Then gnome-control-center will spit out a confusing error message dialog. Let's spare our users from that. useradd (in shadow-utils) gets its max name size from the following magic: /* Maximum length of usernames */ #ifdef HAVE_UTMPX_H # include <utmpx.h> # define USER_NAME_MAX_LENGTH (sizeof (((struct utmpx *)NULL)->ut_user)) #else # include <utmp.h> # ifdef HAVE_STRUCT_UTMP_UT_USER # define USER_NAME_MAX_LENGTH (sizeof (((struct utmp *)NULL)->ut_user)) # else # ifdef HAVE_STRUCT_UTMP_UT_NAME # define USER_NAME_MAX_LENGTH (sizeof (((struct utmp *)NULL)->ut_name)) # else # define USER_NAME_MAX_LENGTH 32 # endif # endif #endif It's more work than necessary. utmpx is standardized by POSIX (it's actually an XSI extension), whereas utmp is not, so let's just use utmpx. This ought to work on at least FreeBSD as well. And if any free operating systems that care about GNOME don't have utmpx yet, no doubt they'll send patches.
(In reply to Ondrej Holy from comment #12) > Review of attachment 328033 [details] [review] [review]: > > Seems definitely better than the current state. utmpx.h should be portable > on POSIX compliant, so probably ok... > > Just it needs to be rebased on master. > > Please add also some commit description. Rebased and added a commit message. OK now...?
Review of attachment 367736 [details] [review]: Yup, thanks :-)
Attachment 367736 [details] pushed as 09b94a9 - user-accounts: Fix definition of MAXNAMELEN