The prior commit focused on avoiding a panic when we encountered an invalid
boundary string. This commit makes thing degrade more gracefully: we'll
treat the bad MIME part in the same way that we deal with overly deep nesting
and handle it as an opqaque leaf part.
The new_multipart constructor now rejects an invalid boundary rather than
building an unserializable part, so a flawed policy can't accidentally create
broken messages.
to_message_bytes now propagates errors instead of panicking. check_fix_conformance
and the HTTP injection API now report this error directly, and DSN generation
omits the original message when it cannot be serialized.
/0 prefixes now correctly match all addresses. Invalid overlong prefix lengths
are rejected at parse time instead of silently producing wrong masks (or, in
debug builds, a panic).
A malformed or forged `id` in an xfer payload could panic kumod, since
spool ids are assumed to always be v1 UUIDs with an embedded
timestamp. Every construction path now rejects anything that isn't
v1.
Deeply nested multipart content is now retained as an opaque part with
its headers parsed and raw body preserved once nesting passes 100
levels, instead of recursing until the process stack overflows.
Those messages are tagged with the new MIME_NESTING_LIMIT_EXCEEDED
conformance flag.
The RFC 5322 comment parser recursed once per nesting level, so a header
with thousands of nested parentheses could exhaust the stack and crash
the process. It now tracks nesting depth with an explicit loop instead.
When `invalid_line_endings` is set to Fix or Allow, correctly measure line
lengths even when messages use bare LF or CR instead of CRLF. Previously
such messages were incorrectly rejected as having overlong lines.
In most cases this error context was discarded, but if your policy
script triggered an explicit parse, you might see the error context in
the lua error that it would trigger in that case. Since it was not very
useful (and was wasteful) we now simply report the input message size
for the context instead. The actual parse error is still the primary
reason in the error chain, so this is not a loss in information.
Bounds memory amplification from crafted messages with many minimal short
headers, which previously consumed far more resident memory than their
wire size would suggest.
Fix O(N^2) CPU usage when rebuilding messages with many MIME parameters.
Decoding the parameter map, re-emitting the header, and merging original
parameters back into the rebuilt header were each quadratic. Duplicate
parameters now use consistent last-wins semantics.
Avoid quadratic work for messages with many malformed line endings when using
`invalid_line_endings="Fix"`. Remove the in-place implementation entirely to
prevent future accidental use.
Strip control characters from all values when encoding Authentication-Results
and ARC headers. This prevents sender-controlled values from embedding raw
CR/LF to forge headers or push trusted results into the message body.
closes: https://github.com/KumoCorp/kumomta/pull/523
Avoid process abort when kumo.fs.glob is passed an absolute pattern
containing a recursive wildcard. This was an issue in the upstream
filenamegen crate, resolved in the latest release.
closes: https://github.com/KumoCorp/kumomta/issues/578
This fixes two vulnerabilities reachable via msg:dkim_verify() on inbound
mail: an out-of-bounds read panic on very short b= tags, and the anti-DoS
cap now correctly bounds all parsed signatures, not just parse failures.
Address headers in the HTTP inject API's generic `headers` map (`Cc`, `To`,
etc.) were emitted as unstructured text. When they contained non-ASCII
content, the entire value (as opposed to just the display name) would
be qp-encoded, producing an invalid header.
closes: https://github.com/KumoCorp/kumomta/pull/598
quote_string passed a CR or LF straight through into a quoted display name.
that would terminate the header line early, causing the subsequent bytes
to appear as a separate injected header.
This has been resolved, taking care to preserve legitimate folding.
Headers were built and rebuilt as "Mime-Version" rather than
"MIME-Version". Both are valid per RFC 2045, but some spam filters,
such as rspamd, score the mixed-case form as a deliverability signal.
Fix the header name in the accessor macro and in ParsedHeader's
grammar table, the places that spell it, and update the snapshot
tests across mailparsing, message, kumo-log-types, mod-mimepart, and
kumod that captured the old spelling, along with the reference docs
that showed the old casing in their examples. #564
Add `ParsedHeader`, which parses a header's raw bytes using the grammar
implied by its name (mailbox list, address list, date, MIME parameters,
and so on) and re-encodes it back to canonical form. `Header::rebuild()`
now goes through this lookup instead of a hand-written table, which
also fixes `Authentication-Results`: it was falling through the old
table's unstructured fallback and being left as free text instead of
being parsed and re-encoded like the other structured headers.
wrap() hard-wrapped an over-long word byte by byte, which could slice a
multi-byte UTF-8 sequence in half and panic when the result was
validated as UTF-8. Walk the word in UTF-8 chunks instead, splitting
only between whole characters and passing invalid byte runs through
unchanged.
When a record exceeds the configured max_line_size, discard just that record
and continue reading the rest of the segment instead of aborting the entire
segment.
The zstd decompression buffer now grows on demand to read records up to
a configurable 128 MiB cap. Oversized records are rejected explicitly
both on read and write, with matching limits so all written records are
guaranteed readable.
The gate could latch permanently when only one of the two error signals
(foreground or background) ever moved, requiring an operator restart. It now
auto-reopens for a retry after `error_unlatch_duration`, regardless of which
signal latched it.
Also serializes foreground error reporting with gate transitions so a
concurrent fatal error cannot be undone by a reopen, and rejects a zero
`error_unlatch_duration` when automatic reopening is enabled. The underlying
cause is always logged. Adds a reusable LD_PRELOAD fault-injection crate and
integration tests that reproduce a full disk and a slow disk to drive the
latch-and-reopen cycle end to end.
Fixes#597
A literal underscore in a header value that needed RFC 2047 Q encoding was
emitted unescaped, so a conforming mail client decoded it back to a space
and silently corrupted the header. Underscores are now escaped as =5F, and
the Q encoder passes through only the punctuation RFC 2047 permits unencoded
in a phrase.
Iterating a response's headers with pairs() looped forever when a header name
repeated, which could happen for example with multiple Set-Cookie headers.
Fallout from the xfer far-future timestamp issue is that we could
try to produce a DSN for a message with a crazy date, which chrono's
default RFC2822 rendering gives up on with a very hostile panic.
This commit adds our own infallible rfc2822 formatter. It is infallible
rather than fallible because dates can be formatted from inside Display
impls where the full chain may not be prepared to handle an error.
A message received via inter-node transfer, on systems hosted on AWS, could
end up with a wildly incorrect far-future timestamp. The underlying mac_address
crate would pick a NIC with the same MAC address as other AWS instances in the
cluster, and that triggered a code path where the collision resolving logic
misinterpreted the timestamp portion of the incoming spool id.
The thing that made this painful was the logic in spool_id.rs: it
misinterpreted the subsecond portion of the timestamp extracted from
the uuid, and due to the way that that field wraps, could produce wildly
inaccurate deltas with a huge multiplier.
As a belt and suspenders treatment, allow the user to influence which
MAC address is selected via the new KUMO_MAC_INTERFACE and
KUMO_MAC_ADDRESS environment variables.
Base64-encoded supplemental trace headers (X-KumoRef) can exceed the
998-octet SMTP line length limit when they include sizeable metadata,
causing a strict receiver (eg: us, when an ARF comes back to us with
that header) to reject the message at DATA with "line too long".
Fold the encoded value across continuation lines so each physical line
stays within the limit, and strip the folding whitespace before decoding
it back into a feedback report.
Running down a test flake and diagnosed this one.
This commit closes a narrow race when attaching to the SMTP tracing
(trace-smtp-server, trace-smtp-client) and TSA subscription
(subscribe_suspension_v1, subscribe_event_v1) websocket endpoints. The
server did not finish registering the new subscriber until just after
the connection handshake completed, so any event produced in the brief
window between those two points was not delivered to that client.
In practice this could cause a freshly-attached SMTP trace to miss the
first event or two of a session that happened to start at the same
instant; it did not affect mail flow. The endpoints now register the
subscriber before completing the handshake
RFC 2822 date parsing now tolerates an obsolete alphabetic time zone
such as the `UTC` that Amazon SES emits in its bounce reports, which
strict parsing would otherwise reject. A recognized abbreviation
resolves to its offset (derived from the IANA time zone database), and
any other alphabetic zone falls back to the `-0000` unknown offset per
RFC 5322 section 4.3 rather than failing the parse.
closes: https://github.com/KumoCorp/kumomta/pull/551
While testing message parsing code, there's a chance that the sender
isn't fully compliant with base64. This leads to failure such as below
base64 decode: non-zero trailing bits at 20211 b='i' in HRtbD4NCi==
As long as it's not destructive, its more convenient to be able to
support non-RFC messages so we can extract message artifacts for its
decision making.
closes: https://github.com/KumoCorp/kumomta/pull/558
RFC 5321 does not permit a space between the colon and the reverse/forward
path in MAIL FROM: and RCPT TO:, but a number of legacy clients emit one
(e.g. "MAIL FROM: <addr>"), which kumod rejected with 501 5.1.7.
Relax the grammar to accept and discard an optional run of spaces/tabs after
the colon in both the success and "valid-address + trailing junk" arms for
MAIL FROM and RCPT TO. This is a grammar-level change per review feedback,
replacing the earlier opt-in allow_space_before_path listener option.
Adds parser tests covering the tolerated space for both verbs, including the
null sender, postmaster, and ESMTP parameter cases.
closes: https://github.com/KumoCorp/kumomta/pull/559
This sets up plumbing to allow testing the broken mta-sts aliasing
issue, and enables feeding an optional resolver through the mx lookups
as well.
closes: https://github.com/KumoCorp/kumomta/pull/524
refs: https://github.com/KumoCorp/kumomta/issues/484
Briefly, the issue is that if some random domain that shares MX records
with another (eg: someone is using google apps or icloud for their
vanity domain) publishes a broken MTA-STS policy that requires eg:
cloudflare MX hosts then because we roll up by site name, that broken
MTA-STS policy bleeds into all the other domains that share those MX
records.
The resolution is simple, but is technically a breaking change.
Moving the policy resolution to happen during site_name resolution
allows us to resolve both per-domain things at the same and have the
MTA-STS policy amend the effective set of MX hosts. The output of that
is then used for site_name aggregation/rollup.
The consequence of this is quite nice: an MTA-STS policy that is more
restrictive than the full set of MX hosts now prevents delivering to
any of the excluded hosts, and a totally broken policy that prevents all
of its MX hosts is now completely undeliverable and will produce
transient failures.
The downside is that for users that had previously disabled mta-sts in
their default shaping block, they will need to change a different config
option to continue to prevent MTA-STS from being consulted. One example
of this that I recall is that one user's network posture prevented
MTA-STS from making HTTPS requests to fetch the policy. Another user
just wanted to cut out the additional DNS traffic. Those use cases
require altering the new kumo.dns.set_mta_sts_enabled enabled to false
during `init`.
Wez Furlong
Mike Hillyer
kay ozaki
Alex Burch