Author: abartlet
Mofified: source/include/smb.h source/lib/util_sock.c source/libsmb/cliconnect.c source/libsmb/clientgen.c source/libsmb/smb_signing.c source/smbd/password.c source/smbd/sesssetup.c
Added:   
Removed:   


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


Index: source/libsmb/smb_signing.c
===================================================================
RCS file: /cvsroot/samba/source/libsmb/smb_signing.c,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -r1.20 -r1.21
--- source/libsmb/smb_signing.c	17 Jan 2004 00:30:28 -0000	1.20
+++ source/libsmb/smb_signing.c	9 Mar 2004 12:37:05 -0000	1.21
@@ -150,7 +150,7 @@
  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 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 @@
  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 @@
 		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 @@
 
 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 @@
 	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 @@
  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 @@
  *         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 @@
  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 @@
 		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 @@
  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);
 }
 
 /***********************************************************
@@ -906,6 +902,42 @@
 {
 	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.

Index: source/lib/util_sock.c
===================================================================
RCS file: /cvsroot/samba/source/lib/util_sock.c,v
retrieving revision 1.89
retrieving revision 1.90
diff -u -r1.89 -r1.90
--- source/lib/util_sock.c	20 Feb 2004 22:42:39 -0000	1.89
+++ source/lib/util_sock.c	9 Mar 2004 12:37:05 -0000	1.90
@@ -596,7 +596,7 @@
 	}
 
 	/* 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;

Index: source/smbd/sesssetup.c
===================================================================
RCS file: /cvsroot/samba/source/smbd/sesssetup.c,v
retrieving revision 1.118
retrieving revision 1.119
diff -u -r1.118 -r1.119
--- source/smbd/sesssetup.c	26 Jan 2004 02:22:49 -0000	1.118
+++ source/smbd/sesssetup.c	9 Mar 2004 12:37:05 -0000	1.119
@@ -287,14 +287,14 @@
 		
 		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 @@
 			
 			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 @@
 		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");
 	}
 

Index: source/include/smb.h
===================================================================
RCS file: /cvsroot/samba/source/include/smb.h,v
retrieving revision 1.504
retrieving revision 1.505
diff -u -r1.504 -r1.505
--- source/include/smb.h	3 Mar 2004 20:55:40 -0000	1.504
+++ source/include/smb.h	9 Mar 2004 12:37:05 -0000	1.505
@@ -1650,7 +1650,7 @@
 
 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 @@
 	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 */

Index: source/libsmb/clientgen.c
===================================================================
RCS file: /cvsroot/samba/source/libsmb/clientgen.c,v
retrieving revision 1.226
retrieving revision 1.227
diff -u -r1.226 -r1.227
--- source/libsmb/clientgen.c	22 Nov 2003 13:29:01 -0000	1.226
+++ source/libsmb/clientgen.c	9 Mar 2004 12:37:05 -0000	1.227
@@ -117,7 +117,7 @@
 		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);

Index: source/smbd/password.c
===================================================================
RCS file: /cvsroot/samba/source/smbd/password.c,v
retrieving revision 1.278
retrieving revision 1.279
diff -u -r1.278 -r1.279
--- source/smbd/password.c	1 Mar 2004 16:10:28 -0000	1.278
+++ source/smbd/password.c	9 Mar 2004 12:37:05 -0000	1.279
@@ -275,7 +275,7 @@
 		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);
 	}

Index: source/libsmb/cliconnect.c
===================================================================
RCS file: /cvsroot/samba/source/libsmb/cliconnect.c,v
retrieving revision 1.147
retrieving revision 1.148
diff -u -r1.147 -r1.148
--- source/libsmb/cliconnect.c	15 Jan 2004 19:08:45 -0000	1.147
+++ source/libsmb/cliconnect.c	9 Mar 2004 12:37:05 -0000	1.148
@@ -325,7 +325,7 @@
 			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 @@
 	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 @@
 		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 @@
 			}
 			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) {