[Snort-devel] ssh preprocessor does not whitelist ssh connections

Florian Westphal florian.westphal at ...3285...
Wed Aug 21 12:11:46 EDT 2013


Seems like the ssh preprocessor has problems decoding all the
ssh key exchange messages (snort 2.9.5.3) - the flow does not
enter the "encrypted" stage.

I've put a reproducer pcap here: http://strlen.de/fw/ssh3.pcap
(the invalid checksums are due to tcp offloading, snort runs with
 checksumming verification disabled).

I have following stream5 settings:

preprocessor stream5_tcp: policy windows, detect_anomalies, require_3whs 180, \
   overlap_limit 10, small_segments 3 bytes 150, timeout 180, \
   ports client 21
   ports both 80

Adding 22 to "client" or "both" does not change the result.

I've tried to debug this, and the main problem seems to be that
without "both 22" it fails to see the server key exchange init
because this packet is too big and will be part of two tcp packets.

With "both 22", it fails to detect the client key exchange init since
that message will follow the client ssh banner in one single packet.

Also, it looks like there are a couple of other problems.
The ssh preprocessor uses STREAM_FLPOLICY_SET_APPEND, but that is ignored
silently by stream5.

Fixing this caused followup bug, looks like _dpd.streamAPI->missed_packets()
is always be true when reasm is enabled mid-stream.

I hacked up a patch that fixes _this_particular_scenario_, perhaps
it helps.  Main point of this exercise is to get snort to issue a
Whitelist verdict - I am working on a nfqueue-based DAQ that can use
this for setting "don't queue this flow anymore" information in the kernel.

If you need more information please let me know.

Cheers,
---
 src/dynamic-preprocessors/ssh/spp_ssh.c |  167 +++++++++++++++++++------------
 src/dynamic-preprocessors/ssh/spp_ssh.h |    1 +
 2 files changed, 103 insertions(+), 65 deletions(-)

diff --git a/src/dynamic-preprocessors/ssh/spp_ssh.c b/src/dynamic-preprocessors/ssh/spp_ssh.c
index 01d8a3b..31518b4 100644
--- a/src/dynamic-preprocessors/ssh/spp_ssh.c
+++ b/src/dynamic-preprocessors/ssh/spp_ssh.c
@@ -93,10 +93,10 @@ static void FreeSSHData( void* );
 static void  ParseSSHArgs(SSHConfig *, u_char*);
 static void ProcessSSH( void*, void* );
 static inline int CheckSSHPort( uint16_t );
-static int ProcessSSHProtocolVersionExchange( SSHData*, SFSnortPacket*,
+static unsigned int ProcessSSHProtocolVersionExchange( SSHData*, SFSnortPacket*,
 		uint8_t, uint8_t );
-static int ProcessSSHKeyExchange( SSHData*, SFSnortPacket*, uint8_t );
-static int ProcessSSHKeyInitExchange( SSHData*, SFSnortPacket*, uint8_t );
+static void ProcessSSHKeyExchange( SSHData*, SFSnortPacket*, uint8_t );
+static int ProcessSSHKeyInitExchange( SSHData*, SFSnortPacket*, uint8_t, unsigned int);
 static void _addPortsToStream5Filter(struct _SnortConfig *, SSHConfig *, tSfPolicyId);
 #ifdef TARGET_BASED
 static void _addServicesToStream5Filter(struct _SnortConfig *, tSfPolicyId);
@@ -499,6 +499,37 @@ DisplaySSHConfig(SSHConfig *config)
 	_dpd.logMsg("\n");
 }
 
+/* Returns the true length of the ssh packet, including
+ * the ssh packet header and all padding.
+ *
+ * If the packet length is invalid, 0 is returned.
+ * The return value is never larger than buflen.
+ *
+ * PARAMETERS:
+ * p: Pointer to the SSH packet.
+ * buflen: the size of packet buffer.
+ */
+static unsigned int SSHPacket_GetLength(SSH2Packet *p, size_t buflen)
+{
+	unsigned int ssh_length;
+
+	if (buflen < sizeof(*p))
+		return 0;
+
+	ssh_length = ntohl(p->packet_length);
+
+	if (ssh_length < sizeof(*p) + 1 || ssh_length > SSH2_PACKET_MAX_SIZE)
+		return 0;
+	ssh_length += sizeof(p->packet_length); /* not included in p->packet_length */
+
+	if (buflen < ssh_length)
+		return buflen; /* truncated */
+	/* buflen > ssh_length is ok.
+	 * We might have another ssh packet following this one in reassembled packet buffers
+	 */
+	return ssh_length;
+}
+
 /* Main runtime entry point for SSH preprocessor.
  * Analyzes SSH packets for anomalies/exploits.
  *
@@ -534,7 +565,7 @@ ProcessSSH( void* ipacketp, void* contextp )
     /* Make sure this preprocessor should run. */
     /* check if we're waiting on stream reassembly */
     if ( packetp->flags & FLAG_STREAM_INSERT )
