Subject: misc/36063: teach /etc/security about duplicate user accounts
To: None <misc-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: None <j+nbsd@2007.salmi.ch>
List: netbsd-bugs
Date: 03/23/2007 17:15:00
>Number:         36063
>Category:       misc
>Synopsis:       teach /etc/security about duplicate user accounts
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    misc-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 23 17:15:00 +0000 2007
>Originator:     Jukka Salmi
>Release:        NetBSD 4.99.13
>Environment:
System: NetBSD moray.salmi.ch 4.99.13 NetBSD 4.99.13 (MORAY.APM) #0: Thu Mar 8 14:20:43 CET 2007 build@moray.salmi.ch:/build/nbsd/i386/sys/arch/i386/compile/MORAY.APM i386
Architecture: i386
Machine: i386
>Description:
Running /etc/security on a system where one or more user accounts have
a "fallback account" (like `toor' is a fallback account for `root')
produces warnings like:

	/etc/master.passwd has duplicate user id's.
	jukka 1010      akkuj 1010
	
	Checking home directories.
	user akkuj home directory is owned by jukka
	
	Checking dot files.
	user akkuj .cshrc file is owned by jukka
	user akkuj .k5login file is owned by jukka
	[...]

For the `toor' account there are no warnings printed; consequently,
there should be no warnings for other fallback accounts.

>How-To-Repeat:
Add a user account with the same UID and home directory as an already
existing user (but with a different username...), run /etc/security
and see warnings like those shown above.

>Fix:
The following patch adds a configuration variable named
`duplicate_accounts' to security.conf(5). This variable can be set to
a comma seperated list of user accounts to be ignored by /etc/security
during the `check_passwd' phase and in the file ownership and permission
checks during the check_homes phase. By default it is set to `toor',
thus the patch doesn't change the default behaviour. However, I slightly
changed the output formatting of the `check_passwd' part: instead of
e.g.

	root 0    toor 0    jdoe 1000    foo 1000    bar 10000

now it would print

	uid 0: root, toor
	uid 1000: jdoe, foo, bar

because I think that's easier to parse for a human...

The patch is also available from
http://salmi.ch/~jukka/patches/nbsd/HEAD/etc/security_dups.patch

Index: etc/defaults/security.conf
===================================================================
RCS file: /cvsroot/src/etc/defaults/security.conf,v
retrieving revision 1.18
diff -u -p -r1.18 security.conf
--- etc/defaults/security.conf	25 May 2006 02:38:10 -0000	1.18
+++ etc/defaults/security.conf	23 Mar 2007 08:29:44 -0000
@@ -42,3 +42,4 @@ check_passwd_permit_star=NO
 check_passwd_permit_nonalpha=NO
 max_loginlen=16
 max_grouplen=16
+duplicate_accounts="toor"
Index: etc/security
===================================================================
RCS file: /cvsroot/src/etc/security,v
retrieving revision 1.100
diff -u -p -r1.100 security
--- etc/security	26 Sep 2006 08:32:40 -0000	1.100
+++ etc/security	23 Mar 2007 08:29:45 -0000
@@ -265,17 +265,26 @@ if checkyesno check_passwd; then
 		column $OUTPUT
 	fi
 
-# To not exclude 'toor', a standard duplicate root account, from the duplicate
-# account test, uncomment the line below (without egrep in it)and comment
-# out the line (with egrep in it) below it.
-#
-#	< $MPBYUID uniq -d -f 1 | awk '{ print $2 }' > $TMP2
-	< $MPBYUID egrep -v '^toor ' | uniq -d -f 1 | awk '{ print $2 }' > $TMP2
+	awk -v "dupaccounts=$duplicate_accounts" \
+	'BEGIN {
+		split(dupaccounts, a, ",");
+		for (i in a) ignore[a[i]]++;
+	}
+	{
+		if ($1 in ignore) next;
+		if ($2 in accounts)
+			dups[$2] = (($2 in dups) ? dups[$2] ", " : "") $1;
+		else
+			accounts[$2] = $1;
+	}
+	END {
+		for (uid in dups)
+			printf "uid %d: %s, %s\n", \
+				uid, accounts[uid], dups[uid];
+	}' < $MPBYUID > $TMP2
 	if [ -s $TMP2 ] ; then
-		printf "\n$MP has duplicate user id's.\n"
-		while read uid; do
-			grep -w $uid $MPBYUID
-		done < $TMP2 | column
+		printf "\n$MP has duplicate user ids.\n"
+		cat $TMP2
 	fi
 fi
 
@@ -492,8 +501,13 @@ if checkyesno check_homes; then
 			printf -- "$uid $file\n"
 		fi
 	done < $MPBYPATH |
-	awk -v "usergroups=$permit_usergroups" '
-	     $1 != $4 && $4 != "root" \
+	awk -v "usergroups=$permit_usergroups" \
+	    -v "dupaccounts=$duplicate_accounts" '
+	     BEGIN {
+		split(dupaccounts, a, ",");
+		for (i in a) ignore[a[i]]++;
+	     }
+	     $1 != $4 && $4 != "root" && !($1 in ignore) \
 		{ print "user " $1 " home directory is owned by " $4 }
 	     $2 ~ /^-....w/ && (!usergroups || $5 != $1) \
 		{ print "user " $1 " home directory is group writable" }
@@ -515,8 +529,13 @@ if checkyesno check_homes; then
 			fi
 		done
 	done < $MPBYPATH |
-	awk  -v "usergroups=$permit_usergroups" '
-	     $1 != $5 && $5 != "root" \
+	awk  -v "usergroups=$permit_usergroups" \
+	     -v "dupaccounts=$duplicate_accounts" '
+	     BEGIN {
+		split(dupaccounts, a, ",");
+		for (i in a) ignore[a[i]]++;
+	     }
+	     $1 != $5 && $5 != "root" && !($1 in ignore) \
 		{ print "user " $1 " " $2 " file is owned by " $5 }
 	     $3 ~ /^-...r/ && (!usergroups || $6 != $1) \
 		{ print "user " $1 " " $2 " file is group readable" }
@@ -544,8 +563,13 @@ if checkyesno check_homes; then
 			fi
 		done
 	done < $MPBYPATH |
-	awk -v "usergroups=$permit_usergroups" '
-	     $1 != $5 && $5 != "root" \
+	awk -v "usergroups=$permit_usergroups" \
+	    -v "dupaccounts=$duplicate_accounts" '
+	     BEGIN {
+		split(dupaccounts, a, ",");
+		for (i in a) ignore[a[i]]++;
+	     }
+	     $1 != $5 && $5 != "root" && !($1 in ignore) \
 		{ print "user " $1 " " $2 " file is owned by " $5 }
 	     $3 ~ /^-....w/ && (!usergroups || $6 != $1) \
 		{ print "user " $1 " " $2 " file is group writable" }
Index: share/man/man5/security.conf.5
===================================================================
RCS file: /cvsroot/src/share/man/man5/security.conf.5,v
retrieving revision 1.31
diff -u -p -r1.31 security.conf.5
--- share/man/man5/security.conf.5	29 May 2006 22:07:25 -0000	1.31
+++ share/man/man5/security.conf.5	23 Mar 2007 08:29:45 -0000
@@ -226,6 +226,15 @@ is enabled, this determines the maximum 
 If
 .Sy check_passwd
 is enabled, this determines the maximum permitted length of login names.
+.It Sy duplicate_accounts
+A comma separated list of user accounts to ignore in the duplicate user
+ID check during the
+.Sy check_passwd
+phase and in the file ownership and permission checks during the
+.Sy check_homes
+phase.
+Defaults to
+.Dq toor .
 .It Sy backup_dir
 Change the backup directory from
 .Pa /var/backup .