2010/6/30 Dag-Erling Smørgrav <d...@des.no>:
> Tom Evans <tevans...@googlemail.com> writes:
>> Sorry to bump this again. I've diluted this issue down to the core
>> points and raised as a pr - can someone take a look, see if my
>> solution is correct and commit if appropriate?
>
> Sorry, fell through the cracks. Your analysis is correct, but I'm not
> 100% sure about the patch. Can you verify that your change does not
> introduce the possibility of an infinite loop in edge cases? I don't
> *think* it does, but like I said, I'm not 100% sure and I don't have
> time to reacquaint myself with the code right now, especially not that
> particularly nasty part of it :)
>
> DES
> --
> Dag-Erling Smørgrav - d...@des.no
>
Like you said, it's quite a large chunk of code to understand. I think
you might be right, there could be a situation where a misbehaving
proxy server could make it loop. The process is like this:
http_auth_challenges_t proxy_challenges struct initialized (line 1497).
The flag 'valid' on the struct is initialized to 0 (line 656)
We enter the loop for the first time.
We dont add proxy auth the first time through the loop, as 'valid'
flag not set (line 1586)
We receive the reply and switch on the response code (line 1676)
If we receive HTTP_NEED_PROXY_AUTH, and the 'valid' flag is not set
(implying we didn't send creds), we simply continue to execute the
loop (lines 1703 - 1716). This is where the patch I supplied
increments the maximum loop counter.
The loop now processes the received headers, and when it receives the
'Proxy-Authenticate' header, it calls http_parse_authenticate with
proxy_challenges (line 1795)
This then populates the proxy_challenges struct, setting the valid flag.
Having processed the headers, the loop then checks that receiving a
HTTP_NEED_PROXY_AUTH response has resulted in us now having valid flag
set in proxy_challenges, otherwise we goto ouch (out of the loop)
(line 1806).
The loop ends, and we go through again.
I thought for a moment while tracing it through that if a misbehaving
proxy server sent a 407 response, but did not add a Proxy-Authenticate
header, then we could increase the loop counter but without adding any
proxy auth, which would mean an infinite loop.
However, there is already that sanity check at the end of the loop to
check that if we received a proxy authentication error, and still dont
have credentials to supply, then we quickly jump out of the loop.
I guess that is a little strange, that we could supply proxy auth
credentials, but because the server didnt ask for them correctly, we
didnt give them - although that would be quite broken behaviour on the
part of the proxy server.
I think this does show that the patch could be made a little better.
We only want to go through the loop one more time if we have
credentials to send, and we have credentials to send if the rv of
http_parse_authenticate is good.
I also think the same bug would affect fetching from servers requiring
authentication, so I've made the same fix there as well.
New patch attached
Cheers
Tom
Index: http.c
===================================================================
RCS file: /home/ncvs/src/lib/libfetch/http.c,v
retrieving revision 1.78.2.5
diff -u -r1.78.2.5 http.c
--- http.c 27 Jan 2010 14:54:48 -0000 1.78.2.5
+++ http.c 1 Jul 2010 13:45:06 -0000
@@ -1786,12 +1786,14 @@
case hdr_www_authenticate:
if (conn->err != HTTP_NEED_AUTH)
break;
- http_parse_authenticate(p, &server_challenges);
+ if (http_parse_authenticate(p,
&server_challenges))
+ ++n;
break;
case hdr_proxy_authenticate:
if (conn->err != HTTP_NEED_PROXY_AUTH)
break;
- http_parse_authenticate(p, &proxy_challenges);
+ if (http_parse_authenticate(p,
&proxy_challenges) == 0);
+ ++n;
break;
case hdr_end:
/* fall through */
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
