Better fix for client signing bug. Ensure we don't malloc/free trans signing

state info each packet.
Jeremy.

2 files changed, 40 insertions, 36 deletions

diff --git a/source/libsmb/clitrans.c b/source/libsmb/clitrans.c
index 3eb7fcc2168..1602dcc6839 100644
--- a/source/libsmb/clitrans.c
+++ b/source/libsmb/clitrans.c
@@ -50,6 +50,12 @@ BOOL cli_send_trans(struct cli_state *cli, int trans,
 	SCVAL(cli->outbuf,smb_com,trans);
 	SSVAL(cli->outbuf,smb_tid, cli->cnum);
 	cli_setup_packet(cli);
+
+	/*
+	 * Save the mid we're using. We need this for finding
+	 * signing replies.
+	 */
+
 	mid = cli->mid;
 
 	if (pipe_name) {
@@ -87,16 +93,13 @@ BOOL cli_send_trans(struct cli_state *cli, int trans,
 
 	show_msg(cli->outbuf);
 
-	cli_signing_trans_start(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)) {
-			cli_signing_trans_stop(cli);
 			return(False);
 		}
 
@@ -130,23 +133,14 @@ BOOL cli_send_trans(struct cli_state *cli, int trans,
 				memcpy(outdata,data+tot_data,this_ldata);
 			cli_setup_bcc(cli, outdata+this_ldata);
 			
-			/* Ensure this packet has the same MID as
-			 * the primary. Important in signing. JRA. */
-			cli->mid = mid;
-
 			/*
-			 * Turns out that we need to increment the
-			 * sequence number for each packet until the
-			 * last one in the signing sequence. That's
-			 * the one that matters to check signing replies. JRA.
+			 * Save the mid we're using. We need this for finding
+			 * signing replies.
 			 */
-
-			cli_signing_trans_stop(cli);
-			cli_signing_trans_start(cli);
+			mid = cli->mid;
 
 			show_msg(cli->outbuf);
 			if (!cli_send_smb(cli)) {
-				cli_signing_trans_stop(cli);
 				return False;
 			}
 			
@@ -155,6 +149,10 @@ BOOL cli_send_trans(struct cli_state *cli, int trans,
 		}
 	}
 
+	/* Note we're in a trans state. Save the sequence
+	 * numbers for replies. */
+
+	cli_signing_trans_start(cli, mid);
 	return(True);
 }
 
@@ -362,6 +360,12 @@ BOOL cli_send_nt_trans(struct cli_state *cli,
 	SCVAL(cli->outbuf,smb_com,SMBnttrans);
 	SSVAL(cli->outbuf,smb_tid, cli->cnum);
 	cli_setup_packet(cli);
+
+	/*
+	 * Save the mid we're using. We need this for finding
+	 * signing replies.
+	 */
+
 	mid = cli->mid;
 
 	outparam = smb_buf(cli->outbuf)+3;
@@ -391,16 +395,13 @@ BOOL cli_send_nt_trans(struct cli_state *cli,
 	cli_setup_bcc(cli, outdata+this_ldata);
 
 	show_msg(cli->outbuf);
-	cli_signing_trans_start(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)) {
-			cli_signing_trans_stop(cli);
 			return(False);
 		}
 
@@ -433,24 +434,15 @@ BOOL cli_send_nt_trans(struct cli_state *cli,
 				memcpy(outdata,data+tot_data,this_ldata);
 			cli_setup_bcc(cli, outdata+this_ldata);
 			
-			/* Ensure this packet has the same MID as
-			 * the primary. Important in signing. JRA. */
-			cli->mid = mid;
-
 			/*
-			 * Turns out that we need to increment the
-			 * sequence number for each packet until the
-			 * last one in the signing sequence. That's
-			 * the one that matters to check signing replies. JRA.
+			 * Save the mid we're using. We need this for finding
+			 * signing replies.
 			 */
-
-			cli_signing_trans_stop(cli);
-			cli_signing_trans_start(cli);
+			mid = cli->mid;
 
 			show_msg(cli->outbuf);
 
 			if (!cli_send_smb(cli)) {
-				cli_signing_trans_stop(cli);
 				return False;
 			}
 			
@@ -459,6 +451,10 @@ BOOL cli_send_nt_trans(struct cli_state *cli,
 		}
 	}
 
+	/* Note we're in a trans state. Save the sequence
+	 * numbers for replies. */
+
+	cli_signing_trans_start(cli, mid);
 	return(True);
 }
 
diff --git a/source/libsmb/smb_signing.c b/source/libsmb/smb_signing.c
index 755a1548eb9..cb35fda220f 100644
--- a/source/libsmb/smb_signing.c
+++ b/source/libsmb/smb_signing.c
@@ -457,9 +457,12 @@ BOOL cli_simple_set_signing(struct cli_state *cli, const DATA_BLOB user_session_
 
 /***********************************************************
  Tell client code we are in a multiple trans reply state.
+ We call this after the last outgoing trans2 packet (which
+ has incremented the sequence numbers), so we must save the
+ current mid and sequence number -2.
 ************************************************************/
 
-void cli_signing_trans_start(struct cli_state *cli)
+void cli_signing_trans_start(struct cli_state *cli, uint16 mid)
 {
 	struct smb_basic_signing_context *data = cli->sign_info.signing_context;
 
@@ -469,9 +472,9 @@ void cli_signing_trans_start(struct cli_state *cli)
 	data->trans_info = smb_xmalloc(sizeof(struct trans_info_context));
 	ZERO_STRUCTP(data->trans_info);
 
-	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;
+	data->trans_info->send_seq_num = data->send_seq_num-2;
+	data->trans_info->mid = 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",
@@ -492,10 +495,15 @@ void cli_signing_trans_stop(struct cli_state *cli)
 	if (!cli->sign_info.doing_signing || !data)
 		return;
 
+	DEBUG(10,("cli_signing_trans_stop: freeing 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 ));
+
 	SAFE_FREE(data->trans_info);
 	data->trans_info = NULL;
-
-	data->send_seq_num += 2;
 }
 
 /***********************************************************