ntlm: remove warnings and improve code style

PATCH In review Antonio Quartulli 4 participants 17 messages Jul 10, 2017 – Aug 11, 2017

lore

Share & tools

lore

Antonio Quartulli

author

2017-07-10 04:34 UTC #0 lore raw

cover ntlm: remove warnings and improve code style

This small patcheset is aimed to simply remove some unnecessary warnings and improve the readability of ntlm.c.

Compile tests have been performed with "-Wall -std=c99".

This is another step towards having a clean build when using -Wall.

Cheers,

Antonio Quartulli (4):
ntlm: convert binary buffers to uint8_t *
ntlm: avoid useless cast
ntlm: restyle compressed multiple function calls
ntlm: improve code style and readability

 src/openvpn/ntlm.c | 129 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 74 insertions(+), 55 deletions(-)

Signature · 1 line

2.13.2

Antonio Quartulli

author

2017-07-10 04:34 UTC #1 lore raw

PATCH 1/4 ntlm: convert binary buffers to uint8_t *

several binary buffers in the ntml component are stored as char *, however this generates a lot of warnings, because hashing functions expect something unsigned.

Convert binary buffers to uint8_t *, while use explicit cast for buffers that are really carrying a string inside.

This commit removes several warnings from nmtl.c that you can catch with "-Wall -std=c99".

Signed-off-by Antonio Quartulli <[email protected]...>

---
 src/openvpn/ntlm.c | 51 +++++++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 24 deletions(-)

src/openvpn/ntlm.c +27 ~~-24~~

index 0b1163ee..76dda576 100644
@@ -71,31 +71,32 @@ create_des_keys(const unsigned char *hash, unsigned char *key)
 }
 
 static void
-gen_md4_hash(const char *data, int data_len, char *result)
+gen_md4_hash(const uint8_t *data, int data_len, uint8_t *result)
 {
     /* result is 16 byte md4 hash */
     const md_kt_t *md4_kt = md_kt_get("MD4");
-    char md[MD4_DIGEST_LENGTH];
+    uint8_t md[MD4_DIGEST_LENGTH];
 
     md_full(md4_kt, data, data_len, md);
     memcpy(result, md, MD4_DIGEST_LENGTH);
 }
 
 static void
-gen_hmac_md5(const char *data, int data_len, const char *key, int key_len,char *result)
+gen_hmac_md5(const uint8_t *data, int data_len, const uint8_t *key, int key_len,
+             uint8_t *result)
 {
     const md_kt_t *md5_kt = md_kt_get("MD5");
     hmac_ctx_t *hmac_ctx = hmac_ctx_new();
 
     hmac_ctx_init(hmac_ctx, key, key_len, md5_kt);
-    hmac_ctx_update(hmac_ctx, (const unsigned char *)data, data_len);
-    hmac_ctx_final(hmac_ctx, (unsigned char *)result);
+    hmac_ctx_update(hmac_ctx, data, data_len);
+    hmac_ctx_final(hmac_ctx, result);
     hmac_ctx_cleanup(hmac_ctx);
     hmac_ctx_free(hmac_ctx);
 }
 
 static void
-gen_timestamp(unsigned char *timestamp)
+gen_timestamp(uint8_t *timestamp)
 {
     /* Copies 8 bytes long timestamp into "timestamp" buffer.
      * Timestamp is Little-endian, 64-bit signed value representing the number of tenths of a microsecond since January 1, 1601.

@@ -195,19 +196,19 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
      */
 
     char pwbuf[sizeof(p->up.password) * 2]; /* for unicode password */
-    unsigned char buf2[128]; /* decoded reply from proxy */
-    unsigned char phase3[464];
+    uint8_t buf2[128]; /* decoded reply from proxy */
+    uint8_t phase3[464];
 
-    char md4_hash[MD4_DIGEST_LENGTH+5];
-    char challenge[8], ntlm_response[24];
+    uint8_t md4_hash[MD4_DIGEST_LENGTH + 5];
+    uint8_t challenge[8], ntlm_response[24];
     int i, ret_val;
 
-    char ntlmv2_response[144];
+    uint8_t ntlmv2_response[144];
     char userdomain_u[256];     /* for uppercase unicode username and domain */
     char userdomain[128];       /* the same as previous but ascii */
-    char ntlmv2_hash[MD5_DIGEST_LENGTH];
-    char ntlmv2_hmacmd5[16];
-    char *ntlmv2_blob = ntlmv2_response + 16;     /* inside ntlmv2_response, length: 128 */
+    uint8_t ntlmv2_hash[MD5_DIGEST_LENGTH];
+    uint8_t ntlmv2_hmacmd5[16];
+    uint8_t *ntlmv2_blob = ntlmv2_response + 16;     /* inside ntlmv2_response, length: 128 */
     int ntlmv2_blob_size = 0;
     int phase3_bufpos = 0x40;     /* offset to next security buffer data to be added */
     size_t len;

@@ -246,12 +247,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
 
 
     /* fill 1st 16 bytes with md4 hash, disregard terminating null */
-    gen_md4_hash(pwbuf, unicodize(pwbuf, p->up.password) - 2, md4_hash);
+    gen_md4_hash((uint8_t *)pwbuf, unicodize(pwbuf, p->up.password) - 2,
+                 md4_hash);
 
     /* pad to 21 bytes */
     memset(md4_hash + MD4_DIGEST_LENGTH, 0, 5);
 
-    ret_val = openvpn_base64_decode( phase_2, (void *)buf2, -1);
+    ret_val = openvpn_base64_decode(phase_2, buf2, -1);
     if (ret_val < 0)
     {
         return NULL;

@@ -281,15 +283,16 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
             msg(M_INFO, "Warning: Username or domain too long");
         }
         unicodize(userdomain_u, userdomain);
-        gen_hmac_md5(userdomain_u, 2 * strlen(userdomain), md4_hash, MD5_DIGEST_LENGTH, ntlmv2_hash);
+        gen_hmac_md5((uint8_t *)userdomain_u, 2 * strlen(userdomain), md4_hash,
+                     MD5_DIGEST_LENGTH, ntlmv2_hash);
 
         /* NTLMv2 Blob */
         memset(ntlmv2_blob, 0, 128);                        /* Clear blob buffer */
         ntlmv2_blob[0x00] = 1;                              /* Signature */
         ntlmv2_blob[0x01] = 1;                              /* Signature */
         ntlmv2_blob[0x04] = 0;                              /* Reserved */
-        gen_timestamp((unsigned char *)&ntlmv2_blob[0x08]);                  /* 64-bit Timestamp */
-        gen_nonce((unsigned char *)&ntlmv2_blob[0x10]);                      /* 64-bit Client Nonce */
+        gen_timestamp(&ntlmv2_blob[0x08]);                  /* 64-bit Timestamp */
+        gen_nonce(&ntlmv2_blob[0x10]);                      /* 64-bit Client Nonce */
         ntlmv2_blob[0x18] = 0;                              /* Unknown, zero should work */
 
         /* Add target information block to the blob */

@@ -301,8 +304,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
                 tib_len = 96;
             }
             {
-                char *tib_ptr;
-                int tib_pos = buf2[0x2c];
+                uint8_t *tib_ptr;
+                uint8_t tib_pos = buf2[0x2c];
                 if (tib_pos + tib_len > sizeof(buf2))
                 {
                     return NULL;

@@ -335,13 +338,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
     {
         unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH], key3[DES_KEY_LENGTH];
 
-        create_des_keys((unsigned char *)md4_hash, key1);
+        create_des_keys(md4_hash, key1);
         cipher_des_encrypt_ecb(key1, challenge, ntlm_response);
 
-        create_des_keys((unsigned char *)&(md4_hash[DES_KEY_LENGTH-1]), key2);
+        create_des_keys(&md4_hash[DES_KEY_LENGTH - 1], key2);
         cipher_des_encrypt_ecb(key2, challenge, &ntlm_response[DES_KEY_LENGTH]);
 
-        create_des_keys((unsigned char *)&(md4_hash[2*(DES_KEY_LENGTH-1)]), key3);
+        create_des_keys(&md4_hash[2 * (DES_KEY_LENGTH - 1)], key3);
         cipher_des_encrypt_ecb(key3, challenge, &ntlm_response[DES_KEY_LENGTH*2]);
     }

Signature · 1 line

2.13.2

Steffan Karger

2017-07-22 15:33 UTC #2 lore raw

Re: [Openvpn-devel] [PATCH 1/4] ntlm: convert binary buffers to uint8_t *

Hi,

Quoted 9 lines · Antonio Quartulli

several binary buffers in the ntml component are stored as char *, however this generates a lot of warnings, because hashing functions expect something unsigned.
Convert binary buffers to uint8_t *, while use explicit cast for buffers that are really carrying a string inside.
This commit removes several warnings from nmtl.c that you can catch with "-Wall -std=c99".

This is great - it largely matches a 'fix all the types' commit I've been keeping in my working branches to silence the warnings. I don't have a ntlm test setup, but I think these changes can be covered by careful review.

There's a typo in nmtl.c in the commit msg.

Quoted 10 lines

Signed-off-by Antonio Quartulli <[email protected]...>
---
 src/openvpn/ntlm.c | 51 +++++++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 24 deletions(-)
src/openvpn/ntlm.c +0 -0
index 0b1163ee..76dda576 100644

src/openvpn/ntlm.c create_des_keys(const unsigned char *hash, unsigned char *key) · line 74

Quoted 5 lines

src/openvpn/ntlm.c +1 -1

@@ -71,31 +71,32 @@ create_des_keys(const unsigned char *hash, unsigned char *key)
 }
 
 static void
-gen_md4_hash(const char *data, int data_len, char *result)
+gen_md4_hash(const uint8_t *data, int data_len, uint8_t *result)

data can simply be a void * here, we don't care about the underlying data type. That also saves us the cast when we pass it a char * in ntlm_phase_3().

(We should probably fix our crypto API to take void pointers too, but that's for some other patch.)

Quoted 5 lines

src/openvpn/ntlm.c +1 -1

 {
     /* result is 16 byte md4 hash */
     const md_kt_t *md4_kt = md_kt_get("MD4");
-    char md[MD4_DIGEST_LENGTH];
+    uint8_t md[MD4_DIGEST_LENGTH];

src/openvpn/ntlm.c create_des_keys(const unsigned char *hash, unsigned char *key) · line 87

Quoted 10 lines

src/openvpn/ntlm.c +2 -1

 
     md_full(md4_kt, data, data_len, md);
     memcpy(result, md, MD4_DIGEST_LENGTH);
 }
 
 static void
-gen_hmac_md5(const char *data, int data_len, const char *key, int key_len,char *result)
+gen_hmac_md5(const uint8_t *data, int data_len, const uint8_t *key, int key_len,
+             uint8_t *result)
 {

Here too, const void *data.

Quoted 69 lines

src/openvpn/ntlm.c +16 ~~-14~~

     const md_kt_t *md5_kt = md_kt_get("MD5");
     hmac_ctx_t *hmac_ctx = hmac_ctx_new();
 
     hmac_ctx_init(hmac_ctx, key, key_len, md5_kt);
-    hmac_ctx_update(hmac_ctx, (const unsigned char *)data, data_len);
-    hmac_ctx_final(hmac_ctx, (unsigned char *)result);
+    hmac_ctx_update(hmac_ctx, data, data_len);
+    hmac_ctx_final(hmac_ctx, result);
     hmac_ctx_cleanup(hmac_ctx);
     hmac_ctx_free(hmac_ctx);
 }
 
 static void
-gen_timestamp(unsigned char *timestamp)
+gen_timestamp(uint8_t *timestamp)
 {
     /* Copies 8 bytes long timestamp into "timestamp" buffer.
      * Timestamp is Little-endian, 64-bit signed value representing the number of tenths of a microsecond since January 1, 1601.

@@ -195,19 +196,19 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
      */
 
     char pwbuf[sizeof(p->up.password) * 2]; /* for unicode password */
-    unsigned char buf2[128]; /* decoded reply from proxy */
-    unsigned char phase3[464];
+    uint8_t buf2[128]; /* decoded reply from proxy */
+    uint8_t phase3[464];
 
-    char md4_hash[MD4_DIGEST_LENGTH+5];
-    char challenge[8], ntlm_response[24];
+    uint8_t md4_hash[MD4_DIGEST_LENGTH + 5];
+    uint8_t challenge[8], ntlm_response[24];
     int i, ret_val;
 
-    char ntlmv2_response[144];
+    uint8_t ntlmv2_response[144];
     char userdomain_u[256];     /* for uppercase unicode username and domain */
     char userdomain[128];       /* the same as previous but ascii */
-    char ntlmv2_hash[MD5_DIGEST_LENGTH];
-    char ntlmv2_hmacmd5[16];
-    char *ntlmv2_blob = ntlmv2_response + 16;     /* inside ntlmv2_response, length: 128 */
+    uint8_t ntlmv2_hash[MD5_DIGEST_LENGTH];
+    uint8_t ntlmv2_hmacmd5[16];
+    uint8_t *ntlmv2_blob = ntlmv2_response + 16;     /* inside ntlmv2_response, length: 128 */
     int ntlmv2_blob_size = 0;
     int phase3_bufpos = 0x40;     /* offset to next security buffer data to be added */
     size_t len;

@@ -246,12 +247,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
 
 
     /* fill 1st 16 bytes with md4 hash, disregard terminating null */
-    gen_md4_hash(pwbuf, unicodize(pwbuf, p->up.password) - 2, md4_hash);
+    gen_md4_hash((uint8_t *)pwbuf, unicodize(pwbuf, p->up.password) - 2,
+                 md4_hash);
 
     /* pad to 21 bytes */
     memset(md4_hash + MD4_DIGEST_LENGTH, 0, 5);
 
-    ret_val = openvpn_base64_decode( phase_2, (void *)buf2, -1);
+    ret_val = openvpn_base64_decode(phase_2, buf2, -1);
     if (ret_val < 0)
     {
         return NULL;

@@ -281,15 +283,16 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
             msg(M_INFO, "Warning: Username or domain too long");
         }
         unicodize(userdomain_u, userdomain);
-        gen_hmac_md5(userdomain_u, 2 * strlen(userdomain), md4_hash, MD5_DIGEST_LENGTH, ntlmv2_hash);
+        gen_hmac_md5((uint8_t *)userdomain_u, 2 * strlen(userdomain), md4_hash,
+                     MD5_DIGEST_LENGTH, ntlmv2_hash);

src/openvpn/ntlm.c ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are · line 295

Quoted 10 lines

src/openvpn/ntlm.c +2 -2

 
         /* NTLMv2 Blob */
         memset(ntlmv2_blob, 0, 128);                        /* Clear blob buffer */
         ntlmv2_blob[0x00] = 1;                              /* Signature */
         ntlmv2_blob[0x01] = 1;                              /* Signature */
         ntlmv2_blob[0x04] = 0;                              /* Reserved */
-        gen_timestamp((unsigned char *)&ntlmv2_blob[0x08]);                  /* 64-bit Timestamp */
-        gen_nonce((unsigned char *)&ntlmv2_blob[0x10]);                      /* 64-bit Client Nonce */
+        gen_timestamp(&ntlmv2_blob[0x08]);                  /* 64-bit Timestamp */
+        gen_nonce(&ntlmv2_blob[0x10]);                      /* 64-bit Client Nonce */

For consistenty with gen_timestamp(), it would be nice to make gen_nonce() also take a uint8_t *.

Quoted 22 lines

src/openvpn/ntlm.c +3 -3

         ntlmv2_blob[0x18] = 0;                              /* Unknown, zero should work */
 
         /* Add target information block to the blob */

@@ -301,8 +304,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
                 tib_len = 96;
             }
             {
-                char *tib_ptr;
-                int tib_pos = buf2[0x2c];
+                uint8_t *tib_ptr;
+                uint8_t tib_pos = buf2[0x2c];
                 if (tib_pos + tib_len > sizeof(buf2))
                 {
                     return NULL;

@@ -335,13 +338,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
     {
         unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH], key3[DES_KEY_LENGTH];
 
-        create_des_keys((unsigned char *)md4_hash, key1);
+        create_des_keys(md4_hash, key1);
         cipher_des_encrypt_ecb(key1, challenge, ntlm_response);

src/openvpn/ntlm.c ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are · line 350

Quoted 10 lines

src/openvpn/ntlm.c +2 -2

 
-        create_des_keys((unsigned char *)&(md4_hash[DES_KEY_LENGTH-1]), key2);
+        create_des_keys(&md4_hash[DES_KEY_LENGTH - 1], key2);
         cipher_des_encrypt_ecb(key2, challenge, &ntlm_response[DES_KEY_LENGTH]);
 
-        create_des_keys((unsigned char *)&(md4_hash[2*(DES_KEY_LENGTH-1)]), key3);
+        create_des_keys(&md4_hash[2 * (DES_KEY_LENGTH - 1)], key3);
         cipher_des_encrypt_ecb(key3, challenge, &ntlm_response[DES_KEY_LENGTH*2]);
     }

My comments on your changes are all very minor, and if you don't agree with me I'm also fine with the patch as is (after the commit msg typo fix). I'll leave it up to you to decide if my suggestions should be included.

So, ACK, with or without my suggested changes.

-Steffan

David Sommerseth

2017-08-11 20:11 UTC #3 lore raw

Re: [Openvpn-devel] [PATCH applied] ntlm: convert binary buffers to uint8_t *

In reply to Antonio Quartulli

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Your patch has been applied to the following branches

commit e7e4070cb7b90f4836b65c53360166e11fc3f383 (master) commit 3ace1139e7aa00580300fb5bef37ac6d47378630 (release/2.4)

Author: Antonio Quartulli
Date:   Mon Jul 10 12:34:38 2017 +0800

     ntlm: convert binary buffers to uint8_t *

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15032.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCgAGBQJZjg9SAAoJEIbPlEyWcf3y0HgP/i35Nvj4wDpV/akcBP0SWLvR L6AcwCM7LBEgqOeNfSO18ysoK1nzp9EpglPyyFDrWtszJQ0ow+Dtwp7VqlzPUM7W fmFe6c0YaogaRCOyv1l/RJ+0E5BoTi7wj3baC2znEkE3MEDGE6I+Yjnxpy2QirTj /arxtDSmtap2/Ygyvtt7K5iRPJZ0y2macUHCGk3b7lPXMVFjyA1akTU7tiVmgUrp JFfiLXOOtJu50jGcE8oF+D7hZ6/cq21S/nfQPUKbG/0t1li43/wGt9exp5mHnHFx LvprBmjoKaQHj9eR/iqrirSvd/Sl9okDeuaFsNfgcTi87wtbwz7wfCudd/11f25H H+uVLT9Whq+PnFpF2ZuDVi7Z1/DejLxEY/uxK2TolV1MfO9hLeSiCPoz2NxPq179 FolBa2/8IzrC+8UhDL652Cp0kYZmSyal1jfQGr5fx8uNPGivQeUo6Y+Zmh8H5jEy 2Ij0dSc9Y+m6F+3VpxMGwhx7IR3Ro9vwW6jjmYpB/HK5o+sMQJ3g8wgqKpbDAzN3 /OTN/9vC47foHTuSGDyyTGR9Hf6zToYamMFX3sIV08XLRSMe92NPljL5pI9S+SS5 xQDLaPIERZLR7CSi9uKzpL5+lcSLNK6sNp2pN29W1e+uwASoszZe6qqqZ1yiLQ6w pV/MW6TKGqH+Ta0UrBO1
=Esi3
-----END PGP SIGNATURE-----

Antonio Quartulli

author

2017-07-10 04:34 UTC #4 lore raw

PATCH 2/4 ntlm: avoid useless cast

In reply to Antonio Quartulli

The argument passed to my_strupr() is converted to an upper case string by means of toupper(). The latter expects a single signed int as argument, therefore it makes sense to have my_strupr() take a signed argument too and avoid an explicit and an implicit cast.

Signed-off-by Antonio Quartulli <[email protected]...>

---
 src/openvpn/ntlm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

src/openvpn/ntlm.c +2 -2

index 76dda576..bb929283 100644
@@ -131,7 +131,7 @@ gen_nonce(unsigned char *nonce)
 }
 
 void
-my_strupr(unsigned char *str)
+my_strupr(char *str)
 {
     /* converts string to uppercase in place */

@@ -273,7 +273,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         int tib_len;
 
         /* NTLMv2 hash */
-        my_strupr((unsigned char *)strcpy(userdomain, username));
+        my_strupr(strcpy(userdomain, username));
         if (strlen(username) + strlen(domain) < sizeof(userdomain))
         {
             strcat(userdomain, domain);

Signature · 1 line

2.13.2

Steffan Karger

2017-07-11 09:08 UTC #5 lore raw

Re: [Openvpn-devel] [PATCH 2/4] ntlm: avoid useless cast

Quoted 25 lines · Antonio Quartulli

The argument passed to my_strupr() is converted to an upper case string by means of toupper(). The latter expects a single signed int as argument, therefore it makes sense to have my_strupr() take a signed argument too and avoid an explicit and an implicit cast.
Signed-off-by Antonio Quartulli <[email protected]...>
---
 src/openvpn/ntlm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/openvpn/ntlm.c +1 -1
index 76dda576..bb929283 100644
@@ -131,7 +131,7 @@ gen_nonce(unsigned char *nonce)
 }
 
 void
-my_strupr(unsigned char *str)
+my_strupr(char *str)
 {
     /* converts string to uppercase in place */
 

src/openvpn/ntlm.c ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are · line 279

Quoted 8 lines

src/openvpn/ntlm.c +1 -1

@@ -273,7 +273,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         int tib_len;
 
         /* NTLMv2 hash */
-        my_strupr((unsigned char *)strcpy(userdomain, username));
+        my_strupr(strcpy(userdomain, username));
         if (strlen(username) + strlen(domain) < sizeof(userdomain))
         {
             strcat(userdomain, domain);

Evidently correct. ACK.

-Steffan

Gert Doering

2017-07-11 10:41 UTC #6 lore raw

Re: [Openvpn-devel] [PATCH 2/4] ntlm: avoid useless cast

In reply to Antonio Quartulli

Hi,

src/openvpn/ntlm.c ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are · line 276

Quoted 3 lines · Antonio Quartulli

         /* NTLMv2 hash */

src/openvpn/ntlm.c +1 -1

-        my_strupr((unsigned char *)strcpy(userdomain, username));
+        my_strupr(strcpy(userdomain, username));

While at it, could you change that eyesore to

           strcpy(userdomain, username);
           my_strupr(userdomain);

as well...?

I find this sort of chaining of function calls tremendously hard to read.

(... in a ntlm.c cleanup 5/4 patch :))

gert

Signature · 4 lines

USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...

Gert Doering

2017-07-17 12:29 UTC #7 lore raw

[Openvpn-devel] [PATCH applied] Re: ntlm: avoid useless cast

In reply to Antonio Quartulli

Your patch has been applied to the master and release/2.4 branch.

commit 1cdfc9302aad8570360d278aded5fb9f110ca2b6 (master) commit 527bf0caad0b2cc353a27682b49d9ca8e6c0e435 (release/2.4)

Author: Antonio Quartulli
Date:   Mon Jul 10 12:34:39 2017 +0800

     ntlm: avoid useless cast

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15031.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

Antonio Quartulli

author

2017-07-10 04:34 UTC #8 lore raw

PATCH 3/4 ntlm: restyle compressed multiple function calls

In reply to Antonio Quartulli

The gen_md4_hash() function is receiving as first argument a buffer that is filled by a function invoked when evaluating the second argument.

Although this is proper C, it makes the call invocation a bit obscure because it is not immediately easy to grasp how the 'pwbuf' buffer is filled.

Unroll the multiple function call onto lines and make the core more readable.

Signed-off-by Antonio Quartulli <[email protected]...>

---
 src/openvpn/ntlm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

src/openvpn/ntlm.c +2 -2

index bb929283..f18e0371 100644
@@ -247,8 +247,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
 
 
     /* fill 1st 16 bytes with md4 hash, disregard terminating null */
-    gen_md4_hash((uint8_t *)pwbuf, unicodize(pwbuf, p->up.password) - 2,
-                 md4_hash);
+    int unicode_len = unicodize(pwbuf, p->up.password) - 2;
+    gen_md4_hash((uint8_t *)pwbuf, unicode_len, md4_hash);
 
     /* pad to 21 bytes */
     memset(md4_hash + MD4_DIGEST_LENGTH, 0, 5);

Signature · 1 line

2.13.2

Steffan Karger

2017-07-22 15:43 UTC #9 lore raw

Re: [Openvpn-devel] [PATCH 3/4] ntlm: restyle compressed multiple function calls

Hi,

Quoted 22 lines · Antonio Quartulli

The gen_md4_hash() function is receiving as first argument a buffer that is filled by a function invoked when evaluating the second argument.
Although this is proper C, it makes the call invocation a bit obscure because it is not immediately easy to grasp how the 'pwbuf' buffer is filled.
Unroll the multiple function call onto lines and make the core more readable.
Signed-off-by Antonio Quartulli <[email protected]...>
---
 src/openvpn/ntlm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/openvpn/ntlm.c +0 -0
index bb929283..f18e0371 100644

src/openvpn/ntlm.c ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are · line 254

Quoted 10 lines

src/openvpn/ntlm.c +2 -2

@@ -247,8 +247,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
 
 
     /* fill 1st 16 bytes with md4 hash, disregard terminating null */
-    gen_md4_hash((uint8_t *)pwbuf, unicodize(pwbuf, p->up.password) - 2,
-                 md4_hash);
+    int unicode_len = unicodize(pwbuf, p->up.password) - 2;
+    gen_md4_hash((uint8_t *)pwbuf, unicode_len, md4_hash);
 
     /* pad to 21 bytes */
     memset(md4_hash + MD4_DIGEST_LENGTH, 0, 5);

ACK

(Though this patch no longer applies cleanly if you change the gen_md4_hash() data type to void*, but that is trivially fixed.)

-Steffan

David Sommerseth

2017-08-11 20:11 UTC #10 lore raw

Re: [Openvpn-devel] [PATCH applied] ntlm: restyle compressed multiple function calls

In reply to Antonio Quartulli

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Your patch has been applied to the following branches

commit c2d08916f1b7933bec81422d1f14f84e9b1ef878 (master) commit 90f2edcbb7c1e890de96a9a44c87ca58dbd60b96 (release/2.4)

Author: Antonio Quartulli
Date:   Mon Jul 10 12:34:40 2017 +0800

     ntlm: restyle compressed multiple function calls

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15030.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCgAGBQJZjg9iAAoJEIbPlEyWcf3ywC8QAMAVm4psZ96qwzryKr6Zteab nwRB11RGH1WK64/X5gem7Epj3Ldmv9BE3++45FQBc78jbZBBAQwW9hPVAdEfcWd2 iVa/KjJvcqZz7obMvt5uu3J0O1UOoHzXHdkXERgToJ375Zr1uUVTZd+e7Cpa+GdR rugF3g+rp3xy+JDyPf71uBHOlg2+35YayMpQ+DVmQzJOxYZzAn2CdloQsw5dN3Ja QM1kIpGa0X2FY/zJJdUOpffXj9ypbDg+06RSVemzIxKOMeZ0PsUiDa7zUSmkd/Fv GdmgwNCKoi3WT/+imHPvNoRVkBIA6cgwlpw3KhMu2Y2lOMYnOMI4yN2pyylIgvG1 KUnKJCwKjCjtd5ZFKSM32yN+XXW27gpHrCcZPn6kIWfFCVqHlv8WCEKkObydrrYU 5Ot/plM7WDlvvTTgVonTY/vUwFS+fc7cT1ixzY3lKGhg+raoGH4PTQsNV2C5Izo6 LN2xiCfvNTZWQkWUVDjual7r00qQvMSwu/IrTywlASRg2qgQH6HiKpL6Wdg1hGdY Fx/kfyo+RId+pD0aJCDFA5nxE0I+eVhMYc/2kDXvFLlPMvf0v+yawKvQ65jgEpxm vuUXYGs0cbL29xS42ZR1zcHpQGMEAwXcDRTD5wnEvRexGZbdtHVk2v5OPGzEpnu7 rAVCUqy4W5m97mJ5L5Uf
=Nna+
-----END PGP SIGNATURE-----

Antonio Quartulli

author

2017-07-10 04:34 UTC #11 lore raw

PATCH 4/4 ntlm: improve code style and readability

In reply to Antonio Quartulli

This patch does not introduce any functional or behavioural change.

The code in ntlm.c has been restyled to better to obey to the new coding style and its readability has been a improved a bit.

Signed-off-by Antonio Quartulli <[email protected]...>

---
 src/openvpn/ntlm.c | 74 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 29 deletions(-)

src/openvpn/ntlm.c +45 ~~-29~~

index f18e0371..c0319287 100644
@@ -60,13 +60,13 @@ static void
 create_des_keys(const unsigned char *hash, unsigned char *key)
 {
     key[0] = hash[0];
-    key[1] = ((hash[0]&1)<<7)|(hash[1]>>1);
-    key[2] = ((hash[1]&3)<<6)|(hash[2]>>2);
-    key[3] = ((hash[2]&7)<<5)|(hash[3]>>3);
-    key[4] = ((hash[3]&15)<<4)|(hash[4]>>4);
-    key[5] = ((hash[4]&31)<<3)|(hash[5]>>5);
-    key[6] = ((hash[5]&63)<<2)|(hash[6]>>6);
-    key[7] = ((hash[6]&127)<<1);
+    key[1] = ((hash[0] & 1) << 7) | (hash[1] >> 1);
+    key[2] = ((hash[1] & 3) << 6) | (hash[2] >> 2);
+    key[3] = ((hash[2] & 7) << 5) | (hash[3] >> 3);
+    key[4] = ((hash[3] & 15) << 4) | (hash[4] >> 4);
+    key[5] = ((hash[4] & 31) << 3) | (hash[5] >> 5);
+    key[6] = ((hash[5] & 63) << 2) | (hash[6] >> 6);
+    key[7] = ((hash[6] & 127) << 1);
     key_des_fixup(key, 8, 1);
 }

@@ -99,7 +99,8 @@ static void
 gen_timestamp(uint8_t *timestamp)
 {
     /* Copies 8 bytes long timestamp into "timestamp" buffer.
-     * Timestamp is Little-endian, 64-bit signed value representing the number of tenths of a microsecond since January 1, 1601.
+     * Timestamp is Little-endian, 64-bit signed value representing the
+     * number of tenths of a microsecond since January 1, 1601.
      */
 
     UINTEGER64 timestamp_ull;

@@ -151,16 +152,17 @@ unicodize(char *dst, const char *src)
     {
         dst[i++] = *src;
         dst[i++] = 0;
-    }
-    while (*src++);
+    } while (*src++);
 
     return i;
 }
 
 static void
-add_security_buffer(int sb_offset, void *data, int length, unsigned char *msg_buf, int *msg_bufpos)
+add_security_buffer(int sb_offset, void *data, int length,
+                    unsigned char *msg_buf, int *msg_bufpos)
 {
-    /* Adds security buffer data to a message and sets security buffer's offset and length */
+    /* Adds security buffer data to a message and sets security buffer's
+     * offset and length */
     msg_buf[sb_offset] = (unsigned char)length;
     msg_buf[sb_offset + 2] = msg_buf[sb_offset];
     msg_buf[sb_offset + 4] = (unsigned char)(*msg_bufpos & 0xff);

@@ -187,7 +189,8 @@ ntlm_phase_1(const struct http_proxy_info *p, struct gc_arena *gc)
 }
 
 const char *
-ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_arena *gc)
+ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
+             struct gc_arena *gc)
 {
     /* NTLM handshake
      *

@@ -296,13 +299,16 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         ntlmv2_blob[0x18] = 0;                              /* Unknown, zero should work */
 
         /* Add target information block to the blob */
-        if (( *((long *)&buf2[0x14]) & 0x00800000) == 0x00800000)          /* Check for Target Information block */
+
+        /* Check for Target Information block */
+        if ((*((long *)&buf2[0x14]) & 0x00800000) == 0x00800000)
         {
             tib_len = buf2[0x28];            /* Get Target Information block size */
             if (tib_len > 96)
             {
                 tib_len = 96;
             }
+
             {
                 uint8_t *tib_ptr;
                 uint8_t tib_pos = buf2[0x2c];

@@ -310,8 +316,10 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
                 {
                     return NULL;
                 }
-                tib_ptr = buf2 + tib_pos;                               /* Get Target Information block pointer */
-                memcpy(&ntlmv2_blob[0x1c], tib_ptr, tib_len);           /* Copy Target Information block into the blob */
+                /* Get Target Information block pointer */
+                tib_ptr = buf2 + tib_pos;
+                /* Copy Target Information block into the blob */
+                memcpy(&ntlmv2_blob[0x1c], tib_ptr, tib_len);
             }
         }
         else

@@ -319,7 +327,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
             tib_len = 0;
         }
 
-        ntlmv2_blob[0x1c + tib_len] = 0;                    /* Unknown, zero works */
+        /* Unknown, zero works */
+        ntlmv2_blob[0x1c + tib_len] = 0;
 
         /* Get blob length */
         ntlmv2_blob_size = 0x20 + tib_len;

@@ -328,15 +337,18 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         memcpy(&ntlmv2_response[8], challenge, 8);
 
         /* hmac-md5 */
-        gen_hmac_md5(&ntlmv2_response[8], ntlmv2_blob_size + 8, ntlmv2_hash, MD5_DIGEST_LENGTH, ntlmv2_hmacmd5);
-
-        /* Add hmac-md5 result to the blob */
-        memcpy(ntlmv2_response, ntlmv2_hmacmd5, MD5_DIGEST_LENGTH);         /* Note: This overwrites challenge previously written at ntlmv2_response[8..15] */
+        gen_hmac_md5(&ntlmv2_response[8], ntlmv2_blob_size + 8, ntlmv2_hash,
+                     MD5_DIGEST_LENGTH, ntlmv2_hmacmd5);
 
+        /* Add hmac-md5 result to the blob.
+         * Note: This overwrites challenge previously written at
+         * ntlmv2_response[8..15] */
+        memcpy(ntlmv2_response, ntlmv2_hmacmd5, MD5_DIGEST_LENGTH);
     }
