The `latest` tag is meant to track the `main` branch, but I just noticed
it hasn't been pushed out in a while. This is because the conditional
gating the push on the branch being `main` is incorrect.
This patch fixes the problem by using the correct conditional on the
branch name.
The current .gitignore pattern doesn't work when the private test cases
are a symlink, which can be convenient.
This patch fixes it by changing the pattern to match symlinks as well as
directories.
This patch adds a cross-tool integration check that uses
driusan/dkim's dkimverify to confirm it can verify our own DKIM signatures.
It is optional, since the tool may not be present.
This patch removes the integration tests that covered using driusan/dkim
and dkimpy's tools in the example hook.
Now that we have internal DKIM support, the example hook doesn't attempt
to use them, so we can remove the tests that cover it.
Those tools, and other DKIM implementations, can still be used in the
post-data hook just as before.
To send mails, today some tests use msmtp and others our internal smtpc.py.
This works, but msmtp slows down the tests significantly, and smtpc.py
is also not particularly fast, and also has some limitations.
This patch introduces a new SMTP client tool written in Go, and makes
almost all the tests use it.
Some tests still remain on msmtp, mainly for client-check compatibility.
It's likely that this will be moved in later patches to a separate
special-purpose test.
With this patch, integration tests take ~20% less time than before.
Dovecot applies an authentication penalty, where it delays failed attempts.
Because we intentionally do bad authentications for testing, this slows
downs the tests significantly. So this patch disables it.
Our tests invoke a variety of helpers, some of them are written in Go.
Today, we call "go build" (directly or indirectly via "go run"), which is
a bit wasteful and slows down the tests.
This patch makes the tests only build our Go helpers once every 10s at
most.
The solution is a bit hacky but in the context of these tests, it's
practical.
The generate_cert cache has a bug because it uses the directory's age,
which won't necessarily change, and it was always re-generating
certificates after 10m.
This patch fixes the bug by checking the age of the private key file
instead of the directory.
This patch adds tests to verify how safeio behaves when *os.File
operations return various errors.
To do this, we allow the possibility of wrapping os.CreateTemp, so we
can simulate the errors during testing.
This is not pretty, but the code is small enough that the readability
overhead is minimal, and we get a lot of tests from it.
We've had a couple of reported issues about the difficulty of setting up
new clients, or confusion due to using broken clients:
- https://github.com/albertito/chasquid/pull/46
- https://github.com/albertito/chasquid/issues/52
This patch adds the first version of a "Clients" document that includes
requirements for all clients, configuration examples, and a list of
known-problematic client software.
The goal is to help reduce friction and confusion when setting up
clients.
The document needs more polishing and examples, which hopefully will be
added later.
Fixes https://github.com/albertito/chasquid/issues/48.
Nearly all the github actions we rely on have increased their major
version, at least to update the Node.js version they run on, since the
previous one is being deprecated and will eventually become unsupported.
So this patch updates all the versions of the github actions we use.
Unfortunately, but understandably, Cirrus CI no longer offers enough
free compute credits to run our tests reliably.
They're currently only used to run the Go tests on FreeBSD.
In the future, this might be replaced with something else; but until a
proper replacement can be found and tested, remove it to avoid
introducing noise in the CI results.
Since we moved the Docker workflows to Github (after v1.10), they have
not been running on tags, so there are no tagged docker images for
v1.11, v1.12 and v1.13.
This is (hopefully) because we're not explicitly asking for the workflow
to be run on tag pushes.
This patch (hopefully) fixes that by adding an explicit section in the
config to make it run on tag pushes.
Thanks to Christoph Mewes (xrstf@github) for reporting this in
https://github.com/albertito/chasquid/issues/51.
The SMTP smuggling vulnerability fixed in 1.13 (and 1.11.1) has been
given a CVE number: CVE-2023-52354
(https://nvd.nist.gov/vuln/detail/CVE-2023-52354).
This patch adds a link to it in the release notes, for ease of reference.
This prevents chasquid from attempting to look for certs under a
non-directory, e.g. `/etc/chasquid/domains/.gitignore/certs`.
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Adjusted commit message, applied `go fmt`.
chasquid v1.11.1 was released on 2023-12-26 with a backport of the
security fixes from 1.13.
This was requested by users of Debian stable, who are on 1.11.
The RFCs are very clear that in DATA contents:
> CR and LF MUST only occur together as CRLF; they MUST NOT appear
> independently in the body.
https://www.rfc-editor.org/rfc/rfc5322#section-2.3https://www.rfc-editor.org/rfc/rfc5321#section-2.3.8
Allowing "independent" CR and LF can cause a number of problems.
In particular, there is a new "SMTP smuggling attack" published recently
that involves the server incorrectly parsing the end of DATA marker
`\r\n.\r\n`, which an attacker can exploit to impersonate a server when
email is transmitted server-to-server.
https://www.postfix.org/smtp-smuggling.htmlhttps://sec-consult.com/blog/detail/smtp-smuggling-spoofing-e-mails-worldwide/
Currently, chasquid is vulnerable to this attack, because Go's standard
libraries net/textproto and net/mail do not enforce CRLF strictly.
This patch fixes the problem by introducing a new "dot reader" function
that strictly enforces CRLF when reading dot-terminated data, used in
the DATA input processing.
When an invalid newline terminator is found, the connection is aborted
immediately because we cannot safely recover from that state.
We still keep the internal representation as LF-terminated for
convenience and simplicity.
However, the MDA courier is changed to pass CRLF-terminated lines, since
that is an external program which could be strict when receiving email
messages.
See https://github.com/albertito/chasquid/issues/47 for more details and
discussion.
This patch makes mail_diff more strict in its parsing, to ensure we
catch any encoding issues that may otherwise be masked by the default
compatibility policy.
The minor dialogs test covers some very specific SMTP exchanges, and
some of those include delivering email.
Today, we don't verify the final mailbox, we just check the SMTP
exchange. However, it can be very useful for some of the tests to do
end-to-end checking of the final mailbox.
This patch implements that ability in the test itself, and on the
(currently only) email delivering dialog.
Subsequent patches that introduce new tests will make use of this
feature.
Some use cases, like receive-only MTAs, need domain users for receiving
emails, but have no real need for passwords since they will never use
submission.
Today, that is not supported, and those use-cases require the
administrator to come up with a password unnecessarily, adding
complexity and possibly risk.
This patch implements "receive-only users", which don't have a valid
password, thus exist for the purposes of delivering mail, but always
fail authentication.
See https://github.com/albertito/chasquid/issues/44 for more details and
rationale.
Thanks to xavierg who suggested this feature on IRC.
Using an empty listening address will result in chasquid listening on a
random port, which is a dangerous misconfiguration.
That is most likely done to prevent it from listening at all.
To prevent this misconfiguration, explicitly reject empty listening
addresses early and with a warning, so there is no ambiguity.
Users can still prevent chasquid from listening by just commenting out
the entry in the config (and not passing any systemd file descriptors).
See https://github.com/albertito/chasquid/issues/45 for more details and
discussion, including alternatives considered.
Thanks to xavierg who reported this via IRC.
When logging the configuration, we currently don't quote the string
values, which can make whitespace-induced problems difficult to identify
and troubleshoot.
This patch changes the formatting to always quote string values when
logging the configuration.
When a single dovecot user exists and their password is being updated via
docker/add-user.sh, the `grep -v` command intended to remove the user's
old password will not match any lines and exit with error code 1, causing
the entire script to fail.
This patch fixes it by replacing the if-grep logic with a simpler sed
invocation.
https://github.com/albertito/chasquid/pull/43
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Minor edits to the commit message.
This patch extends docker/add-user.sh to support getting the email and
password from environment variables.
That way, docker/add-user.sh can be used in scripts.
https://github.com/albertito/chasquid/pull/43
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Minor edits to the commit message.
There is a bug causing an invalid config in the generated
/etc/dovecot/auto-ssl.conf, e.g.:
ssl_cert = </etc/letsencrypt/live//etc/letsencrypt/live/mail.grouplok.com/fullchain.pem
ssl_key = </etc/letsencrypt/live//etc/letsencrypt/live/mail.grouplok.com/privkey.pem
This patch fixes it by using the domain name instead of the path, which
matches the original intent.
https://github.com/albertito/chasquid/pull/42
Amended-by: Alberto Bertogli <albertito@blitiri.com.ar>
Minor edits to the commit message.
This patch adds the embedded aliases file to the fuzz corpus, because it
is trivial to do so, and is a reasonable seed which will be naturally
adjusted over time as the package evolves (as it happened in recent
commits).
The testing package does not allow t.Fatal to be called from a different
goroutine; however, we do that if the testing server fails listen or
accept connections.
Since that is unexpected and rare, this patch turns those calls into
panics.
Today, when a user sets an alias with drop characters and/or suffixes,
those go unused, since we always "clean" addresses before alias
resolution.
This results in unexpected and surprising behaviour, and it's not
properly documented either.
This patch resolves this unexpected behaviour as follows:
- Drop characters are ignored, both at parsing time and at lookup time.
- Lookups are done including the suffixes first, and if that results in
no matches, they are retried without suffixes.
This results in aliases working more intuitively for the most common use
cases: of users wanting to have different aliases for specific suffixes,
and not having to care for drop characters.
Hooks can be used to get different behaviour if needed, since the first
lookup is done with the address as-is.
Thanks to znerol@ (lo+github@znerol.ch) for reporting this, and the
discussion on how to fix it, in
https://github.com/albertito/chasquid/issues/41.
Today, the parsing functions are stand-alone since they don't need
anything from the resolver.
But in future patches, that will change.
In anticipation of that, move those functions to be methods of the
resolver.
