884 Commits

Author SHA1 Message Date
Mechiel Lukkien
3050baa15a
consistently use store.CloseRemoveTempFile for closing and removing temp files 2025-03-06 11:35:43 +01:00
Mechiel Lukkien
b822533df3
imapserver: Don't keep account write-locked during IMAP FETCH command
We effectively held the account write-locked by using a writable transaction
while processing the FETCH command. We did this because we may have to update
\Seen flags, for non-PEEK attribute fetches. This meant other FETCHes would
block, and other write access to the account too.

We now read the messages in a read-only transaction. We gather messages that
need marking as \Seen, and make that change in one (much shorter) database
transaction at the end of the FETCH command.

In practice, it doesn't seem too sensible to mark messages as seen
automatically. Most clients probably use the PEEK-variant of attribute fetches.

Related to issue #128.
2025-03-06 11:35:43 +01:00
Mechiel Lukkien
caaace403a
Add package smtp as fuzzing target since its addition in previous commit
The previous commit fixed an array out of bounds access that resulted in a
panic on an smtpserver connection. The panic is recovered and marked as
"unhandled panic" in metrics and the connection closed.
2025-03-06 11:15:25 +01:00
Martin Holst Swende
f10bb2c1ae
smtp: add data reader fuzzer + fix OOB read 2025-03-06 09:57:13 +01:00
Mechiel Lukkien
44d37892b8
imapserver: REPLACE commands when in read-only mode should fail 2025-02-26 18:39:41 +01:00
Mechiel Lukkien
d7bd50b5a5
imapserver: fix spurious test failure due to recently added account consistency check
By removing message file while holding the account wlock. We were seeing
messages that weren't removed yet.
2025-02-26 18:33:01 +01:00
Mechiel Lukkien
f235b6ad83
imapclient: log traces of sensitive data with traceauth, and of bulk data with tracedata
Similar to the imapserver. This also fixes tracing of APPEND messages, which
was completely absent before.
2025-02-26 18:13:20 +01:00
Mechiel Lukkien
9c40205343
imapserver: Prevent spurious test failures due to compression layer being closed and TLS close-writes failing 2025-02-26 15:41:46 +01:00
Mechiel Lukkien
062c3ac182
when writing updated word counts to the junk filter, remove entries where both counts are 0
no point in keeping them around.
also pass on error when getting a word from database returned an error.
2025-02-26 15:07:27 +01:00
Mechiel Lukkien
394bdef39d
In storage consistency checks, verify the junk filter has the expected word counts
Fix up a test or two. Simplify the XOR logic when we train the junk filter:
Only if junk or nonjunk is set, but not when both (or none are set). i.e. when
the values aren't the same.

Locking the account when we do consistency checks prevents spurious test
failures that may have been introduced in the previous commit.
2025-02-26 14:44:05 +01:00
Mechiel Lukkien
aa85baf511
add consistency check (enabled during tests only) for unexpected message files in the account message directory 2025-02-26 11:40:36 +01:00
Mechiel Lukkien
17de90e29d
imapserver: Prevent spurious unhandled panics for connections with compress=deflate that break
Writing to a connection goes through the flate library to compress. That writes
the compressed bytes to the underlying connection. But that underlying
connection is wrapped to raise a panic with an i/o error instead of returning a
normal error.  Jumping out of flate leaves the internal state of the compressor
in undefined state. So far so good. But as part of cleaning up the connection,
we could try to flush output again. Specifically: If we were writing user data,
we had switched from tracing of protocol data to tracing of user data, and we
registered a defer that restored the tracing kind and flushed (to ensure data
was traced at the right level). That flush would cause a write into the
compressor again, which could panic with an out of bounds slice access due to
its inconsistent internal state.

This fix prevents that compressor panic in two ways:

1. We wrap the flate.Writer with a moxio.FlateWriter that keeps track of
   whether a panic came out of an operation on it. If so, any further operation
   raises the same panic. This prevents access to the inconsistent internal flate
   state entirely.
2. Once we raise an i/o error, we mark the connection as broken and that makes
   flushes a no-op.
2025-02-26 11:26:54 +01:00
Mechiel Lukkien
ea55c85938
for trace logging, log size of the data (but not for redacted auth data, could be a password) 2025-02-26 10:14:07 +01:00
Mechiel Lukkien
92a87acfcb
Implement IMAP REPLACE extension, RFC 8508.
REPLACE can be used to update draft messages as you are editing. Instead of
requiring an APPEND and STORE of \Deleted and EXPUNGE. REPLACE works
atomically.

