GitLab

This patch adds some basic instructions to the documentation on how to
set up DKIM, using the tools supported by the example hook.

It's not meant to be a full DKIM how-to, but to help someone who already
knows enough, or who is complementing it with a more general purpose
DKIM guide.

This patch adds support in the default hook for using dkimpy for DKIM
signing.

Unfortunately, dkimpy binaries have the same name as driusan/dkim's, so
we need to use --help to disambiguate. It's not pretty but it should
work, and is quite self contained.

Also, for the integration tests, we still need driusan/dkim because
dkimpy lacks the features needed. Specifically, dkimpy's dkimverify
can't be made to use custom DNS, or override the TXT values in any way,
so we can't verify that the generated signature is reasonable.

Thanks to ne9z@github for suggesting this change and providing an
alternative patch in https://github.com/albertito/chasquid/pull/19.

Most integration tests depend on the $HOSTALIASES environment variable
being functional. That variable works on most systems, but not all. In
particular, systems with `systemd-resolved` can cause the variable to be
ignored.

This was reported by Alex Ellwein in
https://github.com/albertito/chasquid/issues/20.

This patch makes the affected tests to be skipped if $HOSTALIASES is not
working properly. It also removes unnecessary hosts files from tests
which don't need it, and documents this behaviour.

Thanks to Alex Ellwein and foxcpp@ for reporting and helping investigate
this issue!

