pgsql: add query keyword - v1 #13312
Conversation
Add the `pgsql.query` rule keyword to match on PGSQL's `simple query` request messages. This matches on the EVE: pgsql.request.simple_query as of now. `pgsql.query` is a sticky buffer and can be used as a fast_pattern. Task OISF#6259
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13312 +/- ##
==========================================
+ Coverage 83.41% 83.42% +0.01%
==========================================
Files 995 996 +1
Lines 272813 272849 +36
==========================================
+ Hits 227558 227620 +62
+ Misses 45255 45229 -26
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Information: QA ran without warnings. Pipeline 26213 |
| ) -> bool { | ||
| let tx = cast_pointer!(tx, PgsqlTransaction); | ||
|
|
||
| *buffer = std::ptr::null(); |
There was a problem hiding this comment.
nit: not sure if the compiler optimizes it, but we do a useless reset here in the (common?) success case
| This keyword is a sticky buffer that allows matching on the contents of `Simple` | ||
| `query` PostgreSQL request messages parsed by the engine. | ||
|
|
||
| As such, it matches on the ``pgsql.request.simple_query`` field from EVE output. |
There was a problem hiding this comment.
should the keyword be simple_query if there is later also non-simple query type coming?
There was a problem hiding this comment.
Good question... with the extended query, the query is sent as part of the Parse message. (cf https://www.postgresql.org/docs/13/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY)
I mentioned simple_query here thinking about the rules keyword/ log output parity.
It feels more straightforward to have this as simple_query and then maybe have the other as parse.query or extended_query and maybe have one that could unify both, in the future?
There was a problem hiding this comment.
heh, the simple query postgresql's message is called "Query" in their docs (https://www.postgresql.org/docs/17/protocol-message-formats.html#PROTOCOL-MESSAGE-FORMATS-QUERY)
There was a problem hiding this comment.
I'd try to stick with the Postgres documentation naming, as its exists and is rather complete. Rather than what we see in Wireshark. So this would just become query?
There was a problem hiding this comment.
It's a bit confusing to me, as the docs uses both-ish (?):
Query (F)
Byte1('Q')
Identifies the message as a simple query.
There was a problem hiding this comment.
Maybe we could just use pgsql.request.query and indicate that it matches on query strings? For now, that would mean the Query from the Simple query sub-proto, but when extended query is added, that would also be the Query seen in the Parse message.
There was a problem hiding this comment.
We're deciding to move on with using query which could be then possibly also used to inspect extended query queries
|
Followed by: #13325 |
Add the
pgsql.queryrule keyword to match on PGSQL'ssimple querymessages.pgsql.queryis a sticky buffer and can be used as a fast_pattern.Task #6259
Changes (if applicable):
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/6259
Describe changes:
querykeyword topgsql, matching on the pgsql.request.simple_query EVE fieldProvide values to any of the below to override the defaults.
SV_BRANCH=OISF/suricata-verify#2524