OpenBSD Journal

ftp-proxy diff: testers needed

Contributed by merdely on from the proxying-ur-packets dept.

Camiel Dobbelaar wrote to tech@:

Can everyone that uses ftp-proxy please help test the diff below? It's urgent because the tree is locking very soon for the 4.2 release.

It's enough to apply the diff and check that everything still works.

Here's what the diff does:
David Krause noticed that some servers / proxies out there like to open the data connection immediately after the client sends the PORT command. The "normal" behaviour is to wait for the client to actually request a transfer. So this diff adds the active mode rules immediately too, so that both scenario's work.

Anyway, please test it (soon). Thanks.
--
Cam

Please test the diff (below or from the archives) and send your feedback to tech@.

[Update 2007-08-15: The fix went in.]

Index: ftp-proxy.c
===================================================================
RCS file: /cvs/src/usr.sbin/ftp-proxy/ftp-proxy.c,v
retrieving revision 1.14
diff -u -p -r1.14 ftp-proxy.c
--- ftp-proxy.c	1 Aug 2007 09:31:41 -0000	1.14
+++ ftp-proxy.c	10 Aug 2007 17:19:43 -0000
@@ -102,6 +102,7 @@ u_int16_t pick_proxy_port(void);
 void	proxy_reply(int, struct sockaddr *, u_int16_t);
 void	server_error(struct bufferevent *, short, void *);
 int	server_parse(struct session *s);
+int	allow_data_connection(struct session *s);
 void	server_read(struct bufferevent *, void *);
 const char *sock_ntop(struct sockaddr *);
 void	usage(void);
@@ -149,8 +150,19 @@ client_parse(struct session *s)
 		return (1);
 
 	if (linebuf[0] == 'P' || linebuf[0] == 'p' ||
-	    linebuf[0] == 'E' || linebuf[0] == 'e')
-		return (client_parse_cmd(s));
+	    linebuf[0] == 'E' || linebuf[0] == 'e') {
+		if (!client_parse_cmd(s))
+			return (0);
+
+		/*
+		 * Allow active mode connections immediately, instead of
+		 * waiting for a positive reply from the server.  Some
+		 * rare servers/proxies try to probe or setup the data
+		 * connection before an actual transfer request.
+		 */
+		if (s->cmd == CMD_PORT || s->cmd == CMD_EPRT)
+			return (allow_data_connection(s));
+	}
 	
 	if (anonymous_only && (linebuf[0] == 'U' || linebuf[0] == 'u'))
 		return (client_parse_anon(s));
@@ -894,12 +906,26 @@ server_error(struct bufferevent *bufev, 
 int
 server_parse(struct session *s)
 {
-	struct sockaddr *client_sa, *orig_sa, *proxy_sa, *server_sa;
-	int prepared = 0;
-
 	if (s->cmd == CMD_NONE || linelen < 4 || linebuf[0] != '2')
 		goto out;
 
+	if ((s->cmd == CMD_PASV && strncmp("227 ", linebuf, 4) == 0) ||
+	    (s->cmd == CMD_EPSV && strncmp("229 ", linebuf, 4) == 0))
+		return (allow_data_connection(s));
+
+ out:
+	s->cmd = CMD_NONE;
+	s->port = 0;
+
+	return (1);
+}
+
+int
+allow_data_connection(struct session *s)
+{
+	struct sockaddr *client_sa, *orig_sa, *proxy_sa, *server_sa;
+	int prepared = 0;
+
 	/*
 	 * The pf rules below do quite some NAT rewriting, to keep up
 	 * appearances.  Points to keep in mind:
@@ -924,8 +950,7 @@ server_parse(struct session *s)
 		orig_sa = sstosa(&s->server_ss);
 
 	/* Passive modes. */
-	if ((s->cmd == CMD_PASV && strncmp("227 ", linebuf, 4) == 0) ||
-	    (s->cmd == CMD_EPSV && strncmp("229 ", linebuf, 4) == 0)) {
+	if (s->cmd == CMD_PASV || s->cmd == CMD_EPSV) {
 		s->port = parse_port(s->cmd);
 		if (s->port < MIN_PORT) {
 			logmsg(LOG_CRIT, "#%d bad port in '%s'", s->id,
@@ -966,8 +991,7 @@ server_parse(struct session *s)
 	}
 
 	/* Active modes. */
-	if ((s->cmd == CMD_PORT || s->cmd == CMD_EPRT) &&
-	    strncmp("200 ", linebuf, 4) == 0) {
+	if (s->cmd == CMD_PORT || s->cmd == CMD_EPRT) {
 		logmsg(LOG_INFO, "#%d active: server to client port %d"
 		    " via port %d", s->id, s->port, s->proxy_port);
 
@@ -1017,7 +1041,6 @@ server_parse(struct session *s)
 			goto fail;
 	}
 
- out:
 	s->cmd = CMD_NONE;
 	s->port = 0;

(Comments are closed)


Credits

Copyright © - Daniel Hartmeier. All rights reserved. Articles and comments are copyright their respective authors, submission implies license to publish on this web site. Contents of the archive prior to as well as images and HTML templates were copied from the fabulous original deadly.org with Jose's and Jim's kind permission. This journal runs as CGI with httpd(8) on OpenBSD, the source code is BSD licensed. undeadly \Un*dead"ly\, a. Not subject to death; immortal. [Obs.]