It has a syntax similar to APPEND, just allows you to specify the message to
replace that's in the currently selected mailbox. The regular REPLACE-command
works on a message sequence number, the UID REPLACE commands on a uid. The
destination mailbox, of the updated message, can be different. For example to
move a draft message from the Drafts folder to the Sent folder.

We have to do quite some bookkeeping, e.g. for updating (message) counts for
the mailbox, checking quota, un/retraining the junk filter. During a
synchronizing literal, we check the parameters early and reject if the replace
would fail (eg over quota, bad destination mailbox).
2025-02-25 23:27:19 +01:00
Mechiel Lukkien
1066eb4c9f
imapclient: add a type Append for messages for the APPEND-command, and accept multiple for servers with MULTIAPPEND capability
and a few nits.
2025-02-25 23:24:37 +01:00
Mechiel Lukkien
88a68e9143
imapserver: properly accept literal8 for APPEND, since we claim to implement the BINARY extension
it's not just for the APPEND with "UTF8()", also any regular append needs to
accept literal8. found testing with pimalaya.
2025-02-25 23:07:56 +01:00
Mechiel Lukkien
78e0c0255f
imapserver: implement MULTIAPPEND extension, rfc 3502
MULTIAPPEND modifies the existing APPEND command to allow multiple messages. it
is somewhat more involved than a regular append of a single message since the
operation (of adding multiple messages) must be atomic. either all are added,
or none are.

we check as early as possible if the messages won't cause an over-quota error.
2025-02-24 15:47:47 +01:00
Mechiel Lukkien
b56d6c4061
imapserver: try harder to get the user-agent (from the ID command) into the loginattempt
our previous approach was to hope clients did the ID command right after the
AUTHENTICATE command. with more extensions implemented, that's a stretch,
clients are doing other commands in between.

the new approach is to allow more commands, but wait at most 1 second. clients
are still assumed to send the ID command soon after authenticate. we also still
ensure login attempts are logged on connection teardown, so we aren't missing
any logging, just may get it slightly delayed. seems reasonable.

we now also keep the useragent value around, and we use when initializing the
login attempt. because the ID command can happen at any time, also before the
AUTHENTICATE command.
2025-02-24 09:54:38 +01:00
Mechiel Lukkien
d27fc1e7fc
gofmt 2025-02-23 22:40:34 +01:00
Mechiel Lukkien
f117cc0fe1
website: mention tls-alpn-01 and http-01 acme challenge types are implemented, but not dns-01 yet
prompted by question by rawtaz on irc
2025-02-23 22:28:07 +01:00
Mechiel Lukkien
0ed820e3b0
imapserver: implement rfc 9590, returning metadata in the extended list command
only with "return" including "metadata". so clients can quickly get certain
metadata (eg for display, such as a color) for mailboxes.

this also adds a protocol token type "mailboxt" that properly encodes to utf7
if required.
2025-02-23 22:12:18 +01:00
Mechiel Lukkien
2809136451
imap metadata extension: allow keys in the /shared/ namespace too
not just /private. /shared/ is the more commonly implemented namespace, because
it is easier te implement: you don't need per-user/account storage of metadata.
i initially approached it from the other direction: we don't have a mechanism
to share metadata with other accounts, so everything is private, and i assumed
that would be what a user would prefer. but email clients make the decisions,
and they'll likely try the /shared/ namespace.
2025-02-23 20:19:07 +01:00
Mechiel Lukkien
463e801909
add more rfc's and shuffle roadmap once more 2025-02-23 12:08:11 +01:00
Mechiel Lukkien
3b224ea0c2
consistent simpler parsing of domains in cli commands
prompted by previous commit, making me look at dns.ParseDomain calls.
2025-02-23 11:34:51 +01:00
Mechiel Lukkien
151729af08
in dns.ParseDomain, don't allow ipv4 addresses (ipv6 addresses were already rejected)
we are expecting a DNS domain name there.
also highlighted a wrong test in the smtp server.
2025-02-23 11:33:31 +01:00
Mechiel Lukkien
797c1cf9f0
do not log an error for tls requests with ipv6 addresses as sni server name
ip addresses are invalid in server names. for ipv6 addresses, the
autocert.GetCertificate calls would return an error, which we logged, and
increased a metric about. but the alerts for this situation aren't helpful. so
recognize ip addresses early. if we are lenient about unknown server names (for
incoming smtp deliveries), we switch to the fallback hostname, otherwise we
return an error.

