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.
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.
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.
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.
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.
The aliases.Resolver.Exists function currently returns the "clean"
address (with the drop characters and suffixes removed), which is relied
upon in its only caller.
That, however, makes the logic more difficult to follow, hiding some
of the address manipulation behind what should be a read-only check.
So this patch reorganizes that code a little bit, removing the
"cleaning" of the address as part of Exists, and making it explicit when
needed instead.
This patch does not have any user-visible change in behaviour, it is
just internal reorganization.
This is in preparation for further patches which will improve the
handling of some aliases corner cases.
This patch makes chasquid run a localrpc server, exporting two methods:
alias resolve, and domaininfo clear.
They will be used by chasquid-util in later patches.
This patch adds a new package for doing local lightweight RPC calls over UNIX
sockets. This will be used in later patches for communication between chasquid
and chasquid-util.
This patch adds a Clear method to the domaininfo database, which removes
information for the given domain.
This can be used to manually make the server forget about a domain, in
case there are operational reasons to do so.
Today, this is done via chasquid-util (which removes the backing file),
but that is hacky, and this is part of replacing it with a cleaner
implementation.
It is not expected that the user modifies the domaininfo database behind
chasquid's back, and reloading it can be somewhat expensive.
So this patch removes the periodic reload, and instead makes it triggered
by SIGHUP so the user can trigger a reload manually if needed.
This patch regenerates the auto-generated files.
There are no significant changes, the protobuf just get an updated
comment due to protoc version change, but it is just informational.
Two new TLS ciphers are added, matching the new IANA assignments.
When the SMTP courier gets an error on STARTTLS (either because the
command itself failed, or because there was a low-level TLS negotiation
error), today we just fail that attempt.
This can cause messages to never be delivered if the underlying reason
is a server misconfiguration (e.g. a server certificate that Go cannot
parse). This is quite rare in practice, but it can happen.
To prevent this situation, this patch adds logic so that the SMTP
courier retries over plaintext when STARTTLS fails.
This is still subject to security level checks, so this type of failures
cannot be used to downgrade connections to domains we successfully
established a TLS connection previously.
Note that certificate validation issues are NOT included in this
type of failure, so they will not trigger a retry. The certificate
validation handling is unchanged by this patch.
This patch changes several internal packages to receive and pass tracing
annotations, making use of the new tracing library, so we can have
better debugging information.
This commit introduces a new tracing library, that replaces
golang.org/x/net/trace, and supports (amongts other thing) nested
traces.
This is a minimal change, future patches will make use of the new
functionality.
ioutil package was deprecated in Go 1.16, replace all uses with their
respective replacements.
This patch was generated with a combination of `gofmt -r`, `eg`, and
manually (for `ioutil.ReadDir`).
This patch is the result of running Go 1.19's `gofmt` on the codebase,
which automatically updates all Go doc comments to the new format.
https://tip.golang.org/doc/go1.19#go-doc
We've accumulated a few linter issues around comments and a couple of
variable names.
While none of them is major, this patch cleans them up so it's easier to
go through the linter output, and we can start being more strict about
it.
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.
This patch implements support for catch-all aliases, where users can add
a `*: destination` alias. Mails sent to unknown users (or other aliases)
will not be rejected, but sent to the indicated destination instead.
Please see https://github.com/albertito/chasquid/issues/23 and
https://github.com/albertito/chasquid/pull/24 for more discussion and
background.
Thanks to Alex Ellwein (aellwein@github) for the alternative patch and
help with testing; and to ThinkChaos (ThinkChaos@github) for help with
testing.
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.
This patch skips the resolution logic if the address is not local.
Today, the resolution logic handles that case transparently, and returns
the original email address, so this should be a no-op.
However, having an explicit early check makes the resolution logic more
robust, and will simplify future patches.
Note this also means that the `alias-resolve` hook is no longer run for
non-local aliases, which should also help simplify their implementation.
This patch simplifies the internal alias lookup logic, unifying it
across Resolve and Exists.
As part of this, the `alias-exists` hook is removed. It was redundant to
begin with, although it enabled a potential optimization, it isn't worth
the complexity. The timeout for execution of both was the same.
This change should be backwards-compatible because `alias-resolve` is
still used, and the semantics haven't changed.
If the `drop_characters` or `suffix_separators` options are set to "",
currently instead of the empty string, their default value is used instead.
This is a bug, and it also happens on other config options, but because
the others have to be set in order for chasquid to function, it's not a
problem in practice.
Thanks Björn Busse (bbusse@github) for finding and reporting this
problem, on irc and in https://github.com/albertito/chasquid/issues/25.
This patch fixes the problem by marking these fields explicitly
optional, which enables presence testing, as described in the protobuf
documentation:
https://github.com/protocolbuffers/protobuf/blob/master/docs/field_presence.md.
This patch adds tests for STS policy checks in combination with TLS
security levels.
This helps ensure we're detecting mis-matches of TLS status
(plain/insecure/secure) and STS policy enforcement.
When using STARTTLS, the SMTP courier needs to determine whether the
server certificates are valid or not.
Today, that's implemented via connecting once with full certificate
verification, and if that fails, reconnecting with verification
disabled.
This works okay in practice, but it is slower on insecure servers (due
to the reconnection), and some of them even complain because we connect
too frequently, causing delivery problems. The latter has only been
observed once, on the drv-berlin-brandenburg.de MX servers.
To improve on that situation, this patch makes the courier do the TLS
connection only once, and uses the verification results directly.
The behaviour of the server is otherwise unchanged. The only difference
is that when delivering mail to servers that have invalid certificates,
we now connect once instead of twice.
The tests are expanded to increase coverage for this particular case.
When resolving MX records, we need to distinguish between "no such
domain" and other kinds of errors. Before Go 1.13, this was not
possible, so we had a workaround that assumed any permanent error was a
"no such domain", which is not great, but functional.
Now that our minimum supported version is Go 1.15, we can remove the
workaround.
This patch replaces the workaround with proper logic using
DNSError.IsNotFound to identify NXDOMAIN results when resolving MX
records.
This requires to adjust a few tests, that used to work on environments
where resolving unknown domains (used for testing) returned a permanent
error, and now they no longer do so. Instead of relying on this
environmental property, we make the affected tests use our own DNS
server, which should make them more hermetic and reproducible.
tls.BuildNameToCertificate has been deprecated, and calling it is no
longer necessary since Go 1.14.
Now that our minimum supported Go version is 1.15, we can remove it.
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.
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 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.
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.
