Skip to content

Comments

Pop3 protocol detection 6366 v1#9500

Closed
catenacyber wants to merge 2 commits intoOISF:masterfrom
catenacyber:pop3-protocol-detection-6366-v1
Closed

Pop3 protocol detection 6366 v1#9500
catenacyber wants to merge 2 commits intoOISF:masterfrom
catenacyber:pop3-protocol-detection-6366-v1

Conversation

@catenacyber
Copy link
Contributor

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

Describe changes:

  • pop3 protocol detection

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

OISF/suricata-verify#1389

SV_BRANCH=pr/1389

First preliminary part for #8892 and https://redmine.openinfosecfoundation.org/issues/1125

This will require a QA rebaseline

After that :

  • QA baseline is wrong because of counting IRC flows on port 5432 as pgsql because pgsql probing parser to client accepts anything
  • See first commits of Smtp server detection 1125 v17 #8892 about generic protocol detection and see if we can craft tests to identify these bugs
  • Make eve.json stats field about flows match the count of flow with app_proto because of so many corner cases
  • Add FTP and SMTP server side detection

@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #9500 (15a4b56) into master (af4bb91) will decrease coverage by 0.02%.
The diff coverage is 71.42%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9500      +/-   ##
==========================================
- Coverage   82.18%   82.17%   -0.02%     
==========================================
  Files         968      968              
  Lines      274203   274215      +12     
==========================================
- Hits       225352   225324      -28     
- Misses      48851    48891      +40     
Flag Coverage Δ
fuzzcorpus 64.04% <71.42%> (-0.04%) ⬇️
suricata-verify 60.89% <71.42%> (-0.01%) ⬇️
unittests 62.87% <50.00%> (-0.02%) ⬇️

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

@suricata-qa
Copy link

WARNING:

ERROR: QA failed on IPS_AFP_drop_chk.

field baseline test %
SURI_TLPW1_stats_chk
.app_layer.flow.ftp 52 43 82.69%
.app_layer.tx.ftp 819 188 22.95%
.app_layer.error.ftp.gap 2 0 -
.app_layer.error.ftp.parser 2 0 -
.ftp.memuse 348 3 0.86%
SURI_TLPR1_stats_chk
.ftp.memuse 11408 10660 93.44%
IPS_AFP_stats_chk
.ips.blocked 1395360 747360 53.56%
.ips.drop_reason.flow_drop 1296000 680400 52.5%
.ips.drop_reason.applayer_error 32400 0 -
.flow.end.state.established 583199 550799 94.44%
.flow.end.tcp_state.established 201960 169560 83.96%
.app_layer.flow.ftp 33480 1080 3.23%
.app_layer.tx.ftp 131760 2160 1.64%
.app_layer.error.ftp.parser 32400 0 -
TREX_GENERIC_stats_chk
.app_layer.flow.ftp 14871 0 -
.app_layer.tx.ftp 59484 0 -
.app_layer.error.ftp.parser 14871 0 -

Pipeline 16041

if (AppLayerProtoDetectConfProtoDetectionEnabled("tcp", "pop3")) {
if (AppLayerProtoDetectPMRegisterPatternCS(
IPPROTO_TCP, ALPROTO_POP3, "+OK ", 4, 0, STREAM_TOCLIENT) < 0) {
SCLogInfo("pop3 proto registration failure");
Copy link
Member

Choose a reason for hiding this comment

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

FatalError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice, also doing it for IMAP

@victorjulien
Copy link
Member

QA result seems very unhappy, we should probably see what a rerun does.

@catenacyber
Copy link
Contributor Author

QA result seems very unhappy, we should probably see what a rerun does.

There is some expectation as QA counts as FTP some POP3 flows (port 110)

Rebased in #9836

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