this was the error logged:

	l=error m="requesting certificate" err="acme/autocert: server name component count invalid"

for ipv4 addresses, the name wouldn't be in our allowlist and should already
have caused us to switch to the fallback hostname.
2025-02-23 10:46:39 +01:00
Mechiel Lukkien
cad585a70e
webmail: when trying to empty an already empty mailbox, make it a user error, not server error
server errors could cause error logging.
2025-02-22 23:11:34 +01:00
Mechiel Lukkien
9f3cb7340b
update modseq when changing mailbox/server metadata, and also for specialuse changes, and keep track of modseq for mailboxes
i added the metadata extension to the imapserver recently. then i wondered how
a client would efficiently find changed metadata. turns out the qresync rfc
mentions that metadata changes should set a new modseq on the mailbox.
shouldn't be hard, except that we were not explicitly keeping track of modseqs
per mailbox. we only kept them for messages, and we were just looking up the
latest message modseq when we needed the modseq (we keep db entries for
expunged messages, so this worked out fine). that approach isn't enough
anymore. so know we keep track of modseq & createseq for mailboxes, just as for
messages. and we also track modseq/createseq for annotations. there's a good
chance jmap is going to need it.

this also adds consistency checks for modseq/createseq on mailboxes and
annotations to the account storage. it helped spot cases i missed where the
values need to be updated.
2025-02-22 22:52:18 +01:00
Mechiel Lukkien
7c7473ef0e
fix tests on bsds, since previous commit
the tls resumption test was failing due to switch from net.Pipe to unix domain
socket pairs. on bsds, they have an empty name (on linux it is "@"), which
prevents tls resumption from working.
2025-02-21 20:38:37 +01:00
Mechiel Lukkien
f40f94670e
implement IMAP extension COMPRESS=DEFLATE, rfc 4978
to compress the entire IMAP connection. tested with thunderbird, meli, k9, ios
mail. the initial implementation had interoperability issues with some of these
clients: if they write the deflate stream and flush in "partial mode", the go
stdlib flate reader does not return any data (until there is an explicit
zero-length "sync flush" block, or until the history/sliding window is full),
blocking progress, resulting in clients closing the seemingly stuck connection
after considering the connection timed out. this includes a coy of the flate
package with a new reader that returns partially flushed blocks earlier.

this also adds imap trace logging to imapclient.Conn, which was useful for
debugging.
2025-02-21 14:56:17 +01:00
Mechiel Lukkien
3f6c45a41f
for trace-level logging in console format (as opposed to logfmt), print the trace as quoted string
so we can easily see the exact bytes on the wire, instead of having \n's
expanded as newlines. much easier to read. we had this in the past, but it must
have been lost in a refactor.
2025-02-20 17:42:00 +01:00
Mechiel Lukkien
95d2002e77
announce support for namespace extension in imap capabilities line
we already implemented it as part of imap4rev2, but older clients need to be
told we implement it.
2025-02-20 08:32:33 +01:00
Mechiel Lukkien
a458920721
pass "go vet" again, can't use unkeyed struct fields from other package 2025-02-19 23:06:11 +01:00
Mechiel Lukkien
6ed97469b7
imapclient: parse fetch attribute "internaldate" as time.Time instead of keeping it as string
similar to the SAVEDATE fetch attribute implemented recently.
2025-02-19 23:01:23 +01:00
Mechiel Lukkien
02c4715724
remove intention to implement \important special-use mailbox and $important message flag, rfc 8457
they are intended to be used by the server to automatically mark some messages
as important, based on server-defined heuristics. we don't have such heuristics
at the moment. perhaps in the future, but until then there are no plans.
2025-02-19 22:44:04 +01:00
Mechiel Lukkien
5e4d80d48e
implement the WITHIN IMAP extension, rfc 5032
for IMAP "SEARCH" command criteria "YOUNGER" and "OLDER".
2025-02-19 21:29:14 +01:00
Mechiel Lukkien
dcaa99a85c
implement IMAP CREATE-SPECIAL-USE extension for the mailbox create command, part of rfc 6154
we already supported special-use flags. settable through the webmail interface,
and new accounts already got standard mailboxes with special-use flags
predefined. but now the IMAP "CREATE" command implements creating mailboxes
with special-use flags.
2025-02-19 20:39:26 +01:00
Mechiel Lukkien
7288e038e6
implement imap savedate extension, rfc 8514
it makes a new field available on stored messages. not when they they were
received (over smtp) or appended to the mailbox (over imap), but when they were
last "saved" in the mailbox. copy/move of a message (eg to the trash) resets
the "savedate" value. this helps implement "remove messages from trash after X
days".
2025-02-19 17:11:20 +01:00
Mechiel Lukkien
cbe5bb235c
fix data race in code for logging login attempts
logging of login attempts happens in the background, because we don't want to
block regular operation with disk since for such logging. however, when a line
is logged, we evaluate some attributes of a connection, notably the username.
but about when we do authentication, we change the username on a connection. so
we were reading and writing at the same time. this is now fixed by evaluating
the attributes before we pass off the logger to the goroutine.

found by the go race detector.
2025-02-19 15:23:19 +01:00
Mechiel Lukkien
de6262b90a
make test for imap getmetadata reliable by sorting output by key 2025-02-19 14:58:22 +01:00
Mechiel Lukkien
f30c44eddb
implement the imap metadata extension, rfc 5464
this allows setting per-mailbox and per-server annotations (metadata). we have
a fixed maximum for total number of annotations (1000) and their total size
(1000000 bytes). this size isn't held against the regular quota for simplicity.
we send unsolicited metadata responses when a connection is in the idle
command and a change to a metadata item is made.

we currently only implement the /private/ namespace.  we should implement the
/shared/ namespace, for mox-global metadata annotations.  only the admin should
be able to configure those, probably through the config file, cli, or admin web
interface.

for issue #290
2025-02-17 22:57:33 +01:00
Mechiel Lukkien
9dff879164
in domain/dns self-check, for unused services, check that port is 0 like how we told users to configure it and fix checking for errors during srv lookups 2025-02-16 17:42:24 +01:00
Mechiel Lukkien
1c4bf8909c
webmail: when forwarding, include the subject,date,from,reply-to,to,cc headers in the message
mentioned some time ago by ilijamt
2025-02-16 16:45:02 +01:00
Mechiel Lukkien
4765bf3b2c
shuffle entries in roadmap
it hasn't been updated in a while. this isn't the full picture either, but at
least closer to the planned order.
2025-02-16 16:28:48 +01:00
Mechiel Lukkien
3d0dc3a79d
in domain/dns self-check, for unexpected SRV records for "srv autoconfig", show the values of the unexpected records
should be more helpful in understanding what's wrong.

feedback from mteege, thanks!
2025-02-16 16:21:01 +01:00
Mechiel Lukkien
6f678125a5
in domain/dns self-check, provide config snippet for HostTLSRPT if it isn't configured and the admin should check again for the DNS records
feedback from mteege, thanks!
2025-02-16 16:12:44 +01:00
Mechiel Lukkien
1d6f45e592
in domain/dns self-check, don't warn about reverse dns that resolves to multiple names
this is fine. we just need to check if the expected name is among them.

feedback from mteege, thanks!
2025-02-16 15:55:31 +01:00
Mechiel Lukkien
6da5f8f586
add config option to an account destination to reject messages that don't pass a dmarc-like aligned spf/aligned dkim check
intended for automated processors that don't want to send messages to senders
without verified domains (because the address may be forged, and the processor
doesn't want to bother innocent bystanders).

such delivery attempts will fail with a permanent error immediately, typically
resulting in a DSN message to the original sender. the configurable error
message will normally be included in the DSN, so it could have alternative
instructions.
2025-02-15 17:34:06 +01:00
Mechiel Lukkien
f33870ba85
move the large commands for generating api docs to separate shell script 2025-02-15 12:56:59 +01:00
Mechiel Lukkien
3e53abc4db
add account config option to prevent the account for setting their own custom password, and enable by default for new accounts
accounts with this option enabled can only generate get a new randomly
generated password. this prevents password reuse across services and weak
passwords. existing accounts keep their current ability to set custom
passwords. only admins can change this setting for an account.

related to issue #286 by skyguy
2025-02-15 12:44:18 +01:00