proposed change to ssh_connect_direct()
Darren Tucker
dtucker at zip.com.au
Thu Jan 12 20:49:44 AEDT 2017
On Thu, Jan 12, 2017 at 04:52:05PM +1100, Darren Tucker wrote:
> On Thu, Jan 12, 2017 at 01:49:29PM +1100, Darren Tucker wrote:
> [...]
> > I would have thought that using non-blocking connect (ie set
> > O_NONBLOCK on the fds, initiate the connections then select on the set
> > until one succeeds) would be feasible, though.
>
> In fact most of the required pieces are already there. Below is a
> rough, barely tested patch that nonetheless does seem to sort of work.
>
> So, it's at least feasible. I'm still not sure it's worth doing, though.
> I can see some downsides: it'll spam server logs with "Did not receive
> identification string" and consume MaxStartups connections. Maybe best
> to leave it to a custom ProxyCommand for people who really want the
> functionality.
Tested it a bit, found it didn't handle failed connections.
diff --git a/sshconnect.c b/sshconnect.c
index 96b91ce..8c1f54d 100644
--- a/sshconnect.c
+++ b/sshconnect.c
@@ -328,89 +328,6 @@ ssh_create_socket(int privileged, struct addrinfo *ai)
return sock;
}
-static int
-timeout_connect(int sockfd, const struct sockaddr *serv_addr,
- socklen_t addrlen, int *timeoutp)
-{
- fd_set *fdset;
- struct timeval tv, t_start;
- socklen_t optlen;
- int optval, rc, result = -1;
-
- gettimeofday(&t_start, NULL);
-
- if (*timeoutp <= 0) {
- result = connect(sockfd, serv_addr, addrlen);
- goto done;
- }
-
- set_nonblock(sockfd);
- rc = connect(sockfd, serv_addr, addrlen);
- if (rc == 0) {
- unset_nonblock(sockfd);
- result = 0;
- goto done;
- }
- if (errno != EINPROGRESS) {
- result = -1;
- goto done;
- }
-
- fdset = xcalloc(howmany(sockfd + 1, NFDBITS),
- sizeof(fd_mask));
- FD_SET(sockfd, fdset);
- ms_to_timeval(&tv, *timeoutp);
-
- for (;;) {
- rc = select(sockfd + 1, NULL, fdset, NULL, &tv);
- if (rc != -1 || errno != EINTR)
- break;
- }
-
- switch (rc) {
- case 0:
- /* Timed out */
- errno = ETIMEDOUT;
- break;
- case -1:
- /* Select error */
- debug("select: %s", strerror(errno));
- break;
- case 1:
- /* Completed or failed */
- optval = 0;
- optlen = sizeof(optval);
- if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &optval,
- &optlen) == -1) {
- debug("getsockopt: %s", strerror(errno));
- break;
- }
- if (optval != 0) {
- errno = optval;
- break;
- }
- result = 0;
- unset_nonblock(sockfd);
- break;
- default:
- /* Should not occur */
- fatal("Bogus return (%d) from select()", rc);
- }
-
- free(fdset);
-
- done:
- if (result == 0 && *timeoutp > 0) {
- ms_subtract_diff(&t_start, timeoutp);
- if (*timeoutp <= 0) {
- errno = ETIMEDOUT;
- result = -1;
- }
- }
-
- return (result);
-}
-
/*
* Opens a TCP/IP connection to the remote server on the given host.
* The address of the remote host will be returned in hostaddr.
@@ -427,15 +344,25 @@ ssh_connect_direct(const char *host, struct addrinfo *aitop,
struct sockaddr_storage *hostaddr, u_short port, int family,
int connection_attempts, int *timeout_ms, int want_keepalive, int needpriv)
{
- int on = 1;
- int sock = -1, attempt;
+ int i, j, on = 1, rc, inprogress = 0, lasterr = 0, fds = -1;
+ int sock = -1, connected_sock = -1, attempt, fdmax;
char ntop[NI_MAXHOST], strport[NI_MAXSERV];
struct addrinfo *ai;
+ struct timeval tv, t_start;
+ fd_set fdset;
+ socklen_t rlen = sizeof(rc);
+#define MAXCONNECT 16
+ int inprogress_fd[MAXCONNECT];
+ void *ai_addr[MAXCONNECT];
+ size_t ai_len[MAXCONNECT];
debug2("%s: needpriv %d", __func__, needpriv);
memset(ntop, 0, sizeof(ntop));
memset(strport, 0, sizeof(strport));
+ FD_ZERO(&fdset);
+ gettimeofday(&t_start, NULL);
+
for (attempt = 0; attempt < connection_attempts; attempt++) {
if (attempt > 0) {
/* Sleep a moment before retrying. */
@@ -443,10 +370,11 @@ ssh_connect_direct(const char *host, struct addrinfo *aitop,
debug("Trying again...");
}
/*
- * Loop through addresses for this host, and try each one in
- * sequence until the connection succeeds.
+ * Loop through addresses for this host, initiate a nonblocking
+ * connnection then see which one succeeds.
*/
- for (ai = aitop; ai; ai = ai->ai_next) {
+ for (ai = aitop; ai != NULL && inprogress < MAXCONNECT &&
+ connected_sock == -1; ai = ai->ai_next) {
if (ai->ai_family != AF_INET &&
ai->ai_family != AF_INET6)
continue;
@@ -465,26 +393,72 @@ ssh_connect_direct(const char *host, struct addrinfo *aitop,
/* Any error is already output */
continue;
- if (timeout_connect(sock, ai->ai_addr, ai->ai_addrlen,
- timeout_ms) >= 0) {
- /* Successful connection. */
- memcpy(hostaddr, ai->ai_addr, ai->ai_addrlen);
- break;
- } else {
- debug("connect to address %s port %s: %s",
- ntop, strport, strerror(errno));
- close(sock);
- sock = -1;
+ set_nonblock(sock);
+ if (connect(sock, ai->ai_addr, ai->ai_addrlen) == 0) {
+ debug("connection established immediately");
+ connected_sock = sock;
+ } else if (errno == EINPROGRESS)
+ debug("nonblocking connect fd %d", sock);
+ else
+ continue;
+ inprogress_fd[inprogress] = sock;
+ ai_len[inprogress] = ai->ai_addrlen;
+ ai_addr[inprogress] = ai->ai_addr;
+ inprogress++;
+ }
+ }
+
+ ms_to_timeval(&tv, *timeout_ms);
+ while (connected_sock == -1 && timeout_ms > 0) {
+ FD_ZERO(&fdset);
+ fds = fdmax = 0;
+ for (i = 0; i < inprogress; i++) {
+ if (inprogress_fd[i] == -1)
+ continue;
+ FD_SET(inprogress_fd[i], &fdset);
+ fdmax = MAX(fdmax, inprogress_fd[i]);
+ fds++;
+ }
+ if (fds == 0) /* no descriptors left */
+ break;
+ rc = select(fdmax + 1, NULL, &fdset, NULL, &tv);
+ ms_subtract_diff(&t_start, timeout_ms);
+ for (i = 0; i <= fdmax; i++) {
+ if (!FD_ISSET(i, &fdset))
+ continue;
+ if (getsockopt(i, SOL_SOCKET, SO_ERROR, &rc, &rlen)
+ == 0) {
+ if (rc == EINPROGRESS) {
+ ;
+ } else if (rc == 0) {
+ debug("connected to fd %d", i);
+ connected_sock = i;
+ unset_nonblock(connected_sock);
+ break;
+ } else {
+ debug("fd %d error %d (%s)", i, rc,
+ strerror(rc));
+ lasterr = rc;
+ close(i);
+ for (j = 0; j < inprogress; j++)
+ if (inprogress_fd[j] == i)
+ inprogress_fd[j] = -1;
+ }
}
}
- if (sock != -1)
- break; /* Successful connection. */
+ }
+
+ for (i = 0; i < inprogress; i++) {
+ if (connected_sock == inprogress_fd[i])
+ memcpy(hostaddr, ai_addr[i], ai_len[i]);
+ else
+ close(inprogress_fd[i]);
}
/* Return failure if we didn't get a successful connection. */
- if (sock == -1) {
+ if (connected_sock == -1) {
error("ssh: connect to host %s port %s: %s",
- host, strport, strerror(errno));
+ host, strport, strerror(lasterr));
return (-1);
}
@@ -492,12 +466,12 @@ ssh_connect_direct(const char *host, struct addrinfo *aitop,
/* Set SO_KEEPALIVE if requested. */
if (want_keepalive &&
- setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, (void *)&on,
+ setsockopt(connected_sock, SOL_SOCKET, SO_KEEPALIVE, (void *)&on,
sizeof(on)) < 0)
error("setsockopt SO_KEEPALIVE: %.100s", strerror(errno));
/* Set the connection. */
- packet_set_connection(sock, sock);
+ packet_set_connection(connected_sock, connected_sock);
return 0;
}
--
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
More information about the openssh-unix-dev
mailing list