From: millert <millert@openbsd.org>
Date: Thu, 15 Sep 2022 12:47:10 +0000 (+0000)
Subject: Use non-blocking connect() with ppoll() and timeout instead of alarm().
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f37faed6bf7abab250c71352d8ea4cc484e8b57c;p=openbsd

Use non-blocking connect() with ppoll() and timeout instead of alarm().
For hosts with multiple IP addrs this makes it possible to fall
over from an unresponsive IP to another.  This also replaces the
other connect(2) + connect_wait() calls with timed_connect() so the
-w option now works for more that just http.  OK sthen@ deraadt@
---

diff --git a/usr.bin/ftp/extern.h b/usr.bin/ftp/extern.h
index 85136eff7fa..892afde13ad 100644
--- a/usr.bin/ftp/extern.h
+++ b/usr.bin/ftp/extern.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: extern.h,v 1.52 2021/02/02 12:58:42 robert Exp $	*/
+/*	$OpenBSD: extern.h,v 1.53 2022/09/15 12:47:10 millert Exp $	*/
 /*	$NetBSD: extern.h,v 1.17 1997/08/18 10:20:19 lukem Exp $	*/
 
 /*
@@ -62,6 +62,7 @@
  */
 
 #include <sys/types.h>
+#include <sys/socket.h>
 
 void	abort_remote(FILE *);
 void	abortpt(int);
@@ -75,7 +76,6 @@ void	cmdabort(int);
 void	cmdscanner(int);
 int	command(const char *, ...);
 int	confirm(const char *, const char *);
-int	connect_wait(int);
 FILE   *dataconn(const char *);
 int	foregroundproc(void);
 int	fileindir(const char *, const char *);
@@ -109,6 +109,7 @@ void	sethash(int, char **);
 void	setpeer(int, char **);
 void	setttywidth(int);
 char   *slurpstring(void);
+int	timed_connect(int s, const struct sockaddr *, socklen_t, int);
 
 __dead void	usage(void);
 
diff --git a/usr.bin/ftp/fetch.c b/usr.bin/ftp/fetch.c
index a494e5a8f15..c1d2af643ed 100644
--- a/usr.bin/ftp/fetch.c
+++ b/usr.bin/ftp/fetch.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: fetch.c,v 1.209 2022/09/08 11:12:44 claudio Exp $	*/
+/*	$OpenBSD: fetch.c,v 1.210 2022/09/15 12:47:10 millert Exp $	*/
 /*	$NetBSD: fetch.c,v 1.14 1997/08/18 10:20:20 lukem Exp $	*/
 
 /*-
@@ -173,14 +173,6 @@ url_encode(const char *path)
 	return (epath);
 }
 
-/* ARGSUSED */
-static void
-tooslow(int signo)
-{
-	dprintf(STDERR_FILENO, "%s: connect taking too long\n", __progname);
-	_exit(2);
-}
-
 /*
  * Copy a local file (used by the OpenBSD installer).
  * Returns -1 on failure, 0 on success
@@ -604,14 +596,8 @@ noslash:
 		}
 #endif /* !SMALL */
 
-		if (connect_timeout) {
-			(void)signal(SIGALRM, tooslow);
-			alarmtimer(connect_timeout);
-		}
-
-		for (error = connect(fd, res->ai_addr, res->ai_addrlen);
-		    error != 0 && errno == EINTR; error = connect_wait(fd))
-			continue;
+		error = timed_connect(fd, res->ai_addr, res->ai_addrlen,
+		    connect_timeout);
 		if (error != 0) {
 			save_errno = errno;
 			close(fd);
@@ -700,11 +686,6 @@ noslash:
 	}
 #endif
 
-	if (connect_timeout) {
-		signal(SIGALRM, SIG_DFL);
-		alarmtimer(0);
-	}
-
 	/*
 	 * Construct and send the request. Proxy requests don't want leading /.
 	 */
@@ -1242,7 +1223,6 @@ aborthttp(int signo)
 {
 	const char errmsg[] = "\nfetch aborted.\n";
 
-	alarmtimer(0);
 	write(fileno(ttyout), errmsg, sizeof(errmsg) - 1);
 	longjmp(httpabort, 1);
 }
diff --git a/usr.bin/ftp/ftp.1 b/usr.bin/ftp/ftp.1
index 12b09d20968..c5dde04be58 100644
--- a/usr.bin/ftp/ftp.1
+++ b/usr.bin/ftp/ftp.1
@@ -1,4 +1,4 @@
-.\"	$OpenBSD: ftp.1,v 1.123 2022/03/27 20:09:12 naddy Exp $
+.\"	$OpenBSD: ftp.1,v 1.124 2022/09/15 12:47:10 millert Exp $
 .\"	$NetBSD: ftp.1,v 1.22 1997/08/18 10:20:22 lukem Exp $
 .\"
 .\" Copyright (c) 1985, 1989, 1990, 1993
@@ -30,7 +30,7 @@
 .\"
 .\"	@(#)ftp.1	8.3 (Berkeley) 10/9/94
 .\"
-.Dd $Mdocdate: March 27 2022 $
+.Dd $Mdocdate: September 15 2022 $
 .Dt FTP 1
 .Os
 .Sh NAME
