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

PostgreSQL: initial support (MVP) #5824

Closed

Conversation

jufajardini
Copy link
Contributor

PostgreSQL: add SSL handshake support

Make sure these boxes are signed before submitting your Pull Request -- thank you.

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

Describe changes:

  • PostgreSQL: this is an attempt to have the SSL handshake part of the postgresql FE/BE protocol covered, using the aid of the setup-applayer-py script, as a means to understand how the whole idea of parsing, identifying and processing packets/stream works, with the minimum real-case scenario I could think of. This isn't complete yet, but parses the possible request (SSL encrytion request message) and two of the three possible responses ('S' or 'N', not covering the case of an error message, yet). There is some tentative work with the probing phase, and although there is an error type, it is not being properly used to let the engine know when it show give up and when it should wait for more data.

#suricata-verify-pr:
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch:

PostgreSQL: add SSL handshake support
@codecov
Copy link

codecov bot commented Feb 7, 2021

Codecov Report

Merging #5824 (7b9a1ea) into master (62e665c) will decrease coverage by 0.00%.
The diff coverage is 59.75%.

@@            Coverage Diff             @@
##           master    #5824      +/-   ##
==========================================
- Coverage   72.38%   72.38%   -0.01%     
==========================================
  Files         604      606       +2     
  Lines      179369   179453      +84     
==========================================
+ Hits       129837   129894      +57     
- Misses      49532    49559      +27     
Flag Coverage Δ
suricata-verify 49.16% <58.02%> (+0.01%) ⬆️
unittests 63.05% <14.63%> (-0.03%) ⬇️

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

@victorjulien
Copy link
Member

Do you have a Suricata-Verify test that goes with this?

If not, a pcap to play with it?

@jufajardini
Copy link
Contributor Author

Oh, I haven't thought of adding the suricata-verify, I'll do that next time.

Here are two pcaps, I don't know if this is doing much, already, though:

named!(pub parse_message<String>,
do_parse!(
len: map_res!(
map_res!(take_until!(":"), std::str::from_utf8), parse_len) >>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not understanding what is happening here. It parses the 4 byte length field, but then also takes something until a : is found? Shouldn't we just to something like take!(len)?

When testing with postgresql-13-simple-select.pcap I see this return incomplete all the time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, that's my fault, I haven't worked on this function, but now I can see that I certainly should, before sharing. (This also helps me understand when I can expect to see things "changing" if I run suricata after changing things here, and when not much will happen...) I will do that, and submit a new PR.

@jufajardini
Copy link
Contributor Author

Closed with: #5830

@jufajardini jufajardini closed this Feb 9, 2021
@jufajardini jufajardini deleted the pgsql-mvp-ssl-handshake-v0 branch January 21, 2022 15:14
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.

2 participants