Skip to content

imap: extend detection patterns - v7#11219

Closed
mmaatuq wants to merge 1 commit intoOISF:masterfrom
mmaatuq:imap-bug-2886-v7
Closed

imap: extend detection patterns - v7#11219
mmaatuq wants to merge 1 commit intoOISF:masterfrom
mmaatuq:imap-bug-2886-v7

Conversation

@mmaatuq
Copy link
Contributor

@mmaatuq mmaatuq commented Jun 2, 2024

Ticket: #2886

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

Link to redmine ticket:2886

Describe changes:

  • extend detection patterns for imap protocol as per rfc9051
  • compared to this previous PR: rebase to latest master.
  • this is not comprehensive and might create more false positives, but i think this tradeoff is acceptable, and we can overcome these limitations when we add a complete parser.

SV_BRANCH=OISF/suricata-verify#1915

Ticket: OISF#2886

Signed-off-by: mmaatuq <mahmoudmatook.mm@gmail.com>
@github-actions
Copy link

github-actions bot commented Jun 3, 2024

NOTE: This PR may contain new authors.

@codecov
Copy link

codecov bot commented Jun 3, 2024

Codecov Report

Attention: Patch coverage is 70.58824% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 83.01%. Comparing base (3b1fecb) to head (648d288).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11219      +/-   ##
==========================================
- Coverage   83.02%   83.01%   -0.01%     
==========================================
  Files         942      943       +1     
  Lines      250606   250632      +26     
==========================================
+ Hits       208067   208074       +7     
- Misses      42539    42558      +19     
Flag Coverage Δ
fuzzcorpus 61.28% <70.58%> (+<0.01%) ⬆️
livemode 18.71% <70.58%> (+0.01%) ⬆️
pcap 44.54% <70.58%> (-0.03%) ⬇️
suricata-verify 61.71% <70.58%> (-0.01%) ⬇️
unittests 60.64% <70.58%> (+<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.flow.imap 0 402 -
SURI_TLPR1_stats_chk
.app_layer.flow.imap 0 231 -

Pipeline 20917

@victorjulien
Copy link
Member

@ct0br0 can you take a sample of both the TLPW1 and TLPR1 imap flows to see if they are really imap? For the TLPW1 pcaps it would be nice to turn a bunch of them into SV tests.

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

@mmaatuq for your info, this PR will probably get merged with #11261

Work is on our side now, we will let you know if we need more

Copy link
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

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

Thanks for the work :-)

This PR looks good to me, it will need a QA baseline update

  • CI : 🟢
  • Code : Looking good for me
  • Commits segmentation : ok
  • Commit messages : ok for me
  • Git ID set : looks fine for me
  • CLA : I do not have the access to check it
  • Doc update : not needed
  • Redmine ticket : ok
  • Rustfmt : not needed
  • Tests : SV ok
  • Dependencies added: none

Copy link
Contributor

@catenacyber catenacyber left a comment

Choose a reason for hiding this comment

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

I forgot to click on approve :-p

@victorjulien victorjulien added this to the 8.0 milestone Jul 17, 2024
@victorjulien
Copy link
Member

Merged in #11515, thanks!

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.

4 participants