Subject: ftpd patch, to handle OOB commands from yacc syntax
To: None <tech-userlevel@netbsd.org>
From: Aidan Cully <aidan@kublai.com>
List: tech-userlevel
Date: 03/25/2001 20:25:57
--IS0zKkzwUGydFO0o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

This is part one in my effort to add rfc2228 support to ftpd...  My plan
has been for the MIC, CONF and ENC commands to be handled directly in the
yacc syntax (as opposed to the way MIT did it, where they're handled in
the getline() function).  This became complicated with OOB commands,
since they don't go through the ordinary yacc chain, but still need to
have MIC/CONF/ENC all work...  This patch makes OOB commands go through
the yacc chain.  There shouldn't be any behavioural changes introduced by
this patch (except that OOB data can now be longer than 7 characters...
it will be when it's encrypted, after all.)

What I've done is make the .y file operate on a line at a time of input
(rather than the whole input stream at once), and then split out different
commands from the .y file into three sections: commands that can come at
any time (namely, MIC, CONF and ENC), those that can come when we're busy
transmitting (STAT and ABOR), and everything else.  "Everything else" also
includes STAT and ABOR, since they have defined actions, even when we're
not transmitting or receiving data.  The SIGURG handler reads in a line of
data, marks that the new data is OOB, and passes it through to yacc.

There is at least one possible complication with this patch, which is that
the yyparse() for the STOR/RETR will not have completed when it's called
for OOB data...  I don't know what kinds of problems this can cause...
It's possible that yacc knows it's done with the input at this point, as
it says it does single-token lookahead on input, and the next token is
guaranteed to be the end-of-input identifier...  I can get around this
problem(?) by deferring calls to store(), retrieve() and send_file_list()
'til the body of ftp_loop(), but that seems a bit contorted.  I'll do it
if I have to, though...

At any rate, does anyone have any comments on this?  Unless people have
objections, or I think better of it, I plan to commit this next Sunday.

--aidan

--IS0zKkzwUGydFO0o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ftpd.patch"

--- extern.h	Mon Feb  5 06:39:05 2001
+++ extern.h	Sun Mar 25 14:00:48 2001
@@ -118,6 +118,9 @@
 # define STRTOLL(x,y,z)	strtoll(x,y,z)
 #endif
 
+#define FTP_BUFLEN	512
+
+void	abor(void);
 void	blkfree(char **);
 void	closedataconn(FILE *);
 char   *conffilename(const char *);
@@ -171,6 +174,7 @@
 void	sizecmd(const char *);
 void	statcmd(void);
 void	statfilecmd(const char *);
+void	statxfer(void);
 void	store(const char *, const char *, int);
 LLT	strsuftoll(const char *);
 void	user(const char *);
@@ -271,9 +275,8 @@
 	mode_t		 umask;		/* Umask to use */
 };
 
-#ifndef YYEMPTY
-extern  int		yyparse(void);
-#endif
+extern void		ftp_loop(void);
+extern void		ftp_handle_line(char *);
 
 #ifndef	GLOBAL
 #define	GLOBAL	extern
@@ -309,11 +312,12 @@
 GLOBAL	int		quietmessages;
 GLOBAL	char		remotehost[MAXHOSTNAMELEN+1];
 GLOBAL	off_t		restart_point;
-GLOBAL	char		tmpline[7];
+GLOBAL	char		tmpline[FTP_BUFLEN];
 GLOBAL	sig_atomic_t	transflag;
 GLOBAL	int		type;
 GLOBAL	int		usedefault;		/* for data transfers */
 GLOBAL	const char     *version;
+GLOBAL	int		is_oob;
 
 						/* total file data bytes */
 GLOBAL	off_t		total_data_in,  total_data_out,  total_data;
--- ftpcmd.y	Fri Jan 12 12:50:51 2001
+++ ftpcmd.y	Sun Mar 25 14:15:01 2001
@@ -121,7 +121,8 @@
 static	int cmd_form;
 static	int cmd_bytesz;
 
-char	cbuf[512];
+char	cbuf[FTP_BUFLEN];
+char	*cmdp;
 char	*fromname;
 
 %}
@@ -164,29 +165,56 @@
 %token	<s> ALL
 %token	<i> NUMBER
 
-%type	<i> check_login octal_number byte_size
+%type	<i> check_noob check_oob check_login octal_number byte_size
 %type	<i> struct_code mode_code type_code form_code decimal_integer
 %type	<s> pathstring pathname password username
 %type	<s> mechanism_name base64data prot_code
 
-%start	cmd_list
+%start	cmd_sel
 
 %%
 
