From 0d6174a5d65625e50704716c3052db119a7c5c80 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 4 Jul 2023 17:55:41 +0200 Subject: [PATCH 1/4] Remove icmp_conn leftovers Roughly 2.5 years ago all events taking the ``icmp_conn`` parameter were removed with 44ad614094db3c57fef7fb226c539f6f090a5421 and the NetVar.cc type not populated anymore. Remove the left-overs in script land, too. --- scripts/base/frameworks/notice/main.zeek | 15 +-------------- scripts/base/init-bare.zeek | 15 --------------- src/NetVar.cc | 1 - 3 files changed, 1 insertion(+), 30 deletions(-) diff --git a/scripts/base/frameworks/notice/main.zeek b/scripts/base/frameworks/notice/main.zeek index 14bbbeb08f..991376dc46 100644 --- a/scripts/base/frameworks/notice/main.zeek +++ b/scripts/base/frameworks/notice/main.zeek @@ -81,10 +81,6 @@ export { ## reference to the actual connection will be deleted after ## applying the notice policy. conn: connection &optional; - ## A shorthand way of giving the uid and id to a notice. The - ## reference to the actual connection will be deleted after - ## applying the notice policy. - iconn: icmp_conn &optional; ## A file record if the notice is related to a file. The ## reference to the actual fa_file record will be deleted after @@ -108,7 +104,7 @@ export { file_desc: string &log &optional; ## The transport protocol. Filled automatically when either - ## *conn*, *iconn* or *p* is specified. + ## *conn* or *p* is specified. proto: transport_proto &log &optional; ## The :zeek:type:`Notice::Type` of the notice. @@ -668,15 +664,6 @@ function apply_policy(n: Notice::Info) if ( n?$p ) n$proto = get_port_transport_proto(n$p); - if ( n?$iconn ) - { - n$proto = icmp; - if ( ! n?$src ) - n$src = n$iconn$orig_h; - if ( ! n?$dst ) - n$dst = n$iconn$resp_h; - } - if ( ! n?$email_body_sections ) n$email_body_sections = vector(); if ( ! n?$email_delay_tokens ) diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index 2352566129..c66d4b5189 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -231,21 +231,6 @@ type flow_id : record { dst_p: port; ##< The destination port number. } &log; -## Specifics about an ICMP conversation. ICMP events typically pass this in -## addition to :zeek:type:`conn_id`. -## -## .. zeek:see:: icmp_echo_reply icmp_echo_request icmp_redirect icmp_sent -## icmp_time_exceeded icmp_unreachable -type icmp_conn: record { - orig_h: addr; ##< The originator's IP address. - resp_h: addr; ##< The responder's IP address. - itype: count; ##< The ICMP type of the packet that triggered the instantiation of the record. - icode: count; ##< The ICMP code of the packet that triggered the instantiation of the record. - len: count; ##< The length of the ICMP payload of the packet that triggered the instantiation of the record. - hlim: count; ##< The encapsulating IP header's Hop Limit value. - v6: bool; ##< True if it's an ICMPv6 packet. -}; - ## Specifics about an ICMP conversation/packet. ## ICMP events typically pass this in addition to :zeek:type:`conn_id`. ## diff --git a/src/NetVar.cc b/src/NetVar.cc index 64d51cbf3f..3389c7b224 100644 --- a/src/NetVar.cc +++ b/src/NetVar.cc @@ -15,7 +15,6 @@ zeek::RecordType* endpoint_stats; zeek::RecordType* connection_type; zeek::RecordType* fa_file_type; zeek::RecordType* fa_metadata_type; -zeek::RecordType* icmp_conn; zeek::RecordType* icmp_context; zeek::RecordType* SYN_packet; zeek::RecordType* pcap_packet; From 640bd4e065412ecd00e7e0f471dfe42c3be9983c Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 4 Jul 2023 17:58:44 +0200 Subject: [PATCH 2/4] Conn: Remove is_version_sep() Leftover from 2655a653314237f5354fdb644bf642d0d5867f63. --- src/Conn.cc | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/Conn.cc b/src/Conn.cc index e01fce29c3..c833b831ba 100644 --- a/src/Conn.cc +++ b/src/Conn.cc @@ -315,18 +315,6 @@ void Connection::AppendAddl(const char* str) cv->Assign(6, util::fmt(format, old, str)); } -// Returns true if the character at s separates a version number. -static inline bool is_version_sep(const char* s, const char* end) - { - return - // foo-1.2.3 - (s < end - 1 && ispunct(s[0]) && isdigit(s[1])) || - // foo-v1.2.3 - (s < end - 2 && ispunct(s[0]) && tolower(s[1]) == 'v' && isdigit(s[2])) || - // foo 1.2.3 - isspace(s[0]); - } - void Connection::Match(detail::Rule::PatternType type, const u_char* data, int len, bool is_orig, bool bol, bool eol, bool clear_state) { From a2214ad6115c2c614c8cd6eeeec1e33151708831 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 4 Jul 2023 19:19:03 +0200 Subject: [PATCH 3/4] Conn: In-place val flip and connection_flipped() Avoids loosing state on a connection value when a connection is flipped. Fixes up the NTP baseline as well where this was visible: analyzer_confirmation_info() was raised for a connection value which was immediately forgotten due to the subsequent connection flipping. Closed #3028 --- NEWS | 7 +++++ src/Conn.cc | 29 ++++++++++++++++++- src/event.bif | 11 +++++++ .../Baseline/core.connection_flipped/out | 4 +++ .../conn.log | 2 +- testing/btest/core/connection_flipped.zeek | 29 +++++++++++++++++++ 6 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 testing/btest/Baseline/core.connection_flipped/out create mode 100644 testing/btest/core/connection_flipped.zeek diff --git a/NEWS b/NEWS index f40752aa08..ea619c1f81 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,13 @@ New Functionality Changed Functionality --------------------- +- A connection's value is now updated in-place when its directionality is + flipped due to Zeek's heuristics (for example, SYN/SYN-ACK reversal or + protocol specific approaches). + Previously, a connection's value was discarded when flipped, including any + values set in a ``new_connection()`` handler. A new ``connection_flipped()`` + event is added to allow updating custom state in script-land. + Removed Functionality --------------------- diff --git a/src/Conn.cc b/src/Conn.cc index c833b831ba..5d39db982a 100644 --- a/src/Conn.cc +++ b/src/Conn.cc @@ -217,6 +217,29 @@ void Connection::HistoryThresholdEvent(EventHandlerPtr e, bool is_orig, uint32_t EnqueueEvent(e, nullptr, GetVal(), val_mgr->Bool(is_orig), val_mgr->Count(threshold)); } +namespace + { +// Flip everything that needs to be flipped in the connection +// record that is known on this level. This needs to align +// with GetVal() and connection's layout in init-bare. +void flip_conn_val(const RecordValPtr& conn_val) + { + // Flip the the conn_id (c$id). + const auto& id_val = conn_val->GetField(0); + const auto& tmp_addr = id_val->GetField(0); + const auto& tmp_port = id_val->GetField(1); + id_val->Assign(0, id_val->GetField(2)); + id_val->Assign(1, id_val->GetField(3)); + id_val->Assign(2, tmp_addr); + id_val->Assign(3, tmp_port); + + // Flip the endpoints within connection. + const auto& tmp_endp = conn_val->GetField(1); + conn_val->Assign(1, conn_val->GetField(2)); + conn_val->Assign(2, tmp_endp); + } + } + const RecordValPtr& Connection::GetVal() { if ( ! conn_val ) @@ -358,7 +381,8 @@ void Connection::FlipRoles() resp_flow_label = orig_flow_label; orig_flow_label = tmp_flow; - conn_val = nullptr; + if ( conn_val ) + flip_conn_val(conn_val); if ( adapter ) adapter->FlipRoles(); @@ -366,6 +390,9 @@ void Connection::FlipRoles() analyzer_mgr->ApplyScheduledAnalyzers(this); AddHistory('^'); + + if ( connection_flipped ) + EnqueueEvent(connection_flipped, nullptr, GetVal()); } void Connection::Describe(ODesc* d) const diff --git a/src/event.bif b/src/event.bif index e474c200b4..14a86098f5 100644 --- a/src/event.bif +++ b/src/event.bif @@ -194,6 +194,17 @@ event connection_reused%(c: connection%); ## new_connection new_connection_contents partial_connection event connection_status_update%(c: connection%); +## Generated for a connection when the direction was flipped by Zeek's +## heuristics and originator and responder roles were reversed. If state is +## kept on a connection record for originator and responder, this event +## can be used to update or reset such state. The ``orig`` and ``resp`` fields +## as well as the contents of the ``id`` field reflect the post-flip state. +## +## c: The connection. +## +## .. zeek:see:: connection_established new_connection +event connection_flipped%(c: connection%); + ## Generated for a connection over IPv6 when one direction has changed ## the flow label that it's using. ## diff --git a/testing/btest/Baseline/core.connection_flipped/out b/testing/btest/Baseline/core.connection_flipped/out new file mode 100644 index 0000000000..cbf07db6d3 --- /dev/null +++ b/testing/btest/Baseline/core.connection_flipped/out @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +1362692526.939084, new_connection, [orig_h=192.150.187.43, orig_p=80/tcp, resp_h=141.142.228.5, resp_p=59856/tcp, extra_id=42], H, 1362692526.939084 +1362692526.939344, connection_flipped, [orig_h=141.142.228.5, orig_p=59856/tcp, resp_h=192.150.187.43, resp_p=80/tcp, extra_id=42], Hs^, 1362692526.939084 +1362692527.080972, connection_state_remove, [orig_h=141.142.228.5, orig_p=59856/tcp, resp_h=192.150.187.43, resp_p=80/tcp, extra_id=42], Hs^ADadFf, 1362692526.939084 diff --git a/testing/btest/Baseline/scripts.base.protocols.ntp.misordered-ntp/conn.log b/testing/btest/Baseline/scripts.base.protocols.ntp.misordered-ntp/conn.log index d2639d9ea4..f15d414a24 100644 --- a/testing/btest/Baseline/scripts.base.protocols.ntp.misordered-ntp/conn.log +++ b/testing/btest/Baseline/scripts.base.protocols.ntp.misordered-ntp/conn.log @@ -7,5 +7,5 @@ #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 192.168.1.95 123 17.253.4.253 123 udp - 0.959285 96 0 S0 T F 0 D^ 2 152 0 0 - +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 192.168.1.95 123 17.253.4.253 123 udp ntp 0.959285 96 0 S0 T F 0 D^ 2 152 0 0 - #close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/core/connection_flipped.zeek b/testing/btest/core/connection_flipped.zeek new file mode 100644 index 0000000000..025e9be9e0 --- /dev/null +++ b/testing/btest/core/connection_flipped.zeek @@ -0,0 +1,29 @@ +# @TEST-DOC: A connection flip does not reset the ConnVal. Regression test for #3028. + +# @TEST-EXEC: zeek -b -r $TRACES/tcp/handshake-reorder.trace %INPUT >out +# @TEST-EXEC: TEST_DIFF_CANONIFIER= btest-diff out + +redef record conn_id += { + extra_id: count &optional; +}; + +redef record connection += { + my_timestamp: time &optional; +}; + +event new_connection(c: connection) + { + c$id$extra_id = 42; + c$my_timestamp = network_time(); + print network_time(), "new_connection", c$id, c$history, c$my_timestamp; + } + +event connection_flipped(c: connection) + { + print network_time(), "connection_flipped", c$id, c$history, c$my_timestamp; + } + +event connection_state_remove(c: connection) + { + print network_time(), "connection_state_remove", c$id, c$history, c$my_timestamp; + } From cbdeb4abf2960d22be64bdd130336a19aabf406b Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Wed, 5 Jul 2023 10:24:29 +0200 Subject: [PATCH 4/4] Update dump-events baseline, not running with OpenSSL 3 --- .../scripts.policy.misc.dump-events/really-all-events.log | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testing/btest/Baseline/scripts.policy.misc.dump-events/really-all-events.log b/testing/btest/Baseline/scripts.policy.misc.dump-events/really-all-events.log index 9d3eca4e48..45e341701c 100644 --- a/testing/btest/Baseline/scripts.policy.misc.dump-events/really-all-events.log +++ b/testing/btest/Baseline/scripts.policy.misc.dump-events/really-all-events.log @@ -8016,6 +8016,9 @@ XXXXXXXXXX.XXXXXX event_queue_flush_point XXXXXXXXXX.XXXXXX event_queue_flush_point XXXXXXXXXX.XXXXXX Broker::log_flush XXXXXXXXXX.XXXXXX run_sync_hook +XXXXXXXXXX.XXXXXX connection_flipped + [0] c: connection = [id=[orig_h=192.168.133.100, orig_p=49336/tcp, resp_h=74.125.71.189, resp_p=443/tcp], orig=[size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=58:b0:35:86:54:8d], resp=[size=85, state=3, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=cc:b2:55:f4:62:92], start_time=XXXXXXXXXX.XXXXXX, duration=0 secs, service={\x0a\x0a}, history=^d, uid=CP5puj4I8PtEU4qzYg, tunnel=, vlan=, inner_vlan=, dpd=, dpd_state=, service_violation={\x0a\x0a}, removal_hooks=, conn=, extract_orig=F, extract_resp=F, thresholds=, dce_rpc=, dce_rpc_state=, dce_rpc_backing=, dhcp=, dnp3=, dns=, dns_state=, ftp=, ftp_data_reuse=F, ssl=, http=, http_state=, irc=, krb=, modbus=, mqtt=, mqtt_state=, mysql=, ntlm=, ntp=, radius=, rdp=, rfb=, sip=, sip_state=, snmp=, smb_state=, smtp=, smtp_state=, socks=, ssh=, syslog=] + XXXXXXXXXX.XXXXXX new_connection_contents [0] c: connection = [id=[orig_h=192.168.133.100, orig_p=49336/tcp, resp_h=74.125.71.189, resp_p=443/tcp], orig=[size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=58:b0:35:86:54:8d], resp=[size=85, state=3, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=cc:b2:55:f4:62:92], start_time=XXXXXXXXXX.XXXXXX, duration=0 secs, service={\x0a\x0a}, history=^d, uid=CP5puj4I8PtEU4qzYg, tunnel=, vlan=, inner_vlan=, dpd=, dpd_state=, service_violation={\x0a\x0a}, removal_hooks=, conn=, extract_orig=F, extract_resp=F, thresholds=, dce_rpc=, dce_rpc_state=, dce_rpc_backing=, dhcp=, dnp3=, dns=, dns_state=, ftp=, ftp_data_reuse=F, ssl=, http=, http_state=, irc=, krb=, modbus=, mqtt=, mqtt_state=, mysql=, ntlm=, ntp=, radius=, rdp=, rfb=, sip=, sip_state=, snmp=, smb_state=, smtp=, smtp_state=, socks=, ssh=, syslog=]