diff --git a/CHANGES b/CHANGES index 4fc7d64493..714c2de12f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,12 @@ +3.1.0-dev.118 | 2019-09-17 17:21:58 +0000 + + * GH-566: Fix cases where ssh_encrypted_packet event wasn't raised. + When encrypted data was bundled within the same segment as the + NewKeys message, it wasn't not reported via a + ssh_encrypted_package event as it should have been. (Jon Siwek, + Corelight) + 3.1.0-dev.116 | 2019-09-17 10:08:38 -0700 * Switch from header guards to pragma once (Dominik Charousset, Corelight) diff --git a/VERSION b/VERSION index d004815fb2..00a1250b53 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.0-dev.116 +3.1.0-dev.118 diff --git a/src/analyzer/protocol/ssh/SSH.cc b/src/analyzer/protocol/ssh/SSH.cc index be4a8e6e2c..6f468fe441 100644 --- a/src/analyzer/protocol/ssh/SSH.cc +++ b/src/analyzer/protocol/ssh/SSH.cc @@ -18,6 +18,7 @@ SSH_Analyzer::SSH_Analyzer(Connection* c) had_gap = false; auth_decision_made = false; skipped_banner = false; + saw_encrypted_client_data = false; service_accept_size = 0; userauth_failure_size = 0; } @@ -56,16 +57,12 @@ void SSH_Analyzer::DeliverStream(int len, const u_char* data, bool orig) if ( interp->get_state(orig) == binpac::SSH::ENCRYPTED ) { - if ( ssh_encrypted_packet ) - BifEvent::generate_ssh_encrypted_packet(interp->bro_analyzer(), interp->bro_analyzer()->Conn(), - orig, len); - - if ( ! auth_decision_made ) - ProcessEncrypted(len, orig); - + ProcessEncryptedSegment(len, orig); return; } + interp->clear_encrypted_byte_count_in_current_segment(); + try { interp->NewData(orig, data, data + len); @@ -74,6 +71,14 @@ void SSH_Analyzer::DeliverStream(int len, const u_char* data, bool orig) { ProtocolViolation(fmt("Binpac exception: %s", e.c_msg())); } + + auto encrypted_len = interp->get_encrypted_bytes_in_current_segment(); + + if ( encrypted_len > 0 ) + // We must have transitioned into the encrypted state during this + // delivery, but also had some portion of the segment be comprised + // of encrypted data, so process the encrypted segment length. + ProcessEncryptedSegment(encrypted_len, orig); } void SSH_Analyzer::Undelivered(uint64_t seq, int len, bool orig) @@ -83,11 +88,31 @@ void SSH_Analyzer::Undelivered(uint64_t seq, int len, bool orig) interp->NewGap(orig, len); } +void SSH_Analyzer::ProcessEncryptedSegment(int len, bool orig) + { + if ( ssh_encrypted_packet ) + BifEvent::generate_ssh_encrypted_packet(interp->bro_analyzer(), + interp->bro_analyzer()->Conn(), + orig, len); + + if ( ! auth_decision_made ) + ProcessEncrypted(len, orig); + } + void SSH_Analyzer::ProcessEncrypted(int len, bool orig) { - // We're interested in messages from the server for SSH2 - if ( ! orig && (interp->get_version() == binpac::SSH::SSH2) ) + if ( interp->get_version() != binpac::SSH::SSH2 ) + return; + + if ( orig ) + saw_encrypted_client_data = true; + else { + // If the client hasn't sent any encrypted data yet, but the + // server is, just ignore it until seeing encrypted client data. + if ( ! saw_encrypted_client_data ) + return; + // The first thing we see and want to know is the length of // SSH_MSG_SERVICE_REQUEST, which has a fixed (decrypted) size // of 24 bytes (17 for content pad-aligned to 8-byte diff --git a/src/analyzer/protocol/ssh/SSH.h b/src/analyzer/protocol/ssh/SSH.h index 15abe4012f..71328b7dcf 100644 --- a/src/analyzer/protocol/ssh/SSH.h +++ b/src/analyzer/protocol/ssh/SSH.h @@ -30,12 +30,14 @@ namespace analyzer { binpac::SSH::SSH_Conn* interp; void ProcessEncrypted(int len, bool orig); + void ProcessEncryptedSegment(int len, bool orig); bool had_gap; // Packet analysis stuff bool auth_decision_made; bool skipped_banner; + bool saw_encrypted_client_data; int service_accept_size; int userauth_failure_size; diff --git a/src/analyzer/protocol/ssh/ssh-protocol.pac b/src/analyzer/protocol/ssh/ssh-protocol.pac index b0caebc740..b35fa9b655 100644 --- a/src/analyzer/protocol/ssh/ssh-protocol.pac +++ b/src/analyzer/protocol/ssh/ssh-protocol.pac @@ -9,9 +9,15 @@ # - Encrypted messages have no usable data, so we'll just ignore them as best we can. # - Finally, key exchange messages have a common format. +type EncryptedByte(is_orig: bool) = record { + encrypted : bytestring &length=1 &transient; +} &let { +proc: bool = $context.connection.inc_encrypted_byte_count_in_current_segment(); +}; + type SSH_PDU(is_orig: bool) = case $context.connection.get_state(is_orig) of { VERSION_EXCHANGE -> version : SSH_Version(is_orig); - ENCRYPTED -> encrypted : bytestring &length=1 &transient; + ENCRYPTED -> encrypted : EncryptedByte(is_orig); default -> kex : SSH_Key_Exchange(is_orig); } &byteorder=bigendian; @@ -265,6 +271,7 @@ refine connection SSH_Conn += { int state_up_; int state_down_; int version_; + int encrypted_bytes_in_current_segment_; bool kex_orig_; bool kex_seen_; @@ -276,6 +283,7 @@ refine connection SSH_Conn += { state_up_ = VERSION_EXCHANGE; state_down_ = VERSION_EXCHANGE; version_ = UNK; + encrypted_bytes_in_current_segment_ = 0; kex_seen_ = false; kex_orig_ = false; @@ -288,6 +296,23 @@ refine connection SSH_Conn += { kex_algs_cache_.free(); %} + function clear_encrypted_byte_count_in_current_segment() : bool + %{ + encrypted_bytes_in_current_segment_ = 0; + return true; + %} + + function inc_encrypted_byte_count_in_current_segment() : bool + %{ + ++encrypted_bytes_in_current_segment_; + return true; + %} + + function get_encrypted_bytes_in_current_segment() : int + %{ + return encrypted_bytes_in_current_segment_; + %} + function get_state(is_orig: bool) : int %{ if ( is_orig ) diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.ssh_segmented_encryption_transition/client.out b/testing/btest/Baseline/scripts.base.protocols.ssh.ssh_segmented_encryption_transition/client.out new file mode 100644 index 0000000000..a63128853a --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.ssh.ssh_segmented_encryption_transition/client.out @@ -0,0 +1,18 @@ +0, T, 64 +1, F, 64 +2, T, 80 +3, F, 80 +4, T, 272 +5, F, 48 +6, T, 80 +7, F, 528 +8, F, 64 +9, T, 176 +10, F, 160 +11, F, 736 +12, F, 96 +13, T, 64 +14, F, 64 +15, F, 64 +16, F, 48 +17, F, 128 diff --git a/testing/btest/Baseline/scripts.base.protocols.ssh.ssh_segmented_encryption_transition/server.out b/testing/btest/Baseline/scripts.base.protocols.ssh.ssh_segmented_encryption_transition/server.out new file mode 100644 index 0000000000..4cc3555cc0 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.ssh.ssh_segmented_encryption_transition/server.out @@ -0,0 +1,46 @@ +0, F, 172 +1, T, 44 +2, F, 44 +3, T, 68 +4, F, 52 +5, T, 148 +6, F, 28 +7, T, 112 +8, F, 500 +9, F, 44 +10, T, 460 +11, F, 108 +12, F, 100 +13, F, 36 +14, F, 36 +15, F, 76 +16, F, 36 +17, F, 84 +18, F, 36 +19, F, 84 +20, F, 36 +21, F, 36 +22, F, 36 +23, F, 92 +24, F, 36 +25, F, 108 +26, F, 36 +27, F, 68 +28, F, 36 +29, F, 36 +30, F, 68 +31, F, 36 +32, F, 68 +33, F, 36 +34, F, 36 +35, F, 68 +36, F, 36 +37, F, 92 +38, F, 36 +39, F, 100 +40, T, 36 +41, F, 44 +42, F, 36 +43, F, 140 +44, T, 36 +45, T, 60 diff --git a/testing/btest/Traces/ssh/ssh_client_sends_first_enc_pkt_with_newkeys.pcap b/testing/btest/Traces/ssh/ssh_client_sends_first_enc_pkt_with_newkeys.pcap new file mode 100644 index 0000000000..4bbf12d87a Binary files /dev/null and b/testing/btest/Traces/ssh/ssh_client_sends_first_enc_pkt_with_newkeys.pcap differ diff --git a/testing/btest/Traces/ssh/ssh_server_sends_first_enc_pkt_with_newkeys.pcap b/testing/btest/Traces/ssh/ssh_server_sends_first_enc_pkt_with_newkeys.pcap new file mode 100644 index 0000000000..9fbad53f08 Binary files /dev/null and b/testing/btest/Traces/ssh/ssh_server_sends_first_enc_pkt_with_newkeys.pcap differ diff --git a/testing/btest/scripts/base/protocols/ssh/ssh_segmented_encryption_transition.zeek b/testing/btest/scripts/base/protocols/ssh/ssh_segmented_encryption_transition.zeek new file mode 100644 index 0000000000..d92215c0f8 --- /dev/null +++ b/testing/btest/scripts/base/protocols/ssh/ssh_segmented_encryption_transition.zeek @@ -0,0 +1,21 @@ +# In the pcaps used here, the first encrypted packet is sent along with NEWKEYS +# message of either the client (1st pcap) or the server (2nd pcap) instead of +# separately. The "ssh_encrypted_packet" should be raised for such encrypted +# data appearing within the same tcp segment delivery as other non-encrypted +# messages. + +# @TEST-EXEC: zeek -b -C -r $TRACES/ssh/ssh_client_sends_first_enc_pkt_with_newkeys.pcap %INPUT > client.out +# @TEST-EXEC: zeek -b -C -r $TRACES/ssh/ssh_server_sends_first_enc_pkt_with_newkeys.pcap %INPUT > server.out +# @TEST-EXEC: btest-diff client.out +# @TEST-EXEC: btest-diff server.out + +@load base/protocols/ssh + +global pkts: count = 0; +redef SSH::disable_analyzer_after_detection = F; + +event ssh_encrypted_packet(c: connection, orig: bool, len: count) + { + print pkts, orig, len; + ++pkts; + }