Merge branch 'topic/awelzel/2696-mysql-analyzer-issues'

* topic/awelzel/2696-mysql-analyzer-issues:
  testing/mysql: Add traces recorded with a free-tier MySQL instance
  MySQL: Fix endianness, introduce mysql_eof() event

CHANGES

@@ -1,3 +1,26 @@
+5.2.0-dev.538 | 2023-01-27 11:24:35 +0100
+
+  * testing/mysql: Add traces recorded with a free-tier MySQL instance (Arne Welzel, Corelight)
+
+  * MySQL: Fix endianness, introduce mysql_eof() event (Arne Welzel, Corelight)
+
+    We were parsing MySQL using bigendian even though the protocol is
+    specified as with "least significant byte first" [1]. This is most
+    problematic when parsing length encoded strings with 2 byte length
+    fields...
+
+    Further, I think, the EOF_Packet parsing was borked, either due to
+    testing the CLIENT_DEPRECATE_EOF with the wrong endianness, or due to
+    the workaround in Resultset processing raising mysql_ok(). Introduce a
+    new mysql_eof() that triggers for EOF_Packet's and remove the fake
+    mysql_ok() Resultset invocation to fix. Adapt the mysql script and tests
+    to account for the new event.
+
+    This is a quite backwards incompatible change on the event level, but
+    due to being quite buggy in general, doubt this matters to many.
+
+    [1] https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_dt_integers.html
+
 5.2.0-dev.534 | 2023-01-26 20:06:24 +0100
 
   * Tame error reporting and abort() for undefined types (Arne Welzel, Corelight)

NEWS

@@ -20,6 +20,11 @@ Breaking Changes
 - The blank identifier ``_`` cannot be used in expressions and options anymore.
   Outside of obfuscation exercises, this should have little real-world impact.
 
+- A new ``mysql_eof`` event has been introduced and the ``mysql_ok`` event
+  is not raised in its place or artificially anymore. The base scripts were
+  adapted accordingly. Users of ``mysql_ok()`` likely need to switch to
+  ``mysql_eof()``.
+
 New Functionality
 -----------------
 
@@ -206,6 +211,9 @@ Changed Functionality
 - Notices created for files transferred over multiple connections will now be
   associated with one of the connections rather than none.
 
+- The MySQL analyzer has been switched to parse in little endian. This avoids
+  analyzer violations due to out of bound errors for length encoded strings.
+
 Deprecated Functionality
 ------------------------
 

VERSION

@@ -1 +1 @@
-5.2.0-dev.534
+5.2.0-dev.538

scripts/base/protocols/mysql/main.zeek

@@ -112,6 +112,25 @@ event mysql_error(c: connection, code: count, msg: string) &priority=-5
 		}
 	}
 
