Added STR_NOALIGN flags to clistr and srvstr fns. Yes, NT actually does

send unaligned unicode strings sometimes!
Fixed our handling of the workgroup name tacked on the end of the
NT1 negprot response (a unaligned unicode)
fixed a couple of places where we should be using the message_end fns instead
of pre-calculated buffer lengths
(This used to be commit 86613493a9b2e56523153486931d0bf8d39beb7a)

2 files changed, 30 insertions, 26 deletions

diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c
index 83c809de1c..2c6575d643 100644
--- a/source3/smbd/negprot.c
+++ b/source3/smbd/negprot.c
@@ -177,6 +177,7 @@ static int reply_nt1(char *outbuf)
   struct cli_state *cli = NULL;
   char cryptkey[8];
   char crypt_len = 0;
+  char *p;
 
   if (lp_security() == SEC_SERVER) {
 	  cli = server_cryptkey();
@@ -215,18 +216,10 @@ static int reply_nt1(char *outbuf)
   if (lp_security() >= SEC_USER) secword |= 1;
   if (doencrypt) secword |= 2;
 
-  /* decide where (if) to put the encryption challenge, and
-     follow it with the OEM'd domain name
-   */
-  data_len = crypt_len + strlen(global_myworkgroup) + 1;
-
-  set_message(outbuf,17,data_len,True);
-  pstrcpy(smb_buf(outbuf)+crypt_len, global_myworkgroup);
+  set_message(outbuf,17,0,True);
 
   CVAL(outbuf,smb_vwv1) = secword;
   SSVALS(outbuf,smb_vwv16+1,crypt_len);
-  if (doencrypt) 
-	  memcpy(smb_buf(outbuf), cryptkey, 8);
 
   Protocol = PROTOCOL_NT1;
 
@@ -240,6 +233,13 @@ static int reply_nt1(char *outbuf)
   SSVALS(outbuf,smb_vwv15+1,TimeDiff(t)/60);
   SSVAL(outbuf,smb_vwv17,data_len); /* length of challenge+domain strings */
 
+  p = smb_buf(outbuf);
+  if (doencrypt) memcpy(p, cryptkey, 8);
+  p += 8;
+  p += srvstr_push(outbuf, p, global_myworkgroup, -1, 
+		   STR_UNICODE|STR_CONVERT|STR_TERMINATE|STR_NOALIGN);
+  set_message_end(outbuf, p);
+
   return (smb_len(outbuf)+4);
 }
 
diff --git a/source3/smbd/srvstr.c b/source3/smbd/srvstr.c
index 6ca34a2428..0651fb725b 100644
--- a/source3/smbd/srvstr.c
+++ b/source3/smbd/srvstr.c
@@ -23,7 +23,20 @@
 
 #include "includes.h"
 
-#define UNICODE_FLAG(buf) (SVAL(buf, smb_flg2) & FLAGS2_UNICODE_STRINGS)
+#define UNICODE_FLAG(buf, flags) (!(flags & STR_ASCII) && \
+                                  ((flags & STR_UNICODE || \
+                                   (SVAL(buf, smb_flg2) & FLAGS2_UNICODE_STRINGS))))
+
+/****************************************************************************
+return an alignment of either 0 or 1
+if unicode is not negotiated then return 0
+otherwise return 1 if offset is off
+****************************************************************************/
+static int srvstr_align(void *inbuf, int offset, int flags)
+{
+	if ((flags & STR_NOALIGN) || !UNICODE_FLAG(inbuf, flags)) return 0;
+	return offset & 1;
+}
 
 /****************************************************************************
 copy a string from a char* src to a unicode or ascii
@@ -35,6 +48,8 @@ flags can have:
   STR_CONVERT   means convert from unix to dos codepage
   STR_UPPER     means uppercase in the destination
   STR_ASCII     use ascii even with unicode servers
+  STR_UNICODE   means to force as unicode
+  STR_NOALIGN   means don't do alignment
 dest_len is the maximum length allowed in the destination. If dest_len
 is -1 then no maxiumum is used
 ****************************************************************************/
@@ -47,14 +62,14 @@ int srvstr_push(void *outbuf, void *dest, const char *src, int dest_len, int fla
 		dest_len = sizeof(pstring);
 	}
 
-	if (!(flags & STR_ASCII) && srvstr_align(outbuf, PTR_DIFF(dest, outbuf))) {
+	if (srvstr_align(outbuf, PTR_DIFF(dest, outbuf), flags)) {
 		*(char *)dest = 0;
 		dest = (void *)((char *)dest + 1);
 		dest_len--;
 		len++;
 	}
 
-	if ((flags & STR_ASCII) || !UNICODE_FLAG(outbuf)) {
+	if (!UNICODE_FLAG(outbuf, flags)) {
 		/* the client doesn't want unicode */
 		safe_strcpy(dest, src, dest_len);
 		len = strlen(dest);
@@ -85,6 +100,7 @@ flags can have:
   STR_CONVERT   means convert from dos to unix codepage
   STR_TERMINATE means the string in src is null terminated
   STR_UNICODE   means to force as unicode
+  STR_NOALIGN   means don't do alignment
 if STR_TERMINATE is set then src_len is ignored
 src_len is the length of the source area in bytes
 return the number of bytes occupied by the string in src
@@ -97,12 +113,12 @@ int srvstr_pull(void *inbuf, char *dest, const void *src, int dest_len, int src_
 		dest_len = sizeof(pstring);
 	}
 
-	if (!(flags & STR_ASCII) && srvstr_align(inbuf, PTR_DIFF(src, inbuf))) {
+	if (srvstr_align(inbuf, PTR_DIFF(src, inbuf), flags)) {
 		src = (void *)((char *)src + 1);
 		if (src_len > 0) src_len--;
 	}
 
-	if ((flags & STR_ASCII) || (!(flags & STR_UNICODE) && !UNICODE_FLAG(inbuf))) {
+	if (!UNICODE_FLAG(inbuf, flags)) {
 		/* the server doesn't want unicode */
 		if (flags & STR_TERMINATE) {
 			safe_strcpy(dest, src, dest_len);
@@ -135,18 +151,6 @@ int srvstr_pull(void *inbuf, char *dest, const void *src, int dest_len, int src_
 }
 
 /****************************************************************************
-return an alignment of either 0 or 1
-if unicode is not negotiated then return 0
-otherwise return 1 if offset is off
-****************************************************************************/
-int srvstr_align(void *inbuf, int offset)
-{
-	if (!UNICODE_FLAG(inbuf)) return 0;
-	return offset & 1;
-}
-
-
-/****************************************************************************
 these are useful for replacing all those StrnCpy() ops for copying data
 to/from the wire 
 ****************************************************************************/