On Mon, 3 Nov 2014, Stefan Bühler wrote:

> * "Proxy-Authentication: Negotiate" headers are never removed;

Right, since we concluded Negotiate has to be treated as NTLM all over it
needs to be converted into connection oriented instead of request oriented.

> * Not reusing easy handles (curl_easy_reset) basically breaks anything
> related to (proxy) authentication.

Can you please back this up with test cases that don't use Negotiate or NTLM?
Our test suite works perfectly fine for such cases so it should be easy to
repeat and then we can work on making this working in case of bugs.

> When I said curl_easy_reset and curl_easy_cleanup + curl_easy_init
> (while using multi handle) should behave the same I didn't mean you
> should break curl_easy_reset :) You should fix cleanup + init by
> putting more state into the multi-handle; or at least add a note to
> the docs recommending to reuse the easy handles. (Because to me it
> sounded like curl_easy_reset was just saving some frees and allocs
> when used with multi handle, and this is certainly not the case).

I still believe you're mostly hanging on to side-effects and/or are seeing
bugs (mostly related to the Negotiate mess), not finding actual conceptual
flaws in either of the functions. And you haven't backed up your claims with
proofs either I think.

> From a security point of view you are screwed anyway with multi
> handle; if you reuse a connection which was authenticated (NTLM or
> Negotiate) in a new handle you inherit the credentials, whether you
> passed them again or not.

Nope. We only re-use NTLM connections that have matching credentials. If not,
there's another bug. We've had such bugs in the past.

Again, Negotiate surely suffers from this since it isn't handled correctly.

> (And right now you also inherit the Basic and Digest proxy authentication
> headers; Basic will continue to work, and Digest often works more than
> once.)

Basic and Digest state lives in the easy handles. They can change connections
and continue using HTTP auth.

> * curl does an unneeded additional round-trip for authentication:
> in the first round it iterates over the available authentication
> headers, then picks one - but doesn't actually handle the header.
> It then sends a new request before actually handling the header;
> instead it should iterate over the headers from the first request
> again.

I don't think I understand this case. Can you elaborate a bit more perhaps
with some real data to show?

> I extended my patch for headers (point 1) to also fix point 4 and 5
> in the list above, and I'd like to get some feedback; especially
> whether this:
>
> bool proxy_connect_auth = (0 == http->writebytecount) &&
> (!conn->bits.protoconnstart);
>
> is a correct way to detect whether we are currently proxy-
> authenticating a CONNECT

Isn't conn->tunnel_state[sockindex] better for that? Or
data->state.authproxy.done ?

-- 
  / daniel.haxx.se

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-11-03