Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 15 Oct 2002 14:50:03 -0700 (PDT)
From:      Bill Fenner <fenner@fee.attlabs.att.com>
To:        FreeBSD-gnats-submit@FreeBSD.org
Subject:   bin/44123: libfetch sends CRLF in seperate packet
Message-ID:  <200210152150.g9FLo3WR070072@fee.attlabs.att.com>

next in thread | raw e-mail | index | archive | help

>Number:         44123
>Category:       bin
>Synopsis:       libfetch sends CRLF in seperate packet
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Oct 15 15:00:11 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Bill Fenner
>Release:        FreeBSD 5.0-CURRENT sparc64
>Organization:
AT&T Labs - Research
>Environment:
System: FreeBSD fee.attlabs.att.com 5.0-CURRENT FreeBSD 5.0-CURRENT #0: Fri Oct 4 22:01:02 PDT 2002 root@fee.attlabs.att.com:/usr/obj/usr/src/sys/FEE sparc64


	
>Description:
	
The introduction of SSL caused libfetch to switch to two individual
write() calls to send the command and associated CRLF.  This causes
two problems:

1. Nagle causes the transmission of the CRLF to wait for the ACK of the
   command.  This means an extra RTT + delayed-ack-delay to get an individual
   line through.
2. Filtering firewalls such as the Checkpoint Firewall/1 that enforce
   that the FTP port carries FTP commands do not allow command packets
   that do not carry CRLF.  This, of course, is somewhat silly since
   there's nothing that says that the CRLF must be in the same packet,
   but is certainly a big usability issue.

>How-To-Repeat:
	
Run fetch, watch what it puts on the network.  Or inspect the code.
Or run it behind a Checkpoint Firewall/1 and see all of your connection
attempts get reset before the authentication.

>Fix:

	

Introduce _fetch_writev() and use it in _fetch_putln().

Note: the partial write recovery code is untested.  It's required
for SSL use, since there is no SSL_writev.

cvs diff: Diffing .
Index: common.c
===================================================================
RCS file: /net/wilson/usr/home/ncvs/src/lib/libfetch/common.c,v
retrieving revision 1.33
diff -u -r1.33 common.c
--- common.c	20 Sep 2002 21:50:57 -0000	1.33
+++ common.c	15 Oct 2002 21:45:25 -0000
@@ -450,6 +450,20 @@
 ssize_t
 _fetch_write(conn_t *conn, const char *buf, size_t len)
 {
+	struct iovec iov;
+
+	iov.iov_base = (char *)buf;
+	iov.iov_len = len;
+	return _fetch_writev(conn, &iov, 1);
+}
+
+/*
+ * Writev to a connection w/ timeout
+ * Can modify iovec; callers should be aware.
+ */
+ssize_t
+_fetch_writev(conn_t *conn, struct iovec *iov, int iovcnt)
+{
 	struct timeval now, timeout, wait;
 	fd_set writefds;
 	ssize_t wlen, total;
@@ -461,7 +475,7 @@
 		timeout.tv_sec += fetchTimeout;
 	}
 
-	while (len > 0) {
+	while (iovcnt > 0) {
 		while (fetchTimeout && !FD_ISSET(conn->sd, &writefds)) {
 			FD_SET(conn->sd, &writefds);
 			gettimeofday(&now, NULL);
@@ -486,10 +500,10 @@
 		errno = 0;
 #ifdef WITH_SSL
 		if (conn->ssl != NULL)
-			wlen = SSL_write(conn->ssl, buf, len);
+			wlen = SSL_write(conn->ssl, iov->iov_base, iov->iov_len);
 		else
 #endif
-			wlen = write(conn->sd, buf, len);
+			wlen = writev(conn->sd, iov, iovcnt);
 		if (wlen == 0)
 			/* we consider a short write a failure */
 			return (-1);
@@ -498,9 +512,18 @@
 				continue;
 			return (-1);
 		}
-		len -= wlen;
-		buf += wlen;
 		total += wlen;
+		while (wlen > 0 && iovcnt > 0) {
+			if (wlen >= (ssize_t)iov->iov_len) {
+				wlen -= iov->iov_len;
+				iov++;
+				iovcnt--;
+			} else {
+				iov->iov_len -= wlen;
+				iov->iov_base = (char *)iov->iov_base + wlen;
+				wlen = 0;
+			}
+		}
 	}
 	return (total);
 }
@@ -512,10 +535,14 @@
 int
 _fetch_putln(conn_t *conn, const char *str, size_t len)
 {
+	struct iovec iov[2];
 
 	DEBUG(fprintf(stderr, ">>> %s\n", str));
-	if (_fetch_write(conn, str, len) == -1 ||
-	    _fetch_write(conn, ENDL, sizeof ENDL) == -1)
+	iov[0].iov_base = (char *)str;
+	iov[0].iov_len = len;
+	iov[1].iov_base = (char *)ENDL;
+	iov[1].iov_len = sizeof ENDL;
+	if (_fetch_writev(conn, iov, 2) == -1)
 		return (-1);
 	return (0);
 }
Index: common.h
===================================================================
RCS file: /net/wilson/usr/home/ncvs/src/lib/libfetch/common.h,v
retrieving revision 1.24
diff -u -r1.24 common.h
--- common.h	11 Jun 2002 11:27:28 -0000	1.24
+++ common.h	15 Oct 2002 21:43:08 -0000
@@ -68,6 +68,9 @@
 	const char	*string;
 };
 
+/* for _fetch_writev */
+struct iovec;
+
 void		 _fetch_seterr(struct fetcherr *, int);
 void		 _fetch_syserr(void);
 void		 _fetch_info(const char *, ...);
@@ -80,6 +83,7 @@
 ssize_t		 _fetch_read(conn_t *, char *, size_t);
 int		 _fetch_getln(conn_t *);
 ssize_t		 _fetch_write(conn_t *, const char *, size_t);
+ssize_t		 _fetch_writev(conn_t *, struct iovec *, int);
 int		 _fetch_putln(conn_t *, const char *, size_t);
 int		 _fetch_close(conn_t *);
 int		 _fetch_add_entry(struct url_ent **, int *, int *,
>Release-Note:
>Audit-Trail:
>Unformatted:

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-bugs" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200210152150.g9FLo3WR070072>