Skip to content

IMAP protocol parser, logger and sticky buffers v8#15648

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

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

Conversation

@glongo

@glongo glongo commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Changes:

  • Vec::with_capacity in literal is bounded with IMAP_MAX_BODY_SIZE
  • Unbounded email header allocation is bounded with IMAP_MAX_HEADERS
  • Line length bounded with IMAP_MAX_LINE_SIZE
  • Plain credentials are no longer logged
  • Added more SV tests (imap-body-too-large, imap-line-too-long, imap-too-many-headers)

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

Previous PR: #15617

SV_BRANCH=OISF/suricata-verify#2908

glongo added 4 commits June 15, 2026 15:57
This introduces a parser for IMAP protocol.

An IMAP transaction has two states driven by the 'complete' field:
  - Open (complete = false): waiting for a matching tagged response.
  - Complete (complete = true): tagged response received, or special
    conditions met (BYE, server greeting).

  Completion logic (is_complete):
  - If a tagged request exists: requires a matching tagged response.
  - No tagged request + BYE: complete (server closing).
  - No tagged request + any response: complete (server greeting).

  Transactions are created in three places:
  1. Request parser: every parsed command creates a new tx.
  2. Response parser (tagged, no matching request): midstream/async;
     orphan tagged response gets its own tx.
  3. Response parser (untagged, no incomplete tx): server greeting or
     unsolicited response.

  Once open, messages accumulate:
  - Request side: continuation and literal data attach to the most
    recent incomplete tx.
  - Response side: untagged responses attach to the most recent
    incomplete tx; tagged responses attach via find_request(tag).

  Email extraction happens at most once per tx:
  - From literal data in APPEND commands (request side).
  - From FETCH response data (response side).

  Seven hard limits prevent unbounded growth:
  - IMAP_MAX_TX (256, configurable): total transactions. Exceeded:
    all incomplete txs force-completed with TooManyTransactions event.
  - IMAP_MAX_MSGS_PER_TX (512): requests/responses per tx. Exceeded:
    message silently dropped.
  - IMAP_MAX_LINES (512): request/response lines per tx. Exceeded:
    line silently dropped.
  - IMAP_MAX_BODY_SIZE (10MB): email body in ImapParsedEmail.
    Exceeded: body truncated, BodyTooLarge event.
  - IMAX_MAX_HEADERS (512): Numbers of headers parsed and stored. Exceeded:
    TooManyHeaders event.
  - IMAP_MAX_LINE_SIZE (8KB): request/response line max length. Exceeded:
    LineTooLong event.
  - Literal size (u32): bounded by declared size in {N} specifier.

  IMAP_MAX_TX only is configurable limit via app-layer.protocols.imap.max-tx.

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
@glongo glongo requested review from a team, jasonish, jufajardini and victorjulien as code owners June 15, 2026 15:21
@glongo glongo changed the title Dev imap proto v8 IMAP protocol parser, logger and sticky buffers v8 Jun 15, 2026
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.86669% with 403 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.94%. Comparing base (28b10fb) to head (a61cf4c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15648      +/-   ##
==========================================
+ Coverage   82.90%   82.94%   +0.03%     
==========================================
  Files        1006     1009       +3     
  Lines      273648   276262    +2614     
==========================================
+ Hits       226869   229134    +2265     
- Misses      46779    47128     +349     
Flag Coverage Δ
fuzzcorpus 60.84% <13.85%> (-0.52%) ⬇️
livemode 18.35% <11.96%> (-0.08%) ⬇️
netns 22.71% <11.96%> (-0.12%) ⬇️
pcap 45.44% <66.08%> (+0.21%) ⬆️
suricata-verify 66.81% <79.29%> (+0.09%) ⬆️
unittests 58.48% <53.43%> (-0.06%) ⬇️

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 = 32039

@catenacyber catenacyber left a comment

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.

Thanks for the work

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 scripts/rustfmt.sh
Tests : 🟡 some unanswered questions on SV PR like OISF/suricata-verify#2908 (comment)
Dependencies added: none
Code : looking
Commits segmentation : ok
Commit messages : cool, some questions below

bounded by declared size in {N} specifier

What is {N} specifier ?

silently dropped

Could we have events for it to be not silent ?

Comment thread rust/src/imap/logger.rs
Comment thread src/detect-email.c
Comment thread rust/src/imap/imap.rs
header_names: Vec::new(),
header_values: Vec::new(),
direction,
body_md5,

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 md5 with truncated data or not ?

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.

With truncated data.

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.

(maybe needs a mention in the docs)

Comment thread rust/src/imap/imap.rs
}

#[no_mangle]
pub unsafe extern "C" fn SCImapMimeConfigBodyMd5(val: bool) {

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.

That should be only if IMAP_MIME_BODY_MD5_DISABLED is false (if we did not explicitily disable md5)

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.

This should be for SMTP as well

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.

Those are checked above, line 295-298.

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.

Which file are you talking about ?

For

line 295-298.

I have

    }
}

impl ImapState {

Comment thread doc/userguide/rules/email-keywords.rst
This keyword works with both SMTP and IMAP protocols.
For SMTP, the config option ``app-layer.protocols.smtp.mime.body-md5``
must be enabled or auto. For IMAP, the config option
``app-layer.protocols.imap.mime.body-md5`` must be enabled or auto.

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.

Also, could we use email.body; to_md5; ?

@catenacyber

Copy link
Copy Markdown
Contributor

Do we have file extraction ? (or a ticket for it) ?

Should we not log the email field ?

@glongo

glongo commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Do we have file extraction ? (or a ticket for it) ?

Should we not log the email field ?

Victor and I agreed on keeping email out for now, and work on it after this is merged since we want to refactor the current email code (which is strictly bounded to SMTP).

@glongo

glongo commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the work

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 scripts/rustfmt.sh Tests : 🟡 some unanswered questions on SV PR like OISF/suricata-verify#2908 (comment) Dependencies added: none Code : looking Commits segmentation : ok Commit messages : cool, some questions below

bounded by declared size in {N} specifier

What is {N} specifier ?

silently dropped

Could we have events for it to be not silent ?

{N} is the literal size (email body) and come from IMAP command, see below:

A001 APPEND INBOX {10485813+}

body-too-large event is set if literal size is greater than IMAP_MAX_BODY_SIZE. I missed to update update this part of commit.

@catenacyber

Copy link
Copy Markdown
Contributor

{N} is the literal size (email body) and come from IMAP command, see below:

Ok, this was not clear for someone who does not know IMAP ;-)

@catenacyber

Copy link
Copy Markdown
Contributor

Victor and I agreed on keeping email out for now, and work on it after this is merged since we want to refactor the current email code (which is strictly bounded to SMTP).

Please add a ticket about it and make it blocked by the current IMAP ticket :-) So I will not ask again next time

@glongo

glongo commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@catenacyber

Copy link
Copy Markdown
Contributor

Done: https://redmine.openinfosecfoundation.org/issues/8666

Not completely :-p

make it blocked by the current IMAP ticket

@catenacyber catenacyber left a comment

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.

Needs some changes for the explicitly disabled md5 : see #15648 (comment)

and likely other changes from inline discussions

@glongo

glongo commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Replaced with #15730

@glongo glongo closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants