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

7 files changed, 101 insertions, 62 deletions

diff --git a/source/include/smb.h b/source/include/smb.h
index add57ddcf47..32fa1b57659 100644
--- a/source/include/smb.h
+++ b/source/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/source/lib/util_sock.c b/source/lib/util_sock.c
index 19fb41f6ca3..845aaa4b13a 100644
--- 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 3f87119ce26..63541e18b5d 100644
--- 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->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 8542eea064d..66edc3ce38b 100644
--- 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 9010dbf5cb2..c71543959dc 100644
--- 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 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/source/smbd/password.c b/source/smbd/password.c
index 9449113ddc5..ef5d0a97ac7 100644
--- a/source/smbd/password.c
+++ b/source/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/source/smbd/sesssetup.c b/source/smbd/sesssetup.c
index 244db6d2c14..b8777be6971 100644
--- a/source/smbd/sesssetup.c
+++ b/source/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");
 	}