diff --git a/CHANGES b/CHANGES index 5503a6dd5d..22d69afa94 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,29 @@ +6.1.0-dev.170 | 2023-07-05 13:32:21 -0700 + + * Update dump-events baseline, not running with OpenSSL 3 (Arne Welzel, Corelight) + + * Conn: In-place val flip and connection_flipped() (Arne Welzel, Corelight) + + 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 + + * Conn: Remove is_version_sep() (Arne Welzel, Corelight) + + Leftover from 2655a653314237f5354fdb644bf642d0d5867f63. + + * Remove icmp_conn leftovers (Arne Welzel, Corelight) + + 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. + 6.1.0-dev.164 | 2023-07-05 10:23:27 -0700 * Bump Spicy to latest release. (Benjamin Bannier, Corelight) diff --git a/NEWS b/NEWS index 18b6ce6b1c..2e4542805f 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,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/VERSION b/VERSION index 734200789b..4295704207 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.1.0-dev.164 +6.1.0-dev.170 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/Conn.cc b/src/Conn.cc index e01fce29c3..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 ) @@ -315,18 +338,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) { @@ -370,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(); @@ -378,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/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; 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/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=] 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; + }