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

glongo · 2026-03-19T13:41:43Z

Changes:

imap-event.rules added
headers and body fields bounded
sticky buffers renamed:
- imap.email.body -> email.body
- imap.email.header -> email.header
- imap.email.header.name -> email.header.name
- imap.email.header.value -> email.header.value
imap.email.direction replaced with email.command
SV test for frames added
Doc updated

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

Previous PR: #14995

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

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

This implement the following sticky buffers for IMAP protocol: - imap.request - imap.response - 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-03-19T14:18:29Z

Codecov Report

❌ Patch coverage is 81.38968% with 458 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.59%. Comparing base (6587e36) to head (b37d069).
⚠️ Report is 76 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15064      +/-   ##
==========================================
- Coverage   82.59%   82.59%   -0.01%     
==========================================
  Files         990      993       +3     
  Lines      271761   274180    +2419     
==========================================
+ Hits       224465   226450    +1985     
- Misses      47296    47730     +434

Flag	Coverage Δ
fuzzcorpus	`60.47% <8.77%> (-0.56%)`	⬇️
livemode	`18.26% <7.90%> (-0.11%)`	⬇️
netns	`18.24% <7.90%> (-0.13%)`	⬇️
pcap	`45.42% <62.53%> (+0.16%)`	⬆️
suricata-verify	`66.19% <74.87%> (+0.04%)`	⬆️
unittests	`58.75% <50.34%> (-0.08%)`	⬇️

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.

coveralls · 2026-03-19T14:25:16Z

coverage: 79.349% (+0.03%) from 79.315%
when pulling b37d069 on glongo:dev-imap-proto-v4
into 6587e36 on OISF:main.

ct0br0 · 2026-03-19T16:26:48Z

ignore that qa run, previous run corrupted things

suricata-qa · 2026-03-19T19:30:03Z

WARNING:

field	baseline	test	%
	SURI_TLPR1_stats_chk
.app_layer.flow.ftp-data	607	568	93.57%

Pipeline = 30397

catenacyber · 2026-03-19T20:30:05Z

Do we keywords file file.data that should work for attachments as is the case for SMTP ? see #14995 (comment)

victorjulien · 2026-03-20T06:33:50Z

Do we keywords file file.data that should work for attachments as is the case for SMTP ? see #14995 (comment)

If they aren't, they should + SV test :)

catenacyber · 2026-03-20T08:18:06Z

More generally speaking, what is the list of features that we want here ? So, we can check for completeness or create later tickets for the rest.

For example, what about lua ?

glongo · 2026-03-20T08:41:48Z

Do we keywords file file.data that should work for attachments as is the case for SMTP ? see #14995 (comment)

If they aren't, they should + SV test :)

My idea was to add support for IMAP in file.data after the parser is merged, but I can implement it right away.

victorjulien · 2026-03-20T09:33:48Z

Do we keywords file file.data that should work for attachments as is the case for SMTP ? see #14995 (comment)

If they aren't, they should + SV test :)

My idea was to add support for IMAP in file.data after the parser is merged, but I can implement it right away.

I prefer a more incremental process, so we can track it in a ticket.

glongo · 2026-03-20T10:28:34Z

I prefer a more incremental process, so we can track it in a ticket.

I’ll open a ticket to track adding IMAP support to file.data and we can handle it in a follow-up PR.

catenacyber · 2026-03-26T19:32:38Z

And what about lua support ?

victorjulien · 2026-03-27T05:55:49Z

And what about lua support ?

We can track this in another ticket.

Lets focus on review of what is in the PR, so we can get this functionality merged. We can add more features in follow up work.

catenacyber

Thanks for the work, some stuff to finish

CI : ✅
Git ID set : looks fine for me
CLA : you already contributed
Doc update : nice
Redmine ticket : ok, should you first fix https://redmine.openinfosecfoundation.org/issues/8265 (in a backport-compatible way)
Rustfmt : yes :-)
Tests : some stuff on SV PR like OISF/suricata-verify#2908 (comment)
Dependencies added: none
Code : putting commens inline
Commits segmentation : I would squash parser and logger but ok
Commit messages : nice, typo This implement vs implements with an s
Also imap.pdu is a frame, not a sticky buffer

catenacyber · 2026-04-03T15:44:04Z

+
+Syntax::
+
+ email.command; content:"<command>";