The chasquid-rspamd utility (https://github.com/Thor77/chasquid-rspamd)
provides a better integration with rspamd, by taking envelope and
connection information from the environment variables, and communicating
with rspamd using its protocol.

So if it is available, use it instead of rspamc in the default hook.

Some LMTP servers (like dovecot) can't handle UTF8 addresses in the LMTP
commands. This can be problematic if we want to use them with UTF8
domains or usernames, which are well supported by chasquid.

To help workaround this issue, this patch adds a new -to_puny flag for
mda-lmtp, that makes it encode `from` and `recipient` in punycode.

That way, the server will get punycode-encoded (ASCII) strings in the
LTMP commands.

This can be particularly convenient when the recipients are ASCII
(because they're under the mail server control), but `from` may not be
(because it comes from the network).

By default, golang.org/x/net/trace currently only allows the tracing
pages to be seen from localhost.

This restriction can be confusing for people accessing the monitoring
server remotely, and adds no value in our environment.

The monitoring server already exports very sensitive information, and
must be enabled with care, and is not on by default. This is well
documented.

This patch removes the restriction, making all the monitoring pages
equally accessible.

Some deployments already have users that authenticate without a domain.
Today, we refuse to even consider those, and reject them at parsing time.

However, it is a use-case worth supporting, at least with some
restrictions that make the complexity manageable.

This patch changes the auth package to support authenticating users
without an "@domaiN" part.

Those requests will always be directly passed on to the fallback
authenticator, if available.

The dovecot fallback authenticator can already handle this case just fine.

The openmetrics proposed standard says we should use the
`application/openmetrics-text` content type when exporting the metrics.

Currently we use `text/plain` for backwards compatibility with
Prometheus, but the new content type is apparently supported since 2018,
so it should be safe to update to match the current proposed standard.

To help with defense-in-depth on cross-protocol attacks (e.g.
https://alpaca-attack.com/), this patch makes chasquid reject HTTP
commands.

Today, we close the connection after 10 errors. While this is fine for
normal use, it is unnecessarily large.

Lowering it to 3 helps with defense-in-depth for cross-protocol attacks
(e.g. https://alpaca-attack.com/), while still being large enough for
useful troubleshooting and normal operation.

As part of this change, we also remove the AUTH-specific failures limit,
because they're covered by the connection limit.

When we receive unknown commands, we use the first 6 bytes for
troubleshooting (e.g. put them in traces and exported metrics).

While this is safe, since the different places know how to quote them
properly, it makes things more difficult to analyse, since it's not
uncommon to see be binary blobs.

This patch makes us use the ascii-quoted version instead, to make things
easier to analyze.

This patch adds a missing golang.org/x/tools sum, that is used by the
coverage view generator, so it was missed in the last general update.

This patch extends the README to mention explicitly that reporting bugs
and sending patches on GitHub is welcome, and also adds a private email
where to report security issues.

The changes matches the common practice so far, but it's useful to have
it explicitly documented.

This patch adds tracing for the auth and domaininfo modules. In the
latter, we replace the long-running event with the short-term request
tracing, which is more practical and useful.

There are no logic changes, it only adds tracing instrumentation to help
troubleshooting.

The Linux tests under the Cirrus CI are currently brittle due to
environmental issues. They're also redundant, since Linux testing is
much better covered by the GitLab CI tests.

So this patch removes them, which removes the false positives and speeds
up the Cirrus CI runs.

This patch does a general pass updating Go modules to recent versions,
and regenerates the protobufs accordingly.

The main purpose is to make sure people building from source are using
relatively recent versions of our dependencies.

We also regenerate protobufs since the newer versions of the liberaries
have a much cleaner dependency tree, which speeds up fetches.

When we fail to check if a user exists, we currently return a permanent
error, which can be misleading and also make things more difficult to
troubleshoot.

This patch makes chasquid return a temporary error in that case.

Thanks to Thor77 (thor77@thor77.org) for suggesting this change.

Currently, chasquid attempts to auto-detect dovecot sockets when
starting up (if needed). If autodetection fails, chasquid emits an
error, continues serving, and never tries again.

This can be problematic if chasquid starts up before dovecot, and at the
time the dovecot sockets are not present (e.g. after a reboot). In that
case, chasquid will not use dovecot for authentication even after
dovecot has started.

This patch changes the autodetect logic, by doing autodetection at
startup and on each request, until we find a working pair of sockets.
Once we do, they're used consistently.

That way, if dovecot is not ready when chasquid starts, it's not a
problem and chasquid will start using dovecot once it becomes available.

Thanks to Thor77 (thor77@thor77.org) for reporting and helping
troubleshoot this issue.

There's a known issue in versions 0.07 to 1.5 where the post-data hook
invocation can fail if the dkimsign binary exists, due to a bug in the
post-data hook check.

This was fixed by commit b6248f30, but it is found on occasion since the
current Debian stable ships 0.07, and Ubuntu 20.04 LTS ships 1.2.

So this patch adds it to the known issues list.

It's common that people running old releases (for example, because of
their Linux distribution version) run into issues that have already been
fixed.

It can be convenient to have a list of the most common known issues and,
when available, their workarounds.

This patch creates the documentation page for them, currently empty. It
will be filled in subsequent patches.

The IRC channel has been moved to OFTC, reflect that in the
documentation.

This patch adds a new link to the RBL checking suggestion, since the
existing one doesn't work with IPv6, and it's important to get good
coverage.

While at it, it also fixes the path to mda-lmtp, which was wrong before.

In commit 5305d584 we fixed an issue with the way the Docker image
adds the "hostname" option to chasquid.conf.

Currently, the Docker entrypoint sets the "hostname" option in
chasquid.conf if it's missing.

That works fine, except when there is a configuration change and the
domain is removed. In that case, the hostname option will have a stale
value, forcing the user to re-create the container, which can be
cumbersome.

This patch fixes the issue by unconditionally setting the hostname
option to one of the available domains at the time of start up.

Thanks to Jaywann@github for finding and reporting this problem on
https://github.com/albertito/chasquid/issues/16, and suggesting an
alternative fix!

In Go 1.16, "go get" on non-module paths now require an explicit version
to point to.  Without a specific version, the invocation fails.

See https://golang.org/doc/go1.16#go-command for more details on the
change.

The test Dockerfile uses "go get" to fetch driusan/dkim's binaries, used
for integration testing.

So this patch adjusts the Dockerfile to fetch the latest version.

When the chasquid docker container is restarted, entrypoint.sh will add
the hostname again, even if it is present.

This causes chasquid to fail to start due to the duplicated option
(`non-repeated field "hostname" is repeated`).

Thanks to Jaywann@github for finding and reporting this problem, on
https://github.com/albertito/chasquid/issues/16.

This patch fixes the issue by only adding the option if it isn't already
present.

The docopt-go library is quite convenient, but it has been abandoned for
a while :(

Since we only use it for chasquid-util, this patch removes it and
replaces it with a custom small parser, that is a reasonable fit for the
required use cases.

The patch also adds a couple of tests to increase coverage.

NOTE: docopt-go accepted some undocumented behaviour, in particular the
use of "-a b" instead of "-a=b". The new parser does not, so some
user scripts may require updating.

I think this should be rare enough not to be worth the complexity of
adjusting the parser to allow it.

This patch adds a minor test to dovecot-auth-cli to verify that the
check for invalid number of arguments is working as expected.

It's mostly for consistency, as the utility is only used for testing
purposes.

This patch adds the EOF marker as required by the new specification, and
also adds some links to it in the comments, as reference.

The image jobs should only run if there are valid credentials for
pushing the images to the respective registries, to avoid false
negatives in the test pipeline.

This can happen when the gitlab CI is run on projects that aren't set up
to push docker images, either because they're clones of the official
repo, or they are under a different gitlab instance (e.g. Debian's
salsa).

We do it by using a "rules:if" clause on specific variables:
- for Docker, $DOCKER_REGISTRY_USER which is set externally
- for GitLab, $CI_REGISTRY_IMAGE which has the address of the registry
  tied to the project.

Note that for GitLab we can't use the credentials for conditional
execution directly, since they are "persisted variables" which are not
available in this context (see [1] for more details). The
$CI_REGISTRY_IMAGE should be good enough to determine whether image
registry is enabled for the repo.

[1]: https://docs.gitlab.com/ee/ci/variables/where_variables_can_be_used.html#persisted-variables

fexp is a testing utility, including it in the regular Go build confuses
some automation as it can think it's part of chasquid proper.

All other testing utilities are ignored via the "+build ignore"
annotation for this reason, so this patch adds it to fexp to fix this
issue.

The haproxy test config includes an obsolete "debug" entry, and is
missing some timeouts which, while harmless in this context, cause a
warning that can be confusing.

This patch fixes the debug entry by running haproxy -d as recommended,
and adds the essential timeouts to avoid the warning.

To debug test failures, it can be convenient to explore the contents of
the test directories after the test runs, as they contain logs and
generated files.

This patch configures the GitLab CI to export the repo tree (which
includes the test directory) as GitLab CI artifacts, so they can be
easily accessed after the tests have completed.

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 makes it more clear how to specify which domain the user being
operated on is the sub-command targeting when using `--help`.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
  Update code to match the updated parameter name.

Allows terminating chasquid via the network. Useful to trigger a restart
(if there is an init system to relaunch chasquid) and thus reload certificates.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
  Added tests, and adjusted shutdown sequence.

This patch removes the dependency on wget for fetching content over
http, which was used in one of the tests to do some checking on debug
and metric pages, as well as loop detection.

Instead of wget, we now use a small built-in utility called fexp.

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.