Serf 1.2 + Subversion + digest auth not working.

[GoogleCodeExporter]

A Subversion user (Klaus Welch <klaus.welch@advantest.com>) reported on the 
users@subversion.apache.org list that Subversion 1.8.0-rc2 was returning a Serf 
context error while trying to do a simple 'svn info' against a server 
configured for digest authentication.  (See 
http://svn.haxx.se/users/archive-2013-05/0323.shtml for the discussion.)

I was able to reproduce this problem, too.

Upon debugging, I find that Serf is raising an error in the following condition 
(in auth_digest.c:serf__validate_response_digest_auth()): 

        /* Incorrect response-digest in Authentication-Info header. */
        if (strcmp(rspauth, resp_hdr_hex) != 0) {
            return SERF_ERROR_AUTHN_FAILED;
        }

Given the mismatched MD5's per the condition, I examined a wiretrace to see if 
any information there was out of sorts.  All of the visible components (cnonce, 
rspauth, nc, etc.) checkout out fine.  That left the calculated ones:  ha1 and 
ha2.  So I added some printf's and re-ran the operation:

build_digest_ha1: username(cmpilato), realm_name(Subversion Repository 
(Digest)), password(cmpilato)
build_digest_ha2: method(OPTIONS), uri(/tests-digest/repos)
build_digest_ha2: method(), uri(/tests-digest/repos)
build_digest_ha2: method(OPTIONS), uri(/tests-digest/repos)
build_digest_ha2: method(), uri(/tests-digest/repos)
build_digest_ha2: method(PROPFIND), uri(/tests-digest/repos/!svn/rvr/27)
build_digest_ha2: method(), uri(/tests-digest/repos)

That final build_digest_ha2() call is the one that comes shortly before the 
error condition is raise.  I was able to determine that, had build_digest_ha2() 
been called with the URI "/tests-digest/repos/!svn/rvr/27" (instead of 
"/tests-digest/repos"), the MD5 calculations which make use of the ha2 would 
have yielded the correct value.

I don't know Serf particularly well, and I know Digest auth even less, but it 
seems odd to me that the one HA2 value would be generated using the actual URI 
of the request, but in the validation step, the URI that's used is pulled from 
conn->host_info (which I must assume is setup per-connection, not per-request).

In my opinion, this is a showstopper for the Subversion 1.8.0 release, which 
depends on Serf for HTTP communication.

Original issue reported on code.google.com by cmpilato on 31 May 2013 at 6:24

[GoogleCodeExporter]

I'll have a look.

Original comment by lieven.govaerts@gmail.com on 31 May 2013 at 7:04

• Changed state: Accepted

[GoogleCodeExporter]

SIDEBAR: It appears that this bit of code is the only code that tries to use 
conn->host_info.path.  But when conn->host_url -- which presumably is supposed 
to represent the same information as conn->host_info, just in another format -- 
is built, the path information is stripped (per the APR_URI_UNP_OMITPATHINFO 
flag to apr_uri_unparse()).

I might suggest that conn->host_info should be re-parsed from the host_url to 
ensure that the two stay in sync?

{{{
Index: outgoing.c
===================================================================
--- outgoing.c  (revision 1877)
+++ outgoing.c  (working copy)
@@ -1227,7 +1227,7 @@
     c->host_url = apr_uri_unparse(c->pool,
                                   &host_info,
                                   APR_URI_UNP_OMITPATHINFO);
-    c->host_info = host_info;
+    (void)apr_uri_parse(c->pool, c->host_url, &(c->host_info));

     *conn = c;

}}}

Original comment by cmpilato on 31 May 2013 at 7:24

[GoogleCodeExporter]

Should be fixed in r1885. The response digest in the Authentication-Info header 
is indeed calculated with the uri of the original request. (RFC 2617, 3.2.3).

When you say: " I was able to determine that, had build_digest_ha2() been 
called with the URI "/tests-digest/repos/!svn/rvr/27" (instead of 
"/tests-digest/repos"), the MD5 calculations which make use of the ha2 would 
have yielded the correct value.", I suppose you tested this by changing this 
uri in gdb? Because, in the response handler the uri of the request is not 
directly available, so I had to work around this a bit.

FYI: when I originally implemented digest this worked ok with subversion, but 
serf didn't validate the successful response headers yet. I've fixed this in 
r1669 for auth_kerb, but then broke auth_digest as that code path had never 
been tested before.

L.

Original comment by lieven.govaerts@gmail.com on 31 May 2013 at 9:57

• Changed state: Fixed

[GoogleCodeExporter]

I didn't change the URI in GDB, no.  Rather, I manually calculate the checksums.

"What would happen if HA2 was md5(method + ':' + '/correct/uri') instead?  Aha! 
 The result is what's expected!"

Either way, glad you found a solution.  It's the same solution I was thinking 
to implement, but I second-guessed myself.  "Surely they'd have stored the URI 
associated with the request_t if it wasn't just outright wrong to do so..."  
Guess not. :-)

Original comment by cmpilato on 1 Jun 2013 at 2:01