From e4e56789db4952af76701417997f2c1bf605c742 Mon Sep 17 00:00:00 2001 From: Fupeng Zhao Date: Sat, 16 Aug 2025 08:23:55 +0000 Subject: [PATCH] Report PostgreSQL login success only after ReadyForQuery Previously, Zeek treated the receipt of `AuthenticationOk` as a successful login. However, according to the PostgreSQL Frontend/Backend Protocol, the startup phase is not complete until the server sends `ReadyForQuery`. It is still possible for the server to emit an `ErrorResponse` (e.g. ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION) after `AuthenticationOk` but before `ReadyForQuery`. This change updates the PostgreSQL analyzer to defer reporting login success until `ReadyForQuery` is observed. This prevents false positives in cases where authentication succeeds but session startup fails. --- scripts/base/protocols/postgresql/main.zeek | 13 +++++++++---- .../conn.cut | 3 +++ .../postgresql.cut | 4 ++++ .../Traces/postgresql/psql-login-no-role.pcap | Bin 0 -> 1567 bytes .../protocols/postgresql/psql-login-no-role.zeek | 12 ++++++++++++ 5 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/conn.cut create mode 100644 testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/postgresql.cut create mode 100644 testing/btest/Traces/postgresql/psql-login-no-role.pcap create mode 100644 testing/btest/scripts/base/protocols/postgresql/psql-login-no-role.zeek diff --git a/scripts/base/protocols/postgresql/main.zeek b/scripts/base/protocols/postgresql/main.zeek index bf262467d6..16ce4f6d3f 100644 --- a/scripts/base/protocols/postgresql/main.zeek +++ b/scripts/base/protocols/postgresql/main.zeek @@ -53,7 +53,7 @@ export { user: string &optional; database: string &optional; application_name: string &optional; - rows: count &default=0; + rows: count &optional; errors: vector of string; }; @@ -197,8 +197,6 @@ event PostgreSQL::authentication_ok(c: connection) { c$postgresql$backend = "auth_ok"; c$postgresql$success = T; - - emit_log(c); } event PostgreSQL::terminate(c: connection) { @@ -224,6 +222,9 @@ event PostgreSQL::simple_query(c: connection, query: string) { event PostgreSQL::data_row(c: connection, column_values: count) { hook set_session(c); + if ( ! c$postgresql_state?$rows ) + c$postgresql_state$rows = 0; + ++c$postgresql_state$rows; } @@ -236,7 +237,11 @@ event PostgreSQL::ready_for_query(c: connection, transaction_status: string) { if ( ! c$postgresql?$success ) c$postgresql$success = transaction_status == "I" || transaction_status == "T"; - c$postgresql$rows = c$postgresql_state$rows; + if ( c$postgresql_state?$rows ) { + c$postgresql$rows = c$postgresql_state$rows; + delete c$postgresql_state$rows; + } + emit_log(c); } diff --git a/testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/conn.cut b/testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/conn.cut new file mode 100644 index 0000000000..23728e7aec --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/conn.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 service +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 10.21.179.53 51625 192.168.115.201 5432 postgresql diff --git a/testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/postgresql.cut b/testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/postgresql.cut new file mode 100644 index 0000000000..66147ecc45 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.postgresql.psql-login-no-role/postgresql.cut @@ -0,0 +1,4 @@ +### 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 user database application_name frontend frontend_arg backend backend_arg success rows +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 10.21.179.53 51625 192.168.115.201 5432 - - - ssl_request - ssl_reply N F - +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 10.21.179.53 51625 192.168.115.201 5432 test postgres Navicat startup - auth_ok,error SeverityLocalized=FATAL,Severity=FATAL,Code=28000,Message=role "test" does not exist,File=miscinit.c,Line=694,Routine=InitializeSessionUserId F - diff --git a/testing/btest/Traces/postgresql/psql-login-no-role.pcap b/testing/btest/Traces/postgresql/psql-login-no-role.pcap new file mode 100644 index 0000000000000000000000000000000000000000..c65069ae52d514c3bf41e31e4ec6ae8874b059c3 GIT binary patch literal 1567 zcmd6mKWGzS7>9p%i8bhw3Mn0r;1H?USYwTYk}6gUvBXxzsqsv%=|P%=`>qmQ%p`bD zDpp)-ORY2rg8x7VAtERl>Yzc2B03nb;?M{#F7^HHvs&&#rwksv+*F~I6&XLw%@pa@5=Yul6C#mybp(fh|wKGscmv`Q{dt5$~>!9 z%Yo=+<=6;MD#^c=U&(;%-S9~AczlxNm3d;h<%!iV{}=_GAQg;24Ex&RKF-HE1JP=E z>%zATaVAS!41a9u4*KS9U{+g*1#x=vE+Si&z$jSIrvrIw^j zQp$ef;N@Zo8#`^5U{jV(S$a20*8rdQO-DEH>}E^THW#G7o?PZbmch+GpK+KUBlDSr zAN3#Cgr}~eggpew%ayXtVs^@{rYW7M-?o53-dgOO1d}3oIYa@YMvpCO)*&iGL(>x^ z>M;M~xn&P9!88M=#!RGDQ$3?H4TU^2Gls^X77AHCrJ8y^m&~c-8V1#~{482I@*gmy zJ7i2#E4*AfuUe+9(_Ys#>!8z6<`Sf{@*WtXn?5SO<2XIs-*>z(j+5ebuqPT0hjG-% zXEi0rB?XmqUSmowZz@_*Cu#lTI!oy}-Q1BvymxN|Lj!cs)vSJA8`c;jbtlO1Kw3EC zysOLR;tg37Ht&>U{BLdEcGMe7BIVhd6xZ}a#x8sMpB>Ymqv_XZWwHEPq*Pjx@?rgy oh|Th;DN8%CEC`m5ElK(LbUnLwN4VYT#}2zSvU`WGXyYsJ8~S<`egFUf literal 0 HcmV?d00001 diff --git a/testing/btest/scripts/base/protocols/postgresql/psql-login-no-role.zeek b/testing/btest/scripts/base/protocols/postgresql/psql-login-no-role.zeek new file mode 100644 index 0000000000..4591b7b86a --- /dev/null +++ b/testing/btest/scripts/base/protocols/postgresql/psql-login-no-role.zeek @@ -0,0 +1,12 @@ +# @TEST-DOC: Test Zeek parsing a trace file through the PostgreSQL analyzer. +# +# @TEST-REQUIRES: ${SCRIPTS}/have-spicy +# @TEST-EXEC: zeek -b -r ${TRACES}/postgresql/psql-login-no-role.pcap %INPUT >output +# @TEST-EXEC: zeek-cut -m ts uid id.orig_h id.orig_p id.resp_h id.resp_p service < conn.log > conn.cut +# @TEST-EXEC: zeek-cut -m < postgresql.log > postgresql.cut +# +# @TEST-EXEC: btest-diff conn.cut +# @TEST-EXEC: btest-diff postgresql.cut + +@load base/protocols/conn +@load base/protocols/postgresql