Skip to content

Pop3 protocol detection 6366 v2#9836

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

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

Conversation

@catenacyber
Copy link
Contributor

@catenacyber catenacyber commented Nov 19, 2023

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#1481

SV_BRANCH=pr/1481

Rebase of #9500 to rerun QA

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 Nov 19, 2023

Codecov Report

Merging #9836 (f4fa193) into master (d2b25af) will increase coverage by 0.00%.
The diff coverage is 71.42%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #9836   +/-   ##
=======================================
  Coverage   82.42%   82.42%           
=======================================
  Files         972      972           
  Lines      273929   273939   +10     
=======================================
+ Hits       225788   225800   +12     
+ Misses      48141    48139    -2     
Flag Coverage Δ
fuzzcorpus 64.32% <71.42%> (+0.03%) ⬆️
suricata-verify 61.08% <71.42%> (-0.02%) ⬇️
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

ERROR:

ERROR: QA failed on ASAN_TLPR1_suri.

Pipeline 16673

@catenacyber
Copy link
Contributor Author

ERROR: QA failed on ASAN_TLPR1_suri.

Need rebase after merge of #9851

@catenacyber catenacyber added the needs rebase Needs rebase to main label Nov 21, 2023
@victorjulien
Copy link
Member

Did you see it triggers an assertion?

@victorjulien
Copy link
Member

suricata: output.c:1176: EveJsonSimpleAppLayerLogger *SCEveJsonSimpleGetLogger(AppProto): Assertion `!(simple_json_applayer_loggers[alproto].proto != alproto)' failed.
1038suricata: output.c:1176: EveJsonSimpleAppLayerLogger *SCEveJsonSimpleGetLogger(AppProto): Assertion `!(simple_json_applayer_loggers[alproto].proto != alproto)' failed.
1039suricata: output.c:1176: EveJsonSimpleAppLayerLogger *SCEveJsonSimpleGetLogger(AppProto): Assertion `!(simple_json_applayer_loggers[alproto].proto != alproto)' failed.
1040suricata: output.c:1176: EveJsonSimpleAppLayerLogger *SCEveJsonSimpleGetLogger(AppProto): Assertion `!(simple_json_applayer_loggers[alproto].proto != alproto)' failed.
1041suricata: output.c:1176: EveJsonSimpleAppLayerLogger *SCEveJsonSimpleGetLogger(AppProto): Assertion `!(simple_json_applayer_loggers[alproto].proto != alproto)' failed.

@catenacyber
Copy link
Contributor Author

Did you see it triggers an assertion?

Yes cf #9836 (comment) ;-p

Replaced by #9874

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

Labels

needs rebase Needs rebase to main

Development

Successfully merging this pull request may close these issues.

3 participants

Comments