[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: crash in serf credential callback

From: Lieven Govaerts <lgo_at_apache.org>
Date: Sun, 2 Mar 2014 11:44:50 +0100

On Sun, Mar 2, 2014 at 12:17 AM, Lieven Govaerts <lgo_at_apache.org> wrote:
> Update:
> On Fri, Feb 28, 2014 at 12:45 PM, Lieven Govaerts <lgo_at_apache.org> wrote:
>> Hi,
>> On Wed, Feb 26, 2014 at 8:18 PM, Stefan Kueng <tortoisesvn_at_gmail.com> wrote:
>>> Hi,
>>> There's a crash happening in libsvn_ra_serf\util.c in the function
>>> svn_ra_serf__credentials_callback. There's a full crash dump available from
>>> here:
>>> https://www.crash-server.com/Problem.aspx?ClientID=tsvn&ProblemID=58311
>>> From what I can figure out from that crash dump is that the 'session' baton
>>> is not set (it's NULL), which leads to a segfault in the line:
>>> *username = apr_pstrdup(pool, session->proxy_username);
>>> The http return code is 407.
>>> But I can't figure out if this is a problem in serf or the svn library.
>>> Maybe one of you guys can have a look at this?
>> I'm looking into this report but haven't reached a conclusion yet. The
>> dump doesn't contain a correct value for each function parameters and
>> local variables, so debugging isn't easy.
>> The user is trying to connect over a http proxy to a https server; The
>> proxy requests username & password (Basic authn scheme). I see that
>> the baton passed to svn_ra_serf__credentials_callback isn't NULL, but
>> is a pointer to the ssl tunnel bucket context.
>> This points me to serf__provide_credentials.
>> Normally that function should setup the next request - the one
>> provided by the application that lead to the setup of the ssl tunnel -
>> so that the response_handler from that request can be passed to the
>> credentials callback.
>> That mechanism must have failed. I see that the first request is
>> indeed a ssltunnel request, but the conn->state is SERF_CONN_INIT
>> instead of SERF_CONN_SETUP_SSLTUNNEL. This explains why the mechanism
>> to find and setup the first application request isn't triggered.
>> The remaining question is how the connection can be in SERF_CONN_INIT
>> state while handling authentication of the response to the CONNECT
>> request. The only place where the connection state is reset is in
>> reset_connection, but I haven't found a scenario yet where that
>> function is called before an outstanding response on that connection
>> is handled.
> I see that the connection queue contains 3 requests:
> 1) a ssltunnel+priority request, handler_baton points to the ssltunnel context
> 2) a normal request, handler baton also points to the ssltunnel context
> 3) a normal request, handler_baton points to the svn_ra_serf
> That second request shouldn't be there. This is a requeued CONNECT
> request but without the ssltunnel flag set.
> I'm quite confident that the problem is the handling of a cancelled
> CONNECT request in ssltunnel.c/handle_response(). Besides the fact
> that it requeues a CONNECT request as normal request, ssltunnel
> requests should not be requeued in case of closed/reset connections,
> because a new CONNECT request will be created when opening the new
> socket.
> Unfortunately I can't imagine a scenario which triggers this code path
> (I've tried some), so I can't write a test case to simulate it.

I've finally found a matching scenario:
1. Serf sends a CONNECT request without authn headers, the (squid)
proxy responds with a 407 response.
2. Serf calls the application to get creds, sends a new CONNECT
request with authn headers
3. The server does not accept the credentials, responds with a 407 and
then closes the connection.
4. Serf reads APR_ECONNRESET from the socket (instead of the normal APR_EOF)

This assumes that squid does not close the connection on the initial
407, but only when incorrect credentials were provided. I don't know
if this is the case.

This also assumes that a closed connection can result in
APR_ECONNRESET. We know this to be true on Windows, serf already has
checks to handle this.

I can reproduce the crash on my Mac with a test triggering steps 1-3,
and if I alter the auth code to simulate step 4. Modifying the
ssltunnel code to not requeue the CONNECT request seems to fix the
issue. I'll cleanup and commit the patch later.


>>> Stefan
>>> --
>>> ___
>>> oo // \\ "De Chelonian Mobile"
>>> (_,\/ \_/ \ TortoiseSVN
>>> \ \_/_\_/> The coolest interface to (Sub)version control
>>> /_/ \_\ http://tortoisesvn.n
Received on 2014-03-02 11:45:58 CET

This is an archived mail posted to the Subversion Dev mailing list.