Do we plan to have this for SMTP ?

catenacyber · 2026-04-03T15:44:44Z

+Email Header Normalization
+--------------------------
+
+Email headers are normalized before matching:


Are these imap email headers mime ?

No, these are standard email message headers.

catenacyber · 2026-04-03T15:47:33Z

+    let mut message_email: Option<&EmailData> = None;
+
+    let command_bytes = extract_command_from_requests(&tx.requests);
+    let command = String::from_utf8_lossy(&command_bytes).into_owned();


Can this be moved only in the condition if let Some(email_data) = message_email { below where it gets used

And I do not think you need to_owned just use the ref

catenacyber · 2026-04-03T15:51:54Z

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


Looks painful to reconstruct, can we not keep the raw bytes as this is what we output ?

catenacyber · 2026-04-03T15:52:29Z

+            {
+                for part in &fetch.body_parts {
+                    if let Some(email_data) = &part.email {
+                        message_email = Some(email_data);


Could there be multiple emails in one tx ?

So, this code seems strange as there is a loop...

Why? I need to look for the part that has parsed email data.

How are you sure that only one part has parsed email data ?

catenacyber · 2026-04-03T15:54:24Z

+    if let Some(email_data) = message_email {
+        js.open_object("email")?;
+        js.set_string("command", &command)?;
+        log_email_header_string(js, &email_data.headers, "from", "from")?;


should we not reuse code from rust/src/mime/smtp_log.rs ? Like I see log_field_comma(js, ctx, "cc", "cc")?; there

I wouldn't reuse it. log_field_comma is tightly coupled to SMTP's MIME layer whereas log_email_header_string operates on a HashMap<String, Vec<String>> where headers are already parsed, normalized and unfolded.

So, we cannot reuse it because one takes input as some C structures, and the other takes input as rust HashMap...

Hmmm... That is a shame... Hope we will not have problems by updating one function and not the other in the future...

catenacyber · 2026-04-03T15:55:23Z

+        name: PARSER_NAME.as_ptr() as *const c_char,
+        default_port: default_port.as_ptr(),
+        ipproto: IPPROTO_TCP,
+        probe_ts: Some(imap_probing_parser),


We no longer have port-independent detection !

catenacyber · 2026-04-03T15:57:54Z

+                                }
+                            }
+
+                            tx.requests.push(literal_msg);


This push does not seem bounded, is it ?

catenacyber · 2026-04-03T16:03:48Z

+    kw.flags = SIGMATCH_NOOPT | SIGMATCH_INFO_STICKY_BUFFER;
+    SCDetectHelperKeywordRegister(&kw);
+    g_mime_email_body_buffer_id = SCDetectHelperBufferMpmRegister("email.body", "IMAP EMAIL BODY",
+            ALPROTO_IMAP, STREAM_TOSERVER | STREAM_TOCLIENT, GetImapEmailBodyData);


Do we wait the end of the email to detect it ?

catenacyber · 2026-04-03T16:04:53Z

+                                    tx.parsed_email = Some(parsed_email);
+                                }
+                            }
+                            tx.responses.push(response);


Should be bounded by tx.response_lines.len() < IMAP_MAX_LINES ?

catenacyber · 2026-04-03T16:07:37Z

+                    if self.request_tls {
+                        if let ImapMessageType::Response { status, .. } = &response.message {
+                            if matches!(status, ImapResponseStatus::Ok) {
+                                SCLogDebug!("IMAP: STARTTLS detected");


you should call SCAppLayerRequestProtocolTLSUpgrade right ?

it's called above, line 363.

I think it should be rather called here at this place, than on next request

glongo · 2026-04-15T10:49:41Z

Replaced with #15209

glongo added 4 commits March 19, 2026 10:01

imap: implement parser

e858a43

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

imap: implement logger

9834176

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

doc: add imap parser and sticky buffers

b37d069

glongo requested review from a team, jasonish, jufajardini and victorjulien as code owners March 19, 2026 13:41

This was referenced Mar 19, 2026

IMAP protocol parser, logger and sticky buffers v3 #14995

Closed

tests: add test for imap proto OISF/suricata-verify#2908

Open

OISF deleted a comment from suricata-qa Mar 19, 2026

catenacyber requested changes Apr 3, 2026

View reviewed changes