On 3 Apr 2006, at 17:21, Daniel Stenberg wrote:
> [...] A few more comments / questions:
>
> 1. What is the comment here meaning (from the patch)? What is the
> alternative
> mentioned?
>
> + lm_resp(lmbuffer, &ntlm->nonce[0], lmresp);
> + /* TODO alternative: lm_resp(ntbuffer, &ntlm->nonce[0],
> lmresp); */

Citing mozilla source "According to http://davenport.sourceforge.net/
ntlm.html#ntlmVersion2, the correct way to not send the LM hash is
to send the NTLM hash twice in both the LM and NTLM response fields."

The LM hash is less secure than the NTLM one, but older servers (the
windows 3.1 - 95 - 98 - millenium line) don't support the NTLM hash.
Due to this reason as far as I know even the latest windows clients
by default use both LM and NTLM hashes. Even firefox decides whether
to sent the LM hash or not based on a preferences setting (not
available through the usual GUI) which defaults to "send".

The "alternative" (i.e. not senting the LM hash) mentioned by the
comment is thus safer but less compatible. Maybe I could add a flag
to easy_setopt to choose whether sending the LM hash or not but as no
client uses by default it seems like an overkill. Also note that curl
currently sends LM hash only and thus the patch is not unsafer than
what curl does now.

> 2. it uses C99/c++ comments, while we aim for C89 compatibility (at
> least)

This was another mistake on my part.

> 3. The failure return if gethostname() fails should probably also
> use failf()
> to set a human error string.

Ok.

> But is that failure really that serious? The
> current code (before this patch) never sets a host name in the
> first place
> so perhaps just using a blank is an acceptable way if it fails
> to fetch it?
> (possibly with an infof() display)

Uhm, I'm returing an error because this is what firefox does in this
case. If I correctly understand the publicly available NTLM documents
the workstation name is used (together with the domain) only to let
the server choose local instead of network authentication and thus in
fact it doesn't seem so critical sending it. Ok, I'll change it to
non-fatal.

If there are no further comments I'll post an updated patch tomorrow
morning (CEST).
Received on 2006-04-03