next/733/20250219/v1 #12625

victorjulien · 2025-02-19T11:33:40Z

Staging:

For setting s->init_data for flowbit rules impacted by `set`, we can first check whether this will be needed, and *then* walk over the flowbits array.

Flowbits can make a rule such as a packet rule be treated as a stateful rule, without actually changing the rule type. Add a flag to allow reporting such cases via engine analysis. Task OISF#7456

Ticket: 7560 When passing INT32_MIN aka 0x80000000, we cannot compute -vali as it does not fit into a i32

for flow.pkts and flow.bytes keywords Ticket: 7562 Avoid null deref when parsing flow.bytes:toserver;

Remove comparisons with true or false with C bools.

Some backend messages can be the shortest pgsql length possible, 4 bytes, but the parser expectd all messages to be longer than that. Related to Bug OISF#5524

The initial parsing for message type checking was more complex than needed be. Related to Bug OISF#5524

Building on top of work done by Jason Ish. Related to Bug OISF#5524

Some inner parsers were using it, some weren't. Better to standardize this. Also take the time to avoid magic numbers for representing the expected lengths for pgsql PDUs. Also throwing PgsqlParseError and allowing for incomplete results. Related to Task OISF#5566 Bug OISF#5524

Even if unknown, if the message is properly parsed, allow the parser to proceed. Related to Bug OISF#5524

This allows the app-proto to continue onto parsing next PDUs, if possible. Bug OISF#5524

Events for: - parsing error when parsing pgsql packet length - parsing error for pgsql requests (post length parsing) - parsing error for pgsql responses (post length parsing) - too many transactions Include `pgsql-events.rules` file, and PGSQL events SID range definition Task OISF#5566

No state change, but since we added Unknown responses, we should handle that case -- should we have a specific state for such cases? Related to Bug OISF#5524 Task OISF#5566

This may happen in some situations if the app-layer parser only sees unknown messages and sets an event: there will be an empty transaction, but nothing to log. Related to Task OISF#5566

Cf https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

Use the expanded form of localstatedir in autoconf.h instead of the unexpanded one, the difference being: #define LOCAL_STATE_DIR "${prefix}/var" and #define LOCAL_STATE_DIR "/usr/local/var" assuming default ./configure arguments. Fixes commit b6a610d.

codecov · 2025-02-19T12:09:37Z

Codecov Report

Attention: Patch coverage is 89.26014% with 45 lines in your changes missing coverage. Please review.

Project coverage is 80.77%. Comparing base (10ede91) to head (6fc617c).
Report is 19 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12625      +/-   ##
==========================================
+ Coverage   80.74%   80.77%   +0.03%     
==========================================
  Files         931      932       +1     
  Lines      259144   259286     +142     
==========================================
+ Hits       209242   209437     +195     
+ Misses      49902    49849      -53

Flag	Coverage Δ
fuzzcorpus	`57.01% <61.21%> (+0.04%)`	⬆️
livemode	`19.37% <7.38%> (-0.02%)`	⬇️
pcap	`44.15% <16.62%> (-0.01%)`	⬇️
suricata-verify	`63.48% <85.86%> (+0.04%)`	⬆️
unittests	`58.34% <41.05%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

catenacyber

CI is green
Consistent with approved PRs
SV PR matches all the needed individual SV PRs

suricata-qa · 2025-02-19T15:15:03Z

Information: QA ran without warnings.

Pipeline 24784

jufajardini and others added 19 commits February 19, 2025 09:21

detect/flowbits: only walk over array if needed

576f2ed

For setting s->init_data for flowbit rules impacted by `set`, we can first check whether this will be needed, and *then* walk over the flowbits array.

analysis: report rule state altered by other rule

06b506e

Flowbits can make a rule such as a packet rule be treated as a stateful rule, without actually changing the rule type. Add a flag to allow reporting such cases via engine analysis. Task OISF#7456

detect/krb5: avoid integer underflow with krb5.ticket_encryption

8ae5665

Ticket: 7560 When passing INT32_MIN aka 0x80000000, we cannot compute -vali as it does not fit into a i32

detect/flow: move keyword parsing code to rust

b514b25

for flow.pkts and flow.bytes keywords Ticket: 7562 Avoid null deref when parsing flow.bytes:toserver;

gen/bool: Clarify bool checks

7d5c170

Remove comparisons with true or false with C bools.

gen/typo: Improve grammar

eeb2166

pgsql/parser: fix response message length

24375a1

Some backend messages can be the shortest pgsql length possible, 4 bytes, but the parser expectd all messages to be longer than that. Related to Bug OISF#5524

pgsql/parser: simplify response parsing

737fea7

The initial parsing for message type checking was more complex than needed be. Related to Bug OISF#5524

pgsql: add PgsqlParserError

29d3aa7

Building on top of work done by Jason Ish. Related to Bug OISF#5524

pgsql: don't fail if proto parses unknown message

4fed424

Even if unknown, if the message is properly parsed, allow the parser to proceed. Related to Bug OISF#5524

pgsql: don't always return error for parsing errors

ff8d4e9

This allows the app-proto to continue onto parsing next PDUs, if possible. Bug OISF#5524

pgsql: apply rustfmt changes

38f8508

pgsql/parser: add more debug statements

6eadb75

pgsql: add case for Unknown response message state

d676d71

No state change, but since we added Unknown responses, we should handle that case -- should we have a specific state for such cases? Related to Bug OISF#5524 Task OISF#5566

pgsql/logger: don't log empty transactions

a4ce9cf

This may happen in some situations if the app-layer parser only sees unknown messages and sets an event: there will be an empty transaction, but nothing to log. Related to Task OISF#5566

pgsql/parser: fix type complexity clippy warning

ae00181

Cf https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

victorjulien requested review from jufajardini, jasonish and a team as code owners February 19, 2025 11:33

catenacyber approved these changes Feb 19, 2025

View reviewed changes

victorjulien merged commit 6fc617c into OISF:master Feb 19, 2025
60 checks passed

This was referenced Feb 19, 2025

pgsql: don't error out with PDU parsing errors - v3 #12614

Closed

build: use expanded localstatedir for #define #12619

Closed

victorjulien deleted the next/733/20250219/v1 branch February 19, 2025 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

next/733/20250219/v1 #12625

next/733/20250219/v1 #12625

victorjulien commented Feb 19, 2025

codecov bot commented Feb 19, 2025 •

edited

Loading

catenacyber left a comment

suricata-qa commented Feb 19, 2025

next/733/20250219/v1 #12625

next/733/20250219/v1 #12625

Conversation

victorjulien commented Feb 19, 2025

codecov bot commented Feb 19, 2025 • edited Loading

Codecov Report

catenacyber left a comment

Choose a reason for hiding this comment

suricata-qa commented Feb 19, 2025

codecov bot commented Feb 19, 2025 •

edited

Loading