-    else         /* Generate NTLM response */
+    else /* Generate NTLM response */
     {
-        unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH], key3[DES_KEY_LENGTH];
+        unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH];
+        unsigned char key3[DES_KEY_LENGTH];
 
         create_des_keys(md4_hash, key1);
         cipher_des_encrypt_ecb(key1, challenge, ntlm_response);

@@ -345,7 +357,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         cipher_des_encrypt_ecb(key2, challenge, &ntlm_response[DES_KEY_LENGTH]);
 
         create_des_keys(&md4_hash[2 * (DES_KEY_LENGTH - 1)], key3);
-        cipher_des_encrypt_ecb(key3, challenge, &ntlm_response[DES_KEY_LENGTH*2]);
+        cipher_des_encrypt_ecb(key3, challenge,
+                               &ntlm_response[DES_KEY_LENGTH * 2]);
     }

@@ -356,7 +369,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
 
     if (ntlmv2_enabled)      /* NTLMv2 response */
     {
-        add_security_buffer(0x14, ntlmv2_response, ntlmv2_blob_size + 16, phase3, &phase3_bufpos);
+        add_security_buffer(0x14, ntlmv2_response, ntlmv2_blob_size + 16,
+                            phase3, &phase3_bufpos);
     }
     else       /* NTLM response */
     {

@@ -364,12 +378,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
     }
 
     /* username in ascii */
