NTLM: increase size of phase 2 response we can handle

With NTLMv2 the target information buffer can be rather large
even with normal domain setups.

In my test setup it was 152 bytes starting at offset 71.
Overall the base64 encode phase 2 response was 300 byte long.
The linked documentation has 98 bytes at offset 60. 128 byte
is clearly too low.

While here improve the error messaging, so that if the buffer
is too small at least one can determine that in the log.

Change-Id: Iefa4930cb1e8c4135056a17ceb4283fc13cc75c8
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20240117094952.25938-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg28052.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c
index 2b735ecda35934e1495fbec9768bd23b2c5ea932..698abfbbd533f832142d860eb8c9eabda97573eb 100644 (file)
--- a/src/openvpn/ntlm.c
+++ b/src/openvpn/ntlm.c
@@ -218,7 +218,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
     uint8_t challenge[8], ntlm_response[24];
     int i, ret_val;
 
-    uint8_t ntlmv2_response[144];
+    uint8_t ntlmv2_response[256];
     char userdomain_u[256];     /* for uppercase unicode username and domain */
     char userdomain[128];       /* the same as previous but ascii */
     uint8_t ntlmv2_hash[MD5_DIGEST_LENGTH];
@@ -270,17 +270,15 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
      * the missing bytes will be NULL, as buf2 is known to be zeroed
      * when this decode happens.
      */
-    uint8_t buf2[128]; /* decoded reply from proxy */
+    uint8_t buf2[512]; /* decoded reply from proxy */
     CLEAR(buf2);
     ret_val = openvpn_base64_decode(phase_2, buf2, -1);
     if (ret_val < 0)
     {
+        msg(M_WARN, "NTLM: base64 decoding of phase 2 response failed");
         return NULL;
     }
 
-    /* we can be sure that phase_2 is less than 128
-     * therefore buf2 needs to be (3/4 * 128) */
-
     /* extract the challenge from bytes 24-31 */
     for (i = 0; i<8; i++)
     {
@@ -300,7 +298,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
         }
         else
         {
-            msg(M_INFO, "Warning: Username or domain too long");
+            msg(M_WARN, "NTLM: Username or domain too long");
         }
         unicodize(userdomain_u, userdomain);
         gen_hmac_md5((uint8_t *)userdomain_u, 2 * strlen(userdomain), md4_hash,
@@ -335,9 +333,10 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
         if ((flags & 0x00800000) == 0x00800000)
         {
             tib_len = buf2[0x28];            /* Get Target Information block size */
-            if (tib_len > 96)
+            if (tib_len + 0x1c + 16 > sizeof(ntlmv2_response))
             {
-                tib_len = 96;
+                msg(M_WARN, "NTLM: target information buffer too long for response (len=%d)", tib_len);
+                return NULL;
             }
 
             {
@@ -345,6 +344,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2,
                 uint8_t tib_pos = buf2[0x2c];
                 if (tib_pos + tib_len > sizeof(buf2))
                 {
+                    msg(M_ERR, "NTLM: phase 2 response from server too long (need %d bytes at offset %u)", tib_len, tib_pos);
                     return NULL;
                 }
                 /* Get Target Information block pointer */

diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index 76e27cb4ac496b1873bd32955641aea97aaf97b1..b2e8b3d369e1de68df60769311aed053a6a7720f 100644 (file)
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -638,7 +638,6 @@ establish_http_proxy_passthru(struct http_proxy_info *p,
 {
     struct gc_arena gc = gc_new();
     char buf[512];
-    char buf2[129];
     char get[80];
     int status;
     int nparms;
@@ -758,7 +757,7 @@ establish_http_proxy_passthru(struct http_proxy_info *p,
         {
 #if NTLM
             /* look for the phase 2 response */
-
+            char buf2[512];
             while (true)
             {
                 if (!recv_line(sd, buf, sizeof(buf), get_server_poll_remaining_time(server_poll_timeout), true, NULL, signal_received))
@@ -768,9 +767,9 @@ establish_http_proxy_passthru(struct http_proxy_info *p,
                 chomp(buf);
                 msg(D_PROXY, "HTTP proxy returned: '%s'", buf);
 
-                openvpn_snprintf(get, sizeof get, "%%*s NTLM %%%ds", (int) sizeof(buf2) - 1);
+                CLEAR(buf2);
+                openvpn_snprintf(get, sizeof(get), "%%*s NTLM %%%zus", sizeof(buf2) - 1);
                 nparms = sscanf(buf, get, buf2);
-                buf2[128] = 0; /* we only need the beginning - ensure it's null terminated. */
 
                 /* check for "Proxy-Authenticate: NTLM TlRM..." */
                 if (nparms == 1)