-        return;
+	 return;
 
     PREPROC_PROFILE_START(sshPerfStats);
 
@@ -617,11 +648,10 @@ ProcessSSH( void* ipacketp, void* contextp )
     if (sessp->state_flags & SSH_FLG_MISSED_PACKETS)
         return;
 
-    /* If we picked up mid-stream or missed any packets (midstream pick up
-     * means we've already missed packets) set missed packets flag and make
-     * sure we don't do any more reassembly on this session */
-    if ((_dpd.streamAPI->get_session_flags(packetp->stream_session_ptr) & SSNFLAG_MIDSTREAM)
-            || _dpd.streamAPI->missed_packets(packetp->stream_session_ptr, SSN_DIR_BOTH))
+    /* If we picked up mid-stream (means we've already missed packets) set
+     * missed packets flag and make sure we don't do any more reassembly on
+     * this session */
+    if (_dpd.streamAPI->get_session_flags(packetp->stream_session_ptr) & SSNFLAG_MIDSTREAM)
     {
         /* Don't turn off reassembly if autodetected since another preprocessor
          * may actually be looking at this session as well and the SSH
@@ -634,7 +664,6 @@ ProcessSSH( void* ipacketp, void* contextp )
         }
 
         sessp->state_flags |= SSH_FLG_MISSED_PACKETS;
-
         return;
     }
 
@@ -642,7 +671,7 @@ ProcessSSH( void* ipacketp, void* contextp )
     if ( !(sessp->state_flags & SSH_FLG_REASSEMBLY_SET ))
     {
         _dpd.streamAPI->set_reassembly(packetp->stream_session_ptr,
-                        STREAM_FLPOLICY_FOOTPRINT, SSN_DIR_BOTH, STREAM_FLPOLICY_SET_APPEND);
+                        STREAM_FLPOLICY_FOOTPRINT, SSN_DIR_BOTH, STREAM_FLPOLICY_SET_ABSOLUTE);
         sessp->state_flags |= SSH_FLG_REASSEMBLY_SET;
     }
 
@@ -652,21 +681,23 @@ ProcessSSH( void* ipacketp, void* contextp )
 
 	if ( !(sessp->state_flags & SSH_FLG_SESS_ENCRYPTED ))
 	{
+		unsigned int offset = 0;
 		/* If server and client have not performed the protocol
 		 * version exchange yet, must look for version strings.
 	 	 */
 		if ( (sessp->state_flags & SSH_FLG_BOTH_IDSTRING_SEEN)
 			!= SSH_FLG_BOTH_IDSTRING_SEEN )
 		{
-			if ( ProcessSSHProtocolVersionExchange( sessp,
-					packetp, direction, known_port ) ==
-				SSH_FAILURE )
+			offset = ProcessSSHProtocolVersionExchange( sessp, packetp, direction, known_port);
+			if (offset == 0)
 			{
 				/*Error processing protovers exchange msg */
+				PREPROC_PROFILE_END(sshPerfStats);
+				return;
 			}
-
-            PREPROC_PROFILE_END(sshPerfStats);
-			return;
+			/* found protocol version.  Stream reassembly might have appended an ssh packet,
+			 * such as the key exchange init.  Thus call ProcessSSHKeyInitExchange() too.
+			 */
 		}
 
 		/* Expecting to see the key init exchange at this point
@@ -677,10 +708,10 @@ ProcessSSH( void* ipacketp, void* contextp )
 		     ((sessp->state_flags & SSH_FLG_V2_KEXINIT_DONE )
 			!= SSH_FLG_V2_KEXINIT_DONE ))
 		{
-		    ProcessSSHKeyInitExchange( sessp, packetp, direction );
+			ProcessSSHKeyInitExchange( sessp, packetp, direction, offset);
 
             PREPROC_PROFILE_END(sshPerfStats);
-			return;
+            return;
 		}
 
 		/* If SSH2, need to process the actual key exchange msgs.
@@ -759,7 +790,6 @@ ProcessSSH( void* ipacketp, void* contextp )
 				packetp->stream_session_ptr,
 				packetp,
 				SSN_DIR_BOTH, -1, 0 );
-
 		}
 
 	}
@@ -919,15 +949,16 @@ static inline int SSHCheckStrlen(char *str, int max) {
  * direction: 	Which direction the packet is going.
  * known_port:  A pre-configured or default server port is involved.
  *
- * RETURNS:	SSH_SUCCESS, if successfully processed a proto exch msg
- *		SSH_FAILURE, otherwise.
+ * RETURNS:	returns length of protocol version string
+ *		0 if no version exchange was found.
  */
-static int
+static unsigned int
 ProcessSSHProtocolVersionExchange( SSHData* sessionp, SFSnortPacket* packetp,
 	uint8_t direction, uint8_t known_port )
 {
 	char* version_stringp = (char*) packetp->payload;
 	uint8_t version;
+	char *version_end;
 
 	/* Get the version. */
 	if ( packetp->payload_size >= 6 &&
@@ -977,7 +1008,7 @@ ProcessSSHProtocolVersionExchange( SSHData* sessionp, SFSnortPacket* packetp,
             ALERT(SSH_EVENT_PROTOMISMATCH, SSH_EVENT_PROTOMISMATCH_STR);
 		}
 
-		return SSH_FAILURE;
+		return 0;
 	}
 
 	/* Saw a valid protocol exchange message. Mark the session
@@ -994,8 +1025,12 @@ ProcessSSHProtocolVersionExchange( SSHData* sessionp, SFSnortPacket* packetp,
 	}
 
 	sessionp->version = version;
+	version_end = memchr(version_stringp, '\n', packetp->payload_size);
+	if (version_end)
+		return (version_end - version_stringp) + 1;
 
-	return SSH_SUCCESS;
+	/* incomplete version string, should end with \n or \r\n for sshv2 */
+	return 6;
 }
 
 /* Called to process SSH1 key exchange or SSH2 key exchange init
@@ -1007,15 +1042,24 @@ ProcessSSHProtocolVersionExchange( SSHData* sessionp, SFSnortPacket* packetp,
  * sessionp:    Pointer to SSH data for packet's session.
  * packetp:	Pointer to the packet to inspect.
  * direction: 	Which direction the packet is going.
+ * offset: 	Offset in payload buffer where we expect the first SSH2Packet
  *
  * RETURN:	SSH_SUCCESS, if a valid key exchange message is processed
  *		SSH_FAILURE, otherwise.
  */
 static int
 ProcessSSHKeyInitExchange( SSHData* sessionp, SFSnortPacket* packetp,
-	uint8_t direction )
+	uint8_t direction, unsigned int offset)
 {
 	SSH2Packet* ssh2packetp = NULL;
+	unsigned int payload_size = packetp->payload_size;
+	const char *payload = packetp->payload;
+
+	if (payload_size < sizeof(*ssh2packetp) || (payload_size - sizeof(*ssh2packetp)) < offset)
+		return SSH_FAILURE;
+
+	payload_size -= offset;
+	payload += offset;
 
 	if ( sessionp->version == SSH_VERSION_1 )
 	{
@@ -1023,30 +1067,16 @@ ProcessSSHKeyInitExchange( SSHData* sessionp, SFSnortPacket* packetp,
 		uint8_t padding_length;
 		uint8_t message_type;
 
-	    /*
-         * Validate packet payload.
-         * First 4 bytes should have the SSH packet length,
-         * minus any padding.
-         */
-		if ( packetp->payload_size < 4 )
-        {
-            if(ssh_eval_config->EnabledAlerts & SSH_ALERT_PAYSIZE)
-            {
-                ALERT(SSH_EVENT_PAYLOAD_SIZE, SSH_PAYLOAD_SIZE_STR);
-            }
-
-			return SSH_FAILURE;
-        }
-
 		/*
 		 * SSH1 key exchange is very simple and
  		 * consists of only two messages, a server
 		 * key and a client key message.`
 		 */
-		length = ntohl( *((uint32_t*) packetp->payload) );
+		memcpy(&length, payload, sizeof(length));
+		length = ntohl(length);
 
 	    /* Packet payload should be larger than length, due to padding. */
-		if ( packetp->payload_size < length )
+		if (payload_size < length )
 		{
             if(ssh_eval_config->EnabledAlerts & SSH_ALERT_PAYSIZE)
             {
@@ -1062,9 +1092,9 @@ ProcessSSHKeyInitExchange( SSHData* sessionp, SFSnortPacket* packetp,
          * With the padding calculated, verify payload is sufficiently large
          * to include the message type.
          */
-        if ( packetp->payload_size < padding_length + 4 + 1)
+        if ( payload_size < padding_length + 4 + 1 + offset)
         {
-            if(ssh_eval_config->EnabledAlerts & SSH_ALERT_PAYSIZE)
+            if(offset == 0 && ssh_eval_config->EnabledAlerts & SSH_ALERT_PAYSIZE)
             {
                 ALERT(SSH_EVENT_PAYLOAD_SIZE, SSH_PAYLOAD_SIZE_STR);
             }
@@ -1073,7 +1103,7 @@ ProcessSSHKeyInitExchange( SSHData* sessionp, SFSnortPacket* packetp,
         }
 
 		message_type =
-		     *( (uint8_t*) (packetp->payload + padding_length + 4));
+		     *( (uint8_t*) (payload + padding_length + 4));
 
 		switch( message_type )
 		{
@@ -1124,22 +1154,20 @@ ProcessSSHKeyInitExchange( SSHData* sessionp, SFSnortPacket* packetp,
          * This may legitimately occur such as in the case of a
          * retransmission.
          */
-        if ( packetp->payload_size < sizeof(SSH2Packet) )
+        if ( payload_size < sizeof(SSH2Packet) )
         {
 			return SSH_FAILURE;
         }
 
 		/* Overlay the SSH2 binary data packet struct on the packet */
-		ssh2packetp = (SSH2Packet*) packetp->payload;
-		if (( packetp->payload_size < SSH2_HEADERLEN + 1) ||
-			( packetp->payload_size < ntohl(ssh2packetp->packet_length) ))
+		ssh2packetp = (SSH2Packet*) payload;
+		if ( payload_size < SSH2_HEADERLEN + 1)
 		{
 			/* Invalid packet length. */
-
 			return SSH_FAILURE;
 		}
 
-		switch ( packetp->payload[SSH2_HEADERLEN] )
+		switch ( payload[SSH2_HEADERLEN] )
 		{
 			case SSH_MSG_KEXINIT:
 				sessionp->state_flags |=
@@ -1175,38 +1203,40 @@ ProcessSSHKeyInitExchange( SSHData* sessionp, SFSnortPacket* packetp,
  * sessionp:    Pointer to SSH data for packet's session.
  * packetp:	Pointer to the packet to inspect.
  * direction: 	Which direction the packet is going.
- *
- * RETURN:	SSH_SUCCESS, if a valid key exchange message is processed
- *		SSH_FAILURE, otherwise.
  */
-static int
+static void
 ProcessSSHKeyExchange( SSHData* sessionp, SFSnortPacket* packetp,
 	uint8_t direction )
 {
 	SSH2Packet* ssh2packetp = NULL;
+	unsigned int offset = 0;
+	unsigned int payload_size = packetp->payload_size;
+	unsigned int ssh_length;
 
-    if ( packetp->payload_size < sizeof(SSH2Packet) )
+    if ( payload_size < sizeof(SSH2Packet) )
     {
 		/* Invalid packet length. */
-		return SSH_FAILURE;
+		return;
     }
 
-	ssh2packetp = (SSH2Packet*) packetp->payload;
+ next_ssh_packet:
+	ssh2packetp = (SSH2Packet*) (packetp->payload + offset);
+	ssh_length = SSHPacket_GetLength(ssh2packetp, payload_size);
 
-	if (( packetp->payload_size < SSH2_HEADERLEN + 1 ) ||
-		( packetp->payload_size < ntohl(ssh2packetp->packet_length) ))
+
+	if (ssh_length == 0)
 	{
 
+
         if(ssh_eval_config->EnabledAlerts & SSH_ALERT_PAYSIZE)
         {
 		    /* Invalid packet length. */
             ALERT(SSH_EVENT_PAYLOAD_SIZE, SSH_PAYLOAD_SIZE_STR);
         }
-
-		return SSH_FAILURE;
+		return;
 	}
 
-	switch( packetp->payload[SSH2_HEADERLEN] )
+	switch(packetp->payload[offset + SSH2_HEADERLEN] )
 	{
 		case SSH_MSG_KEXDH_INIT:
 			if ( direction == SSH_DIR_FROM_CLIENT )
@@ -1306,9 +1336,16 @@ ProcessSSHKeyExchange( SSHData* sessionp, SFSnortPacket* packetp,
 	{
 		sessionp->state_flags |=
 			SSH_FLG_SESS_ENCRYPTED;
+
+		return;
 	}
 
-	return SSH_SUCCESS;
+	if (ssh_length < payload_size && ssh_length >= 4)
+	{
+		offset += ssh_length;
+		payload_size -= ssh_length;
+		goto next_ssh_packet;
+	}
 }
 
 static void _addPortsToStream5Filter(struct _SnortConfig *sc, SSHConfig *config, tSfPolicyId policy_id)
diff --git a/src/dynamic-preprocessors/ssh/spp_ssh.h b/src/dynamic-preprocessors/ssh/spp_ssh.h
index 6b31b4c..a88d031 100644
--- a/src/dynamic-preprocessors/ssh/spp_ssh.h
+++ b/src/dynamic-preprocessors/ssh/spp_ssh.h
@@ -191,6 +191,7 @@ typedef struct _sshData
  * Length of SSH2 header, in bytes.
  */
 #define SSH2_HEADERLEN		(5)
+#define SSH2_PACKET_MAX_SIZE    (256 * 1024)
 
 /*
  * SSH2 binary packet struct.
-- 
1.7.8.6





More information about the Snort-devel mailing list