Snort mailing list archives
Re: ssh preprocessor does not whitelist ssh connections
From: Bhagya Bantwal <bbantwal () sourcefire com>
Date: Tue, 10 Sep 2013 10:36:38 -0400
Florian, Thanks for reporting this issue. Comments inline. -B On Wed, Aug 21, 2013 at 12:11 PM, Florian Westphal < florian.westphal () sophos com> wrote:
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.
Yep. This should have been ABSOLUTE.
Fixing this caused followup bug, looks like _dpd.streamAPI->missed_packets() is always be true when reasm is enabled mid-stream.
-- Yes. This is a bug we are looking into.
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
------------------------------------------------------------------------------
Introducing Performance Central, a new site from SourceForge and
AppDynamics. Performance Central is your source for news, insights,
analysis and resources for efficient Application Performance Management.
Visit us today!
http://pubads.g.doubleclick.net/gampad/clk?id=48897511&iu=/4140/ostg.clktrk
_______________________________________________
Snort-devel mailing list
Snort-devel () lists sourceforge net
https://lists.sourceforge.net/lists/listinfo/snort-devel
Archive:
http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel
Please visit http://blog.snort.org for the latest news about Snort!
------------------------------------------------------------------------------ How ServiceNow helps IT people transform IT departments: 1. Consolidate legacy IT systems to a single system of record for IT 2. Standardize and globalize service processes across IT 3. Implement zero-touch automation to replace manual, redundant tasks http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk
_______________________________________________ Snort-devel mailing list Snort-devel () lists sourceforge net https://lists.sourceforge.net/lists/listinfo/snort-devel Archive: http://sourceforge.net/mailarchive/forum.php?forum_name=snort-devel Please visit http://blog.snort.org for the latest news about Snort!
Current thread:
- ssh preprocessor does not whitelist ssh connections Florian Westphal (Aug 21)
- Re: ssh preprocessor does not whitelist ssh connections Bhagya Bantwal (Sep 10)