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)

7 files changed, 101 insertions, 62 deletions

diff --git a/source3/include/smb.h b/source3/include/smb.h
index add57ddcf4..32fa1b5765 100644
--- 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 19fb41f6ca..845aaa4b13 100644
--- 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 3f87119ce2..63541e18b5 100644
--- 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 8542eea064..66edc3ce38 100644
--- 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 9010dbf5cb..c71543959d 100644
--- 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 9449113ddc..ef5d0a97ac 100644
--- 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 244db6d2c1..b8777be697 100644
--- 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");
 	}