From ba04f4c31d12b8f8f8a762e2b9f4b4b1848906b8 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 17 Aug 2023 13:03:08 +0200 Subject: [PATCH 1/3] IPBasedAnalyzer: Don't flip connections when destination is broadcast Closes #3235 --- NEWS | 4 ++++ .../protocol/ip/IPBasedAnalyzer.cc | 2 +- .../core.udp-broadcast-no-flip/conn.log.cut | 3 +++ testing/btest/Traces/udp-broadcast.pcap | Bin 0 -> 255 bytes testing/btest/core/udp-broadcast-no-flip.zeek | 9 +++++++++ 5 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 testing/btest/Baseline/core.udp-broadcast-no-flip/conn.log.cut create mode 100644 testing/btest/Traces/udp-broadcast.pcap create mode 100644 testing/btest/core/udp-broadcast-no-flip.zeek diff --git a/NEWS b/NEWS index e9769a31b6..2a17735a93 100644 --- a/NEWS +++ b/NEWS @@ -78,6 +78,10 @@ Changed Functionality ``policy/protocols/conn/community-id-logging.zeek`` was loaded before. This was fairly unusual and hard to debug behavior. +- Connections to broadcast addresses are not flipped based on ``likely_server_ports`` + anymore. Previously, broadcast packets originating from a likely server port + resulted in 255.255.255.255 being the originator in ``conn.log``. + Removed Functionality --------------------- diff --git a/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc b/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc index 345bfe0722..7a6874f6e6 100644 --- a/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc +++ b/src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc @@ -171,7 +171,7 @@ zeek::Connection* IPBasedAnalyzer::NewConn(const ConnTuple* id, const detail::Co pkt->ip_hdr->FlowLabel(), pkt); conn->SetTransport(transport); - if ( flip ) + if ( flip && ! id->dst_addr.IsBroadcast() ) conn->FlipRoles(); BuildSessionAnalyzerTree(conn); diff --git a/testing/btest/Baseline/core.udp-broadcast-no-flip/conn.log.cut b/testing/btest/Baseline/core.udp-broadcast-no-flip/conn.log.cut new file mode 100644 index 0000000000..a284951a98 --- /dev/null +++ b/testing/btest/Baseline/core.udp-broadcast-no-flip/conn.log.cut @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +ts uid id.orig_h id.orig_p id.resp_h id.resp_p history +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 192.168.1.1 40190 255.255.255.255 7437 D diff --git a/testing/btest/Traces/udp-broadcast.pcap b/testing/btest/Traces/udp-broadcast.pcap new file mode 100644 index 0000000000000000000000000000000000000000..986f725ad0a8428e74dbe789bbea4257ba3dc711 GIT binary patch literal 255 zcmca|c+)~A1{MYwaA0F#U<7imyhsRjNn~NT4rGJye=vA)PyPRa3JwNW28NRi3=Rwq zf)&*VRxmPxBlpPzrIsvp=yh=vd!-Efy6V>1H-upk3EA0mpMG}JS+ SFu_m)aw-TjL1_#H1Ox!Om@3-< literal 0 HcmV?d00001 diff --git a/testing/btest/core/udp-broadcast-no-flip.zeek b/testing/btest/core/udp-broadcast-no-flip.zeek new file mode 100644 index 0000000000..601d1f7290 --- /dev/null +++ b/testing/btest/core/udp-broadcast-no-flip.zeek @@ -0,0 +1,9 @@ +# @TEST-DOC: Pcap contains broadcast with port 40190 to port 7437. Set likely_server_ports to 40190 but don't expect this connection to be flipped. + +# @TEST-EXEC: zeek -b -r $TRACES/udp-broadcast.pcap %INPUT +# @TEST-EXEC: zeek-cut -m ts uid id.orig_h id.orig_p id.resp_h id.resp_p history < conn.log > conn.log.cut +# @TEST-EXEC: btest-diff conn.log.cut + +@load base/protocols/conn + +redef likely_server_ports += { 40190/udp }; From fc768a9e0167089bde2a5d41d7bb84967b832362 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 17 Aug 2023 14:05:01 +0200 Subject: [PATCH 2/3] dhcp: Handle is_orig=T for connections from server to 255.255.255.255 This works around the new semantics of is_orig=T for "connections" from DHCP servers to broadcast addresses. IMO, having the server address as originator in the conn.log is still more intuitive. --- scripts/base/protocols/dhcp/main.zeek | 34 +++++++++++++++---- .../conn.log | 14 ++++++++ .../protocols/dhcp/dhcp-all-msg-types.zeek | 2 ++ 3 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.dhcp.dhcp-all-msg-types/conn.log diff --git a/scripts/base/protocols/dhcp/main.zeek b/scripts/base/protocols/dhcp/main.zeek index 6bbd5d13e2..1d53cbfd63 100644 --- a/scripts/base/protocols/dhcp/main.zeek +++ b/scripts/base/protocols/dhcp/main.zeek @@ -204,11 +204,16 @@ event DHCP::aggregate_msgs(ts: time, id: conn_id, uid: string, is_orig: bool, ms log_info$msg_types += DHCP::message_types[msg$m_type]; + # The is_orig flag is T for "connections" initiated by servers + # to broadcast addresses, otherwise is_orig indicates that this + # is a DHCP client. + local is_client = is_orig && (id$orig_h == 0.0.0.0 || id$orig_p == 68/udp || id$resp_p == 67/udp); + # Let's watch for messages in any DHCP message type # and split them out based on client and server. if ( options?$message ) { - if ( is_orig ) + if ( is_client ) log_info$client_message = options$message; else log_info$server_message = options$message; @@ -218,7 +223,7 @@ event DHCP::aggregate_msgs(ts: time, id: conn_id, uid: string, is_orig: bool, ms # expiration handling. log_info$last_message_ts = ts; - if ( is_orig ) # client requests + if ( is_client ) # client requests { # Assign the client addr in case this is a session # of only INFORM messages (no lease handed out). @@ -246,12 +251,27 @@ event DHCP::aggregate_msgs(ts: time, id: conn_id, uid: string, is_orig: bool, ms { # Only log the address of the server if it handed out # an IP address. - if ( msg$yiaddr != 0.0.0.0 && - id$resp_h != 255.255.255.255 ) + if ( msg$yiaddr != 0.0.0.0 ) { - log_info$server_addr = id$resp_h; - log_info$server_port = id$resp_p; - log_info$client_port = id$orig_p; + if ( is_orig ) + { + # This is a server message and is_orig is T. + # This means it's a DHCP server broadcasting + # and the server is the originator. + log_info$server_addr = id$orig_h; + log_info$server_port = id$orig_p; + log_info$client_port = id$resp_p; + } + else + { + # When a server sends to a non-broadcast + # address, Zeek's connection flipping is + # in effect and the server is the responder + # instead. + log_info$server_addr = id$resp_h; + log_info$server_port = id$resp_p; + log_info$client_port = id$orig_p; + } } # Only use the client hardware address from the server diff --git a/testing/btest/Baseline/scripts.base.protocols.dhcp.dhcp-all-msg-types/conn.log b/testing/btest/Baseline/scripts.base.protocols.dhcp.dhcp-all-msg-types/conn.log new file mode 100644 index 0000000000..9f18c0e6e7 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.dhcp.dhcp-all-msg-types/conn.log @@ -0,0 +1,14 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path conn +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents +#types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 0.0.0.0 68 255.255.255.255 67 udp dhcp 5.099034 1560 0 S0 T T 0 D 6 1728 0 0 - +XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 128.2.6.97 68 128.2.6.152 67 udp dhcp - - - SHR F F 0 ^d 0 0 1 395 - +XXXXXXXXXX.XXXXXX CtPZjS20MLrsMUOJi2 128.2.6.189 68 128.2.6.152 67 udp dhcp - - - SHR F F 0 ^d 0 0 1 395 - +XXXXXXXXXX.XXXXXX C4J4Th3PJpwUYZZ6gc 128.2.6.152 67 255.255.255.255 68 udp dhcp - - - S0 F T 0 D 1 328 0 0 - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/scripts/base/protocols/dhcp/dhcp-all-msg-types.zeek b/testing/btest/scripts/base/protocols/dhcp/dhcp-all-msg-types.zeek index ed6a49b015..4f549d01c0 100644 --- a/testing/btest/scripts/base/protocols/dhcp/dhcp-all-msg-types.zeek +++ b/testing/btest/scripts/base/protocols/dhcp/dhcp-all-msg-types.zeek @@ -3,6 +3,8 @@ # but only one lease should show up in the logs. # @TEST-EXEC: zeek -b -r $TRACES/dhcp/dhcp.trace %INPUT +# @TEST-EXEC: btest-diff conn.log # @TEST-EXEC: btest-diff dhcp.log +@load base/protocols/conn @load base/protocols/dhcp From 4cabcd43c64d5165c9eba8c748c035a8d8b256e6 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 17 Aug 2023 16:27:31 +0200 Subject: [PATCH 3/3] testing: Bump external test suite --- testing/external/commit-hash.zeek-testing | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/external/commit-hash.zeek-testing b/testing/external/commit-hash.zeek-testing index d666b26bde..295376b2ea 100644 --- a/testing/external/commit-hash.zeek-testing +++ b/testing/external/commit-hash.zeek-testing @@ -1 +1 @@ -5218e6b23477a8fc21c17655c9d955fb80c7de4a +5f720ed896c325680129459da7aadc1f9f44992e