Skip to content

IMAP protocol parser, logger and sticky buffers v5#15209

Closed
glongo wants to merge 4 commits into
OISF:mainfrom
glongo:dev-imap-proto-v5
Closed

IMAP protocol parser, logger and sticky buffers v5#15209
glongo wants to merge 4 commits into
OISF:mainfrom
glongo:dev-imap-proto-v5

Conversation

@glongo
Copy link
Copy Markdown
Contributor

@glongo glongo commented Apr 15, 2026

Changes:

  • Add missing imap.body and imap.headers frames in the doc
  • Remove direction, body and optional fields from schema.json
  • Add port-independent detection
  • Remove IncompleteData event
  • tx.requests and tx.responses are bounded
  • Remove GetImapEmailBodyData and use SCDetectImapEmailGetBody directly
  • Call SCAppLayerRequestProtocolTLSUpgrade when TLS is requested and ImapResponseStatus::Ok is seen

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

Previous PR: #15064

SV_BRANCH=OISF/suricata-verify#2908

glongo added 4 commits April 15, 2026 10:43
This introduces a parser for IMAP protocol.

Ticket OISF#8276
This introduces a logger for IMAP protocol.

Ticket OISF#8276
This implements the following sticky buffers for IMAP protocol:
- imap.request
- imap.response

The following frames have been added:
- imap.body
- imap.headers
- imap.pdu

The following email sticky buffers have been updated to work with IMAP:
- email.from
- email.subject
- email.to
- email.cc
- email.date
- email.message_id
- email.x_mailer

The following email sticky buffers have been added and are supported
only for IMAP:
- email.command
- email.body
- email.header
- email.header.name
- email.header.value

Ticket OISF#8276
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 81.63265% with 468 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.67%. Comparing base (be36e67) to head (e0cf729).
⚠️ Report is 82 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15209      +/-   ##
==========================================
- Coverage   82.69%   82.67%   -0.02%     
==========================================
  Files         993      996       +3     
  Lines      271697   274203    +2506     
==========================================
+ Hits       224667   226686    +2019     
- Misses      47030    47517     +487     
Flag Coverage Δ
fuzzcorpus 60.48% <12.61%> (-0.55%) ⬇️
livemode 18.26% <11.78%> (-0.10%) ⬇️
netns 22.44% <11.78%> (-0.17%) ⬇️
pcap 45.45% <63.75%> (+0.11%) ⬆️
suricata-verify 66.38% <75.49%> (+0.08%) ⬆️
unittests 58.79% <51.84%> (-0.07%) ⬇️

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@suricata-qa
Copy link
Copy Markdown

Information: QA ran without warnings.

Pipeline = 30954

@catenacyber
Copy link
Copy Markdown
Contributor

There are missing tickets that should relate to https://redmine.openinfosecfoundation.org/issues/8276

  • imap: add support for files
  • imap: add lua support

@catenacyber
Copy link
Copy Markdown
Contributor

There are unanswered questions in the previous PR like #15064 (comment)

Comment thread rust/src/imap/logger.rs

if let Some(email_data) = message_email {
let command_bytes = extract_command_from_requests(&tx.requests);
let command = String::from_utf8_lossy(&command_bytes).into_owned();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As proposed in #15064 (comment), I checked :

diff --git a/rust/src/imap/logger.rs b/rust/src/imap/logger.rs
index 1f7d91bf5c..54eeae7b2f 100644
--- a/rust/src/imap/logger.rs
+++ b/rust/src/imap/logger.rs
@@ -126,7 +126,7 @@ fn log_imap(tx: &ImapTransaction, js: &mut JsonBuilder) -> Result<(), JsonError>
 
     if let Some(email_data) = message_email {
         let command_bytes = extract_command_from_requests(&tx.requests);
-        let command = String::from_utf8_lossy(&command_bytes).into_owned();
+        let command = String::from_utf8_lossy(&command_bytes);
 
         js.open_object("email")?;
         js.set_string("command", &command)?;

compiles for me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ops, missed that.

@catenacyber
Copy link
Copy Markdown
Contributor

Main concern in previous remarks was bounding the stateful structures ;-)

Comment thread rust/src/imap/imap.rs
tx.parsed_email = Some(parsed_email);
}
}
tx.responses.push(response);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this one bounded ?

Copy link
Copy Markdown
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.

needs a rebase now for the rules range

@glongo
Copy link
Copy Markdown
Contributor Author

glongo commented May 16, 2026

Replaced with #15400

@glongo glongo closed this May 16, 2026
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.

4 participants