bugGNU Wget - Bugs: bug #38554, Due to keep_alive handling login...

 
 

bug #38554: Due to keep_alive handling login to protected site using http://username:password@server/ does not work

Submitter:  Tomas Hozza <thozza>
Submitted:  Wed 20 Mar 2013 11:52:09 AM UTC
   
 
Category:  Program Logic Severity:  3 - Normal
Priority:  5 - Normal Status:  None
Privacy:  Public Assigned to:  None
Originator Name:  Open/Closed:  Open
Release:  trunk Operating System:  GNU/Linux
Reproducibility:  Every Time Fixed Release:  None
Planned Release:  None Regression:  Yes
Work Required:  None Patch Included:  Yes
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 05 Apr 2013 07:42:13 AM UTC, comment #3: 

Thank you for your response.

> wouldn't that leak the socket if it is not valid?


I scanned wget-1.14 without the patch and with the patch
using static analysis tool Coverity 6.5.1. It didn't find
any added errors, so there should be NO resource leak. I know
static analysis is not perfect, but from my experiences with
Coverity I must say it is very reliable.

Anyway there is the following construction:

if (persistent_available_p(...))
{
   ...
}
else if (host_lookup_failed)
{
   ...
}
else if (sock != -1)
{
   sock = -1;
}

persistent_available_p(...) returns false only if there is NO active
connection OR if we want to use new connection (SSL) or if there
was some problem with connection and it was invalidated. In other
words if persistent_available_p(...) doesn't returned true, then we
have to create new connection anyway.

> Should you perhaps use CLOSE_INVALIDATE?


Problem is that persistent_available_p(...) deals with "pconn.socket"
while outside of this function we use "sock" variable. Even if we used
CLOSE_INVALIDATE() in persistent_available_p() we have to set "sock"
to -1 because we might be using already closed socket. Also calling
CLOSE_INVALIDATE() on "sock" if persistent_available_p() returned
false would be not a good idea, because we might be closing already
closed socket.

> Can you also add the entry to the src/ChangeLog file?


I attached new patch with entry in src/ChangeLog.

(file #27778)

Tomas Hozza <thozza>
Thu 04 Apr 2013 05:08:30 PM UTC, comment #2: 

wouldn't that leak the socket if it is not valid?  Should you perhaps use CLOSE_INVALIDATE?

Can you also add the entry to the src/ChangeLog file?

Giuseppe Scrivano <gscrivano>
Group administrator
Wed 20 Mar 2013 11:56:06 AM UTC, comment #1: 

I think Bug https://savannah.gnu.org/bugs/?34141 may be connected
to this one. Sorry for the new Bug BUT in 34141 was no activity
what so ever so hopefully now it will be better.

Tomas Hozza <thozza>
Wed 20 Mar 2013 11:52:09 AM UTC, original submission:  

This problem exists for example when trying to download something
from Boa web server with Basic authentication.

Problem description from Red Hat Bugzilla [1] (on Fedora 18 with wget-1.13.4 AND wget-1.14)

The problem is that boa server answers to the initial GET packet with two 401
Unauthorised packets (at least in my case). From your dumps I see that boa is
answering with only one packet, but it is somehow broken. Although wget seems
to interpret it correctly it will close the connection without setting opened
socket variable to -1. Then it fails trying to write to that already closed
socket. This repeats for default maximum retries times.

Reason why wget-1.12 works is how it handle connections. In wget-1.12 the
connection established for GET request is closed before authentication and
the socket variable is set to -1. So when wget tries to authenticate to the
server, it establishes new connection and everything works.

Reason why --user and --password options works is that those credentials are
used as default for site-wide authentication. What wget does in this case is
pretty much the same as when you pass credentials in the URL with one difference.
When wget receives 401 Unauthorised from server and have those "default"
credentials set AND have NOT credentials in URL, then it adds the host address
to a list of hosts which request authentication. After that it also closes the
initial connection, then fails to write new GET request with authentication
credentials. BUT when it tries for the second time it will find out that the
host address is in list of hosts which request authentication and therefore
it will send GET request including authentication credentials. So that's why
it works.

I think the proper fix for this would be to set the socket variable to -1
in case of a problem when authenticating and closing the socket. This will
make sure that wget will not try to write data to already closed socket
but rather it will create new connection.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=912358

Tomas Hozza <thozza>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gscrivano (Posted a comment)
  •  

    Follow 2 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-04-05 thozza Attached File- Added 0001-Wget-should-set-sock-variable-to-1-if-no-persistent-.patch, #27778
    2013-03-20 thozza Attached File- Added 0001-Wget-should-set-sock-variable-to-1-if-no-persistent-.patch, #27637

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code