More fixes for client and server side signing. Ensure sequence numbers

are updated correctly on returning an error for server trans streams.
Ensure we turn off client trans streams on error.
Jeremy.
(This used to be commit 3a789cb7f01115c37404e5a696de363287cb0e5f)

4 files changed, 108 insertions, 35 deletions

diff --git a/source3/libsmb/clitrans.c b/source3/libsmb/clitrans.c
index 49897d0bb0..e6771ac688 100644
--- a/source3/libsmb/clitrans.c
+++ b/source3/libsmb/clitrans.c
@@ -88,13 +88,17 @@ BOOL cli_send_trans(struct cli_state *cli, int trans,
 	show_msg(cli->outbuf);
 
 	cli_signing_trans_start(cli);
-	if (!cli_send_smb(cli))
+	if (!cli_send_smb(cli)) {
+		cli_signing_trans_stop(cli);
 		return False;
+	}
 
 	if (this_ldata < ldata || this_lparam < lparam) {
 		/* receive interim response */
-		if (!cli_receive_smb(cli) || cli_is_error(cli))
+		if (!cli_receive_smb(cli) || cli_is_error(cli)) {
+			cli_signing_trans_stop(cli);
 			return(False);
+		}
 
 		tot_data = this_ldata;
 		tot_param = this_lparam;
@@ -131,8 +135,10 @@ BOOL cli_send_trans(struct cli_state *cli, int trans,
 			cli->mid = mid;
 
 			show_msg(cli->outbuf);
-			if (!cli_send_smb(cli))
+			if (!cli_send_smb(cli)) {
+				cli_signing_trans_stop(cli);
 				return False;
+			}
 			
 			tot_data += this_ldata;
 			tot_param += this_lparam;
@@ -159,8 +165,10 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 
 	*data_len = *param_len = 0;
 
-	if (!cli_receive_smb(cli))
+	if (!cli_receive_smb(cli)) {
+		cli_signing_trans_stop(cli);
 		return False;
+	}
 
 	show_msg(cli->inbuf);
 	
@@ -169,6 +177,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 		DEBUG(0,("Expected %s response, got command 0x%02x\n",
 			 trans==SMBtrans?"SMBtrans":"SMBtrans2", 
 			 CVAL(cli->inbuf,smb_com)));
+		cli_signing_trans_stop(cli);
 		return(False);
 	}
 
@@ -179,8 +188,10 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 	 */
 	status = cli_nt_error(cli);
 	
-	if (NT_STATUS_IS_ERR(status))
+	if (NT_STATUS_IS_ERR(status)) {
+		cli_signing_trans_stop(cli);
 		return False;
+	}
 
 	/* parse out the lengths */
 	total_data = SVAL(cli->inbuf,smb_tdrcnt);
@@ -191,6 +202,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 		tdata = Realloc(*data,total_data);
 		if (!tdata) {
 			DEBUG(0,("cli_receive_trans: failed to enlarge data buffer\n"));
+			cli_signing_trans_stop(cli);
 			return False;
 		}
 		else
@@ -201,6 +213,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 		tparam = Realloc(*param,total_param);
 		if (!tparam) {
 			DEBUG(0,("cli_receive_trans: failed to enlarge param buffer\n"));
+			cli_signing_trans_stop(cli);
 			return False;
 		}
 		else
@@ -214,6 +227,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 		if (this_data + *data_len > total_data ||
 		    this_param + *param_len > total_param) {
 			DEBUG(1,("Data overflow in cli_receive_trans\n"));
+			cli_signing_trans_stop(cli);
 			return False;
 		}
 
@@ -222,6 +236,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 				this_param + *param_len < this_param ||
 				this_param + *param_len < *param_len) {
 			DEBUG(1,("Data overflow in cli_receive_trans\n"));
+			cli_signing_trans_stop(cli);
 			return False;
 		}
 
@@ -234,6 +249,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 					data_offset_out + this_data < data_offset_out ||
 					data_offset_out + this_data < this_data) {
 				DEBUG(1,("Data overflow in cli_receive_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 			if (data_offset_in > cli->bufsize ||
@@ -241,6 +257,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 					data_offset_in + this_data < data_offset_in ||
 					data_offset_in + this_data < this_data) {
 				DEBUG(1,("Data overflow in cli_receive_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 
@@ -255,6 +272,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 					param_offset_out + this_param < param_offset_out ||
 					param_offset_out + this_param < this_param) {
 				DEBUG(1,("Param overflow in cli_receive_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 			if (param_offset_in > cli->bufsize ||
@@ -262,6 +280,7 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 					param_offset_in + this_param < param_offset_in ||
 					param_offset_in + this_param < this_param) {
 				DEBUG(1,("Param overflow in cli_receive_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 
@@ -273,8 +292,10 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 		if (total_data <= *data_len && total_param <= *param_len)
 			break;
 		
-		if (!cli_receive_smb(cli))
-			return False;
+		if (!cli_receive_smb(cli)) {
+			cli_signing_trans_stop(cli);
+			return False;	
+		}
 
 		show_msg(cli->inbuf);
 		
@@ -283,9 +304,11 @@ BOOL cli_receive_trans(struct cli_state *cli,int trans,
 			DEBUG(0,("Expected %s response, got command 0x%02x\n",
 				 trans==SMBtrans?"SMBtrans":"SMBtrans2", 
 				 CVAL(cli->inbuf,smb_com)));
+			cli_signing_trans_stop(cli);
 			return(False);
 		}
 		if (NT_STATUS_IS_ERR(cli_nt_error(cli))) {
+			cli_signing_trans_stop(cli);
 			return(False);
 		}
 
@@ -359,13 +382,17 @@ BOOL cli_send_nt_trans(struct cli_state *cli,
 
 	show_msg(cli->outbuf);
 	cli_signing_trans_start(cli);
-	if (!cli_send_smb(cli))
+	if (!cli_send_smb(cli)) {
+		cli_signing_trans_stop(cli);
 		return False;
+	}	
 
 	if (this_ldata < ldata || this_lparam < lparam) {
 		/* receive interim response */
-		if (!cli_receive_smb(cli) || cli_is_error(cli))
+		if (!cli_receive_smb(cli) || cli_is_error(cli)) {
+			cli_signing_trans_stop(cli);
 			return(False);
+		}
 
 		tot_data = this_ldata;
 		tot_param = this_lparam;
@@ -402,8 +429,10 @@ BOOL cli_send_nt_trans(struct cli_state *cli,
 
 			show_msg(cli->outbuf);
 
-			if (!cli_send_smb(cli))
+			if (!cli_send_smb(cli)) {
+				cli_signing_trans_stop(cli);
 				return False;
+			}
 			
 			tot_data += this_ldata;
 			tot_param += this_lparam;
@@ -413,8 +442,6 @@ BOOL cli_send_nt_trans(struct cli_state *cli,
 	return(True);
 }
 
-
-
 /****************************************************************************
   receive a SMB nttrans response allocating the necessary memory
   ****************************************************************************/
@@ -433,8 +460,10 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 
 	*data_len = *param_len = 0;
 
-	if (!cli_receive_smb(cli))
+	if (!cli_receive_smb(cli)) {
+		cli_signing_trans_stop(cli);
 		return False;
+	}
 
 	show_msg(cli->inbuf);
 	
@@ -442,6 +471,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 	if (CVAL(cli->inbuf,smb_com) != SMBnttrans) {
 		DEBUG(0,("Expected SMBnttrans response, got command 0x%02x\n",
 			 CVAL(cli->inbuf,smb_com)));
+		cli_signing_trans_stop(cli);
 		return(False);
 	}
 
@@ -452,8 +482,10 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 	 */
 	if (cli_is_dos_error(cli)) {
                 cli_dos_error(cli, &eclass, &ecode);
-		if (cli->nt_pipe_fnum == 0 || !(eclass == ERRDOS && ecode == ERRmoredata))
+		if (cli->nt_pipe_fnum == 0 || !(eclass == ERRDOS && ecode == ERRmoredata)) {
+			cli_signing_trans_stop(cli);
 			return(False);
+		}
 	}
 
 	/* parse out the lengths */
@@ -465,6 +497,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 		tdata = Realloc(*data,total_data);
 		if (!tdata) {
 			DEBUG(0,("cli_receive_nt_trans: failed to enlarge data buffer to %d\n",total_data));
+			cli_signing_trans_stop(cli);
 			return False;
 		} else {
 			*data = tdata;
@@ -475,6 +508,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 		tparam = Realloc(*param,total_param);
 		if (!tparam) {
 			DEBUG(0,("cli_receive_nt_trans: failed to enlarge param buffer to %d\n", total_param));
+			cli_signing_trans_stop(cli);
 			return False;
 		} else {
 			*param = tparam;
@@ -488,6 +522,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 		if (this_data + *data_len > total_data ||
 		    this_param + *param_len > total_param) {
 			DEBUG(1,("Data overflow in cli_receive_nt_trans\n"));
+			cli_signing_trans_stop(cli);
 			return False;
 		}
 
@@ -496,6 +531,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 				this_param + *param_len < this_param ||
 				this_param + *param_len < *param_len) {
 			DEBUG(1,("Data overflow in cli_receive_nt_trans\n"));
+			cli_signing_trans_stop(cli);
 			return False;
 		}
 
@@ -508,6 +544,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 					data_offset_out + this_data < data_offset_out ||
 					data_offset_out + this_data < this_data) {
 				DEBUG(1,("Data overflow in cli_receive_nt_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 			if (data_offset_in > cli->bufsize ||
@@ -515,6 +552,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 					data_offset_in + this_data < data_offset_in ||
 					data_offset_in + this_data < this_data) {
 				DEBUG(1,("Data overflow in cli_receive_nt_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 
@@ -530,6 +568,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 					param_offset_out + this_param < param_offset_out ||
 					param_offset_out + this_param < this_param) {
 				DEBUG(1,("Param overflow in cli_receive_nt_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 			if (param_offset_in > cli->bufsize ||
@@ -537,6 +576,7 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 					param_offset_in + this_param < param_offset_in ||
 					param_offset_in + this_param < this_param) {
 				DEBUG(1,("Param overflow in cli_receive_nt_trans\n"));
+				cli_signing_trans_stop(cli);
 				return False;
 			}
 
@@ -549,8 +589,10 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 		if (total_data <= *data_len && total_param <= *param_len)
 			break;
 		
-		if (!cli_receive_smb(cli))
+		if (!cli_receive_smb(cli)) {
+			cli_signing_trans_stop(cli);
 			return False;
+		}
 
 		show_msg(cli->inbuf);
 		
@@ -558,13 +600,15 @@ BOOL cli_receive_nt_trans(struct cli_state *cli,
 		if (CVAL(cli->inbuf,smb_com) != SMBnttrans) {
 			DEBUG(0,("Expected SMBnttrans response, got command 0x%02x\n",
 				 CVAL(cli->inbuf,smb_com)));
+			cli_signing_trans_stop(cli);
 			return(False);
 		}
 		if (cli_is_dos_error(cli)) {
                         cli_dos_error(cli, &eclass, &ecode);
-			if(cli->nt_pipe_fnum == 0 || 
-                           !(eclass == ERRDOS && ecode == ERRmoredata))
+			if(cli->nt_pipe_fnum == 0 || !(eclass == ERRDOS && ecode == ERRmoredata)) {
+				cli_signing_trans_stop(cli);
 				return(False);
+			}
 		}
 		/* parse out the total lengths again - they can shrink! */
 		if (SVAL(cli->inbuf,smb_ntr_TotalDataCount) < total_data)
diff --git a/source3/libsmb/smb_signing.c b/source3/libsmb/smb_signing.c
index 5fc44eb953..a62b5a4cdf 100644
--- a/source3/libsmb/smb_signing.c
+++ b/source3/libsmb/smb_signing.c
@@ -186,9 +186,6 @@ static void free_signing_context(struct smb_sign_info *si)
 
 static BOOL signing_good(char *inbuf, struct smb_sign_info *si, BOOL good) 
 {
-	DEBUG(10, ("signing_good: got SMB signature of\n"));
-	dump_data(10,&inbuf[smb_ss_field] , 8);
-
 	if (good && !si->doing_signing) {
 		si->doing_signing = True;
 	}
@@ -199,11 +196,11 @@ static BOOL signing_good(char *inbuf, struct smb_sign_info *si, BOOL good)
 
 			/* W2K sends a bad first signature but the sign engine is on.... JRA. */
 			if (data->send_seq_num > 1)
-				DEBUG(1, ("SMB signature check failed!\n"));
+				DEBUG(1, ("signing_good: SMB signature check failed!\n"));
 
 			return False;
 		} else {
-			DEBUG(3, ("Server did not sign reply correctly\n"));
+			DEBUG(3, ("signing_good: Peer did not sign reply correctly\n"));
 			free_signing_context(si);
 			return False;
 		}
@@ -366,6 +363,9 @@ static BOOL client_check_incoming_message(char *inbuf, struct smb_sign_info *si)
 		}
 #endif /* JRATEST */
 
+	} else {
+		DEBUG(10, ("client_check_incoming_message:: seq %u: got good SMB signature of\n", (unsigned int)reply_seq_number));
+		dump_data(10, server_sent_mac, 8);
 	}
 	return signing_good(inbuf, si, good);
 }
@@ -455,7 +455,7 @@ void cli_signing_trans_start(struct cli_state *cli)
 {
 	struct smb_basic_signing_context *data = cli->sign_info.signing_context;
 
-	if (!cli->sign_info.doing_signing)
+	if (!cli->sign_info.doing_signing || !data)
 		return;
 
 	data->trans_info = smb_xmalloc(sizeof(struct trans_info_context));
@@ -464,6 +464,13 @@ void cli_signing_trans_start(struct cli_state *cli)
 	data->trans_info->send_seq_num = data->send_seq_num;
 	data->trans_info->mid = SVAL(cli->outbuf,smb_mid);
 	data->trans_info->reply_seq_num = data->send_seq_num+1;
+
+	DEBUG(10,("cli_signing_trans_start: storing mid = %u, reply_seq_num = %u, send_seq_num = %u \
+data->send_seq_num = %u\n",
+			(unsigned int)data->trans_info->mid,
+			(unsigned int)data->trans_info->reply_seq_num,
+			(unsigned int)data->trans_info->send_seq_num,
+			(unsigned int)data->send_seq_num ));
 }
 
 /***********************************************************
@@ -474,7 +481,7 @@ void cli_signing_trans_stop(struct cli_state *cli)
 {
 	struct smb_basic_signing_context *data = cli->sign_info.signing_context;
 
-	if (!cli->sign_info.doing_signing)
+	if (!cli->sign_info.doing_signing || !data)
 		return;
 
 	SAFE_FREE(data->trans_info);
@@ -638,12 +645,15 @@ static void srv_sign_outgoing_message(char *outbuf, struct smb_sign_info *si)
 	/* See if this is a reply for a deferred packet. */
 	was_deferred_packet = get_sequence_for_reply(&data->outstanding_packet_list, mid, &send_seq_number);
 
-	if (data->trans_info && (data->trans_info->mid == mid))
+	if (data->trans_info && (data->trans_info->mid == mid)) {
+		/* This is a reply in a trans stream. Use the sequence
+		 * number associated with the stream mid. */
 		send_seq_number = data->trans_info->send_seq_num;
+	}
 
 	simple_packet_signature(data, outbuf, send_seq_number, calc_md5_mac);
 
-	DEBUG(10, ("srv_sign_outgoing_message: sent SMB signature of\n"));
+	DEBUG(10, ("srv_sign_outgoing_message: seq %u: sent SMB signature of\n", (unsigned int)send_seq_number));
 	dump_data(10, calc_md5_mac, 8);
 
 	memcpy(&outbuf[smb_ss_field], calc_md5_mac, 8);
@@ -651,8 +661,16 @@ static void srv_sign_outgoing_message(char *outbuf, struct smb_sign_info *si)
 /*	cli->outbuf[smb_ss_field+2]=0; 
 	Uncomment this to test if the remote server actually verifies signatures...*/
 
-	if (!was_deferred_packet && !data->trans_info)
-		data->send_seq_num++;
+	if (!was_deferred_packet) {
+	       	if (!data->trans_info) {
+			/* Always increment if not in a trans stream. */
+			data->send_seq_num++;
+		} else if ((data->trans_info->send_seq_num == data->send_seq_num) || (data->trans_info->mid != mid)) {
+			/* Increment if this is the first reply in a trans stream or a
+			 * packet that doesn't belong to this stream (different mid). */
+			data->send_seq_num++;
+		}
+	}
 }
 
 /***********************************************************
@@ -717,6 +735,9 @@ static BOOL srv_check_incoming_message(char *inbuf, struct smb_sign_info *si)
 		}
 #endif /* JRATEST */
 
+	} else {
+		DEBUG(10, ("srv_check_incoming_message: seq %u: got good SMB signature of\n", (unsigned int)reply_seq_number));
+		dump_data(10, server_sent_mac, 8);
 	}
 	return signing_good(inbuf, si, good);
 }
@@ -830,6 +851,8 @@ void srv_signing_trans_start(uint16 mid)
 		return;
 
 	data = (struct smb_basic_signing_context *)srv_sign_info.signing_context;
+	if (!data)
+		return;
 
 	data->trans_info = smb_xmalloc(sizeof(struct trans_info_context));
 	ZERO_STRUCTP(data->trans_info);
@@ -838,9 +861,12 @@ void srv_signing_trans_start(uint16 mid)
 	data->trans_info->mid = mid;
 	data->trans_info->send_seq_num = data->send_seq_num;
 
-	/* Increment now in case we need to send a non-trans
-	 * reply in the middle of the trans stream. */
-	data->send_seq_num++;
+	DEBUG(10,("srv_signing_trans_start: storing mid = %u, reply_seq_num = %u, send_seq_num = %u \
+data->send_seq_num = %u\n",
+			(unsigned int)mid,
+			(unsigned int)data->trans_info->reply_seq_num,
+			(unsigned int)data->trans_info->send_seq_num,
+			(unsigned int)data->send_seq_num ));
 }
 
 /***********************************************************
@@ -855,12 +881,13 @@ void srv_signing_trans_stop(void)
 		return;
 
 	data = (struct smb_basic_signing_context *)srv_sign_info.signing_context;
+	if (!data || !data->trans_info)
+		return;
 
 	SAFE_FREE(data->trans_info);
 	data->trans_info = NULL;
 }
 
-
 /***********************************************************
  Turn on signing from this packet onwards. 
 ************************************************************/
diff --git a/source3/smbd/ipc.c b/source3/smbd/ipc.c
index f0b5c4a92f..39072f9b91 100644
--- a/source3/smbd/ipc.c
+++ b/source3/smbd/ipc.c
@@ -509,11 +509,10 @@ int reply_trans(connection_struct *conn, char *inbuf,char *outbuf, int size, int
 			memcpy(data+ddisp,smb_base(inbuf)+doff,dcnt);      
 		}
 	}
-	
-	
+
 	DEBUG(3,("trans <%s> data=%u params=%u setup=%u\n",
 		 name,tdscnt,tpscnt,suwcnt));
-	
+
 	/*
 	 * WinCE wierdness....
 	 */
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index ab7c6e8344..19e6956d9e 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -800,6 +800,9 @@ static BOOL oplock_break(SMB_DEV_T dev, SMB_INO_T inode, unsigned long file_id,
 			} else if (smb_read_error == READ_ERROR) {
 				DEBUG( 0, ("oplock_break: receive_smb error (%s)\n", strerror(errno)) );
 				shutdown_server = True;
+			} else if (smb_read_error == READ_BAD_SIG) {
+				DEBUG( 0, ("oplock_break: bad signature from client\n" ));
+				shutdown_server = True;
 			} else if (smb_read_error == READ_TIMEOUT) {
 				DEBUG( 0, ( "oplock_break: receive_smb timed out after %d seconds.\n", OPLOCK_BREAK_TIMEOUT ) );
 				oplock_timeout = True;