-    add_security_buffer(0x24, username, strlen(username), phase3, &phase3_bufpos);
+    add_security_buffer(0x24, username, strlen(username), phase3,
+                        &phase3_bufpos);
 
-    /* Set domain. If <domain> is empty, default domain will be used (i.e. proxy's domain) */
+    /* Set domain. If <domain> is empty, default domain will be used
+     * (i.e. proxy's domain) */
     add_security_buffer(0x1c, domain, strlen(domain), phase3, &phase3_bufpos);
 
-
     /* other security buffers will be empty */
     phase3[0x10] = phase3_bufpos;     /* lm not used */
     phase3[0x30] = phase3_bufpos;     /* no workstation name supplied */

@@ -379,7 +394,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
     phase3[0x3c] = 0x02; /* negotiate oem */
     phase3[0x3d] = 0x02; /* negotiate ntlm */
 
-    return ((const char *)make_base64_string2((unsigned char *)phase3, phase3_bufpos, gc));
+    return ((const char *)make_base64_string2((unsigned char *)phase3,
+                                              phase3_bufpos, gc));
 }
 
 #else  /* if NTLM */

Signature · 1 line

2.13.2

Steffan Karger

2017-07-22 15:48 UTC #12 lore raw

Re: [Openvpn-devel] [PATCH 4/4] ntlm: improve code style and readability

