Re: [Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
Tomas Hozza writes: > - Original Message - >> Thanks to have submitted the patch. It looks fine (I have just a minor >> comment below). Could I please ask you to provide also the entries for >> the ChangeLog file as part of your patch? It is a boring task but this >> is required by the GNU Coding standards[1]. If you have no time for >> this, I can do it. > > Whitespace removed, changelog entry added. Fixed patch is attached. I added more information to the ChangeLog file and pushed the patch. Thanks! Giuseppe
Re: [Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
- Original Message -
> Thanks to have submitted the patch. It looks fine (I have just a minor
> comment below). Could I please ask you to provide also the entries for
> the ChangeLog file as part of your patch? It is a boring task but this
> is required by the GNU Coding standards[1]. If you have no time for
> this, I can do it.
Whitespace removed, changelog entry added. Fixed patch is attached.
Regards,
Tomas HozzaFrom af919b1126c407a2e1e1a2c6ad4af5831247ee4a Mon Sep 17 00:00:00 2001
From: Karsten Hopp
Date: Thu, 11 Jul 2013 11:27:35 +0200
Subject: [PATCH] Fix timeout option when used with SSL
Previously wget didn't honor the --timeout option if the remote host did
not answer SSL handshake
Signed-off-by: Tomas Hozza
---
src/ChangeLog | 4
src/openssl.c | 62 ++-
2 files changed, 57 insertions(+), 9 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 8822973..658e933 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2013-07-11 Karsten Hopp
+
+ * openssl.c: respect connect timeout
+
2013-07-11 Tim Ruehsen
* gnutls.c (ssl_connect_wget): respect connect timeout.
diff --git a/src/openssl.c b/src/openssl.c
index 3924e41..189b334 100644
--- a/src/openssl.c
+++ b/src/openssl.c
@@ -256,19 +256,42 @@ struct openssl_transport_context {
char *last_error; /* last error printed with openssl_errstr */
};
-static int
-openssl_read (int fd, char *buf, int bufsize, void *arg)
-{
- int ret;
- struct openssl_transport_context *ctx = arg;
+struct openssl_read_args {
+ int fd;
+ struct openssl_transport_context *ctx;
+ char *buf;
+ int bufsize;
+ int retval;
+};
+
+static void openssl_read_callback(void *arg) {
+ struct openssl_read_args *args = (struct openssl_read_args *) arg;
+ struct openssl_transport_context *ctx = args->ctx;
SSL *conn = ctx->conn;
+ char *buf = args->buf;
+ int bufsize = args->bufsize;
+ int ret;
+
do
ret = SSL_read (conn, buf, bufsize);
- while (ret == -1
- && SSL_get_error (conn, ret) == SSL_ERROR_SYSCALL
+ while (ret == -1 && SSL_get_error (conn, ret) == SSL_ERROR_SYSCALL
&& errno == EINTR);
+ args->retval = ret;
+}
- return ret;
+static int
+openssl_read (int fd, char *buf, int bufsize, void *arg)
+{
+ struct openssl_read_args args;
+ args.fd = fd;
+ args.buf = buf;
+ args.bufsize = bufsize;
+ args.ctx = (struct openssl_transport_context*) arg;
+
+ if (run_with_timeout(opt.read_timeout, openssl_read_callback, &args)) {
+return -1;
+ }
+ return args.retval;
}
static int
@@ -386,6 +409,18 @@ static struct transport_implementation openssl_transport = {
openssl_peek, openssl_errstr, openssl_close
};
+struct scwt_context {
+ SSL *ssl;
+ int result;
+};
+
+static void
+ssl_connect_with_timeout_callback(void *arg)
+{
+ struct scwt_context *ctx = (struct scwt_context *)arg;
+ ctx->result = SSL_connect(ctx->ssl);
+}
+
/* Perform the SSL handshake on file descriptor FD, which is assumed
to be connected to an SSL server. The SSL handle provided by
OpenSSL is registered with the file descriptor FD using
@@ -398,6 +433,7 @@ bool
ssl_connect_wget (int fd, const char *hostname)
{
SSL *conn;
+ struct scwt_context scwt_ctx;
struct openssl_transport_context *ctx;
DEBUGP (("Initiating SSL handshake.\n"));
@@ -425,7 +461,14 @@ ssl_connect_wget (int fd, const char *hostname)
if (!SSL_set_fd (conn, FD_TO_SOCKET (fd)))
goto error;
SSL_set_connect_state (conn);
- if (SSL_connect (conn) <= 0 || conn->state != SSL_ST_OK)
+
+ scwt_ctx.ssl = conn;
+ if (run_with_timeout(opt.read_timeout, ssl_connect_with_timeout_callback,
+ &scwt_ctx)) {
+DEBUGP (("SSL handshake timed out.\n"));
+goto timeout;
+ }
+ if (scwt_ctx.result <= 0 || conn->state != SSL_ST_OK)
goto error;
ctx = xnew0 (struct openssl_transport_context);
@@ -441,6 +484,7 @@ ssl_connect_wget (int fd, const char *hostname)
error:
DEBUGP (("SSL handshake failed.\n"));
print_errors ();
+ timeout:
if (conn)
SSL_free (conn);
return false;
--
1.8.3.1
Re: [Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
Tim Rühsen writes: > I saw it, but took the routine from 'Mget' (It is my code, so I can > contribute > to Wget). This was a matter of time I had and I knew that it works. > The idea is to define 'connect_timeout' as the time nothing happens while > connecting. > But please feel free to change it to work as in wgnutls_read_timeout(). OK, I will commit your patch for now and try to find some time to change it. > To explain my repeated indentation problems: > The IDE I am working with (Netbeans) doesn't allow project-based indentation > style. Since i always have several dozen project open, almost all of them > having 'Linux' style, I have to hand-remove the tabs and replace them by > spaces (in each line). Pretty awful, especially because the 'artifical > intelligence' screws in from time to time. I really can't write larger Gnu > code, just some fixes or hacks (though I would like to, but my poor > nerves...). > > Does Eclipse do it any better ? I leave the answer to someone who uses Eclipse :-) I use Emacs and I have the opposite problem, configure another style for non-GNU projects. For what it matters, you can just ignore indentation, I always run again trough the patches and check these sort of issues before push them. Sometimes I try to deal also with missing ChangeLog entries, but most of the time it is too painful :-) -- Giuseppe
Re: [Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
Am Donnerstag, 11. Juli 2013 schrieb Giuseppe Scrivano:
> Tim Rühsen writes:
>
> > diff --git a/src/gnutls.c b/src/gnutls.c
> > index 54422fc..a3b4ecc 100644
> > --- a/src/gnutls.c
> > +++ b/src/gnutls.c
> >do
> > {
> >err = gnutls_handshake (session);
> > - if (err < 0)
> > +
> > + if (opt.connect_timeout && err == GNUTLS_E_AGAIN)
> > +{
> > + if (gnutls_record_get_direction (session))
> > +{
> > + /* wait for writeability */
> > + err = select_fd (fd, opt.connect_timeout, WAIT_FOR_WRITE);
> > +}
> > + else
> > +{
> > + /* wait for readability */
> > + err = select_fd (fd, opt.connect_timeout, WAIT_FOR_READ);
>
> since this is in a loop, should we also decrement the time we wait for
> at each iteration? We do something similar in wgnutls_read_timeout.
I saw it, but took the routine from 'Mget' (It is my code, so I can contribute
to Wget). This was a matter of time I had and I knew that it works.
The idea is to define 'connect_timeout' as the time nothing happens while
connecting.
But please feel free to change it to work as in wgnutls_read_timeout().
BTW, maybe Wget should have something like Curls -m (a total/maximum timeout).
I need such a thing in several projects (where I use Curl instead of Wget
because of this reason).
> I have fixed some indentation problems and also I had some troubles to
> apply your patch with "git am" so I had to apply the changes
> separately. Could you please use the version I have attached?
I locally revert my commit and pull it in from master.
To explain my repeated indentation problems:
The IDE I am working with (Netbeans) doesn't allow project-based indentation
style. Since i always have several dozen project open, almost all of them
having 'Linux' style, I have to hand-remove the tabs and replace them by
spaces (in each line). Pretty awful, especially because the 'artifical
intelligence' screws in from time to time. I really can't write larger Gnu
code, just some fixes or hacks (though I would like to, but my poor nerves...).
Does Eclipse do it any better ?
Regards, Tim
signature.asc
Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
Tim Rühsen writes:
> diff --git a/src/gnutls.c b/src/gnutls.c
> index 54422fc..a3b4ecc 100644
> --- a/src/gnutls.c
> +++ b/src/gnutls.c
>do
> {
>err = gnutls_handshake (session);
> - if (err < 0)
> +
> + if (opt.connect_timeout && err == GNUTLS_E_AGAIN)
> +{
> + if (gnutls_record_get_direction (session))
> +{
> + /* wait for writeability */
> + err = select_fd (fd, opt.connect_timeout, WAIT_FOR_WRITE);
> +}
> + else
> +{
> + /* wait for readability */
> + err = select_fd (fd, opt.connect_timeout, WAIT_FOR_READ);
since this is in a loop, should we also decrement the time we wait for
at each iteration? We do something similar in wgnutls_read_timeout.
I have fixed some indentation problems and also I had some troubles to
apply your patch with "git am" so I had to apply the changes
separately. Could you please use the version I have attached?
>From 68a0ded101f7a5cc92014012254bb6f9d31738b9 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen
Date: Thu, 11 Jul 2013 14:29:20 +0200
Subject: [PATCH] gnutls: honor connect timeout
---
src/ChangeLog | 4
src/gnutls.c | 60 ++-
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 5b978eb..efdc6b4 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2013-07-11 Tim Ruehsen
+
+* gnutls.c (ssl_connect_wget): respect connect timeout.
+
2013-04-26 Tomas Hozza (tiny change)
* log.c (redirect_output): Use DEFAULT_LOGFILE in diagnostic message
diff --git a/src/gnutls.c b/src/gnutls.c
index 54422fc..06f9020 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -374,6 +374,9 @@ static struct transport_implementation wgnutls_transport =
bool
ssl_connect_wget (int fd, const char *hostname)
{
+#ifdef F_GETFL
+ int flags = 0;
+#endif
struct wgnutls_transport_context *ctx;
gnutls_session_t session;
int err,alert;
@@ -441,11 +444,54 @@ ssl_connect_wget (int fd, const char *hostname)
return false;
}
+ if (opt.connect_timeout)
+{
+#ifdef F_GETFL
+ flags = fcntl (fd, F_GETFL, 0);
+ if (flags < 0)
+return flags;
+ if (fcntl (fd, F_SETFL, flags | O_NONBLOCK))
+return -1;
+#else
+ /* XXX: Assume it was blocking before. */
+ const int one = 1;
+ if (ioctl (fd, FIONBIO, &one) < 0)
+return -1;
+#endif
+}
+
/* We don't stop the handshake process for non-fatal errors */
do
{
err = gnutls_handshake (session);
- if (err < 0)
+
+ if (opt.connect_timeout && err == GNUTLS_E_AGAIN)
+{
+ if (gnutls_record_get_direction (session))
+{
+ /* wait for writeability */
+ err = select_fd (fd, opt.connect_timeout, WAIT_FOR_WRITE);
+}
+ else
+{
+ /* wait for readability */
+ err = select_fd (fd, opt.connect_timeout, WAIT_FOR_READ);
+}
+
+ if (err <= 0)
+{
+ if (err == 0)
+{
+ errno = ETIMEDOUT;
+ err = -1;
+}
+ break;
+}
+
+ if (err <= 0)
+break;
+}
+ else if (err < 0)
{
logprintf (LOG_NOTQUIET, "GnuTLS: %s\n", gnutls_strerror (err));
if (err == GNUTLS_E_WARNING_ALERT_RECEIVED ||
@@ -461,6 +507,18 @@ ssl_connect_wget (int fd, const char *hostname)
}
while (err == GNUTLS_E_WARNING_ALERT_RECEIVED && gnutls_error_is_fatal (err) == 0);
+ if (opt.connect_timeout)
+{
+#ifdef F_GETFL
+ if (fcntl (fd, F_SETFL, flags) < 0)
+return -1;
+#else
+ const int zero = 0;
+ if (ioctl (fd, FIONBIO, &zero) < 0)
+return -1;
+#endif
+}
+
if (err < 0)
{
gnutls_deinit (session);
--
1.8.3.1
--
Giuseppe
Re: [Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
Tomas Hozza writes: > From b565c9fcf37fb8d71b3c338f0ec8982295e283fe Mon Sep 17 00:00:00 2001 > From: Karsten Hopp > Date: Thu, 11 Jul 2013 11:27:35 +0200 > Subject: [PATCH] Fix timeout option when used with SSL > > Previously wget didn't honor the --timeout option if the remote host did > not answer SSL handshake > > Signed-off-by: Tomas Hozza > --- > src/openssl.c | 62 > ++- > 1 file changed, 53 insertions(+), 9 deletions(-) Thanks to have submitted the patch. It looks fine (I have just a minor comment below). Could I please ask you to provide also the entries for the ChangeLog file as part of your patch? It is a boring task but this is required by the GNU Coding standards[1]. If you have no time for this, I can do it. > diff --git a/src/openssl.c b/src/openssl.c > @@ -425,7 +461,14 @@ ssl_connect_wget (int fd, const char *hostname) >if (!SSL_set_fd (conn, FD_TO_SOCKET (fd))) > goto error; >SSL_set_connect_state (conn); > - if (SSL_connect (conn) <= 0 || conn->state != SSL_ST_OK) > + > + scwt_ctx.ssl = conn; > + if (run_with_timeout(opt.read_timeout, ssl_connect_with_timeout_callback, trailing whitespace. git am complained about it. -- Giuseppe 1) http://www.gnu.org/prep/standards/standards.html#Change-Logs
Re: [Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
Am Donnerstag, 11. Juli 2013 schrieb Tomas Hozza:
> Calling wget on https server with --timeout option does not work
> when the server does not answer SSL handshake. Note that this has
> been tested on wget-1.14 compiled with OpenSSL.
Hi,
here is the corresponding patch for GnuTLS.
Regards, Tim
From 5862c2e0e84838f40eda6332650bab10274bb211 Mon Sep 17 00:00:00 2001
From: Tim Ruehsen
Date: Thu, 11 Jul 2013 14:29:20 +0200
Subject: [PATCH] add connect timeout to gnutls code
---
src/ChangeLog | 6 ++
src/gnutls.c | 63 +--
2 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 5b978eb..c39cfcb 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2013-07-11 Tim Ruehsen
+
+* gnutls.c (ssl_connect_wget): respect connect timeout
+
2013-04-26 Tomas Hozza (tiny change)
* log.c (redirect_output): Use DEFAULT_LOGFILE in diagnostic message
diff --git a/src/gnutls.c b/src/gnutls.c
index 54422fc..a3b4ecc 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -374,6 +374,9 @@ static struct transport_implementation wgnutls_transport =
bool
ssl_connect_wget (int fd, const char *hostname)
{
+#ifdef F_GETFL
+ int flags = 0;
+#endif
struct wgnutls_transport_context *ctx;
gnutls_session_t session;
int err,alert;
@@ -441,11 +444,55 @@ ssl_connect_wget (int fd, const char *hostname)
return false;
}
+ if (opt.connect_timeout)
+{
+#ifdef F_GETFL
+ flags = fcntl (fd, F_GETFL, 0);
+ if (flags < 0)
+return flags;
+ if (fcntl (fd, F_SETFL, flags | O_NONBLOCK))
+return -1;
+#else
+ /* XXX: Assume it was blocking before. */
+ const int one = 1;
+ if (ioctl (fd, FIONBIO, &one) < 0)
+return -1;
+#endif
+}
+
/* We don't stop the handshake process for non-fatal errors */
do
{
err = gnutls_handshake (session);
- if (err < 0)
+
+ if (opt.connect_timeout && err == GNUTLS_E_AGAIN)
+{
+ if (gnutls_record_get_direction (session))
+{
+ /* wait for writeability */
+ err = select_fd (fd, opt.connect_timeout, WAIT_FOR_WRITE);
+}
+ else
+{
+ /* wait for readability */
+ err = select_fd (fd, opt.connect_timeout, WAIT_FOR_READ);
+}
+
+ if (err <= 0)
+{
+ if (err == 0)
+{
+ errno = ETIMEDOUT;
+ err = -1;
+}
+
+ break;
+}
+
+ if (err <= 0)
+ break;
+}
+ else if (err < 0)
{
logprintf (LOG_NOTQUIET, "GnuTLS: %s\n", gnutls_strerror (err));
if (err == GNUTLS_E_WARNING_ALERT_RECEIVED ||
@@ -461,6 +508,18 @@ ssl_connect_wget (int fd, const char *hostname)
}
while (err == GNUTLS_E_WARNING_ALERT_RECEIVED && gnutls_error_is_fatal (err) == 0);
+ if (opt.connect_timeout)
+{
+#ifdef F_GETFL
+ if (fcntl (fd, F_SETFL, flags) < 0)
+return -1;
+#else
+ const int zero = 0;
+ if (ioctl (fd, FIONBIO, &zero) < 0)
+return -1;
+#endif
+}
+
if (err < 0)
{
gnutls_deinit (session);
@@ -468,7 +527,7 @@ ssl_connect_wget (int fd, const char *hostname)
}
ctx = xnew0 (struct wgnutls_transport_context);
- ctx->session = session;
+ ctx->session = session;
fd_register_transport (fd, &wgnutls_transport, ctx);
return true;
}
--
1.8.3.2
signature.asc
Description: This is a digitally signed message part.
[Bug-wget] [PATCH] timeout option is ingnored if host does not answer SSL handshake (openssl)
Hi.
Calling wget on https server with --timeout option does not work
when the server does not answer SSL handshake. Note that this has
been tested on wget-1.14 compiled with OpenSSL.
This issue can be reproduced as follows:
- on first terminal run:
# nc -l localhost 12345
- on second terminal run:
wget --timeout=2 --no-check-certificate https://localhost:12345
Without the attached patch wget does not exit after specified
timeout.
Regards,
Tomas HozzaFrom b565c9fcf37fb8d71b3c338f0ec8982295e283fe Mon Sep 17 00:00:00 2001
From: Karsten Hopp
Date: Thu, 11 Jul 2013 11:27:35 +0200
Subject: [PATCH] Fix timeout option when used with SSL
Previously wget didn't honor the --timeout option if the remote host did
not answer SSL handshake
Signed-off-by: Tomas Hozza
---
src/openssl.c | 62 ++-
1 file changed, 53 insertions(+), 9 deletions(-)
diff --git a/src/openssl.c b/src/openssl.c
index 3924e41..189b334 100644
--- a/src/openssl.c
+++ b/src/openssl.c
@@ -256,19 +256,42 @@ struct openssl_transport_context {
char *last_error; /* last error printed with openssl_errstr */
};
-static int
-openssl_read (int fd, char *buf, int bufsize, void *arg)
-{
- int ret;
- struct openssl_transport_context *ctx = arg;
+struct openssl_read_args {
+ int fd;
+ struct openssl_transport_context *ctx;
+ char *buf;
+ int bufsize;
+ int retval;
+};
+
+static void openssl_read_callback(void *arg) {
+ struct openssl_read_args *args = (struct openssl_read_args *) arg;
+ struct openssl_transport_context *ctx = args->ctx;
SSL *conn = ctx->conn;
+ char *buf = args->buf;
+ int bufsize = args->bufsize;
+ int ret;
+
do
ret = SSL_read (conn, buf, bufsize);
- while (ret == -1
- && SSL_get_error (conn, ret) == SSL_ERROR_SYSCALL
+ while (ret == -1 && SSL_get_error (conn, ret) == SSL_ERROR_SYSCALL
&& errno == EINTR);
+ args->retval = ret;
+}
- return ret;
+static int
+openssl_read (int fd, char *buf, int bufsize, void *arg)
+{
+ struct openssl_read_args args;
+ args.fd = fd;
+ args.buf = buf;
+ args.bufsize = bufsize;
+ args.ctx = (struct openssl_transport_context*) arg;
+
+ if (run_with_timeout(opt.read_timeout, openssl_read_callback, &args)) {
+return -1;
+ }
+ return args.retval;
}
static int
@@ -386,6 +409,18 @@ static struct transport_implementation openssl_transport = {
openssl_peek, openssl_errstr, openssl_close
};
+struct scwt_context {
+ SSL *ssl;
+ int result;
+};
+
+static void
+ssl_connect_with_timeout_callback(void *arg)
+{
+ struct scwt_context *ctx = (struct scwt_context *)arg;
+ ctx->result = SSL_connect(ctx->ssl);
+}
+
/* Perform the SSL handshake on file descriptor FD, which is assumed
to be connected to an SSL server. The SSL handle provided by
OpenSSL is registered with the file descriptor FD using
@@ -398,6 +433,7 @@ bool
ssl_connect_wget (int fd, const char *hostname)
{
SSL *conn;
+ struct scwt_context scwt_ctx;
struct openssl_transport_context *ctx;
DEBUGP (("Initiating SSL handshake.\n"));
@@ -425,7 +461,14 @@ ssl_connect_wget (int fd, const char *hostname)
if (!SSL_set_fd (conn, FD_TO_SOCKET (fd)))
goto error;
SSL_set_connect_state (conn);
- if (SSL_connect (conn) <= 0 || conn->state != SSL_ST_OK)
+
+ scwt_ctx.ssl = conn;
+ if (run_with_timeout(opt.read_timeout, ssl_connect_with_timeout_callback,
+ &scwt_ctx)) {
+DEBUGP (("SSL handshake timed out.\n"));
+goto timeout;
+ }
+ if (scwt_ctx.result <= 0 || conn->state != SSL_ST_OK)
goto error;
ctx = xnew0 (struct openssl_transport_context);
@@ -441,6 +484,7 @@ ssl_connect_wget (int fd, const char *hostname)
error:
DEBUGP (("SSL handshake failed.\n"));
print_errors ();
+ timeout:
if (conn)
SSL_free (conn);
return false;
--
1.8.3.1
