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

state info each packet.
Jeremy.

diff --git a/source/libsmb/clitrans.c b/source/libsmb/clitrans.c
index 3eb7fcc2168d0984861ca30fdac5551d2cea1a88..1602dcc6839f044f40ec4a81b1ee13acf6ac6542 100644 (file)
--- 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 755a1548eb9680935cfa279b2646fa9f6cd88bc6..cb35fda220f8ef19bdf8ede5e0e2bdc5a054564d 100644 (file)
--- 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;
 }
 
 /***********************************************************