+event mysql_eof(c: connection, is_intermediate: bool) &priority=-5
+	{
+	if ( is_intermediate )
+		return;
+
+	if ( c?$mysql )
+		{
+		# We don't have more information, so just
+		# place what mysql_ok() would've done.
+		if ( ! c$mysql?$success )
+			c$mysql$success = T;
+		if ( ! c$mysql?$rows )
+			c$mysql$rows = 0;
+
+		Log::write(mysql::LOG, c$mysql);
+		delete c$mysql;
+		}
+	}
+
 event mysql_ok(c: connection, affected_rows: count) &priority=5
 	{
 	if ( c?$mysql )

src/analyzer/protocol/mysql/events.bif

@@ -38,6 +38,18 @@ event mysql_error%(c: connection, code: count, msg: string%);
 ## .. zeek:see:: mysql_command_request mysql_error mysql_server_version mysql_handshake
 event mysql_ok%(c: connection, affected_rows: count%);
 
+## Generated for a MySQL EOF packet.
+##
+## See the MySQL `documentation <http://dev.mysql.com/doc/internals/en/client-server-protocol.html>`__
+## for more information about the MySQL protocol.
+##
+## c: The connection.
+##
+## is_intermediate: True if this is an EOF packet between the column definition and the rows, false if a final EOF.
+##
+## .. zeek:see:: mysql_command_request mysql_error mysql_server_version mysql_handshake
+event mysql_eof%(c: connection, is_intermediate: bool%);
+
 ## Generated for each MySQL ResultsetRow response packet.
 ##
 ## See the MySQL `documentation <http://dev.mysql.com/doc/internals/en/client-server-protocol.html>`__

src/analyzer/protocol/mysql/mysql-analyzer.pac

@@ -65,19 +65,19 @@ refine flow MySQL_Flow += {
 		return true;
 		%}
 
-	function proc_resultset(msg: Resultset): bool
+	function proc_eof_packet(msg: EOF_Packet): bool
 		%{
-		if ( connection()->get_results_seen() == 1 )
-			{
-			// This is a bit fake...
-			if ( mysql_ok )
-				zeek::BifEvent::enqueue_mysql_ok(connection()->zeek_analyzer(),
-				                           connection()->zeek_analyzer()->Conn(),
-				                           0);
-			}
+		if ( mysql_eof )
+			zeek::BifEvent::enqueue_mysql_eof(connection()->zeek_analyzer(),
+			                                  connection()->zeek_analyzer()->Conn(),
+			                                  ${msg.typ} == EOF_INTERMEDIATE);
+		return true;
+		%}
 
+	function proc_resultset(msg: Resultset): bool
+		%{
 		if ( ${msg.is_eof} )
-			return true;
+			return true;  // Raised through proc_eof_packet()
 
 		if ( ! mysql_result_row )
 			return true;
@@ -127,6 +127,10 @@ refine typeattr OK_Packet += &let {
 	proc = $context.flow.proc_ok_packet(this);
 };
 
+refine typeattr EOF_Packet += &let {
+	proc = $context.flow.proc_eof_packet(this);
+};
+
 refine typeattr Resultset += &let {
 	proc = $context.flow.proc_resultset(this);
 };

src/analyzer/protocol/mysql/mysql-protocol.pac

@@ -48,6 +48,7 @@ type LengthEncodedStringArg(first_byte: uint8) = record {
 		public:
 		int operator()(uint24le* num) const
 			{
+			// Convert 24bit little endian int parsed as 3 uint8 into host endianess.
 			return (num->byte1() << 16) | (num->byte2() << 8) | num->byte3();
 			}
 
@@ -145,12 +146,17 @@ enum Expected {
 	EXPECT_COLUMN_DEFINITION,
 	EXPECT_COLUMN_DEFINITION_OR_EOF,
 	EXPECT_COLUMN_COUNT,
-	EXPECT_EOF,
+	EXPECT_EOF_THEN_RESULTSET,
 	EXPECT_RESULTSET,
 	EXPECT_REST_OF_PACKET,
 	EXPECT_AUTH_SWITCH,
 };
 
+enum EOFType {
+	EOF_INTERMEDIATE,  # column definition to result row transition
+	EOF_END,
+};
+
 enum Client_Capabilities {
 	# Expects an OK (instead of EOF) after the resultset rows of a Text Resultset. 
 	CLIENT_DEPRECATE_EOF = 0x01000000,
@@ -168,7 +174,7 @@ type MySQL_PDU(is_orig: bool) = record {
 	} &requires(state);
 } &let {
 	state: int = $context.connection.get_state();
-} &length=hdr.len &byteorder=bigendian;
+} &length=hdr.len &byteorder=littleendian;
 
 type Header = record {
 	le_len: uint24le;
@@ -229,7 +235,7 @@ type Handshake_Response_Packet = case $context.connection.get_version() of {
 	9  -> v9_response : Handshake_Response_Packet_v9;
 } &let {
 	version: uint8 = $context.connection.get_version();
-} &byteorder=bigendian;
+};
 
 type Handshake_Response_Packet_v10 = record {
 	cap_flags   : uint32;
@@ -273,15 +279,15 @@ type Command_Response(pkt_len: uint32) = case $context.connection.get_expectatio
 	EXPECT_REST_OF_PACKET           -> rest          : bytestring &restofdata;
 	EXPECT_STATUS                   -> status        : Command_Response_Status;
 	EXPECT_AUTH_SWITCH              -> auth_switch   : AuthSwitchRequest;
-	EXPECT_EOF                      -> eof           : EOFIfLegacy(pkt_len);
+	EXPECT_EOF_THEN_RESULTSET       -> eof           : EOFIfLegacyThenResultset(pkt_len);
 	default                         -> unknown       : empty;
 };
 
 type Command_Response_Status = record {
 	pkt_type: uint8;
 	response: case pkt_type of {
 		0x00    -> data_ok:  OK_Packet;
-		0xfe    -> data_eof: EOF_Packet;
+		0xfe    -> data_eof: EOF_Packet(EOF_END);
 		0xff    -> data_err: ERR_Packet;
 		default -> unknown:  empty;
 	};
@@ -311,11 +317,12 @@ type ColumnDefinition = record {
 	def  : ColumnDefinition41(dummy);
 } &let {
 	update_remain     : bool = $context.connection.dec_remaining_cols();
-	update_expectation: bool = $context.connection.set_next_expected($context.connection.get_remaining_cols() > 0 ? EXPECT_COLUMN_DEFINITION : EXPECT_EOF);
+	update_expectation: bool = $context.connection.set_next_expected($context.connection.get_remaining_cols() > 0 ? EXPECT_COLUMN_DEFINITION : EXPECT_EOF_THEN_RESULTSET);
 };
 
+# Only used to indicate the end of a result, no intermediate eofs here.
 type EOFOrOK = case $context.connection.get_deprecate_eof() of {
-	false -> eof: EOF_Packet;
+	false -> eof: EOF_Packet(EOF_END);
 	true  -> ok: OK_Packet;
 };
 
@@ -333,8 +340,8 @@ type ColumnDefinitionOrEOF(pkt_len: uint32) = record {
 };
 
 
-type EOFIfLegacy(pkt_len: uint32) = case $context.connection.get_deprecate_eof() of {
-	false -> eof: EOF_Packet;
+type EOFIfLegacyThenResultset(pkt_len: uint32) = case $context.connection.get_deprecate_eof() of {
+	false -> eof: EOF_Packet_With_Marker(EOF_INTERMEDIATE);
 	true  -> resultset: Resultset(pkt_len);
 } &let {
 	update_result_seen: bool = $context.connection.set_results_seen(0);
@@ -408,9 +415,14 @@ type ERR_Packet = record {
 	update_state: bool = $context.connection.update_state(COMMAND_PHASE);
 };
 
-type EOF_Packet = record {
+type EOF_Packet(typ: EOFType) = record {
 	warnings: uint16;
 	status  : uint16;
+};
+
+type EOF_Packet_With_Marker(typ: EOFType) = record {
+	marker : uint8;
+	payload: EOF_Packet(typ);
 } &let {
 	update_state: bool = $context.connection.update_state(COMMAND_PHASE);
 };

testing/btest/Baseline/scripts.base.protocols.mysql.encrypted-aws-rds/tls-12.conn.log

@@ -0,0 +1,11 @@
+### 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	82.239.87.25	58132	79.107.90.25	3306	tcp	-	2.043921	724	3255	SF	-	-	0	ShAdDaFf	14	1460	11	3835	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.mysql.encrypted-aws-rds/tls-13.conn.log

@@ -0,0 +1,11 @@
+### 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	82.239.87.25	57902	79.107.90.25	3306	tcp	-	6.756360	1076	3776	SF	-	-	0	ShAdDaFf	19	2072	14	4512	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.mysql.plain-aws-rds/mysql.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	mysql
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd	arg	success	rows	response
+#types	time	string	addr	port	addr	port	string	string	bool	count	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	82.239.87.25	58514	79.107.90.25	3306	login	admin	T	0	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	82.239.87.25	58514	79.107.90.25	3306	query	select @@version_comment limit 1	T	0	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	82.239.87.25	58514	79.107.90.25	3306	query	select now()	T	0	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	82.239.87.25	58514	79.107.90.25	3306	quit	(empty)	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/mysql.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	mysql
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	cmd	arg	success	rows	response
+#types	time	string	addr	port	addr	port	string	string	bool	count	string
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59586	127.0.0.1	3306	login	root	T	0	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59586	127.0.0.1	3306	query	select @@version_comment limit 1	T	0	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59586	127.0.0.1	3306	query	SHOW ENGINE INNODB STATUS	T	0	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	127.0.0.1	59586	127.0.0.1	3306	quit	(empty)	-	-	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/out

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+mysql ok, 0
+mysql request, 3, select @@version_comment limit 1
+mysql eof, T
+mysql result row, 1, MySQL Community Server - GPL
+mysql eof, F
+mysql request, 3, SHOW ENGINE INNODB STATUS
+mysql eof, T
+mysql result row, 3, InnoDB
+mysql eof, F
+mysql request, 1, 

testing/btest/Baseline/scripts.base.protocols.mysql.wireshark/out

@@ -1,57 +1,49 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 mysql ok, 0
 mysql request, 3, select @@version_comment limit 1
-mysql ok, 0
-mysql ok, 0
-mysql ok, 0
+mysql eof, T
 mysql result row, [Gentoo Linux mysql-5.0.54]
-mysql ok, 0
+mysql eof, F
 mysql request, 3, SELECT DATABASE()
-mysql ok, 0
-mysql ok, 0
+mysql eof, T
 mysql result row, []
-mysql ok, 0
+mysql eof, F
 mysql request, 2, test
 mysql ok, 0
 mysql request, 3, show databases
-mysql ok, 0
-mysql ok, 0
+mysql eof, T
 mysql result row, [information_schema]
 mysql result row, [test]
-mysql ok, 0
+mysql eof, F
 mysql request, 3, show tables
-mysql ok, 0
-mysql ok, 0
+mysql eof, T
 mysql result row, [agent]
-mysql ok, 0
+mysql eof, F
 mysql request, 4, agent\x00
-mysql ok, 0
+mysql eof, F
 mysql request, 3, create table foo (id BIGINT( 10 ) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, animal VARCHAR(64) NOT NULL, name VARCHAR(64) NULL DEFAULT NULL) ENGINE = MYISAM
 mysql ok, 0
 mysql request, 3, insert into foo (animal, name) values ("dog", "Goofy")
 mysql ok, 1
 mysql request, 3, insert into foo (animal, name) values ("cat", "Garfield")
 mysql ok, 1
 mysql request, 3, select * from foo
-mysql ok, 0
-mysql ok, 0
+mysql eof, T
 mysql result row, [1, dog, Goofy]
 mysql result row, [2, cat, Garfield]
-mysql ok, 0
+mysql eof, F
 mysql request, 3, delete from foo where name like '%oo%'
 mysql ok, 1
 mysql request, 3, delete from foo where id = 1
 mysql ok, 0
 mysql request, 3, select count(*) from foo
-mysql ok, 0
-mysql ok, 0
+mysql eof, T
 mysql result row, [1]
-mysql ok, 0
+mysql eof, F
 mysql request, 3, select * from foo
-mysql ok, 0
-mysql ok, 0
+mysql eof, T
 mysql result row, [2, cat, Garfield]
-mysql ok, 0
+mysql eof, F
 mysql request, 3, delete from foo
 mysql ok, 1
 mysql request, 3, drop table foo