Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pgsql: don't error out with PDU parsing errors - v1 #12543

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jufajardini
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/5524

This isn't fully ready yet, the different errors should lead to pgsql events.
I'd also like to better handle the length error handling, to be able to actually capture that when parsing requests or responses -- and set the right event.

Describe changes:

  • allow parsing response messages that have the minimum length
  • don't return AppLayerResult::err() for parsing errors, to allow the parser to reach further messages
  • simplify responses parsing
  • don't set app-proto to failure when unknown messages are parsed -- to allow the parser to check further messages

Provide values to any of the below to override the defaults.

SV_BRANCH=OISF/suricata-verify#2276

jufajardini and others added 4 commits February 7, 2025 18:53
Some backend messages can be the shortest pgsql length possible,
4 bytes, but the parser expectd all messages to be longer than that.
The initial parsing for message type checking was more complex than
needed be.

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

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

Task OISF#5524
@jufajardini
Copy link
Contributor Author

TODO: edit commit messages, since this isn't a task, but a bug-fix

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24669

@victorjulien victorjulien marked this pull request as draft February 10, 2025 08:12
@victorjulien
Copy link
Member

Think this is looking good so far. Looking forward to the next round :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants