From 672602dae7ecff2039335bfa55ebef6fe6553dc8 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 24 Jan 2023 09:35:27 +0100 Subject: [PATCH] MySQL: Fix endianness, introduce mysql_eof() event 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. I think there is more buried, but this fixes the violation of the simple "SHOW ENGINE INNODB STATUS" and the existing tests continue to succeed... [1] https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_basic_dt_integers.html --- scripts/base/protocols/mysql/main.zeek | 19 +++++++++ src/analyzer/protocol/mysql/events.bif | 12 ++++++ .../protocol/mysql/mysql-analyzer.pac | 24 ++++++----- .../protocol/mysql/mysql-protocol.pac | 32 ++++++++++----- .../mysql.log | 14 +++++++ .../out | 11 +++++ .../out | 38 +++++++----------- ...show-engine-innodb-status-no-password.pcap | Bin 0 -> 7337 bytes .../mysql/show-engine-innodb-status.test | 32 +++++++++++++++ .../base/protocols/mysql/wireshark.test | 5 +++ 10 files changed, 144 insertions(+), 43 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/mysql.log create mode 100644 testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/out create mode 100644 testing/btest/Traces/mysql/mysql-show-engine-innodb-status-no-password.pcap create mode 100644 testing/btest/scripts/base/protocols/mysql/show-engine-innodb-status.test diff --git a/scripts/base/protocols/mysql/main.zeek b/scripts/base/protocols/mysql/main.zeek index 4ceda849f4..d6513b5123 100644 --- a/scripts/base/protocols/mysql/main.zeek +++ b/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 ) diff --git a/src/analyzer/protocol/mysql/events.bif b/src/analyzer/protocol/mysql/events.bif index 7ce65276a6..ec5fa61ae6 100644 --- a/src/analyzer/protocol/mysql/events.bif +++ b/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 `__ +## 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 `__ diff --git a/src/analyzer/protocol/mysql/mysql-analyzer.pac b/src/analyzer/protocol/mysql/mysql-analyzer.pac index 2e82bbc63c..b764dd3f63 100644 --- a/src/analyzer/protocol/mysql/mysql-analyzer.pac +++ b/src/analyzer/protocol/mysql/mysql-analyzer.pac @@ -65,19 +65,19 @@ refine flow MySQL_Flow += { return true; %} + function proc_eof_packet(msg: EOF_Packet): bool + %{ + 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 ( 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 ( ${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); }; diff --git a/src/analyzer/protocol/mysql/mysql-protocol.pac b/src/analyzer/protocol/mysql/mysql-protocol.pac index a54246ef9c..d404200976 100644 --- a/src/analyzer/protocol/mysql/mysql-protocol.pac +++ b/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,7 +279,7 @@ 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; }; @@ -281,7 +287,7 @@ 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); }; diff --git a/testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/mysql.log b/testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/mysql.log new file mode 100644 index 0000000000..fd95b799ff --- /dev/null +++ b/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 diff --git a/testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/out b/testing/btest/Baseline/scripts.base.protocols.mysql.show-engine-innodb-status/out new file mode 100644 index 0000000000..a489c2b740 --- /dev/null +++ b/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, diff --git a/testing/btest/Baseline/scripts.base.protocols.mysql.wireshark/out b/testing/btest/Baseline/scripts.base.protocols.mysql.wireshark/out index 1663ebbfa3..c5e9e7d901 100644 --- a/testing/btest/Baseline/scripts.base.protocols.mysql.wireshark/out +++ b/testing/btest/Baseline/scripts.base.protocols.mysql.wireshark/out @@ -1,31 +1,26 @@ ### 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") @@ -33,25 +28,22 @@ 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 diff --git a/testing/btest/Traces/mysql/mysql-show-engine-innodb-status-no-password.pcap b/testing/btest/Traces/mysql/mysql-show-engine-innodb-status-no-password.pcap new file mode 100644 index 0000000000000000000000000000000000000000..8d9cad506f42d3388adc00dd2c89b882948fa1ce GIT binary patch literal 7337 zcmdT|TZ|jk86NKqXIP-As1#KYPAatlvY#PwwELz*CuMYJ2#p9=T9H4PufB$ua z{Kl5Zf@XUj{dB&%=1FFXk-`n-)J9t$rJ;(D7JWyz;rJ;#%G%7`Ia%q+o!&G%`C_E~SO3`hFMoqTLx=|lm zjgA+_Bf||tuNZQjV-42V%!g{j1yeDwtWecWYnv3BfSZD&5}D)$n52i11tAhn@gT5V zlh{FU;^x6;&dkEd>ra2P!z2eeV%je(zy$Pp8xN~{!fB3J?TWa=xBeZy6Y)Ci2%ykM z;BNrDhqx3IKOyk*11`876K7_pPoB7qV&dszo#2G{!$rSP0Xlu&#$)18`KN? z+hF%4ZueY^?fqbTh!ja3@C&EFo<48mcK>epUXFNUS47zT&uLty&6jr%iO{y8C@`%70u z_;dUBUBsR5!Lav#77+cpx%2^+NbO6e5-gcYrDvvCE+5Y?=7`@J@av%gIWo)5-f;Oj zjywiOK2DDO*2^t^7muIX`5qyz9qSh^z)JLa8#!|D58)gk_W!0A;yV{R5y_GLP{VcP zKz^;EkneT^d)-IA!)(9f=7AIv5X}lh?(8jas53j1vt+AjcCK#ub78wMG?dgdeP&Ad z*VK0L?rUjcC>RQl1f>xv#-z|>cx*B>!K6rVJT?&u#X`|oNE&yIzK~8O^XV*0FXl6g z`D?__CBZCx%g<#K@tH0(rm0go!v$AV+?BRP9L7co={-drl#Q> z!wxqs;K7e`s%h!Q8j?N&tEOlbs|r)kCv$n0o@HKv*errN5ucu8xkN4p5hqD_4+V>a zLefMi91TxIBhpwXz%<<=#s)%I3?jygOXwQSqIJ7@M}Sooxxq~Jh{7Z(6s2%8^c7Bp z=`FCs;Yj4ag&i6TkAG}zz9+@mDH1{?b^j_iK8R8N*C3_*Cf`5fFN&OyM1~|GWJP;jDHjmoK=W!IVrjfcv(V4 zy}fL%X~jE;1S$H9?v85cdW|ZT6hwJPktnM*1=|mj1XyT%tTkkd5J3Hv_!xniB2Vp{ zdJpJ>sNYDhYbbtuXwXUa_EJtjm4M4(?&(s$ude6R7n7-6BAaJZi?g$dEQ_PG$7kZ1 zeDb~on~Ue>P%~!|2U~fj8$@eNOu}B1PTCFNV4>hp;pBcYxv5m_Nu=)}t1BomrpW)5 zn2!3!OEOH$oTP*$C0rydh2b)|LtfDvyZrNPR1d^k`KcRygY;>;}bE; z$|c1v8dVuJ-0>dOivQOI;r~4yB#%fuvSI5gqC%$3|M1qCf01YYw>mTbvv0eZ|8pO7 zp81Vp6`O?i3~dQb*G61k&J0DgIG^5U|HrmAsU&Jeg=x+DVU)sF3rkTc78#3(^PqC- zBpzvSI7IX{8_d#)v>R2s=DK7ZRJ3m`MQdZYEIX*|{NmxXi7H;AeNa*jTr(T|bnmkB z%9Y_WJvPG~qjs-{C3n7LzOD2~I---RGU@a@&sPhHg>?2%FY3H*A^+E8I?vXXI-N)58jg$n?h_Pe zR9r}OO~&x1*+?`P86Oj;?6VPNayw@*hKLA>vy|WLfN*u62u8x8|DUh15SL_*?%UQ){{dGN5iBG{@fJonp%Z4bKTYCgM2NzuL-@kC2W7F7?)jRFB> z$M$+%Ms{1nU8O?wbaHk&c7frY&fVe;IRGREi^V1atcag zwt?M3C8l(DBXOg0St|;TqYxfDuyq-27>Gf=X?dlPakMY-ZB5%-S%&{0&WdVPLCEA{ zQ8AIg!JekGxGv&$%fFMe&jfItIifZSt-&Ba0GrMF1HlI;i%kRn92(X! zcvZ}aS-g^@Gl^_`^DPV6CdiOs3>9Yu)Hf9#v0n_hm$mz}7CjGBa}Cb}@CtZS=0A$eel2xXj(=X2jobjd+G5UfGQJ;t?SJ?W=EXMm)DQ z;x9Sk`ewwJ=u!T{k{!rG6YFU=x&pMaq2%Xhk}6YQ=eAs*|ENDhCU3;ItN(Q)__ z( out +# @TEST-EXEC: btest-diff out +# @TEST-EXEC: btest-diff mysql.log + +@load base/protocols/mysql + +event mysql_ok(c: connection, affected_rows: count) + { + print "mysql ok", affected_rows; + } + +event mysql_eof(c: connection, is_intermediate: bool) + { + print "mysql eof", is_intermediate; + } + +event mysql_result_row(c: connection, row: string_vec) + { + print "mysql result row", |row|, row[0][:70]; + } + +event mysql_error(c: connection, code: count, msg: string) + { + print "mysql error", code, msg; + } + +event mysql_command_request(c: connection, command: count, arg: string) + { + print "mysql request", command, arg; + } diff --git a/testing/btest/scripts/base/protocols/mysql/wireshark.test b/testing/btest/scripts/base/protocols/mysql/wireshark.test index 64c8eb7ffa..fdf77cbbe6 100644 --- a/testing/btest/scripts/base/protocols/mysql/wireshark.test +++ b/testing/btest/scripts/base/protocols/mysql/wireshark.test @@ -11,6 +11,11 @@ event mysql_ok(c: connection, affected_rows: count) print "mysql ok", affected_rows; } +event mysql_eof(c: connection, is_intermediate: bool) + { + print "mysql eof", is_intermediate; + } + event mysql_result_row(c: connection, row: string_vec) { print "mysql result row", row;