Document new options and new configuration setup.

README

@@ -1,6 +1,9 @@
 This is tftp-hpa-0.17; this version was put out by H. Peter Anvin
 <hpa@zytor.com>.
 
+===> IMPORTANT: SEE THE FILE "README.security" FOR IMPORTANT SECURITY
+===> CHANGES IN THIS VERSION!!!!!!!!!
+
 Changes in 0.17:
 
 	Add support for tcpwrapper checking (/etc/hosts.allow;
@@ -8,6 +11,15 @@ Changes in 0.17:
 
 	Compile correctly on glibc 2.1.2.
 
+	Add -u option to specify the user id to run as (default
+	"nobody".)
+
+	Operate in "daemon mode" as long as we keep getting requests.
+	This should speed up handling large amounts of requests at
+	once, as can happen when a client starts up, and avoids inetd
+	misconfiguration problems.
+	
+
 Changes in 0.16:
 
 	Correct massive lossage from 0.15: apparently 0.15 was based

README.security

@@ -0,0 +1,33 @@
+Starting in version 0.17, tftp-hpa operates in genuine "wait" mode,
+which means that an in.tftpd process hangs around for some time after
+the last service request has arrived.  This speeds up servicing a
+subsequent request, which apparently has been a problem in the past,
+resulting in "request storms" as the client keeps retrying, resulting
+in multiple connections on the server which the client has already
+abandoned.
+
+This also means that spawning tftp via tcpd is useless (in fact, this
+indirection seems to be part of the reason for these "request
+storms.")  Instead, tftp-hpa supports calling the tcpwrapper library
+directly.  Thus, if your /etc/inetd.conf looks like this (all on one
+line):
+
+tftp	dgram	udp	wait	root	/usr/sbin/tcpd
+/usr/sbin/in.tftpd -s /tftpboot -r blksize
+
+... it's better to change to ...
+
+tftp	dgram	udp	wait	root	/usr/sbin/in.tftpd
+in.tftpd -s /tftpboot -r blksize
+
+You should make sure that you are using "wait" option in tftpd; you
+also need to have tftpd spawned as root in order for chroot (-s) to
+work.  tftpd automatically drops privilege and changes user ID to
+"nobody" by default; the appropriate user ID for tftpd can be
+specified with the -u option (e.g. "-u tftpuser").
+
+If you are running a very busy boot server in a secure (firewalled!)
+configuration, you may want to compile tftpd without tcpwrapper
+support, in order to provide significantly better performance.  To do
+so, specify the --without-tcpwrappers option to configure when
+compiling.

tftpd/tftpd.8

@@ -1,3 +1,4 @@
+.\"   tftp-hpa: $Id$
 .\"   $OpenBSD: tftpd.8,v 1.7 1999/07/09 13:35:51 aaron Exp $
 .\"
 .\" Copyright (c) 1983, 1991 The Regents of the University of California.
@@ -34,18 +35,19 @@
 .\"	from: @(#)tftpd.8	6.7 (Berkeley) 5/13/91
 .\"	$OpenBSD: tftpd.8,v 1.7 1999/07/09 13:35:51 aaron Exp $
 .\"
-.Dd July 9, 2000
+.Dd Apr 23, 2001
 .Dt TFTPD 8
 .Os
 .Sh NAME
 .Nm tftpd
 .Nd
-.Tn DARPA
+IPv4 Trivial File Transfer Protocol server
-Trivial File Transfer Protocol server
 .Sh SYNOPSIS
-.Nm tftpd
+.Nm in.tftpd
-.Op Fl cs
+.Op Fl c
+.Op Fl u Ar userid
 .Op Fl r Ar option...
+.Op Fl s
 .Op Ar directory
 .Sh DESCRIPTION
 .Nm
@@ -78,7 +80,10 @@ to include
 all users on all hosts that can be reached through the network;
 this may not be appropriate on all systems, and its implications
 should be considered before enabling tftp service.
-The server should have the user ID with the lowest possible privilege.
+The server should have the user ID with the lowest possible privilege
+(see the
+.Fl u
+flag below.)
 .Pp
 Access to files may be restricted by invoking
 .Nm
@@ -102,9 +107,16 @@ flag with a directory name,
 .Nm
 will
 .Xr chroot 2
-on startup; therefore the remote host is not expected to pass the directory
+on startup; therefore the remote host is not expected to pass the
-as part of the file name to transfer.  This option is intended primarily for
+directory as part of the file name to transfer.  This option is
-compatibility with SunOS boot ROMs which do not include a directory name.
+recommended for security, as well as compatibility with boot ROMs
+which do not include a directory name.
+.Pp
+The
+.Fl u
+flag can be used to specify a user ID which
+.Nm
+will run as; the default is ``nobody''.
 .Pp
 This version of
 .Nm
@@ -139,3 +151,7 @@ The
 .Fl r
 flag and RFC 2347 options were added by H. Peter Anvin based on
 patches by Markus Gutschke and Gero Kulhman.
+.Pp
+The
+.Fl u
+flag was added by H. Peter Anvin.

tftpd/tftpd.c

@@ -53,6 +53,7 @@ static const char *rcsid = "tftp-hpa $Id$";
 #include <sys/types.h>
 #include <sys/ioctl.h>
 #include <sys/stat.h>
+#include <sys/time.h>
 #include <signal.h>
 #include <fcntl.h>
 
@@ -154,7 +155,7 @@ struct options {
 static void
 usage(void)
 {
-	syslog(LOG_ERR, "Usage: %s [-c] [-u user] [-r option...] [-s] [directory ...]",
+	syslog(LOG_ERR, "Usage: %s [-c] [-u user] [-t timeout] [-r option...] [-s] [directory ...]",
 	       __progname);
 	exit(1);
 }
@@ -170,16 +171,16 @@ main(int argc, char **argv)
 	int on = 1;
 	int fd = 0;
 	int pid;
-	int i, j;
 	int c;
 	int setrv;
+	int timeout = 900;	/* Default timeout */
 	char *user = "nobody";	/* Default user */
 
 	__progname = basename(argv[0]);
 
 	openlog(__progname, LOG_PID | LOG_NDELAY, LOG_DAEMON);
 
-	while ((c = getopt(argc, argv, "csu:r:")) != -1)
+	while ((c = getopt(argc, argv, "csu:r:t:")) != -1)
 		switch (c) {
 		case 'c':
 			cancreate = 1;
@@ -187,6 +188,9 @@ main(int argc, char **argv)
 		case 's':
 			secure = 1;
 			break;
+		case 't':
+		  timeout = atoi(optarg);
+		  break;
 		case 'u':
 		  user = optarg;
 		  break;
@@ -248,10 +252,27 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
+	/* This means we don't want to wait() for children */
+	bsd_signal(SIGCHLD, SIG_IGN);
+
+	do {
+	  fd_set readset;
+	  struct timeval tv_timeout;
+	  
+
+	  FD_ZERO(&readset);
+	  FD_SET(fd, &readset);
+	  tv_timeout.tv_sec = timeout;
+	  tv_timeout.tv_usec = 0;
+
+	  if ( select(fd+1, &readset, NULL, NULL, &tv_timeout) == 0 )
+	    exit(0);		/* Timeout, return to inetd */
+
 	  fromlen = sizeof (from);
 	  n = myrecvfrom(fd, buf, sizeof (buf), 0,
 			 (struct sockaddr *)&from, &fromlen,
 			 &myaddr);
+
 	  if (n < 0) {
 	    syslog(LOG_ERR, "recvfrom: %m");
 	    exit(1);
@@ -278,7 +299,22 @@ main(int argc, char **argv)
 	  }
 #endif
 
-	/* Drop privileges */
+	/*
+	 * Now that we have read the message out of the UDP
+	 * socket, we fork and go back to listening to the
+	 * socket.
+	 */
+	  pid = fork();
+	  if (pid < 0) {
+	    syslog(LOG_ERR, "fork: %m");
+	    exit(1);		/* Return to inetd, just in case */
+	  }
+	} while ( pid > 0 );	/* Parent process continues... */
+
+	/* Child process: handle the actual request here */
+
+	/* Chroot and drop privileges */
+
 	if (secure && chroot(".")) {
 		syslog(LOG_ERR, "chroot: %m");
 		exit(1);
@@ -303,55 +339,15 @@ main(int argc, char **argv)
 	  exit(1);
 	}
 
-	/*
+	/* Close file descriptors we don't need */
-	 * Now that we have read the message out of the UDP
-	 * socket, we fork and exit.  Thus, inetd will go back
-	 * to listening to the tftp port, and the next request
-	 * to come in will start up a new instance of tftpd.
-	 *
-	 * We do this so that inetd can run tftpd in "wait" mode.
-	 * The problem with tftpd running in "nowait" mode is that
-	 * inetd may get one or more successful "selects" on the
-	 * tftp port before we do our receive, so more than one
-	 * instance of tftpd may be started up.  Worse, if tftpd
-	 * break before doing the above "recvfrom", inetd would
-	 * spawn endless instances, clogging the system.
-	 */
-	for (i = 1; i < 20; i++) {
-		pid = fork();
-		if (pid < 0) {
-			sleep(i);
-			/*
-			 * flush out to most recently sent request.
-			 *
-			 * This may drop some request, but those
-			 * will be resent by the clients when
-			 * they timeout.  The positive effect of
-			 * this flush is to (try to) prevent more
-			 * than one tftpd being started up to service
-			 * a single request from a single client.
-			 */
-			j = sizeof from;
-			i = myrecvfrom(fd, buf, sizeof (buf), 0,
-			    (struct sockaddr *)&from, &j, &myaddr);
-			if (i > 0) {
-				n = i;
-				fromlen = j;
-			}
-		} else
-			break;
-	}
-	if (pid < 0) {
-		syslog(LOG_ERR, "fork: %m");
-		exit(1);
-	} else if (pid != 0)
-		exit(0);
 
 	from.sin_family = AF_INET;
 	alarm(0);
 	close(fd);
 	close(1);
 
+	/* Process the request... */
+
 	peer = socket(AF_INET, SOCK_DGRAM, 0);
 	if (peer < 0) {
 		syslog(LOG_ERR, "socket: %m");