Bug 22860

There's been a lot of useless mumbling on this PR.

Unless someone can come up with proof that the $ character is illegal in
usernames, could everyone please stop arguing with the guy who keeps on
bringing up Microsoft.  He's not listening and doesn't seem to be here
to contribute.

By the way, what I can dig out of my POSIX.1 book suggests that the $
character, while not recommended, is legal.

It's stupid not to support legal characters in usernames.  It may be
stupid to use certain legal characters in usernames, but let's leave
policy up to the administrator and provide tools that do the right
thing.

Ciao,
Sheldon.

On Tue, Nov 14, 2000 at 10:01:22PM +0100, Gerhard Sittig scribbled:
|
| >Number:         22860
| >Category:       bin
| >Synopsis:       [PATCH] adduser & friends with '$' in usernames
| >Confidential:   no
| >Severity:       non-critical
| >Priority:       low
| >Responsible:    freebsd-bugs
| >State:          open
| >Quarter:
| >Keywords:
| >Date-Required:
| >Class:          change-request
| >Submitter-Id:   current-users
| >Arrival-Date:   Tue Nov 14 22:00:01 PST 2000
| >Closed-Date:
| >Last-Modified:
| >Originator:     Gerhard Sittig
| >Release:        FreeBSD 4.1-STABLE i386
| >Organization:
| in private
| >Environment:
|
| adduser(8), rmuser(8), pw(8) administrative commands
| and usernames with non-alphanumeric characters in them

Please commit this patch, we require this for I18N support.
I have changes that are similiar to these changes.  Then we
can do other stuff that I plan to change.

--
+------------------------------------------------------------------+
| keichii@peorth.iteration.net         | keichii@bsdconspiracy.net |
| http://peorth.iteration.net/~keichii | Yes, BSD is a conspiracy. |
+------------------------------------------------------------------+

To create some kind of "link" and to keep some kind of state or
progress in the audit trail:  I understand this PR is heavily
connected to bin/31049 (dots in usernames) and yar@freebsd.org
tried to attack the issue in revs 1.51 and 1.52 of
src/usr.sbin/adduser/adduser.perl as of 2002-01-02.

Doug Barton fixed src/usr.sbin/adduser/rmuser.perl so it has
been removing (well, "has been able to remove" might be better
wording:) any existing user since rev 1.12 as of 2000-12-17.

Yar Tikhi yesterday taught adduser.perl about an option to force
the administrator's wish onto the system what the name should
look like.  I have to state two points here:  I'm not completely
sure about which characters are allowed in usernames while PR
bin/31049 cites POSIX and talks about "characters from the
portable filename character set".  So I cannot decide if the
current implementation (short circuit *any* test except for
"there must be a username" and "it must not contain a colon"
because of the passwd(5) format) is going too far and would
allow invalid usernames.  And I feel that the wording of the
warn messages is too strong.  I would talk about "unexpected"
or "not recommended" characters unless I'm sure they really
_are_ invalid characters.  BTW:  Should there be a length check
like in pw(8)?

