Updates!

- Don't print an uninitialised buffer in service.c
- Change some charcnv.c functions to take smb_ucs2_t ** instead of void **
- Update NTLMv2 code to use dynamic buffers
- Update experimental SMB signing code - still more work to do
- Move sys_getgrouplist() to SAFE_FREE() and do a DEBUG() on initgroups()
  failure.

Andrew Bartlett
(This used to be commit de1964f7fa855022258a84556b266100b917444b)

9 files changed, 113 insertions, 52 deletions

diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c
index ca611c46b9..bc98f46dc2 100644
--- a/source3/auth/auth_sam.c
+++ b/source3/auth/auth_sam.c
@@ -106,7 +106,10 @@ static BOOL smb_pwd_check_ntlmv2(const DATA_BLOB ntv2_response,
 	client_key_data = data_blob(ntv2_response.data+16, ntv2_response.length-16);
 	memcpy(client_response, ntv2_response.data, sizeof(client_response));
 
-	ntv2_owf_gen(part_passwd, user, domain, kr);
+	if (!ntv2_owf_gen(part_passwd, user, domain, kr)) {
+		return False;
+	}
+
 	SMBOWFencrypt_ntv2(kr, sec_blob, client_key_data, value_from_encryption);
 	if (user_sess_key != NULL)
 	{
diff --git a/source3/lib/charcnv.c b/source3/lib/charcnv.c
index 03337cbf26..cd8aa4fe55 100644
--- a/source3/lib/charcnv.c
+++ b/source3/lib/charcnv.c
@@ -434,12 +434,12 @@ int push_ucs2(const void *base_ptr, void *dest, const char *src, int dest_len, i
  * @retval The number of bytes occupied by the string in the destination
  *         or -1 in case of error.
  **/
-int push_ucs2_talloc(TALLOC_CTX *ctx, void **dest, const char *src)
+int push_ucs2_talloc(TALLOC_CTX *ctx, smb_ucs2_t **dest, const char *src)
 {
 	int src_len = strlen(src)+1;
 
 	*dest = NULL;
-	return convert_string_talloc(ctx, CH_UNIX, CH_UCS2, src, src_len, dest);
+	return convert_string_talloc(ctx, CH_UNIX, CH_UCS2, src, src_len, (void **)dest);
 }
 
 /**
@@ -450,12 +450,12 @@ int push_ucs2_talloc(TALLOC_CTX *ctx, void **dest, const char *src)
  * @retval The number of bytes occupied by the string in the destination
  *         or -1 in case of error.
  **/
-int push_ucs2_allocate(void **dest, const char *src)
+int push_ucs2_allocate(smb_ucs2_t **dest, const char *src)
 {
 	int src_len = strlen(src)+1;
 
 	*dest = NULL;
-	return convert_string_allocate(CH_UNIX, CH_UCS2, src, src_len, dest);	
+	return convert_string_allocate(CH_UNIX, CH_UCS2, src, src_len, (void **)dest);	
 }
 
 /****************************************************************************
diff --git a/source3/lib/system_smbd.c b/source3/lib/system_smbd.c
index 5eda37d903..0cd3086945 100644
--- a/source3/lib/system_smbd.c
+++ b/source3/lib/system_smbd.c
@@ -61,13 +61,14 @@ static int getgrouplist_internals(const char *user, gid_t gid, gid_t *groups, in
 
 	ngrp_saved = getgroups(ngrp_saved, gids_saved);
 	if (ngrp_saved == -1) {
-		free(gids_saved);
+		SAFE_FREE(gids_saved);
 		/* very strange! */
 		return -1;
 	}
 
 	if (initgroups(user, gid) != 0) {
-		free(gids_saved);
+		DEBUG(0, ("getgrouplist_internals: initgroups() failed!\n"));
+		SAFE_FREE(gids_saved);
 		return -1;
 	}
 
diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index 7db377c25f..ded5a843f3 100644
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -237,6 +237,23 @@ static BOOL cli_session_setup_plaintext(struct cli_state *cli, char *user,
 	return True;
 }
 
+static void set_signing_on_cli (struct cli_state *cli, char* pass, uint8 response[24]) 
+{
+	uint8 zero_sig[8];
+	ZERO_STRUCT(zero_sig);
+	if (memcmp(&cli->outbuf[smb_ss_field], zero_sig, 8) != 0) {
+		cli->sign_info.use_smb_signing = True;
+		cli_calculate_mac_key(cli, pass, response);
+	}
+}
+
+static void set_temp_signing_on_cli(struct cli_state *cli) 
+{
+	if (cli->sign_info.negotiated_smb_signing) {
+		cli->sign_info.temp_smb_signing = True;
+	}
+}
+
 
 /**
    do a NT1 NTLM/LM encrypted session setup
@@ -256,7 +273,6 @@ static BOOL cli_session_setup_nt1(struct cli_state *cli, char *user,
 	uchar pword[24];
 	uchar ntpword[24];
 	char *p;
-	BOOL tried_signing = False;
 
 	if (passlen > sizeof(pword) || ntpasslen > sizeof(ntpword)) {
 		return False;
@@ -268,12 +284,12 @@ static BOOL cli_session_setup_nt1(struct cli_state *cli, char *user,
 		ntpasslen = 24;
 		SMBencrypt(pass,cli->secblob.data,pword);
 		SMBNTencrypt(pass,cli->secblob.data,ntpword);
-		if (!cli->sign_info.use_smb_signing && cli->sign_info.negotiated_smb_signing) {
-			cli_calculate_mac_key(cli, pass, ntpword);
-			tried_signing = True;
-		}
+
+		set_temp_signing_on_cli(cli);
+
 	} else {
-		/* pre-encrypted password supplied.  Only used for security=server, can't do
+		/* pre-encrypted password supplied.  Only used for 
+		   security=server, can't do
 		   signing becouse we don't have oringial key */
 		memcpy(pword, pass, 24);
 		if (ntpasslen == 24) {
@@ -307,22 +323,16 @@ static BOOL cli_session_setup_nt1(struct cli_state *cli, char *user,
 	p += clistr_push(cli, p, "Samba", -1, STR_TERMINATE);
 	cli_setup_bcc(cli, p);
 
-	cli_send_smb(cli);
+	if (!cli_send_smb(cli)) {
+		return False;
+	}
+
 	if (!cli_receive_smb(cli)) {
-		if (tried_signing) {
-			/* We only use it if we have a successful non-guest connect */
-			cli->sign_info.use_smb_signing = False;
-		}
 		return False;
 	}
 
 	show_msg(cli->inbuf);
 
-	if (tried_signing && (cli_is_error(cli) || SVAL(cli->inbuf,smb_vwv2) /* guest */)) {
-		/* We only use it if we have a successful non-guest connect */
-		cli->sign_info.use_smb_signing = False;
-	}
-
 	if (cli_is_error(cli)) {
 		return False;
 	}
@@ -337,6 +347,11 @@ static BOOL cli_session_setup_nt1(struct cli_state *cli, char *user,
 
 	fstrcpy(cli->user_name, user);
 
+	if (passlen != 24) {
+		/* Have plaintext orginal */
+		set_signing_on_cli(cli, pass, ntpword);
+	}
+
 	return True;
 }
 
@@ -360,6 +375,9 @@ static DATA_BLOB cli_session_setup_blob(struct cli_state *cli, DATA_BLOB blob)
 
 	set_message(cli->outbuf,12,0,True);
 	SCVAL(cli->outbuf,smb_com,SMBsesssetupX);
+
+	set_temp_signing_on_cli(cli);
+
 	cli_setup_packet(cli);
 			
 	SCVAL(cli->outbuf,smb_vwv0,0xFF);
@@ -375,8 +393,8 @@ static DATA_BLOB cli_session_setup_blob(struct cli_state *cli, DATA_BLOB blob)
 	p += clistr_push(cli, p, "Unix", -1, STR_TERMINATE);
 	p += clistr_push(cli, p, "Samba", -1, STR_TERMINATE);
 	cli_setup_bcc(cli, p);
-
 	cli_send_smb(cli);
+
 	if (!cli_receive_smb(cli))
 		return blob2;
 
@@ -451,7 +469,7 @@ static BOOL cli_session_setup_ntlmssp(struct cli_state *cli, char *user,
 	uint32 neg_flags;
 
 	neg_flags = NTLMSSP_NEGOTIATE_UNICODE | 
-		NTLMSSP_NEGOTIATE_LM_KEY | 
+		NTLMSSP_NEGOTIATE_128 | 
 		NTLMSSP_NEGOTIATE_NTLM;
 
 	memset(sess_key, 0, 16);
@@ -525,7 +543,13 @@ static BOOL cli_session_setup_ntlmssp(struct cli_state *cli, char *user,
 	data_blob_free(&auth);
 	data_blob_free(&blob);
 
-	return !cli_is_error(cli);
+	if (cli_is_error(cli)) {
+		return False;
+	}
+
+	set_signing_on_cli(cli, pass, nthash);
+
+	return True;
 }
 
 /****************************************************************************
@@ -541,9 +565,6 @@ static BOOL cli_session_setup_spnego(struct cli_state *cli, char *user,
 	int i;
 	BOOL got_kerberos_mechanism = False;
 
-	/* spnego security cannot use SMB signing (for now). */
-	cli->sign_info.use_smb_signing = False;
-
 	DEBUG(2,("Doing spnego session setup (blob length=%d)\n", cli->secblob.length));
 
 	/* the server might not even do spnego */
@@ -643,6 +664,9 @@ BOOL cli_session_setup(struct cli_state *cli,
 		return cli_session_setup_plaintext(cli, user, pass, workgroup);
 	}
 
+	/* Indidicate signing */
+	
+
 	/* if the server supports extended security then use SPNEGO */
 	if (cli->capabilities & CAP_EXTENDED_SECURITY) {
 		return cli_session_setup_spnego(cli, user, pass, workgroup);
@@ -891,12 +915,8 @@ BOOL cli_negprot(struct cli_state *cli)
 				    smb_buflen(cli->inbuf)-8, STR_UNICODE|STR_NOALIGN);
 		}
 
-		/* A way to attempt to force SMB signing */
-		if (getenv("CLI_FORCE_SMB_SIGNING"))
+		if ((cli->sec_mode & NEGOTIATE_SECURITY_SIGNATURES_ENABLED))
 			cli->sign_info.negotiated_smb_signing = True;
-                                   
-		if (cli->sign_info.negotiated_smb_signing && !(cli->sec_mode & NEGOTIATE_SECURITY_SIGNATURES_ENABLED))
-			cli->sign_info.negotiated_smb_signing = False;
 
 	} else if (cli->protocol >= PROTOCOL_LANMAN1) {
 		cli->use_spnego = False;
diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c
index 9ae3882301..560d391320 100644
--- a/source3/libsmb/clientgen.c
+++ b/source3/libsmb/clientgen.c
@@ -160,7 +160,8 @@ void cli_setup_packet(struct cli_state *cli)
 		if (cli->use_spnego) {
 			flags2 |= FLAGS2_EXTENDED_SECURITY;
 		}
-		if (cli->sign_info.use_smb_signing)
+		if (cli->sign_info.use_smb_signing 
+		    || cli->sign_info.temp_smb_signing)
 			flags2 |= FLAGS2_SMB_SECURITY_SIGNATURES;
 		SSVAL(cli->outbuf,smb_flg2, flags2);
 	}
@@ -245,6 +246,10 @@ struct cli_state *cli_initialise(struct cli_state *cli)
 		cli->force_dos_errors = True;
 	}
 
+	/* A way to attempt to force SMB signing */
+	if (getenv("CLI_FORCE_SMB_SIGNING"))
+		cli->sign_info.negotiated_smb_signing = True;
+                                   
 	if (!cli->outbuf || !cli->inbuf)
                 goto error;
 
diff --git a/source3/libsmb/clireadwrite.c b/source3/libsmb/clireadwrite.c
index 756a6cce2f..875df11dca 100644
--- a/source3/libsmb/clireadwrite.c
+++ b/source3/libsmb/clireadwrite.c
@@ -127,7 +127,7 @@ ssize_t cli_read(struct cli_state *cli, int fnum, char *buf, off_t offset, size_
 	return total;
 }
 
-#if 0  /* relies on client_recieve_smb(), now a static in libsmb/clientgen.c */
+#if 0  /* relies on client_receive_smb(), now a static in libsmb/clientgen.c */
 
 /* This call is INCOMPATIBLE with SMB signing.  If you remove the #if 0
    you must fix ensure you don't attempt to sign the packets - data
diff --git a/source3/libsmb/smbencrypt.c b/source3/libsmb/smbencrypt.c
index dfa355a7ec..db265c4bf7 100644
--- a/source3/libsmb/smbencrypt.c
+++ b/source3/libsmb/smbencrypt.c
@@ -116,39 +116,63 @@ void nt_lm_owf_gen(const char *pwd, uchar nt_p16[16], uchar p16[16])
 }
 
 /* Does both the NTLMv2 owfs of a user's password */
-void ntv2_owf_gen(const uchar owf[16],
-		  const char *user_n, const char *domain_n, uchar kr_buf[16])
+BOOL ntv2_owf_gen(const uchar owf[16],
+		  const char *user_in, const char *domain_in, uchar kr_buf[16])
 {
-	pstring user_u;
-	pstring dom_u;
+	smb_ucs2_t *user;
+	smb_ucs2_t *domain;
+	
+	int user_byte_len;
+	int domain_byte_len;
+
 	HMACMD5Context ctx;
 
-	int user_l = strlen(user_n);
-	int domain_l = strlen(domain_n);
+	user_byte_len = push_ucs2_allocate(&user, user_in);
+	if (user_byte_len < 0) {
+		DEBUG(0, ("push_uss2_allocate() for user returned %d (probably malloc() failure)\n", user_byte_len));
+		return False;
+	}
 
-	push_ucs2(NULL, user_u, user_n, (user_l+1)*2, STR_UNICODE|STR_NOALIGN|STR_TERMINATE|STR_UPPER);
-	push_ucs2(NULL, dom_u, domain_n, (domain_l+1)*2, STR_UNICODE|STR_NOALIGN|STR_TERMINATE|STR_UPPER);
+	domain_byte_len = push_ucs2_allocate(&domain, domain_in);
+	if (domain_byte_len < 0) {
+		DEBUG(0, ("push_uss2_allocate() for domain returned %d (probably malloc() failure)\n", user_byte_len));
+		return False;
+	}
+
+	strupper_w(user);
+	strupper_w(domain);
+
+	/* We don't want null termination */
+	user_byte_len = user_byte_len - 2;
+	domain_byte_len = domain_byte_len - 2;
+	
+	SMB_ASSERT(user_byte_len >= 0);
+	SMB_ASSERT(domain_byte_len >= 0);
 
 	hmac_md5_init_limK_to_64(owf, 16, &ctx);
-	hmac_md5_update((const unsigned char *)user_u, user_l * 2, &ctx);
-	hmac_md5_update((const unsigned char *)dom_u, domain_l * 2, &ctx);
+	hmac_md5_update((const unsigned char *)user, user_byte_len, &ctx);
+	hmac_md5_update((const unsigned char *)domain, domain_byte_len, &ctx);
 	hmac_md5_final(kr_buf, &ctx);
 
 #ifdef DEBUG_PASSWORD
 	DEBUG(100, ("ntv2_owf_gen: user, domain, owfkey, kr\n"));
-	dump_data(100, user_u, user_l * 2);
-	dump_data(100, dom_u, domain_l * 2);
+	dump_data(100, (const char *)user, user_byte_len);
+	dump_data(100, (const char *)domain, domain_byte_len);
 	dump_data(100, owf, 16);
 	dump_data(100, kr_buf, 16);
 #endif
+
+	SAFE_FREE(user);
+	SAFE_FREE(domain);
+	return True;
 }
 
 /* Does the des encryption from the NT or LM MD4 hash. */
 void SMBOWFencrypt(const uchar passwd[16], const uchar *c8, uchar p24[24])
 {
 	uchar p21[21];
- 
-	memset(p21,'\0',21);
+
+	ZERO_STRUCT(p21);
  
 	memcpy(p21, passwd, 16);    
 	E_P24(p21, c8, p24);
@@ -362,6 +386,12 @@ void cli_caclulate_sign_mac(struct cli_state *cli)
 	unsigned char calc_md5_mac[16];
 	struct MD5Context md5_ctx;
 
+	if (cli->sign_info.temp_smb_signing) {
+		memcpy(&cli->outbuf[smb_ss_field], "SignRequest", 8);
+		cli->sign_info.temp_smb_signing = False;
+		return;
+	}
+
 	if (!cli->sign_info.use_smb_signing) {
 		return;
 	}
@@ -380,6 +410,8 @@ void cli_caclulate_sign_mac(struct cli_state *cli)
 	MD5Final(calc_md5_mac, &md5_ctx);
 
 	memcpy(&cli->outbuf[smb_ss_field], calc_md5_mac, 8);
+/*	cli->outbuf[smb_ss_field+2]=0; 
+	Uncomment this to test if the remote server actually verifies signitures...*/
 	cli->sign_info.send_seq_num++;
 	cli->sign_info.reply_seq_num = cli->sign_info.send_seq_num;
 	cli->sign_info.send_seq_num++;
diff --git a/source3/smbd/mangle_hash.c b/source3/smbd/mangle_hash.c
index 1d4697474c..0446a953ff 100644
--- a/source3/smbd/mangle_hash.c
+++ b/source3/smbd/mangle_hash.c
@@ -730,7 +730,7 @@ static void name_map(char *OutName, BOOL need83, BOOL cache83)
 	DEBUG(5,("name_map( %s, need83 = %s, cache83 = %s)\n", OutName,
 		 need83 ? "True" : "False", cache83 ? "True" : "False"));
 	
-	if (push_ucs2_allocate((void **)&OutName_ucs2, OutName) < 0) {
+	if (push_ucs2_allocate(&OutName_ucs2, OutName) < 0) {
 		DEBUG(0, ("push_ucs2_allocate failed!\n"));
 		return;
 	}
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 7c38cf5793..d475451e6b 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -808,7 +808,7 @@ connection_struct *make_connection(const char *service_in, DATA_BLOB password,
 	} else if ((lp_security() != SEC_SHARE) && (vuser->homes_snum != -1)
 		   && strequal(service_in, lp_servicename(vuser->homes_snum))) {
 		DATA_BLOB no_pw = data_blob(NULL, 0);
-		DEBUG(5, ("making a connection to 'homes' service [%s] created at session setup time\n", service));
+		DEBUG(5, ("making a connection to 'homes' service [%s] created at session setup time\n", service_in));
 		return make_connection_snum(vuser->homes_snum,
 					    vuser, no_pw, 
 					    dev, status);