@@ -320,9 +320,9 @@ Forces
 to show all responses from the remote server, as well
 as report on data transfer statistics.
 .It Fl w Ar seconds
-For URL format connections to HTTP/HTTPS servers, abort a
-slow connection after
-.Ar seconds .
+Wait for
+.Ar seconds
+for the remote server to connect before giving up.
 .El
 .Pp
 The host with which
diff --git a/usr.bin/ftp/ftp.c b/usr.bin/ftp/ftp.c
index e1702011ebe..3ce5f7de02c 100644
--- a/usr.bin/ftp/ftp.c
+++ b/usr.bin/ftp/ftp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ftp.c,v 1.107 2019/11/18 04:37:35 deraadt Exp $	*/
+/*	$OpenBSD: ftp.c,v 1.108 2022/09/15 12:47:10 millert Exp $	*/
 /*	$NetBSD: ftp.c,v 1.27 1997/08/18 10:20:23 lukem Exp $	*/
 
 /*
@@ -212,9 +212,8 @@ hookup(char *host, char *port)
 			}
 		}
 #endif /* !SMALL */
-		for (error = connect(s, res->ai_addr, res->ai_addrlen);
-		    error != 0 && errno == EINTR; error = connect_wait(s))
-			continue;
+		error = timed_connect(s, res->ai_addr, res->ai_addrlen,
+		    connect_timeout);
 		if (error != 0) {
 			/* this "if" clause is to prevent print warning twice */
 			if (verbose && res->ai_next) {
@@ -1509,10 +1508,8 @@ reinit:
 		} else
 			goto bad;
 
-		for (error = connect(data, &data_addr.sa, data_addr.sa.sa_len);
-		    error != 0 && errno == EINTR;
-		    error = connect_wait(data))
-			continue;
+		error = timed_connect(data, &data_addr.sa, data_addr.sa.sa_len,
+		    connect_timeout);
 		if (error != 0) {
 			if (activefallback) {
 				(void)close(data);
diff --git a/usr.bin/ftp/util.c b/usr.bin/ftp/util.c
index ef187dc800f..06008c1e12e 100644
--- a/usr.bin/ftp/util.c
+++ b/usr.bin/ftp/util.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: util.c,v 1.95 2021/02/02 12:58:42 robert Exp $	*/
+/*	$OpenBSD: util.c,v 1.96 2022/09/15 12:47:10 millert Exp $	*/
 /*	$NetBSD: util.c,v 1.12 1997/08/18 10:20:27 lukem Exp $	*/
 
 /*-
@@ -1076,6 +1076,84 @@ controlediting(void)
 }
 #endif /* !SMALL */
 
+/*
+ * connect(2) with an optional timeout if secs > 0.
+ */
+int
+timed_connect(int s, const struct sockaddr *name, socklen_t namelen, int secs)
+{
+	struct timespec now, target, timebuf, *timeout = NULL;
+	int flags, nready, optval, ret = -1;
+	socklen_t optlen;
+	struct pollfd pfd;
+
+	if (secs > 0) {
+		timebuf.tv_sec = secs;
+		timebuf.tv_nsec = 0;
+		timeout = &timebuf;
+		clock_gettime(CLOCK_MONOTONIC, &target);
+		timespecadd(&target, timeout, &target);
+	}
+
+	flags = fcntl(s, F_GETFL, 0);
+	if (flags == -1) {
+		warn("fcntl(F_GETFL)");
+		return -1;
+	}
+	if (!(flags & O_NONBLOCK)) {
+		if (fcntl(s, F_SETFL, flags | O_NONBLOCK) == -1) {
+			warn("fcntl(F_SETFL)");
+			return -1;
+		}
+	}
+
+	ret = connect(s, name, namelen);
+	if (ret == 0 || errno != EINPROGRESS)
+		goto done;
+
+	for (;;) {
+		pfd.fd = s;
+		pfd.events = POLLOUT;
+		nready = ppoll(&pfd, 1, timeout, NULL);
+		switch (nready) {
+		case -1:
+			if (errno != EINTR && errno != EAGAIN) {
+				warn("ppoll");
+				goto done;
+			}
+			if (timeout == NULL)
+				continue;
+			clock_gettime(CLOCK_MONOTONIC, &now);
+			timespecsub(&now, &target, timeout);
+			if (timeout->tv_sec >= 0)
+				continue;
+			/* FALLTHROUGH */
+		case 0:
+			errno = ETIMEDOUT;
+			goto done;
+		default:
+			optlen = sizeof(optval);
+			ret = getsockopt(s, SOL_SOCKET, SO_ERROR, &optval,
+			    &optlen);
+			if (ret == 0 && optval != 0) {
+				ret = -1;
+				errno = optval;
+			}
+			goto done;
+		}
+	}
+
+done:
+	if (!(flags & O_NONBLOCK)) {
+		if (fcntl(s, F_SETFL, flags) == -1) {
+			warn("fcntl(F_SETFL)");
+			ret = -1;
+		}
+	}
+
+	return ret;
+}
+
 /*
  * Wait for an asynchronous connect(2) attempt to finish.
  */