IMAP protocol parser, logger and sticky buffers v6 by glongo · Pull Request #15400 · OISF/suricata

glongo · 2026-05-16T15:32:18Z

Changes:

email object inside imap event is no longer logged. Existing email code will be refactored after this PR is merged in order to introduce a dedicated email event.
Updated SID range
Bounded missing request/response.

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

Previous PR: #15209

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 · 2026-05-16T16:01:44Z

Codecov Report

❌ Patch coverage is 81.85373% with 464 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.61%. Comparing base (bb4e79c) to head (e6cca48).
⚠️ Report is 86 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15400      +/-   ##
==========================================
- Coverage   82.65%   82.61%   -0.04%     
==========================================
  Files         996      999       +3     
  Lines      271109   273624    +2515     
==========================================
+ Hits       224076   226057    +1981     
- Misses      47033    47567     +534

Flag	Coverage Δ
fuzzcorpus	`60.50% <15.04%> (-0.52%)`	⬇️
livemode	`18.28% <11.91%> (-0.09%)`	⬇️
netns	`22.45% <11.91%> (-0.17%)`	⬇️
pcap	`45.37% <63.00%> (+0.16%)`	⬆️
suricata-verify	`66.46% <74.97%> (+0.05%)`	⬆️
unittests	`58.51% <53.46%> (-0.05%)`	⬇️

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 · 2026-05-16T17:05:03Z

Information: QA ran without warnings.

Pipeline = 31514

catenacyber · 2026-05-21T08:25:13Z

Needs a rebase now

catenacyber

Thanks for the work, needs a rebase

CI/QA : ✅
Git ID set : looks fine for me
CLA : you already contributed
Doc update : nice :-)
Redmine ticket : ok, should https://redmine.openinfosecfoundation.org/issues/3244 be closed as duplicate ?
Rustfmt : 🟡 please add the check in CI see .github/workflows/builds.yml: - run: rustfmt --check rust/src/dns/*.rs
Tests : some unanswered questions on SV PR like OISF/suricata-verify#2908 (comment)
Dependencies added: none
Code : looking
Commits segmentation : I would squash parser and logger but ok
Commit messages : cool

victorjulien · 2026-05-28T07:45:42Z

In the new PR, please add a explanation of the transaction state machine(s) and life cycle, including how transactions and other data structures are bounded.

catenacyber · 2026-05-28T07:48:13Z

+    pub direction: u8,
+}
+
+pub fn extract_command_from_requests(requests: &[ImapMessage]) -> Vec<u8> {


Does this need to be pub ? (I do not think so)

catenacyber · 2026-05-28T07:49:00Z

+pub fn extract_command_from_requests(requests: &[ImapMessage]) -> Vec<u8> {
+    for req in requests {
+        if let ImapMessageType::Command { command, arguments } = &req.message {
+            return if matches!(command, ImapCommand::Uid) {


Why is uid different ? please comment in the code ;-)

catenacyber · 2026-05-28T07:49:36Z

+            return if matches!(command, ImapCommand::Uid) {
+                arguments
+                    .first()
+                    .map(|a| String::from_utf8_lossy(a).to_lowercase().into_bytes())


Why converting to string and then back to Vec with loss on the way ?

Ops, forgot to remove the string part.

catenacyber · 2026-05-28T07:50:43Z

+                    .map(|a| String::from_utf8_lossy(a).to_lowercase().into_bytes())
+                    .unwrap_or_default()
+            } else {
+                command.to_string().to_lowercase().into_bytes()


Why lowercasing ?

Because in that way the sticky buffer can match with a single case.

I think we should preserve casing, and let rule writers use to_lowercase transform if they want

catenacyber · 2026-05-28T07:51:08Z

+}
+
+impl ImapTransaction {
+    pub fn new() -> ImapTransaction {


Can we not just derive Default ?

catenacyber · 2026-05-28T07:54:28Z

+                            .iter()
+                            .map(|a| String::from_utf8_lossy(a))
+                            .collect();
+                        js.append_string(&format!("{} {} {}", tag, cmd, args.join(" ")))?;


Still feels strange to recompute what we have seen on the wire...

I think I can just log the raw_line directly.

catenacyber · 2026-05-28T07:54:59Z

+            } = &response.message
+            {
+                if let Some(merged) = fetch.get_email() {
+                    let _message_email = Some(merged);


Why this unused code ?

catenacyber · 2026-05-28T07:59:31Z

+        }
+        let hname = CStr::from_ptr(hname);
+        if let Ok(hname_str) = hname.to_str() {
+            let normalized = hname_str.to_lowercase().replace('-', "_");


Why this normalization from message-id to message_id ?

Also, I do not see SV tests for this normlaization for x-mailer for example

Following the same convention used by SMTP to log email events.

But this is not logging, this is detection

Is IMAP using x_mailer when SMTP is using x-mailer in the network traffic ?

catenacyber · 2026-05-28T08:00:59Z

+        }
+        let hname = CStr::from_ptr(hname);
+        if let Ok(hname_str) = hname.to_str() {
+            let normalized = hname_str.to_lowercase().replace('-', "_");


unused, right ?

used in line 201.

I mean you can use hname_str straight away as the only caller of SCDetectImapEmailGetDataArray passes a string that is already "normalized"

catenacyber · 2026-05-28T08:01:40Z

+
 email.body_md5
 --------------



Works only for SMTP, right ?

So, please add it as you did for the other keywords ;-)

catenacyber · 2026-05-28T08:02:43Z

+email.body
+----------
+
+Matches on the body content of an email transferred over IMAP.


There is a limit of the size of the body we inspect, right ?
So, I think it should be mentioned in the docs

Yes, the limit is applied when an email is parsed.

glongo added 4 commits May 16, 2026 10:40

imap: implement parser

8b88a96

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

imap: implement logger

a581338

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

doc: add imap parser and sticky buffers

e6cca48

glongo requested review from a team, jasonish, jufajardini and victorjulien as code owners May 16, 2026 15:32

glongo mentioned this pull request May 16, 2026

IMAP protocol parser, logger and sticky buffers v5 #15209

Closed

catenacyber added the needs rebase Needs rebase to main label May 21, 2026

catenacyber requested changes May 28, 2026

View reviewed changes

catenacyber reviewed May 28, 2026

View reviewed changes

Conversation

glongo commented May 16, 2026

Uh oh!

codecov Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

suricata-qa commented May 16, 2026

Uh oh!

catenacyber commented May 21, 2026

Uh oh!

catenacyber left a comment

Choose a reason for hiding this comment

Uh oh!

victorjulien commented May 28, 2026

Uh oh!

catenacyber May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

codecov Bot commented May 16, 2026 •

edited

Loading

catenacyber May 28, 2026 •

edited

Loading