This allows pre-defining connection metadata values. When coupled with
`peer` and/or `via`, these can be done based on the corresponding
addresses associated with the session.
closes: https://github.com/KumoCorp/kumomta/issues/355
The motivation here is to remove tls_config from EsmtpListenerParams
to make some future configuration changes easier, so this commit
moves that simple cache out to an explicit lru ttl cache.
This has the welcome side effect of enabling periodic reloading
of the tls parameters, which in turn makes it a hands-off process
for updating certificates: we no longer require the service to
be restarted for that.
These are hooked up only for memoize at this time. No default
behavior is changed by this commit, but you can optionally
specify these parameters in order to change the behavior.
The introduction of the
`opportunistic_tls_reconnect_on_failed_handshake` option resulted in
this regression, which is because I misread the `match` statement
for this case as being only for the opportunistic case, but it
also encompasses the required case.
The issue is:
* A site has an MTA-STS policy enforcing Required tls
* The handshake with that site fails (for reasons unknown and
irrelevant)
* We would unconditionally (wrt. Required vs. Opportunistic) respect
opportunistic_tls_reconnect_on_failed_handshake and re-queue the
current address for the next connection attempt
* Ordinarily, opportunistic_tls_reconnect_on_failed_handshake +
the remembered broken state would cause that next attempt to
downgrade to clear text, but MTA-STS forces the policy to
Require
* Goto step 2 (modulated by connection rate throttling)
The fix is simply to only apply
opportunistic_tls_reconnect_on_failed_handshake when the policy
is actually opportunistic.
We were using a fairly tight limit of 16 messages in the channel
that buffers the effects of changing bounces/suspensions from
any websocket-connected-clients.
A busy server could hit that limit fairly easily, resulting
in a `channel lagged by NUMBER` error that drops the websocket,
causing the client to need to reconnect and resync.
This commit resolves that by making the buffer a much more healthy size.
We were deduping just by rule_hash, but each of these tables has
additional required fields as part of the primary key.
The result was that, for sites with a lot of bounces/suspensions
triggered by the same rules across a related set of sources,
the full set of bounces and suspensions would not be correctly
reported as part of a websocket push.
sqlite doesn't have a native async interface, and instead will
use traditional OS-level mutexes to ensure thread safety.
Using those when under contention in a tokio scheduler thread
can lead to blocking of the tokio scheduler threads, which can
prevent timely delivery of data via websockets, or timely
processing of incoming log records.
This commit fixes up the sqlite access points to use tokio's
spawn_blocking function to move that style of mutex acquisition to a
more suitable context.
We'll wait up to 3s at a time for however many mesages are available
to extract from the tsa daemon websocket, then process the results
in batches.
This avoids the potential for geometric complexity if there is a run of
subscription updates happening around the same time.
I'm not totally sure why this isn't universally broken when using
openssl (instead of rustls), but in the specific case we were
investigating, the destination was configured via a routing_domain
and the resulting mx_host name had the trailing FQDN dot on it.
Removing that dot allows the certificate to verify, so let's
ensure that we strip it here in the client.
The issue here is that when an rfc2047 encoded display name is split
across multiple lines, the whitespace between them is not recognized
at the right time, which results in the second encoded word being
passed through as-is, without being decoded.
This commit fixes the precedence of whitespace parsing in that
case.
This commit allows setting a per-message `expires` timestamp
via msg:set_scheduling (and thus msg:import_scheduling_header).
The expiration takes precedence over max_age; max_age will be
ignored for messages that have configured and expiration time.
The expiration time is independent of the other scheduling
restrictions.
This resolves an issue where the default behavior for serde is to
silently swallow issues with this struct, because we use a flattened
optional structure for those restrictions.
Previously, we'd pick a source whether it had room for the new
message or not, then generate a TransientFailure when we subsequent
figure out that it is full.
This commit will try to deliver through one of the other possible
sources instead of delaying the message.
While auditing Answer::as_txt usage as a follow up from the recent
SPF fix, I noticed a TODO in the dkim code (which we forked from
another implementation) to support processing multiple TXT
records.
This commit implements the necessary tweaks to extract multiple
signatures and attempt to verify them against the incoming message.
The issue here was essentially a data fidelity issue around
TXT record representation.
A DNS TXT record can be composed from multiple strings, and a domain can
return multiple TXT records, so there is some nesting.
The SPF RFC says:
```
3.3. Multiple Strings in a Single DNS Record
As defined in [RFC1035], Sections 3.3 and 3.3.14, a single text DNS
record can be composed of more than one string. If a published
record contains multiple character-strings, then the record MUST be
treated as if those strings are concatenated together without adding
spaces. For example:
IN TXT "v=spf1 .... first" "second string..."
is equivalent to:
IN TXT "v=spf1 .... firstsecond string..."
TXT records containing multiple strings are useful in constructing
records that would exceed the 255-octet maximum length of a
character-string within a single TXT record.
```
so the SPF logic was dutifully joining records together around the
empty string.
Howerver, if you look at `dig yahoo.com txt` you'll see a bunch
of non-SPF records:
```
yahoo.com. 1800 IN TXT "google-site-verification=Z3-Vh6zqUMgybVH4wQl1GxKSKN7JE13kyCyeZ3TZZ-I"
yahoo.com. 1800 IN TXT "v=spf1 redirect=_spf.mail.yahoo.com"
yahoo.com. 1800 IN TXT "Zoom=13284637"
yahoo.com. 1800 IN TXT "edb3bff2c0d64622a9b2250438277a59"
```
these were getting joined together and producing a bogus input.
Obviously we should not join the results from the txt lookup
together like that, but then why would the RFC make a point
of talking about joining stuff together, and where should
that logic live?
Our `Answer::as_txt` implementation was doing some joining
of its own and it turned out that it was concatenating across
the outer layer of the aforementioned TXT record nesting.
This commit fixes that up and adds some test coverage.
I worry that 1 minute might be too short to be universally good,
particularly as a change in behavior from an earlier build: going from
no timeout to 1 minute could be super surprising and un-welcome.
Let's bump this up to 5 minutes which more closely matches the
default for DATA in the base SMTP RFC.
This is why slow DNS often expanded to 3-4x as long as the expected
timeout value; we make a handful of calls in succession, assuming
that the cache was effective in an earlier stage.
We now cache errors for 5 minutes by default, and it can be configured
as desired.
refs: https://github.com/KumoCorp/kumomta/issues/325
Wez Furlong
Ryan Bonnell
Mike Hillyer
Blake Patton