Re: [Bug-wget] [PATCH] Trust on first use
If nobody has complains about this change, we should add some documentation, (possibly a test?), and rewrite the commit message to list the changes using a ChangeLog style. Giuseppe If this gets accepted, I would love the opportunity to write the corresponding test. Unless it would be preferred that the patch author handle it. Krell
Re: [Bug-wget] --connect-timeout doesn't work on Windows
Date: Sat, 14 Mar 2015 21:18:26 +0100
From: Gisle Vanem gva...@yahoo.no
I see the from the call-stack that connect() is hanging inside
Gnulib:
...
ntdll.dll!ZwWaitForSingleObject+0xc
ntdll.dll!RtlInterlockedPushEntrySList+0x32b
ntdll.dll!RtlInterlockedPushEntrySList+0x355
ntdll.dll!RtlResetNtUserPfn+0x3f
wget.exe!close_fd_maybe_socket+0x36
wget.exe!execute_close_hooks+0x30
wget.exe!execute_all_close_hooks+0x17
wget.exe!rpl_close+0x12
wget.exe!connect_to_host+0x3ea
wget.exe!gethttp+0x686
wget.exe!http_loop+0x430
wget.exe!retrieve_url+0x1cb
..
And from the command 'wget -d --connect-timeout=4 http://192.0.2.1:12345/' +
^C:
Setting --connect-timeout (connecttimeout) to 4
DEBUG output created by Wget 1.15.00 (09-March-2015) on Win-8.1. Build
9600 (MSVC).
...
seconds 0.00, Connecting to 192.0.2.1:12345... seconds 4.00,
So it seems the 'run_with_timeout()' used to connect works fine since
the thread is not running. So it seems the Gnulib's close() is blocking
for an unknown reason.
Attaching a debugger shows that it hangs inside the call to
WSAEnumNetworkEvents, here:
/* Test whether fd refers to a socket. */
sock = FD_TO_SOCKET (fd);
ev.lNetworkEvents = 0xDEADBEEF;
WSAEnumNetworkEvents (sock, NULL, ev);
if (ev.lNetworkEvents != 0xDEADBEEF)
{
/* fd refers to a socket. */
/* FIXME: other applications, like squid, use an undocumented
_free_osfhnd free function. But this is not enough: The 'osfile'
flags for fd also needs to be cleared, but it is hard to access it.
Instead, here we just close twice the file descriptor. */
if (closesocket (sock))
This is Gnulib trying to establish that the file descriptor refers to
a socket. WSAEnumNetworkEvents waits forever, I think because we
never succeeded to connect, and perhaps also because the socket is
blocking.
The following simple kludge works around the problem for me:
--- src/connect.c~0 2014-12-02 09:49:37.0 +0200
+++ src/connect.c 2015-03-17 17:14:48.414375000 +0200
@@ -364,7 +364,12 @@ connect_to_ip (const ip_address *ip, int
logprintf. */
int save_errno = errno;
if (sock = 0)
- fd_close (sock);
+ {
+#ifdef WIN32
+ if (errno != ETIMEDOUT)
+#endif
+ fd_close (sock);
+ }
if (print)
logprintf (LOG_VERBOSE, _(failed: %s.\n), strerror (errno));
errno = save_errno;
After applying this, I get the expected results:
wget --connect-timeout=1 -t 3 http://192.0.2.1:12345/
--2015-03-17 17:18:07-- http://192.0.2.1:12345/
Connecting to 192.0.2.1:12345... failed: Connection timed out.
Retrying.
--2015-03-17 17:18:09-- (try: 2) http://192.0.2.1:12345/
Connecting to 192.0.2.1:12345... failed: Connection timed out.
Retrying.
--2015-03-17 17:18:12-- (try: 3) http://192.0.2.1:12345/
Connecting to 192.0.2.1:12345... failed: Connection timed out.
Giving up.
If there's a more elegant way of fixing this, I'm all ears.
Re: [Bug-wget] [PATCH] UTF-8-ify contributors' names in wget.texi (#40472)
Rohan Prinja rohan.pri...@gmail.com writes: Yes, I encountered it, sorry for not checking first. Turns out some more changes were required. Attached is a new patch that also touches doc/texi2pod.pl and doc/Makefile.am, and builds without errors. I commit-amended before creating this patch instead of creating a new commit, I hope that was the right thing to do. yes, I've tweaked the commit message to include the changes to Makefile.am and pushed your patch. Congratulations on your first contribution! Giuseppe
Re: [Bug-wget] [PATCH] UTF-8-ify contributors' names in wget.texi (#40472)
Thank you :) On 17 March 2015 at 14:23, Giuseppe Scrivano gscriv...@gnu.org wrote: Rohan Prinja rohan.pri...@gmail.com writes: Yes, I encountered it, sorry for not checking first. Turns out some more changes were required. Attached is a new patch that also touches doc/texi2pod.pl and doc/Makefile.am, and builds without errors. I commit-amended before creating this patch instead of creating a new commit, I hope that was the right thing to do. yes, I've tweaked the commit message to include the changes to Makefile.am and pushed your patch. Congratulations on your first contribution! Giuseppe
Re: [Bug-wget] [PATCH] two Coverity bugs fixed
Ack. They look fine to me On Tue, Mar 17, 2015 at 2:02 AM, Tim Rühsen tim.rueh...@gmx.de wrote: I would like to push these two patches tomorrow if nobody complains. Tim -- Thanking You, Darshit Shah
Re: [Bug-wget] [PATCH] Trust on first use
Thanks for your contribution!
I've some comments on the code:
Molnár Géza packman.g...@gmail.com writes:
diff --git a/src/gnutls.c b/src/gnutls.c
index 5a89e06..38bf2af 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -76,6 +76,85 @@ key_type_to_gnutls_type (enum keyfile_type type)
preprocessor macro. */
static gnutls_certificate_credentials_t credentials;
+
+int
+save_trusted_certificate(gnutls_x509_crt_t* cert, const char* host){
please leave a space between the function name and '(', same applies to
the code below.
+ FILE* of;
+ int result = -1;
+ char* outputfile = aprintf(%s.crt,host); // for now
please not use C++ comments, use the /* C style. */ (and drop the
comment above completely). Also, put the * near the variable name, so
it will be char *outputfile, should be changed in the code below too,
where it happens.
+
+ if(!file_exists_p(outputfile))
+{
+ size_t bufferSize = 4096;
+ char *cert_data = (char*) malloc(bufferSize*sizeof(char));
+ if(!cert_data) return -1; // Could not allocate memory, let's bail
out. Maybe an error message here?
just use xmalloc, it will abort if the memory cannot be allocated.
+ result = gnutls_x509_crt_export (*cert, GNUTLS_X509_FMT_PEM,
cert_data, bufferSize);
I think you should retry with a bigger buffer (use xrealloc) on a
GNUTLS_E_SHORT_MEMORY_BUFFER error, the needed size will be in
bufferSize.
diff --git a/src/openssl.c b/src/openssl.c
index b8a9614..07d7cc4 100644
--- a/src/openssl.c
+++ b/src/openssl.c
@@ -270,6 +270,7 @@ ssl_init (void)
SSL_CTX_set_default_verify_paths (ssl_ctx);
SSL_CTX_load_verify_locations (ssl_ctx, opt.ca_cert, opt.ca_directory);
+ if(opt.trust_model != built_in_only) SSL_CTX_load_verify_locations
(ssl_ctx, opt.ca_cert, .);
move the if body to a new line.
if (opt.crl_file)
{
@@ -631,6 +632,37 @@ static char *_get_rfc2253_formatted (X509_NAME *name)
return out ? out : xstrdup();
}
+int
+save_trusted_certificate(X509 *cert, const char* host){
+ FILE* of;
+ int result = 0;
+ char *signer_hash = aprintf(%x,X509_subject_name_hash(cert));
+ char *outputfile = NULL;
+ int n = -1;
+
+ do
+ {
+free(outputfile);
+++n;
+outputfile = aprintf(%s.%d,signer_hash,n);
+ }
space after commas.
+ while(file_exists_p(outputfile));
+
+ of = fopen(outputfile,wb);
+ if(of)
+{
+ result = PEM_write_X509(of,cert);
+ fclose(of);
+}
+
+ if(!result)
+logprintf (LOG_NOTQUIET, _(ERROR: Could not save certificate to file:
%s\n), outputfile);
+
+ free(outputfile);
+ free(signer_hash);
+ return result;
+}
+
/* Verify the validity of the certificate presented by the server.
Also check that the common name of the server, as presented by
its certificate, corresponds to HOST. (HOST typically comes from
@@ -654,6 +686,7 @@ ssl_check_certificate (int fd, const char *host)
long vresult;
bool success = true;
bool alt_name_checked = false;
+ bool could_save_crt = false;
/* If the user has specified --no-check-cert, we still want to warn
him about problems with the server's certificate. */
@@ -705,6 +738,15 @@ ssl_check_certificate (int fd, const char *host)
case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT:
logprintf (LOG_NOTQUIET,
_( Self-signed certificate encountered.\n));
+ if(opt.trust_model == trust_on_first_use)
+{
+logprintf (LOG_NOTQUIET, _(Saving certificate as requested.\n)
);
+could_save_crt = save_trusted_certificate(cert,host);
+}
+ else
+if(opt.trust_model != built_in_only)
+ logprintf (LOG_NOTQUIET, _(Use
--trust-mode=trust-on-first-use to save this certificate as trusted!\n));
+
break;
case X509_V_ERR_CERT_NOT_YET_VALID:
logprintf (LOG_NOTQUIET, _( Issued certificate not yet
valid.\n));
@@ -718,7 +760,7 @@ ssl_check_certificate (int fd, const char *host)
logprintf (LOG_NOTQUIET, %s\n,
X509_verify_cert_error_string (vresult));
}
- success = false;
+ success = could_save_crt;
/* Fall through, so that the user is warned about *all* issues
with the cert (important with --no-check-certificate.) */
}
diff --git a/src/options.h b/src/options.h
index 5a1532c..9363c43 100644
--- a/src/options.h
+++ b/src/options.h
@@ -277,6 +277,14 @@ struct options
char *encoding_remote;
char *locale;
+#ifdef HAVE_SSL
+ enum trust_types{
space here before '{'
If nobody has complains about this change, we should add some
documentation, (possibly a test?), and rewrite the commit message to
list the changes using a ChangeLog style.
Giuseppe
