Commit to HEAD the updates to smb signing code that I was propsing for 3.0.

This code implements 'opportunistic signing' in our client (when the
server supports it, we will use it), and correct downgrading on both
the client and server for the 'enabled' (rather than required) signing
level.

This means that we can actually set 'server signing = yes' and not
have the world fall apart.  We had a number of bugs in that code, and
it certainly looks like most of the testing was with the 'requried'
setting.

While the changes are reasonable, I'm putting this into HEAD rather
than 3.0 for the timebeing.  SMB signing, like NTLMSSP, tends to have
gotchas in it :-)

(I also need to give it a workout with more than smbclient before I
move it across).

Andrew Bartlett

diff --git a/source/include/smb.h b/source/include/smb.h
index 298944a0b1bcfc1f1774065b4af9c35336017068..60be41b12e94c7e3875b7a16193a9eec4aa9b054 100644 (file)
--- a/source/include/smb.h
+++ b/source/include/smb.h
@@ -1650,7 +1650,7 @@ struct ip_service {
 
 typedef struct smb_sign_info {
        void (*sign_outgoing_message)(char *outbuf, struct smb_sign_info *si);
-       BOOL (*check_incoming_message)(char *inbuf, struct smb_sign_info *si);
+       BOOL (*check_incoming_message)(char *inbuf, struct smb_sign_info *si, BOOL expected_ok);
        void (*free_signing_context)(struct smb_sign_info *si);
        void *signing_context;
 
@@ -1658,6 +1658,7 @@ typedef struct smb_sign_info {
        BOOL allow_smb_signing;
        BOOL doing_signing;
        BOOL mandatory_signing;
+       BOOL seen_valid; /* Have I ever seen a validly signed packet? */
 } smb_sign_info;
 
 #endif /* _SMB_H */

diff --git a/source/lib/util_sock.c b/source/lib/util_sock.c
index a275ddabb9af6406faa802d1fc95ef3e4155005e..15ce883b099baa59d4d6946d2cd7f444d754abd1 100644 (file)
--- a/source/lib/util_sock.c
+++ b/source/lib/util_sock.c
@@ -596,7 +596,7 @@ BOOL receive_smb(int fd,char *buffer, unsigned int timeout)
        }
 
        /* Check the incoming SMB signature. */
-       if (!srv_check_sign_mac(buffer)) {
+       if (!srv_check_sign_mac(buffer, True)) {
                DEBUG(0, ("receive_smb: SMB Signature verification failed on incoming packet!\n"));
                if (smb_read_error == 0)
                        smb_read_error = READ_BAD_SIG;

diff --git a/source/libsmb/cliconnect.c b/source/libsmb/cliconnect.c
index 84159e5d62a7af216b9d6132eee338712e336201..b5993cd1e66e42e6dd55a944d4a143ecff33004b 100644 (file)
--- a/source/libsmb/cliconnect.c
+++ b/source/libsmb/cliconnect.c
@@ -325,7 +325,7 @@ static BOOL cli_session_setup_nt1(struct cli_state *cli, const char *user,
                        session_key = data_blob(NULL, 16);
                        SMBsesskeygen_ntv1(nt_hash, NULL, session_key.data);
                }
-               cli_simple_set_signing(cli, session_key, nt_response, 0); 
+               cli_simple_set_signing(cli, session_key, nt_response); 
        } else {
                /* pre-encrypted password supplied.  Only used for 
                   security=server, can't do
@@ -521,7 +521,7 @@ static ADS_STATUS cli_session_setup_kerberos(struct cli_state *cli, const char *
        file_save("negTokenTarg.dat", negTokenTarg.data, negTokenTarg.length);
 #endif
 
-       cli_simple_set_signing(cli, session_key_krb5, null_blob, 0); 
+       cli_simple_set_signing(cli, session_key_krb5, null_blob); 
                        
        blob2 = cli_session_setup_blob(cli, negTokenTarg);
 
@@ -643,13 +643,16 @@ static NTSTATUS cli_session_setup_ntlmssp(struct cli_state *cli, const char *use
                fstrcpy(cli->server_domain, ntlmssp_state->server_domain);
                cli_set_session_key(cli, ntlmssp_state->session_key);
 
-               /* Using NTLMSSP session setup, signing on the net only starts
-                * after a successful authentication and the session key has
-                * been determined, but with a sequence number of 2. This
-                * assumes that NTLMSSP needs exactly 2 roundtrips, for any
-                * other SPNEGO mechanism it needs adapting. */
-
-               cli_simple_set_signing(cli, key, null_blob, 2);
+               if (cli_simple_set_signing(cli, key, null_blob)) {
+                       
+                       /* 'resign' the last message, so we get the right sequence numbers
+                          for checking the first reply from the server */
+                       cli_calculate_sign_mac(cli);
+                       
+                       if (!cli_check_sign_mac(cli, True)) {
+                               nt_status = NT_STATUS_ACCESS_DENIED;
+                       }
+               }
        }
 
        /* we have a reference conter on ntlmssp_state, if we are signing
@@ -1088,6 +1091,8 @@ BOOL cli_negprot(struct cli_state *cli)
                        }
                        cli->sign_info.negotiated_smb_signing = True;
                        cli->sign_info.mandatory_signing = True;
+               } else if (cli->sign_info.allow_smb_signing && cli->sec_mode & NEGOTIATE_SECURITY_SIGNATURES_ENABLED) {
+                       cli->sign_info.negotiated_smb_signing = True;
                }
 
        } else if (cli->protocol >= PROTOCOL_LANMAN1) {

diff --git a/source/libsmb/clientgen.c b/source/libsmb/clientgen.c
index 0873700fc0ad35e3803c8075d489ee56cffb6b13..20a048388199f46f9f9020ce78c8038aba9f54d6 100644 (file)
--- a/source/libsmb/clientgen.c
+++ b/source/libsmb/clientgen.c
@@ -117,7 +117,7 @@ BOOL cli_receive_smb(struct cli_state *cli)
                return ret;
        }
 
-       if (!cli_check_sign_mac(cli)) {
+       if (!cli_check_sign_mac(cli, True)) {
                DEBUG(0, ("SMB Signature verification failed on incoming packet!\n"));
                cli->smb_rw_error = READ_BAD_SIG;
                close(cli->fd);

diff --git a/source/libsmb/smb_signing.c b/source/libsmb/smb_signing.c
index 9010dbf5cb28e64c4e06f60884e41b74617e8392..28ff0e0c2e9887290818e5a63d23ace0db728581 100644 (file)
--- a/source/libsmb/smb_signing.c
+++ b/source/libsmb/smb_signing.c
@@ -150,7 +150,7 @@ static void null_sign_outgoing_message(char *outbuf, struct smb_sign_info *si)
  SMB signing - NULL implementation - check a MAC sent by server.
 ************************************************************/
 
-static BOOL null_check_incoming_message(char *inbuf, struct smb_sign_info *si)
+static BOOL null_check_incoming_message(char *inbuf, struct smb_sign_info *si, BOOL expected_ok)
 {
        return True;
 }
@@ -197,25 +197,39 @@ static void free_signing_context(struct smb_sign_info *si)
 }
 
 
-static BOOL signing_good(char *inbuf, struct smb_sign_info *si, BOOL good, uint32 seq) 
+static BOOL signing_good(char *inbuf, struct smb_sign_info *si, BOOL good, uint32 seq, BOOL expected_ok) 
 {
-       if (good && !si->doing_signing) {
-               si->doing_signing = True;
-       }
+       if (good) {
 
-       if (!good) {
-               if (si->doing_signing) {
-                       struct smb_basic_signing_context *data = si->signing_context;
+               if (!si->doing_signing) {
+                       si->doing_signing = True;
+               }
+               
+               if (!si->seen_valid) {
+                       si->seen_valid = True;
+               }
 
-                       /* W2K sends a bad first signature but the sign engine is on.... JRA. */
-                       if (data->send_seq_num > 1)
-                               DEBUG(1, ("signing_good: SMB signature check failed on seq %u!\n",
-                                                       (unsigned int)seq ));
+       } else {
+               if (!si->mandatory_signing && !si->seen_valid) {
 
-                       return False;
-               } else {
-                       DEBUG(3, ("signing_good: Peer did not sign reply correctly\n"));
+                       if (!expected_ok) {
+                               return True;
+                       }
+                       /* Non-mandatory signing - just turn off if this is the first bad packet.. */
+                       DEBUG(5, ("signing_good: signing negotiated but not required and the other side \
+isn't sending correct signatures. Turning signatures off.\n"));
+                       si->negotiated_smb_signing = False;
+                       si->allow_smb_signing = False;
+                       si->doing_signing = False;
                        free_signing_context(si);
+                       return True;
+               } else if (!expected_ok) {
+                       /* This packet is known to be unsigned */
+                       return True;
+               } else {
+                       /* Mandatory signing or bad packet after signing started - fail and disconnect. */
+                       if (seq)
+                               DEBUG(0, ("signing_good: BAD SIG: seq %u\n", (unsigned int)seq));
                        return False;
                }
        }
@@ -323,7 +337,7 @@ static void client_sign_outgoing_message(char *outbuf, struct smb_sign_info *si)
  SMB signing - Client implementation - check a MAC sent by server.
 ************************************************************/
 
-static BOOL client_check_incoming_message(char *inbuf, struct smb_sign_info *si)
+static BOOL client_check_incoming_message(char *inbuf, struct smb_sign_info *si, BOOL expected_ok)
 {
        BOOL good;
        uint32 reply_seq_number;
@@ -381,7 +395,7 @@ We were expecting seq %u\n", reply_seq_number, saved_seq ));
                DEBUG(10, ("client_check_incoming_message: seq %u: got good SMB signature of\n", (unsigned int)reply_seq_number));
                dump_data(10, (const char *)server_sent_mac, 8);
        }
-       return signing_good(inbuf, si, good, saved_seq);
+       return signing_good(inbuf, si, good, saved_seq, expected_ok);
 }
 
 /***********************************************************
@@ -415,7 +429,7 @@ static void simple_free_signing_context(struct smb_sign_info *si)
 
 BOOL cli_simple_set_signing(struct cli_state *cli,
                            const DATA_BLOB user_session_key,
-                           const DATA_BLOB response, int initial_send_seq_num)
+                           const DATA_BLOB response)
 {
        struct smb_basic_signing_context *data;
 
@@ -453,7 +467,7 @@ BOOL cli_simple_set_signing(struct cli_state *cli,
        dump_data_pw("MAC ssession key is:\n", data->mac_key.data, data->mac_key.length);
 
        /* Initialise the sequence number */
-       data->send_seq_num = initial_send_seq_num;
+       data->send_seq_num = 0;
 
        /* Initialise the list of outstanding packets */
        data->outstanding_packet_list = NULL;
@@ -535,7 +549,7 @@ static void temp_sign_outgoing_message(char *outbuf, struct smb_sign_info *si)
  SMB signing - TEMP implementation - check a MAC sent by server.
 ************************************************************/
 
-static BOOL temp_check_incoming_message(char *inbuf, struct smb_sign_info *si)
+static BOOL temp_check_incoming_message(char *inbuf, struct smb_sign_info *si, BOOL expected_ok)
 {
        return True;
 }
@@ -597,9 +611,9 @@ void cli_calculate_sign_mac(struct cli_state *cli)
  *         which had a bad checksum, True otherwise.
  */
  
-BOOL cli_check_sign_mac(struct cli_state *cli) 
+BOOL cli_check_sign_mac(struct cli_state *cli, BOOL expected_ok) 
 {
-       if (!cli->sign_info.check_incoming_message(cli->inbuf, &cli->sign_info)) {
+       if (!cli->sign_info.check_incoming_message(cli->inbuf, &cli->sign_info, expected_ok)) {
                free_signing_context(&cli->sign_info);  
                return False;
        }
@@ -688,7 +702,7 @@ static BOOL is_oplock_break(char *inbuf)
  SMB signing - Server implementation - check a MAC sent by server.
 ************************************************************/
 
-static BOOL srv_check_incoming_message(char *inbuf, struct smb_sign_info *si)
+static BOOL srv_check_incoming_message(char *inbuf, struct smb_sign_info *si, BOOL expected_ok)
 {
        BOOL good;
        struct smb_basic_signing_context *data = si->signing_context;
@@ -762,25 +776,7 @@ We were expecting seq %u\n", reply_seq_number, saved_seq ));
                dump_data(10, (const char *)server_sent_mac, 8);
        }
 
-       if (!signing_good(inbuf, si, good, saved_seq)) {
-               if (!si->mandatory_signing && (data->send_seq_num < 3)){
-                       /* Non-mandatory signing - just turn off if this is the first bad packet.. */
-                       DEBUG(5, ("srv_check_incoming_message: signing negotiated but not required and client \
-isn't sending correct signatures. Turning off.\n"));
-                       si->negotiated_smb_signing = False;
-                       si->allow_smb_signing = False;
-                       si->doing_signing = False;
-                       free_signing_context(si);
-                       return True;
-               } else {
-                       /* Mandatory signing or bad packet after signing started - fail and disconnect. */
-                       if (saved_seq)
-                               DEBUG(0, ("srv_check_incoming_message: BAD SIG: seq %u\n", (unsigned int)saved_seq));
-                       return False;
-               }
-       } else {
-               return True;
-       }
+       return (signing_good(inbuf, si, good, saved_seq, expected_ok));
 }
 
 /***********************************************************
@@ -813,13 +809,13 @@ BOOL srv_oplock_set_signing(BOOL onoff)
  Called to validate an incoming packet from the client.
 ************************************************************/
 
-BOOL srv_check_sign_mac(char *inbuf)
+BOOL srv_check_sign_mac(char *inbuf, BOOL expected_ok)
 {
        /* Check if it's a session keepalive. */
        if(CVAL(inbuf,0) == SMBkeepalive)
                return True;
 
-       return srv_sign_info.check_incoming_message(inbuf, &srv_sign_info);
+       return srv_sign_info.check_incoming_message(inbuf, &srv_sign_info, expected_ok);
 }
 
 /***********************************************************
@@ -907,6 +903,42 @@ BOOL srv_is_signing_active(void)
        return srv_sign_info.doing_signing;
 }
 
+
+/***********************************************************
+ Returns whether signing is negotiated. We can't use it unless it was
+ in the negprot.  
+************************************************************/
+
+BOOL srv_is_signing_negotiated(void)
+{
+       return srv_sign_info.negotiated_smb_signing;
+}
+
+/***********************************************************
+ Returns whether signing is negotiated. We can't use it unless it was
+ in the negprot.  
+************************************************************/
+
+BOOL srv_signing_started(void)
+{
+       struct smb_basic_signing_context *data;
+
+       if (!srv_sign_info.doing_signing) {
+               return False;
+       }
+
+       data = (struct smb_basic_signing_context *)srv_sign_info.signing_context;
+       if (!data)
+               return False;
+
+       if (data->send_seq_num == 0) {
+               return False;
+       }
+
+       return True;
+}
+
+
 /***********************************************************
  Tell server code we are in a multiple trans reply state.
 ************************************************************/

diff --git a/source/smbd/password.c b/source/smbd/password.c
index 8438f2a59365dcb3e74cad7b791dda7c62025198..ba57fecd51d8abdbc14e724899aade99b6bd0b4a 100644 (file)
--- a/source/smbd/password.c
+++ b/source/smbd/password.c
@@ -275,7 +275,7 @@ int register_vuid(auth_serversupplied_info *server_info, DATA_BLOB session_key,
                vuser->homes_snum = -1;
        }
        
-       if (lp_server_signing() && !vuser->guest && !srv_is_signing_active()) {
+       if (srv_is_signing_negotiated() && !vuser->guest && !srv_signing_started()) {
                /* Try and turn on server signing on the first non-guest sessionsetup. */
                srv_set_signing(vuser->session_key, response_blob);
        }

diff --git a/source/smbd/sesssetup.c b/source/smbd/sesssetup.c
index c42a35e809c38831a7bab4cc75cb89f2b8f74c10..c66ccfd8ebdeb6bb5d18629a4f02d034c36ea46a 100644 (file)
--- a/source/smbd/sesssetup.c
+++ b/source/smbd/sesssetup.c
@@ -287,14 +287,14 @@ static int reply_spnego_kerberos(connection_struct *conn,
                
                SSVAL(outbuf, smb_uid, sess_vuid);
 
-               if (!server_info->guest) {
+               if (!server_info->guest && !srv_signing_started()) {
                        /* We need to start the signing engine
                         * here but a W2K client sends the old
                         * "BSRSPYL " signature instead of the
                         * correct one. Subsequent packets will
                         * be correct.
                         */
-                       srv_check_sign_mac(inbuf);
+                       srv_check_sign_mac(inbuf, False);
                }
        }
 
@@ -360,14 +360,15 @@ static BOOL reply_spnego_ntlmssp(connection_struct *conn, char *inbuf, char *out
                        
                        SSVAL(outbuf,smb_uid,sess_vuid);
 
-                       if (!server_info->guest) {
+                       if (!server_info->guest && !srv_signing_started()) {
                                /* We need to start the signing engine
                                 * here but a W2K client sends the old
                                 * "BSRSPYL " signature instead of the
                                 * correct one. Subsequent packets will
                                 * be correct.
                                 */
-                               srv_check_sign_mac(inbuf);
+
+                               srv_check_sign_mac(inbuf, False);
                        }
                }
        }
@@ -907,7 +908,7 @@ int reply_sesssetup_and_X(connection_struct *conn, char *inbuf,char *outbuf,
                return ERROR_NT(NT_STATUS_LOGON_FAILURE);
        }
 
-       if (!server_info->guest && !srv_check_sign_mac(inbuf)) {
+       if (!server_info->guest && !srv_signing_started() && !srv_check_sign_mac(inbuf, True)) {
                exit_server("reply_sesssetup_and_X: bad smb signature");
        }