This patch groups a series of commands that redirect to a file into a
single grouped redirect, for readability reasons.
Suggested by shellcheck SC2129.
In a few places, we call Printf-like functions, but for the format we
use either non-format messages (which is not tidy, but okay), or
variable messages (which can be problematic if they contain %-format
directives).
The patch fixes the calls by either moving to Print-like functions, or
using `Printf("%s", message)` instead.
These were found by a combination of `go vet` (which complains about
"non-constant format string in call"), and manual inspection.
staticcheck found a couple of minor code cleanup improvements, like
unused variables or an out-of-order defer, mostly in tests.
This patch fixes those problems by making the necessary adjustments.
They're all fairly small, and should not change the logic in any
significant way.
There are a couple of places in the tests when we attempt to build and
run simultaneously. Here, if the build is slow, there is a race where
"text file busy" can appear.
To fix this, build to a temporary file with a random name, then
atomically rename it to the final binary name.
When we put something in the queue and respond "250 ok" to the client,
that is taken as accepting the email.
As part of putting something in the queue, we write it to disk, but
today we don't do an fsync on that file.
That leaves a gap where a badly timed crash on some systems could lead
to the file being empty, causing us to lose an email that we accepted.
To elliminate (or drastically reduce on some filesystems) the chances of
that situation, we call fsync on the file that gets written when we put
something in the queue.
Thanks to nolanl@github for reporting this in
https://github.com/albertito/chasquid/issues/78.
The current fail2ban regexp catches all SMTP connection errors.
This works fine, but includes connection errors, that can be caused by
transient external causes, and accidentally delay email delivery.
This patch changes the regexp to be more targeted towards specific SMTP
errors that are likely to be caused by deliberate actions.
The expression was cross-checked with a few month of errors to confirm
it should not have false positives, and that it correctly left
connection errors alone.
Thanks to pepperbob@github for reporting this in
https://github.com/albertito/chasquid/issues/77.
When constructing the "Received" header, in some cases we want to
include the remote IP address in addition to the EHLO domain.
The way we did that is not fully compliant with RFC 5321 (section 4.4),
and this has the potential to confuse some tools that parse the header.
This patch fixes this problem by adjusting the order of the two pieces
of data, which makes it comply with the RFC.
Before:
Received: from [1.2.3.4] (ehlo.domain.example.com)
After:
Received: from ehlo.domain.example.com ([1.2.3.4])
Thanks to nolanl@github for reporting this problem in
https://github.com/albertito/chasquid/issues/76.
On NixOS, `/bin` is basically empty and this causes the courier tests
(which invoke `/bin/sleep`) to fail.
This patch fixes the tests by removing the hardcoded path.
https://github.com/albertito/chasquid/pull/73
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message.
Currently, we rely on Debian to pick a UID and GID for daemon users.
However, those numbers can change as software evolves over time, in
particular as the base distribution changes.
Because those IDs are relevant in the data volume, which has a lifetime
independent from the daemon container, it is important that they don't
change.
Other projects have run into this issue over the years too, this is not
a purely theoretical concern.
This patch fixes the UID/GIDs for the daemon users to their current
values, to prevent problems in the future.
See https://github.com/albertito/chasquid/pull/72 for further
discussion.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message, formatted RUN command line, changed the
dovecot group ID to match the previous value.
Using the "slim" version of the debian:stable image helps reduce size,
while having no impact on runtime usability or performance. The main
differences are around locale and manpages.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message.
Today, we launch dovecot in the background and chasquid in the
foreground using sudo.
This means that dovecot failures won't propagate, and signals to the
container (e.g. to stop it) also don't get propagated to dovecot
(because it's in the background) or chasquid (because they don't go
beyond the sudo process).
Thanks to [Guiorgy@github](https://github.com/Guiorgy) for identifying
the problem, proposing alternatives, help debugging, and discussing this
in https://github.com/albertito/chasquid/pull/70.
When we get SIGINT or SIGTERM, today chasquid exits with code 1. This
can confuse some of the supervision tools.
In particular, Docker and Kubernetes expect exit 0 upon an intentional
stop.
And systemd, the Restart= semantics make a difference with 0 and non-0,
and exiting with 1 prevents users from making that distinction.
This patch changes the SIGINT/SIGTERM exit code to 0, to make it easier
for users to set up things as desired in those environments.
Thanks to [Guiorgy@github](https://github.com/Guiorgy) for reporting
this problem in https://github.com/albertito/chasquid/pull/70.
From the Dockerfile docs:
> Environment variable persistence can cause unexpected side effects.
> For example, setting ENV DEBIAN_FRONTEND=noninteractive changes the
> behavior of apt-get, and may confuse users of your image.
>
> If an environment variable is only needed during build, and not in the
> final image, consider setting a value for a single command instead.
So this patch adjusts the use of the DEBIAN_FRONTEND variable to match
the documented best practice.
https://github.com/albertito/chasquid/pull/68
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message.
This patch fixes some Dockerfile style warnings:
- `WARN: FromAsCasing: 'as' and 'FROM' keywords' casing do not match`
- `LegacyKeyValueFormat: "ENV key=value" should be used instead of
legacy "ENV key value" format`
https://github.com/albertito/chasquid/pull/68
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message.
When creating containers to run a single one-off command that alters a
volume, the `--rm` option is needed, otherwise that container is left
around and can cause confusion later on.
https://github.com/albertito/chasquid/pull/69
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message.
When using git on Windows, git may try to "fix" line endings to CRLF.
Then, when building the Docker image, the files copied can end up having
the wrong line ending, which causes scripts to fail to run.
This patch fixes the problem by using .gitattributes to indicate to git
which line ending to use for the files in the docker/ directory.
https://github.com/albertito/chasquid/pull/66
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message, extended comment on .gitattributes.
Dovecot 2.4 has a new configuration format, which is unfortunately
backwards-incompatible with Dovecot 2.3.
This patch adds a 2.4-compatible config, and selects which one to use
based on the Dovecot version in the environment.
In the future, once 2.4 becomes more common, we will drop the 2.3 config
from the test.
Note that we don't change the config used in the Docker image, because
that is based on Debian **stable** which is still on 2.3.
The manpages are generated from pod files using pod2man, but it has been
a long time since we last re-generated them, and the new versions of
pod2man generate significantly different (and simpler) pages.
So this patch just regenerates the man pages, to make future changes
easier and more self contained to review.
Today, the maximum number of items in the queue, as well as how long we
keep attempting to send each item, is hard-coded and not changed by end
users.
While they are totally adequate for chasquid's main use cases, it can
still be useful for some users to change them.
So this patch adds two new configuration options for those settings.
They're marked experimental for now, so we can adjust them if needed
after they get more exposure.
Thanks to Lewis Ross-Jones <lewis_r_j@hotmail.com> for suggesting this
improvement, and help with testing it.
This patch implements "via" aliases, which let us explicitly select a
server to use for delivery.
This feature is useful in different scenarios, such as a secondary MX
server that forwards all incoming email to a primary.
For now, it is experimental and the syntax and semantics are subject to
change.
We have a few Python scripts which over the years ended up with a
variety of formatting.
This patch auto-formats them using `black -l 79` to make them more
uniform, and easier to read and write.
minidns supports MX records, but today it hard-codes priority=10.
This is limiting when creating test scenarios that depend on having
different MX priorities.
This patch adds support for specifying the priority in MX records.
In the tests, we create a lot of Recipient{}s, and that ends up being
very verbose and sometimes cumbersome.
Also, if we ever want to extend it, it would result in a lot of
unnecessary refactoring.
So this patch replaces the Recipient{} instantiations with helper
functions, to help readability and extendability.
This only affects the tests, and there are no changes to them either, it
is purely a refactor.
Today, aliases parsing is too lax, silently ignoring most kinds of invalid
lines.
That behaviour can cause a lot of confusion when users think the aliases
are being parsed, and also cause problems when extending the syntax.
This patch fixes that problem by making aliases parsing return errors on
the invalid lines.
Unfortunately this will cause some previously-accepted files to now be
rejected, but it should be quite visible.
This patch implements support for aliases that contain '*' as the
destination user.
In that case, we replace it with the original user.
For example, `*: *@pond` will redirect `lilly@domain` to `lilly@pond`.
This is experimental for now, and marked as such in the documentation.
The semantics can be subtle, so we may need to adjust them later.
This patch fixes some minor typos in comments and strings found by
codespell.
While at it, also expand some variable names that were not typos, but
caused false positives, and end up being more readable anyway.
Today, we only require that Docker integration tests use a Go version
>= 1.20. In practice, it almost always picks the latest version.
For consistency, this patch requests it to run the latest Go version.
Today, the step to build the Docker public image depends on the coverage
run. This dependency isn't necessary, as the coverage could be failing
for a variety of reasons (e.g. codecov being down) and doesn't signal
any problem with chasquid itself.
So this patch fixes that: if the integration tests pass, then that is
good enough for building the public image.
There's an inconsistency between chasquid (which uses `--config_dir`) and
chasquid-util (which uses `--configdir`).
That is prone to cause confusion, so this patch renames chasquid-util's
flag, leaving the old one as deprecated with a warning message.
Closes https://github.com/albertito/chasquid/pull/60.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Added test case for the deprecated option, adjusted commit message.
Microsoft SMTP servers have a bug that prevents them from successfully
establishing a TLS connection against modern Go TLS servers, and some
OpenSSL versions. It also doesn't fall back to plain-text, so this has
been causing deliverablity issues.
The problem started by the end of 2024 and it's still not fixed.
Unfortunately, because they're quite a big provider and are not fixing
their problem, it is worth to do a server-side workaround.
This patch implements that workaround: it disables TLS session tickets.
There is no security impact for doing so, and there is a small
performance penalty which is likely to be insignificant for chasquid's
main use cases.
This workaround should be removed once Microsoft fixes their problem.
We are going to make a 1.15.1 release for this, which this patch also
documents.
Thanks to Michael (l6d-dev@github) for reporting this issue and
suggesting this workaround!
See https://github.com/albertito/chasquid/issues/64 and
https://github.com/golang/go/issues/70232 for more details.
This commit updates the uses of math/rand to math/rand/v2, which was
released in Go 1.22 (2024-02).
The new package is generally safer, see https://go.dev/blog/randv2 for
the details.
There are no user-visible changes, it is only adjusting the name of
functions, simplify some code thanks to v2 having a better API, etc.
Authenticated users are intentionally allowed to send email as other users or
domains. This is a design choice made to balance simplicity of operation and
use.
However, it can be surprising and it's not obvious, so this patch adds a
note to the documentation about it.
Thanks to Matěj Volf for suggesting this improvement!
Fixes: https://github.com/albertito/chasquid/issues/62
This patch adds a document with guidelines for contributing to chasquid.
It includes suggestions for how to ask questions, how to send patches
(and the expectations around them), and documents how the different
branches are used.
Thanks to raspbeguy (https://github.com/raspbeguy) for suggesting this
improvement.
`chasquid-util user-add` is meant to create the domain directory if it
doesn't exist; however there's a bug that makes this not happen, and
instead the command fails with:
Error writing database: open <path>: no such file or directory
This patch fixes the issue and adds a test to ensure we don't have any
regressions on this behaviour.
Thanks to raspbeguy (https://github.com/raspbeguy) for reporting this
issue (on IRC).
