diff --git a/CHANGES b/CHANGES index e3adb31d0d..f5a73e6d0f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,15 @@ +6.1.0-dev.336 | 2023-08-28 12:19:45 +0200 + + * testing: Bump external test suite (Arne Welzel, Corelight) + + * dhcp: Handle is_orig=T for connections from server to 255.255.255.255 (Arne Welzel, Corelight) + + 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. + + * GH-3235: IPBasedAnalyzer: Don't flip connections when destination is broadcast (Arne Welzel, Corelight) + 6.1.0-dev.331 | 2023-08-25 21:38:40 +0200 * BTests for any/vector-of-any fixes (Vern Paxson, Corelight) 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/VERSION b/VERSION index 7b2e46d07d..cac9fc3989 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.1.0-dev.331 +6.1.0-dev.336 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/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/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/Traces/udp-broadcast.pcap b/testing/btest/Traces/udp-broadcast.pcap new file mode 100644 index 0000000000..986f725ad0 Binary files /dev/null and b/testing/btest/Traces/udp-broadcast.pcap differ 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 }; 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 diff --git a/testing/external/commit-hash.zeek-testing b/testing/external/commit-hash.zeek-testing index d666b26bde..55ea2df69b 100644 --- a/testing/external/commit-hash.zeek-testing +++ b/testing/external/commit-hash.zeek-testing @@ -1 +1 @@ -5218e6b23477a8fc21c17655c9d955fb80c7de4a +cfc57a8a0513d8651c8946739e13ee0d8cfaad52