Merge from HEAD the SMB signing patch that I developed a couple of weeks

ago.

This patch re-adds support for 'optional' SMB signing.  It also ensures that
we are much more careful about when we enable signing, particularly with
on-the-fly smb.conf reloads.

The client code will now attempt to use smb signing by default, and disable
it if the server doesn't correctly support it.

Andrew Bartlett
(This used to be commit e27b5cbe75d89ec839dafd52dd33101885a4c263)

diff --git a/source3/include/smb.h b/source3/include/smb.h
index add57ddcf47e02ba382423e6b5c76a8db7b6a0ce..32fa1b57659344a13aadbb233f813f8340ee7e4f 100644 (file)
--- a/source3/include/smb.h
+++ b/source3/include/smb.h
@@ -1649,7 +1649,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 must_be_ok);
        void (*free_signing_context)(struct smb_sign_info *si);
        void *signing_context;
 
@@ -1657,6 +1657,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;
 
 struct ea_struct {

diff --git a/source3/lib/util_sock.c b/source3/lib/util_sock.c
index 19fb41f6ca305b11f584c9c05410d0cd11dab18d..845aaa4b13acc506aea57fb58d82da3ee48f7bdd 100644 (file)
--- a/source3/lib/util_sock.c
+++ b/source3/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/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index 3f87119ce26693571752a7560558e5ba6443bcb6..63541e18b5da913ab4f36960ba0b4ec42589af22 100644 (file)
--- a/source3/libsmb/cliconnect.c
+++ b/source3/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->sec_mode & NEGOTIATE_SECURITY_SIGNATURES_ENABLED) {
+                       cli->sign_info.negotiated_smb_signing = True;
                }
 
        } else if (cli->protocol >= PROTOCOL_LANMAN1) {

diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c
index 8542eea064df77e538405199df8d9f903f7bd643..66edc3ce38bbc81d34a3e412b1da8e8c4a22e94c 100644 (file)
--- a/source3/libsmb/clientgen.c
+++ b/source3/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/source3/libsmb/smb_signing.c b/source3/libsmb/smb_signing.c
index 9010dbf5cb28e64c4e06f60884e41b74617e8392..c71543959dc296e2bc76c3f59baff2e36c0b28d6 100644 (file)
--- a/source3/libsmb/smb_signing.c
+++ b/source3/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 must_be_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 must_be_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 (!must_be_ok) {
+                               return True;
+                       }
+                       /* 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 if (!must_be_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 must_be_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, must_be_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 foo)
 {
        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 must_be_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, must_be_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 must_be_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, must_be_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 must_be_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, must_be_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/source3/smbd/password.c b/source3/smbd/password.c
index 9449113ddc550d4060fe37c68b2ed915b37cf80c..ef5d0a97ac7df540caf749a0d3cb69508e1880c4 100644 (file)
--- a/source3/smbd/password.c
+++ b/source3/smbd/password.c
@@ -269,7 +269,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/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c
index 244db6d2c140739a2868b23bc1671ae0c0caad86..b8777be6971c44d175ed455798d980e945ae4b82 100644 (file)
--- a/source3/smbd/sesssetup.c
+++ b/source3/smbd/sesssetup.c
@@ -294,14 +294,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);
                }
        }
 
@@ -370,14 +370,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);
                        }
                }
        }
@@ -920,7 +921,7 @@ int reply_sesssetup_and_X(connection_struct *conn, char *inbuf,char *outbuf,
        /* current_user_info is changed on new vuid */
        reload_services( True );
 
-       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");
        }