Hi, On 10-11-15 20:48, Gert Doering wrote: > On Fri, Oct 30, 2015 at 03:07:47PM +0100, Holger Kummert wrote: >> Am 30.10.2015 um 14:58 schrieb Steffan Karger: > [..] >>> Seems I forgot the * before the second msg_bufpos, sorry. msg_bufpos >>> itself is a pointer indeed, but *msg_bufpos is (used as) an offset >>> into msg_buf: >>> >>> memcpy(&msg_buf[*msg_bufpos], data, msg_buf[sb_offset]); >>> >>> msg_bufpos (without the *) is not related to msg_buf. >> >> Ah, then your first proposal makes sense >> (i.e. if (msg_buflen - length < *msg_bufpos) ) >> >> We could take that. > > So, how do we move onward here? Holger, can you send a new patch (set) > incorporating Steffan's comments? > > Since we found a proper reviewer now, merging this looks possible, after > all the time... ;-)
Would still be nice to get this patch in. I've had this patch (plus a type-fix patch) in my local repo for a long time now, so to get things started a bit, I rebased it onto the current (post-reformatting) master branch. The patch is attached. Any news from the author's side? -Steffan
From c5f5fc4278d34e8d99733672a6a324830907aa23 Mon Sep 17 00:00:00 2001
From: Holger Kummert <holger.kumm...@sophos.com>
Date: Fri, 4 Jul 2014 09:35:24 +0200
Subject: [PATCH] Get NTLMv1 and NTLMv2 up and running
* Force conversion to UTF-16 of username and domain if server requires UTF-16.
* Rewrite conversion function to cleanly convert UTF-8 to UTF-16.
* Fix bug in length computation in NTLMv2-code.
* Architecture independent access to NTLM NegotiateFlags.
Signed-off-by: Holger Kummert <holger.kumm...@sophos.com>
---
src/openvpn/ntlm.c | 190 ++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 166 insertions(+), 24 deletions(-)
diff --git a/src/openvpn/ntlm.c b/src/openvpn/ntlm.c
index e78af9e..062efba 100644
--- a/src/openvpn/ntlm.c
+++ b/src/openvpn/ntlm.c
@@ -56,6 +56,21 @@
#endif
+#define UNI_MAX_UTF16 0x0010FFFF
+#define UNI_HALF_BASE 0x00010000
+#define UNI_HALF_MASK 0x000003FF
+#define UNI_HALF_SHIFT 10
+#define UNI_SUR_HIGH_START 0xD800
+#define UNI_SUR_LOW_START 0xDC00
+#define UNI_SUR_LOW_END 0xDFFF
+
+
+#define NTLM_NEGOTIATE_UNICODE (1<<0)
+#define NTLM_NEGOTIATE_OEM (1<<1)
+#define NTLM_NEGOTIATE_NTLM2_KEY (1<<19)
+#define NTLM_NEGOTIATE_TARGET_INFO (1<<23)
+
+
static void
create_des_keys(const unsigned char *hash, unsigned char *key)
@@ -78,8 +93,11 @@ gen_md4_hash(const char *data, int data_len, char *result)
const md_kt_t *md4_kt = md_kt_get("MD4");
char md[MD4_DIGEST_LENGTH];
- md_full(md4_kt, data, data_len, md);
- memcpy(result, md, MD4_DIGEST_LENGTH);
+ if (data_len >= 0)
+ {
+ md_full(md4_kt, data, data_len, md);
+ memcpy(result, md, MD4_DIGEST_LENGTH);
+ }
}
static void
@@ -140,23 +158,106 @@ my_strupr(unsigned char *str)
}
static int
-unicodize(char *dst, const char *src)
+utf8_to_utf16LE(unsigned char *dst, int dst_len, unsigned char *src)
{
- /* not really unicode... */
- int i = 0;
- do
+ /* Convert UTF-8 to UTF-16 little endian
+ *
+ * http://clang.llvm.org/doxygen/ConvertUTF_8c_source.html
+ */
+ unsigned char *end = src + strlen(src),
+ *dst_start = dst;
+ const char trailingBytesForUTF8[256] = {
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
+ 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, 1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,
+ 2,2,2,2,2,2,2,2,2,2,2,2,2,2,2,2, 3,3,3,3,3,3,3,3,4,4,4,4,5,5,5,5
+ };
+ const unsigned int offsetsUtf8[6] = { 0x00000000UL, 0x00003080UL, 0x000E2080UL,
+ 0x03C82080UL, 0xFA082080UL, 0x82082080UL };
+
+ while (src < end)
{
- dst[i++] = *src;
- dst[i++] = 0;
+ unsigned int ch = 0,
+ extraBytes = trailingBytesForUTF8[*src];
+
+ if (extraBytes >= end - src)
+ {
+ return -1;
+ }
+
+ /* The cases all fall through. */
+ switch (extraBytes) {
+ case 5: ch += *src++; ch <<= 6;
+
+ case 4: ch += *src++; ch <<= 6;
+
+ case 3: ch += *src++; ch <<= 6;
+
+ case 2: ch += *src++; ch <<= 6;
+
+ case 1: ch += *src++; ch <<= 6;
+
+ case 0: ch += *src++;
+ }
+ ch -= offsetsUtf8[extraBytes]; /* clean up UTF-8 control bits */
+
+ /* Now 'ch' contains the codepoint of the UTF-8-character. */
+
+ if (ch <= 0xFFFF)
+ {
+ if (ch >= UNI_SUR_HIGH_START && ch <= UNI_SUR_LOW_END)
+ {
+ msg(M_INFO, "Warning: Illegal character value: %x is in reserved area of UTF-16 [%x, %x]", UNI_SUR_HIGH_START, UNI_SUR_LOW_END);
+ return -1;
+ }
+ else /* normal case */
+ {
+ if (dst_len - (dst - dst_start) < 2)
+ {
+ msg(M_INFO, "Warning: Not enough space in destination buffer for conversion to UTF-16");
+ return -1;
+ }
+ *dst++ = ch & 0xff;
+ *dst++ = (ch >> 8) & 0xff;
+ }
+ }
+ else if (ch > UNI_MAX_UTF16)
+ {
+ msg(M_INFO, "Warning: Illegal character value :%x is too large for UTF-16", ch);
+ return -1;
+ }
+ else
+ {
+ /* character is in range 0xFFFF - 0x10FFFF. */
+ if (dst_len - (dst - dst_start) < 4)
+ {
+ msg(M_INFO, "Warning: Not enough space in destination buffer for conversion to UTF-16");
+ return -1;
+ }
+ ch -= UNI_HALF_BASE;
+ *dst++ = ( (ch >> UNI_HALF_SHIFT) + UNI_SUR_HIGH_START) & 0xff;
+ *dst++ = (((ch >> UNI_HALF_SHIFT) + UNI_SUR_HIGH_START) >> 8) & 0xff;
+ *dst++ = ( (ch & UNI_HALF_MASK) + UNI_SUR_LOW_START) & 0xff;
+ *dst++ = (((ch & UNI_HALF_MASK) + UNI_SUR_LOW_START) >> 8) & 0xff;
+ }
}
- while (*src++);
- return i;
+ return dst - dst_start;
}
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, int msg_buflen)
{
+ if ((*msg_bufpos - *msg_buf) + length > msg_buflen)
+ {
+ msg(M_INFO, "Warning: Not enough space in destination buffer");
+ return;
+ }
+
/* 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];
@@ -195,10 +296,12 @@ 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 */
char buf2[128]; /* decoded reply from proxy */
unsigned char phase3[464];
+ unsigned long ntlm_negotiate_flags;
char md4_hash[MD4_DIGEST_LENGTH+5];
char challenge[8], ntlm_response[24];
int i, ret_val;
+ int ntlm_unicode;
char ntlmv2_response[144];
char userdomain_u[256]; /* for uppercase unicode username and domain */
@@ -211,7 +314,9 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
size_t len;
char domain[128];
- char username[128];
+ char domain_u[256];
+ char username[USER_PASS_LEN];
+ char username_u[USER_PASS_LEN*2];
char *separator;
bool ntlmv2_enabled = (p->auth_method == HTTP_AUTH_NTLM2);
@@ -244,7 +349,7 @@ 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(pwbuf, utf8_to_utf16LE(pwbuf, sizeof(p->up.password) * 2, (unsigned char *)p->up.password), md4_hash);
/* pad to 21 bytes */
memset(md4_hash + MD4_DIGEST_LENGTH, 0, 5);
@@ -258,12 +363,23 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
/* we can be sure that phase_2 is less than 128
* therefore buf2 needs to be (3/4 * 128) */
+ /* extract NegotiateFlags from bytes 20-23 */
+ ntlm_negotiate_flags = (buf2[0x14] & 0xff) | (buf2[0x15] & 0xff) << 8 | (buf2[0x16] & 0xff) << 16 | (buf2[0x17] & 0xff) << 24;
+
+ ntlm_unicode = ntlm_negotiate_flags & NTLM_NEGOTIATE_UNICODE ? 1 : 0;
+
/* extract the challenge from bytes 24-31 */
for (i = 0; i<8; i++)
{
challenge[i] = buf2[i+24];
}
+ msg(M_DEBUG, "Received NTLM negotiate flags 0x%08x: %s encoding, %sNTLM2sec, %sTargetInfo - doing NTLMv%s",
+ ntlm_negotiate_flags, ntlm_unicode ? "Unicode" : "OEM",
+ ntlm_negotiate_flags & NTLM_NEGOTIATE_NTLM2_KEY ? "" : "no ",
+ ntlm_negotiate_flags & NTLM_NEGOTIATE_TARGET_INFO ? "" : "no ",
+ ntlmv2_enabled ? "2" : "1");
+
if (ntlmv2_enabled) /* Generate NTLMv2 response */
{
int tib_len;
@@ -278,8 +394,11 @@ 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);
+ len = utf8_to_utf16LE(userdomain_u, sizeof(userdomain_u), userdomain);
+ if (len >= 0)
+ {
+ gen_hmac_md5(userdomain_u, len, md4_hash, MD5_DIGEST_LENGTH, ntlmv2_hash);
+ }
/* NTLMv2 Blob */
memset(ntlmv2_blob, 0, 128); /* Clear blob buffer */
@@ -291,9 +410,9 @@ 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 */
+ if (ntlm_negotiate_flags & NTLM_NEGOTIATE_TARGET_INFO) /* Check for Target Information block */
{
- tib_len = buf2[0x28]; /* Get Target Information block size */
+ tib_len = buf2[0x28] & 0xff | (buf2[0x29] & 0xff) << 8; /* Get Target Information block size */
if (tib_len > 96)
{
tib_len = 96;
@@ -311,7 +430,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
ntlmv2_blob[0x1c + tib_len] = 0; /* Unknown, zero works */
/* Get blob length */
- ntlmv2_blob_size = 0x20 + tib_len;
+ ntlmv2_blob_size = 0x1c + tib_len;
/* Add challenge from message 2 */
memcpy(&ntlmv2_response[8], challenge, 8);
@@ -345,18 +464,41 @@ 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, sizeof(phase3));
}
else /* NTLM response */
{
- add_security_buffer(0x14, ntlm_response, 24, phase3, &phase3_bufpos);
+ add_security_buffer(0x14, ntlm_response, 24, phase3, &phase3_bufpos, sizeof(phase3));
+ }
+
+ /* Set username. */
+ if (ntlm_unicode)
+ {
+ len = utf8_to_utf16LE(username_u, sizeof(username_u), username);
+ if (len>=0)
+ {
+ add_security_buffer(0x24, username_u, len, phase3, &phase3_bufpos, sizeof(phase3));
+ }
+ }
+ else
+ {
+ add_security_buffer(0x24, username, strlen(username), phase3, &phase3_bufpos, sizeof(phase3));
}
- /* username in ascii */
- 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) */
- add_security_buffer(0x1c, domain, strlen(domain), phase3, &phase3_bufpos);
+ if (ntlm_unicode)
+ {
+ len = utf8_to_utf16LE(domain_u, sizeof(domain_u), domain);
+ if (len>=0)
+ {
+ add_security_buffer(0x1c, domain_u, len, phase3, &phase3_bufpos, sizeof(phase3));
+ }
+ }
+ else
+ {
+ add_security_buffer(0x1c, domain, strlen(domain), phase3, &phase3_bufpos, sizeof(phase3));
+ }
/* other security buffers will be empty */
@@ -365,7 +507,7 @@ ntlm_phase_3(const struct http_proxy_info *p, const char *phase_2, struct gc_are
phase3[0x38] = phase3_bufpos; /* no session key */
/* flags */
- phase3[0x3c] = 0x02; /* negotiate oem */
+ phase3[0x3c] = ntlm_unicode ? 0x01 : 0x02; /* negotiate unicode/oem */
phase3[0x3d] = 0x02; /* negotiate ntlm */
return ((const char *)make_base64_string2((unsigned char *)phase3, phase3_bufpos, gc));
--
2.7.4
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