Hi,

Quoted 182 lines · Antonio Quartulli

This patch does not introduce any functional or behavioural change.

The code in ntlm.c has been restyled to better to obey to the new coding style and its readability has been a improved a bit.

Signed-off-by Antonio Quartulli <[email protected]...>

---
 src/openvpn/ntlm.c | 74 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 45 insertions(+), 29 deletions(-)

src/openvpn/ntlm.c +43 ~~-28~~

index f18e0371..c0319287 100644
@@ -60,13 +60,13 @@ static void
 create_des_keys(const unsigned char *hash, unsigned char *key)
 {
     key[0] = hash[0];
-    key[1] = ((hash[0]&1)<<7)|(hash[1]>>1);
-    key[2] = ((hash[1]&3)<<6)|(hash[2]>>2);
-    key[3] = ((hash[2]&7)<<5)|(hash[3]>>3);
-    key[4] = ((hash[3]&15)<<4)|(hash[4]>>4);
-    key[5] = ((hash[4]&31)<<3)|(hash[5]>>5);
-    key[6] = ((hash[5]&63)<<2)|(hash[6]>>6);
-    key[7] = ((hash[6]&127)<<1);
+    key[1] = ((hash[0] & 1) << 7) | (hash[1] >> 1);
+    key[2] = ((hash[1] & 3) << 6) | (hash[2] >> 2);
+    key[3] = ((hash[2] & 7) << 5) | (hash[3] >> 3);
+    key[4] = ((hash[3] & 15) << 4) | (hash[4] >> 4);
+    key[5] = ((hash[4] & 31) << 3) | (hash[5] >> 5);
+    key[6] = ((hash[5] & 63) << 2) | (hash[6] >> 6);
+    key[7] = ((hash[6] & 127) << 1);
     key_des_fixup(key, 8, 1);
 }

