Re: bin/49868: tftpd(8) doesn't play well with clients that return acknowledgements to the broadcast address

[Brian Buhrow <buhrow%nfbcal.org@localhost>]

The following reply was made to PR bin/49868; it has been noted by GNATS.

From: Brian Buhrow <buhrow%nfbcal.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, gnats-admin%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Cc: buhrow%nfbcal.org@localhost
Subject: Re: bin/49868: tftpd(8) doesn't play well with clients that return acknowledgements to the broadcast address
Date: Thu, 30 Apr 2015 12:20:10 -0700

 	Hello.  Here are  patches which implement Christos request, as well as
 documenting the change.  Any reason I shouldn't commit these?
 
 -thanks
 -Brian
 
 Index: tftpd.8
 ===================================================================
 RCS file: /cvsroot/src/libexec/tftpd/tftpd.8,v
 retrieving revision 1.28
 diff -u -r1.28 tftpd.8
 --- tftpd.8	29 Apr 2010 21:34:04 -0000	1.28
 +++ tftpd.8	30 Apr 2015 18:57:10 -0000
 @@ -1,4 +1,4 @@
 -.\"	$NetBSD$
 +.\"	$NetBSD: tftpd.8,v 1.28 2010/04/29 21:34:04 wiz Exp $
  .\"
  .\" Copyright (c) 1983, 1991, 1993
  .\"	The Regents of the University of California.  All rights reserved.
 @@ -39,7 +39,7 @@
  Internet Trivial File Transfer Protocol server
  .Sh SYNOPSIS
  .Nm
 -.Op Fl cdln
 +.Op Fl bcdln
  .Op Fl g Ar group
  .Op Fl p Ar pathsep
  .Op Fl s Ar directory
 @@ -95,6 +95,12 @@
  .Pp
  The options are:
  .Bl -tag -width "XsXdirectoryX"
 +.It Fl b
 +Allow clients which return acknowledgements to the broadcast address to
 +communicate with the tftp server.
 +Some tftp clients, notably ones resident in the ROMs of older Cisco
 +equipment, return their acknowledgements to the broadcast address rather
 +than the server's unicast address.  
  .It Fl c
  Allow unrestricted creation of new files.
  Without this flag, only existing publicly writable files can be overwritten.
 
 Index: tftpd.c
 ===================================================================
 RCS file: /cvsroot/src/libexec/tftpd/tftpd.c,v
 retrieving revision 1.43
 diff -u -r1.43 tftpd.c
 --- tftpd.c	4 Oct 2013 07:51:48 -0000	1.43
 +++ tftpd.c	30 Apr 2015 18:57:30 -0000
 @@ -1,4 +1,4 @@
 -/*	$NetBSD$	*/
 +/*	$NetBSD: tftpd.c,v 1.31.4.1 2011/11/02 19:58:23 riz Exp $	*/
  
  /*
   * Copyright (c) 1983, 1993
 @@ -36,7 +36,7 @@
  #if 0
  static char sccsid[] = "@(#)tftpd.c	8.1 (Berkeley) 6/4/93";
  #else
 -__RCSID("$NetBSD$");
 +__RCSID("$NetBSD: tftpd.c,v 1.43 2013/10/04 07:51:48 jnemeth Exp $");
  #endif
  #endif /* not lint */
  
 @@ -110,6 +110,7 @@
  static char	pathsep = '\0';
  static char	*securedir;
  static int	unrestricted_writes;    /* uploaded files don't have to exist */
 +static int	broadcast_client = 0; /* Some clients ack to the broadcast address */
  
  struct formats;
  
 @@ -142,7 +143,7 @@
  {
  
  	syslog(LOG_ERR,
 -    "Usage: %s [-cdln] [-g group] [-p pathsep] [-s directory] [-u user] [directory ...]",
 +    "Usage: %s [-bcdln] [-g group] [-p pathsep] [-s directory] [-u user] [directory ...]",
  		    getprogname());
  	exit(1);
  }
 @@ -172,8 +173,11 @@
  	curuid = getuid();
  	curgid = getgid();
  
 -	while ((ch = getopt(argc, argv, "cdg:lnp:s:u:")) != -1)
 +	while ((ch = getopt(argc, argv, "bcdg:lnp:s:u:")) != -1)
  		switch (ch) {
 +		case 'b':
 +			broadcast_client = 1;
 +			break;
  		case 'c':
  			unrestricted_writes = 1;
  			break;
 @@ -393,14 +397,17 @@
  		syslog(LOG_ERR, "socket: %m");
  		exit(1);
  	}
 +	if (broadcast_client) {
 +		soopt = 1;
 +		if (setsockopt(peer, SOL_SOCKET, SO_BROADCAST, (void *) &soopt, sizeof(soopt)) < 0) {
 +			syslog(LOG_ERR, "set SO_BROADCAST: %m");
 +			exit(1);
 +		}
 +	}
  	if (bind(peer, (struct sockaddr *)&me, me.ss_len) < 0) {
  		syslog(LOG_ERR, "bind: %m");
  		exit(1);
  	}
 -	if (connect(peer, (struct sockaddr *)&from, from.ss_len) < 0) {
 -		syslog(LOG_ERR, "connect: %m");
 -		exit(1);
 -	}
  	soopt = 65536;	/* larger than we'll ever need */
  	if (setsockopt(peer, SOL_SOCKET, SO_SNDBUF, (void *) &soopt, sizeof(soopt)) < 0) {
  		syslog(LOG_ERR, "set SNDBUF: %m");
 @@ -955,7 +962,7 @@
  send_data:
  		if (!etftp && debug)
  			syslog(LOG_DEBUG, "Send DATA %u", block);
 -		if ((n = send(peer, dp, size + 4, 0)) != size + 4) {
 +		if ((n = sendto(peer, dp, size + 4, 0, (struct sockaddr *)&from, fromlen)) != size + 4) {
  			syslog(LOG_ERR, "tftpd: write: %m");
  			goto abort;
  		}
 @@ -963,7 +970,8 @@
  			read_ahead(file, tftp_blksize, pf->f_convert);
  		for ( ; ; ) {
  			alarm(rexmtval);        /* read the ack */
 -			n = recv(peer, ackbuf, tftp_blksize, 0);
 +			n = recvfrom(peer, ackbuf, tftp_blksize, 0,(struct sockaddr
 +			*)&from, &fromlen );
  			alarm(0);
  			if (n < 0) {
  				syslog(LOG_ERR, "tftpd: read: %m");
 @@ -1049,14 +1057,15 @@
  		(void) setjmp(timeoutbuf);
  send_ack:
  		ap = (struct tftphdr *) (etftp ? oackbuf : ackbuf);
 -		if (send(peer, ap, acklength, 0) != acklength) {
 +		if (sendto(peer, ap, acklength, 0, (struct sockaddr *)&from, fromlen) != acklength) {
  			syslog(LOG_ERR, "tftpd: write: %m");
  			goto abort;
  		}
  		write_behind(file, pf->f_convert);
  		for ( ; ; ) {
  			alarm(rexmtval);
 -			n = recv(peer, dp, tftp_blksize + 4, 0);
 +			n = recvfrom(peer, dp, tftp_blksize + 4, 0, (struct sockaddr
 +			*)&from, &fromlen);
  			alarm(0);
  			if (n < 0) {            /* really? */
  				syslog(LOG_ERR, "tftpd: read: %m");
 @@ -1108,16 +1117,16 @@
  	ap->th_block = htons((u_short)(block));
  	if (debug)
  		syslog(LOG_DEBUG, "Send final ACK %u", block);
 -	(void) send(peer, ackbuf, 4, 0);
 +	(void) sendto(peer, ackbuf, 4, 0, (struct sockaddr *)&from, fromlen);
  
  	signal(SIGALRM, justquit);      /* just quit on timeout */
  	alarm(rexmtval);
 -	n = recv(peer, buf, sizeof (buf), 0); /* normally times out and quits */
 +	n = recvfrom(peer, buf, sizeof (buf), 0, (struct sockaddr *)&from, &fromlen); /* normally times out and quits */
  	alarm(0);
  	if (n >= 4 &&                   /* if read some data */
  	    dp->th_opcode == DATA &&    /* and got a data block */
  	    block == dp->th_block) {	/* then my last ack was lost */
 -		(void) send(peer, ackbuf, 4, 0);     /* resend final ack */
 +		(void) sendto(peer, ackbuf, 4, 0, (struct sockaddr *)&from, fromlen);     /* resend final ack */
  	}
  abort:
  	return;
 @@ -1185,7 +1194,7 @@
  		syslog(LOG_DEBUG, "Send NACK %s", tp->th_msg);
  	length = strlen(tp->th_msg);
  	msglen = &tp->th_msg[length + 1] - buf;
 -	if (send(peer, buf, msglen, 0) != (ssize_t)msglen)
 +	if (sendto(peer, buf, msglen, 0, (struct sockaddr *)&from, fromlen) != (ssize_t)msglen)
  		syslog(LOG_ERR, "nak: %m");
  }