Install a tracing subscriber in the test process itself, routed through the
libtest capture, so diagnostics from in-process library code show up alongside
a failing test's output instead of being discarded.
Have the tsa test helper fetch shaping at Error level so a failed HTTP
fetch fails the test with the real reason, rather than being downgraded
to an empty result that caused the tsa_basic_automation test to fail.
Resolve the test's dead domain through a local test resolver instead of
live DNS. The real NXDOMAIN lookup took a variable few seconds, which got
folded into the first retry interval and tripped the assertion that the
first delivery attempt happens right after reception.
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
Changing a message's schedule now marks its metadata dirty, so the new
schedule survives changes post-reception. Without this a schedule applied
after the message was first persisted would be dropped.
set_sender and set_recipient_list mutate metadata fields (meta.sender /
meta.recipient) but were flagging the message DATA_DIRTY instead of
META_DIRTY. This had two consequences:
1. The rewritten sender/recipient was never written to the meta spool
(save_to only serializes metadata when META_DIRTY is set), so the
change could be silently lost after a spool reload or restart.
2. If the message body had already been saved and shrunk (empty
in-memory data buffer), flagging DATA_DIRTY made the next save fail
the `message data must not be empty` assertion in save_to. This
surfaced as noisy queue-maintainer requeue errors:
- "Error reinserting message: ... message data must not be empty"
- "while shrinking: <id>: message data must not be empty"
Flag META_DIRTY instead, and add regression tests covering the dirty-flag
behavior and the shrunk-data case.
closes: https://github.com/KumoCorp/kumomta/pull/570
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`.
We spent some time running down an issue where the combination
of the root-user umask and some ad-hoc testing to recreate the
spool directory led to a very difficult to understand situation
where the spool seemed to corrupt itself on the next restart.
The root cause was that the spool directories had been re-created
by the operator with permissions that led to an asymmetric
filesystem view for the dropped privs scenario (eg: the default
when running via systemd).
The crux of it is that the explicit existence check used by rocksdb to
determine if the database was new or pre-existing was inconsistent with
what the kernel actually allowed for the permissions on the directory,
causing the second start to believe that it was a new database, then
surprise itself when it found other evidence that it wasn't new at all.
This commit adds a dir-probe crate that tests for this misconfiguration
and raises an error before rocksdb can try to open and corrupt itself.
We can't reasonably exercise this in CI because of the priv dropping
component, but the included example probe utility confirmed it for
me outside of the harness:
```
bdad88ed680e# install -d -o wez -g wez -m 2700 /tmp/probe-demo
bdad88ed680e# ./target/debug/examples/probe /tmp/probe-demo --user wez
probing /tmp/probe-demo as ruid=0 euid=1000 rgid=1000 egid=1000
FAIL: /tmp/probe-demo (directory owner uid=1000 gid=1000 mode=2700); process ruid=0 euid=1000 rgid=1000 egid=1000: inconsistent
view of /tmp/probe-demo/.kumo-dir-probe9nMaUj.renamed: access(2) reports the file present=false but open(2) reports it present
=true. This means the process real and effective user ids differ (a privilege drop) and the directory permissions are too restr
ictive for one of those identities. A database opened here would decide to create a fresh instance yet write over the existing
files, corrupting itself on the next startup. Ensure the directory is owned by, and grants rwx to, the identity the service run
s as.
bdad88ed680e# chmod 2755 /tmp/probe-demo
bdad88ed680e# ./target/debug/examples/probe /tmp/probe-demo --user wez
probing /tmp/probe-demo as ruid=0 euid=1000 rgid=1000 egid=1000
PASS: /tmp/probe-demo is usable by this identity
bdad88ed680e# rm -rf /tmp/probe-demo
```
When running under a cgroup limit, get_cgroup() reported
stat.usage_in_bytes (memory.current on v2, memory.usage_in_bytes on v1)
as the process memory usage. That counter includes all page
cache, including cold reclaimable cache (inactive_file). For a
workload that writes logs and spool to disk, cold cache can dominate:
on a real pod memory.current was ~51GB while inactive_file was ~46GB
and the true non-cache footprint was ~1.8GB. kumod saw usage near its
limit, hit get_headroom() == 0, and ran
shrink_ready_queue_due_to_low_mem
and other reductions against pressure that was almost entirely
reclaimable cache the kernel would drop before any OOM.
Report the working set instead:
```
working_set = max(memory.current - inactive_file, anon)
```
This matches container_memory_working_set_bytes (kubelet/cAdvisor) and
the kernel's own reclaimability accounting. It still counts everything
that can drive an OOM: anonymous memory, dirty page cache, active_file,
tmpfs/shm, and slab.
Signed-off-by: Daniel Schaaff <daniel@danielschaaff.com>
closes: https://github.com/KumoCorp/kumomta/pull/549
Add spool_restart_clean_recovery and spool_restart_wal_replay to
exercise that a rocksdb-backed spool recovers queued messages across a
graceful stop/start, covering both the flushed-to-SST and WAL-only
on-disk states. These mirror a user incident report claiming the spool
was re-initialized as new on every restart; both pass, confirming
recovery works.
A command line that decoded to invalid UTF-8 propagated the decode
error out of read_line, which the session handler turned into a
421 technical difficulties response that tore down the connection.
Model it as a ReadLine variant so both the command loop and the AUTH
exchange emit a 501 syntax error and keep the session alive.
Closes#550
The hickory backend hardcoded the secure and bogus flags to false for
NODATA/NXDOMAIN responses, discarding the DNSSEC proof that hickory does
deliver. A securely denied answer carries validated authority records
(SOA/NSEC/RRSIG) proving the absence; derive secure and bogus from those
the same way we already do for the answer section of a positive response.
This closes a divergence from the unbound backend: a securely proven "no
MX" (common for signed domains that publish no MX) now marks the implicit
MX secure so DANE can engage, and a tampered denial can surface as bogus
and defer rather than silently reporting not-applicable.
Per RFC 7672 section 2.2.2, an MX host that is a securely published
CNAME remains DANE-eligible at its original name even when the alias
target lands in an unsigned zone: it is the secure TLSA RRset, not the
address records, that authenticates the peer. When the address chain is
insecure but MX selection was secure, an explicit CNAME query isolates
the alias's own DNSSEC status; a secure alias engages DANE, an
indeterminate status defers for downgrade resistance.
refs: https://github.com/KumoCorp/kumomta/pull/545#discussion_r3472353021
This allows you to explicit indicate that an mx_list should
be treated as DNSSEC secure.
We cannot simply assume that lua code setting mx_list be treated
as secure, because the lua code may be simply passing through
the result of some other DNS lookup--we need an explicit way
to thread through the overall secure flag in order for the
secure property to be correctly upheld.
cargo-dependency-cooldown[bot]
Wez Furlong
dependabot[bot]
wez
kay ozaki
Alex Burch
Mike Hillyer
Daniel Schaaff