@@ -99,7 +99,8 @@ static void
 gen_timestamp(uint8_t *timestamp)
 {
     /* Copies 8 bytes long timestamp into "timestamp" buffer.
-     * Timestamp is Little-endian, 64-bit signed value representing the number of tenths of a microsecond since January 1, 1601.
+     * Timestamp is Little-endian, 64-bit signed value representing the
+     * number of tenths of a microsecond since January 1, 1601.
      */
 
     UINTEGER64 timestamp_ull;

@@ -151,16 +152,17 @@ unicodize(char *dst, const char *src)
     {
         dst[i++] = *src;
         dst[i++] = 0;
-    }
-    while (*src++);
+    } while (*src++);
 
     return i;
 }
 
 static void
-add_security_buffer(int sb_offset, void *data, int length, unsigned char *msg_buf, int *msg_bufpos)
+add_security_buffer(int sb_offset, void *data, int length,
+                    unsigned char *msg_buf, int *msg_bufpos)
 {
-    /* Adds security buffer data to a message and sets security buffer's offset and length */
+    /* Adds security buffer data to a message and sets security buffer's
+     * offset and length */
     msg_buf[sb_offset] = (unsigned char)length;
     msg_buf[sb_offset + 2] = msg_buf[sb_offset];
     msg_buf[sb_offset + 4] = (unsigned char)(*msg_bufpos & 0xff);

@@ -187,7 +189,8 @@ ntlm_phase_1(const struct http_proxy_info *p, struct gc_arena *gc)
 }
 
 const char *
-ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_arena *gc)
+ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
+             struct gc_arena *gc)
 {
     /* NTLM handshake
      *

@@ -296,13 +299,16 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         ntlmv2_blob[0x18] = 0;                              /* Unknown, zero should work */
 
         /* Add target information block to the blob */
-        if (( *((long *)&buf2[0x14]) & 0x00800000) == 0x00800000)          /* Check for Target Information block */
+
+        /* Check for Target Information block */
+        if ((*((long *)&buf2[0x14]) & 0x00800000) == 0x00800000)
         {
             tib_len = buf2[0x28];            /* Get Target Information block size */
             if (tib_len > 96)
             {
                 tib_len = 96;
             }
+
             {
                 uint8_t *tib_ptr;
                 uint8_t tib_pos = buf2[0x2c];

@@ -310,8 +316,10 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
                 {
                     return NULL;
                 }
-                tib_ptr = buf2 + tib_pos;                               /* Get Target Information block pointer */
-                memcpy(&ntlmv2_blob[0x1c], tib_ptr, tib_len);           /* Copy Target Information block into the blob */
+                /* Get Target Information block pointer */
+                tib_ptr = buf2 + tib_pos;
+                /* Copy Target Information block into the blob */
+                memcpy(&ntlmv2_blob[0x1c], tib_ptr, tib_len);
             }
         }
         else

@@ -319,7 +327,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
             tib_len = 0;
         }
 
-        ntlmv2_blob[0x1c + tib_len] = 0;                    /* Unknown, zero works */
+        /* Unknown, zero works */
+        ntlmv2_blob[0x1c + tib_len] = 0;
 
         /* Get blob length */
         ntlmv2_blob_size = 0x20 + tib_len;

@@ -328,15 +337,18 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         memcpy(&ntlmv2_response[8], challenge, 8);
 
         /* hmac-md5 */
-        gen_hmac_md5(&ntlmv2_response[8], ntlmv2_blob_size + 8, ntlmv2_hash, MD5_DIGEST_LENGTH, ntlmv2_hmacmd5);
-
-        /* Add hmac-md5 result to the blob */
-        memcpy(ntlmv2_response, ntlmv2_hmacmd5, MD5_DIGEST_LENGTH);         /* Note: This overwrites challenge previously written at ntlmv2_response[8..15] */
+        gen_hmac_md5(&ntlmv2_response[8], ntlmv2_blob_size + 8, ntlmv2_hash,
+                     MD5_DIGEST_LENGTH, ntlmv2_hmacmd5);
 
+        /* Add hmac-md5 result to the blob.
+         * Note: This overwrites challenge previously written at
+         * ntlmv2_response[8..15] */
+        memcpy(ntlmv2_response, ntlmv2_hmacmd5, MD5_DIGEST_LENGTH);
     }
-    else         /* Generate NTLM response */
+    else /* Generate NTLM response */
     {
-        unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH], key3[DES_KEY_LENGTH];
+        unsigned char key1[DES_KEY_LENGTH], key2[DES_KEY_LENGTH];
+        unsigned char key3[DES_KEY_LENGTH];
 
         create_des_keys(md4_hash, key1);
         cipher_des_encrypt_ecb(key1, challenge, ntlm_response);

@@ -345,7 +357,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
         cipher_des_encrypt_ecb(key2, challenge, &ntlm_response[DES_KEY_LENGTH]);
 
         create_des_keys(&md4_hash[2 * (DES_KEY_LENGTH - 1)], key3);
-        cipher_des_encrypt_ecb(key3, challenge, &ntlm_response[DES_KEY_LENGTH*2]);
+        cipher_des_encrypt_ecb(key3, challenge,
+                               &ntlm_response[DES_KEY_LENGTH * 2]);
     }

