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.
The daemon attempts to change to the config directory on startup, for
security and convenience.
We currently don't check if this works, which is not a big deal since it
will just fail later on when it can't find the files. However, it makes
things more awkward to debug, so this patch adds an explicit check.
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.
When creating a database directory, we were missing the check to see if
it had succeeded, which would make issues more difficult to troubleshoot.
This patch adds the missing check.
docopt.Parse is deprecated. This patch updates the code to the newer
variant, ParseDoc, since the default options are what we want.
There are no functional changes.
Currently the modules are ignored in the Go 1.11 build, because the
files are within $GOPATH.
This causes problems when some dependencies are updated in
backwards-incompatible ways, and assuming that Go modules are being
used. In particular, the new protobuf release caused this problem which
was caught by the automated builds:
https://travis-ci.org/github/albertito/chasquid/jobs/674701956.
This patch enables Go modules in 1.11 builds.
Thanks to Jonas Seydel (thor77) for the help investigating and finding a
fix for this problem.
There is an AUR package for chasquid, so this patch adds references to
it in the documentation.
Thanks to Max Mazurov (fox.cpp@disroot.org) for adding the package.
If the load generator is sending emails too fast, chasquid queue might
hit the maximum size and fail the test.
This patch makes it sleep and retry, to give the server some time to
catch up.
Thanks to Max Mazurov (fox.cpp@disroot.org) for reporting this problem.
It can be convenient to upload images to dockerhub for redundancy and
visibility, so this patch updates the gitlab CI configuration to do
that.
While at it, rename the stages for readability.
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.
This patch adds a section on dovecot authentication troubleshooting,
with common suggestions that can help identify what is going on when
the chasquid-dovecot interaction isn't working as expected.
When an alias has a remote destination, chasquid uses sender rewriting
(also known as SRS [1]) to forward the email without risking being in
violation of SPF policies.
See https://en.wikipedia.org/wiki/Sender_Rewriting_Scheme for more
details.
This, however, wasn't documented anywhere, as noted in
https://github.com/albertito/chasquid/issues/6.
This patch adds a paragraph to the alias documentation explaining this
behaviour.
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.
The example post-data hook was missing a quote around a sub-shell
execution.
This is harmless because the content itself is admin-provided and not
related to user input, but this commit fixes the quote for defense in
depth and consistency.
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.
This patch adds a new integration test to cover SPF checks. The main
goal is not to cover the SPF parsing, since that's handled by the
library already, but the higher level aspects: that the mails are indeed
rejected, that the DSN looks reasonable, etc.
Hook output is checked to see if it looks like a header, which includes
the possibility of multi-line headers.
This patch extends the tests to include a multi-line header, to prevent
accidental regressions.
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).
We want Travis CI to check against the Go version shipped in the latest
Debian stable, to make sure chasquid can be built and run there.
There was a new Debian release which has Go 1.11, so raise the CI config
version accordingly.
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.
