On Tuesday 02 August 2011 10:39:22 walter harms wrote:
>
> Am 01.08.2011 22:01, schrieb Tito:
> > Hi,
> > this patch improves the checks performed on usernames
> > with the die_if_bad_username() function by adduser and addgroup.
> > The changes are:
> > 1) better comments;
> > 2) use of the portable filename character set plus '@' and '$';
> > 3) don't use isalnum as it will allow non-ASCII letters in legacy 8-bit
> > locales as pointed out by Rich Felker;
> > 3) enforce minimum length of 1 char (or at least 2 chars if '$' is used as
> > last char);
> > 4) enforce maximum length of LOGIN_NAME_MAX (including null termination);
> > 5) don't use goto to jump into the loop (requested by Matthias Andree);
> > 6) don't allow '$', '.', '@' and '-' as first char;
> > 7) allow '$' only as last char.
> > 8) don't print the illegal char in error message as if it is a wide char it
> > will be unreadable.
> >
> > On Debian the default is more conservative as defined by the regular
> > expression /^[a-z][-a-z0-9]*$/
> > we use a more permissive /^[_A-Za-z0-9][-\@_.A-Za-z0-9]*\$?$/ instead.
> > The function is tested with this test cases:
> > "", "$", "a", "aa", "aA", "@a", "a@", "a@a", ".a", "a.", "a.a", "-a",
> > "a-", "a$", "$a", "a$a", "_a", "a_", "a_a", "a1", "1a", "a?",
> > "aè",
> > "a€", "aQzECE2-A_l@s$"
> > and seems to behave well.
> > I would like to point out that this check can be simply turned off
> > by disabling FEATURE_CHECK_NAMES in config
> > (Enable sanity check on user/group names in adduser and addgroup)
> > to avoid mixing of "mechanism (code) and policy" and also that
> > it mimics what adduser/addgroup do on my debian box (with
> > a more restrictive policy).
> > Hints, critics and improvements are welcome.
> > Special thanks go to Matthias Andree and Rich Felker for the patience
> > thay had in explaining me my errors.
> >
> > Please apply if you like it.
> >
> > Ciao,
> > Tito
> >
> >
> > /* The username to "(...) be portable across systems conforming to IEEE Std
> > * 1003.1-2001, (...) is composed of characters from the portable
> > * filename character set. The hyphen should not be used as the first
> > character
> > * of a portable username" (SUSv3). The Portable Filename Character Set is:
> > * A B C D E F G H I J K L M N O P Q R S T U V W X Y Z
> > * a b c d e f g h i j k l m n o p q r s t u v w x y z
> > * 0 1 2 3 4 5 6 7 8 9 . _ -
> > * We allow also the '@' and the '$' sign, but to avoid problems, '@','$'
> > and '.'
> > * are not permitted at the beginning of the username (for example, a name
> > * like ".." can make adduser chown "/home/..).
> > * For compatibility with Samba machine accounts '$' is supported
> > * (only) at the end of the username. On Debian the default is more
> > conservative as
> > * defined by the regular expression /^[a-z][-a-z0-9]*$/.
> > */
> >
> > void FAST_FUNC die_if_bad_username(const char *name)
> > {
> > const char *s = name;
> >
> > assert(name != NULL);
> >
> > /* The minimum size of the login name is one char or two if
> > * last char is the '$', this exception is catched later
> > * as the dollar sign could not be the first char.
> > * The maximum size of the login name is LOGIN_NAME_MAX
> > * including the terminating null byte.
> > */
> > if (!*name || strlen(name) + 1 > LOGIN_NAME_MAX)
> > bb_error_msg_and_die("illegal name length");
> >
>
> when there is a "char buf[LOGIN_NAME_MAX]", maybe you need
> strlen(name) + 1 >= LOGIN_NAME_MAX
Don't think so LOGIN_NAME_MAX is the length of the string including
the terminating null byte, strlen returns the length without the null byte
so a strlen + 1 /the null byte*/ fits in LOGIN_NAME_MAX it is ok
if it greater than LOGIN_NAME_MAX it is a error.
strlen(name) + 1 < LOGIN_NAME_MAX = ok
strlen(name) + 1 = LOGIN_NAME_MAX = ok
strlen(name) + 1 > LOGIN_NAME_MAX = error
>
> > do {
> > /* We don't use isalnum as it will allow locale-specific
> > non-ASCII */
> > /* letters in legacy 8-bit locales. */
> > if (((*name == '-' || *name == '.' || *name == '@') && name !=
> > s) /* not as first char */
> > || (*name == '$' && (!name[1] && name != s)) /* not as first,
> > only as last char */
> > || *name == '_'
> > || (*name >= 48 && *name <= 57) /* 0-9 */
> > || (*name >= 65 && *name <= 90) /* A-Z */
> > || (*name >= 97 && *name <= 122) /* a-z */
> > ) {
> > continue;
> > }
>
>
> the user should habe a change to understand what he did wrong ...
> perhaps you can do something like that:
> bb_error_msg_and_die("illegal character >%c< in name",*name);
I tried this but in some cases it prints garbage to terminal e.g.:
printf("'%c'\n", 'é');
./test
'�'
so i tought that it is not worth the hassle;
> > bb_error_msg_and_die("illegal character in name");
> > } while (*++name);
> > }
>
> How important do you cansider that $ can be "nowhere else that last place"
> thing ?
> So far i understand that Samba can life with $, dropping support for that
> would simplify the thing.
I cannot say, I've extracted this info from an adduser error message:
"To avoid problems, the username should consist only of
letters, digits, underscores, periods, at signs and dashes, and not start with
a dash (as defined by IEEE Std 1003.1-2001). For compatibility with Samba
machine accounts $ is also supported at the end of the username"
To be honest original adduser implements a first regex that honors
more or less this error message: /^[_.A-Za-z0-9][-\@_.A-Za-z0-9]*\$?$/
but is later overriden NAME_REGEX setting in /etc/addduser.conf
or hardcoded in /usr/share/perl5/Debian/AdduserCommon.pm
which on my debian box is more restrictive: /^[a-z][-a-z0-9]*$/
and doesn't support '$' at all. So eventually it could be dropped.
Ciao,
Tito
> >
> just my 2 cents,
> re,
> wh
>
>
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
