This patch implements support for incoming connections wrapped in the
HAProxy protocol v1.
This is useful when running chasquid behind a HAProxy server, as it
needs the original source IP to perform SPF checks.
This patch is a reimplementation of one originally provided by Denys
Vitali in pull request #15, except the logic for the protocol handling
is moved to a new package, and the smtpsrv.Conn handling of the source
IP is simplified.
It is marked as experimental for now, since we want to give it a bit
more exposure just in case the option/api needs adjustment.
Thanks a lot to Denys Vitali (@denysvitali in github) for sending the
original patch for this, and helping test it!
This patch renames courier.Procmail to courier.MDA, to make it more
obvious that the functionality is not tied to that particular MDA.
It's just for readability, there are no functional changes.
Some utilities might want to access the EHLO/HELO domain in the
post-data hook (for example, to do additional SPF validations).
This patch implements that support, including sanitizing the EHLO domain
on the environment variable to reduce the risk of problems.
The EHLO parameter is generally referred to as "domain", even though it
can take either a domain or an address.
For clarity, rename the variable and comments to match.
This is stylistic only, there are no functional changes.
The queue protobuf definition currently uses the well-known timestamp
protobuf package.
This adds a build-time dependency on it, which is fairly harmless when
building from source (since the golang protobuf compiler includes it
already), but adds overhead for packaging on distributions.
Since this is the only external proto dependency we have, and the
protobuf message itself is trivial, this patch removes it an instead
embeds a compatible definition.
That way we remove the dependency and simplify packaging, with almost
negligible code overhead.
The change is fully backwards compatible and has no functional changes.
This patch makes chasquid's monitoring server expose an OpenMetrics
metrics endpoint.
It adds a new package "expvarom" which implements an HTTP handler that
exports expvar variables in the OpenMetrics text format.
Then, the handler is registered by the monitoring server at /metrics
(where most things expect it to be).
The existing exported variables are also extended with descriptions,
which is optional, but improves the readability of the metrics.
This patch makes protoio use the new protobuf API for
marshalling/unmarshalling text protobufs, as well as extends the tests
to cover marshalling failures.
The protobuf text output is not stable/deterministic and some spaces are
added randomly, so some integration tests have to be adjusted to account
for it.
This patch adds support for writing maillog to stdout and stderr, which
can be desirable in certain environments.
Thanks to Denys Vitali <denys@denv.it> who sent an alternative patch for
this functionality.
When we can't authenticate due to a transient issue, for example if we
rely on Dovecot and it is not responding, we should use a differentiated
error code to avoid confusing users.
However, today we return the same error code as when the user enters the
wrong password, which could confuse users as their MUA might think their
credentials are no longer valid.
This patch fixes the issue by returning a differentiated error code in
that case, as per RFC 4954.
Thanks to Max Mazurov (fox.cpp@disroot.org) for reporting this problem.
This makes it possible to manage chasquid logs using logrotate.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Added tests, minor style and comment changes.
In preparation for supporting log rotation, this patch makes the maillog
package to use blitiri.com.ar/go/log instead of its own writer.
Some of the tests are made more strict, to better test the log format.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Fixed build, extended commit message, adjusted to the log options
API, and added tests.
The output of the alias-exists hook is unused, so currently it's
discarded silently.
However, it can be very useful to debug issues when the hook is not
working as expected.
So this patch makes chasquid log the combined output (stdout and stderr)
to the execution trace.
This patch allows the configuration values to be overridden from the
command-line, with a new -config_overrides flag.
There is a fairly specific use case for this, when editing the
configuration file is not feasible or convenient (e.g. running an
user-supplied configuration in a managed environment).
This patch tidies how defaults are handled in the config, using a new
logic to allow "overriding" one config (the default) with another (the
user supplied).
It also improves how the comparisons are done in the tests, using the
more convenient "github.com/google/go-cmp/cmp" package, which also
prints nice diffs on errors.
This is in preparation for a future path where the override mechanism
will be reused.
Currently, the config package logs errors itself, in addition to
returning them.
That is confusing and results in some duplication of logging.
This patch makes config just return errors, and adjusts the callers
to log them properly.
There is a new protobuf library (and corresponding code generator) for
Go: google.golang.org/protobuf.
It is fairly compatible with the previous v1 API
(github.com/golang/protobuf), but there are some changes.
This patch adjusts the code and generated files to the new API.
The on-wire/on-disk format remains unchanged so this should be
transparent to the users.
tls.Config.BuildNameToCertificate was deprecated in Go 1.14, and is no
longer necessary.
However, we support down to 1.11, so we will keep it for now.
This patch adds a TODO to remove it in the future once the minimum
supported version is 1.14; and adjust the CI linter accordingly.
The SMTP courier, which handles outgoing connections, uses the domain of
the envelope's from as the domain in the HELO/EHLO greeting.
This works fine in practice, but ideally the domain used in the greeting
should match the reverse DNS record. This used to be more relevant but
nowadays it is not really enforced; however, it sometimes comes up in
self checks, and might cause some confusion when troubleshooting.
So this patch makes it use the configured hostname instead, which is
under the users' control and more likely to be compliant. It also
simplifies the code.
The documentation of the hostname configuration option is also updated
to mention this behaviour.
Thanks to Jonas Seydel (thor77) for bringing this up.
When creating a new Queue instance, we os.MkdirAll the queue directory.
Currently we don't check if it fails, which will cause us to find out
about problems when the queue is first used, where it is more annoying
to troubleshoot.
This patch adjusts the code so that we check and propagate the error.
That way, problems with the queue directory will be more evident and
easier to handle.
The linter complains that we're not checking for errors, but on some
cases it's on code paths were it is reasonable to do so (e.g. we're
closing the connection and it's a best-effort write).
This patch adjusts the code to make those cases explicit.
When receiving a message on a TLS socket, we currently don't check the
Handshake result, so connections often fail in a way that is not easy to
troubleshoot.
This patch fixes that by checking the result and emitting a nicer error
message before closing the connection.
This patch contains some readability improvements to testlib: it
adds/reformats some comments for exported functions for consistency, and
unexports some structs that are not used outside the library.
The smtpsrv fuzzer doesn't handle DATA commands particularly well:
it will continue to read but will skip lines that have STARTTLS as
content, and only really care for the first line due to a bug.
This patch fixes the handling, and moves the logic to a separate
function for readability.
When the client closes the connection, which is very common, chasquid
logs it as an error ("exiting with error: EOF").
That can be confusing and mislead users, and also makes a lot of
traces be marked as errored, when nothing wrong occurred.
So this patch changes the log to not treat it as an error.
In the SMTP courier, when the MX lookup fails with a DNS temporary
error, we should propagate that up and cause delivery to fail with a
temporary error too.
That way the delivery can be attempted later by the queue.
However, the code today doesn't make the distinction and will treat any
temporary DNS error as a permanent delivery failure, which is a bug.
The bug was found by ludusrusso and reported in
https://github.com/albertito/chasquid/issues/4
This patch fixes the bug by propagating the error properly.
In https://github.com/albertito/chasquid/issues/4, ludusrusso comments
that the DNS lookup error handling in the SMTP courier could be improved
by using DNSError.IsNotFound.
That is true, but unfortunately it was only added in Go 1.13, and we are
currently trying to support Go 1.11 since that's what in Debian stable.
So this patch updates the TODO to note this, so that when we can use a
newer Go version, we improve this code.
When the DATA input is too large, we should keep on reading through it
until we reach the end marker, otherwise there is a security problem:
the remaining data will be interpreted as SMTP commands, so for example
a forwarded message that is too long might end up executing SMTP
commands under an authenticated user.
This patch implements this behaviour, while being careful not to consume
extra memory to avoid opening up the possibility of a DoS.
Note the equivalent logic for single long lines is already implemented.
Reloading during tests will cause the testing aliases to be removed,
which makes test runs that extend beyond 30s to be flaky.
This patch fixes the bug by disabling reloads during these tests.
The testing couriers are currently only used in the queue tests, but we
also want to use them in smtpsrv tests so we can make them more robusts
by checking the emails got delivered.
This patch moves the testing couriers to testlib, and makes both queue
and smtpsrv use them.
Currently, there is no limit to incoming line length, so an evil client
could cause a memory exhaustion DoS by issuing very long lines.
This patch fixes the bug by limiting the size of the lines.
To do that, we replace the textproto.Conn with a pair of buffered reader
and writer, which simplify the code and allow for better and cleaner
control.
Thanks to Max Mazurov (fox.cpp@disroot.org) for finding and reporting
this issue.
On the smtp client package, there is no limit to the length of the
server's replies, so an evil server could cause a memory exhaustion DoS
by issuing very long lines.
This patch fixes the bug by limiting the total size of received data.
Ideally this would be done per-line instead, but gets much more complex,
so this is a compromise.
The limit chosen is 2 MiB, which should be plenty for any the total size
of server-side replies, considering we only send a single message per
connection anyway.
This is similar to 06d808c (smtpsrv: Limit incoming line length), which
was found and reported by Max Mazurov (fox.cpp@disroot.org).
Some tests require picking ports, and today resort to hard-coding,
which is brittle.
This patch adds a testlib.GetFreePort function to help pick free ports.
It is not totally race-free, but is much better than hard-coding.
If we fail to put the message in the queue (e.g. because we're out of
storage space, or the aliases-resolve hook errored out), it should be
considered a transient failure.
Currently we return a permanent error, which is misleading, as we want
clients to retry in these situations.
So this patch changes the error returned accordingly.
This patch updates the auto-generated code to match the latest tooling
versions.
In particular, the protobufs are regenerated, and the new version no
longer supports unkeyed literals, so some minor changes are needed.
Other than that, the cipher list is extended with the latest ciphers.
This patch implements two new hooks: alias-resolve and alias-exists.
They are called during the aliases resolution process, to allow for more
complex integration with other systems, such as storing the aliases in a
database.
See the included documentation for more details.
The spf library has gained support for macros, but to process them
properly, a new function needs to be called with the full sender
address, spf.CheckHostWithSender.
This patch updates chasquid's calls to the new API.
Some tests did not make use of testlib.RemoveIfOk, which resulted in
some duplication; this patch fixes that.
While at it, userdb tests have its own simpler variant, so add some
safety checks to it.
The MTA-STS standard explicitly says the maximum max_age is 1 year.
This patch adds a check to the STS library to enforce this. Policies
with max_age > 1y will be treated as invalid.
See this email thread for some discussion on the topic:
https://mailarchive.ietf.org/arch/msg/uta/bnUjy9jxM_Va-lDXVtbB32zIkYI