src/usr.sbin/pw/* hasn't been touched yet.  But I take it from
the bin/31049 audit trail that yar is aware of this tool, too,
and tries to sync both the Perl and the C version of the user
database manipulating frontends -- once it's determined what a
username is allowed to look like.


virtually yours   82D1 9B9C 01DC 4FB4 D7B4  61BE 3F49 4F77 72DE DA76
Gerhard Sittig   true | mail -s "get gpg key" Gerhard.Sittig@gmx.net
-- 
     If you don't understand or are scared by any of the above
             ask your parents or an adult to help you.

Comment 4 Yar Tikhiy 2002-01-04 22:15:10 UTC

On Thu, Jan 03, 2002 at 10:07:50PM +0100, Gerhard Sittig wrote:
> 
> Yar Tikhi yesterday taught adduser.perl about an option to force
> the administrator's wish onto the system what the name should
> look like.  I have to state two points here:  I'm not completely
> sure about which characters are allowed in usernames while PR
> bin/31049 cites POSIX and talks about "characters from the
> portable filename character set".  So I cannot decide if the
> current implementation (short circuit *any* test except for
> "there must be a username" and "it must not contain a colon"
> because of the passwd(5) format) is going too far and would
> allow invalid usernames.  And I feel that the wording of the
> warn messages is too strong.  I would talk about "unexpected"
> or "not recommended" characters unless I'm sure they really
> _are_ invalid characters.

The problem is that there are no distinct grades of validity for
characters to use in a username.  I did some simple tests that
showed the base system could treat a number of weird characterd
such as the whitespace, asterisk or dollar sign without any troubles.
On the other hand, a poorly written program or (which is more
likely) shell script may be tricked into doing illegitimate actions
using such unusual usernames.  Someone would say even period is no
good in usernames due to the NIS or Kerberos issues.

As an alternative to the "allow anything the admin wants" solution,
I would propose to make the regular expression usernames are checked
against configurable and saved in /etc/adduser.conf.

> BTW:  Should there be a length check like in pw(8)?

There is one in adduser.perl; however, it's not in the obvious
place: it's in "sub new_users_name" instead of "sub
new_users_name_valid."
 
> src/usr.sbin/pw/* hasn't been touched yet.  But I take it from
> the bin/31049 audit trail that yar is aware of this tool, too,
> and tries to sync both the Perl and the C version of the user
> database manipulating frontends -- once it's determined what a
> username is allowed to look like.

pw has a maintainer: David Nugent (davidn.) I wrote him an email
asking for help in this issue.  Honestly, adduser has a maintainer
(wosch,) too, but its commit log lists so many FreeBSD folks
modifying it without any sign of approval from wosch... :-)

-- 
Yar

On Fri, 04 Jan 2002 15:10:01 PST, Yar Tikhiy wrote:

>  As an alternative to the "allow anything the admin wants" solution,
>  I would propose to make the regular expression usernames are checked
>  against configurable and saved in /etc/adduser.conf.

I like this idea.

Either that, or do something like what passwd(1) does with weak new
passwords root gives it. passwd(1) will warn against the offered
password (e.g. "Please enter a password at least 6 characters in
length") but will accept the new password if it is repeated.

Ciao,
Sheldon.

Comment 6 Yar Tikhiy 2002-01-17 00:34:58 UTC

On Wed, Jan 16, 2002 at 05:08:23PM +0200, Sheldon Hearn wrote:
> 
> On Fri, 04 Jan 2002 15:10:01 PST, Yar Tikhiy wrote:
> 
> >  As an alternative to the "allow anything the admin wants" solution,
> >  I would propose to make the regular expression usernames are checked
> >  against configurable and saved in /etc/adduser.conf.
> 
> I like this idea.

Ok, I've implemented that instead of the -force option.
Would you mind trying the attached adduser.perl patch (it's against
-current) and telling about your impression?

-- 
Yar

Index: adduser.perl
===================================================================
RCS file: /home/ncvs/src/usr.sbin/adduser/adduser.perl,v
retrieving revision 1.53
diff -u -r1.53 adduser.perl
--- adduser.perl	4 Jan 2002 21:28:32 -0000	1.53
+++ adduser.perl	17 Jan 2002 00:29:43 -0000
@@ -30,7 +30,8 @@
 # read variables
 sub variables {
     $verbose = 1;		# verbose = [0-2]
-    $force = 0;			# relax username validity check if true
+    $usernameregexp = "^[a-z0-9_][a-z0-9_-]*\$"; # configurable
+    $defaultusernameregexp = $usernameregexp; # remains constant
     $defaultusepassword = "yes";	# use password authentication for new users
     $defaultenableaccount = "yes"; # enable the account by default
     $defaultemptypassword = "no"; # don't create an empty password
@@ -315,7 +316,7 @@
     local($name);
 
     while(1) {
-	$name = &confirm_list("Enter username", 1, "a-z0-9_-", "");
+	$name = &confirm_list("Enter username", 1, $usernameregexp, "");
 	if (length($name) > 16) {
 	    warn "Username is longer than 16 chars\a\n";
 	    next;
@@ -328,29 +329,20 @@
 sub new_users_name_valid {
     local($name) = @_;
 
-    if ($force) {
-	if ($name eq "a-z0-9_-") {
-	    warn "Please enter a username.\a\n";
-	    return 0;
-	}
-	if ($name =~ /[:\n]/) {
-	    warn "Illegal username, which would break your passwd file.\a\n";
-	    return 0;
+    if ($name eq $usernameregexp) {
+	warn "Please enter a username\a\n";
+	return 0;
+    } elsif ($name !~ /$usernameregexp/) {
+	if ($usernameregexp eq $defaultusernameregexp) {
+	    warn "Illegal username.\n" .
+		"Please use only lowercase Roman, decimal, underscore, " .
+		"or hyphen characters.\n" .
+		"Additionally, a username should not start with a hyphen.\a\n";
+	} else {
+	    warn "Username doesn't match the regexp /$usernameregexp/\a\n";
 	}
-	if ($name !~ /^[a-z0-9_][a-z0-9_\-]*$/) {
-	    warn "Caution: Username contains illegal characters.\n" .
-		"Adding this user may cause utilities " .
-		"or applications to malfunction,\n" .
-		"or even impose a security risk on your system.\a\n";
-	}
-    } elsif ($name !~ /^[a-z0-9_][a-z0-9_\-]*$/ || $name eq "a-z0-9_-") {
-	warn "Illegal username.\n" .
-	    "Please use only lowercase Roman, decimal, underscore, " .
-	    "or hyphen characters.\n" .
-	    "Additionally, a username should not start with a hyphen.\a\n";
 	return 0;
-    }
-    if ($username{$name}) {
+    } elsif ($username{$name}) {
 	warn "Username ``$name'' already exists!\a\n"; return 0;
     }
     return 1;
@@ -878,7 +870,6 @@
     [-class login_class]
     [-config_create]
     [-dotdir dotdir]
-    [-f|-force]
     [-group login_group]
     [-h|-help]
     [-home home]
@@ -952,7 +943,6 @@
 	if    (/^--?(v|verbose)$/)	{ $verbose = 1 }
 	elsif (/^--?(s|silent|q|quiet)$/)  { $verbose = 0 }
 	elsif (/^--?(debug)$/)	    { $verbose = 2 }
-	elsif (/^--?(f|force)$/)	{ $force = 1 }
 	elsif (/^--?(h|help|\?)$/)	{ &usage }
 	elsif (/^--?(home)$/)	 { $home = $argv[0]; shift @argv }
 	elsif (/^--?(shell)$/)	 { $defaultshell = $argv[0]; shift @argv }
@@ -1223,6 +1213,21 @@
     return 0;
 }
 
+# allow configuring usernameregexp
+sub usernameregexp_default {
+    local($r) = $usernameregexp;
+
+    while ($verbose) {
+	$r = &confirm_list("Usernames must match regular expression:", 1,
+	    $r, "");
+	eval "'foo' =~ /$r/";
+	last unless $@;
+	warn "Invalid regular expression\a\n";
+    }
+    $changes++ if $r ne $usernameregexp;
+    return $r;
+}
+
 # test if $dotdir exist
 # return "no" if $dotdir not exist or dotfiles should not copied
 sub dotdir_default {
@@ -1438,6 +1443,10 @@
 # verbose = [0-2]
 verbose = $verbose
 
+# regular expression usernames are checked against (see perlre(1))
+# usernameregexp = 'regexp'
+usernameregexp = '$usernameregexp'
+
 # use password-based authentication for new users
 # defaultusepassword =  "yes" | "no"
 defaultusepassword = "$defaultusepassword"
@@ -1522,6 +1531,7 @@
 
 # interactive
 # some questions
+$usernameregexp = &usernameregexp_default; # regexp to check usernames against
 &shells_add;			# maybe add some new shells
 $defaultshell = &shell_default;	# enter default shell
 $home = &home_partition($home);	# find HOME partition

On Thu, 17 Jan 2002 03:34:58 +0300, Yar Tikhiy wrote:

> Ok, I've implemented that instead of the -force option.
> Would you mind trying the attached adduser.perl patch (it's against
> -current) and telling about your impression?

Hi Yar,

Sorry about the delay.  I don't have time to test this properly at the
moment.  All I can tell you is that the patch looks okay, although
adding a comment that explains the odd return values would be a good
idea. :-)

Ciao,
Sheldon.

Comment 8 Sheldon Hearn 2002-01-28 13:35:11 UTC

Responsible Changed
From-To: freebsd-bugs->yar

Over to the guy with the patch. :-)

Adduser in -current and -stable can be now configured to
allow usernames matching a desirable regular expression.
I hope I will be able to do something about pw(8) some day.
If someone comes with a solution for pw(8) before that, I'll
pass this PR to him happily.

Comment 10 Garance A Drosehn 2003-01-28 02:36:23 UTC

State Changed
From-To: open->closed

I have installed a change to pw/pw_user.c which should address 
the last piece of this PR.  The change is only in freebsd-current, 
but I assume that will be good enough. 

Comment 11 Garance A Drosehn 2003-01-28 02:36:23 UTC

Responsible Changed
From-To: yar->gad

I have installed a change to pw/pw_user.c which should address 
the last piece of this PR.  The change is only in freebsd-current, 
but I assume that will be good enough.