cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: Request to review the code changes for NTLMv2 Support in Curl

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Tue, 21 Jan 2014 14:54:13 +0100 (CET)

On Tue, 21 Jan 2014, Prash Dush wrote:

> Kindly review the code changes and provide your valuable feedback.

Awesome contributition Prash. Let's get this in shape for merging immediately
after the next release!

1. Okay, first I think you should rebase your work on top of the current git
HEAD. Then you'll see that we don't have curl_rand.h anymore!

curl_ntlm_core.c:106:23: fatal error: curl_rand.h: No such file or directory
  #include "curl_rand.h"

2. Then please enable picky compiler warnings, for example with configure
--enable-debug as then you'll see this:

curl_ntlm_core.c:449:10: error: no previous prototype for 'Curl_hmac_md5'
[-Werror=missing-prototypes]
  CURLcode Curl_hmac_md5 (const unsigned char *key, unsigned int keylen,
           ^
curl_ntlm_core.c: In function 'Curl_ntlm_core_mk_ntlmv2_hash':
curl_ntlm_core.c:485:3: error: pointer targets in passing argument 2 of
'ascii_uppercase_to_unicode_le' differ in signedness [-Werror=pointer-sign]
    ascii_uppercase_to_unicode_le(identity, user, userlen);
    ^
curl_ntlm_core.c:382:13: note: expected 'const char *' but argument is of type
'const unsigned char *'
  static void ascii_uppercase_to_unicode_le(unsigned char *dest, const char
*src,
              ^
curl_ntlm_core.c:486:3: error: pointer targets in passing argument 2 of
'ascii_to_unicode_le' differ in signedness [-Werror=pointer-sign]
    ascii_to_unicode_le(identity+(userlen<<1), domain, domlen);
    ^
curl_ntlm_core.c:372:13: note: expected 'const char *' but argument is of type
'const unsigned char *'
  static void ascii_to_unicode_le(unsigned char *dest, const char *src,
              ^
curl_ntlm_core.c: In function 'Curl_ntlm_core_mk_lmv2_resp':
curl_ntlm_core.c:612:3: error: ISO C90 forbids mixed declarations and code
[-Werror=pedantic]
    unsigned char hmac_output[16];
    ^

3. You also need to pay slightly closer attention to the code style you use,
which you can check with this:

   "make checksrc"

It shows 93 warnings right now (most of them being for the same same error on
multiple places), like TABS or trailing whitespaces. Also note how we position
the open braces for if/while/do expressions.

4. Do you by any chance have any server around that supports NTLMv2 that we
can test this against? I would like to test it out against a live server first
to see that it works fine, then work on setting up tests for our own suite
that makes sure it keeps working the same way

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