Skip to content

Smtp server detection 1125 v2.4#11314

Closed
catenacyber wants to merge 5 commits intoOISF:masterfrom
catenacyber:smtp-server-detection-1125-v2.4
Closed

Smtp server detection 1125 v2.4#11314
catenacyber wants to merge 5 commits intoOISF:masterfrom
catenacyber:smtp-server-detection-1125-v2.4

Conversation

@catenacyber
Copy link
Contributor

Link to ticket: https://redmine.openinfosecfoundation.org/issues/
https://redmine.openinfosecfoundation.org/issues/1125
https://redmine.openinfosecfoundation.org/issues/6821
https://redmine.openinfosecfoundation.org/issues/5491

Describe changes:

  • smtp server detection (ie to_client)
  • ftp server detection (ie to_client)
  • smtp recognize more reply codes

SV_BRANCH=OISF/suricata-verify#1894

#11261 with 220 pattern being less prone to qualify flow as SMTP when it is FTP.

Diff with latest PR is more precisely :

diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c
index d7cb889b57..4fb17e56f6 100644
--- a/src/app-layer-smtp.c
+++ b/src/app-layer-smtp.c
@@ -1744,10 +1744,6 @@ static int SMTPRegisterPatternsForProtocolDetection(void)
         AppLayerProtoDetectPPRegister(IPPROTO_TCP, "25", ALPROTO_SMTP, 0, 5, STREAM_TOSERVER, NULL,
                 SMTPServerProbingParser);
     }
-    if (AppLayerProtoDetectPMRegisterPatternCSwPP(IPPROTO_TCP, ALPROTO_SMTP, "220 ", 4, 0,
-                STREAM_TOCLIENT, SMTPServerProbingParser, 5, 5) < 0) {
-        return -1;
-    }
     if (AppLayerProtoDetectPMRegisterPatternCSwPP(IPPROTO_TCP, ALPROTO_SMTP, "220-", 4, 0,
                 STREAM_TOCLIENT, SMTPServerProbingParser, 5, 5) < 0) {
         return -1;

@catenacyber catenacyber added the needs baseline update QA will need a new base line label Jun 17, 2024
@codecov
Copy link

codecov bot commented Jun 17, 2024

Codecov Report

Attention: Patch coverage is 86.04651% with 12 lines in your changes missing coverage. Please review.

Project coverage is 82.49%. Comparing base (49ecf37) to head (9b31258).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11314      +/-   ##
==========================================
+ Coverage   82.47%   82.49%   +0.02%     
==========================================
  Files         934      934              
  Lines      252270   252337      +67     
==========================================
+ Hits       208055   208167     +112     
+ Misses      44215    44170      -45     
Flag Coverage Δ
fuzzcorpus 60.27% <83.33%> (+0.01%) ⬆️
livemode 18.76% <13.88%> (+<0.01%) ⬆️
pcap 43.81% <81.94%> (+0.03%) ⬆️
suricata-verify 61.34% <80.55%> (+0.02%) ⬆️
unittests 59.92% <47.67%> (+<0.01%) ⬆️

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
.app_layer.error.smtp.parser 409 41 10.02%
SURI_TLPR1_stats_chk
.app_layer.flow.smtp 335817 347614 103.51%
.app_layer.flow.failed_tcp 178240 167266 93.84%
.app_layer.tx.ftp 101030 95384 94.41%
.app_layer.error.smtp.parser 527 144 27.32%
.ftp.memuse 10637 2906 27.32%

Pipeline 21112

@suricata-qa
Copy link

WARNING:

field baseline test %
SURI_TLPW1_stats_chk
.app_layer.error.smtp.parser 409 42 10.27%
SURI_TLPR1_stats_chk
.app_layer.flow.smtp 335817 347580 103.5%
.app_layer.flow.failed_tcp 178240 167204 93.81%
.app_layer.tx.ftp 101030 95372 94.4%
.app_layer.error.smtp.parser 527 144 27.32%
.ftp.memuse 10637 2878 27.06%

Pipeline 21132

@ct0br0
Copy link

ct0br0 commented Jun 19, 2024

If you could make a branch to print IP tuple for ftp tx that would help split pcaps for this. Thanks

@catenacyber
Copy link
Contributor Author

Continued in #11327

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

Labels

needs baseline update QA will need a new base line

Development

Successfully merging this pull request may close these issues.

3 participants