This commit is a step towards some general improvements around
our handling of authentication and authorization.
This commit is focused primarily on authorization, but there are
some adjustments to how we track authentication as part of enabling
that.
We now have a separate AuthInfo type that holds the overall
authentication information/context associated with an inbound
SMTP or HTTP session.
It is populated with the peer_address as a fact rather than a statement
of trust.
If authentication via the appropriate lua auth callback is successful,
then the AuthInfo has additional identities added.
There are some types and events for loading access control lists and
matching their rules against an AuthInfo.
There is now a system default ACL that is equivalent to the prior
hard-coded access policy that was encoded into each HTTP endpoint.
This change makes it possible to replace the ACL with a
user-defined ACL.
Later will be some work on authentication to allow more options
for HTTP auth.
We found an interesting issue where a MIME part was labelled as `text/`
with no explicit charset (and was thus implicitly "us-ascii", according
to the specs). The content was actually UTF-8 encoded farsi text.
Since encoding_rs treats us-ascii as being an alias for windows-1252,
that part was re-encoded as bogus data, rather than being recognized as
UTF-8.
This commit improves the outcome in this situation:
1. Remove encoding_rs. Replace its Charset type with the Encoding type
that I added to the charset-normalizer crate in
https://github.com/nickspring/charset-normalizer-rs/pull/45
which allows us to actually differentiate between 7-bit ascii
and latin-1.
2. Improve the handling of body part extraction; there were a couple
of cases where we didn't consider the charset for text parts(!)
and now we do, with some additional smarts for when the charset
fails to decode.
3. Improve the conformance checking. We do the majority of this
during parsing, but we don't check for deeper issues such as
the charset not matching the transfer-decoded payload because
that is too costly to do for the majoriy of parses. This commit
introduces a deep conformance check that does validate that we
can extract the part, flagging any failures as
NEEDS_TRANSFER_ENCODING.
4. The charset detection options are threaded through the conformance
checks and into the logic that extracts the parts during a rebuild,
so that we can fix up those parts during rebuild.
The bulk of the conformance checking/fixing has been moved into the
mailparsing crate from the message crate, which makes the above a bit
easier and centralizes that logic better--fewer places to look to figure
things out.
we were producing output like this when `prefix=kumomta_`:
```
# HELP kumomta_total_readyq_runs total number of times a readyq maintainer was run
# TYPE kumomta_total_readyq_runs counter
kumomta_total_readyq_runs 17
# HELP kumomta_connection_count number of active connections
# TYPE kumomta_connection_count gauge
connection_count{service="smtp_client"} 0
connection_count{service="esmtp_listener"} 0
connection_count{service="lua"} 0
```
Those `connection_count` lines should be `kumomta_connection_count`.
When using an explicit mx_list, rather than consuming the DNS-provided
MX records, we were populating the connection plan LIFO with the mx_list
which is specified as a FIFO.
The result was that we would connect to the addresses in the reverse of
the intended order.
Most users only populate a single entry for mx_list, if they use this at
all, so it has flown under the radar. This issue was introduced
together with the mx_list feature in bcd2946c53.
This commit corrects that issue so that the behavior conforms to the
intended and documented ordering, and tweaks the tests to accommodate
it; the test changes required were:
1. disconnect_in_data was actually sensitive to the order, and encoded
the reverse ordering in its expectations. :-/
It now correctly encodes the correct order, which should allow
detection of regression in the future.
2. disconnect_terminate_ok is sensitive to extraneous TransientFailures.
It was implicitly dependent on the ordering of the mx_list in the
sense that it relied on the FIFO list returning as [127.0.0.1, ::1],
which resulted in trying ::1 first (because that list is consumed as
LIFO), failing due to ::1 not actually being bound in the test
environment, and proceeding to test with 127.0.0.1 being the last
working address.
With the ordering fixed that test started to fail because the second
host in the correct connection plan was ::1 and that was not
reachable, triggering a TransientFailure against the expectations
of the test.
The goal of the test was to verify the behavior in the
exhausted-connection-plan case, rather than the
unreachable-final-mx-case, so the resolution here is to add v6
addresses to the skip_hosts config. Will that come back to haunt us?
Time will tell.
Saw an MX record that expands to 0.0.0.0 in the wild.
As well as being nonsense, that causes a mail loop, so we should add it
to prohibited_hosts.
I noticed that we also did not document its default value, so the
doc update here shows the before and after for this.
in 18e55a6e91 I noticed that we should be
handling 552 as a transient failure, but I neglected to mention this
change in the changelog because it seemed obscure and "couldn't" hurt
anyone.
I overlooked that we have client response rewriting and of course this
change did impact one of our sponsors that rewrites certain transient
452 responses to 552 responses.
So, let's add a note explaining this to see if we can prevent others
from hitting it in the future.
This aggregates the various authentication-results producing auth checks
into a more convenient function.
Part of this change improves some plumbing in the dkim checking (we no
longer raise an error for a missing From, but instead indicate a failed
dkim result), and we now support passing down a resolver name to the
various checking functions, to facilitate testing and other more
advanced use cases.
refs: https://github.com/KumoCorp/kumomta/issues/16
refs: https://github.com/KumoCorp/kumomta/issues/84
Augment the docs to clarify that msg:append_header and
msg:prepend_header do no modification of the value by default.
Adjust the wrapping used by Header:new_unstructured so that it
hard wraps before the SMTP line length limit.
Add optional encode parameter to msg:append_header and
msg:prepend_header so that those methods can opt-in to the
Header:new_unstructured formatting.
Add HeaderMap:append method to mirror the existing HeaderMap:prepend
method.
Add unit tests that demonstrate the effect of wrapping/encoding
for both the "dumb" msg:append_header and msg:prepend_header functions,
and the "smarter" HeaderMap:prepend and HeaderMap:append methods that
always encode their parameters.
Update the docs for the headermap flavor of these methods to indicate
that they will always encode the value.
This allows a (more or less) arbitrary lua function to be used to load
data.
The primary advantage of this is that the size of the resulting
KeySource struct is smaller than it would be to hold the actual data
inline. It also enables the surrounding code to be slightly better
factored.
Add CBC and ecb Block mode aes encryption/decryption.
To facilitate testing this better, and just to be nicer overall, adjust
data-loader's KeySource::key_data variant to support loading raw binary
bytes.
Co-authored-by: Wez Furlong <wez@wezfurlong.org>
Closes: https://github.com/KumoCorp/kumomta/pull/395
Add a convenience function that is intended to be used around the
time that you might be building up the overall set of
Authentication-Results.
Populate missing smtp.mailfrom in SPF authentication result.
This is incremented each time we log a Rejection record from the smtp
server.
The service label identifies the listener address, and there is a
special `total` that is incremented as a rollup across all listeners.
```
# HELP smtp_server_rejections number of Rejection records logged by the smtp server
# TYPE smtp_server_rejections counter
smtp_server_rejections{service="127.0.0.1:2025"} 1
smtp_server_rejections{service="total"} 1
```
closes: https://github.com/KumoCorp/kumomta/issues/429
Let's base our quoting decisions based on whether the string is fully
atext compatible, and once quoting, on whether the character requires
quoting within the quoted string.
refs: https://github.com/KumoCorp/kumomta/issues/428
Partially reverts aac842439f and fixes the
underlying issue that led me to throw that together: the issue was that
the check_line_lengths function needed to add 2 to the index otherwise
iterations after the first would over-estimate the length of the
currently found line by 2, because the prior index included the CRLF
that it found in the input.
While poking at this, I found that there was a fairly harmless edge case
around non-DATA line handling: we used a 1024 stack buffer to read the
command in, and would accept anything that fit into that as valid, even
though the limit is set to a slightly smaller value.
We were including the CRLF in the measurement that we were making,
while the default value was the value from the spec excluding the CRLF.
Let's increase the default value to 1000 to make things match up.
* Add exp= test based on an example in the rfc
* Discovered that we were a bit too strict and didn't allow spaces
as shown in that example
* Add integration test for the ehlo_domain string quoting issue
resolved in 236afa417f
* lua spf check: Assume ehlo_domain = domain when sender is none
This commit does some plumbing work to allow defining alternative
resolvers with different names that can be used to query different
upstream DNS servers.
This is most useful when implement DNSBLs where you might have a custom
DNS server configured with one or more RBL zones that is reserved purely
for RBL lookups.
The plumbing introduces a more regular syntax for defining one of the
various implementations of the Resolver trait, as well as a new
Aggregate resolver impl that can query across multiple Resolvers until a
query is satisfied.
This allows some interesting and powerful configurations, such as
loading a static zone file into memory to query it directly, and/or
blending that together with querying either the system or some other
upstream DNS server as a fallback.
The various lookup functions (except for MX!) have been updated to
accept an optional alternate resolver name, so that they work together
with the above.
A new rbl_lookup function is also provided as a convenience for querying
the most common form of RBLS.
ptr_host and reverse_ip are two string utility functions that are likely
not going to be widely used, but are very convenient to have when you
do have a usecase that requires it!
closes: https://github.com/KumoCorp/kumomta/issues/269
Similar to the general purpose resolve function, we should map
NoRecordsFound to empty list(s) of results in the specialized
ip, mx and ptr lookup functions, otherwise we can cause rule
evaluation to terminate too soon.
This never worked as well as the other amqp option that we support,
and upgrading it to the current version is a bit of a chore.
I'd rather just drop this dep and keep things more trim.
Since none of our sponsors are using the lapin implementation, let's
mark it as deprecated and we can take it out after we cut the next
stable release.
A "casualty" of this is that the client-ip crate, which we were using
for its InsecureClientIp type for informational purposes, has retired
the InsecureClientIp type because it wasn't trustworthy overall
(https://github.com/imbolc/axum-client-ip/issues/32).
What we do for the time being is configure that crate to use the direct
peer IP. There isn't a way to automagically select the "best"
information available without building in some kind of middleware, and
we probably should make that configurable in order to be fully
trustworthy.
So for now, we're punting on that until someone shows up with some
requirements and sponsorship and we'll make it happen then.
This commit implements a kumomta-specific message transfer
protocol that is intended to be used to migrate messages
from one kumomta node to another.
The transfer is carried out using an HTTP POST request
to the destination node's http listener.
The request includes the full message metadata and body,
in a compressed form.
An xfer request can be made via `kcli xfer` (and thus also via an HTTP API
endpoint). It works similarly to a rebind operation; you specify the
criteria to be used to match scheduled queues, along with the target
node for the xfer, and kumomta will find matching queues, drain out the
messages, make an adjustment to the metadata to capture current
scheduling information, and then place the messages into an xfer queue.
The xfer queue has hard-coded scheduling queue configuration of its own,
with the base retry interval set to 10 seconds, which should be suitably
aggressive for the intended use case.
You may apply shaping to affect the number of concurrent requests in a
similar way to how TSA shaping is configured.
On the receiving side, the incoming xfer sanity check to prohibit
trying to xfer to itself.
The spool id of the Message is not suitable to be reused verbatim on
another node (spool ids include the local mac address and creation
timestamp information, as well as a random component), so the receiving
side will derive an id that should be suitable for use on that node.
The originating node id and spool id will be preserved in metadata to
aid in tracing.
It is possible for an xfer request to target an existing xfer queue, so
that you can correct/update the target in various circumstances. In that
situation the messages will be "simply" moved from the source queue to
the destination queue.
It is possible to cancel an xfer request via `kcli xfer-cancel` (and
thus also via an HTTP API endpoint). You specify the target queue,
which must be an xfer queue, and it will have its messages drained and
the metadata changes that were applied when the xfer was initiated will
be reversed, allowing the messages to then be reinserted into their
originating queue.
refs: https://github.com/KumoCorp/kumomta/issues/311
This commit adds a test for exceeding the number of recipients
allowed in a transaction and fleshes out our handling of that
status so that we will immediately try the remaining recipients
of the batch, without delaying the message by eg: 20 minutes
in the default retry parameters.
As part of this, I noticed that we were returning the wrong
smtp status code in our server implementation, so this
commit fixes that up.
Wez Furlong
crxandWez Furlong
Dario MaiocchiandWez Furlong