Skip to content

Smtp server detection 1125 v14#8696

Closed
catenacyber wants to merge 5 commits intoOISF:masterfrom
catenacyber:smtp-server-detection-1125-v14
Closed

Smtp server detection 1125 v14#8696
catenacyber wants to merge 5 commits intoOISF:masterfrom
catenacyber:smtp-server-detection-1125-v14

Conversation

@catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/1125 preliminary work

Describe changes:

  • protocoldetect: use stricter max depth
  • pop3: add detection

Modifies #8512 by only taking some commits

Draft : waiting for QA

For probing parsers used with a pattern, restrict the max depth
to these probing parsers and not all probing parsers.

Finishing protocol detection earlier allows to parse data earlier
in the case we recognize only one side.
When an app-layer is recognized for one side, but the other
is still unknown, and the parsing of the one side errors,
it disables app-layer, and thus the ending pseudo-packets do
not get a chance to conclude for app-layer detection on the
unknown side.

So count in stats the app-layer protocol that was recognized
the first time.
When probing parser is only tried from one recognized side
of the protocol, even if this parser failed, it never set
probing parser done as no mask were used.
@codecov
Copy link

codecov bot commented Apr 7, 2023

Codecov Report

Merging #8696 (708c057) into master (9adb59b) will decrease coverage by 0.01%.
The diff coverage is 70.37%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8696      +/-   ##
==========================================
- Coverage   81.86%   81.85%   -0.01%     
==========================================
  Files         968      968              
  Lines      279018   279042      +24     
==========================================
- Hits       228419   228416       -3     
- Misses      50599    50626      +27     
Flag Coverage Δ
fuzzcorpus 64.23% <70.37%> (+<0.01%) ⬆️
suricata-verify 59.78% <59.25%> (-0.02%) ⬇️
unittests 63.22% <40.74%> (-0.02%) ⬇️

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

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.tcp.rst 113181 145067 128.17%
.app_layer.tx.ftp 646 187 28.95%
.app_layer.error.ftp.gap 2 0 -
.app_layer.error.ftp.parser 2 0 -
.ftp.memuse 348 3 0.86%
IPS_AFP_stats_chk
.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 18444 0 -
.app_layer.tx.ftp 73776 0 -
.app_layer.error.ftp.parser 18444 0 -

Pipeline 13059

@catenacyber catenacyber marked this pull request as ready for review April 27, 2023 15:10
@catenacyber
Copy link
Contributor Author

@ct0br0 how can we confirm that the QA ftp changes are related to in fact pop3 ?

@ct0br0
Copy link

ct0br0 commented Apr 28, 2023

After taking a look at TLPW1 it looks like POP3 counters aren't incremented at all?

tcpdump -nr tlpw1_smtp_pop3.pcap "(port 110 or port 995)" | wc -l
reading from file tlpw1_smtp_pop3.pcap, link-type EN10MB (Ethernet), snapshot length 262144
201521

I'll send you a link with ftp+data/email pcaps.

@catenacyber
Copy link
Contributor Author

Thanks Corey, continuing in #8804

@catenacyber catenacyber closed this May 4, 2023
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