-cmd_list
-	: /* empty */
-
-	| cmd_list cmd
+cmd_sel
+	: cmd
 		{
 			fromname = NULL;
 			restart_point = (off_t) 0;
 		}
 
-	| cmd_list rcmd
+	| rcmd
 
 	;
 
 cmd
+	: enccmd
+
+	| check_oob oobcmd
+
+	| check_noob noobcmd
+
+	;
+
+enccmd
+	: MIC SP base64data CRLF
+		{
+			reply(502, "RFC 2228 authentication not implemented.");
+			free($3);
+		}
+
+	| CONF SP base64data CRLF
+		{
+			reply(502, "RFC 2228 authentication not implemented.");
+			free($3);
+		}
+
+	| ENC SP base64data CRLF
+		{
+			reply(502, "RFC 2228 authentication not implemented.");
+			free($3);
+		}
+	;
+
+noobcmd
 						/* RFC 959 */
 	: USER SP username CRLF
 		{
@@ -748,25 +776,6 @@
 		{
 			reply(533, "No protection enabled.");
 		}
-
-	| MIC SP base64data CRLF
-		{
-			reply(502, "RFC 2228 authentication not implemented.");
-			free($3);
-		}
-
-	| CONF SP base64data CRLF
-		{
-			reply(502, "RFC 2228 authentication not implemented.");
-			free($3);
-		}
-
-	| ENC SP base64data CRLF
-		{
-			reply(502, "RFC 2228 authentication not implemented.");
-			free($3);
-		}
-
 						/* RFC 2389 */
 	| FEAT CRLF
 		{
@@ -857,6 +866,17 @@
 		}
 	;
 
+oobcmd
+	: ABOR CRLF
+		{
+			abor();
+		}
+	| STAT CRLF
+		{
+			statxfer();
+		}
+	;
+
 rcmd
 	: REST check_login SP byte_size CRLF
 		{
@@ -1158,6 +1178,20 @@
 		}
 	;
 
+check_oob
+	: /* empty */
+		{
+			$$ = is_oob;
+		}
+	;
+
+check_noob
+	: /* empty */
+		{
+			$$ = ! is_oob;
+		}
+	;
+
 %%
 
 #define	CMD	0	/* beginning of command */
@@ -1170,6 +1204,7 @@
 #define	SITECMD	7	/* SITE command */
 #define	NSTR	8	/* Number followed by a string */
 #define NOARGS	9	/* No arguments allowed */
+#define EOLN	10	/* End of line */
 
 struct tab cmdtab[] = {
 				/* From RFC 959, in order defined (5.3.1) */
@@ -1418,6 +1453,29 @@
 	dologout(1);
 }
 
+void
+ftp_handle_line(char *cp)
+{
+	cmdp = cp;
+	yyparse();
+}
+
+void
+ftp_loop(void)
+{
+	while (1) {
+		(void) signal(SIGALRM, toolong);
+		(void) alarm(curclass.timeout);
+		if (getline(cbuf, sizeof(cbuf)-1, stdin) == NULL) {
+			reply(221, "You could at least say goodbye.");
+			dologout(0);
+		}
+		(void) alarm(0);
+		ftp_handle_line(cbuf);
+	}
+	/*NOTREACHED*/
+}
+
 static int
 yylex(void)
 {
@@ -1431,31 +1489,24 @@
 
 	case CMD:
 		hasyyerrored = 0;
-		(void) signal(SIGALRM, toolong);
-		(void) alarm(curclass.timeout);
-		if (getline(cbuf, sizeof(cbuf)-1, stdin) == NULL) {
-			reply(221, "You could at least say goodbye.");
-			dologout(0);
-		}
-		(void) alarm(0);
-		if ((cp = strchr(cbuf, '\r'))) {
+		if ((cp = strchr(cmdp, '\r'))) {
 			*cp = '\0';
 #if HAVE_SETPROCTITLE
-			if (strncasecmp(cbuf, "PASS", 4) != 0 &&
-			    strncasecmp(cbuf, "ACCT", 4) != 0)
-				setproctitle("%s: %s", proctitle, cbuf);
+			if (strncasecmp(cmdp, "PASS", 4) != 0 &&
+			    strncasecmp(cmdp, "ACCT", 4) != 0)
+				setproctitle("%s: %s", proctitle, cmdp);
 #endif /* HAVE_SETPROCTITLE */
 			*cp++ = '\n';
 			*cp = '\0';
 		}
-		if ((cp = strpbrk(cbuf, " \n")))
-			cpos = cp - cbuf;
+		if ((cp = strpbrk(cmdp, " \n")))
+			cpos = cp - cmdp;
 		if (cpos == 0)
 			cpos = 4;
-		c = cbuf[cpos];
-		cbuf[cpos] = '\0';
-		p = lookup(cmdtab, cbuf);
-		cbuf[cpos] = c;
+		c = cmdp[cpos];
+		cmdp[cpos] = '\0';
+		p = lookup(cmdtab, cmdp);
+		cmdp[cpos] = c;
 		if (p != NULL) {
 			if (! CMD_IMPLEMENTED(p)) {
 				reply(502, "%s command not implemented.",
@@ -1470,17 +1521,17 @@
 		break;
 
 	case SITECMD:
-		if (cbuf[cpos] == ' ') {
+		if (cmdp[cpos] == ' ') {
 			cpos++;
 			return (SP);
 		}
-		cp = &cbuf[cpos];
+		cp = &cmdp[cpos];
 		if ((cp2 = strpbrk(cp, " \n")))
-			cpos = cp2 - cbuf;
-		c = cbuf[cpos];
-		cbuf[cpos] = '\0';
+			cpos = cp2 - cmdp;
+		c = cmdp[cpos];
+		cmdp[cpos] = '\0';
 		p = lookup(sitetab, cp);
-		cbuf[cpos] = c;
+		cmdp[cpos] = c;
 		if (p != NULL) {
 			if (!CMD_IMPLEMENTED(p)) {
 				reply(502, "SITE %s command not implemented.",
@@ -1495,8 +1546,8 @@
 		break;
 
 	case OSTR:
-		if (cbuf[cpos] == '\n') {
-			state = CMD;
+		if (cmdp[cpos] == '\n') {
+			state = EOLN;
 			return (CRLF);
 		}
 		/* FALLTHROUGH */
@@ -1504,7 +1555,7 @@
 	case STR1:
 	case ZSTR1:
 	dostr1:
-		if (cbuf[cpos] == ' ') {
+		if (cmdp[cpos] == ' ') {
 			cpos++;
 			state = state == OSTR ? STR2 : state+1;
 			return (SP);
@@ -1512,41 +1563,41 @@
 		break;
 
 	case ZSTR2:
-		if (cbuf[cpos] == '\n') {
-			state = CMD;
+		if (cmdp[cpos] == '\n') {
+			state = EOLN;
 			return (CRLF);
 		}
 		/* FALLTHROUGH */
 
 	case STR2:
-		cp = &cbuf[cpos];
+		cp = &cmdp[cpos];
 		n = strlen(cp);
 		cpos += n - 1;
 		/*
 		 * Make sure the string is nonempty and \n terminated.
 		 */
-		if (n > 1 && cbuf[cpos] == '\n') {
-			cbuf[cpos] = '\0';
+		if (n > 1 && cmdp[cpos] == '\n') {
+			cmdp[cpos] = '\0';
 			yylval.s = xstrdup(cp);
-			cbuf[cpos] = '\n';
+			cmdp[cpos] = '\n';
 			state = ARGS;
 			return (STRING);
 		}
 		break;
 
 	case NSTR:
-		if (cbuf[cpos] == ' ') {
+		if (cmdp[cpos] == ' ') {
 			cpos++;
 			return (SP);
 		}
-		if (isdigit(cbuf[cpos])) {
-			cp = &cbuf[cpos];
-			while (isdigit(cbuf[++cpos]))
+		if (isdigit(cmdp[cpos])) {
+			cp = &cmdp[cpos];
+			while (isdigit(cmdp[++cpos]))
 				;
-			c = cbuf[cpos];
-			cbuf[cpos] = '\0';
+			c = cmdp[cpos];
+			cmdp[cpos] = '\0';
 			yylval.i = atoi(cp);
-			cbuf[cpos] = c;
+			cmdp[cpos] = c;
 			state = STR1;
 			return (NUMBER);
 		}
@@ -1554,26 +1605,26 @@
 		goto dostr1;
 
 	case ARGS:
-		if (isdigit(cbuf[cpos])) {
-			cp = &cbuf[cpos];
-			while (isdigit(cbuf[++cpos]))
+		if (isdigit(cmdp[cpos])) {
+			cp = &cmdp[cpos];
+			while (isdigit(cmdp[++cpos]))
 				;
-			c = cbuf[cpos];
-			cbuf[cpos] = '\0';
+			c = cmdp[cpos];
+			cmdp[cpos] = '\0';
 			yylval.i = atoi(cp);
-			cbuf[cpos] = c;
+			cmdp[cpos] = c;
 			return (NUMBER);
 		}
-		if (strncasecmp(&cbuf[cpos], "ALL", 3) == 0
-		 && !isalnum(cbuf[cpos + 3])) {
+		if (strncasecmp(&cmdp[cpos], "ALL", 3) == 0
+		 && !isalnum(cmdp[cpos + 3])) {
 			yylval.s = xstrdup("ALL");
 			cpos += 3;
 			return ALL;
 		}
-		switch (cbuf[cpos++]) {
+		switch (cmdp[cpos++]) {
 
 		case '\n':
-			state = CMD;
+			state = EOLN;
 			return (CRLF);
 
 		case ' ':
@@ -1634,17 +1685,21 @@
 		break;
 
 	case NOARGS:
-		if (cbuf[cpos] == '\n') {
-			state = CMD;
+		if (cmdp[cpos] == '\n') {
+			state = EOLN;
 			return (CRLF);
 		}
-		c = cbuf[cpos];
-		cbuf[cpos] = '\0';
-		reply(501, "'%s' command does not take any arguments.", cbuf);
+		c = cmdp[cpos];
+		cmdp[cpos] = '\0';
+		reply(501, "'%s' command does not take any arguments.", cmdp);
 		hasyyerrored = 1;
-		cbuf[cpos] = c;
+		cmdp[cpos] = c;
 		break;
 
+	case EOLN:
+		state = CMD;
+		return (0);
+
 	default:
 		fatal("Unknown state in scanner.");
 	}
@@ -1660,11 +1715,11 @@
 {
 	char *cp;
 
-	if (hasyyerrored)
+	if (hasyyerrored || is_oob)
 		return;
-	if ((cp = strchr(cbuf,'\n')) != NULL)
+	if ((cp = strchr(cmdp,'\n')) != NULL)
 		*cp = '\0';
-	reply(500, "'%s': command not understood.", cbuf);
+	reply(500, "'%s': command not understood.", cmdp);
 	hasyyerrored = 1;
 }
 
--- ftpd.c	Sat Mar 17 06:34:22 2001
+++ ftpd.c	Sun Mar 25 14:16:55 2001
@@ -259,6 +259,7 @@
 	hostname[0] = '\0';
 	homedir[0] = '\0';
 	gidcount = 0;
+	is_oob = 0;
 	version = FTPD_VERSION;
 
 	/*
@@ -516,8 +517,7 @@
 		reply(220, "%s FTP server (%s) ready.", hostname, version);
 
 	(void) setjmp(errcatch);
-	for (;;)
-		(void) yyparse();
+	ftp_loop();
 	/* NOTREACHED */
 }
 
@@ -2184,6 +2184,29 @@
 	_exit(status);
 }
 
+void abor(void)
+{
+	tmpline[0] = '\0';
+	is_oob = 0;
+	reply(426, "Transfer aborted. Data connection closed.");
+	reply(226, "Abort successful");
+	longjmp(urgcatch, 1);
+}
+
+void statxfer(void)
+{
+	tmpline[0] = '\0';
+	is_oob = 0;
+	if (file_size != (off_t) -1)
+		reply(213,
+		    "Status: " LLF " of " LLF " byte%s transferred",
+		    (LLT)byte_count, (LLT)file_size,
+		    PLURAL(byte_count));
+	else
+		reply(213, "Status: " LLF " byte%s transferred",
+		    (LLT)byte_count, PLURAL(byte_count));
+}
+
 static void
 myoob(int signo)
 {
@@ -2193,27 +2216,13 @@
 	if (!transflag)
 		return;
 	cp = tmpline;
-	if (getline(cp, 7, stdin) == NULL) {
+	if (getline(cp, sizeof(tmpline), stdin) == NULL) {
 		reply(221, "You could at least say goodbye.");
 		dologout(0);
 	}
-	if (strcasecmp(cp, "ABOR\r\n") == 0) {
-		tmpline[0] = '\0';
-		reply(426, "Transfer aborted. Data connection closed.");
-		reply(226, "Abort successful");
-		longjmp(urgcatch, 1);
-	}
-	if (strcasecmp(cp, "STAT\r\n") == 0) {
-		tmpline[0] = '\0';
-		if (file_size != (off_t) -1)
-			reply(213,
-			    "Status: " LLF " of " LLF " byte%s transferred",
-			    (LLT)byte_count, (LLT)file_size,
-			    PLURAL(byte_count));
-		else
-			reply(213, "Status: " LLF " byte%s transferred",
-			    (LLT)byte_count, PLURAL(byte_count));
-	}
+	is_oob = 1;
+	ftp_handle_line(cp);
+	is_oob = 0;
 }
 
 static int

--IS0zKkzwUGydFO0o--