@@ -356,7 +369,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
 
     if (ntlmv2_enabled)      /* NTLMv2 response */
     {
-        add_security_buffer(0x14, ntlmv2_response, ntlmv2_blob_size + 16, phase3, &phase3_bufpos);
+        add_security_buffer(0x14, ntlmv2_response, ntlmv2_blob_size + 16,
+                            phase3, &phase3_bufpos);
     }
     else       /* NTLM response */
     {

@@ -364,12 +378,13 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
     }
 
     /* username in ascii */
-    add_security_buffer(0x24, username, strlen(username), phase3, &phase3_bufpos);
+    add_security_buffer(0x24, username, strlen(username), phase3,
+                        &phase3_bufpos);
 
-    /* Set domain. If <domain> is empty, default domain will be used (i.e. proxy's domain) */
+    /* Set domain. If <domain> is empty, default domain will be used
+     * (i.e. proxy's domain) */
     add_security_buffer(0x1c, domain, strlen(domain), phase3, &phase3_bufpos);
 
-
     /* other security buffers will be empty */
     phase3[0x10] = phase3_bufpos;     /* lm not used */
     phase3[0x30] = phase3_bufpos;     /* no workstation name supplied */

src/openvpn/ntlm.c ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are · line 401

Quoted 9 lines

src/openvpn/ntlm.c +2 -1

@@ -379,7 +394,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
     phase3[0x3c] = 0x02; /* negotiate oem */
     phase3[0x3d] = 0x02; /* negotiate ntlm */
 
-    return ((const char *)make_base64_string2((unsigned char *)phase3, phase3_bufpos, gc));
+    return ((const char *)make_base64_string2((unsigned char *)phase3,
+                                              phase3_bufpos, gc));
 }
 
 #else  /* if NTLM */

As it says on the tin: improves readability, no functional changes. ACK.

-Steffan

David Sommerseth

2017-08-11 20:11 UTC #13 lore raw

Re: [Openvpn-devel] [PATCH applied] ntlm: improve code style and readability

In reply to Antonio Quartulli

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Your patch has been applied to the following branches

commit c310f1ecba905f091e3a31cb3e6cba5ae75e996b (master) commit 8eb2f571e148e178e62a8fce20a06d4692203aeb (release/2.4)

Author: Antonio Quartulli
Date:   Mon Jul 10 12:34:41 2017 +0800

     ntlm: improve code style and readability

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15028.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCgAGBQJZjg9uAAoJEIbPlEyWcf3y0l0P/RIDHjeC+dH7OgAVyrbb5Rfm HKGqqEaJcg1qAfQxOpH4+CAPF/FHBRToJ9mrTYK/BpelE8LxxCpf4tEjwgfrJWYX /1ZFZqBo29r/G6yvcVdvvtUkvYzfMytj2eXuKcX0nYTTTjsJBOjvHoyr4itqDrkR s7FAUXJ5TCKpxS1++nscq2AqasB4K7lo9r1oDBIATthj5sVf7JskmOKvvYITKPa/ kAVgV+fNzCtCVr16/7vI5oqiMify9bWc379+hebezwfAv2M2OAaKQXvknA13KTgW UCABEh33PY1JNbMKFHxlALVodSLdBI3cMLzmAZ9dVF4xAZmPVxHIV7TdUvs2ag5n ySqWmTWVjqV/CbtdhSW48EJ9XYVQ2KLmubtMWURSB9BUIOEvmBlcraJb8BJdgra5 l8++gEDocMgoFDK9zM77DqZOCIqBhZhJ5k9Gh5Qtp6fmN979ImEllaIJZvYot833 yl3pyEKsDq4jko3kWvO9993Ruubnu7VSfCabUjZ5jmU6/1CB/Tos6g5mXc/aMpXc D4uT9IURuG6Wx62nK0K6yuLhstaeRPUYeCM5HCcHSroID85LUYwKAB7oL3Z2Adnh R/1F4oY4Fxzo/RBjZGPx0hTwRYktEeYG3d7sYLbfFhmCh+b3uTNQ22D1+SWSzjRf XGc21zn2a0/M+PnJxRzo
=KFUG
-----END PGP SIGNATURE-----

Antonio Quartulli

author

2017-07-12 03:58 UTC #14 lore raw

[Openvpn-devel] [PATCH 5/4] ntlm: unwrap multiple function calls

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]...>

In order to improve code readability it is better to unwrap multiple function calls onto multiple lines.

Signed-off-by Antonio Quartulli <[email protected]...>

---
 src/openvpn/ntlm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

src/openvpn/ntlm.c +2 -1

index c0319287..47ad8959 100644
@@ -276,7 +276,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
         int tib_len;
 
         /* NTLMv2 hash */
-        my_strupr(strcpy(userdomain, username));
+        strcpy(userdomain, username)
+        my_strupr(userdomain);
         if (strlen(username) + strlen(domain) < sizeof(userdomain))
         {
             strcat(userdomain, domain);

Signature · 1 line

2.13.2

Antonio Quartulli

author

2017-07-12 04:30 UTC #15 lore raw

[Openvpn-devel] [PATCH 5/4 v2] ntlm: unwrap multiple function calls

In order to improve code readability it is better to unwrap multiple function calls onto multiple lines.

Signed-off-by Antonio Quartulli <[email protected]...>

---

- add missing ;
- use my personal mail address

 src/openvpn/ntlm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

src/openvpn/ntlm.c +2 -1

index c0319287..65c1cbf5 100644
@@ -276,7 +276,8 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
         int tib_len;
 
         /* NTLMv2 hash */
-        my_strupr(strcpy(userdomain, username));
+        strcpy(userdomain, username);
+        my_strupr(userdomain);
         if (strlen(username) + strlen(domain) < sizeof(userdomain))
         {
             strcat(userdomain, domain);

Signature · 1 line

2.13.2

Gert Doering

2017-07-17 12:29 UTC #16 lore raw

[Openvpn-devel] [PATCH applied] Re: ntlm: unwrap multiple function calls

ACK. Thanks.

Your patch has been applied to the master and release/2.4 branch.

commit ad7f7e56d34bbf477a7e5639f1b78b2c7e58186c (master) commit 0bc107b454c134a60e4a9d7fe7bcb5cb10743afe (release/2.4)

Author: Antonio Quartulli
Date:   Wed Jul 12 12:30:02 2017 +0800

     ntlm: unwrap multiple function calls

     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15041.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering