>Number:         4061
>Category:       security
>Synopsis:       /etc/security check of /etc/ftpusers is incomplete and doesn't support full syntax
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Fri Aug 29 17:50:01 1997
>Last-Modified:
>Originator:     Jim Bernard
>Organization:
	speaking for myself
>Release:        August 29, 1997
>Environment:
System: NetBSD zoo 1.2G NetBSD 1.2G (ZOO) #0: Sat Jul 19 12:48:58 MDT 1997 jim@zoo:/jaz/home/local/compile/sys/arch/i386/compile/ZOO i386


>Description:
	The portion of /etc/security that checks /etc/ftpusers to verify that
	root and uucp users are denied access has two problems:
	  * It doesn't check for all root users (which it should, since the
	    OS is distributed with two root users in /etc/master.passwd).
	    Thus, on a default system, user "toor" may be granted ftp access.
	  * It is out of sync with the current syntax of the /etc/ftpusers
	    file as implemented in ftpd.  (PR 4046 contains a fix for this,
	    but the fix does not maintain backward compatibility with the
	    old syntax, which is still supported by ftpd.  A fully compatible
	    fix is provided below.)
>How-To-Repeat:
	* Try to gain ftp access as "toor" on a system with both "root" and
	  "toor" password entries and find that it works.
	* Read the /etc/security script and note the incompatibility with
	  the syntax described in ftpd(8).
>Fix:
	Since the general problem of implementing in a shell script the
	syntax supported by ftpd for /etc/ftpusers is difficult, and since
	it is important to track possible future changes in that syntax,
	I chose to use ftpd itself to do the checking.  The patches to ftpd
	implement a new option (-c user) that simply runs the same routine
	already used to check ftpusers for real connections.  This feature
	is then used in new /etc/security script code to check ftpusers.
	Also, there's a patch to add "toor" to ftpusers in the distribution.

	While making the changes to ftpd, I noted a couple of problems there:
	  * The getopt return code was checked against EOF (not -1)
	  * If the -a option is given without an argument, the session will
	    run chrooted to the standard directory (~ftp) rather than failing.
	    This could have unintended security-related consequences.
	These are fixed as well in the patch below.

	Here are the specific changes:

ftpd.c:
	- Moved arg-list processing up to top of main (otherwise there's
	  no way to get a check-access-and-exit option to work, since
	  there is no connection to set up).

	- Added "-c user" option to check access for "user" and exit
	  after suitable notification; /etc/ftpusers is checked, using
	  the same routine called in normal use, so the check is guaranteed
	  to be compatible with and in sync with the working code.

	- Added explicit check for option -a in case its argument is
	  omitted, so the program will exit, rather than running
	  chrooted to the wrong directory.

	- Test getopt return against -1, not EOF.

ftpd.8:
	- Added documentation for -c option.

/etc/security:
	- Check _all_ uid=0 users (e.g., both "root" and "toor")

	- Use new ftpd -c feature to check /etc/ftpusers with full
	  compatibility with the syntax implemented in ftpd

	- Print a heading if any problems are found--no output otherwise

/etc/ftpusers:
	- Added entry for "toor" (which is in the distributed master.passwd)


