Michihito Shigemura
2018-Dec-09 14:39 UTC
[PATCH] Enable ConnectTimeout with ConnectionAttempts
Fix bug ConnectTimeout=N only works on the first ConnectionAttempts
https://bugzilla.mindrot.org/show_bug.cgi?id=2918
---
sshconnect.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sshconnect.c b/sshconnect.c
index 4862da5e..b837a83a 100644
--- a/sshconnect.c
+++ b/sshconnect.c
@@ -454,11 +454,12 @@ waitrfd(int fd, int *timeoutp)
{
struct pollfd pfd;
struct timeval t_start;
- int oerrno, r;
+ int oerrno, r, next_timeout;
monotime_tv(&t_start);
pfd.fd = fd;
pfd.events = POLLIN;
+ next_timeout = *timeoutp;
for (; *timeoutp >= 0;) {
r = poll(&pfd, 1, *timeoutp);
oerrno = errno;
@@ -473,6 +474,7 @@ waitrfd(int fd, int *timeoutp)
}
/* timeout */
errno = ETIMEDOUT;
+ *timeoutp = next_timeout;
return -1;
}
--
2.19.2
On Sun, 9 Dec 2018, Michihito Shigemura wrote:> Fix bug ConnectTimeout=N only works on the first ConnectionAttempts > https://bugzilla.mindrot.org/show_bug.cgi?id=2918Thanks for the reminder :) ConnectTimeout is supposed to apply to both the initial TCP connection and the subsequent banner exchange. This is done to allow it to detect servers (or middleboxes) that accept the connection but never return a banner. This diff seems to make the connect timeout apply independently to each TCP connection and then whatever is left of the timeout interval to be applied to banner exchange. Is that correct? If so, IMO it's a reasonable compromise between two fairly conflictual options...> sshconnect.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/sshconnect.c b/sshconnect.c > index 4862da5e..b837a83a 100644 > --- a/sshconnect.c > +++ b/sshconnect.c > @@ -454,11 +454,12 @@ waitrfd(int fd, int *timeoutp) > { > struct pollfd pfd; > struct timeval t_start; > - int oerrno, r; > + int oerrno, r, next_timeout; > > monotime_tv(&t_start); > pfd.fd = fd; > pfd.events = POLLIN; > + next_timeout = *timeoutp; > for (; *timeoutp >= 0;) { > r = poll(&pfd, 1, *timeoutp); > oerrno = errno; > @@ -473,6 +474,7 @@ waitrfd(int fd, int *timeoutp) > } > /* timeout */ > errno = ETIMEDOUT; > + *timeoutp = next_timeout; > return -1; > } > > -- > 2.19.2 > > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev >
Thank you for your replying.> This diff seems to make the connect timeout apply independently to each > TCP connection and then whatever is left of the timeout interval to be > applied to banner exchange. Is that correct?Yes, it's correct. I forgot about banner exchange. I want to test "Connection timed out during banner exchange" error, but I don't know how to audit this error on purpose. Do you have any ideas? 2018?12?14?(?) 11:32 Damien Miller <djm at mindrot.org>:> > On Sun, 9 Dec 2018, Michihito Shigemura wrote: > > > Fix bug ConnectTimeout=N only works on the first ConnectionAttempts > > https://bugzilla.mindrot.org/show_bug.cgi?id=2918 > > Thanks for the reminder :) > > ConnectTimeout is supposed to apply to both the initial TCP connection > and the subsequent banner exchange. This is done to allow it to detect > servers (or middleboxes) that accept the connection but never return > a banner. > > This diff seems to make the connect timeout apply independently to each > TCP connection and then whatever is left of the timeout interval to be > applied to banner exchange. Is that correct? > > If so, IMO it's a reasonable compromise between two fairly conflictual > options... > > > sshconnect.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/sshconnect.c b/sshconnect.c > > index 4862da5e..b837a83a 100644 > > --- a/sshconnect.c > > +++ b/sshconnect.c > > @@ -454,11 +454,12 @@ waitrfd(int fd, int *timeoutp) > > { > > struct pollfd pfd; > > struct timeval t_start; > > - int oerrno, r; > > + int oerrno, r, next_timeout; > > > > monotime_tv(&t_start); > > pfd.fd = fd; > > pfd.events = POLLIN; > > + next_timeout = *timeoutp; > > for (; *timeoutp >= 0;) { > > r = poll(&pfd, 1, *timeoutp); > > oerrno = errno; > > @@ -473,6 +474,7 @@ waitrfd(int fd, int *timeoutp) > > } > > /* timeout */ > > errno = ETIMEDOUT; > > + *timeoutp = next_timeout; > > return -1; > > } > > > > -- > > 2.19.2 > > > > _______________________________________________ > > openssh-unix-dev mailing list > > openssh-unix-dev at mindrot.org > > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev > >-- Michihito Shigemura