--- ftpd.c-dist	Wed Aug 27 05:18:37 1997
+++ ftpd.c	Fri Aug 29 06:59:39 1997
@@ -219,42 +219,23 @@
 {
 	int addrlen, ch, on = 1, tos;
 	char *cp, line[LINE_MAX];
 	FILE *fd;
 
-	/*
-	 * LOG_NDELAY sets up the logging connection immediately,
-	 * necessary for anonymous ftp's that chroot and can't do it later.
-	 */
-	openlog("ftpd", LOG_PID | LOG_NDELAY, LOG_FTP);
-	addrlen = sizeof(his_addr);
-	if (getpeername(0, (struct sockaddr *)&his_addr, &addrlen) < 0) {
-		syslog(LOG_ERR, "getpeername (%s): %m",argv[0]);
-		exit(1);
-	}
-	addrlen = sizeof(ctrl_addr);
-	if (getsockname(0, (struct sockaddr *)&ctrl_addr, &addrlen) < 0) {
-		syslog(LOG_ERR, "getsockname (%s): %m",argv[0]);
-		exit(1);
-	}
-#ifdef IP_TOS
-	tos = IPTOS_LOWDELAY;
-	if (setsockopt(0, IPPROTO_IP, IP_TOS, (char *)&tos, sizeof(int)) < 0)
-		syslog(LOG_WARNING, "setsockopt (IP_TOS): %m");
-#endif
-	data_source.sin_port = htons(ntohs(ctrl_addr.sin_port) - 1);
-	debug = 0;
-
-	/* set this here so klogin can use it... */
-	(void)snprintf(ttyline, sizeof(ttyline), "ftp%d", getpid());
-
-	while ((ch = getopt(argc, argv, "a:dlt:T:u:v")) != EOF) {
+	while ((ch = getopt(argc, argv, "a:c:dlt:T:u:v")) != -1) {
 		switch (ch) {
 		case 'a':
 			anondir = optarg;
 			break;
 
+		case 'c':
+			printf("/etc/ftpusers %s access by %s\n",
+				(checkaccess(optarg)) ? "denies" : "allows",
+				optarg);
+			exit(0);
+			break;
+
 		case 'd':
 		case 'v':		/* deprecated */
 			debug = 1;
 			break;
 
@@ -268,14 +249,42 @@
 			warnx("-%c has been deprecated in favour of ftpd.conf",
 			    ch);
 			break;
 
 		default:
+			if (optopt == 'a' || optopt == 'c') exit(1);
 			warnx("unknown flag -%c ignored", optopt);
 			break;
 		}
 	}
+
+	/*
+	 * LOG_NDELAY sets up the logging connection immediately,
+	 * necessary for anonymous ftp's that chroot and can't do it later.
+	 */
+	openlog("ftpd", LOG_PID | LOG_NDELAY, LOG_FTP);
+	addrlen = sizeof(his_addr);
+	if (getpeername(0, (struct sockaddr *)&his_addr, &addrlen) < 0) {
+		syslog(LOG_ERR, "getpeername (%s): %m",argv[0]);
+		exit(1);
+	}
+	addrlen = sizeof(ctrl_addr);
+	if (getsockname(0, (struct sockaddr *)&ctrl_addr, &addrlen) < 0) {
+		syslog(LOG_ERR, "getsockname (%s): %m",argv[0]);
+		exit(1);
+	}
+#ifdef IP_TOS
+	tos = IPTOS_LOWDELAY;
+	if (setsockopt(0, IPPROTO_IP, IP_TOS, (char *)&tos, sizeof(int)) < 0)
+		syslog(LOG_WARNING, "setsockopt (IP_TOS): %m");
+#endif
+	data_source.sin_port = htons(ntohs(ctrl_addr.sin_port) - 1);
+	debug = 0;
+
+	/* set this here so klogin can use it... */
+	(void)snprintf(ttyline, sizeof(ttyline), "ftp%d", getpid());
+
 	(void) freopen(_PATH_DEVNULL, "w", stderr);
 	(void) signal(SIGPIPE, lostconn);
 	(void) signal(SIGCHLD, SIG_IGN);
 	if ((long)signal(SIGURG, myoob) < 0)
 		syslog(LOG_ERR, "signal: %m");
--- ftpd.8-dist	Sat Jun 14 05:20:30 1997
+++ ftpd.8	Thu Aug 28 21:04:32 1997
@@ -42,10 +42,11 @@
 Internet File Transfer Protocol server
 .Sh SYNOPSIS
 .Nm
 .Op Fl dl
 .Op Fl a Ar anondir
+.Op Fl c Ar user
 .Sh DESCRIPTION
 .Nm
 is the
 Internet File Transfer Protocol
 server process.  The server uses the
@@ -61,10 +62,16 @@
 .It Fl a
 Define the directory to
 .Xr chroot 2
 into for anonymous logins.
 Default is the home directory for the ftp user.
+.It Fl c
+Check whether the specified user would be granted access under
+the restrictions given in
+.Pa /etc/ftpusers
+and exit without attempting a connection.  This can be useful for
+testing configurations.
 .It Fl d
 Debugging information is written to the syslog using LOG_FTP.
 .It Fl l
 Each successful and failed
 .Xr ftp 1
--- security-dist	Fri Aug 22 05:36:17 1997
+++ security	Fri Aug 29 17:58:32 1997
@@ -245,18 +245,24 @@
 		fi
 	fi
 fi
 
 # Root and uucp should both be in /etc/ftpusers.
-# XXX This should be updated to support the new format...
 if [ "$check_ftpusers" = YES ]; then
-	list="root uucp"
+	cp /dev/null $OUTPUT
+	root_users=`awk -F: '$3 == 0 { print $1 }' $MP`
+	list="$root_users uucp"
 	for i in $list; do
-		if ! egrep "^$i$" /etc/ftpusers > /dev/null ; then
-			printf "\n$i is not listed in /etc/ftpusers file.\n"
+		msg=`/usr/libexec/ftpd -c $i`
+		if echo $msg | grep -q "allows access"; then
+			printf "\t$msg.\n" >> $OUTPUT
 		fi
 	done
+	if [ -s $OUTPUT ]; then
+		printf "\nChecking the /etc/ftpusers configuration:\n"
+		cat $OUTPUT
+	fi
 fi
 
 # Uudecode should not be in the /etc/aliases file.
 if [ "$check_aliases" = YES ]; then
 	if egrep '^[^#]*(uudecode|decode).*\|' /etc/aliases; then
--- ftpusers-dist	Sun Apr  6 05:08:11 1997
+++ ftpusers	Fri Aug 29 18:17:51 1997
@@ -3,5 +3,6 @@
 # list of users denied (or allowed) ftp access
 # read by ftpd(8).
 root	deny
+toor	deny
 uucp	deny
 *	allow
>Audit-Trail:
>Unformatted: