commit e059bbcc10b86ee36a3b805c784b4057c0970bee from: David Williams date: Sat Sep 19 03:37:29 2026 UTC subscriptions, large messages, lock timeout, login grace, logging, IDLE SUBSCRIBE and UNSUBSCRIBE are implemented, and LIST takes the SUBSCRIBED selection option of RFC 9051 section 6.3.9.1. Both commands used to reply NO, and LSUB returned every mailbox the account had, because no subscription state was kept at all: the deprecated command worked and its replacement was missing. Subscriptions now live in one file per account, imapd.subscriptions, at the maildir root beside imapd.uidvalidity, one mailbox name per line. An absent file means every mailbox is subscribed. Only UNSUBSCRIBE creates one, and the first UNSUBSCRIBE writes out every other mailbox on its way past, so upgrading from 0.1.4 cannot leave an account subscribed to nothing, and a client that hides unsubscribed mailboxes cannot come back to an empty tree. A file that cannot be read is treated the same way, so damage shows too many mailboxes rather than too few. INBOX is never named in the file: it always exists and cannot be deleted, so it is permanently subscribed and UNSUBSCRIBE INBOX replies NO. A name outlives its mailbox, which RFC 9051 section 6.3.8 requires. LIST (SUBSCRIBED) reports such a name \Subscribed \NonExistent, and LSUB reports it \Noselect, which is RFC 3501 section 6.3.9's spelling of the same fact. A plain LIST reports no attributes at all and never opens the subscription file, so the request a client makes on every connection costs what it did before any of this existed. Neither CREATE nor RENAME changes the file; what is subscribed is left to the client to say, and the clients tested say it. Unsupported LIST selection options, REMOTE and RECURSIVEMATCH, are refused rather than ignored, since accepting one silently would misreport which names the response contains. A mailbox name the server refuses now draws NO rather than BAD. RFC 9051 section 7.1.3 reserves a tagged BAD for a protocol-level error in the client's command, and these are not errors of that kind: SELECT "" parses, since an empty quoted string is well-formed, and so does a name carrying the hierarchy delimiter. Each of SELECT, EXAMINE, CREATE, RENAME, SUBSCRIBE and COPY carries an explicit "NO - ... that name" in its own result table. Five sites changed: NO [NONEXISTENT] for SELECT's empty name, which is what the store's own no-such-mailbox reply already said, and NO [CANNOT] for the rest, which is what the non-UTF-8 refusal one line above four of them already said. BAD is untouched wherever a command genuinely fails to parse. Apple Mail sends SELECT "" before renaming a selected mailbox, which is how this surfaced. imapd now logs to LOG_MAIL rather than LOG_DAEMON. Upgrading from 0.1.4, this is the change that needs attention: lines that landed in /var/log/messages now land in /var/log/maillog under the rule set syslog.conf(5) documents, so anyone routing daemon.* somewhere particular will need to adjust. The syslog tag is now the daemon name rather than the per-process role, it read "listener[29527]:" before, naming no daemon at all, with the role moved into the message text, where smtpd and ntpd keep theirs. imapd.8 states the facility. Every session now gets one connect line and one close line at LOG_INFO, where a default-verbosity daemon previously said nothing about any session it served. The close line carries the peer address, the authenticated user where there was one, the duration, whether the session ended up TLS-protected, and why it closed: one of logout, client-closed, io-error, tls-error, protocol-error, limit-exceeded, login-grace or shutdown. The connect line cannot report the first of those, since it describes how the connection was accepted and a port-143 session may issue STARTTLS afterwards. ps(1) now shows one line per session, "imapd: session 42 [authenticated]" beside "session 42 auth", "session 42 search" and "session 42 store", since the daemon forks a worker group per connection. Titles carry the session id and lifecycle stage only, never the username or peer address: a process title is readable by any local user, and unlike an ssh login an IMAP login is not otherwise visible locally. Identity is recovered by joining the session id to the log, which is not world-readable. The parent is deliberately left untitled, since rc.d finds a daemon by matching the command line setproctitle(3) overwrites, and a titled parent could not be stopped by rcctl. rcctl stop now stops the daemon. The parent used to signal every child and exit without waiting, so it returned before they were gone and every open session lost its record at the moment an operator would most want one. It now closes each listener worker's imsg channel, which that worker takes as the cue to tear its own session down: the store child is asked to exit through a message it already understood rather than shot, the client gets its TLS close_notify, and the session gets a close line with reason=shutdown. The parent then waits for the last child, bounded at five seconds, after which anything still there is killed and counted; a second SIGTERM skips the wait. keymgr is asked to exit rather than signalled, so a bare EOF still means the parent died, and it frees the loaded TLS key on the way out. Starting as a non-root user now says so, and names the uid. It previously failed reading the root-only imapd.conf and reported that instead, naming the wrong problem entirely. Under -d, a log line is now written with one write(2), so concurrent sessions no longer interleave mid-line. The store child no longer addresses mailboxes by changing its working directory. It chdir(2)s once at startup, to the account's maildir root, and afterwards names every mailbox by an open descriptor: one for the root, one for the SELECTed mailbox, and a temporary one for any command that visits a second mailbox. The commands that used to save the working directory, switch away and switch back do none of those things now, and the thirteen places that switched back are gone. Eleven of those thirteen only logged a failure to switch back, and nothing else re-established the directory. A session whose switch back failed therefore kept serving with the client's selection naming one mailbox and the process standing in another, so the next FETCH read the wrong mailbox and the next EXPUNGE deleted from it, both under a tagged OK. Triggering it needs no bug, only a second session renaming or deleting the mailbox between the switch away and the switch back, which two ordinary IMAP commands arrange; done that way it destroyed every message in a mailbox the client had never named. A regression test does exactly that and is in the suite. A COPY into a mailbox another session renames mid-command now succeeds, where before it failed and left the session working in the destination. MOVE loses the error path that reported messages duplicated in both mailboxes because it could not get back to the source, since there is no longer anything to get back to. The can't-happen truncation recovery in the old switching function is gone with the function, having been unable to do what it claimed: it walked into the truncated name it had just written. unveil(2) covers a lookup relative to a descriptor exactly as it covers one relative to the working directory, and every *at call this uses is already permitted by the store's pledge. A FETCH that cannot return an item it was asked for now ends in NO rather than OK. A message whose body could not be read came back as a bare "* 1 FETCH (UID 42)" under a tagged OK, which a client can only take to mean the message has no body. RFC 9051 section 6.4.5 gives FETCH the result "NO - fetch error: can't fetch that data". The untagged responses for whatever could be produced are still sent, and a section-part the message does not have is still an empty item rather than a failure. A corrupt index can no longer widen a UID sequence set. The function that finds the highest UID, which resolves "*" for UID FETCH, STORE, EXPUNGE, COPY, MOVE and SEARCH, parsed the index's last line with its own code and no sign check, so a last line of "-1:name::1" made "*" 4294967295 and "1:*" the whole UID space; for STORE, EXPUNGE, COPY and MOVE that decides which messages are changed. It now uses the index's one line parser. That parser, and the header's UIDVALIDITY and UIDNEXT, now refuse a value above 4294967295 instead of truncating it: a UIDNEXT of 4294967297 had become 1, so the next APPEND would have reused UIDs already given out, which RFC 9051 section 2.3.1.1 does not allow. The index lives in the user's own maildir, which is why it is checked at all. A connection that never authenticates is now closed, after 60 seconds by default, set by a new imapd.conf directive, login grace, from 1 to 3600 seconds or 0 to disable. Every accepted connection costs three processes and nothing closed one that completed TCP and then sent nothing, so about a hundred silent connections reached the startups full limit and every later client was refused. The timer covers a connection on the implicit-TLS port that never starts its handshake as well as one on port 143 that never sends a command, and is cancelled the moment a session authenticates. RFC 9051 section 5.4 permits exactly this: servers "are allowed to use a shortened pre-authentication timer to protect themselves from Denial-of-Service attacks". It is not an autologout for authenticated sessions, which imapd does not have. Such a connection closes with reason=login-grace. A STORE or EXPUNGE that answers NO now changes nothing. System flags live in the maildir filename and keywords and modseq in the index, and both commands used to change files message by message and save the index once at the end, reporting each message to the client as they went. A STORE whose merged keyword set did not fit on a later message was refused with earlier messages already renamed: their system flags changed on disk while their keywords and modseq were discarded, so the client was shown flags that were never stored, and a client syncing by CONDSTORE modseq never learned of the change that did stick. STORE now works out every message before renaming any, undoes its renames if a later rename or the index save fails, and reports only after the save. EXPUNGE told the client each message was gone before saving the index, and RFC 9051 section 7.5.1 has the client renumber on each such response. If the save then failed, client and server numbered messages differently for the rest of the session, so a later STORE and EXPUNGE by sequence number could delete a message the user never chose, and the index kept entries for files already unlinked, which nothing ever removed and which made any COPY including them fail. EXPUNGE now saves the index first, then removes the files, then reports. Neither command reports a HIGHESTMODSEQ the index does not hold, and a failed command no longer moves the value a session later reports on ENABLE CONDSTORE. Finding a message's file no longer rescans cur/. Every message access looked for new/ and then read cur/ until it matched, and APPEND and STORE both leave messages in cur/, so FETCH 1:* (FLAGS) over 10,000 messages did 10,000 scans of 10,000 entries and took 19.9 s. The store now reads cur/ once per command into a sorted array; a miss still falls back to a real scan, so the snapshot can cost a lookup but never change an answer. The same FETCH takes 0.87 s. A large FETCH now starts answering at once. The store used to compose the reply for every matching message before sending any of it, so FETCH 1:* (BODY.PEEK[]) over 10,000 messages waited 1.46 s for its first line and grew the store child by 92 MB. It now pauses after composing 1 MB, resumes when that has drained, and answers the first line in 0.06 s with the store growing 3.7 MB. A message larger than 12000 octets can now be uploaded and downloaded. APPEND used to refuse one with NO [LIMIT] and FETCH returned no BODY[] for one, so a client could neither save nor read an ordinary message with an attachment. The cap was there because a message travelled between the store child and the listener inside one imsg, which libutil bounds at 16384 octets. APPEND now streams. The listener tells the store to open the tmp/ file when the literal is announced, passes the literal on in pieces as it arrives, and tells the store to commit once the command's closing CRLF is in, so neither process holds the whole message. A failure part way through is reported when the command ends, and a session that dies mid-upload leaves no tmp/ file behind. A new imapd.conf directive, append max, bounds what APPEND accepts, 35 MiB by default, the same as the max-message-size default of smtpd.conf(5). A larger literal draws NO [LIMIT] stating the limit as a number; the old reply had an unmatched parenthesis and named a C macro. imapd refuses a configuration whose append max exceeds attachment max, since a message larger than that could be stored but its BODYSTRUCTURE never fetched. FETCH of BODY[], BODY[TEXT] and BODY[] no longer copies the message into an imsg. The store finds the octet range, refusing a message that contains NUL, which a literal cannot carry (RFC 9051 section 9, CHAR8), and passes the listener a read-only descriptor on the message file with an offset and length; the listener reads and writes and parses nothing. The store's pledge gains sendfd, and the listener already held recvfd. A FETCH walk also pauses after passing 16 descriptors, each of which holds a slot in the system-wide file table until it is sent, and a store that runs out of descriptors with some of its own still queued waits and retries the message rather than failing it. On premio, a 10 W Celeron J1900, FETCH 1:* (BODY.PEEK[]) over 1,000 messages of 64 KB answered in 1.97 s, with the store growing 260 KiB and holding at most 13 descriptors in flight. The partial-fetch clamp at 12000 octets is gone with the cap. BODY[]<0.1000000> of a large message used to report the text as 12000 octets long, which RFC 9051 section 6.4.5 does not allow: it truncates a partial fetch at the end of the text, not at a server limit. Nothing in src/ exceeds 80 columns; 750 lines did, across 29 of 33 files. IDLE now reports flag changes, and a refresh costs what the change cost rather than what the mailbox holds. A client in IDLE heard about new mail and about expunges, but never about a flag another session set, so a message marked read on a phone stayed unread on a desktop until that desktop asked. RFC 9051 section 6.3.13 names flag changes among the things IDLE exists to report. The server now sends an untagged FETCH carrying the UID and the flags for a message whose flags changed elsewhere, and the mod-sequence as well once the session has issued a CONDSTORE enabling command, which RFC 7162 section 3.1 requires. A message that arrived since the last poll is still announced by EXISTS alone, since its flags are news to nobody. The refresh behind IDLE also stopped sending the whole mailbox. The store child used to send the listener one imsg per message on any change, and the listener compared that list against the previous one by rescanning it from the start for each message, which is quadratic. The store child now keeps the list it last reported, compares in a single walk of two ascending lists, and sends only what the listener is to print: the EXPUNGE sequence numbers, the new count, and the flag changes. Nothing about the mailbox is kept in the listener any more. On premio, a 10 W Celeron J1900, with a mailbox of 100,000 messages and a session idling on it, the work behind one push fell from between 6.33 and 7.33 seconds to under 0.52 seconds. At 10,000 messages it was already under 0.12 seconds. What remains at 100,000 is reading and parsing the index, not comparing it. testing/imap_idle_flags_test.py is new and joins the regression set: two sessions on one scratch mailbox, one idling, with the flag push asserted and an APPEND and an EXPUNGE as controls, and a third session that selects with the CONDSTORE parameter to assert the mod-sequence is there for it and absent for the session that did not ask. testing/idle_diff_test.c, the standalone test for the sequence-number diff, now carries both the old rescan and the new walk and checks every case through both, including RFC 9051 section 6.4.3's EXPUNGE example and section 6.3.13's IDLE transcript. An IDLE refresh that gives up no longer loses the change it was about to report. The cheap probe in front of the refresh records the mailbox directory's inode and modification time whether or not its caller then succeeds, so a refresh that failed after the probe had said "something moved" had already consumed that change, and the next poll found nothing to report. The client was never told. The refresh now clears the probe on any reply it does not mark ok, so the following poll looks properly. Reachable today only through an index read or an allocation failing, which is why nothing exercises it. SELECT no longer moves this session's state before it knows the selection succeeded. It closed the old mailbox descriptor, installed the new one and recorded the new name, and only then took the lock and read the index, so a failure in either left the store child's descriptor and recorded name pointing at a mailbox whose SELECT had failed. The gate that guards the commands using that descriptor was never at risk: store_dispatch() clears mailbox_selected before dispatching a SELECT, so FETCH, STORE, EXPUNGE, SEARCH, COPY, MOVE and the IDLE refresh are all refused after one fails. What was left was the recorded name, which DELETE and RENAME read without consulting that gate, so either could act on bookkeeping for a mailbox this session never selected. The descriptor and the name now move with the gate, after the index has been read, and every failure before that point leaves all three alone. Not exercised by a test. Reaching the interesting failures needs the index read or the lock to fail, which a client cannot arrange, and the one failure a client can arrange returns before any of this. Waiting for another session's mailbox index lock is now bounded. The lock was taken with a blocking flock(2) and nothing anywhere gave up, so a session waited as long as the holder took. Measured three times on a 100,000 message mailbox: an ordinary STORE 1:* holds the lock for about 46 seconds, and a second session of the same user issuing a one-message STORE was blocked for about 45 of those, with no error and nothing to show for it. Two clients on one account is ordinary, so this is the common case rather than a corner. index_lock_acquire() grows a third answer for a caller that adds LOCK_NB: another process holds it. That is an ordinary outcome rather than a failure, so it is not logged, and blocking callers cannot see it because flock(2) does not report EWOULDBLOCK without LOCK_NB. A command whose handler cannot take the lock is kept by the store child and run again from the top on a timer, backing off from 50ms to a cap of 250ms, until it can or until the new "lock timeout" passes. That cap is also the worst case delay between the lock coming free and the command noticing; at it, a command waiting behind a 46 second STORE costs about 190 wakeups, four a second. One slot is enough, since the listener holds a session's next command while one is in flight. The child never blocks, so it keeps serving its listener while it waits. Re-running rather than resuming means a handler that defers must not have touched the mailbox or the session first, which is checked per handler. At the deadline the command answers NO with the RFC 9051 section 7.1 INUSE response code, whose description is this case exactly: someone else may be holding an exclusive lock needed for this operation, and the operation may succeed if the client tries again later. The default is deliberately generous at 120 seconds, because it is a safety net for a holder that is stuck rather than a cure for one that is slow: an ordinary command over a large mailbox legitimately runs for the better part of a minute, and a deadline under that would refuse ordinary concurrent use. A client that does not retry INUSE would discard the user's change with nothing on screen, which is worse than waiting. Setting it to 0 restores the unbounded wait. Every command that takes a mailbox's index lock is served this way: STORE, EXPUNGE, CLOSE, FETCH, SEARCH, SELECT, STATUS, COPY, MOVE and APPEND, each answered with its own reply type so that the client sees NO [INUSE] rather than a generic failure or "no such mailbox". What each handler has to clean up before returning busy differs by command: SELECT and STATUS close the directory they opened, FETCH takes the lock before resetting its walk, and APPEND skips the fsync(2) an earlier try already did and removes its tmp/ file when it gives up. COPY and MOVE still take their two locks in name order, but when the second is busy they now release the first as well and start again from nothing, since the command is run again from the top. Holding the first while waiting would keep every other session off that mailbox for as long as the deadline, and the ordering already rules out the deadlock that releasing might otherwise risk. A CLOSE that fails now leaves the mailbox selected. It removes nothing unless its index save succeeds, so a NO always meant the mailbox was untouched, yet the session was deselected anyway, and a client retrying the CLOSE as the INUSE text invites got BAD. The listener and the store child now both deselect only on success. The IDLE refresh skips a poll when the lock is busy, and the next poll tries again. The first refresh of an IDLE cannot skip: it records the mailbox as the client now knows it, and without it the next poll would compare against an older record and resend EXPUNGE responses the client has already processed, renumbering its view wrongly (RFC 9051 section 7.5.1). So a busy first refresh reads the committed index without the lock, which is safe because index_save() only ever replaces it whole by rename(2), and the change it found the lock held for is reported once the holder saves. One window remains: a holder that has saved but not yet released has committed its change, and a first refresh in that window adopts it unreported, as the blocking refresh it replaces did for every change made under the lock. A busy skip is logged at debug rather than as a failed refresh. Every call to index_lock_acquire() now passes LOCK_NB. testing/imap_lock_timeout_test.py covers eleven commands, choosing the waiter from STORE, EXPUNGE, FETCH, SEARCH, CLOSE, SELECT, STATUS, COPY, MOVE, APPEND and IDLE. For the first ten it asserts the bound, that it does not fire early, the response code, and that the same command succeeds afterwards, which for CLOSE is the check that the refused one left the mailbox selected. For IDLE it asserts that the idling client is told of every message the holder changed. It needs a lock timeout far below the default and so is not in run-all-tests.sh, as the login grace test is not. commit - 8b6932843b87183548532dae81ebad384663950a commit + e059bbcc10b86ee36a3b805c784b4057c0970bee blob - 269c1b36d07fd286e1773f1a58763b2b5e5182c3 blob + df2d90ea0e7fe4d18d6356afd2387738fd7053cb --- contrib/imapduser.8 +++ contrib/imapduser.8 @@ -2,7 +2,7 @@ .\" .\" Written for the OpenIMAPD project. Public domain / no rights reserved. .\" -.Dd $Mdocdate: September 9 2026 $ +.Dd $Mdocdate: September 18 2026 $ .Dt IMAPDUSER 8 .Os .Sh NAME blob - 79a3db86c5d124e3e3840c6d57cf4566a6586c13 blob + 0b06f37a593afdff9c8300f25cf94fe6ea57708b --- src/append_cmd.c +++ src/append_cmd.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -14,7 +16,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* append_cmd.c: APPEND literal-driven message upload and its async IMSG_MBOX_APPENDED completion handling. */ +/* append_cmd.c: APPEND literal upload, async IMSG_MBOX_APPENDED completion. */ #include #include @@ -40,7 +42,7 @@ #include "listener.h" #include "mboxname.h" -/* RFC 9051 SS9 date-time via sscanf(3); calendar validity (e.g. Feb 31) unchecked, timegm(3) normalizes it. */ +/* RFC 9051 SS9 date-time via sscanf(3); timegm(3) normalizes bad dates. */ int parse_date_time(const char *s, int64_t *out) { @@ -83,7 +85,11 @@ parse_date_time(const char *s, int64_t *out) if (zsign == '-') zoff = -zoff; - /* Reject out-of-range zone offsets -- sscanf/RFC 9051 don't bound them, and an extreme offset can produce a maildir basename unsafe for shell globs. */ + /* + * Reject out-of-range zone offsets -- sscanf/RFC 9051 don't bound + * them, and an extreme offset can produce a maildir basename unsafe + * for shell globs. + */ if ((int64_t)t - zoff < 0) return (-1); @@ -91,7 +97,7 @@ parse_date_time(const char *s, int64_t *out) return (0); } -/* Result struct for parse_append_args(), avoids an unwieldy number of out-parameters. */ +/* Result struct for parse_append_args(), avoids many out-parameters. */ struct append_parsed { char mailbox[MBOX_NAME_MAX]; uint32_t sysflags; @@ -102,7 +108,7 @@ struct append_parsed { int litnonsync; }; -/* RFC 9051 SS6.3.12 append grammar; flag-list parsing reused from parse_store_flags(). */ +/* RFC 9051 SS6.3.12 append grammar; reuses parse_store_flags() for flags. */ int parse_append_args(char *args, struct append_parsed *out, const char **errmsg) { @@ -116,7 +122,7 @@ parse_append_args(char *args, struct append_parsed *ou return (-1); } - /* one parser for every mailbox argument in the tree; see mailbox_cmd.c */ + /* one parser for every mailbox argument; see mailbox_cmd.c */ if (parse_mailbox_name(&p, out->mailbox, sizeof(out->mailbox), errmsg) == -1) return (-1); @@ -209,7 +215,12 @@ parse_append_args(char *args, struct append_parsed *ou memcpy(digitsbuf, start, digits_len); digitsbuf[digits_len] = '\0'; - /* RFC 9051 SS9's number64 is unsigned, but strtoull(3) accepts a sign, so "{-1}" would arrive as ULLONG_MAX and get a misleading NO [LIMIT] instead of BAD -- same digit guard listener.c's literal pre-scan already applies. */ + /* + * RFC 9051 SS9's number64 is unsigned, but strtoull(3) accepts + * a sign, so "{-1}" would arrive as ULLONG_MAX and get a + * misleading NO [LIMIT] instead of BAD -- same digit guard + * listener.c's literal pre-scan already applies. + */ if (digitsbuf[0] < '0' || digitsbuf[0] > '9') { *errmsg = "malformed literal octet count"; return (-1); @@ -223,7 +234,10 @@ parse_append_args(char *args, struct append_parsed *ou out->litlen = (uint64_t)litlen; if (out->litnonsync && out->litlen > 4096) { - /* RFC 9051 SS4.3: non-sync literals capped at 4096 octets, BAD, not cmd_append()'s NO size cap. */ + /* + * RFC 9051 SS4.3: non-sync literals capped at 4096B, + * BAD not NO. + */ *errmsg = "non-synchronizing literal exceeds RFC " "9051 SS4.3's 4096-octet limit, use a " "synchronizing literal instead"; @@ -239,11 +253,12 @@ parse_append_args(char *args, struct append_parsed *ou return (0); } -/* Parses the literal announcement, allocates s->literal_buf, and enters literal-read mode (s->literal_pending). */ +/* Parses the literal announcement, starts the store's file, reads the rest. */ int cmd_append(struct session *s, const char *tag, char *args) { struct append_parsed parsed; + struct imsg_mbox_append req; int rc; const char *errmsg; @@ -257,56 +272,75 @@ cmd_append(struct session *s, const char *tag, char *a return (1); } - if (parsed.litlen > APPEND_LITERAL_MAX) { - /* RFC 5530 LIMIT code, matches APPEND_LITERAL_MAX's situation precisely. */ - session_reply(s, tag, "NO", - "[LIMIT] message too large for this server " - "limit, see APPEND_LITERAL_MAX)"); + if (parsed.litlen > listener_append_max) { + char text[96]; + + /* + * RFC 5530 LIMIT code. The text states the number, as the + * example in RFC 9051 SS7.1 does, since it reaches the client. + */ + (void)snprintf(text, sizeof(text), "[LIMIT] message exceeds " + "this server's %llu octet limit", + (unsigned long long)listener_append_max); + session_reply(s, tag, "NO", text); return (1); } if (s->store_iev == NULL) { - /* Same store_iev invariant as cmd_select()/cmd_fetch(), ST_AUTH requires it already wired. */ + /* + * Same store_iev invariant as cmd_select()/cmd_fetch(); ST_AUTH + * needs it. + */ log_warnx("session %u: APPEND with no store channel wired", s->id); session_reply(s, tag, "NO", "[SERVERBUG] internal error"); return (1); } - if (parsed.litlen > 0) { - if ((s->literal_buf = malloc((size_t)parsed.litlen)) == - NULL) { - log_warn("session %u: malloc APPEND literal buffer", - s->id); - session_reply(s, tag, "NO", "[SERVERBUG] internal error"); - return (1); - } - } else - s->literal_buf = NULL; /* zero-length literal; session_dispatch_client() handles it without special-casing */ - + memset(&req, 0, sizeof(req)); if (strlcpy(s->append_mailbox, parsed.mailbox, sizeof(s->append_mailbox)) >= sizeof(s->append_mailbox) || - strlcpy(s->append_keywords, parsed.keywords, - sizeof(s->append_keywords)) >= sizeof(s->append_keywords)) { + strlcpy(req.mailbox, parsed.mailbox, sizeof(req.mailbox)) >= + sizeof(req.mailbox) || + strlcpy(req.keywords, parsed.keywords, sizeof(req.keywords)) >= + sizeof(req.keywords)) { session_reply(s, tag, "NO", "[SERVERBUG] internal error"); return (1); } - s->append_sysflags = parsed.sysflags; - s->append_has_date = parsed.has_date; - s->append_date = parsed.date; + req.sysflags = parsed.sysflags; + req.has_date = parsed.has_date; + req.date = parsed.date; + req.msglen = parsed.litlen; s->append_prev_state = s->state; - /* tag is already IMAP_TAG_MAX-bounded by session_handle_line(); re-checked here defensively. */ + /* tag is IMAP_TAG_MAX-bounded by session_handle_line(); rechecked */ if (strlcpy(s->pending_tag, tag, sizeof(s->pending_tag)) >= sizeof(s->pending_tag)) { session_reply(s, tag, "NO", "[SERVERBUG] internal error"); return (1); } + + /* + * The store opens the message's tmp/ file now and is sent the literal + * in pieces as it arrives, rather than this process holding it whole. + */ + if (imsg_compose(&s->store_iev->ibuf, IMSG_MBOX_APPEND, 0, 0, -1, + &req, sizeof(req)) == -1) { + log_warn("session %u: imsg_compose IMSG_MBOX_APPEND", s->id); + session_reply(s, tag, "NO", "[SERVERBUG] internal error"); + return (1); + } + s->literal_len = parsed.litlen; s->literal_remaining = parsed.litlen; s->literal_pending = 1; - /* RFC 9051 SS4.3: "+" continuation is only for synchronizing literals. Uses sizeof()-1, not a hand count -- a wrong hand-counted length once wrote a stray NUL onto the wire, same idiom as listener.c's session_write() calls. */ + /* + * RFC 9051 SS4.3: "+" continuation is only for synchronizing + * literals. Uses sizeof()-1, not a hand count -- a wrong + * hand-counted length once wrote a stray NUL onto the wire, same + * idiom as listener.c's session_write() calls. + */ if (!parsed.litnonsync) { static const char cont[] = "+ Ready for literal data\r\n"; @@ -316,78 +350,39 @@ cmd_append(struct session *s, const char *tag, char *a return (1); } -/* Builds the combined header+message imsg, enters SESSION_APPENDING; s->literal_buf is freed either way. */ +/* Literal and its CRLF are in: tells the store to commit the message. */ int session_finish_append(struct session *s) { - struct imsg_mbox_append req; - char *combined; - size_t combined_len; - - memset(&req, 0, sizeof(req)); - if (strlcpy(req.mailbox, s->append_mailbox, sizeof(req.mailbox)) >= - sizeof(req.mailbox) || - strlcpy(req.keywords, s->append_keywords, sizeof(req.keywords)) >= - sizeof(req.keywords)) { - log_warnx("session %u: APPEND mailbox/keywords truncated, " - "can't happen (both already bounded when first stored)", - s->id); - session_reply(s, s->pending_tag, "NO", "[SERVERBUG] internal error"); - free(s->literal_buf); - s->literal_buf = NULL; - s->state = s->append_prev_state; - return (1); - } - req.sysflags = s->append_sysflags; - req.has_date = s->append_has_date; - req.date = s->append_date; - req.msglen = (uint32_t)s->literal_len; - if (s->store_iev == NULL) { log_warnx("session %u: APPEND with no store channel wired " "(literal already read)", s->id); - session_reply(s, s->pending_tag, "NO", "[SERVERBUG] internal error"); - free(s->literal_buf); - s->literal_buf = NULL; - s->state = s->append_prev_state; + session_reply(s, s->pending_tag, "NO", + "[SERVERBUG] internal error"); return (1); } - combined_len = sizeof(req) + (size_t)s->literal_len; - if ((combined = malloc(combined_len)) == NULL) { - log_warn("session %u: malloc APPEND imsg buffer", s->id); - session_reply(s, s->pending_tag, "NO", "[SERVERBUG] internal error"); - free(s->literal_buf); - s->literal_buf = NULL; - s->state = s->append_prev_state; - return (1); - } - memcpy(combined, &req, sizeof(req)); - if (s->literal_len > 0) - memcpy(combined + sizeof(req), s->literal_buf, - (size_t)s->literal_len); - - free(s->literal_buf); - s->literal_buf = NULL; - s->state = SESSION_APPENDING; - /* Same fail-soft shape as send_mbox_request(): without it, a compose failure leaves s->state stuck at SESSION_APPENDING and session_is_busy() blocks every further command. */ - if (imsg_compose(&s->store_iev->ibuf, IMSG_MBOX_APPEND, 0, 0, -1, - combined, combined_len) == -1) { - log_warn("session %u: imsg_compose IMSG_MBOX_APPEND", s->id); - free(combined); + /* + * Same fail-soft shape as send_mbox_request(): without it, a + * compose failure leaves s->state stuck at SESSION_APPENDING and + * session_is_busy() blocks every further command. + */ + if (imsg_compose(&s->store_iev->ibuf, IMSG_MBOX_APPEND_END, 0, 0, -1, + NULL, 0) == -1) { + log_warn("session %u: imsg_compose IMSG_MBOX_APPEND_END", + s->id); s->state = s->append_prev_state; session_reply(s, s->pending_tag, "NO", "[SERVERBUG] internal error"); return (1); } - free(combined); return (1); } -/* Terminal APPEND reply; restores s->state to s->append_prev_state (AUTHENTICATED or SELECTED). */ +/* Terminal APPEND reply; restores s->state to s->append_prev_state. */ void session_handle_mbox_appended(struct session *s, const struct imsg_mbox_appended *res) @@ -399,14 +394,24 @@ session_handle_mbox_appended(struct session *s, if (res->error != MBOX_OP_OK) { if (res->error == MBOX_OP_ERR_NO_SUCH_MAILBOX) session_reply(s, s->pending_tag, "NO", - "[TRYCREATE] no such mailbox"); /* SS6.3.12: reports why, not a promise CREATE would help */ + /* + * SS6.3.12: reports why, not a promise CREATE would + * help + */ + "[TRYCREATE] no such mailbox"); + else if (res->error == MBOX_OP_ERR_BUSY) + session_reply(s, s->pending_tag, "NO", + IMAP_BUSY_TEXT); else session_reply(s, s->pending_tag, "NO", "APPEND failed"); return; } - /* INBOX compared case-insensitively (SS5.1); any other mailbox name case-sensitively. */ + /* + * INBOX compared case-insensitively (SS5.1); other names + * case-sensitively. + */ if (mailbox_name_is_inbox(s->append_mailbox) && mailbox_name_is_inbox(s->selected_mailbox)) appended_to_selected = 1; blob - 27fa92b6d7ede434293de8142b8d3afcc477c6c6 blob + 9e9fdeab180b7849b4578bc3b08810eb0bdc06be --- src/auth.c +++ src/auth.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -14,7 +16,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* auth.c, credential verification process: AUTHENTICATE PLAIN against the flat cred file. */ +/* auth.c: credential verification, AUTHENTICATE PLAIN vs flat cred file. */ #include #include @@ -42,7 +44,8 @@ struct cred_entry { }; static struct imsgev iev_listener; -static struct imsgev iev_parent; /* fd 3, alive for the process's lifetime */ +/* fd 3, alive for the process's lifetime */ +static struct imsgev iev_parent; static char cred_file_basename[256]; static int cred_lookup(const char *, const char *username, @@ -52,8 +55,19 @@ static void auth_verify(struct imsg_auth_request *, static void auth_dispatch(int, short, void *); static void auth_dispatch_parent(int, short, void *); -/* SS6.2: refuses a second IMSG_AUTH_REQUEST for an already-resolved session_id, defending against a compromised/buggy listener replaying a grant; tracked in a fixed-size ring (not a TAILQ) since auth is never notified of session end, and session_id is unique-per-daemon so an evicted entry is harmless. */ -/* Caps bcrypt-costing auth attempts per connection (sshd's MaxAuthTries default) so a client retrying without limit can't convert cheap packets into unbounded server CPU; past the cap, requests are refused without calling crypt_checkpass(3), closing the cost asymmetry (though not dropping the connection). */ +/* + * Refuses a second IMSG_AUTH_REQUEST for an already-resolved session_id, + * defending against a compromised/buggy listener replaying a grant; tracked in + * a fixed-size ring (not a TAILQ) since auth is never notified of session end, + * and session_id is unique-per-daemon so an evicted entry is harmless. + */ +/* + * Caps bcrypt-costing auth attempts per connection (sshd's MaxAuthTries + * default) so a client retrying without limit can't convert cheap packets into + * unbounded server CPU; past the cap, requests are refused without calling + * crypt_checkpass(3), closing the cost asymmetry (though not dropping the + * connection). + */ #define AUTH_MAX_TRIES 6 static unsigned int auth_failures; @@ -65,7 +79,8 @@ static int auth_resolved_full; /* 1 once the ring has static int auth_session_already_resolved(uint32_t sid) { - size_t limit = auth_resolved_full ? AUTH_RESOLVED_MAX : auth_resolved_next; + size_t limit = auth_resolved_full ? AUTH_RESOLVED_MAX : + auth_resolved_next; size_t i; for (i = 0; i < limit; i++) { @@ -97,10 +112,16 @@ auth_main(void) char chrootdir[1024]; ssize_t n; - /* fd-passing is allowed on this channel for the IMSG_SETUP_PEER peer fd below; see imsgev_ibuf_init()'s own comment */ + /* + * fd-passing allowed here for IMSG_SETUP_PEER below; see + * imsgev_ibuf_init() + */ imsgev_ibuf_init(&ibuf3, 3); - /* IMSG_AUTH_INIT must be read first: cred_file is needed before chroot() can be computed */ + /* + * IMSG_AUTH_INIT read first: cred_file needed before chroot() is + * computed + */ for (;;) { if ((n = imsgbuf_get(&ibuf3, &imsg)) == -1) fatal("imsgbuf_get"); @@ -116,7 +137,11 @@ auth_main(void) imsg_get_type(&imsg)); if (imsg_get_data(&imsg, &init, sizeof(init)) == -1) fatalx("auth: bad IMSG_AUTH_INIT payload"); - /* imsg_get_data() guarantees size, not NUL termination -- force it, since strlcpy(3) would otherwise read unboundedly past this stack struct while computing the chroot(2) target. */ + /* + * imsg_get_data() guarantees size, not NUL termination -- force it, + * since strlcpy(3) would otherwise read unboundedly past this stack + * struct while computing the chroot(2) target. + */ init.cred_file[sizeof(init.cred_file) - 1] = '\0'; imsg_free(&imsg); @@ -125,11 +150,19 @@ auth_main(void) fatalx("getpwnam _imapauth: no such user " "(expected, not yet provisioned by an install script)"); - /* chroot into the dir containing the cred file, not the file itself; basename kept for unveil() */ + /* + * chroot into dir holding cred file, not itself; basename kept for + * unveil() + */ if (strlcpy(chrootdir, init.cred_file, sizeof(chrootdir)) >= sizeof(chrootdir)) fatalx("cred_file too long: %s", init.cred_file); - /* Actually verifies what the fatalx() below claims: strrchr() finding '/' only proves a directory component exists, not an absolute path (e.g. "etc/creds" would otherwise chroot(2) relative to cwd), and parse.y doesn't enforce this upstream. */ + /* + * Actually verifies what the fatalx() below claims: strrchr() finding + * '/' only proves a directory component exists, not an absolute path + * (e.g. "etc/creds" would otherwise chroot(2) relative to cwd), and + * parse.y doesn't enforce this upstream. + */ if (init.cred_file[0] != '/') fatalx("cred_file must be an absolute path: %s", init.cred_file); @@ -140,7 +173,8 @@ auth_main(void) sizeof(cred_file_basename)) >= sizeof(cred_file_basename)) fatalx("cred_file basename too long: %s", init.cred_file); if (slash == chrootdir) - chrootdir[1] = '\0'; /* "/creds" -> chroot("/"), not chroot("") */ + /* "/creds" -> chroot("/"), not chroot("") */ + chrootdir[1] = '\0'; else *slash = '\0'; @@ -154,7 +188,16 @@ auth_main(void) setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) fatal("cannot drop privileges to _imapauth"); - /* SS7: wires auth-worker's one and only peer via parent.c's spawn_connection()/setup_peer_send(), with no IMSG_SETUP_DONE ack needed since this boot sequence already reads a fixed, statically-known message set before touching the event loop. */ + /* no session id yet; retitled on the first request below */ + /* the imsg id cannot carry one: listener.c reads id 0 as "auth peer" */ + setproctitle("auth"); + + /* + * Wires auth-worker's one and only peer via parent.c's + * spawn_connection()/setup_peer_send(), with no IMSG_SETUP_DONE ack + * needed since this boot sequence already reads a fixed, + * statically-known message set before touching the event loop. + */ peer_fd = setup_recv_one_peer(&ibuf3); event_init(); @@ -174,7 +217,13 @@ auth_main(void) fatal("unveil lock"); } - /* No recvfd, no sendfd: this process gets its one peer fd via setup_recv_one_peer() before this line and never attaches a descriptor to an imsg itself (only parent.c does); a plain imsg with fd == -1 needs neither pledge promise, and a wrong guess here is an uncatchable SIGABRT, not silent breakage. */ + /* + * No recvfd, no sendfd: this process gets its one peer fd via + * setup_recv_one_peer() before this line and never attaches a + * descriptor to an imsg itself (only parent.c does); a plain imsg with + * fd == -1 needs neither pledge promise, and a wrong guess here is an + * uncatchable SIGABRT, not silent breakage. + */ #ifdef __OpenBSD__ if (pledge("stdio rpath", NULL) == -1) fatal("pledge"); @@ -184,7 +233,7 @@ auth_main(void) fatalx("auth: exited event loop"); } -/* EV_WRITE must be handled: imsg_compose() only queues, imsgbuf_write() puts it on the wire */ +/* EV_WRITE must be handled: imsg_compose() queues, imsgbuf_write() sends */ static void auth_dispatch(int fd, short event, void *arg) { @@ -201,7 +250,13 @@ auth_dispatch(int fd, short event, void *arg) if ((n = imsgbuf_read(&iev->ibuf)) == -1) fatal("imsgbuf_read"); if (n == 0) { - /* SS7: this auth-worker was spawned to serve exactly one connection and will never serve another, so it exits here on listener EOF rather than idling in event_dispatch() forever -- the ordinary, expected end of a session per parent.c's reap_child(). */ + /* + * This auth-worker was spawned to serve exactly one + * connection and will never serve another, so it exits + * here on listener EOF rather than idling in + * event_dispatch() forever -- the ordinary, expected + * end of a session per parent.c's reap_child(). + */ log_debug("auth-worker: listener closed channel, " "exiting"); exit(0); @@ -223,15 +278,22 @@ auth_dispatch(int fd, short event, void *arg) log_warnx("bad IMSG_AUTH_REQUEST"); break; } - /* imsg_get_data() guarantees size, not NUL termination, force it */ + /* + * imsg_get_data() guarantees size, not NUL termination, + * force it + */ req.username[sizeof(req.username) - 1] = '\0'; req.password[sizeof(req.password) - 1] = '\0'; + /* this worker's session; one worker per session */ + setproctitle("session %u auth", req.session_id); + if (auth_session_already_resolved(req.session_id)) { log_warnx("session %u: IMSG_AUTH_REQUEST for a " - "session already successfully authenticated, " - "refusing (SS6.2)", req.session_id); - explicit_bzero(req.password, sizeof(req.password)); + "session already successfully " + "authenticated, refusing", req.session_id); + explicit_bzero(req.password, + sizeof(req.password)); break; } @@ -255,7 +317,12 @@ auth_dispatch(int fd, short event, void *arg) cred.session_id = res.session_id; cred.uid = res.uid; cred.gid = res.gid; - /* cred.maildir and res.maildir are both sized AUTH_MAILDIR_MAX, so truncation is structurally impossible and the strlcpy() return value is discarded deliberately. */ + /* + * cred.maildir and res.maildir are both sized + * AUTH_MAILDIR_MAX, so truncation is + * structurally impossible and the strlcpy() + * return value is discarded deliberately. + */ (void)strlcpy(cred.maildir, res.maildir, sizeof(cred.maildir)); if (imsg_compose(&iev_parent.ibuf, @@ -276,7 +343,7 @@ auth_dispatch(int fd, short event, void *arg) (void)fd; } -/* parent never sends auth anything post-boot, so this exists to flush queued IMSG_AUTH_CRED writes and notice if parent's end closes */ +/* parent never sends auth anything post-boot; flushes CRED writes, sees EOF */ static void auth_dispatch_parent(int fd, short event, void *arg) { @@ -313,7 +380,11 @@ auth_dispatch_parent(int fd, short event, void *arg) (void)fd; } -/* Usernames come off the network and reach syslog only through this: anything outside printable ASCII becomes '?', since auth can't assume listener.c's CR/LF rejection held. */ +/* + * Usernames come off the network and reach syslog only through this: anything + * outside printable ASCII becomes '?', since auth can't assume listener.c's + * CR/LF rejection held. + */ static void auth_safe_name(const char *in, char *out, size_t outsize) { @@ -327,7 +398,7 @@ auth_safe_name(const char *in, char *out, size_t outsi out[i] = '\0'; } -/* always calls crypt_checkpass() with hash == NULL on unknown username, to avoid timing leaks */ +/* calls crypt_checkpass() with hash NULL on unknown user, avoids timing leak */ static void auth_verify(struct imsg_auth_request *req, struct imsg_auth_result *res) { @@ -336,7 +407,11 @@ auth_verify(struct imsg_auth_request *req, struct imsg int found; char safename[AUTH_USERNAME_MAX]; - /* Budget spent: refuses before cred_lookup() so a client past AUTH_MAX_TRIES can't even trigger a re-read/re-scan of the credential file; reported identically to any other failure. */ + /* + * Budget spent: refuses before cred_lookup() so a client past + * AUTH_MAX_TRIES can't even trigger a re-read/re-scan of the credential + * file; reported identically to any other failure. + */ if (auth_failures >= AUTH_MAX_TRIES) { char overname[AUTH_USERNAME_MAX]; @@ -363,12 +438,18 @@ auth_verify(struct imsg_auth_request *req, struct imsg auth_failures++; } - /* Logs every authentication outcome (previously nothing did, leaving password-guessing runs untraceable for fail2ban-style tooling); log_info() is always emitted, and the failure line deliberately doesn't distinguish "no such user" from "wrong password" to avoid an enumeration oracle. */ + /* + * Logs every authentication outcome (previously nothing did, leaving + * password-guessing runs untraceable for fail2ban-style tooling); + * log_info() is always emitted, and the failure line deliberately + * doesn't distinguish "no such user" from "wrong password" to avoid an + * enumeration oracle. + */ auth_safe_name(req->username, safename, sizeof(safename)); if (res->ok) log_info("session %u: authentication succeeded for \"%s\" " "(uid %u)", req->session_id, safename, - (unsigned)res->uid); + (unsigned int)res->uid); else log_info("session %u: authentication failed for \"%s\"", req->session_id, safename); @@ -376,7 +457,14 @@ auth_verify(struct imsg_auth_request *req, struct imsg explicit_bzero(&ce, sizeof(ce)); } -/* True if a privileged file at st is safe to trust here: owned by root or the current (post-chroot, post-setresuid) uid, and not group-writable, group-executable, or accessible to world at all; same policy parse.y's check_file_secrecy() applies to imapd.conf (parse.y:678-695), kept as its own function since that one runs pre-privsep against an fd the parent still owns and logs a different message. */ +/* + * True if a privileged file at st is safe to trust here: owned by root or the + * current (post-chroot, post-setresuid) uid, and not group-writable, + * group-executable, or accessible to world at all; same policy parse.y's + * check_file_secrecy() applies to imapd.conf (parse.y:678-695), kept as its own + * function since that one runs pre-privsep against an fd the parent still owns + * and logs a different message. + */ static int cred_file_secure(const struct stat *st) { @@ -400,7 +488,15 @@ cred_lookup(const char *path, const char *username, st return (-1); } - /* Rejects a credentials file not owned by root or the current uid, or that is group-writable, group-executable, or accessible to world at all (parent.c already enforces an equivalent policy for the TLS key, and parse.y's check_file_secrecy() for imapd.conf itself; this file holding every bcrypt hash had no such check until this one); group-read stays permissive so the documented "root:_imapauth 0640" layout keeps working, and a bad mode or owner fails closed. */ + /* + * Rejects a credentials file not owned by root or the current uid, or + * that is group-writable, group-executable, or accessible to world at + * all (parent.c already enforces an equivalent policy for the TLS key, + * and parse.y's check_file_secrecy() for imapd.conf itself; this file + * holding every bcrypt hash had no such check until this one); + * group-read stays permissive so the documented "root:_imapauth 0640" + * layout keeps working, and a bad mode or owner fails closed. + */ { struct stat st; @@ -415,8 +511,8 @@ cred_lookup(const char *path, const char *username, st "not group-writable, group-executable, or " "world-accessible; refusing all authentication " "until this is fixed", - path, (unsigned)(st.st_mode & 07777), - (unsigned)st.st_uid); + path, (unsigned int)(st.st_mode & 07777), + (unsigned int)st.st_uid); fclose(fp); return (-1); } @@ -429,7 +525,11 @@ cred_lookup(const char *path, const char *username, st char *ep; unsigned long ulval; - /* fgets(3) silently splits an over-long line, which would otherwise parse the tail as a phantom credential entry -- refuses to read the whole file rather than guess. */ + /* + * fgets(3) silently splits an over-long line, which would + * otherwise parse the tail as a phantom credential entry -- + * refuses to read the whole file rather than guess. + */ if (strchr(line, '\n') == NULL && strlen(line) == sizeof(line) - 1) { log_warnx("%s: over-long line, refusing to parse the " @@ -456,16 +556,32 @@ cred_lookup(const char *path, const char *username, st if (strcmp(fields[0], username) != 0) continue; - /* skip rather than silently truncate a field, same as every other malformed-line case */ + /* + * skip rather than truncate a field, same as other malformed + * lines + */ if (strlcpy(out->username, fields[0], sizeof(out->username)) >= sizeof(out->username) || strlcpy(out->passwordhash, fields[1], sizeof(out->passwordhash)) >= sizeof(out->passwordhash)) continue; - /* Requires a bcrypt hash ("$2" prefix): crypt_checkpass(3) treats an empty stored hash plus empty password as a successful login rather than a disabled account, so a blank field must be rejected here, and non-bcrypt values are skipped the same way as every other malformed entry to avoid an enumeration oracle -- use imapduser -d to disable an account instead. */ + /* + * Requires a bcrypt hash ("$2" prefix): crypt_checkpass(3) + * treats an empty stored hash plus empty password as a + * successful login rather than a disabled account, so a blank + * field must be rejected here, and non-bcrypt values are + * skipped the same way as every other malformed entry to avoid + * an enumeration oracle -- use imapduser -d to disable an + * account instead. + */ if (fields[1][0] != '$' || fields[1][1] != '2') continue; - /* strtoul(3) accepts a leading '-', so "-1" would parse as 0xffffffff (and "0" is root); the credential file shouldn't be able to express either uid/gid, so both are rejected here before the wrap case slips through unnoticed. */ + /* + * strtoul(3) accepts a leading '-', so "-1" would parse as + * 0xffffffff (and "0" is root); the credential file shouldn't + * be able to express either uid/gid, so both are rejected here + * before the wrap case slips through unnoticed. + */ if (fields[2][0] < '0' || fields[2][0] > '9' || fields[3][0] < '0' || fields[3][0] > '9') continue; @@ -488,7 +604,12 @@ cred_lookup(const char *path, const char *username, st break; } - /* line[] held the raw credential record (username, bcrypt hash, uid, gid, maildir) for every entry scanned; auth_verify() and auth_dispatch() scrub their own copies, so this buffer is the one left behind. */ + /* + * line[] held the raw credential record (username, bcrypt hash, uid, + * gid, maildir) for every entry scanned; auth_verify() and + * auth_dispatch() scrub their own copies, so this buffer is the one + * left behind. + */ explicit_bzero(line, sizeof(line)); fclose(fp); return (found ? 0 : -1); blob - 86be637642814c5966c967710f302bf7574884b3 blob + f40fc88dc9f436f73ddc512da25a07fc77ded283 --- src/auth_cmd.c +++ src/auth_cmd.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -14,7 +16,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* auth_cmd.c: CAPABILITY/NOOP/LOGOUT/ID/LOGIN/STARTTLS/AUTHENTICATE/ENABLE handlers that don't need a mailbox session. */ +/* auth_cmd.c: CAPABILITY/NOOP/LOGOUT/ID/LOGIN/AUTH/STARTTLS/ENABLE handlers. */ #include #include @@ -40,17 +42,21 @@ #include "listener.h" /* RFC 9051 SS6.1.1 capability strings, selected by session->tls_active. */ -#define CAPABILITY_PRE_TLS "IMAP4rev2 STARTTLS LOGINDISABLED ID CONDSTORE QRESYNC" -#define CAPABILITY_POST_TLS "IMAP4rev2 AUTH=PLAIN LOGINDISABLED ID CONDSTORE QRESYNC" +#define CAPABILITY_PRE_TLS "IMAP4rev2 STARTTLS LOGINDISABLED ID " \ + "CONDSTORE QRESYNC" +#define CAPABILITY_POST_TLS "IMAP4rev2 AUTH=PLAIN LOGINDISABLED ID " \ + "CONDSTORE QRESYNC" int cmd_capability(struct session *s, const char *tag, char *args) { - (void)args; /* RFC 9051: "Arguments: none", extra args ignored, not rejected */ + /* RFC 9051: "Arguments: none", extra args ignored, not rejected */ + (void)args; session_untagged(s, s->tls_active ? - "CAPABILITY " CAPABILITY_POST_TLS : "CAPABILITY " CAPABILITY_PRE_TLS); + "CAPABILITY " CAPABILITY_POST_TLS : + "CAPABILITY " CAPABILITY_PRE_TLS); session_reply(s, tag, "OK", "CAPABILITY completed"); return (1); } @@ -73,7 +79,7 @@ cmd_logout(struct session *s, const char *tag, char *a /* Exact example text from RFC 9051 SS6.1.3. */ session_untagged(s, "BYE IMAP4rev2 Server logging out"); session_reply(s, tag, "OK", "LOGOUT completed"); - session_teardown(s); + session_teardown(s, "logout"); return (0); } @@ -81,7 +87,7 @@ cmd_logout(struct session *s, const char *tag, char *a int cmd_id(struct session *s, const char *tag, char *args) { - /* RFC 2971 SS3.1: field/value list not parsed, just logged and discarded; always replies NIL per SS3.2 */ + /* RFC 2971 SS3.1: field/value list logged, not parsed; replies NIL */ log_debug("session %u: ID params: %s", s->id, args != NULL ? args : "(none)"); session_untagged(s, "ID NIL"); @@ -90,13 +96,14 @@ cmd_id(struct session *s, const char *tag, char *args) } -/* LOGIN is permanently disabled, matching LOGINDISABLED in both CAPABILITY strings above. */ +/* LOGIN permanently disabled, matching LOGINDISABLED in CAPABILITY strings. */ int cmd_login(struct session *s, const char *tag, char *args) { (void)args; - session_reply(s, tag, "NO", "LOGIN not supported, use AUTHENTICATE PLAIN"); + session_reply(s, tag, "NO", + "LOGIN not supported, use AUTHENTICATE PLAIN"); return (1); } @@ -110,29 +117,50 @@ cmd_starttls(struct session *s, const char *tag, char return (1); } if (s->tls_active) { - /* RFC 9051 SS6.2.1: BAD if STARTTLS received after negotiation */ + /* RFC 9051 SS6.2.1: BAD if STARTTLS comes after negotiation */ session_reply(s, tag, "BAD", "TLS already active"); return (1); } if (listener_tls_ctx == NULL) { - /* RFC 9051 SS6.2.1 NO + RFC 5530 UNAVAILABLE: cert/key loading failed at boot */ + /* + * RFC 9051 SS6.2.1 NO + RFC 5530 UNAVAILABLE: cert/key load + * failed at boot + */ session_reply(s, tag, "NO", "[UNAVAILABLE] TLS negotiation unavailable"); return (1); } - session_reply(s, tag, "OK", "Begin TLS negotiation now"); /* must precede TLS start, so goes out in cleartext */ + /* precedes TLS */ + session_reply(s, tag, "OK", "Begin TLS negotiation now"); - s->inbuflen = 0; /* command-injection mitigation: discard plaintext already buffered past this line */ + /* command-injection mitigation: discard buffered plaintext */ + s->inbuflen = 0; session_tls_start(s); return (1); } -/* RFC 4616 SS2: authzid/authcid/passwd each up to 255 octets + 2 NUL delimiters = 767, rounded up */ +/* RFC 4616 SS2: authzid/authcid/passwd up to 255 octets + 2 NULs = 767 */ #define SASL_PLAIN_MAX 768 -/* decodes+verifies one SASL PLAIN message (RFC 4616 SS2), sends IMSG_AUTH_REQUEST; never tears down the session, always returns 1 */ +/* Stores the username for the close line; mirrors auth.c's auth_safe_name() */ +/* non-printable becomes '?', so s->user is safe wherever it is logged */ +static void +session_set_user(struct session *s, const unsigned char *in, + size_t inlen) +{ + size_t i; + + for (i = 0; i < inlen && i + 1 < sizeof(s->user); i++) { + unsigned char c = in[i]; + + s->user[i] = (c >= 0x20 && c < 0x7f) ? (char)c : '?'; + } + s->user[i] = '\0'; +} + +/* decodes+verifies one SASL PLAIN msg (RFC 4616 SS2); never tears down */ int sasl_plain_finish(struct session *s, const char *tag, const char *b64, int allow_empty_equals) @@ -178,15 +206,20 @@ sasl_plain_finish(struct session *s, const char *tag, passwd = nul + 1; passwdlen = (size_t)rawlen - off - authcidlen - 1; - /* RFC 4616 SS2: empty prep result SHALL fail verification, NO not BAD, framing is fine */ + /* RFC 4616 SS2: empty prep result fails verification, NO not BAD */ if (authcidlen == 0 || passwdlen == 0) { - session_reply(s, tag, "NO", "[AUTHENTICATIONFAILED] authentication failed"); + session_reply(s, tag, "NO", + "[AUTHENTICATIONFAILED] authentication failed"); explicit_bzero(raw, sizeof(raw)); return (1); } - /* too big for imsg_auth_request's fixed fields; same generic NO, avoids a distinct error leaking an oracle */ + /* + * too big for imsg_auth_request's fixed fields; generic NO avoids an + * oracle + */ if (authcidlen >= AUTH_USERNAME_MAX || passwdlen >= AUTH_PASSWORD_MAX) { - session_reply(s, tag, "NO", "[AUTHENTICATIONFAILED] authentication failed"); + session_reply(s, tag, "NO", + "[AUTHENTICATIONFAILED] authentication failed"); explicit_bzero(raw, sizeof(raw)); return (1); } @@ -194,6 +227,7 @@ sasl_plain_finish(struct session *s, const char *tag, memset(&req, 0, sizeof(req)); req.session_id = s->id; memcpy(req.username, authcid, authcidlen); + session_set_user(s, authcid, authcidlen); memcpy(req.password, passwd, passwdlen); explicit_bzero(raw, sizeof(raw)); @@ -202,7 +236,11 @@ sasl_plain_finish(struct session *s, const char *tag, session_reply(s, tag, "NO", "[SERVERBUG] internal error"); return (1); } - /* SS7: the auth-worker may not exist if its fork failed (parent.c), leaving iev_auth.ibuf.fd at -1 -- fail gracefully rather than compose to an unwired imsgev. */ + /* + * The auth-worker may not exist if its fork failed (parent.c), leaving + * iev_auth.ibuf.fd at -1 -- fail gracefully rather than compose to an + * unwired imsgev. + */ if (iev_auth.ibuf.fd == -1) { session_reply(s, tag, "NO", "[UNAVAILABLE] authentication " "temporarily unavailable"); @@ -221,13 +259,15 @@ sasl_plain_finish(struct session *s, const char *tag, return (1); } -/* client's response to our "+ " continuation after bare "AUTHENTICATE PLAIN" (see cmd_authenticate()) */ +/* reply to "+ " continuation after "AUTHENTICATE PLAIN" (cmd_authenticate) */ int session_handle_auth_continuation(struct session *s, const char *line) { - s->auth_cont = 0; /* next line is back to an ordinary tagged command regardless of outcome */ + /* next line back to ordinary tagged command either way */ + s->auth_cont = 0; - if (strcmp(line, "*") == 0) { /* RFC 9051 SS6.2.2: lone "*" cancels the exchange */ + /* RFC 9051 SS6.2.2: lone "*" cancels exchange */ + if (strcmp(line, "*") == 0) { session_reply(s, s->pending_tag, "BAD", "AUTHENTICATE cancelled"); return (1); @@ -236,7 +276,7 @@ session_handle_auth_continuation(struct session *s, co return sasl_plain_finish(s, s->pending_tag, line, 0); } -/* client's response to our "+ idling" continuation (RFC 9051 SS6.3.13; see cmd_idle()); only "DONE" terminates IDLE */ +/* reply to our "+ idling" continuation (SS6.3.13); only "DONE" ends IDLE */ int session_handle_idle_continuation(struct session *s, const char *line) { @@ -292,13 +332,17 @@ cmd_authenticate(struct session *s, const char *tag, c return (1); } - if (initial != NULL) { /* RFC 9051 SS6.2.2 initial-resp: finishes in one round trip */ - /* `initial` is the base64 cleartext password and points into s->inbuf; have the reader scrub it once consumed. */ + /* RFC 9051 SS6.2.2 initial-resp: one round trip */ + if (initial != NULL) { + /* + * `initial` is base64 cleartext password into s->inbuf; reader + * scrubs it + */ s->scrub_inbuf = 1; return sasl_plain_finish(s, tag, initial, 1); } - /* no initial response: send "+", auth_cont routes the reply line to session_handle_auth_continuation() */ + /* no initial response: send "+"; auth_cont routes the reply line */ if (strlcpy(s->pending_tag, tag, sizeof(s->pending_tag)) >= sizeof(s->pending_tag)) { session_reply(s, tag, "NO", "[SERVERBUG] internal error"); @@ -309,7 +353,7 @@ cmd_authenticate(struct session *s, const char *tag, c return (1); } -/* shared reply for a recognized command store.c can't run yet (no wire payload designed); NO not BAD, syntax is fine */ +/* reply for a command store.c can't run yet (no payload); NO not BAD */ int stub_not_implemented(struct session *s, const char *tag, const char *cmdname) { @@ -319,7 +363,7 @@ stub_not_implemented(struct session *s, const char *ta return (1); } -/* RFC 9051 SS6.3.1 ENABLE: unknown extensions ignored; ENABLED lists only what THIS command newly enabled, even if empty */ +/* RFC 9051 SS6.3.1 ENABLE: unknown exts ignored; ENABLED lists only new ones */ int cmd_enable(struct session *s, const char *tag, char *args) { @@ -352,7 +396,7 @@ cmd_enable(struct session *s, const char *tag, char *a if (newly_condstore || newly_qresync) session_condstore_enable(s); - /* buf[64] can never truncate here: worst case is two fixed literals plus a space, 18 bytes ("QRESYNC CONDSTORE"). */ + /* buf[64] never truncates: worst case two literals plus a space */ buf[0] = '\0'; if (newly_qresync) (void)strlcat(buf, "QRESYNC", sizeof(buf)); blob - f9228ac75a7d99082afc200f02579cf5847e1b7a blob + 37d8cbb635a00549cdc76e86d82a9256c9d165a3 --- src/envelope.c +++ src/envelope.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -40,7 +42,7 @@ int envbuf_append(char *buf, size_t bufsize, size_t *outlen, const char *data, size_t datalen) { - /* Subtract rather than add -- "*outlen + datalen" could wrap near SIZE_MAX and falsely pass the check. */ + /* Subtract, don't add: "*outlen + datalen" could wrap near SIZE_MAX */ if (*outlen > bufsize || datalen > bufsize - *outlen) return (-1); memcpy(buf + *outlen, data, datalen); @@ -55,7 +57,7 @@ envbuf_append_str(char *buf, size_t bufsize, size_t *o return (envbuf_append(buf, bufsize, outlen, s, strlen(s))); } -/* Appends one RFC 9051 nstring: NIL if val is NULL, else quoted+escaped; not RFC 2047 decoded (verbatim). */ +/* Appends one RFC 9051 nstring: NIL if val NULL, else quoted+escaped. */ int envbuf_append_nstring(char *buf, size_t bufsize, size_t *outlen, const char *val, size_t vallen) @@ -71,7 +73,10 @@ envbuf_append_nstring(char *buf, size_t bufsize, size_ for (i = 0; i < vallen; i++) { char c = val[i]; - /* RFC 9051 SS4.3 quoted strings exclude NUL/CR/LF; substitute rather than reject so one bad byte doesn't drop the whole field. */ + /* + * RFC 9051 SS4.3 quoted strings exclude NUL/CR/LF; substitute + * rather than reject so one bad byte doesn't drop the field. + */ if (c == '\0' || c == '\r' || c == '\n') c = ' '; if ((c == '"' || c == '\\') && @@ -85,12 +90,15 @@ envbuf_append_nstring(char *buf, size_t bufsize, size_ return (0); fail: - /* All-or-nothing: a partial append would leave an unterminated quoted string in the caller's buffer. */ + /* + * All-or-nothing: a partial append leaves an unterminated quoted + * string. + */ *outlen = save; return (-1); } -/* Formats one RFC 5322 mailbox as an IMAP address tuple (RFC 9051 SS9); no group syntax, addr-adl always NIL. */ +/* Formats an RFC 5322 mailbox as an address tuple (RFC 9051 SS9); no groups. */ int envbuf_append_one_address(char *buf, size_t bufsize, size_t *outlen, const char *tok, size_t toklen) @@ -111,12 +119,13 @@ envbuf_append_one_address(char *buf, size_t bufsize, s tok++; toklen--; } - while (toklen > 0 && (tok[toklen - 1] == ' ' || tok[toklen - 1] == '\t')) + while (toklen > 0 && (tok[toklen - 1] == ' ' || + tok[toklen - 1] == '\t')) toklen--; if (toklen == 0) return (-1); - /* unquoted '<' splits display-name (before) from addr-spec (up to matching unquoted '>') */ + /* unquoted '<' splits display-name from addr-spec (to unquoted '>') */ lt = toklen; { size_t i; @@ -151,17 +160,22 @@ envbuf_append_one_address(char *buf, size_t bufsize, s const char *disp = tok; size_t displen = lt; - while (displen > 0 && (disp[0] == ' ' || disp[0] == '\t')) { + while (displen > 0 && + (disp[0] == ' ' || disp[0] == '\t')) { disp++; displen--; } while (displen > 0 && - (disp[displen - 1] == ' ' || disp[displen - 1] == '\t')) + (disp[displen - 1] == ' ' || + disp[displen - 1] == '\t')) displen--; if (displen >= 2 && disp[0] == '"' && disp[displen - 1] == '"') { - /* emission loop below re-escapes for the wire; no separate unescape pass needed */ + /* + * emission loop below re-escapes for the wire; + * no unescape pass needed + */ disp++; displen -= 2; } @@ -182,7 +196,8 @@ envbuf_append_one_address(char *buf, size_t bufsize, s spec++; speclen--; } - while (speclen > 0 && (spec[speclen - 1] == ' ' || spec[speclen - 1] == '\t')) + while (speclen > 0 && (spec[speclen - 1] == ' ' || + spec[speclen - 1] == '\t')) speclen--; found_at = 0; @@ -208,8 +223,12 @@ envbuf_append_one_address(char *buf, size_t bufsize, s host = spec + at + 1; hostlen = speclen - at - 1; - /* strip surrounding quotes from a quoted local-part (unescaped "@" inside not handled) */ - if (mailboxlen >= 2 && mailbox[0] == '"' && mailbox[mailboxlen - 1] == '"') { + /* + * strip quotes from a quoted local-part (unescaped "@" inside not + * handled) + */ + if (mailboxlen >= 2 && mailbox[0] == '"' && + mailbox[mailboxlen - 1] == '"') { mailbox++; mailboxlen -= 2; } @@ -220,7 +239,8 @@ envbuf_append_one_address(char *buf, size_t bufsize, s if (name != NULL) { size_t j; - if (envbuf_append(addrbuf, sizeof(addrbuf), &addrlen, "\"", 1) == -1) + if (envbuf_append(addrbuf, sizeof(addrbuf), &addrlen, + "\"", 1) == -1) return (-1); for (j = 0; j < namelen; j++) { char c = name[j]; @@ -237,14 +257,17 @@ envbuf_append_one_address(char *buf, size_t bufsize, s &c, 1) == -1) return (-1); } - if (envbuf_append(addrbuf, sizeof(addrbuf), &addrlen, "\"", 1) == -1) + if (envbuf_append(addrbuf, sizeof(addrbuf), &addrlen, + "\"", 1) == -1) return (-1); } else { - if (envbuf_append_str(addrbuf, sizeof(addrbuf), &addrlen, "NIL") == -1) + if (envbuf_append_str(addrbuf, sizeof(addrbuf), &addrlen, + "NIL") == -1) return (-1); } - if (envbuf_append_str(addrbuf, sizeof(addrbuf), &addrlen, " NIL ") == -1) + if (envbuf_append_str(addrbuf, sizeof(addrbuf), &addrlen, + " NIL ") == -1) return (-1); if (envbuf_append_nstring(addrbuf, sizeof(addrbuf), &addrlen, mailbox, mailboxlen) == -1) @@ -257,11 +280,17 @@ envbuf_append_one_address(char *buf, size_t bufsize, s if (envbuf_append(addrbuf, sizeof(addrbuf), &addrlen, ")", 1) == -1) return (-1); - /* One atomic append: the whole "(...)" tuple lands or none of it does, since envbuf_append() leaves *outlen untouched on failure. */ + /* + * One atomic append: the whole "(...)" tuple lands or none of it + * does, since envbuf_append() leaves *outlen untouched on failure. + */ return (envbuf_append(buf, bufsize, outlen, addrbuf, addrlen)); } -/* Formats an RFC 5322 address-list as "(" 1*address ")", or NIL if none parse (RFC 9051 SS7.5.2); splits on top-level commas only. */ +/* + * Formats an RFC 5322 address-list as "(" 1*address ")", or NIL if none + * parse (RFC 9051 SS7.5.2); splits on top-level commas only. + */ int envbuf_append_address_list(char *buf, size_t bufsize, size_t *outlen, const char *val, size_t vallen) @@ -274,7 +303,8 @@ envbuf_append_address_list(char *buf, size_t bufsize, val++; vallen--; } - while (vallen > 0 && (val[vallen - 1] == ' ' || val[vallen - 1] == '\t')) + while (vallen > 0 && (val[vallen - 1] == ' ' || + val[vallen - 1] == '\t')) vallen--; if (vallen == 0) @@ -311,7 +341,12 @@ envbuf_append_address_list(char *buf, size_t bufsize, tok_len--; if (tok_len > 0) { - /* Safe to skip a malformed or non-fitting address and continue: envbuf_append_one_address() builds the tuple locally before one atomic append, leaving buf/outlen untouched on failure. */ + /* + * Safe to skip a malformed or non-fitting address and + * continue: envbuf_append_one_address() builds the + * tuple locally before one atomic append, leaving + * buf/outlen untouched on failure. + */ if (envbuf_append_one_address(buf, bufsize, outlen, val + tok_start, tok_len) == 0) any = 1; @@ -325,7 +360,7 @@ envbuf_append_address_list(char *buf, size_t bufsize, return (envbuf_append(buf, bufsize, outlen, ")", 1)); } -/* Looks up header field `name`, appends its nstring form (NIL if absent); shared by ENVELOPE's plain-string members. */ +/* Looks up header `name`, appends nstring (NIL if absent); for ENVELOPE. */ int append_field_nstring(char *out, size_t outsize, size_t *outlen, const char *hdrbuf, uint32_t hdrlen, const char *name) @@ -343,9 +378,13 @@ append_field_nstring(char *out, size_t outsize, size_t return (rc); } -/* Builds RFC 9051 SS7.5.2 ENVELOPE list; Sender/Reply-To default to From if absent/empty; -1 if unreadable or over ENVELOPE_MAX. */ +/* + * Builds RFC 9051 SS7.5.2 ENVELOPE list; Sender/Reply-To default to From + * if absent/empty; -1 if unreadable or over ENVELOPE_MAX. + */ int -build_envelope(const char *basename, char **buf_out, uint32_t *len_out) +build_envelope(int dfd, const char *basename, char **buf_out, + uint32_t *len_out) { char *hdrbuf = NULL; uint32_t hdrlen = 0; @@ -357,7 +396,7 @@ build_envelope(const char *basename, char **buf_out, u *buf_out = NULL; *len_out = 0; - if (read_message_header(basename, &hdrbuf, &hdrlen) == -1) + if (read_message_header(dfd, basename, &hdrbuf, &hdrlen) == -1) return (-1); if (envbuf_append(out, sizeof(out), &outlen, "(", 1) == -1) @@ -426,7 +465,8 @@ build_envelope(const char *basename, char **buf_out, u envbuf_append(out, sizeof(out), &outlen, from_formatted, from_len) == -1) goto fail; - if (envbuf_append(out, sizeof(out), &outlen, " ", 1) == -1) + if (envbuf_append(out, sizeof(out), &outlen, + " ", 1) == -1) goto fail; } } @@ -452,7 +492,8 @@ build_envelope(const char *basename, char **buf_out, u &outlen, "NIL") == -1) goto fail; } - if (envbuf_append(out, sizeof(out), &outlen, " ", 1) == -1) + if (envbuf_append(out, sizeof(out), &outlen, + " ", 1) == -1) goto fail; } } @@ -487,7 +528,11 @@ fail: return (-1); } -/* BODYSTRUCTURE (RFC 9051 SS7.5.2): recursive RFC 2045/2046 MIME parse, bounded by MIME_MAX_DEPTH/MIME_MAX_PARTS; no extension data, message/rfc822, or RFC 2231 continuations. */ +/* + * BODYSTRUCTURE (RFC 9051 SS7.5.2): recursive RFC 2045/2046 MIME parse, + * bounded by MIME_MAX_DEPTH/MIME_MAX_PARTS; no extension data, + * message/rfc822, or RFC 2231 continuations. + */ int build_body_structure(int depth, int *nparts_used, const char *hdr, size_t hdrlen, const char *body, size_t bodylen, char *out, @@ -495,7 +540,8 @@ build_body_structure(int depth, int *nparts_used, cons { char type[64], subtype[64]; char params_fmt[600]; - char boundary[70 + 1]; /* RFC 2046 SS5.1.1 caps boundary at 70 chars, +1 NUL */ + /* RFC 2046 SS5.1.1 caps boundary at 70 chars, +1 NUL */ + char boundary[70 + 1]; int has_boundary; if (depth > MIME_MAX_DEPTH) @@ -526,7 +572,10 @@ build_body_structure(int depth, int *nparts_used, cons size_t plen = part_ends[i] - part_starts[i]; size_t phdrend; - /* zero-length body-part is spec-legal (RFC 2046 SS5.1.1); treat as hdrlen==0/bodylen==0 directly */ + /* + * zero-length body-part is spec-legal (RFC 2046 + * SS5.1.1); treat as 0/0 + */ if (plen == 0) phdrend = 0; else if (find_header_body_split(pbuf, plen, @@ -548,7 +597,8 @@ build_body_structure(int depth, int *nparts_used, cons if (strcasecmp(type, "MESSAGE") == 0 && (strcasecmp(subtype, "RFC822") == 0 || strcasecmp(subtype, "GLOBAL") == 0)) - return (-1); /* scoped out, see BODYSTRUCTURE comment above */ + /* scoped out, see BODYSTRUCTURE comment above */ + return (-1); { char *idval = NULL, *descval = NULL, *encval = NULL; @@ -580,7 +630,8 @@ build_body_structure(int depth, int *nparts_used, cons rc = envbuf_append_nstring(out, outsize, outlen, idval, idlen); else - rc = envbuf_append_nstring(out, outsize, outlen, NULL, 0); + rc = envbuf_append_nstring(out, outsize, outlen, + NULL, 0); free(idval); if (rc == -1) return (-1); @@ -592,7 +643,8 @@ build_body_structure(int depth, int *nparts_used, cons rc = envbuf_append_nstring(out, outsize, outlen, descval, desclen); else - rc = envbuf_append_nstring(out, outsize, outlen, NULL, 0); + rc = envbuf_append_nstring(out, outsize, outlen, + NULL, 0); free(descval); if (rc == -1) return (-1); @@ -633,7 +685,8 @@ build_body_structure(int depth, int *nparts_used, cons char numbuf[32]; snprintf(numbuf, sizeof(numbuf), "%zu", bodylen); - if (envbuf_append_str(out, outsize, outlen, numbuf) == -1) + if (envbuf_append_str(out, outsize, outlen, + numbuf) == -1) return (-1); } @@ -646,7 +699,8 @@ build_body_structure(int depth, int *nparts_used, cons lines++; } snprintf(numbuf2, sizeof(numbuf2), " %zu", lines); - if (envbuf_append_str(out, outsize, outlen, numbuf2) == -1) + if (envbuf_append_str(out, outsize, outlen, + numbuf2) == -1) return (-1); } @@ -654,9 +708,13 @@ build_body_structure(int depth, int *nparts_used, cons } } -/* Top-level entry: reads message once (capped at bodystructure_read_max), finds header/body split, walks from depth 0; -1 on any failure. */ +/* + * Top-level entry: reads message once (capped at bodystructure_read_max), + * finds header/body split, walks from depth 0; -1 on any failure. + */ int -build_bodystructure(const char *basename, char **buf_out, uint32_t *len_out) +build_bodystructure(int dfd, const char *basename, char **buf_out, + uint32_t *len_out) { char *wholebuf = NULL; uint32_t wholelen = 0; @@ -668,7 +726,7 @@ build_bodystructure(const char *basename, char **buf_o *buf_out = NULL; *len_out = 0; - if (read_message_body(basename, 0, bodystructure_read_max, + if (read_message_body(dfd, basename, 0, bodystructure_read_max, "BODYSTRUCTURE", &wholebuf, &wholelen) == -1) return (-1); if (wholelen == 0 || blob - 204ba724b7978cb4705e7709d340b52914883ba1 blob + b3a8877617850b49d567d824e44dfc14c5f6f6ca --- src/fetch_cmd.c +++ src/fetch_cmd.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -57,7 +59,11 @@ parse_nz_number(const char *str, uint32_t *out) return (0); } -/* Parses one range token (single optional colon, no comma) into r; factored out of the old parse_seq_range() so parse_sequence_set() can reuse it per comma-separated segment. */ +/* + * Parses one range token (single optional colon, no comma) into r; factored out + * of the old parse_seq_range() so parse_sequence_set() can reuse it per + * comma-separated segment. + */ static int parse_one_seq_range(const char *tok, struct seq_range *r) { @@ -102,7 +108,13 @@ parse_one_seq_range(const char *tok, struct seq_range return (0); } -/* Parses an RFC 9051 SS9 sequence-set (comma-separated seq-number/seq-range) by splitting on top-level commas and parsing each with parse_one_seq_range(), writing up to SEQSET_MAX_RANGES entries to ranges[] and the count to *nranges, or returning -1 with *errmsg set on a malformed, empty, or excess segment. */ +/* + * Parses an RFC 9051 SS9 sequence-set (comma-separated seq-number/seq-range) by + * splitting on top-level commas and parsing each with parse_one_seq_range(), + * writing up to SEQSET_MAX_RANGES entries to ranges[] and the count to + * *nranges, or returning -1 with *errmsg set on a malformed, empty, or excess + * segment. + */ int parse_sequence_set(const char *text, struct seq_range ranges[SEQSET_MAX_RANGES], uint32_t *nranges, const char **errmsg) @@ -150,7 +162,7 @@ parse_sequence_set(const char *text, struct seq_range return (0); } -/* like strtok_r(str, " ", &savep), but space isn't a delimiter inside an unclosed '[' or '(' (RFC 9051 SS9 header-list) */ +/* like strtok_r(); space isn't a delim inside an unclosed '[' or '(' (SS9) */ static char * fetch_att_tok(char *str, char **savep) { @@ -184,7 +196,7 @@ fetch_att_tok(char *str, char **savep) return (start); } -/* parses "HEADER.FIELDS[.NOT] (name ...)" bracket body (RFC 9051 SS9); -1 on syntax error is a real client BAD, not a silent drop */ +/* parses HEADER.FIELDS[.NOT] body (SS9); -1 is BAD, not a silent drop */ int parse_header_fields_att(const char *inner, int *not_out, char *fields_out, size_t fields_outsize) @@ -241,7 +253,7 @@ parse_header_fields_att(const char *inner, int *not_ou return (0); } -/* RFC 9051 SS6.4.5.1 section-part grammar check; verbatim string still crosses to store.c's parse_section_part() */ +/* SS6.4.5.1 section-part check; string still reaches parse_section_part() */ int section_part_valid(const char *s) { @@ -269,7 +281,7 @@ section_part_valid(const char *s) return (1); } -/* RFC 9051 SS6.4.5 partial-range suffix ""; count may be 0 (apply_partial_range() in store.c handles that) */ +/* SS6.4.5 ""; count 0 handled by partial_range() */ int parse_partial_suffix(const char *s, int *has_partial_out, uint32_t *start_out, uint32_t *count_out) @@ -310,7 +322,13 @@ parse_partial_suffix(const char *s, int *has_partial_o return (0); } -/* generic BODY.PEEK[...] tok (already known not to be HEADER.FIELDS): [], [TEXT], or [], optional <> (SS6.4.5); updates *attrs_inout/section_part_out/partial-range out-params. Returns 1 on success, 0 if silently degraded (*degraded_out set, same lenient skip as other unsupported forms), -1 on a hard parse error (*errmsg set). */ +/* + * generic BODY.PEEK[...] tok (already known not to be HEADER.FIELDS): [], + * [TEXT], or [], optional <> (SS6.4.5); updates + * *attrs_inout/section_part_out/partial-range out-params. Returns 1 on success, + * 0 if silently degraded (*degraded_out set, same lenient skip as other + * unsupported forms), -1 on a hard parse error (*errmsg set). + */ static int parse_body_peek_section_tok(const char *tok, uint32_t *attrs_inout, char *section_part_out, size_t section_part_outsize, @@ -355,14 +373,21 @@ parse_body_peek_section_tok(const char *tok, uint32_t return (0); } } else { - *degraded_out = 1; /* recognized shape, unsupported section (e.g. "2.1.TEXT") */ + /* recognized shape, unsupported section e.g. "2.1.TEXT" */ + *degraded_out = 1; return (0); } return (1); } -/* BODY.PEEK[HEADER.FIELDS...] tok: extracts the bracket body, dedupes a second HEADER.FIELDS item, delegates to parse_header_fields_att(). Returns 1 on success (*attrs_inout and the header_fields_*_out params updated), 0 if this token should be silently ignored (a duplicate), -1 on a hard parse error (*errmsg set). */ +/* + * BODY.PEEK[HEADER.FIELDS...] tok: extracts the bracket body, dedupes a second + * HEADER.FIELDS item, delegates to parse_header_fields_att(). Returns 1 on + * success (*attrs_inout and the header_fields_*_out params updated), 0 if this + * token should be silently ignored (a duplicate), -1 on a hard parse error + * (*errmsg set). + */ static int parse_body_peek_header_fields_tok(const char *tok, uint32_t *attrs_inout, int *header_fields_not_out, char *header_fields_out, @@ -377,7 +402,8 @@ parse_body_peek_header_fields_tok(const char *tok, uin return (-1); } if (*attrs_inout & MBOX_FETCH_HEADER_FIELDS) - return (0); /* already captured one, ignore any further duplicates */ + /* already captured one, ignore any further duplicates */ + return (0); if (toklen - strlen("BODY.PEEK[") - 1 >= sizeof(inner)) { *errmsg = "HEADER.FIELDS section too long"; @@ -401,7 +427,7 @@ parse_body_peek_header_fields_tok(const char *tok, uin return (1); } -/* RFC 9051 SS6.4.5 fetch-att + ALL/FULL/FAST macros; unsupported items silently skipped (*degraded_out=1) unless all are, then -2/NO */ +/* SS6.4.5: ALL/FULL/FAST; unsupported skipped (*degraded_out=1), else -2/NO */ int parse_fetch_atts(char *spec, uint32_t *attrs_out, int *degraded_out, int *header_fields_not_out, char *header_fields_out, @@ -457,7 +483,10 @@ parse_fetch_atts(char *spec, uint32_t *attrs_out, int attrs |= MBOX_FETCH_FLAGS | MBOX_FETCH_INTERNALDATE | MBOX_FETCH_RFC822_SIZE | MBOX_FETCH_ENVELOPE; } else if (strcasecmp(tok, "FULL") == 0) { - /* SS6.4.5 macro: ALL + bare BODY (bodystructure_full_out stays 0) */ + /* + * SS6.4.5 macro: ALL + bare BODY + * (bodystructure_full_out stays 0) + */ attrs |= MBOX_FETCH_FLAGS | MBOX_FETCH_INTERNALDATE | MBOX_FETCH_RFC822_SIZE | MBOX_FETCH_ENVELOPE | MBOX_FETCH_BODYSTRUCTURE; @@ -470,11 +499,14 @@ parse_fetch_atts(char *spec, uint32_t *attrs_out, int } else if (strcasecmp(tok, "RFC822.SIZE") == 0) { attrs |= MBOX_FETCH_RFC822_SIZE; } else if (strcasecmp(tok, "MODSEQ") == 0) { - attrs |= MBOX_FETCH_MODSEQ; /* RFC 7162 SS3.1.4.2, also CONDSTORE-enabling, cmd_fetch() checks this bit */ + /* SS3.1.4.2, CONDSTORE; cmd_fetch() checks */ + attrs |= MBOX_FETCH_MODSEQ; } else if (strcasecmp(tok, "BODY.PEEK[HEADER]") == 0) { - attrs |= MBOX_FETCH_BODY_HEADER; /* the one exact-match BODY[...]; plain BODY[HEADER] would need \Seen, unimplemented */ - } else if (strncasecmp(tok, "BODY.PEEK[", strlen("BODY.PEEK[")) == - 0 && strncasecmp(tok, "BODY.PEEK[HEADER.FIELDS", + /* exact BODY[...]; \Seen unimplemented */ + attrs |= MBOX_FETCH_BODY_HEADER; + } else if (strncasecmp(tok, "BODY.PEEK[", + strlen("BODY.PEEK[")) == 0 && + strncasecmp(tok, "BODY.PEEK[HEADER.FIELDS", strlen("BODY.PEEK[HEADER.FIELDS")) != 0) { if (parse_body_peek_section_tok(tok, &attrs, section_part_out, section_part_outsize, @@ -489,10 +521,14 @@ parse_fetch_atts(char *spec, uint32_t *attrs_out, int header_fields_label_outsize, errmsg) == -1) return (-1); } else if (strcasecmp(tok, "ENVELOPE") == 0) { - attrs |= MBOX_FETCH_ENVELOPE; /* RFC 9051 SS7.5.2; no .PEEK variant, no \Seen side effect */ + /* SS7.5.2; no .PEEK, no \Seen effect */ + attrs |= MBOX_FETCH_ENVELOPE; } else if (strcasecmp(tok, "BODY") == 0 || strcasecmp(tok, "BODYSTRUCTURE") == 0) { - /* both produce identical output; exact-matched ahead of the "BODY" prefix catch-all below */ + /* + * both produce identical output, exact-matched ahead of + * "BODY" catch-all + */ attrs |= MBOX_FETCH_BODYSTRUCTURE; *bodystructure_full_out = (strcasecmp(tok, "BODYSTRUCTURE") == 0); @@ -500,7 +536,10 @@ parse_fetch_atts(char *spec, uint32_t *attrs_out, int strcasecmp(tok, "RFC822") == 0 || strcasecmp(tok, "RFC822.HEADER") == 0 || strcasecmp(tok, "RFC822.TEXT") == 0) { - /* MIME part-addressed BODY[...] and RFC822(.HEADER/.TEXT) shorthands unimplemented; dropped */ + /* + * MIME part BODY[...] and RFC822(.HEADER/.TEXT) + * shorthands unimplemented + */ degraded = 1; } else { *errmsg = "unknown message data item"; @@ -509,21 +548,30 @@ parse_fetch_atts(char *spec, uint32_t *attrs_out, int } if (attrs == 0) { - /* every requested item was unsupported, e.g. BODY[] or RFC822(.HEADER/.TEXT) alone */ + /* + * every item unsupported, e.g. BODY[] or + * RFC822(.HEADER/.TEXT) alone + */ *errmsg = "cannot fetch that message content yet, " "supported: FLAGS/UID/INTERNALDATE/RFC822.SIZE/MODSEQ/" "ENVELOPE/(BODY|BODYSTRUCTURE)/BODY.PEEK[...]"; return (-2); } - /* both HEADER and HEADER.FIELDS requested (legal, SS6.4.5): HEADER wins, only one pending_header_* slot exists */ + /* + * both HEADER/HEADER.FIELDS requested (legal, SS6.4.5): HEADER wins + * here + */ if ((attrs & MBOX_FETCH_BODY_HEADER) && (attrs & MBOX_FETCH_HEADER_FIELDS)) attrs &= ~MBOX_FETCH_HEADER_FIELDS; *attrs_out = attrs; *degraded_out = degraded; - /* accumulated in locals like attrs, copied out here so the HEADER-wins resolution above stays the one adjustment point */ + /* + * accumulated in attrs, copied out so HEADER-wins is the one adjust + * point + */ *has_partial_out = has_partial; *partial_start_out = partial_start; *partial_count_out = partial_count; @@ -535,7 +583,7 @@ const char *fetch_month_names[12] = { "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" }; -/* RFC 9051 SS9 date-time; always formats in UTC "+0000", ts carries no tz info and a chroot'd store child has no tzdata */ +/* SS9 date-time; always UTC "+0000": ts has no tz, chroot child lacks tzdata */ void format_internaldate(int64_t ts, char *out, size_t outsize) { @@ -555,7 +603,7 @@ format_internaldate(int64_t ts, char *out, size_t outs tm.tm_hour, tm.tm_min, tm.tm_sec); } -/* snprintf-into-growing-buffer helper; clamps *len to bufsize so a prior truncation can't underflow the next call's remaining size */ +/* growing-buf helper; clamps *len, truncation can't underflow bufsize */ static void fetch_append(char *buf, size_t bufsize, size_t *len, const char *fmt, ...) { @@ -577,7 +625,42 @@ fetch_append(char *buf, size_t bufsize, size_t *len, c *len = bufsize; } -/* sends one untagged "* FETCH (...)" (RFC 9051 SS7.5.2); literal-syntax items flush buf then write their payload raw */ +/* + * Writes len octets of fd, starting at off, to the client. The literal's + * length is already on the wire, so a short read cannot be repaired and + * the connection is shut down instead of desynchronized. + */ +static void +session_write_file_range(struct session *s, int fd, uint64_t off, + uint64_t len) +{ + char buf[16384]; + size_t want; + ssize_t n; + + while (len > 0 && !s->write_failed) { + want = len < sizeof(buf) ? (size_t)len : sizeof(buf); + if ((n = pread(fd, buf, want, (off_t)off)) == -1 && + errno == EINTR) + continue; + if (n <= 0) { + if (n == -1) + log_warn("session %u: read FETCH body", s->id); + else + log_warnx("session %u: FETCH body ended %llu " + "octets early", s->id, + (unsigned long long)len); + s->write_failed = 1; + (void)shutdown(s->client_fd, SHUT_RDWR); + return; + } + session_write(s, buf, (size_t)n); + off += (uint64_t)n; + len -= (uint64_t)n; + } +} + +/* sends untagged FETCH response (SS7.5.2); literals flush buf, write raw */ void session_send_fetch_response(struct session *s, struct imsg_mbox_fetch_meta *meta) @@ -593,7 +676,8 @@ session_send_fetch_response(struct session *s, MBOX_FETCH_BODY_PART)) && s->pending_body_found; int have_envelope = (s->fetch_attrs & MBOX_FETCH_ENVELOPE) && s->pending_envelope_found; - int have_bodystructure = (s->fetch_attrs & MBOX_FETCH_BODYSTRUCTURE) && + int have_bodystructure = + (s->fetch_attrs & MBOX_FETCH_BODYSTRUCTURE) && s->pending_bodystructure_found; fetch_append(buf, sizeof(buf), &len, "%u FETCH (", meta->seqno); @@ -661,22 +745,26 @@ session_send_fetch_response(struct session *s, need_sp = 1; } if (have_body) { - /* RFC 9051 SS6.4.5: echo the origin octet only if the client sent one; never echo store.c's count */ + /* + * SS6.4.5: echo origin octet only if client sent one, + * not store.c's count + */ len = 0; if (s->pending_body_has_partial) fetch_append(buf, sizeof(buf), &len, - "%sBODY[%s]<%u> {%u}\r\n", + "%sBODY[%s]<%u> {%llu}\r\n", need_sp ? " " : "", s->pending_body_label, s->pending_body_partial_origin, - s->pending_body_len); + (unsigned long long)s->pending_body_len); else fetch_append(buf, sizeof(buf), &len, - "%sBODY[%s] {%u}\r\n", need_sp ? " " : "", - s->pending_body_label, s->pending_body_len); + "%sBODY[%s] {%llu}\r\n", need_sp ? " " : "", + s->pending_body_label, + (unsigned long long)s->pending_body_len); session_write(s, buf, len); if (s->pending_body_len > 0) - session_write(s, s->pending_body_buf, - s->pending_body_len); + session_write_file_range(s, s->pending_body_fd, + s->pending_body_off, s->pending_body_len); } session_write(s, ")\r\n", 3); } else { @@ -689,42 +777,62 @@ session_send_fetch_response(struct session *s, session_untagged(s, buf); } - /* reset pending_*_found even when have_* is false, so it doesn't leak into the next message's response */ + /* + * Reset pending_*_found when have_* is false, so it can't leak to + * the next reply. Requested but not found also means the store + * could not produce the item, which RFC 9051 SS6.4.5 answers with a + * tagged NO once the command finishes. + */ if (have_header) { free(s->pending_header_buf); s->pending_header_buf = NULL; s->pending_header_len = 0; } - if (s->fetch_attrs & (MBOX_FETCH_BODY_HEADER | MBOX_FETCH_HEADER_FIELDS)) + if (s->fetch_attrs & + (MBOX_FETCH_BODY_HEADER | MBOX_FETCH_HEADER_FIELDS)) { + if (!have_header) + s->fetch_incomplete = 1; s->pending_header_found = 0; + } if (have_body) { - free(s->pending_body_buf); - s->pending_body_buf = NULL; + if (s->pending_body_fd != -1) + close(s->pending_body_fd); + s->pending_body_fd = -1; + s->pending_body_off = 0; s->pending_body_len = 0; } if (s->fetch_attrs & (MBOX_FETCH_BODY_WHOLE | MBOX_FETCH_BODY_TEXT | - MBOX_FETCH_BODY_PART)) + MBOX_FETCH_BODY_PART)) { + if (!have_body) + s->fetch_incomplete = 1; s->pending_body_found = 0; + } if (have_envelope) { free(s->pending_envelope_buf); s->pending_envelope_buf = NULL; s->pending_envelope_len = 0; } - if (s->fetch_attrs & MBOX_FETCH_ENVELOPE) + if (s->fetch_attrs & MBOX_FETCH_ENVELOPE) { + if (!have_envelope) + s->fetch_incomplete = 1; s->pending_envelope_found = 0; + } if (have_bodystructure) { free(s->pending_bodystructure_buf); s->pending_bodystructure_buf = NULL; s->pending_bodystructure_len = 0; } - if (s->fetch_attrs & MBOX_FETCH_BODYSTRUCTURE) + if (s->fetch_attrs & MBOX_FETCH_BODYSTRUCTURE) { + if (!have_bodystructure) + s->fetch_incomplete = 1; s->pending_bodystructure_found = 0; + } } -/* STORE's untagged FETCH response (RFC 9051 SS6.4.6) always shows FLAGS; MODSEQ shown whenever CONDSTORE-aware (SS3.1.3) */ +/* STORE's FETCH (SS6.4.6) shows FLAGS; MODSEQ if CONDSTORE-aware (SS3.1.3) */ void session_send_store_fetch_response(struct session *s, const struct imsg_mbox_fetch_meta *meta) @@ -732,7 +840,10 @@ session_send_store_fetch_response(struct session *s, char buf[MBOX_FLAGS_MAX + 96]; size_t len; - /* RFC 9051 SS6.4.9: a UID STORE's echo must include UID, right after FLAGS */ + /* + * RFC 9051 SS6.4.9: a UID STORE's echo must include UID, right after + * FLAGS + */ len = (size_t)snprintf(buf, sizeof(buf), "%u FETCH (FLAGS (%s)", meta->seqno, meta->flags); if (s->cmd_by_uid && len < sizeof(buf)) @@ -747,7 +858,7 @@ session_send_store_fetch_response(struct session *s, session_untagged(s, buf); } -/* splits a trailing RFC 4466 modifier list off spec (shared by cmd_fetch()/cmd_store_cmd()); NUL-terminates spec in place */ +/* splits trailing RFC4466 modifiers off spec; NUL-terminates spec in place */ char * split_trailing_modifiers(char *spec) { @@ -766,7 +877,11 @@ split_trailing_modifiers(char *spec) break; } } else if (*p == '\0') - return (NULL); /* unterminated; caller's own parser produces the BAD for this */ + /* + * unterminated; caller's parser produces the + * BAD for this + */ + return (NULL); p++; } } else { @@ -784,7 +899,10 @@ split_trailing_modifiers(char *spec) return (p); } -/* FETCH's trailing fetch-modifier list (RFC 4466 + RFC 7162 SS3.1.4.1/SS3.2.6); *want_vanished lets fetch_dispatch() pair-check later */ +/* + * FETCH's trailing fetch-modifier list (RFC 4466 + RFC 7162 SS3.1.4.1/SS3.2.6); + * *want_vanished lets fetch_dispatch() pair-check later. + */ int parse_fetch_modifiers(char *modspec, struct imsg_mbox_fetch *req, const struct session *s, int by_uid, int *want_vanished, @@ -814,7 +932,13 @@ parse_fetch_modifiers(char *modspec, struct imsg_mbox_ "mod-sequence value"; return (-1); } - /* RFC 7162 SS7 mod-sequence-values are unsigned only, but strtoull(3) accepts a leading sign, so a guard rejects non-digit-leading input to stop "-1" silently becoming ULLONG_MAX (same check as auth.c/index.c/listener.c's literal parser). */ + /* + * RFC 7162 SS7 mod-sequence-values are unsigned only, + * but strtoull(3) accepts a leading sign, so a guard + * rejects non-digit-leading input to stop "-1" silently + * becoming ULLONG_MAX (same check as + * auth.c/index.c/listener.c's literal parser). + */ if (*valtok < '0' || *valtok > '9') { *errmsg = "invalid CHANGEDSINCE mod-sequence"; return (-1); @@ -831,7 +955,10 @@ parse_fetch_modifiers(char *modspec, struct imsg_mbox_ req->attrs |= MBOX_FETCH_MODSEQ; } else if (strcasecmp(tok, "VANISHED") == 0) { if (!by_uid) { - /* RFC 7162 SS3.2.6: VANISHED not allowed with plain FETCH, MUST return tagged BAD */ + /* + * RFC 7162 SS3.2.6: VANISHED with plain FETCH + * MUST return tagged BAD + */ *errmsg = "VANISHED is only valid as a UID " "FETCH modifier (RFC 7162 SS3.2.6)"; return (-1); @@ -851,14 +978,14 @@ parse_fetch_modifiers(char *modspec, struct imsg_mbox_ return (0); } -/* RFC 9051 SS6.4.5 fetch + RFC 4466/7162 modifier list; plain BODY[...] and BODY[] are a deliberate scope cut */ +/* SS6.4.5 fetch+RFC4466/7162 modifiers; BODY[...]/BODY[] a scope cut */ int cmd_fetch(struct session *s, const char *tag, char *args) { return fetch_dispatch(s, tag, args, 0); } -/* shared body for cmd_fetch() (by_uid=0) and cmd_uid()'s FETCH branch (by_uid=1); RFC 9051 SS6.4.9 forces MBOX_FETCH_UID into attrs */ +/* shared cmd_fetch/cmd_uid FETCH (uid 0/1); forces MBOX_FETCH_UID (SS6.4.9) */ int fetch_dispatch(struct session *s, const char *tag, char *args, int by_uid) { @@ -929,7 +1056,7 @@ fetch_dispatch(struct session *s, const char *tag, cha req.by_uid = by_uid; req.header_fields_not = header_fields_not; - /* re-checked though already bounds-checked above; store.c applies has_partial/section_part to whichever of WHOLE/TEXT/PART wins */ + /* bounds-checked above; applies per WHOLE/TEXT/PART winner */ if (strlcpy(req.header_fields, header_fields, sizeof(req.header_fields)) >= sizeof(req.header_fields) || strlcpy(req.section_part, section_part, sizeof(req.section_part)) @@ -941,51 +1068,62 @@ fetch_dispatch(struct session *s, const char *tag, cha req.partial_start = partial_start; req.partial_count = partial_count; - /* verbatim client-typed label never crosses to store.c, stashed here for session_send_fetch_response() to echo */ + /* client label never reaches store.c; stashed to echo in the reply */ if (attrs & MBOX_FETCH_BODY_HEADER) { if (strlcpy(s->pending_header_label, "HEADER", sizeof(s->pending_header_label)) >= sizeof(s->pending_header_label)) { - session_reply(s, tag, "NO", "[SERVERBUG] internal error"); + session_reply(s, tag, "NO", + "[SERVERBUG] internal error"); return (1); } } else if (attrs & MBOX_FETCH_HEADER_FIELDS) { if (strlcpy(s->pending_header_label, header_fields_label, sizeof(s->pending_header_label)) >= sizeof(s->pending_header_label)) { - session_reply(s, tag, "NO", "[SERVERBUG] internal error"); + session_reply(s, tag, "NO", + "[SERVERBUG] internal error"); return (1); } } - /* same idea, for BODY.PEEK[]/[TEXT]/[]; WHOLE>TEXT>PART must match store.c's handle_mbox_fetch() */ + /* + * same, for BODY.PEEK[]/[TEXT]/[]; matches handle_mbox_fetch() + * order + */ if (attrs & MBOX_FETCH_BODY_WHOLE) { s->pending_body_label[0] = '\0'; } else if (attrs & MBOX_FETCH_BODY_TEXT) { if (strlcpy(s->pending_body_label, "TEXT", sizeof(s->pending_body_label)) >= sizeof(s->pending_body_label)) { - session_reply(s, tag, "NO", "[SERVERBUG] internal error"); + session_reply(s, tag, "NO", + "[SERVERBUG] internal error"); return (1); } } else if (attrs & MBOX_FETCH_BODY_PART) { if (strlcpy(s->pending_body_label, section_part, sizeof(s->pending_body_label)) >= sizeof(s->pending_body_label)) { - session_reply(s, tag, "NO", "[SERVERBUG] internal error"); + session_reply(s, tag, "NO", + "[SERVERBUG] internal error"); return (1); } } s->pending_body_has_partial = has_partial; s->pending_body_partial_origin = partial_start; - /* same idea, for BODYSTRUCTURE: response label echoes whichever bare token ("BODY"/"BODYSTRUCTURE") the client used */ + /* + * same, BODYSTRUCTURE: echoes "BODY"/"BODYSTRUCTURE" bare token client + * used + */ if (attrs & MBOX_FETCH_BODYSTRUCTURE) { if (strlcpy(s->pending_bodystructure_label, bodystructure_full ? "BODYSTRUCTURE" : "BODY", sizeof(s->pending_bodystructure_label)) >= sizeof(s->pending_bodystructure_label)) { - session_reply(s, tag, "NO", "[SERVERBUG] internal error"); + session_reply(s, tag, "NO", + "[SERVERBUG] internal error"); return (1); } } @@ -999,7 +1137,10 @@ fetch_dispatch(struct session *s, const char *tag, cha } if (want_vanished && !req.has_changedsince) { - /* RFC 7162 SS3.2.6: VANISHED MUST be paired with CHANGEDSINCE, else tagged BAD */ + /* + * RFC 7162 SS3.2.6: VANISHED MUST pair with CHANGEDSINCE, else + * tagged BAD + */ session_reply(s, tag, "BAD", "VANISHED requires CHANGEDSINCE also be specified " "(RFC 7162 SS3.2.6)"); @@ -1011,7 +1152,10 @@ fetch_dispatch(struct session *s, const char *tag, cha req.attrs |= MBOX_FETCH_UID; if (s->store_iev == NULL) { - /* same invariant check as cmd_select(), ST_SELECTED requires store_iev already wired */ + /* + * same invariant as cmd_select(): ST_SELECTED requires + * store_iev wired + */ log_warnx("session %u: %s with no store channel wired", s->id, cmdname); session_reply(s, tag, "NO", "[SERVERBUG] internal error"); @@ -1020,7 +1164,10 @@ fetch_dispatch(struct session *s, const char *tag, cha req.nranges = nranges; - /* RFC 7162 SS3.1: MODSEQ fetch-att and CHANGEDSINCE modifier are both CONDSTORE-enabling */ + /* + * RFC 7162 SS3.1: MODSEQ fetch-att and CHANGEDSINCE both + * CONDSTORE-enabling + */ if (req.attrs & MBOX_FETCH_MODSEQ) session_condstore_enable(s); @@ -1031,6 +1178,7 @@ fetch_dispatch(struct session *s, const char *tag, cha } s->fetch_attrs = req.attrs; s->cmd_by_uid = by_uid; + s->fetch_incomplete = 0; s->state = SESSION_FETCHING; if (!send_mbox_request(s, IMSG_MBOX_FETCH, cmdname, "IMSG_MBOX_FETCH", blob - 3063e8439e40e298e3a780bbc9955988325c4100 blob + f3d26fd552c675eab5d75707a86c008bcd83c659 --- src/imapd.8 +++ src/imapd.8 @@ -3,7 +3,7 @@ .\" Written for the OpenIMAPD project. Public domain / no rights reserved, .\" matching the project's ports-oriented, OpenBSD-base-inclusion goal. .\" -.Dd $Mdocdate: September 9 2026 $ +.Dd $Mdocdate: September 18 2026 $ .Dt IMAPD 8 .Os .Sh NAME @@ -82,6 +82,8 @@ Within that scope it implements .Li CREATE , .Li DELETE , .Li RENAME , +.Li SUBSCRIBE , +.Li UNSUBSCRIBE , .Li LIST , .Li LSUB , .Li NAMESPACE , @@ -99,12 +101,32 @@ Within that scope it implements .Li IDLE , and the RFC 7162 CONDSTORE and QRESYNC extensions .Pq mod-sequence tracking, conditional STORE, VANISHED responses . -.Li SUBSCRIBE -and +.Li LIST +accepts the +.Li SUBSCRIBED +selection option of RFC 9051 Section 6.3.9.1. +.Li LSUB , +which RFC 9051 deprecates in favour of that option, is retained and +reports the same set of names. +An account that has never issued .Li UNSUBSCRIBE -are deliberately out of scope, not merely unimplemented; see CAVEATS -below. +has every mailbox subscribed; see +.Pa imapd.subscriptions +under FILES. .Pp +.Nm +logs to +.Xr syslogd 8 +with the +.Dv LOG_MAIL +facility, the same one +.Xr smtpd 8 +uses, so its messages go wherever +.Xr syslog.conf 5 +directs the mail facility. +Releases before 0.1.5 used +.Dv LOG_DAEMON . +.Pp The options are as follows: .Bl -tag -width Ds .It Fl d @@ -195,8 +217,11 @@ rereads .Pq or the file given via Fl f and reloads the .Ic spool , +.Ic append max , .Ic attachment max , .Ic idle poll , +.Ic lock timeout , +.Ic login grace , .Ic startups , .Ic tls certificate , and @@ -347,7 +372,7 @@ Defaults to TLS private key file. Defaults to .Pa /etc/ssl/private/imapd.key . -Must be owned by root or the current user, mode 0740 or stricter. +Must be owned by root, mode 0740 or stricter. .It Ic idle poll Ar seconds How often a session in .Li IDLE @@ -437,6 +462,81 @@ Defaults to 100, matching .Xr sshd_config 5 Ns 's own default of 10:30:100. +.It Ic login grace Ar seconds +How long a connection may go without authenticating before +.Nm +closes it. +.Pp +Every accepted connection costs three processes, a listener-worker, an +auth-worker and a search-oracle, and a connection that completes the TCP +handshake and then sends nothing would otherwise hold all three +indefinitely. +Enough such connections reach the +.Ic startups full +limit and every later connection is refused, so the timer bounds that +exposure. +It covers a connection that never begins a TLS handshake on the implicit +TLS port as well as one that never sends a command, since the former never +reaches the command path at all. +.Pp +RFC 9051, section 5.4 permits this explicitly: servers +.Qq are allowed to use a shortened pre-authentication timer to protect +.Qq themselves from Denial-of-Service attacks . +The timer is cancelled the moment a session authenticates and never applies +to an established session. +It is not the post-authentication autologout timer that the same section +requires to be at least 30 minutes; +.Nm +has no such timer. +.Pp +Must be between 1 and 3600 seconds inclusive, or 0 to disable it, which +reopens the denial of service described above. +Defaults to 60. +.It Ic lock timeout Ar seconds +How long a command waits for another session of the same user to release a +mailbox's index lock before giving up and answering +.Li NO +with the RFC 9051, section 7.1 +.Li INUSE +response code. +.Pp +Two connections for one account are ordinary, and a command that changes a +mailbox holds its index lock for the whole of the change. +A client marking a large mailbox read can therefore hold the lock for the +better part of a minute, and another of that user's clients waits behind it. +This directive bounds that wait. +.Pp +It is deliberately generous, because it is a safety net for a holder that +is stuck rather than a cure for one that is merely slow. +A deadline shorter than an ordinary command's own duration would refuse +ordinary concurrent use, and a client that does not retry +.Li INUSE +would discard the user's change with nothing shown on screen. +Raise it for mailboxes much larger than a hundred thousand messages, where +a single command can legitimately run longer than the default. +.Pp +Must be between 1 and 3600 seconds inclusive, or 0 to disable the bound, +which restores an unbounded wait. +Defaults to 120. +.It Ic append max Ar bytes +Largest message a client may upload with +.Li APPEND . +A larger one is refused with a +.Li NO +response carrying the RFC 5530 +.Li LIMIT +code, and nothing is stored. +.Ar bytes +must be between 1 and 1073741824 (1 GiB) inclusive; values outside that +range are a configuration error. +Defaults to 36700160 (35 MiB), the same as the +.Ic max-message-size +default of +.Xr smtpd.conf 5 . +It may not exceed +.Ic attachment max , +since a message larger than that could be stored but its structure +could not be fetched; such a configuration is an error. .It Ic attachment max Ar bytes Largest message .Nm @@ -450,6 +550,8 @@ structure cannot be produced, not truncated. .Ar bytes must be between 12000 and 1073741824 (1 GiB) inclusive; values outside that range are a configuration error. +It must be at least +.Ic append max . Defaults to 41943040 (40 MiB). .It Ic include Ar path Parse @@ -531,6 +633,52 @@ re-added keeps its history here, which is the desired removing a maildir by hand and recreating it resets the record, and a client holding a cache from before that point could in principle be misled. +.It Pa imapd.subscriptions +Per-user list of subscribed mailboxes, at the root of each maildir, one +mailbox name per line. +.Pp +The file is created on demand by the first +.Li UNSUBSCRIBE . +While it is absent, every mailbox is subscribed. +That rule is what lets an account upgraded from a version without +subscriptions keep the mailbox list its client already had, rather than +come back subscribed to nothing; the first +.Li UNSUBSCRIBE +therefore writes out every mailbox then present, less the one being +removed. +.Pp +A name stays in the file after its mailbox is deleted, which RFC 9051 +Section 6.3.8 requires. +.Li LIST +with the +.Li SUBSCRIBED +option reports such a name with the +.Li \eNonExistent +attribute, and +.Li LSUB +reports it with +.Li \eNoselect . +.Li INBOX +is never named in the file. +.Pp +Neither +.Li CREATE +nor +.Li RENAME +changes the file. +A new mailbox is subscribed only when a client subscribes it, and renaming +a subscribed mailbox leaves the old name in the file rather than moving it +to the new one. +RFC 9051 Section 6.3.8 tells a server not to remove a name from the +subscription list because the mailbox by that name no longer exists, and +moving it is exactly that, so what is subscribed is left to the client to +say. +A subscription stranded by a rename is repaired with one +.Li SUBSCRIBE . +.Pp +A file that cannot be read is treated as absent, so damage shows too many +mailboxes rather than too few, and the failure is logged. +It is removed with the maildir and needs no separate administration. .El .Sh NETWORK .Nm @@ -560,9 +708,11 @@ directive under FILES above. .Xr tls_init 3 , .Xr httpd.conf 5 , .Xr sshd_config 5 , +.Xr syslog.conf 5 , .Xr httpd 8 , .Xr imapduser 8 , -.Xr smtpd 8 +.Xr smtpd 8 , +.Xr syslogd 8 .Sh STANDARDS .Rs .%A A. Melnikov @@ -604,11 +754,31 @@ privilege-separation tradition of .Xr smtpd 8 . .Sh CAVEATS This implementation is under active development. -.Li SUBSCRIBE , -.Li UNSUBSCRIBE , -and shared or multi-user mailboxes +.Pp +.Li INBOX +cannot be unsubscribed. +RFC 9051 Section 5.1 guarantees that it always exists and that nothing +can delete it, so it is treated as permanently subscribed: +.Li SUBSCRIBE INBOX +succeeds and changes nothing, and +.Li UNSUBSCRIBE INBOX +replies +.Li NO . +A subscription-filtered view therefore always includes it. +.Pp +.Li LIST +selection options other than +.Li SUBSCRIBED +are refused rather than ignored. +.Li REMOTE +and +.Li RECURSIVEMATCH +are not implemented, and accepting either silently would misreport which +names the response contains. +.Pp +Shared or multi-user mailboxes .Pq no Li ACL support -are deliberately left out. +are deliberately out of scope. .Pp Mailbox names are required to be well-formed UTF-8. RFC 9051 Section 5.1 encodes them in Net-Unicode blob - 3488e7c9336f34c6c5d5a011a449644f91959cfa blob + 5b86996154c9b3069bea23f9c2a7cf037c026690 --- src/imapd.conf.example +++ src/imapd.conf.example @@ -46,7 +46,8 @@ listen on 0.0.0.0 tls port 993 # TLS certificate and private key. Default to /etc/ssl/imapd.crt and # /etc/ssl/private/imapd.key respectively. The key must be owned by -# root (or the current user) and mode 0740 or stricter. +# root (uid 0 specifically -- unlike imapd.conf itself, this check does +# not accept the current user) and mode 0740 or stricter. #tls certificate "/etc/ssl/imapd.crt" #tls key "/etc/ssl/private/imapd.key" @@ -59,6 +60,13 @@ listen on 0.0.0.0 tls port 993 # routinely carries larger attachments than that. attachment max 41943040 +# Largest message a client may upload with APPEND, see imapd(8). Must be +# between 1 and 1073741824 (1 GiB) bytes. Defaults to 36700160 (35 MiB), +# the same as smtpd.conf(5)'s max-message-size default, so a message the +# local MTA accepts can also be saved by a client. It may not exceed +# attachment max above, or imapd refuses the configuration. +#append max 36700160 + # How often a session in IDLE rechecks its selected mailbox. imapd does # not get an event when mail arrives, so IDLE is served by polling: new # mail is announced within one interval, whether it was delivered by an @@ -68,6 +76,35 @@ attachment max 41943040 # nothing until it sends DONE). Defaults to 5. #idle poll 5 +# How long a connection may go without authenticating before imapd +# closes it. Every accepted connection costs three processes (a +# listener-worker, an auth-worker and a search-oracle), and a connection +# that completes TCP and then says nothing would otherwise hold them for +# ever, so a few dozen silent connections can reach the "startups full" +# limit below and lock everyone else out. RFC 9051 section 5.4 permits +# this timer explicitly: "servers are allowed to use a shortened +# pre-authentication timer to protect themselves from Denial-of-Service +# attacks". The timer is cancelled the moment a session authenticates, +# so it never applies to an established session, and this is NOT the +# post-authentication autologout that the same RFC section puts a 30 +# minute floor under; imapd has no such timer. Must be 1-3600 seconds, +# or 0 to disable, which reopens the denial of service just described. +# Defaults to 60. +#login grace 60 + +# How long a command waits for another session of the same user to +# release a mailbox's index lock before answering NO [INUSE] (RFC 9051 +# section 7.1). Two clients on one account is ordinary, and a command +# that changes a mailbox holds the lock for the whole change, so a client +# marking a large mailbox read can hold it for the better part of a +# minute. This is a safety net for a holder that is stuck, not a cure for +# one that is slow: a deadline shorter than an ordinary command would +# refuse ordinary concurrent use, and a client that does not retry INUSE +# discards the user's change silently. Raise it for mailboxes much larger +# than a hundred thousand messages. Must be 1-3600 seconds, or 0 to +# disable the bound (an unbounded wait). Defaults to 120. +#lock timeout 120 + # Admission-control throttle on concurrent, not-yet-authenticated # connections, modeled on sshd_config(5)'s MaxStartups (see that # man page for the canonical description of this algorithm). blob - b39ecd65a8d22008183ce59025d7bcd9da215595 blob + 4b279ff164f1c69ee2048b601d4709ae2c096844 --- src/imapd.h +++ src/imapd.h @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -14,9 +16,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* - * Shared definitions for all four imapd(8) process roles - */ +/* Shared definitions for every imapd(8) process role. */ #ifndef IMAPD_H #define IMAPD_H @@ -24,32 +24,26 @@ #include #include /* __dead */ #include -#include /* struct sockaddr_storage, socklen_t -- - * imsg_listener_session_init below */ +#include /* sockaddr_storage, socklen_t */ #include #include #include -#define IMAPD_VERSION "0.1.4" +#define IMAPD_VERSION "0.1.5" -/* - * Process roles, selected at exec time via "-x ". See main.c. - */ +/* Process roles, selected at exec time via "-x ". See main.c. */ enum openimap_proc_type { PROC_PARENT, PROC_LISTENER, PROC_AUTH, PROC_STORE, PROC_KEYMGR, - PROC_SEARCH /* SS8: per-connection SEARCH-grammar - * parsing oracle, docs/openimap-tls-privsep- - * design.md SS8.1 */ + PROC_SEARCH /* per-connection SEARCH-grammar + * parsing oracle */ }; -/* - * imsg message catalog. - */ +/* imsg message catalog. */ enum imsg_type { IMSG_NONE, @@ -57,26 +51,12 @@ enum imsg_type { IMSG_SETUP_PEER, IMSG_SETUP_DONE, - IMSG_TLS_CERT, /* parent -> new listener-worker, once, - * during its own per-connection boot - * sequence (part of the same handshake - * as IMSG_LISTENER_SESSION_INIT below); - * parent -> keymgr, at boot AND on every - * SIGHUP reload (keymgr.c is still the - * one long-lived process that needs a - * live reload path, see its header - * comment). The certificate is public, - * so every recipient just gets its own - * copy. */ + /* parent -> listener-worker at fork, and -> keymgr at boot and on */ + /* every SIGHUP. The certificate is public; each gets its own copy. */ + IMSG_TLS_CERT, - /* - * parent -> new listener-worker, once, immediately after - * fork -- SS7's replicated-listener model has parent doing - * the accept() itself (see parent.c's header comment) and - * handing off one already-accepted connection (fd-passed - * alongside this payload) instead of a long-lived listener - * accept()ing from bound sockets handed to it at boot. - */ + /* parent -> listener-worker at fork: one already-accepted */ + /* connection, the client fd riding as this imsg's fd-pass. */ IMSG_LISTENER_SESSION_INIT, /* parent -> auth, at boot */ @@ -86,49 +66,31 @@ enum imsg_type { IMSG_AUTH_REQUEST, IMSG_AUTH_RESULT, - /* - * parent -> new listener-worker AND parent -> new search- - * oracle-worker, once each, immediately after fork -- SS8's - * narrow per-connection SEARCH-parsing oracle (docs/openimap- - * tls-privsep-design.md SS8.1). Kept distinct from - * IMSG_SETUP_PEER rather than reusing its id field: - * listener_main()'s boot-drain loop already uses id as a - * hard binary discriminator (0 for the auth peer, nonzero/ - * session_id for the keymgr peer), and a third peer with no - * unambiguous id to claim is cleaner as its own type than a - * guessed sentinel value. - */ + /* parent -> listener-worker and -> search-oracle at fork, wiring the */ + /* per-connection SEARCH-parsing oracle. Its own type rather than */ + /* IMSG_SETUP_PEER, whose id field is already a peer discriminator. */ IMSG_SETUP_SEARCH_PEER, - /* - * listener -> search-oracle: already-buffered SEARCH argument - * text (post RETURN/CHARSET; the highest-risk grammar only, - * see SS8.1), sent as raw trailing bytes with no fixed - * struct, same technique as IMSG_TLS_CERT. search-oracle -> - * listener: struct imsg_search_parse_result below, echoing - * parse_search_key_list()'s own (rc, errmsg) contract - * (search_cmd.c). At most one of these round-trips is ever - * in flight per session -- listener.c's session_is_busy()/ - * cmd_queue pipelining already makes a second SEARCH wait, - * not race, so no correlation id is needed. - */ + /* listener -> search-oracle: SEARCH argument text as raw trailing */ + /* bytes, no fixed struct. Oracle -> listener: struct */ + /* imsg_search_parse_result. One round trip per session at most, so */ + /* no correlation id. */ IMSG_SEARCH_PARSE_REQUEST, IMSG_SEARCH_PARSE_RESULT, - /* parent -> keymgr, at boot and on SIGHUP reload: the real TLS - * private key (IMSG_TLS_CERT above carries the matching - * certificate). See docs/openimap-tls-privsep-design.md SS5. */ + /* parent -> keymgr, at boot and on SIGHUP: the real TLS private key */ IMSG_KEYMGR_INIT, - /* listener <-> keymgr: a private-key operation, forwarded - * synchronously from listener's OpenSSL RSA_METHOD/EC_KEY_METHOD - * engine override (see listener.c) to keymgr and back. Each - * reply reuses the same type as its request, correlated by the - * imsg id field, mirroring smtpd's ca.c IMSG_CA_* convention. */ + /* listener <-> keymgr: one private-key operation, forwarded from */ + /* listener's OpenSSL engine override. Each reply reuses its */ + /* request's type, correlated by the imsg id field. */ IMSG_KEYMGR_RSA_PRIVENC, IMSG_KEYMGR_RSA_PRIVDEC, IMSG_KEYMGR_ECDSA_SIGN, + /* parent -> keymgr, on shutdown: exit, this was not a crash */ + IMSG_KEYMGR_SHUTDOWN, + /* auth -> parent, per successful login */ IMSG_AUTH_CRED, @@ -137,25 +99,21 @@ enum imsg_type { IMSG_STORE_INIT, IMSG_STORE_SHUTDOWN, - /* listener <-> store, once a session's store child is wired up */ - IMSG_MBOX_SELECT, /* EXAMINE too: it is a SELECT with the - * request's "readonly" field set, see - * select_or_examine() in mailbox_cmd.c */ + /* listener <-> store, once a session's store child is wired up. */ + /* SELECT carries EXAMINE too, as a request with "readonly" set. */ + IMSG_MBOX_SELECT, IMSG_MBOX_SELECTED, IMSG_MBOX_FETCH, IMSG_MBOX_FETCH_META, - IMSG_MBOX_FETCH_HEADER, /* raw BODY.PEEK[HEADER] bytes for one - * message (store -> listener) */ - IMSG_MBOX_FETCH_BODY, /* raw BODY.PEEK[] / BODY.PEEK[TEXT] bytes for - * one message (store -> listener) */ - IMSG_MBOX_FETCH_ENVELOPE, /* pre-formatted ENVELOPE parenthesized- - * list text for one message (store -> - * listener)*/ - IMSG_MBOX_FETCH_BODYSTRUCTURE, /* pre-formatted BODYSTRUCTURE - * parenthesized-list text for one - * message (store -> listener) */ + /* the four below are all store -> listener, one per message */ + IMSG_MBOX_FETCH_HEADER, /* raw BODY.PEEK[HEADER] bytes */ + IMSG_MBOX_FETCH_BODY, /* body descriptor and octet range */ + IMSG_MBOX_FETCH_ENVELOPE, /* formatted ENVELOPE text */ + IMSG_MBOX_FETCH_BODYSTRUCTURE, /* formatted BODYSTRUCTURE text */ IMSG_MBOX_STORE, - IMSG_MBOX_APPEND, + IMSG_MBOX_APPEND, /* opens the message's tmp/ file */ + IMSG_MBOX_APPEND_DATA, /* one piece of the literal */ + IMSG_MBOX_APPEND_END, /* literal complete, commit it */ IMSG_MBOX_APPENDED, IMSG_MBOX_COPY, IMSG_MBOX_MOVE, @@ -172,35 +130,28 @@ enum imsg_type { IMSG_MBOX_RENAME, IMSG_MBOX_RESULT, - /* - * RFC 7162 (CONDSTORE/QRESYNC) additions - */ - IMSG_MBOX_SELECT_VANISHED, /* one vanished UID during a QRESYNC - * SELECT resync (store -> listener, - * before IMSG_MBOX_SELECTED) */ - IMSG_MBOX_STORE_MODIFIED, /* one message that failed a STORE's - * UNCHANGEDSINCE test (store -> - * listener, before IMSG_MBOX_RESULT) */ + /* RFC 7162 CONDSTORE/QRESYNC. Both store -> listener, streamed */ + /* before the terminal reply. */ + IMSG_MBOX_SELECT_VANISHED, /* one vanished UID, QRESYNC resync */ + IMSG_MBOX_STORE_MODIFIED, /* one UNCHANGEDSINCE failure */ - /* - * RFC 9051 SS6.3.13 (IDLE) additions. - */ - IMSG_MBOX_IDLE_REFRESH, /* listener -> store, no payload */ - IMSG_MBOX_IDLE_UID, /* one currently-existing UID, in - * ascending order (store -> listener, - * before IMSG_MBOX_IDLE_REFRESHED) */ - IMSG_MBOX_IDLE_REFRESHED, /* terminal reply (store -> listener) */ + /* RFC 9051 SS6.3.13 (IDLE) */ + IMSG_MBOX_IDLE_REFRESH, /* listener -> store, seed or diff */ + IMSG_MBOX_IDLE_EXPUNGE, /* one untagged EXPUNGE to print */ + IMSG_MBOX_IDLE_FETCH, /* one flag change, as fetch_meta */ + IMSG_MBOX_IDLE_REFRESHED, /* terminal reply */ - /* - * RFC 9051 SS6.3.4-SS6.3.6 (CREATE/DELETE/RENAME) and SS6.3.9 (LIST). - */ - IMSG_MBOX_LIST_ITEM /* one mailbox name (store -> listener), - * before the terminal IMSG_MBOX_RESULT */ + /* RFC 9051 SS6.3.9 (LIST): one mailbox name, store -> listener, */ + /* before the terminal IMSG_MBOX_RESULT. */ + IMSG_MBOX_LIST_ITEM, + + /* RFC 9051 SS6.3.7/SS6.3.8: both carry struct imsg_mbox_subscribe */ + /* and reply with IMSG_MBOX_RESULT. */ + IMSG_MBOX_SUBSCRIBE, + IMSG_MBOX_UNSUBSCRIBE }; -/* - * privsep imsg-over-event(3) wrapper. - */ +/* privsep imsg-over-event(3) wrapper. */ struct imsgev { struct imsgbuf ibuf; void (*handler)(int, short, void *); @@ -213,18 +164,9 @@ struct imsgev { struct openimap_config { char listen_addr[64]; /* "0.0.0.0" (default), "::", a literal - * IPv4/IPv6 address, or "*" for both - *, see LISTENER_MAX_ADDRS above */ - /* - * A port of 0 means "this listener is not configured, do not bind - * it". parse.y seeds both with their defaults and clears the one a - * config did not ask for -- but only when the config named at least - * one "listen" line, so a config with none still gets both, as it - * always has. Before this existed, parse.y recorded which listeners - * were named in file-static variables that never reached this struct, - * so parent.c bound both unconditionally and "listen on * tls port - * 993" alone still served cleartext on 143. - */ + * address, or "*" for both */ + /* A port of 0 means this listener is not configured. parse.y clears */ + /* the one a config did not name, but only if it named either. */ uint16_t port_cleartext; /* 143, STARTTLS; 0 = not configured */ uint16_t port_implicit_tls; /* 993, RFC 8314; 0 = not configured */ char spool_root[1024]; /* mail spool root, store's chroot */ @@ -235,57 +177,50 @@ struct openimap_config { char tls_cert_file[1024]; char tls_key_file[1024]; uint32_t bodystructure_read_max; /* "attachment max" directive */ + uint64_t append_max; /* "append max" directive */ - /* SS7's "startups begin/rate/full" directive; see IMSG_ - * LISTENER_MAXSTARTUPS's enum comment. Defaults (10/30/100) - * set by config_load(), matching sshd_config(5)'s own - * default of "10:30:100". */ + /* the "startups begin/rate/full" directive. config_load() defaults */ + /* to 10/30/100, matching sshd_config(5)'s own "10:30:100". */ uint32_t max_startups_begin; uint32_t max_startups_rate; /* percent, 0-100 */ uint32_t max_startups_full; - /* - * "idle poll" directive: how often an IDLEing session asks its store - * child whether the selected mailbox has changed. 0 disables polling - * entirely, which restores the pre-poll behaviour -- an IDLEing - * session then sees nothing until it sends DONE. See listener.c's - * session_idle_poll() and index.c's idle_probe_unchanged(). - */ + /* "idle poll": how often an IDLEing session asks its store child */ + /* whether the mailbox changed. 0 disables polling, and an IDLEing */ + /* session then sees nothing until it sends DONE. */ uint32_t idle_poll_secs; + + /* "lock timeout": seconds a command waits for another session's */ + /* index lock before answering NO [INUSE]. 0 disables the bound, */ + /* restoring the unbounded wait it replaced. */ + uint32_t lock_timeout_secs; + + /* "login grace": seconds a connection may go without authenticating */ + /* before it is closed. 0 disables the timer, which reopens the */ + /* denial of service it exists to stop. */ + uint32_t login_grace_secs; }; -/* - * imsg payload wire structs. - */ +/* imsg payload wire structs. */ #define AUTH_USERNAME_MAX 64 #define AUTH_PASSWORD_MAX 128 #define AUTH_MAILDIR_MAX 256 -/* - * Boot-time config-delivery payloads. - */ -/* - * IMSG_LISTENER_SESSION_INIT's payload; see its enum comment. - * The accepted client fd itself rides as the imsg's fd-pass, - * not a field here. remote_ss/remote_sslen are the raw - * sockaddr accept(2) filled in for parent, carried as-is - * rather than pre-formatted, so the listener-worker keeps - * doing its own getnameinfo() formatting into struct - * session's remote_addr, same as listener_start_session() - * always has (listener.c). - */ +/* Boot-time config-delivery payloads. */ + +/* IMSG_LISTENER_SESSION_INIT's payload. The client fd rides as the imsg's */ +/* fd-pass, not a field here. remote_ss/remote_sslen are the raw sockaddr */ +/* from accept(2), so the worker does its own getnameinfo() formatting. */ struct imsg_listener_session_init { uint32_t session_id; int implicit_tls; struct sockaddr_storage remote_ss; socklen_t remote_sslen; - /* - * Carried per connection rather than read from a config the - * listener-worker does not have: under SS7 this process is spawned - * fresh per connection, so a SIGHUP that changes "idle poll" reaches - * every later connection with no reload machinery of its own. - */ + /* carried per connection, since this process is spawned fresh per */ + /* connection and so needs no reload path of its own */ uint32_t idle_poll_secs; + uint32_t login_grace_secs; + uint64_t append_max; }; struct imsg_auth_init { @@ -293,45 +228,39 @@ struct imsg_auth_init { }; /* - * IMSG_KEYMGR_RSA_PRIVENC / IMSG_KEYMGR_RSA_PRIVDEC / IMSG_KEYMGR_ - * ECDSA_SIGN (listener -> keymgr, request; keymgr -> listener, - * reply, same imsg type both ways, correlated by the imsg id - * field). Mirrors smtpd's ca.c IMSG_CA_RSA_PRIVENC/_PRIVDEC/_ECDSA_ - * SIGN payload shape (request id, pubkey hash, input bytes, target - * length/padding mode; result length + output bytes), adapted to - * imapd's own "fixed header + trailing raw bytes on one imsg" - * convention (imsg_get_buf()+imsg_get_len(), see imsg_mbox_append - * below) instead of smtpd's m_* message-abstraction macros. + * IMSG_KEYMGR_RSA_PRIVENC / _RSA_PRIVDEC / _ECDSA_SIGN (listener to keymgr + * and back, same imsg type each way, correlated by the imsg id field). + * Fixed header plus trailing raw bytes on one imsg, as imsg_mbox_append + * below does. * - * hash is libtls's tls_cert_pubkey_hash() format ("SHA256:" plus - * lowercase hex of the certificate's DER SubjectPublicKeyInfo - * digest), computed independently by keymgr.c's keymgr_pubkey_ - * hash() from the certificate it holds; a request whose hash - * doesn't match is refused. padding is an RSA_PKCS1_PADDING-style - * OpenSSL padding constant, meaningful only for the two RSA - * operations, ignored for ECDSA_SIGN. - * - * KEYMGR_DATA_MAX (1024 bytes) covers both directions: an RSA - * to/from buffer sized to RSA_size() (1024 bytes exactly covers an - * 8192-bit RSA key, comfortably past any realistic configuration) - * and an ECDSA digest/signature, both far smaller in practice. + * hash is libtls's tls_cert_pubkey_hash() format ("SHA256:" plus lowercase + * hex of the certificate's DER SubjectPublicKeyInfo digest); keymgr.c + * recomputes it from the certificate it holds and refuses a mismatch. + * padding is an OpenSSL RSA_PKCS1_PADDING-style constant, ignored for + * ECDSA_SIGN. KEYMGR_DATA_MAX (1024) covers an RSA buffer up to an + * 8192-bit key and any ECDSA digest or signature. */ #define KEYMGR_HASH_MAX 80 /* "SHA256:" + 64 hex chars + NUL, generous */ #define KEYMGR_DATA_MAX 1024 struct imsg_keymgr_sign_request { char hash[KEYMGR_HASH_MAX]; - uint32_t padding; /* RSA padding mode; ignored for ECDSA */ - uint32_t fromlen; /* trailing input bytes, <= KEYMGR_DATA_MAX */ + /* RSA padding mode; ignored for ECDSA */ + uint32_t padding; + /* trailing input bytes, <= KEYMGR_DATA_MAX */ + uint32_t fromlen; }; struct imsg_keymgr_sign_reply { - int ok; /* 0 = refused/failed; listener's engine callback - * returns this straight to OpenSSL, which fails - * that one RSA/EC operation, same as any other - * engine failure -- see keymgr.c's header comment - * on why this is a reply, not a fatalx() */ - uint32_t tolen; /* trailing output bytes, meaningful only if ok */ + /* + * 0 = refused/failed; listener's engine callback returns this straight + * to OpenSSL, which fails that one RSA/EC operation, same as any other + * engine failure -- see keymgr.c's header comment on why this is a + * reply, not a fatalx() + */ + int ok; + /* trailing output bytes, meaningful only if ok */ + uint32_t tolen; }; struct imsg_auth_request { @@ -365,13 +294,18 @@ struct imsg_store_init { uint32_t session_id; uid_t uid; gid_t gid; - char spool_root[1024]; /* store needs this to chroot() */ - char maildir[STORE_MAILDIR_MAX]; /* THIS session's own - * mailbox subdirectory, relative - * to spool_root above */ - uint32_t bodystructure_read_max; /* copied from struct - * openimap_config's field of the - * same name */ + /* store needs this to chroot() */ + char spool_root[1024]; + /* + * THIS session's own mailbox subdirectory, relative to spool_root above + */ + char maildir[STORE_MAILDIR_MAX]; + /* + * copied from struct openimap_config's field of the same name + */ + uint32_t bodystructure_read_max; + uint64_t append_max; + uint32_t lock_timeout_secs; }; /* @@ -387,7 +321,7 @@ struct imsg_store_init { * * MBOX_OP_ERR_NO_SUCH_MAILBOX and MBOX_OP_ERR_ALREADY_EXISTS are * client-visible via RFC 5530 SS3's NONEXISTENT and ALREADYEXISTS codes - * respectively. + * respectively, and MBOX_OP_ERR_BUSY via RFC 9051 SS7.1's INUSE. */ enum mbox_op_error { MBOX_ERR_UNSET = 0, @@ -395,6 +329,8 @@ enum mbox_op_error { MBOX_OP_ERR_GENERIC, MBOX_OP_ERR_NO_SUCH_MAILBOX, MBOX_OP_ERR_ALREADY_EXISTS, + /* gave up waiting for another session's index lock */ + MBOX_OP_ERR_BUSY, }; /* QRESYNC select-param (RFC 7162 SS3.2.5). */ @@ -465,8 +401,8 @@ struct imsg_mbox_selected { * streaming. * * mailbox targets any named mailbox independent of what's selected - * (SS6.3.11); store.c visits it via select_mailbox_dir() and restores the - * prior selection afterward. + * (SS6.3.11); store.c opens it with mailbox_open_dir() and closes it + * afterward, leaving the selection alone. */ struct imsg_mbox_status { char mailbox[MBOX_NAME_MAX]; @@ -575,126 +511,77 @@ struct imsg_mbox_select_vanished { * hence a separate bit from * WHOLE/TEXT. */ -/* - * Cap on the dotted-numeric section-part string (struct imsg_mbox_fetch's - * section_part below). Sized for MIME_MAX_DEPTH (10) levels x 2 digits - * plus dots: 29 worst case; 40 leaves headroom. - * - * An oversized section-part is never truncated. fetch_cmd.c's - * parse_body_peek_section_tok() DROPS that one fetch-att and sets its - * degraded flag -- the same lenient skip every other unsupported - * BODY[...] form gets -- so the response simply omits that item, and the - * client only sees a NO if every item it asked for was dropped. (This - * comment previously said listener.c rejects an oversized section-part - * with BAD; it does not, and never did.) - */ +/* Cap on the dotted-numeric section-part string. An oversized one is */ +/* dropped as an unsupported fetch-att, not truncated and not an error. */ #define SECTION_PART_MAX 40 -/* - * Cap on raw header bytes IMSG_MBOX_FETCH_HEADER can carry (must fit - * under imsg's MAX_IMSGSIZE, 16384). Real RFC 5322 headers are well - * under 8192; an oversized header is "not found" for BODY.PEEK[HEADER], - * not truncated. - */ +/* Cap on raw header bytes one IMSG_MBOX_FETCH_HEADER carries. An */ +/* oversized header is "not found", not truncated. */ #define FETCH_HEADER_MAX 8192 -/* - * Cap on a whole message's size, for both APPEND and BODY.PEEK[]/ - * BODY.PEEK[TEXT] (shared so the two enforcement points can't drift). - * 12000 leaves headroom under imsg's 16384-byte MAX_IMSGSIZE. Larger - * messages need real fd-passing, not implemented; rejected rather than - * truncated. - */ -#define APPEND_LITERAL_MAX 12000 +/* "append max" default and ceiling. The default matches smtpd.conf(5)'s */ +/* max-message-size default of 35M. */ +#define APPEND_MAX_DEFAULT (35 * 1024 * 1024) +#define APPEND_MAX_MAX 1073741824 -/* - * Cap on bytes any single BODY[
]/BODY.PEEK[
] response - * can carry on one IMSG_MBOX_FETCH_BODY; reuses APPEND_LITERAL_MAX's - * value and MAX_IMSGSIZE-headroom reasoning. Real clients (confirmed: - * Apple Mail) re-fetch large parts via <> ranges rather than - * expecting a whole part in one response. A <> count larger - * than this is clamped down, per RFC 9051 SS6.4.5's own truncate-past- - * end-of-text precedent, rather than rejected. - */ -#define FETCH_PART_MAX APPEND_LITERAL_MAX +/* Bytes a FETCH walk composes before it pauses to let them drain, so */ +/* the store holds this much of a reply rather than all of it. */ +#define FETCH_BATCH_MAX (1024 * 1024) -/* - * Cap on the space-joined header-field-name list a BODY.PEEK[HEADER. - * FIELDS[.NOT] (...)] fetch-att carries to store.c. 256 bytes covers - * any realistic request; listener.c rejects (BAD) an oversized list - * rather than truncating it. - */ +/* Descriptors a FETCH walk passes before it pauses. Each holds a slot in */ +/* the system-wide file table, kern.maxfiles, until it has been sent. */ +#define FETCH_FD_MAX 16 + +/* Cap on the space-joined header-field-name list; an oversized list is */ +/* rejected, not truncated. */ #define HEADER_FIELDS_MAX 256 -/* - * Cap on the fully-formatted ENVELOPE text store.c's build_envelope() - * can carry on one IMSG_MBOX_FETCH_ENVELOPE. Sized off FETCH_HEADER_MAX - * (8192), since every envelope field comes from a header bounded by - * that same cap. An oversized envelope is "not found", not truncated. - */ +/* Cap on formatted ENVELOPE text; an oversized one is "not found". */ #define ENVELOPE_MAX 8192 -/* - * Caps on store.c's recursive BODYSTRUCTURE builder (build_body_ - * structure()): a message's own headers claim its own part count and - * nesting depth, so both need a hard ceiling rather than trusting them. - * Exceeding either is "not found" for that message's BODYSTRUCTURE - * rather than a truncated part tree. - */ +/* Ceilings on the recursive BODYSTRUCTURE builder: a message's own */ +/* headers claim its part count and depth, so neither may be trusted. */ +/* Exceeding either is "not found" rather than a truncated part tree. */ #define MIME_MAX_DEPTH 10 #define MIME_MAX_PARTS 64 -/* - * Cap on the fully-formatted BODYSTRUCTURE text store.c's - * build_bodystructure() can carry on one IMSG_MBOX_FETCH_BODYSTRUCTURE. - * 12000 mirrors APPEND_LITERAL_MAX's MAX_IMSGSIZE-headroom reasoning; - * MIME_MAX_PARTS/MIME_MAX_DEPTH, not this byte cap, are expected to be - * the limiting factor in practice. - */ +/* Cap on formatted BODYSTRUCTURE text; MIME_MAX_PARTS and */ +/* MIME_MAX_DEPTH above are the limits that bite first in practice. */ #define BODYSTRUCTURE_MAX 12000 -/* - * "idle poll" bounds. The default is short because a poll is cheap: the store - * child answers an unchanged mailbox with two stat(2) calls and no lock (see - * index.c's idle_probe_unchanged()), so the cost of a tighter interval is - * two syscalls and one small imsg round trip per idling session. - * IDLE_POLL_MAX is a sanity bound, not a protocol limit -- RFC 2177 lets a - * client hold an IDLE for 29 minutes, and a poll slower than a few minutes - * would make IDLE indistinguishable from the broken behaviour this replaced. - */ +/* "idle poll" bounds. The default is short because an unchanged mailbox */ +/* costs two stat(2) calls and no lock. IDLE_POLL_MAX is a sanity bound. */ #define IDLE_POLL_DEFAULT 5 /* seconds */ #define IDLE_POLL_MAX 300 /* seconds; 0 disables polling */ -/* - * Cap on raw on-disk bytes build_bodystructure() (via read_message_ - * body()) will read while deriving a message's MIME structure. - * Independent from APPEND_LITERAL_MAX: mail delivered by an external - * MTA isn't size-bounded by this server's own APPEND cap, and - * real-hardware testing showed base64 attachments routinely exceed it. - * The read is never sent back over the wire whole (only the derived, - * already-bounded structure summary is), so it can afford to be large. - * - * 41943040 (40 MiB) is sized off Gmail's documented 25MB attachment - * limit (support.google.com/mail/answer/6584) plus base64 overhead, not - * a protocol requirement. Exceeding it is "not found" for that - * message's BODYSTRUCTURE, not truncated. - * - * Operator-configurable via imapd.conf's "attachment max " - * directive (parse.y, struct openimap_config's bodystructure_read_max). - * store.c uses the runtime value from IMSG_STORE_INIT; this macro now - * only serves as config_load()'s default when the directive is absent. - */ +/* "login grace" bounds. RFC 9051 SS5.4 permits a shortened */ +/* pre-authentication timer specifically against denial of service; the */ +/* 30 minute floor in that section governs a POST-authentication */ +/* autologout, which this server does not have. */ +#define LOGIN_GRACE_DEFAULT 60 /* seconds */ +#define LOGIN_GRACE_MAX 3600 /* seconds; 0 disables the timer */ + +/* "lock timeout" bounds. A command that cannot take a mailbox's index */ +/* lock waits this long before answering NO [INUSE] (RFC 9051 SS7.1). It */ +/* is a safety net for a holder that is stuck, not a cure for one that is */ +/* merely slow: an ordinary STORE over a large mailbox holds the lock for */ +/* the better part of a minute on modest hardware, and a deadline under */ +/* that would refuse ordinary concurrent use. */ +#define LOCK_TIMEOUT_DEFAULT 120 /* seconds */ +#define LOCK_TIMEOUT_MAX 3600 /* seconds; 0 disables the bound */ + +/* Cap on on-disk bytes read while deriving a message's MIME structure. */ +/* Deliberately above APPEND_MAX_DEFAULT: mail from an external MTA is */ +/* not bounded by "append max", and only the derived summary */ +/* goes back over the wire. Exceeding it is "not found", not truncated. */ +/* config_load()'s default for imapd.conf's "attachment max". */ #define BODYSTRUCTURE_READ_DEFAULT 41943040 /* - * RFC 9051 SS9 sequence-set: (seq-number/seq-range) *("," seq-number/ - * seq-range) -- one comma-separated range. "*" ("the last message") is - * carried unresolved via lo_is_star/hi_is_star; only the store process - * knows the live value (highest sequence number or UID in use) to - * resolve it against. A full sequence-set travels to the store process - * as an imsg request's trailing variable-length array of these (struct - * seq_range ranges[nranges]), the same pattern already used below for - * IMSG_MBOX_SEARCH's search_node array. + * One comma-separated range of an RFC 9051 SS9 sequence-set. "*" travels + * unresolved via lo_is_star/hi_is_star, since only the store knows the live + * value to resolve it against. A whole sequence-set rides as an imsg + * request's trailing seq_range[nranges] array. */ struct seq_range { uint32_t lo; /* 1-based, inclusive; ignored if lo_is_star */ @@ -703,16 +590,8 @@ struct seq_range { int hi_is_star; }; -/* - * Bounds a sequence-set's comma-separated range count, both on the wire - * (so a struct seq_range trailing array can't grow an imsg past - * MAX_IMSGSIZE, 16384 -- see imsgev.c) and for the store side's own - * per-range membership check. 500 ranges is 8000 bytes of trailing - * array, well under budget alongside any of this file's imsg_mbox_* - * request headers; a client whose sequence-set has more comma segments - * than fit in listener.h's 8192-byte SESSION_INBUF_MAX command line - * already can't reach this cap in practice. - */ +/* Bounds a sequence-set's range count, so the trailing seq_range array */ +/* cannot grow an imsg past MAX_IMSGSIZE. */ #define SEQSET_MAX_RANGES 500 struct imsg_mbox_fetch { @@ -763,8 +642,8 @@ struct imsg_mbox_fetch { * range (SS6.4.5), applying uniformly to whole/TEXT/section_part. * store.c slices the extracted content to * [partial_start, partial_start + partial_count), clamped to - * FETCH_PART_MAX and the content's actual length (SS6.4.5's - * truncate-past-end-of-text rule). has_partial distinguishes + * the content's actual length (SS6.4.5's truncate-past-end-of-text + * rule) and to nothing else. has_partial distinguishes * "no range" from a legal partial_start of 0. */ char section_part[SECTION_PART_MAX]; @@ -819,28 +698,22 @@ struct imsg_mbox_fetch_header { /* * IMSG_MBOX_FETCH_BODY: sent by store.c immediately before the * IMSG_MBOX_FETCH_META for the same message, iff req->attrs & - * (MBOX_FETCH_BODY_WHOLE | MBOX_FETCH_BODY_TEXT). Same ordering contract - * and wire shape as imsg_mbox_fetch_header above. + * (MBOX_FETCH_BODY_WHOLE | MBOX_FETCH_BODY_TEXT | MBOX_FETCH_BODY_PART). + * The octets do not ride on the imsg. A found, non-empty body carries a + * read-only descriptor on the message file, and offset and length say + * which octets the literal is; the store has already done any parsing, + * so the listener only reads and writes. */ struct imsg_mbox_fetch_body { uint32_t seqno; /* 1-based, matches the following * IMSG_MBOX_FETCH_META */ uint32_t uid; int found; /* 0 if the message file couldn't be - * found, it exceeded - * APPEND_LITERAL_MAX, it contained a - * NUL byte, or (is_text only) no - * header/body separator was found. - * bodylen and the trailing bytes are - * only meaningful if 1. */ - int is_text; /* 0 = BODY.PEEK[] bytes (whole - * message); 1 = BODY.PEEK[TEXT] bytes - * (body only). Set from which of - * MBOX_FETCH_BODY_WHOLE/_TEXT was - * requested (WHOLE wins if both). */ - uint32_t bodylen; /* length of the trailing raw body - * bytes on this imsg, capped at - * APPEND_LITERAL_MAX */ + * found, it contained a NUL byte, or + * (TEXT only) no header/body + * separator was found */ + uint64_t offset; /* first octet, from the file's start */ + uint64_t length; /* octets; no descriptor when 0 */ }; /* @@ -1083,17 +956,11 @@ struct imsg_mbox_copy_mapping { * IMSG_MBOX_APPEND (listener -> store) / IMSG_MBOX_APPENDED (store -> * listener, exactly once). * - * RFC 9051 SS6.3.12 append literal (the message body) is carried as - * variable-length trailing data on the SAME imsg after this fixed - * struct, not fd-passed: store.c's handle_mbox_append() reads the - * struct with imsg_get_buf(), then reads whatever imsg_get_len() - * reports afterward as the body (imsg_get_data() can't be used here, - * it requires an exact length match against the whole imsg). - * - * Only works because the message is capped at APPEND_LITERAL_MAX to - * fit under imsg's MAX_IMSGSIZE (16384) alongside this struct. A larger - * message needs real fd-passing, not implemented; listener.c rejects an - * oversized literal announcement with a plain NO before reading it. + * The RFC 9051 SS6.3.12 literal does not ride on this imsg. This struct + * goes alone when the literal is announced, the octets follow as + * IMSG_MBOX_APPEND_DATA pieces as they are read, and IMSG_MBOX_APPEND_END + * follows the command's closing CRLF. The store answers only after END. + * Each piece is at most SESSION_INBUF_MAX, under MAX_IMSGSIZE. */ struct imsg_mbox_append { char mailbox[MBOX_NAME_MAX]; @@ -1105,17 +972,14 @@ struct imsg_mbox_append { * (SS6.3.12) */ int64_t date; /* Unix timestamp; meaningful only if * has_date */ - uint32_t msglen; /* length of the trailing message - * bytes; redundant with imsg_get_len() - * after the header is read, kept as - * an explicit cross-check against - * struct-layout skew or truncation */ + uint64_t msglen; /* announced literal length; the + * DATA pieces must add up to it */ }; struct imsg_mbox_appended { enum mbox_op_error error; /* MBOX_OP_ERR_NO_SUCH_MAILBOX, "not * INBOX", tagged NO gets [TRYCREATE] - * per SS6.3.12; MBOX_OP_ERR_GENERIC, + * per SS6.3.12; MBOX_OP_ERR_GENERIC, * plain NO */ uint32_t uidvalidity; uint32_t uid; /* appended message's UID; with @@ -1237,9 +1101,11 @@ struct imsg_mbox_search { #define SEARCH_ORACLE_ERRMSG_MAX 128 struct imsg_search_parse_result { int rc; /* 0 ok, -1 BAD, -2 NO */ - uint32_t nnodes; /* valid when rc == 0; struct - * search_node[nnodes] trails, same - * technique as imsg_mbox_search above */ + /* + * valid when rc == 0; struct search_node[nnodes] trails, same technique + * as imsg_mbox_search above + */ + uint32_t nnodes; int uses_modseq; /* valid when rc == 0, see this * struct's own comment */ char errmsg[SEARCH_ORACLE_ERRMSG_MAX]; /* valid when @@ -1260,56 +1126,60 @@ struct imsg_mbox_search_match { }; /* - * RFC 9051 SS6.3.13 (IDLE). IMSG_MBOX_IDLE_REFRESH (listener -> store, no - * payload) / IMSG_MBOX_IDLE_UID (store -> listener, one per existing - * message, ascending UID order) / IMSG_MBOX_IDLE_REFRESHED (store -> - * listener, terminal). + * RFC 9051 SS6.3.13 (IDLE). IMSG_MBOX_IDLE_REFRESH (listener -> store), + * IMSG_MBOX_IDLE_EXPUNGE (store -> listener, one per untagged EXPUNGE to + * print, in order), IMSG_MBOX_IDLE_FETCH (store -> listener, one per + * message whose flags changed, carrying imsg_mbox_fetch_meta as FETCH and + * QRESYNC resync do), IMSG_MBOX_IDLE_REFRESHED (terminal). * - * Used by listener.c first synchronously after "+ idling", to seed - * s->idle_known_uids with a baseline, and then once per "idle poll" - * interval for as long as the session stays in IDLE. store.c reports - * current state via the same refresh_index() helper handle_mbox_select() - * uses, so an idle-refresh is as fresh as a fresh SELECT -- including - * mail an external MTA has just delivered into new/, which refresh_index() - * indexes on the way past. + * SS6.3.13 names flag changes among what IDLE exists to report, and + * requires an unsolicited FETCH to carry a UID item (SS7.5.2 repeats it). + * Messages that arrived since the last refresh are reported by EXISTS + * alone: their flags are news to nobody, and a client that wants them + * asks. * - * The poll is what makes IDLE push at all. It replaced - * session_notify_idle_peers(), which asked OTHER sessions in this - * process's "sessions" list to recheck after an EXPUNGE/APPEND/MOVE: under - * the replicated-listener model each listener process owns exactly one - * session, so that loop always skipped its only element and an IDLEing - * session was never told about anything -- not another session's changes - * and not new mail either. A poll covers both, and needs no notification - * path between processes at all. + * Sent once after "+ idling" with seed set, then once per "idle poll" + * interval without it. store.c answers from refresh_index(), so a refresh + * sees what a fresh SELECT would, new mail from an external MTA included. + * Most polls cost two stat(2) calls and no lock; see index.c's + * idle_probe_unchanged(). * - * Most polls cost two stat(2) calls and no lock: see index.c's - * idle_probe_unchanged(), and the "unchanged" flag in struct - * imsg_mbox_idle_refreshed above. + * The store child holds the UID list it last reported and compares in one + * walk, so what crosses the socket is what to print rather than the whole + * mailbox: a change to one message costs one imsg, not one per message. */ -struct imsg_mbox_idle_uid { - uint32_t uid; +struct imsg_mbox_idle_refresh { + /* + * Adopt the current state as the baseline and report nothing. Set + * whenever the listener cannot vouch for what the client has already + * been told: at "+ idling", since the mailbox may have changed while + * the session was not idling. + */ + int seed; }; +struct imsg_mbox_idle_expunge { + uint32_t seqno; /* 1-based, already decremented for + * the EXPUNGEs sent before it + * (RFC 9051 SS7.5.1) */ +}; + struct imsg_mbox_idle_refreshed { int ok; /* * Set when the store child's cheap probe found neither the mailbox - * directory nor new/ touched since the last look, in which case NO - * IMSG_MBOX_IDLE_UID messages preceded this one and every field - * below is left zero and is meaningless. The listener MUST treat - * this as "nothing to do" before it diffs: diffing the resulting - * empty list against the baseline would report every message in the - * mailbox as expunged. + * directory nor new/ touched since the last look, in which case no + * IMSG_MBOX_IDLE_EXPUNGE messages preceded this one and every field + * below is left zero and is meaningless. */ int unchanged; + int exists_changed; /* print exists as "* n EXISTS" */ uint32_t exists; - uint32_t uidvalidity; - uint32_t uidnext; - uint64_t highestmodseq; + int busy; /* lock held elsewhere; ok says if it seeded */ }; /* - * RFC 9051 SS6.3.4/SS6.3.5 (CREATE/DELETE) and SS6.3.9 (LIST), this pass, + * RFC 9051 SS6.3.4/SS6.3.5 (CREATE/DELETE) and SS6.3.9 (LIST), this pass, * flat, non-nested mailboxes as sibling subdirectories of the * session's own per-user maildir root; CREATE/DELETE/RENAME all reply with * the existing struct imsg_mbox_result, only "ok" meaningful). listener.c @@ -1339,15 +1209,42 @@ struct imsg_mbox_rename { }; /* - * RFC 9051 SS6.3.9 (LIST). One IMSG_MBOX_LIST_ITEM per on-disk mailbox - * subdirectory (INBOX excluded), unordered; listener.c does its own - * wildcard matching. Terminal reply reuses imsg_mbox_result ("ok" = 0 - * only on a real I/O error, not on finding zero mailboxes). + * RFC 9051 SS6.3.9 (LIST). One IMSG_MBOX_LIST_ITEM per mailbox, unordered, + * INBOX excluded; listener.c does its own wildcard matching. Terminal reply + * reuses imsg_mbox_result ("ok" = 0 only on a real I/O error, not on finding + * zero mailboxes). + * + * subscribed_only picks WHICH set of names is streamed: 0 is every mailbox + * on disk, 1 is every subscribed name, which SS6.3.9.1 says may include names + * with no mailbox behind them. The store reads the subscription file only + * when this is 1, so a plain LIST -- what a client sends on every connection + * -- costs exactly what it did before subscriptions existed. */ +struct imsg_mbox_list { + int subscribed_only; +}; + +/* + * One mailbox name for the LIST above. `exists` is 0 only in a + * subscribed_only stream, naming something subscribed with no mailbox on + * disk: SS6.3.8 forbids dropping such a name from the list, and SS6.3.9.6's + * Table 3 has the server report it rather than stay silent. Every item in a + * subscribed_only stream is subscribed by construction, so no field says so. + */ struct imsg_mbox_list_item { char mailbox[MBOX_NAME_MAX]; + int exists; }; +/* + * RFC 9051 SS6.3.7 (SUBSCRIBE) / SS6.3.8 (UNSUBSCRIBE). One struct for + * both, as IMSG_MBOX_COPY and IMSG_MBOX_MOVE already share imsg_mbox_copy. + * Terminal reply is imsg_mbox_result, only "error" meaningful. + */ +struct imsg_mbox_subscribe { + char mailbox[MBOX_NAME_MAX]; +}; + /* main.c */ const char *log_procname(enum openimap_proc_type); blob - 1867c3df066b4fed8fcc23aac6f1b02eb03727eb blob + 52f95b54c764213581617143650d8138ecb2e94b --- src/imsgev.c +++ src/imsgev.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * Copyright (c) 2009 Eric Faurot @@ -35,7 +37,11 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* Shared imsgbuf+event(3) wrapper for parent/listener/auth/store, built on the current imsgbuf_*() API (imsg_get() and friends were removed upstream); imsgbuf_get()'s 1/0/-1 return is handled exactly as before. */ +/* + * Shared imsgbuf+event(3) wrapper for parent/listener/auth/store, built on the + * current imsgbuf_*() API (imsg_get() and friends were removed upstream); + * imsgbuf_get()'s 1/0/-1 return is handled exactly as before. + */ #include @@ -46,7 +52,12 @@ #include "imapd.h" #include "log.h" -/* Sets imapd-wide imsgbuf settings for every channel: imsgbuf_set_maxsize() raises the whole-message limit by IMSG_HEADER_SIZE since its argument is payload-only, and imsgbuf_allow_fdpass() is needed since the parent fd-passes on these channels at spawn time. */ +/* + * Sets imapd-wide imsgbuf settings for every channel: imsgbuf_set_maxsize() + * raises the whole-message limit by IMSG_HEADER_SIZE since its argument is + * payload-only, and imsgbuf_allow_fdpass() is needed since the parent fd-passes + * on these channels at spawn time. + */ void imsgev_ibuf_init(struct imsgbuf *ibuf, int fd) { @@ -57,7 +68,11 @@ imsgev_ibuf_init(struct imsgbuf *ibuf, int fd) imsgbuf_allow_fdpass(ibuf); } -/* Arms EV_WRITE via libutil's imsg_close() callback so every queued message gets it exactly once; the early return skips re-arming once EV_WRITE is already pending mid-batch. */ +/* + * Arms EV_WRITE via libutil's imsg_close() callback so every queued message + * gets it exactly once; the early return skips re-arming once EV_WRITE is + * already pending mid-batch. + */ static void imsgev_on_compose(struct imsgbuf *ibuf, void *arg) { @@ -83,12 +98,17 @@ imsgev_init(struct imsgev *iev, int fd, void (*handler event_set(&iev->ev, fd, iev->events, iev->handler, iev->data); event_add(&iev->ev, NULL); - /* Must follow event_set()/event_add() (callback touches iev->ev) and imsgev_ibuf_init() (imsgbuf_init() memset()s the struct); not done inside imsgev_ibuf_init() itself since roles call it pre-event-loop on fd 3. */ + /* + * Must follow event_set()/event_add() (callback touches iev->ev) and + * imsgev_ibuf_init() (imsgbuf_init() memset()s the struct); not done + * inside imsgev_ibuf_init() itself since roles call it pre-event-loop + * on fd 3. + */ imsgbuf_set_userdata(&iev->ibuf, iev); imsgbuf_set_close_callback(&iev->ibuf, imsgev_on_compose); } -/* like imsgev_init(), but copies an already-init'd *ibuf instead of re-init'ing (would discard buffered bytes) */ +/* like imsgev_init(), but copies an already-init'd *ibuf (no re-init) */ void imsgev_init_from_ibuf(struct imsgev *iev, const struct imsgbuf *ibuf, void (*handler)(int, short, void *), void *data) @@ -108,7 +128,7 @@ imsgev_init_from_ibuf(struct imsgev *iev, const struct imsgbuf_set_close_callback(&iev->ibuf, imsgev_on_compose); } -/* re-arm after imsg_compose(); adds EV_WRITE if output is queued. Call at the end of any compose path. */ +/* re-arm after imsg_compose(); adds EV_WRITE if output is queued */ void imsgev_add(struct imsgev *iev) { @@ -122,14 +142,21 @@ imsgev_add(struct imsgev *iev) event_add(&iev->ev, NULL); } -/* Re-arms EV_READ (which imsgev_init() sets without EV_PERSIST, so it drops after firing) at the end of every dispatch handler, delegating to imsgev_add() -- kept as a separate name for clarity, not different behavior. */ +/* + * Re-arms EV_READ (which imsgev_init() sets without EV_PERSIST, so it drops + * after firing) at the end of every dispatch handler, delegating to + * imsgev_add() -- kept as a separate name for clarity, not different behavior. + */ void imsgev_rearm_read(struct imsgev *iev) { imsgev_add(iev); } -/* blocks for one IMSG_SETUP_PEER, returns its fd-passed fd; imsgbuf_get() checked before imsgbuf_read() to avoid coalesced-message stalls */ +/* + * blocks for one IMSG_SETUP_PEER, returns its fd-passed fd; imsgbuf_get() + * checked before imsgbuf_read() to avoid coalesced-message stalls + */ int setup_recv_one_peer(struct imsgbuf *ibuf3) { @@ -159,7 +186,10 @@ setup_recv_one_peer(struct imsgbuf *ibuf3) return (fd); } -/* blocks for IMSG_SETUP_DONE, then sends one back as an ack (see setup_recv_one_peer() re: imsgbuf_get() ordering) */ +/* + * blocks for IMSG_SETUP_DONE, then sends one back as an ack (see + * setup_recv_one_peer() re: imsgbuf_get() ordering) + */ void setup_recv_done_and_ack(struct imsgbuf *ibuf3) { @@ -174,7 +204,8 @@ setup_recv_done_and_ack(struct imsgbuf *ibuf3) if ((n = imsgbuf_read(ibuf3)) == -1) fatal("imsgbuf_read"); if (n == 0) - fatalx("setup_recv_done_and_ack: parent closed channel"); + fatalx("setup_recv_done_and_ack: parent closed " + "channel"); } if (imsg_get_type(&imsg) != IMSG_SETUP_DONE) blob - 9a77c2b79924002c66a151ee72482f4bebf69da2 blob + 294bf25f0d4dfad1fb3c5ff997f48046af9e8abe --- src/index.c +++ src/index.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -14,7 +16,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* index.c, the maildir index file format: load/save/append, QRESYNC resync, and vanished-UID tracking. */ +/* index.c: maildir index format -- load/save/append, QRESYNC, vanished-UID. */ #include #include @@ -36,8 +38,16 @@ #include "log.h" #include "store_internal.h" -/* Index lines are colon-delimited text, so no field may contain ':', CR, or LF -- centralized here since keywords-field callers bypass index_append() and hand-build lines. */ -/* RFC 7162 SS7 bounds a mod-sequence to a positive 63-bit integer; values read back from the index are bounded the same way the wire-facing parsers already are. */ +/* + * Index lines are colon-delimited text, so no field may contain ':', CR, or LF + * -- centralized here since keywords-field callers bypass index_append() and + * hand-build lines. + */ +/* + * RFC 7162 SS7 bounds a mod-sequence to a positive 63-bit integer; values read + * back from the index are bounded the same way the wire-facing parsers already + * are. + */ #define INDEX_MODSEQ_MAX INT64_MAX int @@ -46,13 +56,20 @@ index_field_valid(const char *field) return (field != NULL && strpbrk(field, ":\r\n") == NULL); } -/* A basename read from the index is pasted into paths for open(2)/stat(2)/rename(2); unveil(2) only stops it leaving the maildir, so load-time enforces the same format rules as the write side. */ +/* + * A basename read from the index is pasted into paths for + * open(2)/stat(2)/rename(2); unveil(2) only stops it leaving the maildir, so + * load-time enforces the same format rules as the write side. + */ int index_basename_valid(const char *basename) { const unsigned char *p; - /* Strictly stronger than index_field_valid(): whatever is unsafe to write into a line is also unsafe to paste into a path. */ + /* + * Strictly stronger than index_field_valid(): whatever is unsafe to + * write into a line is also unsafe to paste into a path. + */ if (!index_field_valid(basename)) return (0); /* excludes "", ".", "..", and dotfiles in one test */ @@ -65,7 +82,12 @@ index_basename_valid(const char *basename) return (1); } -/* Grows idx->lines by doubling (from 16) when it is full; index_load() and index_append() carried byte-identical copies of this block, so the growth policy and its failure log now live in one place. Returns 0 when there is room for one more line, -1 on allocation failure (already logged). */ +/* + * Grows idx->lines by doubling (from 16) when it is full; index_load() and + * index_append() carried byte-identical copies of this block, so the growth + * policy and its failure log now live in one place. Returns 0 when there is + * room for one more line, -1 on allocation failure (already logged). + */ static int index_lines_grow(struct mbox_index *idx) { @@ -111,7 +133,11 @@ index_load(int fd, struct mbox_index *idx) } while (fgets(line, sizeof(line), fp) != NULL) { - /* fgets(3) silently splits an over-long line; peek at the next byte to distinguish a legal max-length line (next byte is '\n' or EOF) from an actual split record. */ + /* + * fgets(3) silently splits an over-long line; peek at the next + * byte to distinguish a legal max-length line (next byte is + * '\n' or EOF) from an actual split record. + */ if (strchr(line, '\n') == NULL && strlen(line) == sizeof(line) - 1) { int c = fgetc(fp); @@ -128,7 +154,8 @@ index_load(int fd, struct mbox_index *idx) continue; if (first) { - char *colon, *colon2, *ep; + char *colon, *colon2, *ep; + unsigned long parsed; first = 0; if ((colon = strchr(line, ':')) == NULL) { @@ -137,21 +164,37 @@ index_load(int fd, struct mbox_index *idx) goto fail; } *colon = '\0'; - /* Each header field is digits-or-nothing: strtoul(3)/strtoull(3) accept leading whitespace and a sign, so unguarded input like "-1:1:1" would silently parse into a bogus value; same guard used elsewhere. */ + /* + * Each header field is digits-or-nothing: + * strtoul(3)/strtoull(3) accept leading whitespace and + * a sign, so unguarded input like "-1:1:1" would + * silently parse into a bogus value; same guard used + * elsewhere. The two uint32 fields are range-checked + * before narrowing as well: RFC 9051 SS2.3.1.1 makes + * UIDVALIDITY and UIDNEXT non-zero 32-bit values, and + * "4294967296" would otherwise truncate to 0 and + * "4294967297" to 1, the second handing out UIDs that + * are already in use. + */ if (line[0] < '0' || line[0] > '9') { log_warnx("session %u: malformed " "UIDVALIDITY: %s", session_id, line); goto fail; } errno = 0; - idx->uidvalidity = (uint32_t)strtoul(line, &ep, 10); - if (*ep != '\0' || errno != 0) { + parsed = strtoul(line, &ep, 10); + if (*ep != '\0' || errno != 0 || + parsed > UINT32_MAX) { log_warnx("session %u: malformed " "UIDVALIDITY: %s", session_id, line); goto fail; } + idx->uidvalidity = (uint32_t)parsed; - /* RFC 7162: optional third field HIGHESTMODSEQ; NULL means older two-field header, defaults to 1 */ + /* + * RFC 7162: optional third field HIGHESTMODSEQ; NULL + * means older two-field header, defaults to 1 + */ if ((colon2 = strchr(colon + 1, ':')) != NULL) *colon2 = '\0'; @@ -161,12 +204,14 @@ index_load(int fd, struct mbox_index *idx) goto fail; } errno = 0; - idx->uidnext = (uint32_t)strtoul(colon + 1, &ep, 10); - if (*ep != '\0' || errno != 0) { + parsed = strtoul(colon + 1, &ep, 10); + if (*ep != '\0' || errno != 0 || + parsed > UINT32_MAX) { log_warnx("session %u: malformed UIDNEXT: %s", session_id, colon + 1); goto fail; } + idx->uidnext = (uint32_t)parsed; if (colon2 != NULL) { if (colon2[1] < '0' || colon2[1] > '9') { @@ -214,34 +259,49 @@ index_load(int fd, struct mbox_index *idx) return (0); fail: - /* idx may hold partially-allocated lines here; index_free() is a safe no-op, making "-1 means idx is already freed" true for every caller including refresh_index(). */ + /* + * idx may hold partially-allocated lines here; index_free() is a safe + * no-op, making "-1 means idx is already freed" true for every caller + * including refresh_index(). + */ index_free(idx); fclose(fp); return (-1); } -/* Parses one "UID:basename:keywords[:MODSEQ]" index line; returns -1 (logged) on a corrupt line, caller skips it. */ +/* Parses "UID:basename:keywords[:MODSEQ]"; -1 (logged) on corrupt line. */ int index_parse_line(const char *line, struct index_rec *rec) { const char *p, *q, *r; char *ep; + unsigned long parsed; memset(rec, 0, sizeof(*rec)); - /* strtoul(3) accepts leading whitespace and a sign, so an unguarded UID field could parse ":x:y:1" as 0 or "-1:x:y:1" as 4294967295; the field must be digits-or-nothing. */ + /* + * strtoul(3) accepts leading whitespace and a sign, so an unguarded UID + * field could parse ":x:y:1" as 0 or "-1:x:y:1" as 4294967295; the + * field must be digits-or-nothing. + */ if (line[0] < '0' || line[0] > '9') { log_warnx("session %u: corrupt index line (UID field is not " "a decimal number)", session_id); return (-1); } + /* + * Narrowed only after the range test, the same four-part form the + * UIDVALIDITY floor read uses: a value strtoul(3) accepts but a + * uint32_t cannot hold was truncating, so "4294967296" became UID 0. + */ errno = 0; - rec->uid = (uint32_t)strtoul(line, &ep, 10); - if (*ep != ':' || errno != 0) { + parsed = strtoul(line, &ep, 10); + if (*ep != ':' || errno != 0 || parsed > UINT32_MAX) { log_warnx("session %u: corrupt index line: %s", session_id, line); return (-1); } + rec->uid = (uint32_t)parsed; p = ep + 1; if ((q = strchr(p, ':')) == NULL) { log_warnx("session %u: corrupt index line: %s", session_id, @@ -255,7 +315,12 @@ index_parse_line(const char *line, struct index_rec *r } memcpy(rec->basename, p, (size_t)(q - p)); rec->basename[q - p] = '\0'; - /* Refuses traversal, hidden names, and control bytes before this basename is pasted into open(2)/stat(2)/rename(2) paths, since unveil(2) only stops paths leaving the maildir; logged by UID, never by the untrusted basename itself. */ + /* + * Refuses traversal, hidden names, and control bytes before this + * basename is pasted into open(2)/stat(2)/rename(2) paths, since + * unveil(2) only stops paths leaving the maildir; logged by UID, never + * by the untrusted basename itself. + */ if (!index_basename_valid(rec->basename)) { log_warnx("session %u: refusing index line with unsafe " "basename (UID %u)", session_id, rec->uid); @@ -273,7 +338,13 @@ index_parse_line(const char *line, struct index_rec *r memcpy(rec->keywords, p, (size_t)(r - p)); rec->keywords[r - p] = '\0'; - /* Same digit-or-nothing guard as the UID field, now applied to MODSEQ: RFC 7162 SS7 bounds it at 9,223,372,036,854,775,807, but strtoull(3)'s sign handling would otherwise turn "-1" into 18446744073709551615 and leak into CHANGEDSINCE/UNCHANGEDSINCE and client-visible MODSEQ. */ + /* + * Same digit-or-nothing guard as the UID field, now applied to + * MODSEQ: RFC 7162 SS7 bounds it at 9,223,372,036,854,775,807, + * but strtoull(3)'s sign handling would otherwise turn "-1" + * into 18446744073709551615 and leak into + * CHANGEDSINCE/UNCHANGEDSINCE and client-visible MODSEQ. + */ if (r[1] < '0' || r[1] > '9') { log_warnx("session %u: malformed per-message MODSEQ " "in index line: %s", session_id, line); @@ -288,7 +359,10 @@ index_parse_line(const char *line, struct index_rec *r return (-1); } } else { - /* no MODSEQ field, pre-CONDSTORE line (struct index_rec backward-compatibility) */ + /* + * no MODSEQ field: pre-CONDSTORE line (index_rec + * backward-compat) + */ if (strlcpy(rec->keywords, p, sizeof(rec->keywords)) >= sizeof(rec->keywords)) { log_warnx("session %u: keywords too long in index " @@ -301,26 +375,38 @@ index_parse_line(const char *line, struct index_rec *r return (0); } -/* Highest UID of a *present* message (idx->lines is UID-ascending), 0 if none; this is "*" for SEARCH/FETCH/STORE/EXPUNGE, not uidnext-1. */ +/* + * Highest UID of a *present* message (idx->lines is UID-ascending), 0 if none; + * this is "*" for SEARCH/FETCH/STORE/EXPUNGE, not uidnext-1. + */ uint32_t index_max_uid(struct mbox_index *idx) { - const char *line; - char *ep; - uint32_t v; + struct index_rec rec; if (idx->nlines == 0) return (0); - line = idx->lines[idx->nlines - 1]; - errno = 0; - v = (uint32_t)strtoul(line, &ep, 10); - if (*ep != ':') - return (0); /* corrupt last line, treat as "no UIDs in use" rather than guessing */ - return (v); + /* + * One parser for the UID field, not two: the hand-rolled strtoul(3) + * that used to live here had neither guard and read "-1:name::1" as + * 4294967295. A line index_parse_line() rejects is skipped as a + * message by every walker of idx->lines, so it has no present UID + * for this to return; 0 means "no UIDs in use", as before. + */ + if (index_parse_line(idx->lines[idx->nlines - 1], &rec) == -1) + return (0); + return (rec.uid); } -/* Resolves "*" entries in a parsed sequence-set against max (index_max_uid() for UID requests, idx->nlines for sequence-number requests); swaps any backwards "*"-involving range per RFC 9051 SS9 (since parse_one_seq_range() can't), then clamps lo up to 1 and, when clamp_hi is set, hi down to max, keeping every range including degenerate ones that seqset_contains() correctly treats as unmatchable. */ +/* + * Resolves "*" entries in a parsed sequence-set against max (index_max_uid() + * for UID requests, idx->nlines for sequence-number requests); swaps any + * backwards "*"-involving range per RFC 9051 SS9 (since parse_one_seq_range() + * can't), then clamps lo up to 1 and, when clamp_hi is set, hi down to max, + * keeping every range including degenerate ones that seqset_contains() + * correctly treats as unmatchable. + */ uint32_t seqset_resolve(const struct seq_range *ranges, uint32_t nranges, uint32_t max, int clamp_hi, struct seq_range resolved[SEQSET_MAX_RANGES]) @@ -348,7 +434,7 @@ seqset_resolve(const struct seq_range *ranges, uint32_ return (n); } -/* True if val falls in any of the nresolved [lo, hi] pairs from seqset_resolve() above. */ +/* True if val is in any nresolved [lo, hi] pair from seqset_resolve() above. */ int seqset_contains(const struct seq_range *resolved, uint32_t nresolved, uint32_t val) @@ -362,7 +448,11 @@ seqset_contains(const struct seq_range *resolved, uint return (0); } -/* Highest hi across all resolved ranges (0 if none), letting an ascending scan of idx->lines break early once past it, same as a single-range scan already did. */ +/* + * Highest hi across all resolved ranges (0 if none), letting an ascending scan + * of idx->lines break early once past it, same as a single-range scan already + * did. + */ uint32_t seqset_max_hi(const struct seq_range *resolved, uint32_t nresolved) { @@ -375,7 +465,16 @@ seqset_max_hi(const struct seq_range *resolved, uint32 return (max); } -/* The "does this command apply to this message?" rule for FETCH/STORE/COPY, in one place: RFC 9051 SS6.4.9 makes a UID command's sequence-set UID-space and a bare one position-space, so the caller passes both and by_uid picks. PAST_END is a stop signal, valid only because those three walk idx->lines in ascending order -- the compaction loops in move_same_mailbox()/handle_mbox_expunge() deliberately don't use this, since breaking early would leave the surviving lines they still have to copy down unwritten. */ +/* + * The "does this command apply to this message?" rule for FETCH/STORE/COPY, in + * one place: RFC 9051 SS6.4.9 makes a UID command's sequence-set UID-space and + * a bare one position-space, so the caller passes both and by_uid picks. + * PAST_END is a stop signal, valid only because those three walk idx->lines in + * ascending order -- the compaction loops in + * move_same_mailbox()/handle_mbox_expunge() deliberately don't use this, since + * breaking early would leave the surviving lines they still have to copy down + * unwritten. + */ enum seqset_pos seqset_position(const struct seq_range *resolved, uint32_t nresolved, uint32_t max_hi, int by_uid, uint32_t uid, uint32_t seqno) @@ -389,7 +488,7 @@ seqset_position(const struct seq_range *resolved, uint return (SEQSET_MATCH); } -/* Reports every UID in [lo, hi] absent from idx as IMSG_MBOX_SELECT_VANISHED ranges; RFC 7162 SS3.2.6 VANISHED modifier. */ +/* Reports UID in [lo, hi] absent from idx as VANISHED; RFC 7162 SS3.2.6. */ void send_vanished_range(const struct mbox_index *idx, uint32_t lo, uint32_t hi, struct imsgev *iev) @@ -422,7 +521,11 @@ send_vanished_range(const struct mbox_index *idx, uint "IMSG_MBOX_SELECT_VANISHED", session_id); } - /* Stop here: a UID of UINT32_MAX would wrap to 0 and make the tail check trivially true, emitting a VANISHED range that wrongly claims every message in the mailbox is gone. */ + /* + * Stop here: a UID of UINT32_MAX would wrap to 0 and make the + * tail check trivially true, emitting a VANISHED range that + * wrongly claims every message in the mailbox is gone. + */ if (rec.uid == UINT32_MAX) return; want = rec.uid + 1; @@ -441,7 +544,7 @@ send_vanished_range(const struct mbox_index *idx, uint } } -/* Linear scan for a basename already in the index; O(n) per lookup, fine for modest-mailbox-size scope. */ +/* Linear scan for a basename in the index; O(n), fine at modest size. */ int index_has_basename(struct mbox_index *idx, const char *basename) { @@ -465,21 +568,31 @@ index_has_basename(struct mbox_index *idx, const char return (0); } -/* Appends one "UID:basename::MODSEQ" record; caller owns idx->uidnext. RFC 7162 SS3.1: each append gets its own bumped modseq. */ +/* Appends "UID:basename::MODSEQ"; caller owns uidnext, bumps modseq (SS3.1). */ int index_append(struct mbox_index *idx, uint32_t uid, const char *basename) { char line[STORE_INDEX_LINE_MAX]; int len; - /* RFC 9051 SS9 forbids UID 0; with no ceiling on uidnext, exhaustion would wrap it to 0 and silently reuse in-use UIDs (forbidden by SS2.3.1.1, and breaking index_max_uid()'s ascending assumption), so refuse here instead -- the RFC's real fix, changing UIDVALIDITY, needs persistent state not yet kept (see index.c review's finding #1). */ + /* + * RFC 9051 SS9 forbids UID 0; with no ceiling on uidnext, exhaustion + * would wrap it to 0 and silently reuse in-use UIDs (forbidden by + * SS2.3.1.1, and breaking index_max_uid()'s ascending assumption), so + * refuse here instead -- the RFC's real fix, changing UIDVALIDITY, + * needs persistent state not yet kept (see index.c review's finding + * #1). + */ if (uid == 0) { log_warnx("session %u: refusing index entry with UID 0 " "(uidnext exhausted or index header corrupt)", session_id); return (-1); } - /* defense in depth: refuse a basename containing ':' or newline (refresh_index() already pre-skips these) */ + /* + * defense in depth: refuse a basename containing ':' or newline + * (refresh_index() already pre-skips these) + */ if (strpbrk(basename, ":\r\n") != NULL) { log_warnx("session %u: refusing index entry with unsafe " "basename: %s", session_id, basename); @@ -506,22 +619,29 @@ index_append(struct mbox_index *idx, uint32_t uid, con return (0); } -/* Rewrites index to STORE_INDEX_TMP_NAME, rename(2)s over STORE_INDEX_NAME so a reader never sees a torn file. */ +/* + * Rewrites index to STORE_INDEX_TMP_NAME, rename(2)s over STORE_INDEX_NAME so a + * reader never sees a torn file. + */ int -index_save(const struct mbox_index *idx) +index_save(int dfd, const struct mbox_index *idx) { FILE *fp; int fd; size_t i; - /* O_EXCL so a pre-planted symlink can't be followed; unlink any stale temp from a prior crash first */ - if (unlink(STORE_INDEX_TMP_NAME) == -1 && errno != ENOENT) { + /* + * O_EXCL so a pre-planted symlink can't be followed; unlink any stale + * temp from a prior crash first + */ + if (unlinkat(dfd, STORE_INDEX_TMP_NAME, 0) == -1 && + errno != ENOENT) { log_warn("session %u: unlink %s", session_id, STORE_INDEX_TMP_NAME); return (-1); } - if ((fd = open(STORE_INDEX_TMP_NAME, O_WRONLY | O_CREAT | O_EXCL, - 0600)) == -1) { + if ((fd = openat(dfd, STORE_INDEX_TMP_NAME, + O_WRONLY | O_CREAT | O_EXCL, 0600)) == -1) { log_warn("session %u: open %s", session_id, STORE_INDEX_TMP_NAME); return (-1); @@ -566,26 +686,17 @@ index_save(const struct mbox_index *idx) return (-1); } - if (rename(STORE_INDEX_TMP_NAME, STORE_INDEX_NAME) == -1) { + if (renameat(dfd, STORE_INDEX_TMP_NAME, dfd, + STORE_INDEX_NAME) == -1) { log_warn("session %u: rename %s -> %s", session_id, STORE_INDEX_TMP_NAME, STORE_INDEX_NAME); return (-1); } /* and the directory entry the rename(2) just repointed */ - { - int dfd; - - if ((dfd = open(".", O_RDONLY | O_DIRECTORY)) == -1) - log_warn("session %u: open . for fsync (continuing)", - session_id); - else { - if (fsync(dfd) == -1) - log_warn("session %u: fsync . (continuing)", - session_id); - close(dfd); - } - } + if (fsync(dfd) == -1) + log_warn("session %u: fsync mailbox directory " + "(continuing)", session_id); return (0); } @@ -601,7 +712,10 @@ index_free(struct mbox_index *idx) memset(idx, 0, sizeof(*idx)); } -/* RFC 7162 SS3.2.5.1 QRESYNC resync: streams VANISHED ranges then FETCH_META for messages with modseq > qresync_modseq. */ +/* + * RFC 7162 SS3.2.5.1 QRESYNC resync: streams VANISHED ranges then FETCH_META + * for messages with modseq > qresync_modseq. + */ void qresync_send_resync(const struct imsg_mbox_select *req, const struct seq_range *ranges, uint32_t nranges, @@ -611,11 +725,20 @@ qresync_send_resync(const struct imsg_mbox_select *req uint32_t nresolved, max_hi, i; if (req->qresync_has_uids) { - /* known-uids is a full RFC 9051 SS9 sequence-set resolved the same way as any UID-space consumer; max is unused since "*" is already rejected upstream, and clamp_hi is 0 because a known UID above the current highest is exactly what RFC 7162 SS3.2.5.1 wants reported VANISHED, not dropped. */ + /* + * known-uids is a full RFC 9051 SS9 sequence-set resolved the + * same way as any UID-space consumer; max is unused since "*" + * is already rejected upstream, and clamp_hi is 0 because a + * known UID above the current highest is exactly what RFC 7162 + * SS3.2.5.1 wants reported VANISHED, not dropped. + */ nresolved = seqset_resolve(ranges, nranges, index_max_uid(idx), 0, resolved); } else { - /* SS3.2.5.1: no known-uids list acts as "1:", or empty if uidnext == 1 (never assigned) */ + /* + * SS3.2.5.1: no known-uids means "1:", empty if + * uidnext == 1 + */ if (idx->uidnext <= 1) return; resolved[0].lo = 1; @@ -624,7 +747,12 @@ qresync_send_resync(const struct imsg_mbox_select *req nresolved = 1; } - /* RFC 7162 SS3.2.6 requires VANISHED (EARLIER) precede FETCH; ordering is guaranteed by store_ipc.c's session_handle_mbox_selected(), which buffers and flushes VANISHED before FETCH, the same two-pass split and helper handle_mbox_fetch() uses. */ + /* + * RFC 7162 SS3.2.6 requires VANISHED (EARLIER) precede FETCH; ordering + * is guaranteed by store_ipc.c's session_handle_mbox_selected(), which + * buffers and flushes VANISHED before FETCH, the same two-pass split + * and helper handle_mbox_fetch() uses. + */ for (i = 0; i < nresolved; i++) send_vanished_range(idx, resolved[i].lo, resolved[i].hi, iev); @@ -633,7 +761,10 @@ qresync_send_resync(const struct imsg_mbox_select *req struct index_rec rec; if (index_parse_line(idx->lines[i], &rec) == -1) - continue; /* corrupt line, already logged, not reported either way */ + /* + * corrupt line, already logged, not reported either way + */ + continue; if (rec.uid > max_hi) break; if (!seqset_contains(resolved, nresolved, rec.uid)) @@ -648,8 +779,8 @@ qresync_send_resync(const struct imsg_mbox_select *req meta.seqno = i + 1; meta.uid = rec.uid; meta.modseq = rec.modseq; - if (locate_message_file(rec.basename, &size, suffix, - sizeof(suffix)) == 0) { + if (locate_message_file(mailbox_dir_fd, rec.basename, + &size, suffix, sizeof(suffix)) == 0) { build_flags_string(suffix, rec.keywords, meta.flags, sizeof(meta.flags)); } @@ -662,27 +793,43 @@ qresync_send_resync(const struct imsg_mbox_select *req } } -/* Takes the index lock (LOCK_EX/LOCK_SH) on STORE_INDEX_LOCK_NAME's stable inode before opening the index, so the descriptor can't refer to an inode a concurrent index_save() already renamed away; release with the idempotent index_lock_release(). */ +/* + * Takes the index lock (LOCK_EX/LOCK_SH) on STORE_INDEX_LOCK_NAME's stable + * inode before opening the index, so the descriptor can't refer to an inode a + * concurrent index_save() already renamed away; release with the idempotent + * index_lock_release(). + * + * Returns 0 holding the lock, or -1 having taken nothing. A caller that + * added LOCK_NB can also get 1, meaning another process holds it: an + * ordinary answer rather than a failure, so it is not logged, and every + * blocking caller is unaffected because flock(2) cannot report EWOULDBLOCK + * without LOCK_NB (flock(2), sys/kern/kern_descrip.c). + */ int -index_lock_acquire(struct index_lock *il, int op) +index_lock_acquire(int dfd, struct index_lock *il, int op) { + int busy; + il->lockfd = -1; il->fd = -1; - if ((il->lockfd = open(STORE_INDEX_LOCK_NAME, O_RDWR | O_CREAT, - 0600)) == -1) { + if ((il->lockfd = openat(dfd, STORE_INDEX_LOCK_NAME, + O_RDWR | O_CREAT, 0600)) == -1) { log_warn("session %u: open %s", session_id, STORE_INDEX_LOCK_NAME); return (-1); } if (flock(il->lockfd, op) == -1) { - log_warn("session %u: flock %s", session_id, - STORE_INDEX_LOCK_NAME); + busy = (op & LOCK_NB) && errno == EWOULDBLOCK; + if (!busy) + log_warn("session %u: flock %s", session_id, + STORE_INDEX_LOCK_NAME); close(il->lockfd); il->lockfd = -1; - return (-1); + return (busy ? 1 : -1); } - if ((il->fd = open(STORE_INDEX_NAME, O_RDWR | O_CREAT, 0600)) == -1) { + if ((il->fd = openat(dfd, STORE_INDEX_NAME, O_RDWR | O_CREAT, + 0600)) == -1) { log_warn("session %u: open %s", session_id, STORE_INDEX_NAME); flock(il->lockfd, LOCK_UN); close(il->lockfd); @@ -692,7 +839,10 @@ index_lock_acquire(struct index_lock *il, int op) return (0); } -/* Drops whatever index_lock_acquire() took; safe to call twice, and safe on an INDEX_LOCK_INIT struct that was never acquired. */ +/* + * Drops whatever index_lock_acquire() took; safe to call twice, and safe on an + * INDEX_LOCK_INIT struct that was never acquired. + */ void index_lock_release(struct index_lock *il) { @@ -700,7 +850,7 @@ index_lock_release(struct index_lock *il) close(il->fd); il->fd = -1; } - /* lock last, so no other process can take it while our index fd is open */ + /* lock last: no other process can take it while our index fd is open */ if (il->lockfd != -1) { flock(il->lockfd, LOCK_UN); close(il->lockfd); @@ -708,7 +858,15 @@ index_lock_release(struct index_lock *il) } } -/* Issues and records a new UIDVALIDITY: RFC 9051 SS2.3.1.1 requires it strictly increase, so the timestamp is only a floor -- the value returned is max(clock, last-issued+1), the high-water mark is persisted at the maildir root (STORE_UIDVALIDITY_NAME, since per-mailbox state is gone after DELETE), and every failure degrades to a bare timestamp except an unparseable-but-present file, which is left untouched rather than overwritten with a lower floor. */ +/* + * Issues and records a new UIDVALIDITY: RFC 9051 SS2.3.1.1 requires it strictly + * increase, so the timestamp is only a floor -- the value returned is + * max(clock, last-issued+1), the high-water mark is persisted at the maildir + * root (STORE_UIDVALIDITY_NAME, since per-mailbox state is gone after DELETE), + * and every failure degrades to a bare timestamp except an + * unparseable-but-present file, which is left untouched rather than overwritten + * with a lower floor. + */ uint32_t uidvalidity_next(void) { @@ -722,14 +880,14 @@ uidvalidity_next(void) ssize_t n; int fd, writeback = 1; - /* The namespace is flat -- select_mailbox_dir() reaches a mailbox with a single chdir("..") -- so the root is either the cwd (INBOX) or exactly one level up. */ - path = current_mailbox_dir[0] == '\0' ? - STORE_UIDVALIDITY_NAME : "../" STORE_UIDVALIDITY_NAME; + /* one file per account, at the maildir root */ + path = STORE_UIDVALIDITY_NAME; now = time(NULL); val = (now > 0 && (uintmax_t)now <= UINT32_MAX) ? (uint32_t)now : 0; - if ((fd = open(path, O_RDWR | O_CREAT, 0600)) == -1) { + if ((fd = openat(maildir_root_fd, path, O_RDWR | O_CREAT, + 0600)) == -1) { log_warn("session %u: open %s (UIDVALIDITY floor); falling " "back to a bare timestamp", session_id, path); return (val != 0 ? val : 1); @@ -741,7 +899,11 @@ uidvalidity_next(void) return (val != 0 ? val : 1); } - /* A zero-length file is the ordinary just-created case (floor 0 is correct); anything present but unreadable is damage and is left alone. */ + /* + * A zero-length file is the ordinary just-created case (floor 0 is + * correct); anything present but unreadable is damage and is left + * alone. + */ if (fstat(fd, &st) == 0 && st.st_size > 0) { if ((n = read(fd, buf, sizeof(buf) - 1)) <= 0) { log_warn("session %u: read %s (UIDVALIDITY floor)", @@ -750,7 +912,11 @@ uidvalidity_next(void) } else { buf[n] = '\0'; buf[strcspn(buf, "\r\n")] = '\0'; - /* same digit guard as the index header: strtoul(3) accepts a leading sign, so "-1" would read as 4294967295 and pin the floor at its ceiling */ + /* + * same digit guard as the index header: strtoul(3) + * accepts a leading sign, so "-1" would read as + * 4294967295 and pin the floor at its ceiling + */ errno = 0; parsed = strtoul(buf, &ep, 10); if (buf[0] < '0' || buf[0] > '9' || *ep != '\0' || @@ -768,7 +934,10 @@ uidvalidity_next(void) if (writeback) { if (val <= floor) { if (floor == UINT32_MAX) { - /* 4 billion issued values, or a clock past 2106: nothing greater is representable. */ + /* + * 4 billion issued, or clock past 2106: nothing + * greater representable. + */ log_warnx("session %u: UIDVALIDITY floor is " "exhausted (%u); reusing it", session_id, floor); @@ -791,7 +960,11 @@ uidvalidity_next(void) "may be issued again", session_id, path); } - /* A successful floor consultation is otherwise silent, and a quietly-wrong UIDVALIDITY looks identical to a correct one to the client; -v logs what was read and what was issued. */ + /* + * A successful floor consultation is otherwise silent, and a + * quietly-wrong UIDVALIDITY looks identical to a correct one to the + * client; -v logs what was read and what was issued. + */ log_debug("session %u: UIDVALIDITY: floor %u in %s -> issued %u%s", session_id, floor, path, val, writeback ? "" : " (floor NOT updated)"); @@ -800,18 +973,33 @@ uidvalidity_next(void) return (val); } -/* Walks new/ for undiscovered maildir deliveries: mutate==0 only answers "is there at least one?" without touching idx or the filesystem (safe under a shared lock, used by the frequent IDLE poll); mutate==1 indexes everything found and needs the exclusive lock. Returns 1 (found/added), 0, or -1 on error -- on error with mutate set, idx is already index_free()'d, per refresh_index()'s contract. */ +/* + * Walks new/ for undiscovered maildir deliveries: mutate==0 only answers "is + * there at least one?" without touching idx or the filesystem (safe under a + * shared lock, used by the frequent IDLE poll); mutate==1 indexes everything + * found and needs the exclusive lock. Returns 1 (found/added), 0, or -1 on + * error -- on error with mutate set, idx is already index_free()'d, per + * refresh_index()'s contract. + */ static int -index_scan_new(struct mbox_index *idx, int mutate) +index_scan_new(int dfd, struct mbox_index *idx, int mutate) { DIR *dp; struct dirent *de; int added = 0; - dp = opendir("new"); + { + int newfd; + + dp = NULL; + newfd = openat(dfd, "new", O_RDONLY | O_DIRECTORY); + if (newfd != -1 && (dp = fdopendir(newfd)) == NULL) + close(newfd); + } if (dp == NULL) { if (errno == ENOENT) - return (0); /* no new/ yet on a never-used mailbox, not an error */ + /* no new/ yet on a never-used mailbox, not an error */ + return (0); log_warn("session %u: opendir new", session_id); if (mutate) index_free(idx); @@ -819,10 +1007,22 @@ index_scan_new(struct mbox_index *idx, int mutate) } while ((de = readdir(dp)) != NULL) { if (de->d_name[0] == '.') - continue; /* ".", "..", and dotfiles, maildir delivery never creates the latter */ - /* never index a filename with ':' or newline, would corrupt the index line format */ + /* + * ".", "..", and dotfiles -- maildir delivery never + * creates the latter + */ + continue; + /* + * never index a filename with ':' or newline, corrupts index + * line format + */ if (strpbrk(de->d_name, ":\r\n") != NULL) { - /* Logged only on the mutating pass -- the read-only pass runs every poll interval for an IDLE's whole life, and a badly-named file would otherwise fill the log forever. */ + /* + * Logged only on the mutating pass -- the read-only + * pass runs every poll interval for an IDLE's whole + * life, and a badly-named file would otherwise fill the + * log forever. + */ if (mutate) log_warnx("session %u: skipping new/ file " "with unsafe name (contains ':' or " @@ -833,7 +1033,8 @@ index_scan_new(struct mbox_index *idx, int mutate) continue; if (!mutate) { closedir(dp); - return (1); /* one is enough to answer the question */ + /* one is enough to answer the question */ + return (1); } if (index_append(idx, idx->uidnext, de->d_name) == -1) { closedir(dp); @@ -847,30 +1048,43 @@ index_scan_new(struct mbox_index *idx, int mutate) return (added); } -/* Loads the index (fd already flock(2)'d LOCK_EX) and indexes any new/ files not yet known; on failure idx is already index_free()'d. */ +/* + * Loads the index (fd already flock(2)'d LOCK_EX) and indexes any new/ files + * not yet known; on failure idx is already index_free()'d. + */ int -refresh_index(struct mbox_index *idx, int fd) +refresh_index(int dfd, struct mbox_index *idx, int fd) { int added; if (index_load(fd, idx) == -1) return (-1); - if ((added = index_scan_new(idx, 1)) == -1) + if ((added = index_scan_new(dfd, idx, 1)) == -1) return (-1); /* index_scan_new() has already freed idx */ - /* Skip index_save()'s cost on a no-op refresh, unless the header itself is new -- a freshly invented UIDVALIDITY that's never written down would just be invented again, differently, next call. */ + /* + * Skip index_save()'s cost on a no-op refresh, unless the header itself + * is new -- a freshly invented UIDVALIDITY that's never written down + * would just be invented again, differently, next call. + */ if (!added && !idx->fresh) return (0); - if (index_save(idx) == -1) { + if (index_save(dfd, idx) == -1) { index_free(idx); return (-1); } return (0); } -/* Cheap change probe: two stat(2) calls (no lock, no read) on "." (moved by every index_save()-based mutation: APPEND/STORE/EXPUNGE/COPY/MOVE) and "new" (touched by an external MTA delivery before anything indexes it); sampled before the caller's work so a change is never missed, at the cost of one harmless extra refresh, modulo theoretical same-nanosecond races. */ +/* + * Cheap change probe: two stat(2) calls (no lock, no read) on "." (moved by + * every index_save()-based mutation: APPEND/STORE/EXPUNGE/COPY/MOVE) and "new" + * (touched by an external MTA delivery before anything indexes it); sampled + * before the caller's work so a change is never missed, at the cost of one + * harmless extra refresh, modulo theoretical same-nanosecond races. + */ static struct { int valid; ino_t dir_ino; @@ -879,13 +1093,124 @@ static struct { struct timespec new_mtim; } idle_probe; -/* Forces the next probe to report a change; call whenever cwd changes mailbox. */ +/* Forces next probe to report a change; call whenever cwd changes mailbox. */ void idle_probe_reset(void) { idle_probe.valid = 0; } +/* + * The UID list this session was last told about, ascending, and what an IDLE + * refresh diffs against. It lives here rather than in the listener so that a + * change to one message costs one imsg instead of one per message in the + * mailbox, and one walk instead of a rescan per message. About 4 bytes per + * message. + */ +static struct { + uint32_t *uids; + size_t n; + uint64_t modseq; /* highest reported; above it is news */ + int valid; +} idle_baseline; + +/* Forces the next refresh to seed rather than diff; pairs with the above. */ +void +idle_baseline_reset(void) +{ + free(idle_baseline.uids); + idle_baseline.uids = NULL; + idle_baseline.n = 0; + idle_baseline.modseq = 0; + idle_baseline.valid = 0; +} + +/* + * One untagged EXPUNGE per UID that went away, in order; returns how many. + * + * RFC 9051 SS7.5.1: each EXPUNGE decrements the sequence numbers above it, so + * seqno counts only messages still present. Both lists are UID-ascending (see + * index_max_uid()), so one walk does it. + */ +static size_t +idle_send_expunges(const uint32_t *old, size_t oldn, const uint32_t *cur, + size_t curn, struct imsgev *iev) +{ + struct imsg_mbox_idle_expunge item; + size_t i, j = 0, gone = 0; + uint32_t seqno = 1; + + for (i = 0; i < oldn; i++) { + while (j < curn && cur[j] < old[i]) + j++; + if (j < curn && cur[j] == old[i]) { + j++; + seqno++; + continue; + } + memset(&item, 0, sizeof(item)); + item.seqno = seqno; + gone++; + if (imsg_compose(&iev->ibuf, IMSG_MBOX_IDLE_EXPUNGE, 0, 0, -1, + &item, sizeof(item)) == -1) + log_warn("session %u: imsg_compose " + "IMSG_MBOX_IDLE_EXPUNGE", session_id); + } + return (gone); +} + +/* + * One untagged FETCH per message whose mod-sequence passed what was last + * reported; returns how many. RFC 9051 SS6.3.13 lists flag changes among + * what IDLE reports and requires an unsolicited FETCH to carry a UID item. + * + * Only messages the client already knows about: one that arrived since the + * last refresh is not in old, and EXISTS is all it gets. Flags live in the + * message file's name, so each one reported costs a lookup, which is why + * this walks the changed messages and not the mailbox. + */ +static size_t +idle_send_flag_fetches(const struct mbox_index *idx, const uint32_t *old, + size_t oldn, uint64_t since, struct imsgev *iev) +{ + struct imsg_mbox_fetch_meta meta; + struct index_rec rec; + char suffix[64]; + off_t size; + size_t i, j = 0, seqno = 0, sent = 0; + + for (i = 0; i < idx->nlines; i++) { + if (index_parse_line(idx->lines[i], &rec) == -1) + continue; /* malformed, skipped as everywhere */ + seqno++; /* position after the EXPUNGEs above */ + if (rec.modseq <= since) + continue; + while (j < oldn && old[j] < rec.uid) + j++; + if (j == oldn || old[j] != rec.uid) + continue; /* arrived since; EXISTS covers it */ + if (locate_message_file(mailbox_dir_fd, rec.basename, &size, + suffix, sizeof(suffix)) == -1) { + log_warnx("session %u: message %s (uid %u) indexed " + "but missing on disk, no IDLE flag push", + session_id, rec.basename, rec.uid); + continue; + } + memset(&meta, 0, sizeof(meta)); + meta.seqno = (uint32_t)seqno; + meta.uid = rec.uid; + meta.modseq = rec.modseq; + build_flags_string(suffix, rec.keywords, meta.flags, + sizeof(meta.flags)); + sent++; + if (imsg_compose(&iev->ibuf, IMSG_MBOX_IDLE_FETCH, 0, 0, -1, + &meta, sizeof(meta)) == -1) + log_warn("session %u: imsg_compose " + "IMSG_MBOX_IDLE_FETCH", session_id); + } + return (sent); +} + static int tspec_eq(const struct timespec *a, const struct timespec *b) { @@ -899,13 +1224,17 @@ idle_probe_unchanged(void) struct stat dst, nst; int same; - if (stat(".", &dst) == -1) { - /* Cannot tell, so do not claim to know: fall through to the full refresh, which will report the failure properly. */ + if (fstatat(mailbox_dir_fd, ".", &dst, 0) == -1) { + /* + * Cannot tell, so do not claim to know: fall through to the + * full refresh, which will report the failure properly. + */ idle_probe.valid = 0; return (0); } - if (stat("new", &nst) == -1) - memset(&nst, 0, sizeof(nst)); /* absent new/ is a stable state */ + if (fstatat(mailbox_dir_fd, "new", &nst, 0) == -1) + /* absent new/ is a stable state */ + memset(&nst, 0, sizeof(nst)); same = idle_probe.valid && dst.st_ino == idle_probe.dir_ino && @@ -922,81 +1251,246 @@ idle_probe_unchanged(void) return (same); } -/* RFC 9051 SS6.3.4-SS6.3.6/SS6.3.9; re-validated here independently of listener.c's client-side check (privsep defense in depth). */ +/* + * The UIDs an IDLE baseline records, in index order, and the mod-sequence + * to diff from next time. Returns -1, having allocated nothing, on failure. + */ +static int +idle_uid_list(const struct mbox_index *idx, uint32_t **listp, size_t *np, + uint64_t *modseqp) +{ + struct index_rec rec; + uint32_t *list; + uint64_t modseq = 0; + size_t i, n = 0; + + /* + * nlines + 1 so that an empty mailbox still asks for a nonzero + * allocation, which keeps a NULL return meaning failure and nothing + * else. + */ + if ((list = reallocarray(NULL, idx->nlines + 1, sizeof(*list))) == + NULL) { + log_warn("session %u: idle refresh: reallocarray", session_id); + return (-1); + } + for (i = 0; i < idx->nlines; i++) { + if (index_parse_line(idx->lines[i], &rec) == -1) + /* skip malformed line, don't fail the whole request */ + continue; + list[n++] = rec.uid; + if (rec.modseq > modseq) + modseq = rec.modseq; + } + + /* + * The header's HIGHESTMODSEQ is what a client is told, but a + * per-message value above it would then never be reported again, so + * take whichever is greater as the mark for next time. A + * pre-CONDSTORE index line parses with modseq defaulted to 1, as + * index_parse_line does, which a header of 0 would otherwise make + * look like a change on every refresh. + */ + if (idx->highestmodseq > modseq) + modseq = idx->highestmodseq; + + *listp = list; + *np = n; + *modseqp = modseq; + return (0); +} + +/* + * Seeds the IDLE baseline from the committed index without its lock, for + * an IDLE that found the lock busy. index_save() only ever replaces the + * index whole, by rename(2), so this reads one committed version, never a + * torn one. Deliveries still in new/ are left for a later refresh to + * index and report. Returns 0 seeded, with the count in reply, or -1. + */ +static int +idle_seed_unlocked(struct imsg_mbox_idle_refreshed *reply) +{ + struct mbox_index idx; + uint32_t *list; + uint64_t modseq; + size_t n; + int fd; + + if ((fd = openat(mailbox_dir_fd, STORE_INDEX_NAME, O_RDONLY)) == -1) { + if (errno != ENOENT) + log_warn("session %u: open %s", session_id, + STORE_INDEX_NAME); + return (-1); + } + if (index_load(fd, &idx) == -1) { + close(fd); + return (-1); + } + close(fd); + if (idle_uid_list(&idx, &list, &n, &modseq) == -1) { + index_free(&idx); + return (-1); + } + index_free(&idx); + + free(idle_baseline.uids); + idle_baseline.uids = list; + idle_baseline.n = n; + idle_baseline.modseq = modseq; + idle_baseline.valid = 1; + reply->exists = (uint32_t)n; + return (0); +} + +/* RFC 9051 SS6.3.4-SS6.3.6/SS6.3.9; re-checked vs listener.c (privsep). */ void -handle_mbox_idle_refresh(struct imsgev *iev) +handle_mbox_idle_refresh(const struct imsg_mbox_idle_refresh *req, + struct imsgev *iev) { struct mbox_index idx; struct imsg_mbox_idle_refreshed reply; - struct imsg_mbox_idle_uid item; struct index_lock il = INDEX_LOCK_INIT; - size_t i; - int pending; - struct index_rec rec; + size_t newn = 0, gone = 0, changed = 0; + int pending = 0, seeded, locked; + uint32_t *newlist; + uint64_t seen_modseq = 0; memset(&reply, 0, sizeof(reply)); + /* a seed adopts what it finds rather than reporting it */ + seeded = req->seed || !idle_baseline.valid; - /* Cheapest question first: on an untouched mailbox this is the whole job, with no lock, no index read, and no per-message IMSG_MBOX_IDLE_UID stream, which matters since the poll runs every few seconds. */ + /* + * Cheapest question first: on an untouched mailbox this is the whole + * job, with no lock and no index read, which matters since the poll + * runs every few seconds. + */ if (idle_probe_unchanged()) { reply.ok = 1; reply.unchanged = 1; - /* The poll mechanism is otherwise silent and a dead one is indistinguishable from a healthy one (as the cross-session push bug showed); at -v these lines make each poll and any real work observable. */ + /* + * The poll mechanism is otherwise silent and a dead one is + * indistinguishable from a healthy one (as the cross-session + * push bug showed); at -v these lines make each poll and any + * real work observable. + */ log_debug("session %u: idle refresh: unchanged (probe: no " "change to . or new/)", session_id); goto send; } - /* Something moved, so the index must be read; take the shared lock since reading alone covers the common case, and escalate only when new/ actually holds a delivery to index. */ - if (index_lock_acquire(&il, LOCK_SH) == -1) + /* + * Something moved, so the index must be read; take the shared lock + * since reading alone covers the common case, and escalate only when + * new/ actually holds a delivery to index. + */ + locked = index_lock_acquire(mailbox_dir_fd, &il, LOCK_SH | LOCK_NB); + if (locked == 1) + goto busy; + if (locked == -1) goto send; if (index_load(il.fd, &idx) == -1) { index_lock_release(&il); goto send; } - if ((pending = index_scan_new(&idx, 0)) == -1) { + if ((pending = index_scan_new(mailbox_dir_fd, &idx, 0)) == -1) { index_free(&idx); index_lock_release(&il); goto send; } - /* idx.fresh joins pending here: index_load() just invented a UIDVALIDITY for a header-less mailbox, and persisting it needs the exclusive lock just as indexing a delivery does. */ + /* + * idx.fresh joins pending here: index_load() just invented a + * UIDVALIDITY for a header-less mailbox, and persisting it needs the + * exclusive lock just as indexing a delivery does. + */ if (pending || idx.fresh) { - /* Deliberately drop the shared lock and redo everything under LOCK_EX rather than upgrading in place -- flock(2) has no atomic upgrade, so another process could slip in between states and invalidate what was read under the shared lock. */ + /* + * Deliberately drop the shared lock and redo everything under + * LOCK_EX rather than upgrading in place -- flock(2) has no + * atomic upgrade, so another process could slip in between + * states and invalidate what was read under the shared lock. + */ index_free(&idx); index_lock_release(&il); - if (index_lock_acquire(&il, LOCK_EX) == -1) + locked = index_lock_acquire(mailbox_dir_fd, &il, + LOCK_EX | LOCK_NB); + if (locked == 1) + goto busy; + if (locked == -1) goto send; - if (refresh_index(&idx, il.fd) == -1) { + if (refresh_index(mailbox_dir_fd, &idx, il.fd) == -1) { index_lock_release(&il); goto send; } } - for (i = 0; i < idx.nlines; i++) { - if (index_parse_line(idx.lines[i], &rec) == -1) - continue; /* skip malformed line, don't fail the whole request */ - memset(&item, 0, sizeof(item)); - item.uid = rec.uid; - if (imsg_compose(&iev->ibuf, IMSG_MBOX_IDLE_UID, 0, 0, -1, - &item, sizeof(item)) == -1) - log_warn("session %u: imsg_compose " - "IMSG_MBOX_IDLE_UID", session_id); + if (idle_uid_list(&idx, &newlist, &newn, &seen_modseq) == -1) { + /* + * reply.ok stays 0, so the listener keeps what it last told + * the client and this poll simply reports nothing; the + * baseline here is untouched for the same reason. + */ + index_free(&idx); + index_lock_release(&il); + goto send; } + if (!seeded) { + gone = idle_send_expunges(idle_baseline.uids, idle_baseline.n, + newlist, newn, iev); + changed = idle_send_flag_fetches(&idx, idle_baseline.uids, + idle_baseline.n, idle_baseline.modseq, iev); + reply.exists_changed = newn != idle_baseline.n; + } + free(idle_baseline.uids); + idle_baseline.uids = newlist; + idle_baseline.n = newn; + idle_baseline.modseq = seen_modseq; + idle_baseline.valid = 1; + reply.ok = 1; - reply.exists = (uint32_t)idx.nlines; - reply.uidvalidity = idx.uidvalidity; - reply.uidnext = idx.uidnext; - reply.highestmodseq = idx.highestmodseq; + reply.exists = (uint32_t)newn; - log_debug("session %u: idle refresh: streamed %zu uid(s), " - "highestmodseq %llu%s", session_id, idx.nlines, - (unsigned long long)idx.highestmodseq, + log_debug("session %u: idle refresh: %zu uid(s), highestmodseq %llu, " + "%zu expunge(s), %zu flag change(s)%s%s", session_id, newn, + (unsigned long long)idx.highestmodseq, gone, changed, + seeded ? " (baseline seeded)" : "", pending ? " (escalated to LOCK_EX, indexed new delivery)" : ""); index_free(&idx); index_lock_release(&il); + goto send; +busy: + /* + * Another session holds the lock. A poll skips, and send: below makes + * the next one look again. A seed cannot skip: the baseline left from + * the last IDLE predates the client's own commands since, and diffing + * against it would resend EXPUNGEs the client has already had (RFC + * 9051 SS7.5.1). So a seed reads the committed index instead, and if + * it cannot, drops the baseline so that the next refresh seeds. + */ + reply.busy = 1; + if (seeded && idle_seed_unlocked(&reply) == 0) { + reply.ok = 1; + /* so the next poll reads the index, not the probe */ + idle_probe_reset(); + } else if (seeded) { + idle_baseline_reset(); + } + log_debug("session %u: idle refresh: index lock busy, %s", session_id, + !seeded ? "skipped this poll" : reply.ok ? "seeded without it" : + "next refresh seeds"); + send: + /* + * A refresh that gives up has already consumed the change: + * idle_probe_unchanged() records the new mtimes whatever its caller + * does next, so without this the next poll reports "unchanged" for + * something the client was never told. + */ + if (!reply.ok) + idle_probe_reset(); + if (imsg_compose(&iev->ibuf, IMSG_MBOX_IDLE_REFRESHED, 0, 0, -1, &reply, sizeof(reply)) == -1) log_warn("session %u: imsg_compose IMSG_MBOX_IDLE_REFRESHED", blob - c359eac84115ad33f85b403204722cfa81cc2edc blob + e97c389fb7ab36d090a992f9435cd362e94a1bd5 --- src/keymgr.c +++ src/keymgr.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * Copyright (c) 2014 Reyk Floeter @@ -11,8 +13,7 @@ * RSA_METHOD/EC_KEY_METHOD engine override that forwards every * private-key operation here as a synchronous imsg round-trip -- is * smtpd's ca.c (Reyk Floeter, Gilles Chehade), ported to imapd's own - * imsg/privsep conventions rather than copied verbatim; see - * docs/openimap-tls-privsep-design.md SS10.1 and SS5.5. The RSA_METHOD/ + * imsg/privsep conventions rather than copied verbatim. The RSA_METHOD/ * EC_KEY_METHOD engine-override code itself (the code this file's * request/reply pair answers) lives in listener.c, not here -- see that * file's header comment for the matching attribution. @@ -41,7 +42,12 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* keymgr.c: holds the real TLS private key for listener.c's fake-key/imsg forwarding (docs/openimap-tls-privsep-design.md SS5); boot-time plumbing failures are fatal, but content failures (bad cert/key) and per-request failures degrade gracefully, keeping the process alive with no usable key rather than crashing. */ +/* + * keymgr.c: holds the real TLS private key for listener.c's fake-key/imsg + * forwarding; boot-time plumbing failures are fatal, but content failures + * (bad cert/key) and per-request failures degrade gracefully, keeping the + * process alive with no usable key rather than crashing. + */ #include #include @@ -66,11 +72,19 @@ #include "imapd.h" #include "log.h" -/* Matches parent.c's read buffer size (8192), kept as a separate local constant rather than a shared imapd.h macro since nothing else needs to agree on the exact value. */ +/* + * Matches parent.c's read buffer size (8192), kept as a separate local constant + * rather than a shared imapd.h macro since nothing else needs to agree on the + * exact value. + */ #define KEYMGR_CERT_MAX 8192 #define KEYMGR_KEY_MAX 8192 -/* SS7: keymgr now serves every live connection's listener-worker via its own peer entry, wired in by IMSG_SETUP_PEER and torn down on channel close; named so keymgr_dispatch_parent() doesn't need a forward declaration. */ +/* + * keymgr serves every live connection's listener-worker via its own peer + * entry, wired in by IMSG_SETUP_PEER and torn down on channel close; named + * so keymgr_dispatch_parent() doesn't need a forward declaration. + */ struct keymgr_peer { uint32_t session_id; struct imsgev iev; @@ -80,20 +94,37 @@ TAILQ_HEAD(keymgr_peer_list, keymgr_peer); static struct keymgr_peer_list keymgr_peers = TAILQ_HEAD_INITIALIZER(keymgr_peers); -static struct imsgev iev_parent; /* fd 3, alive for the process's lifetime */ +/* fd 3, alive for the process's lifetime */ +static struct imsgev iev_parent; -/* SIGHUP reload staging; keymgr_dispatch_parent() fires once both flags are set -- same pattern listener.c used for its own now-removed cert/key reload gating. */ -static char reload_cert_buf[KEYMGR_CERT_MAX], reload_key_buf[KEYMGR_KEY_MAX]; +/* + * SIGHUP reload staging; keymgr_dispatch_parent() fires once both flags are set + * -- same pattern listener.c used for its own now-removed cert/key reload + * gating. + */ +static char reload_cert_buf[KEYMGR_CERT_MAX]; +static char reload_key_buf[KEYMGR_KEY_MAX]; static size_t reload_cert_len, reload_key_len; static int reload_got_cert, reload_got_key; -/* The currently-loaded real key and its libtls-compatible pubkey hash; NULL/empty iff no usable key has ever loaded successfully. */ +/* + * The currently-loaded real key and its libtls-compatible pubkey hash; + * NULL/empty iff no usable key has ever loaded successfully. + */ static EVP_PKEY *keymgr_pkey; static char keymgr_hash[KEYMGR_HASH_MAX]; -/* SS6.1's explicit permission gate: true once the boot-time cert+key pair has been processed at all (even if it was rejected as unusable) -- distinct from keymgr_pkey being non-NULL, which tracks whether a *usable* key is currently loaded. A signing request arriving before this is set is refused outright, not merely "refused because no key is loaded yet", so the gate is checkable on its own rather than an incidental side effect of message ordering. */ +/* + * The explicit permission gate: true once the boot-time cert+key pair has + * been processed at all (even if it was rejected as unusable) -- distinct from + * keymgr_pkey being non-NULL, which tracks whether a *usable* key is currently + * loaded. A signing request arriving before this is set is refused outright, + * not merely "refused because no key is loaded yet", so the gate is checkable + * on its own rather than an incidental side effect of message ordering. + */ static int keymgr_got_init; +static void keymgr_key_free(void); static int keymgr_load(const char *, size_t, const char *, size_t); static void keymgr_try_reload(void); static int keymgr_pubkey_hash(X509 *, char *, size_t); @@ -120,10 +151,19 @@ keymgr_main(void) size_t cert_len = 0, key_len = 0; int got_cert = 0, got_key = 0; - /* fd-passing is allowed on this channel for the fd-passed IMSG_SETUP_PEER peer fds; see imsgev_ibuf_init()'s own comment */ + /* + * fd-passing is allowed on this channel for the fd-passed + * IMSG_SETUP_PEER peer fds; see imsgev_ibuf_init()'s own comment + */ imsgev_ibuf_init(&ibuf3, 3); - /* Both IMSG_TLS_CERT and IMSG_KEYMGR_INIT must be read before the peer handshake so keymgr_got_init/keymgr_pkey are final before any signing request can arrive; sent as two imsgs (mirroring parent.c's send_tls_cert()/send_keymgr_key() split) rather than one, to stay comfortably under MAX_IMSGSIZE. */ + /* + * Both IMSG_TLS_CERT and IMSG_KEYMGR_INIT must be read before the peer + * handshake so keymgr_got_init/keymgr_pkey are final before any signing + * request can arrive; sent as two imsgs (mirroring parent.c's + * send_tls_cert()/send_keymgr_key() split) rather than one, to stay + * comfortably under MAX_IMSGSIZE. + */ while (!got_cert || !got_key) { if ((n = imsgbuf_get(&ibuf3, &imsg)) == -1) fatal("imsgbuf_get"); @@ -183,12 +223,19 @@ keymgr_main(void) explicit_bzero(key_buf, sizeof(key_buf)); keymgr_got_init = 1; - /* keymgr's own daemon-user identity; SS5.5's chosen new account, following imapd's per-role convention (_imapd for listener, _imapauth for auth) over smtpd's literal SMTPD_USER reuse. */ + /* + * keymgr's own daemon-user identity: a dedicated account, following + * imapd's per-role convention (_imapd for listener, _imapauth for auth) + * over smtpd's literal SMTPD_USER reuse. + */ if ((pw = getpwnam("_imapkey")) == NULL) fatalx("getpwnam _imapkey: no such user " "(expected, not yet provisioned by an install script)"); - /* No filesystem access needed at all -- the key arrives over imsg from parent, never touches disk in this process. */ + /* + * No filesystem access needed at all -- the key arrives over imsg from + * parent, never touches disk in this process. + */ if (chroot("/var/empty") == -1) fatal("chroot /var/empty"); if (chdir("/") == -1) @@ -199,7 +246,14 @@ keymgr_main(void) setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) fatal("cannot drop privileges to _imapkey"); - /* SS7: keymgr stays the one boot-time, daemon-lifetime child, but no peer is wired to it at boot -- parent.c sends only IMSG_SETUP_DONE; every listener-worker peer arrives later over this same channel via IMSG_SETUP_PEER, handled below. */ + setproctitle("keymgr"); + + /* + * keymgr stays the one boot-time, daemon-lifetime child, but no peer is + * wired to it at boot -- parent.c sends only IMSG_SETUP_DONE; every + * listener-worker peer arrives later over this same channel via + * IMSG_SETUP_PEER, handled below. + */ setup_recv_done_and_ack(&ibuf3); event_init(); @@ -208,7 +262,12 @@ keymgr_main(void) NULL); #ifdef __OpenBSD__ - /* recvfd only: keymgr keeps receiving peer fds via IMSG_SETUP_PEER for its whole life but never sends one (only parent attaches descriptors to imsgs, and keymgr_reply() composes with fd == -1); no rpath either since keymgr touches no filesystem (SS5.5). */ + /* + * recvfd only: keymgr keeps receiving peer fds via IMSG_SETUP_PEER for + * its whole life but never sends one (only parent attaches descriptors + * to imsgs, and keymgr_reply() composes with fd == -1); no rpath either + * since keymgr touches no filesystem. + */ if (pledge("stdio recvfd", NULL) == -1) fatal("pledge"); #endif @@ -217,12 +276,21 @@ keymgr_main(void) fatalx("exited event loop"); } -/* Replicates smtpd's ssl.c hash_x509() byte-for-byte: SHA256 of the cert's DER SubjectPublicKeyInfo, formatted "SHA256:" plus lowercase hex -- the exact format is load-bearing (see file header), not cosmetic. */ +/* + * Replicates smtpd's ssl.c hash_x509() byte-for-byte: SHA256 of the cert's DER + * SubjectPublicKeyInfo, formatted "SHA256:" plus lowercase hex -- the exact + * format is load-bearing (see file header), not cosmetic. + */ static int keymgr_pubkey_hash(X509 *cert, char *hash, size_t hashlen) { static const char hex[] = "0123456789abcdef"; - /* Uses unsigned char/unsigned int, not smtpd hash_x509()'s signed types, to match X509_pubkey_digest(3)'s prototype and avoid -Wpointer-sign warnings; the emitted string is unchanged since digest[i] is already unsigned. */ + /* + * Uses unsigned char/unsigned int, not smtpd hash_x509()'s signed + * types, to match X509_pubkey_digest(3)'s prototype and avoid + * -Wpointer-sign warnings; the emitted string is unchanged since + * digest[i] is already unsigned. + */ unsigned char digest[EVP_MAX_MD_SIZE]; size_t off; unsigned int dlen, i; @@ -241,7 +309,23 @@ keymgr_pubkey_hash(X509 *cert, char *hash, size_t hash return (0); } -/* Parses a new cert+key pair fully before replacing the live one, so a malformed SIGHUP reload leaves the last known-good key in place instead of none; not sourced from smtpd's ca.c reload logic. cert_buf/key_buf aren't retained past this call -- callers must scrub key_buf themselves. */ +/* Frees the loaded key; EVP_PKEY_free wipes it via BN_free */ +/* unnecessary -- freed pages are zeroed -- but right on every exit path */ +static void +keymgr_key_free(void) +{ + if (keymgr_pkey != NULL) { + EVP_PKEY_free(keymgr_pkey); + keymgr_pkey = NULL; + } +} + +/* + * Parses a new cert+key pair fully before replacing the live one, so a + * malformed SIGHUP reload leaves the last known-good key in place instead of + * none; not sourced from smtpd's ca.c reload logic. cert_buf/key_buf aren't + * retained past this call -- callers must scrub key_buf themselves. + */ static int keymgr_load(const char *cert_buf, size_t cert_len, const char *key_buf, size_t key_len) @@ -279,7 +363,12 @@ keymgr_load(const char *cert_buf, size_t cert_len, con goto fail; } - /* A cert and key can each parse fine yet not correspond to each other (e.g. a rotation that replaced only one) -- checked here via X509_check_private_key() before swapping, since the per-request hash check elsewhere can't catch a cert/key mismatch. */ + /* + * A cert and key can each parse fine yet not correspond to each other + * (e.g. a rotation that replaced only one) -- checked here via + * X509_check_private_key() before swapping, since the per-request hash + * check elsewhere can't catch a cert/key mismatch. + */ if (X509_check_private_key(cert, pkey) != 1) { log_warnx("certificate and private key do not match, not " "(re)loading"); @@ -287,11 +376,14 @@ keymgr_load(const char *cert_buf, size_t cert_len, con } /* Both parsed; only now touch the live state. */ - if (keymgr_pkey != NULL) - EVP_PKEY_free(keymgr_pkey); + keymgr_key_free(); keymgr_pkey = pkey; pkey = NULL; - /* strlcpy, not memcpy of sizeof(): keymgr_pubkey_hash() only writes 72 of KEYMGR_HASH_MAX's 80 bytes, and memcpy would drag uninitialised stack bytes into a static. */ + /* + * strlcpy, not memcpy of sizeof(): keymgr_pubkey_hash() only writes 72 + * of KEYMGR_HASH_MAX's 80 bytes, and memcpy would drag uninitialised + * stack bytes into a static. + */ (void)strlcpy(keymgr_hash, hash, sizeof(keymgr_hash)); log_info("TLS key loaded (%s)", keymgr_hash); @@ -313,7 +405,13 @@ fail: return (-1); } -/* Commits a SIGHUP reload once BOTH halves have arrived: IMSG_TLS_CERT and IMSG_KEYMGR_INIT can land in either order, so both cases call this and only the second one finds the pair complete. The key buffer is scrubbed whether or not the load succeeded, and both flags clear so the next reload starts from a clean pair rather than half of this one. */ +/* + * Commits a SIGHUP reload once BOTH halves have arrived: IMSG_TLS_CERT and + * IMSG_KEYMGR_INIT can land in either order, so both cases call this and only + * the second one finds the pair complete. The key buffer is scrubbed whether or + * not the load succeeded, and both flags clear so the next reload starts from a + * clean pair rather than half of this one. + */ static void keymgr_try_reload(void) { @@ -327,7 +425,14 @@ keymgr_try_reload(void) reload_got_cert = reload_got_key = 0; } -/* PARENT channel (fd 3): SIGHUP reload's IMSG_TLS_CERT/IMSG_KEYMGR_INIT pair (same paired-flags shape listener.c used for its own now-removed cert/key reload gating), plus IMSG_SETUP_PEER wiring in a fresh listener-worker's peer (SS7: one per parent.c's spawn_connection() call, imsg_get_id() carries session_id -- see listener.c's own IMSG_SETUP_PEER (store) case for the same pattern). */ +/* + * PARENT channel (fd 3): SIGHUP reload's IMSG_TLS_CERT/IMSG_KEYMGR_INIT pair + * (same paired-flags shape listener.c used for its own now-removed cert/key + * reload gating), plus IMSG_SETUP_PEER wiring in a fresh listener-worker's peer + * (one per parent.c's spawn_connection() call, imsg_get_id() carries + * session_id -- see listener.c's own IMSG_SETUP_PEER (store) case for the same + * pattern). + */ static void keymgr_dispatch_parent(int fd, short event, void *arg) { @@ -343,8 +448,17 @@ keymgr_dispatch_parent(int fd, short event, void *arg) if ((n = imsgbuf_read(&iev->ibuf)) == -1) fatal("imsgbuf_read"); if (n == 0) { - /* Parent gone means this process is done: it used to linger serving existing peers, but keymgr is only ever consulted during a TLS handshake, so that only kept a key-holding process alive for as long as any client held a connection open; log_warnx (an operator should notice) and exit(0) (not a failure, just following the parent's death) rather than fatalx(). */ + /* + * Parent gone means this process is done: it used to + * linger serving existing peers, but keymgr is only + * ever consulted during a TLS handshake, so that only + * kept a key-holding process alive for as long as any + * client held a connection open; log_warnx (an operator + * should notice) and exit(0) (not a failure, just + * following the parent's death) rather than fatalx(). + */ log_warnx("parent closed channel, exiting"); + keymgr_key_free(); exit(0); } } @@ -356,6 +470,12 @@ keymgr_dispatch_parent(int fd, short event, void *arg) break; switch (imsg_get_type(&imsg)) { + case IMSG_KEYMGR_SHUTDOWN: + /* asked to go, unlike the EOF above; not a crash */ + log_debug("asked to shut down, exiting"); + imsg_free(&imsg); + keymgr_key_free(); + exit(0); case IMSG_SETUP_PEER: { uint32_t sess_id = imsg_get_id(&imsg); int peer_fd = imsg_get_fd(&imsg); @@ -426,7 +546,11 @@ keymgr_dispatch_parent(int fd, short event, void *arg) (void)fd; } -/* LISTENER channel: handles the three signing/decrypt request types (SS5.2/SS6.1); arg is the owning struct keymgr_peer (not a bare imsgev) so the EOF path knows which of possibly many live peers just went away. */ +/* + * LISTENER channel: handles the three signing/decrypt request types; + * arg is the owning struct keymgr_peer (not a bare imsgev) so the EOF + * path knows which of possibly many live peers just went away. + */ static void keymgr_dispatch_listener(int fd, short event, void *arg) { @@ -435,7 +559,14 @@ keymgr_dispatch_listener(int fd, short event, void *ar struct imsg imsg; ssize_t n; - /* A transport failure on one listener-worker's channel drops only that peer, not the whole process -- fatal()ing here would turn one connection's worker dying into a daemon-wide TLS outage, since keymgr is never restarted by parent.c's reap_child(); keymgr_dispatch_parent() (the fd-3 channel) stays strict since losing the parent leaves keymgr with no future. */ + /* + * A transport failure on one listener-worker's channel drops only that + * peer, not the whole process -- fatal()ing here would turn one + * connection's worker dying into a daemon-wide TLS outage, since keymgr + * is never restarted by parent.c's reap_child(); + * keymgr_dispatch_parent() (the fd-3 channel) stays strict since losing + * the parent leaves keymgr with no future. + */ if (event & EV_WRITE) { if (imsgbuf_write(&iev->ibuf) == -1) { log_warnx("session %u: write error on listener " @@ -485,7 +616,12 @@ keymgr_dispatch_listener(int fd, short event, void *ar (void)fd; } -/* Drops one listener-worker peer: unregisters its event, closes and clears its channel, unlinks and frees it -- shared by every exit path in keymgr_dispatch_listener() so EOF and transport error give the same outcome; imsgbuf_clear() is required or imsgbuf_init()'s allocation leaks. */ +/* + * Drops one listener-worker peer: unregisters its event, closes and clears its + * channel, unlinks and frees it -- shared by every exit path in + * keymgr_dispatch_listener() so EOF and transport error give the same outcome; + * imsgbuf_clear() is required or imsgbuf_init()'s allocation leaks. + */ static void keymgr_peer_teardown(struct keymgr_peer *kp) { @@ -496,7 +632,13 @@ keymgr_peer_teardown(struct keymgr_peer *kp) free(kp); } -/* Bound-checked read of this imsg's trailing raw bytes into a caller-supplied fixed buffer; same "fixed header + trailing raw bytes on one imsg" shape as store.c's recv_trailing_array()/imapd.h's imsg_mbox_append, without the malloc since KEYMGR_DATA_MAX is a small fixed cap rather than message-dependent. */ +/* + * Bound-checked read of this imsg's trailing raw bytes into a caller-supplied + * fixed buffer; same "fixed header + trailing raw bytes on one imsg" shape as + * store.c's recv_trailing_array()/imapd.h's imsg_mbox_append, without the + * malloc since KEYMGR_DATA_MAX is a small fixed cap rather than + * message-dependent. + */ static int keymgr_recv_trailing(struct imsg *imsg, uint32_t len, unsigned char *buf, size_t bufsize) @@ -518,15 +660,27 @@ keymgr_recv_trailing(struct imsg *imsg, uint32_t len, return (1); } -/* Composes a struct imsg_keymgr_sign_reply plus its trailing output bytes, reusing the SAME imsg type as the request (correlated by id) -- matches ca_imsg()'s own convention of replying on imsg->hdr.type rather than a distinct reply type. */ +/* + * Composes a struct imsg_keymgr_sign_reply plus its trailing output bytes, + * reusing the SAME imsg type as the request (correlated by id) -- matches + * ca_imsg()'s own convention of replying on imsg->hdr.type rather than a + * distinct reply type. + */ static void keymgr_reply(struct imsgev *iev, uint32_t type, uint32_t id, int ok, const void *to, size_t tolen) { struct imsg_keymgr_sign_reply rep; - unsigned char combined[sizeof(rep) + KEYMGR_DATA_MAX]; + unsigned char combined[sizeof(rep) + + KEYMGR_DATA_MAX]; - /* Every caller already bounds its own result, but combined[] is a fixed stack buffer holding the private key's output, so this function bound-checks independently rather than relying on that discipline holding forever; an oversized result is reported as a failed operation. */ + /* + * Every caller already bounds its own result, but combined[] is a fixed + * stack buffer holding the private key's output, so this function + * bound-checks independently rather than relying on that discipline + * holding forever; an oversized result is reported as a failed + * operation. + */ if (ok && tolen > KEYMGR_DATA_MAX) { log_warnx("keymgr_reply: %zu-byte result exceeds " "KEYMGR_DATA_MAX (%d), refusing", tolen, @@ -546,11 +700,19 @@ keymgr_reply(struct imsgev *iev, uint32_t type, uint32 sizeof(rep) + (ok ? tolen : 0)) == -1) log_warn("imsg_compose reply"); - /* imsg_compose() has copied the reply; this buffer may hold a decrypted premaster secret (on RSA_PRIVDEC), so it's scrubbed here like every other key-material buffer in this file. */ + /* + * imsg_compose() has copied the reply; this buffer may hold a decrypted + * premaster secret (on RSA_PRIVDEC), so it's scrubbed here like every + * other key-material buffer in this file. + */ explicit_bzero(combined, sizeof(combined)); } -/* IMSG_KEYMGR_RSA_PRIVENC / IMSG_KEYMGR_RSA_PRIVDEC: mirrors ca_imsg()'s RSA_private_encrypt()/RSA_private_decrypt() dispatch (ca.c:216-253), on imapd's own single-key state (SS5.5) rather than ca.c's hash-keyed dict. */ +/* + * IMSG_KEYMGR_RSA_PRIVENC / IMSG_KEYMGR_RSA_PRIVDEC: mirrors ca_imsg()'s + * RSA_private_encrypt()/RSA_private_decrypt() dispatch (ca.c:216-253), on + * imapd's own single-key state rather than ca.c's hash-keyed dict. + */ static void keymgr_handle_rsa(struct imsgev *iev, struct imsg *imsg, uint32_t type, uint32_t id) @@ -568,7 +730,10 @@ keymgr_handle_rsa(struct imsgev *iev, struct imsg *ims keymgr_reply(iev, type, id, 0, NULL, 0); return; } - /* imsg_get_buf() guarantees size, not NUL termination, force it (same reasoning as auth.c's inbound username/password fields). */ + /* + * imsg_get_buf() guarantees size, not NUL termination, force it (same + * reasoning as auth.c's inbound username/password fields). + */ req.hash[sizeof(req.hash) - 1] = '\0'; if (!keymgr_recv_trailing(imsg, req.fromlen, from, sizeof(from))) { @@ -578,7 +743,7 @@ keymgr_handle_rsa(struct imsgev *iev, struct imsg *ims if (!keymgr_got_init) { log_warnx("%s request before IMSG_KEYMGR_INIT " - "completed, refusing (SS6.1)", opname); + "completed, refusing", opname); keymgr_reply(iev, type, id, 0, NULL, 0); return; } @@ -617,11 +782,14 @@ keymgr_handle_rsa(struct imsgev *iev, struct imsg *ims return; } keymgr_reply(iev, type, id, 1, to, (size_t)ret); - /* RSA_PRIVDEC's output is the session's decrypted premaster secret; don't leave it on this process's stack. */ + /* + * RSA_PRIVDEC's output is the session's decrypted premaster secret; + * don't leave it on this process's stack. + */ explicit_bzero(to, sizeof(to)); } -/* IMSG_KEYMGR_ECDSA_SIGN: mirrors ca_imsg()'s ECDSA_sign() dispatch (ca.c:255-279). */ +/* IMSG_KEYMGR_ECDSA_SIGN: mirrors ca_imsg()'s ECDSA_sign() (ca.c:255-279). */ static void keymgr_handle_ecdsa(struct imsgev *iev, struct imsg *imsg, uint32_t id) { @@ -646,7 +814,7 @@ keymgr_handle_ecdsa(struct imsgev *iev, struct imsg *i if (!keymgr_got_init) { log_warnx("ECDSA_SIGN request before " - "IMSG_KEYMGR_INIT completed, refusing (SS6.1)"); + "IMSG_KEYMGR_INIT completed, refusing"); keymgr_reply(iev, IMSG_KEYMGR_ECDSA_SIGN, id, 0, NULL, 0); return; } blob - 6d37b8daf020763e4daddf91c126a00a30bea7d4 blob + 1b39f7831b03ae04c5fdd26c4b5eeee105463eb1 --- src/listener.c +++ src/listener.c @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * Copyright (c) 2014 Reyk Floeter @@ -8,11 +10,10 @@ * (rsa_engine_init()/ecdsa_engine_init()/rsae_priv_enc()/rsae_priv_ * dec()/ecdsae_do_sign(), ca.c:289-558) to imapd's own imsg * conventions: the OpenSSL API shape leaves little room for - * independent structure, and this project's own docs/LICENSE-AUDIT.md + * independent structure, and this project's own licensing * precedent (log.c, imsgev.c) already treats a borrow this close as * needing the original author's copyright even where the - * implementation differs; see docs/openimap-tls-privsep-design.md - * SS5.4 and SS10.1. keymgr_use_fake_private_key()'s two-line call + * implementation differs. keymgr_use_fake_private_key()'s two-line call * shape is lifted from smtpd's smtp.c:187-193 (Gilles Chehade, * Pierre-Yves Ritschard, Jacek Masiulaniec); see that function's own * comment. @@ -30,7 +31,11 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* listener.c, protocol/network process: client sockets, IMAP dispatch, TLS. Real TLS private-key operations are forwarded to keymgr(8); see keymgr_engine_init() below and docs/openimap-tls-privsep-design.md SS5. */ +/* + * listener.c, protocol/network process: client sockets, IMAP + * dispatch, TLS. Real TLS private-key operations are forwarded to + * keymgr(8); see keymgr_engine_init() below. + */ #include #include @@ -69,16 +74,35 @@ struct session_list sessions = TAILQ_HEAD_INITIALIZER(sessions); -struct imsgev iev_auth; /* Channel to the AUTH process; .ibuf.fd == -1 if this connection's auth-worker spawn failed, checked by auth_cmd.c's sasl_plain_finish() before sending IMSG_AUTH_REQUEST. */ -struct imsgev iev_search; /* Channel to the search-oracle process (SS8.1); .ibuf.fd == -1 if spawn failed, checked by search_cmd.c's search_dispatch() before sending IMSG_SEARCH_PARSE_REQUEST. */ +/* + * Channel to the AUTH process; .ibuf.fd == -1 if this connection's + * auth-worker spawn failed, checked by auth_cmd.c's + * sasl_plain_finish() before sending IMSG_AUTH_REQUEST. + */ +struct imsgev iev_auth; +/* + * Channel to the search-oracle process; .ibuf.fd == -1 if + * spawn failed, checked by search_cmd.c's search_dispatch() before + * sending IMSG_SEARCH_PARSE_REQUEST. + */ +struct imsgev iev_search; struct imsgev iev_parent; /* fd 3, alive for the process's lifetime */ -/* Channel to the keymgr process: private-key ops are forwarded here synchronously from OpenSSL callbacks, never via imsgev dispatch, so it's a plain struct imsgbuf; see keymgr_forward_rsa()/keymgr_forward_ecdsa(). */ +/* + * Channel to the keymgr process: private-key ops are forwarded here + * synchronously from OpenSSL callbacks, never via imsgev dispatch, so + * it's a plain struct imsgbuf; see + * keymgr_forward_rsa()/keymgr_forward_ecdsa(). + */ static struct imsgbuf keymgr_ibuf; static struct tls_config *listener_tls_config; -struct tls *listener_tls_ctx; /* NULL if TLS setup failed, degrades to no-TLS, not fatal */ -uint32_t listener_idle_poll_secs = IDLE_POLL_DEFAULT; /* overwritten from IMSG_LISTENER_SESSION_INIT below */ +/* NULL if TLS setup failed, no-TLS fallback */ +struct tls *listener_tls_ctx; +/* set from imsg */ +uint32_t listener_idle_poll_secs = IDLE_POLL_DEFAULT; +uint32_t listener_login_grace_secs = LOGIN_GRACE_DEFAULT; +uint64_t listener_append_max = APPEND_MAX_DEFAULT; /* Matches parent.c's send_tls_cert() read buffer size. */ #define TLS_CERT_MAX 8192 @@ -102,7 +126,7 @@ struct imap_cmd_entry { (1U << SESSION_LISTING)) #define ST_NOTAUTH (1U << SESSION_NOT_AUTH) -/* RFC 9051 SS9; transient in-flight states excluded (own pending_tag until their reply arrives). */ +/* RFC 9051 SS9; excludes transient states, which have their own pending_tag. */ #define ST_AUTH \ ((1U << SESSION_AUTHENTICATED) | (1U << SESSION_SELECTED)) #define ST_SELECTED (1U << SESSION_SELECTED) @@ -145,10 +169,19 @@ static const struct imap_cmd_entry imap_cmds[] = { }; #define NUM_IMAP_CMDS (sizeof(imap_cmds) / sizeof(imap_cmds[0])) -/* tls_config_use_fake_private_key() is an internal, undeclared libtls symbol forward-declared here, same as smtpd's smtp.c does; see docs/openimap-tls-privsep-design.md SS9 on the risk of depending on it. */ +/* + * tls_config_use_fake_private_key() is an internal, undeclared libtls + * symbol forward-declared here, same as smtpd's smtp.c does. Being + * an internal symbol, it can change or vanish without notice. + */ void tls_config_use_fake_private_key(struct tls_config *); -/* Installs libtls's placeholder private key plus the real certificate (smtp.c:187-193's call shape) in one function so listener_main()'s tls_config-building if/else-if chains need only one call per branch. */ +/* + * Installs libtls's placeholder private key plus the real certificate + * (smtp.c:187-193's call shape) in one function so listener_main()'s + * tls_config-building if/else-if chains need only one call per + * branch. + */ static int keymgr_set_fake_keypair(struct tls_config *config, const char *cert_buf, size_t cert_len) @@ -158,14 +191,24 @@ keymgr_set_fake_keypair(struct tls_config *config, con cert_len, NULL, 0); } -/* RSA/ECDSA privsep engine, installed once process-wide: intercepts every private-key operation OpenSSL performs against this process's fake key and forwards it to keymgr; adapted from smtpd's ca.c (ca.c:289-558) with imapd's own imsg framing. */ +/* + * RSA/ECDSA privsep engine, installed once process-wide: intercepts + * every private-key operation OpenSSL performs against this + * process's fake key and forwards it to keymgr; adapted from smtpd's + * ca.c (ca.c:289-558) with imapd's own imsg framing. + */ static const RSA_METHOD *keymgr_rsa_default; static RSA_METHOD *keymgr_rsae_method; static const EC_KEY_METHOD *keymgr_ecdsa_default; static EC_KEY_METHOD *keymgr_ecdsae_method; -/* Blocks reading keymgr_ibuf directly from inside an OpenSSL RSA_METHOD callback; unlike ca.c's rsae_send_imsg(), nothing else is ever multiplexed on this channel so there's no need to hand off unrelated imsgs. */ +/* + * Blocks reading keymgr_ibuf directly from inside an OpenSSL + * RSA_METHOD callback; unlike ca.c's rsae_send_imsg(), nothing else + * is ever multiplexed on this channel so there's no need to hand off + * unrelated imsgs. + */ static int keymgr_forward_rsa(uint32_t type, const char *hash, const unsigned char *from, int fromlen, unsigned char *to, size_t tosize, int padding) @@ -232,7 +275,12 @@ keymgr_forward_rsa(uint32_t type, const char *hash, co imsg_free(&imsg); break; } - /* Bound by tosize (OpenSSL's actual output buffer, e.g. 256 bytes for a 2048-bit key), not by the larger KEYMGR_DATA_MAX wire cap, or an oversized reply could overrun it; mirrors ca.c's own RSA_size() bound. */ + /* + * Bound by tosize (OpenSSL's actual output buffer, e.g. 256 + * bytes for a 2048-bit key), not by the larger + * KEYMGR_DATA_MAX wire cap, or an oversized reply could + * overrun it; mirrors ca.c's own RSA_size() bound. + */ if (rep.ok && rep.tolen <= tosize && imsg_get_len(&imsg) == rep.tolen) { if (imsg_get_buf(&imsg, to, rep.tolen) == -1) @@ -332,24 +380,45 @@ keymgr_forward_ecdsa(const char *hash, const unsigned return (sig); } -/* Checks A, B and C1 (docs/OpenIMAPD-TLS-Review/Opus-5-DESIGN-B3-implementation.md) before any key op is forwarded to keymgr; fatalx(), not a log line, since a failure here means privilege separation isn't actually in effect. */ +/* + * Runs checks A, B and C1, each described at its own test below, + * before any key op is forwarded to keymgr; fatalx(), not a log + * line, since a failure here means privilege separation isn't + * actually in effect. + */ static void keymgr_assert_fake_key(const char *hash, const BIGNUM *priv, const char *op) { size_t i; - /* Check A: a public-key-only object from tls_config_use_fake_private_key() never has d/priv_key set, so a non-NULL priv here means libtls is no longer using the placeholder key. */ + /* + * Check A: a public-key-only object from + * tls_config_use_fake_private_key() never has d/priv_key set, + * so a non-NULL priv here means libtls is no longer using the + * placeholder key. + */ if (priv != NULL) fatalx("%s: key object carries a private component -- " "libtls is no longer using a placeholder key, and this " "process is not separated from the TLS private key", op); - /* Check B: unlike smtpd's ca.c, listener configures exactly one keypair and never reaches this callback for an unrelated key, so a missing pubkey-hash tag is itself the regression, not a benign case to fall through on. */ + /* + * Check B: unlike smtpd's ca.c, listener configures exactly one + * keypair and never reaches this callback for an unrelated + * key, so a missing pubkey-hash tag is itself the regression, + * not a benign case to fall through on. + */ if (hash == NULL) fatalx("%s: no pubkey-hash tag on the key object -- libtls's " "ex_data slot 0 tagging has changed", op); - /* Check C1, done before the tag is read as a string: strlcpy(3) has no bound once the destination is full, so this bounded loop (not memchr/strnlen) confirms the tag is NUL-terminated within KEYMGR_HASH_MAX bytes before anything trusts it. */ + /* + * Check C1, done before the tag is read as a string: + * strlcpy(3) has no bound once the destination is full, so + * this bounded loop (not memchr/strnlen) confirms the tag is + * NUL-terminated within KEYMGR_HASH_MAX bytes before anything + * trusts it. + */ for (i = 0; i < KEYMGR_HASH_MAX; i++) if (hash[i] == '\0') return; @@ -385,7 +454,10 @@ keymgr_ecdsa_do_sign(const unsigned char *dgst, int dg { const char *hash = EC_KEY_get_ex_data(eckey, 0); - /* inv/rp are ECDSA_sign_setup() precomputation, unused since keymgr performs the operation, not this process. */ + /* + * inv/rp: ECDSA_sign_setup() precomputation, unused; keymgr does the + * op. + */ (void)inv; (void)rp; @@ -436,7 +508,12 @@ keymgr_ecdsa_engine_init(void) EC_KEY_set_default_method(keymgr_ecdsae_method); } -/* Installs both engine overrides; call exactly once, before any tls_config touches a key -- listener_main() calls this right before its TLS setup block, mirroring ca_engine_init()'s call from smtpd's dispatcher() (dispatcher.c:135). */ +/* + * Installs both engine overrides; call exactly once, before any + * tls_config touches a key -- listener_main() calls this right + * before its TLS setup block, mirroring ca_engine_init()'s call from + * smtpd's dispatcher() (dispatcher.c:135). + */ static void keymgr_engine_init(void) { @@ -444,7 +521,7 @@ keymgr_engine_init(void) keymgr_ecdsa_engine_init(); } -/* Builds and starts this process's one and only session; defined below, forward-declared here since listener_main() calls it. */ +/* Builds/starts this one session; forward-declared for listener_main(). */ static void listener_start_session(uint32_t, int, int, const struct sockaddr_storage *, socklen_t); @@ -453,22 +530,33 @@ listener_main(void) { struct imsgbuf ibuf3; struct passwd *pw; - int auth_peer_fd = -1, keymgr_peer_fd = -1; - int search_peer_fd = -1; /* SS8.1 */ + int auth_peer_fd = -1; + int keymgr_peer_fd = -1; + int search_peer_fd = -1; struct imsg imsg; struct imsg_listener_session_init sinit; ssize_t n; char cert_buf[TLS_CERT_MAX]; size_t cert_len = 0; int client_fd = -1; - int got_cert = 0, got_session_init = 0, got_keymgr_peer = 0; + int got_cert = 0, got_session_init = 0; + int got_keymgr_peer = 0; memset(&sinit, 0, sizeof(sinit)); - /* fd-passing is allowed on this channel: it receives fd-passed peer/session messages below; see imsgev_ibuf_init()'s own comment */ + /* + * fd-passing allowed here: receives fd-passed peer/session + * messages below; see imsgev_ibuf_init(). + */ imsgev_ibuf_init(&ibuf3, 3); - /* SS7: this process is spawned fresh per connection, so the peer handshake is drained in this same synchronous loop rather than separate blocking calls; the auth peer may never arrive, and IMSG_SETUP_PEER's id (0 vs session_id) tells auth from keymgr, matching parent.c's setup_peer_send(). */ + /* + * This process is spawned fresh per connection, so the + * peer handshake is drained in this same synchronous loop + * rather than separate blocking calls; the auth peer may never + * arrive, and IMSG_SETUP_PEER's id (0 vs session_id) tells + * auth from keymgr, matching parent.c's setup_peer_send(). + */ while (!got_cert || !got_session_init || !got_keymgr_peer) { if ((n = imsgbuf_get(&ibuf3, &imsg)) == -1) fatal("imsgbuf_get"); @@ -490,7 +578,13 @@ listener_main(void) "carried no fd"); break; } - /* A repeat can't happen today, but this runs pre-pledge/pre-privdrop where trusting the parent matters most, so state the invariant rather than silently overwrite; imsg_get_fd(3) already handed us peer_fd, so the duplicate must be closed here. */ + /* + * A repeat can't happen today, but this runs + * pre-pledge/pre-privdrop where trusting the parent + * matters most, so state the invariant rather than + * silently overwrite; imsg_get_fd(3) already handed + * us peer_fd, so the duplicate must be closed here. + */ if (id == 0) { if (auth_peer_fd != -1) { log_warnx("listener: duplicate auth " @@ -514,7 +608,11 @@ listener_main(void) case IMSG_SETUP_SEARCH_PEER: { int peer_fd = imsg_get_fd(&imsg); - /* SS8.1: optional, like the auth peer above -- not gated by the while() condition, spawn_connection() may not have wired one at all */ + /* + * Optional, like the auth peer above -- not + * gated by the while() condition, spawn_connection() + * may not have wired one at all + */ if (peer_fd == -1) log_warnx("listener: IMSG_SETUP_SEARCH_PEER " "carried no fd"); @@ -547,7 +645,12 @@ listener_main(void) log_warnx("bad IMSG_LISTENER_SESSION_INIT"); break; } - /* Refuse before claiming, unlike the peer cases above: an unclaimed fd on this imsg is closed by imsg_free() below, so there's nothing to clean up by hand. */ + /* + * Refuse before claiming, unlike the peer cases + * above: an unclaimed fd on this imsg is closed by + * imsg_free() below, so there's nothing to clean up + * by hand. + */ if (client_fd != -1) { log_warnx("listener: duplicate " "IMSG_LISTENER_SESSION_INIT, ignoring"); @@ -577,16 +680,22 @@ listener_main(void) if (chdir("/") == -1) fatal("chdir /"); - /* _imapd: ordinary daemon user, listener-role counterpart to auth.c's _imapauth. */ + /* _imapd: ordinary daemon user, counterpart to auth.c's _imapauth. */ if (setgroups(1, &pw->pw_gid) == -1 || setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) == -1 || setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) fatal("cannot drop privileges to _imapd"); - /* Installs the process-wide RSA_METHOD/EC_KEY_METHOD override before any tls_config touches a key. */ + /* + * Installs RSA_METHOD/EC_KEY_METHOD override before tls_config touches + * key. + */ keymgr_engine_init(); - /* Failure here isn't fatal, degrades to no-TLS, checked via listener_tls_ctx == NULL below. */ + /* + * Failure isn't fatal; degrades to no-TLS, checked via + * listener_tls_ctx. + */ if (cert_len == 0) { log_warnx("listener: no TLS cert received, TLS " "disabled for this session"); @@ -628,7 +737,12 @@ listener_main(void) event_init(); - /* auth_peer_fd may be -1 (no auth-worker spawned); iev_auth.ibuf.fd is left at -1 rather than defaulting to fd 0, and auth_cmd.c's sasl_plain_finish() checks that before composing to it. */ + /* + * auth_peer_fd may be -1 (no auth-worker spawned); iev_auth. + * ibuf.fd is left at -1 rather than defaulting to fd 0, and + * auth_cmd.c's sasl_plain_finish() checks that before composing + * to it. + */ if (auth_peer_fd != -1) imsgev_init(&iev_auth, auth_peer_fd, listener_dispatch_auth, NULL); @@ -639,9 +753,15 @@ listener_main(void) "connection gets one", sinit.session_id); } - /* SS8.1: search_peer_fd may be -1 (no search-oracle spawned, independent of auth's fork); iev_search.ibuf.fd is left at -1, checked by search_cmd.c's search_dispatch() before composing to it, mirroring iev_auth above. */ + /* + * search_peer_fd may be -1 (no search-oracle spawned, + * independent of auth's fork); iev_search.ibuf.fd is left at + * -1, checked by search_cmd.c's search_dispatch() before + * composing to it, mirroring iev_auth above. + */ if (search_peer_fd != -1) - imsgev_init(&iev_search, search_peer_fd, listener_dispatch_search, + imsgev_init(&iev_search, search_peer_fd, + listener_dispatch_search, NULL); else { iev_search.ibuf.fd = -1; @@ -654,17 +774,35 @@ listener_main(void) fatal("imsgbuf_init keymgr"); imsgbuf_set_maxsize(&keymgr_ibuf, MAX_IMSGSIZE); - /* Reuses fd 3's populated ibuf3, a fresh imsgbuf_init() would drop buffered bytes. */ + /* + * Reuses fd 3's populated ibuf; imsgbuf_init() would drop buffered + * bytes. + */ imsgev_init_from_ibuf(&iev_parent, &ibuf3, listener_dispatch_parent, NULL); - /* This process's one and only session, built from what boot just drained; must run after event_init() and the TLS setup above since session_tls_start()/session_arm_client_read() register libevent events needing listener_tls_ctx already set. */ + /* + * This process's one and only session, built from what boot + * just drained; must run after event_init() and the TLS setup + * above since session_tls_start()/session_arm_client_read() + * register libevent events needing listener_tls_ctx already + * set. + */ listener_idle_poll_secs = sinit.idle_poll_secs; + listener_login_grace_secs = sinit.login_grace_secs; + listener_append_max = sinit.append_max; listener_start_session(sinit.session_id, client_fd, sinit.implicit_tls, &sinit.remote_ss, sinit.remote_sslen); - /* pledge(2) promises: no socket/connect/bind/listen/accept call remains here (SS7 moved them to parent.c), so "inet" is dropped since getnameinfo(3) below only formats already-numeric bytes; "recvfd" stays for the store child's peer fd arriving later; "sendfd" goes since this process never attaches a descriptor to an imsg. */ + /* + * pledge(2) promises: no socket/connect/bind/listen/accept call + * remains here (parent.c owns them), so "inet" is + * dropped since getnameinfo(3) below only formats + * already-numeric bytes; "recvfd" stays for the store child's + * peer fd arriving later; "sendfd" goes since this process + * never attaches a descriptor to an imsg. + */ #ifdef __OpenBSD__ if (pledge("stdio recvfd", NULL) == -1) fatal("pledge"); @@ -674,8 +812,65 @@ listener_main(void) fatalx("listener: exited event loop"); } -/* Builds and starts this process's one and only session from the IMSG_LISTENER_SESSION_INIT payload drained at boot (SS7), doing what the old accept()-driven listener_accept() did: build struct session, format remote_addr, log, then begin the TLS handshake or send the plaintext greeting. */ +/* + * A connection that completes TCP and then says nothing held a + * listener-worker, an auth-worker and a search-oracle for ever, and at + * MaxStartups "full" that refuses every later connection. RFC 9051 SS5.4 + * permits a shortened pre-authentication timer for exactly this; its 30 + * minute floor governs a post-authentication autologout, which this server + * does not have. sshd's LoginGraceTime and smtpd's SMTPD_SESSION_TIMEOUT + * are the base-system analogues, and both time out in the process holding + * the client descriptor, as this does. + */ static void +session_login_grace_expired(int fd, short event, void *arg) +{ + struct session *s = arg; + + (void)fd; + (void)event; + + log_info("session %u: closing peer=%.200s, no authentication within " + "%u seconds", s->id, s->remote_addr, listener_login_grace_secs); + session_teardown(s, "login-grace"); +} + +/* + * Covers every pre-authentication stall, not just a missing command: an + * implicit-TLS connection that never sends a ClientHello never reaches the + * command path at all, so a timeout armed on one event would miss it. + */ +void +session_login_grace_init(struct session *s) +{ + struct timeval tv; + + evtimer_set(&s->grace_ev, session_login_grace_expired, s); + if (listener_login_grace_secs == 0) + return; /* "login grace 0": disabled by config */ + + tv.tv_sec = (time_t)listener_login_grace_secs; + tv.tv_usec = 0; + if (evtimer_add(&s->grace_ev, &tv) == -1) + log_warnx("session %u: evtimer_add (login grace); this " + "session will not be closed if it never authenticates", + s->id); +} + +void +session_login_grace_disarm(struct session *s) +{ + evtimer_del(&s->grace_ev); +} + +/* + * Builds and starts this process's one and only session from the + * IMSG_LISTENER_SESSION_INIT payload drained at boot, doing + * what the old accept()-driven listener_accept() did: build struct + * session, format remote_addr, log, then begin the TLS handshake or + * send the plaintext greeting. + */ +static void listener_start_session(uint32_t session_id, int client_fd, int implicit_tls, const struct sockaddr_storage *ss, socklen_t sslen) { @@ -685,20 +880,35 @@ listener_start_session(uint32_t session_id, int client if (s == NULL) { log_warn("calloc"); close(client_fd); - /* Exit directly rather than return: nothing else will ever run in this process, and returning would park it in event_dispatch() forever holding a MaxStartups slot with no client and no session to tear down. */ + /* + * Exit directly rather than return: nothing else will + * ever run in this process, and returning would park it + * in event_dispatch() forever holding a MaxStartups slot + * with no client and no session to tear down. + */ exit(1); } + s->pending_body_fd = -1; /* calloc(3)'s 0 is a real descriptor */ session_idle_poll_init(s); /* before anything can tear s down */ s->id = session_id; s->client_fd = client_fd; s->state = SESSION_NOT_AUTH; s->implicit_tls = implicit_tls; + session_login_grace_init(s); + /* CLOCK_MONOTONIC; see connected_at in listener.h. */ + if (clock_gettime(CLOCK_MONOTONIC, &s->connected_at) == -1) + log_warn("session %u: clock_gettime", s->id); TAILQ_INSERT_TAIL(&sessions, s, entry); { char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV]; - /* NI_NUMERIC*: pure formatting of already-numeric address bytes, no resolver or network I/O -- the fact listener_main()'s pledge() comment rests dropping "inet" on. */ + /* + * NI_NUMERIC*: pure formatting of already-numeric address + * bytes, no resolver or network I/O -- the fact + * listener_main()'s pledge() comment rests dropping + * "inet" on. + */ if (getnameinfo((const struct sockaddr *)ss, sslen, hbuf, sizeof(hbuf), sbuf, sizeof(sbuf), NI_NUMERICHOST | NI_NUMERICSERV) == 0) @@ -709,15 +919,20 @@ listener_start_session(uint32_t session_id, int client strlcpy(s->remote_addr, "?", sizeof(s->remote_addr)); } - log_debug("session %u: accepted from %s (%s)", s->id, + /* %.200s bounds untrusted text, the way sshd's auth.c does */ + /* tls= is how it was accepted; the close line has the final state */ + log_info("session %u: connected peer=%.200s tls=%s", s->id, s->remote_addr, - s->implicit_tls ? "implicit TLS" : "cleartext/STARTTLS"); + s->implicit_tls ? "implicit" : "none"); + /* id and stage only: ps titles are world-readable */ + setproctitle("session %u [accepted]", s->id); + if (s->implicit_tls) { if (listener_tls_ctx == NULL) { log_warnx("session %u: implicit-TLS port, but TLS " "isn't configured, closing", s->id); - session_teardown(s); + session_teardown(s, "tls-error"); return; } s->pending_greeting = 1; @@ -729,7 +944,7 @@ listener_start_session(uint32_t session_id, int client session_send_greeting(s); } -/* (Re-)registers client_ev for steady-state reads; guarded for handshake-repurposed re-registration. */ +/* (Re-)registers client_ev for steady-state reads; guards re-registration. */ void session_arm_client_read(struct session *s) { @@ -741,7 +956,7 @@ session_arm_client_read(struct session *s) s->client_ev_added = 1; } -/* Creates the per-connection struct tls (non-blocking), then arms client_ev to drive the handshake. */ +/* Creates per-conn struct tls (non-blocking), arms client_ev to drive it. */ void session_tls_start(struct session *s) { @@ -749,7 +964,7 @@ session_tls_start(struct session *s) != 0) { log_warnx("session %u: tls_accept_socket: %s", s->id, tls_error(listener_tls_ctx)); - session_teardown(s); + session_teardown(s, "tls-error"); return; } @@ -761,7 +976,7 @@ session_tls_start(struct session *s) s->client_ev_added = 1; } -/* Drives a non-blocking TLS handshake, re-arming client_ev for whichever direction it wants next. */ +/* Drives non-blocking TLS handshake, re-arms client_ev for wanted direction. */ void session_tls_handshake(int fd, short event, void *arg) { @@ -801,7 +1016,7 @@ session_tls_handshake(int fd, short event, void *arg) log_warnx("session %u: tls_handshake: %s (peer %s)", s->id, tls_error(s->tls_ctx), s->remote_addr); - session_teardown(s); + session_teardown(s, "tls-error"); } /* RFC 9051 SS7.1.1's example OK-response text, used verbatim. */ @@ -813,14 +1028,19 @@ session_send_greeting(struct session *s) session_write(s, greeting, sizeof(greeting) - 1); } -/* Forward decls: session_is_busy()/session_enqueue_cmd() are defined below session_dispatch_client() but called from it. */ +/* Forward decls: defined below session_dispatch_client() but called from it. */ static int session_is_busy(const struct session *); static int session_enqueue_cmd(struct session *, const char *); /* RFC 9051 SS4.3 hard cap on a non-synchronizing literal. */ #define IMAP_NONSYNC_LITERAL_MAX 4096 -/* True if `line` ends in a non-synchronizing literal announcement "{n+}" (RFC 9051 SS4.3), whose octets are already in flight and must be accounted for regardless of the command's fate; octet count returned in *lenp. */ +/* + * True if `line` ends in a non-synchronizing literal announcement + * "{n+}" (RFC 9051 SS4.3), whose octets are already in flight and + * must be accounted for regardless of the command's fate; octet + * count returned in *lenp. + */ static int line_nonsync_literal(const char *line, uint64_t *lenp) { @@ -839,7 +1059,8 @@ line_nonsync_literal(const char *line, uint64_t *lenp) open++; dlen = (size_t)(stop - open); if (dlen >= sizeof(digits) || *open < '0' || *open > '9') - return (0); /* also rejects strtoull(3)'s sign/space forms */ + /* also rejects strtoull(3)'s sign/space forms */ + return (0); memcpy(digits, open, dlen); digits[dlen] = '\0'; @@ -851,7 +1072,11 @@ line_nonsync_literal(const char *line, uint64_t *lenp) return (1); } -/* RFC 9051 SS9: tag = 1*; previously only length was checked, so a tag of "+" could turn session_reply()'s own reply into a command continuation request. */ +/* + * RFC 9051 SS9: tag = 1*; previously only + * length was checked, so a tag of "+" could turn session_reply()'s + * own reply into a command continuation request. + */ static int tag_is_valid(const char *tag) { @@ -868,20 +1093,25 @@ tag_is_valid(const char *tag) return (1); } -/* Splits s->inbuf into CRLF lines (bare LF isn't one, RFC 9051 SS2.2); session_handle_line() can free *s* (LOGOUT). */ +/* Splits s->inbuf into CRLF lines (bare LF isn't one, SS2.2); may free *s*. */ void session_dispatch_client(int fd, short event, void *arg) { struct session *s = arg; ssize_t n; char *crlf; - size_t tls_want = 0; /* bytes offered to tls_read(); re-armed at the end of this function -- named tls_want, not want, to avoid shadowing the literal-assembly loop's own uint64_t want (-Wshadow). */ + /* + * Bytes offered to tls_read(); re-armed at the end of this + * function -- named tls_want, not want, to avoid shadowing the + * literal-assembly loop's own uint64_t want (-Wshadow). + */ + size_t tls_want = 0; (void)event; if (s->write_failed) { /* A prior session_write() couldn't finish; see listener.h. */ - session_teardown(s); + session_teardown(s, "io-error"); return; } @@ -889,7 +1119,10 @@ session_dispatch_client(int fd, short event, void *arg tls_want = sizeof(s->inbuf) - s->inbuflen; n = tls_read(s->tls_ctx, s->inbuf + s->inbuflen, tls_want); if (n == TLS_WANT_POLLIN || n == TLS_WANT_POLLOUT) { - /* tls_read() can want to write (renegotiation); re-arm one-shot for the direction it needs. */ + /* + * tls_read() can want to write (renegotiation); re-arm + * for what it needs. + */ event_del(&s->client_ev); event_set(&s->client_ev, s->client_fd, (n == TLS_WANT_POLLIN) ? EV_READ : EV_WRITE, @@ -900,27 +1133,31 @@ session_dispatch_client(int fd, short event, void *arg if (n == -1) { log_warnx("session %u: tls_read: %s", s->id, tls_error(s->tls_ctx)); - session_teardown(s); + session_teardown(s, "tls-error"); return; } - session_arm_client_read(s); /* restore steady-state EV_READ|EV_PERSIST; harmless if already correct */ + /* restore EV_READ|EV_PERSIST; harmless if OK */ + session_arm_client_read(s); } else { n = read(fd, s->inbuf + s->inbuflen, sizeof(s->inbuf) - s->inbuflen); if (n == -1) { - /* client_fd is O_NONBLOCK since parent.c's parent_accept(). */ + /* + * client_fd is O_NONBLOCK since parent.c's + * parent_accept(). + */ if (errno == EINTR || errno == EAGAIN || errno == EWOULDBLOCK) return; log_warn("session %u: read", s->id); - session_teardown(s); + session_teardown(s, "io-error"); return; } } if (n == 0) { log_debug("session %u: client closed connection", s->id); - session_teardown(s); + session_teardown(s, "client-closed"); return; } s->inbuflen += (size_t)n; @@ -930,7 +1167,12 @@ session_dispatch_client(int fd, short event, void *arg size_t consumed, linelen; int alive; - /* Octets of a refused non-synchronizing literal (bad syntax, over cap, session busy, or not APPEND) are already on the wire, so swallow them instead of parsing the message body as further IMAP commands. */ + /* + * Octets of a refused non-synchronizing literal (bad syntax, + * over cap, session busy, or not APPEND) are already on the + * wire, so swallow them instead of parsing the message body + * as further IMAP commands. + */ if (s->literal_discard > 0) { uint64_t take; @@ -944,7 +1186,13 @@ session_dispatch_client(int fd, short event, void *arg } if (s->literal_discard > 0) break; /* need more data */ - /* Swallow the announcing command line's trailing CRLF so the line parser doesn't see a spurious zero-length line and answer "* BAD Empty command line" after every refused literal; anything besides CRLF is left for the parser. */ + /* + * Swallow the announcing command line's trailing CRLF + * so the line parser doesn't see a spurious + * zero-length line and answer "* BAD Empty command + * line" after every refused literal; anything besides + * CRLF is left for the parser. + */ if (s->inbuflen >= 2 && s->inbuf[0] == '\r' && s->inbuf[1] == '\n') { memmove(s->inbuf, s->inbuf + 2, @@ -954,7 +1202,10 @@ session_dispatch_client(int fd, short event, void *arg continue; } - /* RFC 9051 SS4.3 literal in flight; checked before CRLF search since raw octets can contain CRLF. */ + /* + * RFC 9051 SS4.3 literal in flight; checked before CRLF, may + * contain one. + */ if (s->literal_pending) { uint64_t want, take; @@ -963,9 +1214,15 @@ session_dispatch_client(int fd, short event, void *arg (uint64_t)s->inbuflen : want; if (take > 0) { - memcpy(s->literal_buf + - (s->literal_len - s->literal_remaining), - s->inbuf, (size_t)take); + /* on to the store; take <= SESSION_INBUF_MAX */ + if (imsg_compose(&s->store_iev->ibuf, + IMSG_MBOX_APPEND_DATA, 0, 0, -1, s->inbuf, + (size_t)take) == -1) { + log_warn("session %u: imsg_compose " + "IMSG_MBOX_APPEND_DATA", s->id); + session_teardown(s, "io-error"); + return; + } s->literal_remaining -= take; memmove(s->inbuf, s->inbuf + take, s->inbuflen - (size_t)take); @@ -975,14 +1232,17 @@ session_dispatch_client(int fd, short event, void *arg if (s->literal_remaining > 0) break; /* need more data */ - /* Literal body received; `command` still needs its trailing CRLF (RFC 9051 SS9). */ + /* + * Literal body received; `command` still needs its CRLF + * (RFC 9051 SS9). + */ if (s->inbuflen < 2) break; /* trailing CRLF hasn't arrived yet */ if (s->inbuf[0] != '\r' || s->inbuf[1] != '\n') { /* no reliable resync point, give up */ log_warnx("session %u: expected CRLF after " "literal data, closing", s->id); - session_teardown(s); + session_teardown(s, "protocol-error"); return; } memmove(s->inbuf, s->inbuf + 2, s->inbuflen - 2); @@ -1002,7 +1262,13 @@ session_dispatch_client(int fd, short event, void *arg linelen = (size_t)(crlf - s->inbuf); consumed = linelen + 2; - /* RFC 9051 SS2.2/SS9: CR/LF only appear as the CRLF terminator and NUL isn't an ASTRING-CHAR; this is the one chokepoint enforcing that before client text gets echoed back or silently truncated by the C-string parsers, so a violation closes the connection. */ + /* + * RFC 9051 SS2.2/SS9: CR/LF only appear as the CRLF + * terminator and NUL isn't an ASTRING-CHAR; this is the one + * chokepoint enforcing that before client text gets echoed + * back or silently truncated by the C-string parsers, so a + * violation closes the connection. + */ if (memchr(s->inbuf, '\r', linelen) != NULL || memchr(s->inbuf, '\n', linelen) != NULL || memchr(s->inbuf, '\0', linelen) != NULL) { @@ -1012,13 +1278,18 @@ session_dispatch_client(int fd, short event, void *arg log_warnx("session %u: bare CR/LF/NUL in command " "line, closing", s->id); session_write(s, bad, sizeof(bad) - 1); - session_teardown(s); + session_teardown(s, "protocol-error"); return; } *crlf = '\0'; - /* Note a trailing "{n+}" before dispatch since its octets follow immediately on the wire; over RFC 9051 SS4.3's 4096-octet cap the client is already out of spec and we can't guess how much to skip, so close instead. */ + /* + * Note a trailing "{n+}" before dispatch since its octets + * follow immediately on the wire; over RFC 9051 SS4.3's + * 4096-octet cap the client is already out of spec and we + * can't guess how much to skip, so close instead. + */ nonsync_len = 0; if (line_nonsync_literal(s->inbuf, &nonsync_len) && nonsync_len > IMAP_NONSYNC_LITERAL_MAX) { @@ -1030,19 +1301,29 @@ session_dispatch_client(int fd, short event, void *arg "literal (%llu), closing", s->id, (unsigned long long)nonsync_len); session_write(s, bad, sizeof(bad) - 1); - session_teardown(s); + session_teardown(s, "limit-exceeded"); return; } - /* SASL continuation (auth_cont) and IDLE's "DONE" (idling) route around the tag/name/args parser and the pipeline queue below. */ + /* + * SASL continuation, IDLE's DONE route around tag/name/args + * parser/queue. + */ if (s->auth_cont) { - /* the line is the user's password in base64, see below */ + /* the line is the user's base64 password, see below */ s->scrub_inbuf = 1; alive = session_handle_auth_continuation(s, s->inbuf); } else if (s->idling) { alive = session_handle_idle_continuation(s, s->inbuf); } else if (session_is_busy(s)) { - /* Queue rather than reject a command while one is already in flight (RFC 9051 SS5.5 pipelining), except one carrying a non-synchronizing literal: its octets are arriving now but cmd_append() wouldn't enter literal-read mode until dequeued, so refuse and swallow instead. */ + /* + * Queue rather than reject a command while one is + * already in flight (RFC 9051 SS5.5 pipelining), except + * one carrying a non-synchronizing literal: its octets + * are arriving now but cmd_append() wouldn't enter + * literal-read mode until dequeued, so refuse and + * swallow instead. + */ if (nonsync_len > 0) { session_reply(s, "*", "BAD", "non-synchronizing literal not accepted on " @@ -1057,7 +1338,7 @@ session_dispatch_client(int fd, short event, void *arg log_warnx("session %u: pipelined command " "queue full, closing", s->id); session_write(s, bad, sizeof(bad) - 1); - session_teardown(s); + session_teardown(s, "limit-exceeded"); return; } else alive = 1; @@ -1067,15 +1348,24 @@ session_dispatch_client(int fd, short event, void *arg if (alive == 0) return; /* s was torn down (LOGOUT), do not touch */ - /* Anything cmd_append() didn't take over (via literal_pending) gets swallowed rather than parsed. */ + /* + * Anything cmd_append() didn't take (literal_pending) is + * swallowed here. + */ if (nonsync_len > 0 && !s->literal_pending) s->literal_discard = nonsync_len; - /* cmd_starttls() zeroes inbuflen to discard pipelined plaintext, clamp to avoid underflow. */ + /* + * cmd_starttls() zeroes inbuflen to discard plaintext; clamps + * underflow. + */ if (consumed > s->inbuflen) consumed = s->inbuflen; - /* Don't leave a SASL response (base64 of the cleartext password) sitting in a long-lived heap buffer after it's been consumed. */ + /* + * Don't leave a SASL response (base64 password) in a long-lived + * buffer. + */ if (s->scrub_inbuf) { explicit_bzero(s->inbuf, consumed); s->scrub_inbuf = 0; @@ -1086,24 +1376,41 @@ session_dispatch_client(int fd, short event, void *arg } if (s->inbuflen == sizeof(s->inbuf)) { - /* Buffer full, no CRLF, matches the spirit of RFC 9051 SS7.1.3's example text. */ + /* + * Buffer full, no CRLF; matches RFC 9051 SS7.1.3's example + * text. + */ static const char bad[] = "* BAD command line too long\r\n"; log_warnx("session %u: command line too long, closing", s->id); - /* was a raw write(2), wrong on a TLS session, bytes would land unencrypted on the wire */ + /* + * was a raw write(2): wrong on a TLS session, bytes would land + * unencrypted + */ session_write(s, bad, sizeof(bad) - 1); - session_teardown(s); + session_teardown(s, "limit-exceeded"); return; } - /* A TLS record can hold more than inbuf's SESSION_INBUF_MAX, and level-triggered EV_READ won't refire for bytes libtls is still holding, so a full read re-queues this callback via event_active() (ncalls=1) to drain the rest; this terminates once tls_read() returns TLS_WANT_POLLIN. */ + /* + * A TLS record can hold more than inbuf's SESSION_INBUF_MAX, + * and level-triggered EV_READ won't refire for bytes libtls is + * still holding, so a full read re-queues this callback via + * event_active() (ncalls=1) to drain the rest; this terminates + * once tls_read() returns TLS_WANT_POLLIN. + */ if (s->tls_active && n > 0 && (size_t)n == tls_want && s->inbuflen < sizeof(s->inbuf)) event_active(&s->client_ev, EV_READ, 1); } -/* Blocking write(2)/tls_write(): retries EAGAIN/TLS_WANT_POLL* via poll(2) up to SESSION_WRITE_POLL_TIMEOUT_MS; on error or timeout it only records the failure in s->write_failed rather than tearing s down itself -- see listener.h and session_dispatch_client(). */ +/* + * Blocking write(2)/tls_write(): retries EAGAIN/TLS_WANT_POLL* via + * poll(2) up to SESSION_WRITE_POLL_TIMEOUT_MS; on error or timeout it + * only records the failure in s->write_failed rather than tearing s + * down itself -- see listener.h and session_dispatch_client(). + */ #define SESSION_WRITE_POLL_TIMEOUT_MS 5000 void @@ -1114,7 +1421,13 @@ session_write(struct session *s, const char *buf, size if (s->write_failed) return; - /* Permanent outbound-traffic diagnostic, gated on log_getverbose() since building/scrubbing dbuf is real work otherwise done on every write; session_write() carries every outbound byte including literal FETCH payloads, so -v -v is a message-content-exposure decision, not just a logging knob. */ + /* + * Permanent outbound-traffic diagnostic, gated on + * log_getverbose() since building/scrubbing dbuf is real work + * otherwise done on every write; session_write() carries every + * outbound byte including literal FETCH payloads, so -v -v is a + * message-content-exposure decision, not just a logging knob. + */ if (log_getverbose() > 0) { char dbuf[301]; size_t dlen = len < sizeof(dbuf) - 1 ? len : sizeof(dbuf) - 1; @@ -1142,7 +1455,8 @@ session_write(struct session *s, const char *buf, size pfd.fd = s->client_fd; pfd.events = POLLOUT; if (poll(&pfd, 1, - SESSION_WRITE_POLL_TIMEOUT_MS) <= 0) { + SESSION_WRITE_POLL_TIMEOUT_MS) + <= 0) { log_warnx("session %u: write: " "timed out or poll error", s->id); @@ -1195,7 +1509,17 @@ session_write(struct session *s, const char *buf, size } -/* Formats one complete response line into a 512-byte buffer and writes it. The one thing this must never do is emit a line the client cannot frame, so on overflow it forces the last two bytes back to CRLF rather than send a truncated, unterminated line -- that fixup is the whole reason session_reply() and session_untagged() are two lines each instead of fifteen: they differed only in their format string, and this is everything else they had in common. fuzz/fuzz_session_reply.c exists to hold exactly this invariant and carries its own stub copy of all three. */ +/* + * Formats one complete response line into a 512-byte buffer and + * writes it. The one thing this must never do is emit a line the + * client cannot frame, so on overflow it forces the last two bytes + * back to CRLF rather than send a truncated, unterminated line -- + * that fixup is the whole reason session_reply() and + * session_untagged() are two lines each instead of fifteen: they + * differed only in their format string, and this is everything else + * they had in common. fuzz/fuzz_session_reply.c exists to hold + * exactly this invariant and carries its own stub copy of all three. + */ static void session_writef(struct session *, const char *, ...) __attribute__((__format__ (printf, 2, 3))); @@ -1234,7 +1558,13 @@ session_untagged(struct session *s, const char *text) session_writef(s, "* %s\r\n", text); } -/* Shared client-composing send for every "fixed request struct + N elemsize-sized trailing elements" imsg to s->store_iev, plus the degenerate no-trailing-array form CREATE/DELETE/RENAME/LIST/STATUS use; malloc failure and imsg_compose failure are both reported back; the caller replies NO and restores s->state. */ +/* + * Shared client-composing send for every "fixed request struct + N + * elemsize-sized trailing elements" imsg to s->store_iev, plus the + * degenerate no-trailing-array form CREATE/DELETE/RENAME/LIST/STATUS + * use; malloc failure and imsg_compose failure are both reported + * back; the caller replies NO and restores s->state. + */ int send_mbox_request(struct session *s, int imsg_type, const char *what, const char *imsgname, const void *req, size_t reqlen, const void *elems, @@ -1243,7 +1573,13 @@ send_mbox_request(struct session *s, int imsg_type, co size_t bodylen = (size_t)nelems * elemsize; char *combined; - /* Nothing trailing: compose the caller's request where it already sits rather than malloc a copy of it just to hand it straight to imsg_compose(). Reached by the five fixed-size commands (LIST with req NULL and reqlen 0, which is the no-payload compose IMSG_MBOX_LIST wants), and by SELECT/EXPUNGE whenever their sequence-set is legitimately empty. */ + /* + * Nothing trailing: compose the caller's request where it + * already sits rather than malloc a copy of it just to hand it + * straight to imsg_compose(). Reached by the fixed-size commands, + * and by SELECT/EXPUNGE whenever their sequence-set is + * legitimately empty. + */ if (bodylen == 0) { if (imsg_compose(&s->store_iev->ibuf, imsg_type, 0, 0, -1, req, reqlen) == -1) { @@ -1261,7 +1597,14 @@ send_mbox_request(struct session *s, int imsg_type, co memcpy(combined, req, reqlen); memcpy(combined + reqlen, elems, bodylen); - /* Report a compose failure via return value instead of just logging it: previously s->state stayed at the busy value with nothing in flight, so session_is_busy() blocked forever waiting for a reply that would never be sent; every caller already handles a 0 return by replying NO and restoring state. */ + /* + * Report a compose failure via return value instead of just + * logging it: previously s->state stayed at the busy value with + * nothing in flight, so session_is_busy() blocked forever + * waiting for a reply that would never be sent; every caller + * already handles a 0 return by replying NO and restoring + * state. + */ if (imsg_compose(&s->store_iev->ibuf, imsg_type, 0, 0, -1, combined, reqlen + bodylen) == -1) { log_warn("session %u: imsg_compose %s", s->id, imsgname); @@ -1272,7 +1615,7 @@ send_mbox_request(struct session *s, int imsg_type, co return (1); } -/* RFC 7162 SS3.1: marks the session CONDSTORE-aware; emits an unsolicited HIGHESTMODSEQ OK if selected. */ +/* RFC 7162 SS3.1: marks CONDSTORE-aware; emits unsolicited HIGHESTMODSEQ. */ void session_condstore_enable(struct session *s) { @@ -1289,7 +1632,7 @@ session_condstore_enable(struct session *s) } } -/* Splits a CRLF-stripped line into tag/name/args (RFC 9051 `command = tag SP ...`); lenient on spaces. */ +/* Splits a CRLF-stripped line into tag/name/args (RFC 9051 `tag SP ...`). */ int parse_command_line(char *line, char **tag, char **name, char **args) { @@ -1333,7 +1676,11 @@ parse_command_line(char *line, char **tag, char **name return (0); } -/* True only for the post-auth async-round-trip states; pre-auth states (AUTHENTICATING/STORE_PENDING) deliberately excluded, see SESSION_CMD_QUEUE_MAX's comment. */ +/* + * True only for the post-auth async-round-trip states; pre-auth + * states (AUTHENTICATING/STORE_PENDING) deliberately excluded, see + * SESSION_CMD_QUEUE_MAX's comment. + */ static int session_is_busy(const struct session *s) { @@ -1357,7 +1704,7 @@ session_is_busy(const struct session *s) } } -/* Appends a pipelined line to s->cmd_queue; returns 0 on a full queue or strdup(3) failure, caller must teardown. */ +/* Appends a line to s->cmd_queue; 0 on a full queue or strdup(3) failure. */ static int session_enqueue_cmd(struct session *s, const char *line) { @@ -1375,7 +1722,7 @@ session_enqueue_cmd(struct session *s, const char *lin return (1); } -/* Dispatches queued pipelined commands while the session stays idle; returns 0 if one of them tore *s* down (LOGOUT). */ +/* Dispatches queued commands while idle; returns 0 if one tore *s* down. */ int session_dequeue_next(struct session *s) { @@ -1396,7 +1743,7 @@ session_dequeue_next(struct session *s) return (1); } -/* Returns 1 if the session is still alive, 0 if torn down (LOGOUT), caller must not touch *s* if 0. */ +/* Returns 1 if alive, 0 if torn down; caller must not touch *s* if 0. */ int session_handle_line(struct session *s, char *line) { @@ -1409,7 +1756,12 @@ session_handle_line(struct session *s, char *line) return (1); } - /* AUTHENTICATE's optional initial response is the base64 of the user's cleartext password (RFC 4616 SS2), and LOGIN sends it in the clear when LOGINDISABLED is ignored, so log the command but never its arguments. */ + /* + * AUTHENTICATE's optional initial response is the base64 of the + * user's cleartext password (RFC 4616 SS2), and LOGIN sends it + * in the clear when LOGINDISABLED is ignored, so log the + * command but never its arguments. + */ if (name != NULL && (strcasecmp(name, "AUTHENTICATE") == 0 || strcasecmp(name, "LOGIN") == 0)) log_debug("session %u: <<< %s %s ", s->id, tag, @@ -1419,7 +1771,10 @@ session_handle_line(struct session *s, char *line) name != NULL ? " " : "", name != NULL ? name : "", args != NULL ? " " : "", args != NULL ? args : ""); - /* Checked once here, not per strlcpy(3) site, an overlong tag must not be echoed back truncated. */ + /* + * Checked once here, not per strlcpy(3) site: tag must not echo + * truncated. + */ if (strlen(tag) >= IMAP_TAG_MAX) { session_reply(s, "*", "BAD", "Tag too long"); return (1); @@ -1443,7 +1798,10 @@ session_handle_line(struct session *s, char *line) return (1); } if (!(imap_cmds[i].states & (1U << s->state))) { - /* RFC 9051 SS3: BAD or NO for wrong-state command, BAD chosen here. */ + /* + * RFC 9051 SS3: BAD or NO for wrong-state command, BAD chosen + * here. + */ session_reply(s, tag, "BAD", "Command not permitted in this state"); return (1); @@ -1459,7 +1817,10 @@ listener_dispatch_auth(int fd, short event, void *arg) struct imsg imsg; ssize_t n; - /* Without this, a queued imsg_compose() never flushes and the EV_WRITE arm imsgev_on_compose() installed busy-loops. */ + /* + * Without this, a queued imsg_compose() never flushes; EV_WRITE + * busy-loops. + */ if (event & EV_WRITE) { if (imsgbuf_write(&iev->ibuf) == -1) fatal("imsgbuf_write"); @@ -1469,7 +1830,14 @@ listener_dispatch_auth(int fd, short event, void *arg) if ((n = imsgbuf_read(&iev->ibuf)) == -1) fatal("imsgbuf_read"); if (n == 0) { - /* SS7: this session's auth-worker may die independently of the session (already authenticated, or unable to AUTHENTICATE again); close and mark it dead rather than just event_del(), or a later AUTHENTICATE would compose onto a dead fd and the client would hang waiting for a reply. */ + /* + * This session's auth-worker may die + * independently of the session (already + * authenticated, or unable to AUTHENTICATE again); + * close and mark it dead rather than just event_del(), + * or a later AUTHENTICATE would compose onto a dead fd + * and the client would hang waiting for a reply. + */ log_warnx("auth-worker closed channel"); event_del(&iev->ev); close(iev->ibuf.fd); @@ -1501,12 +1869,16 @@ listener_dispatch_auth(int fd, short event, void *arg) } if (!res.ok) { s->state = SESSION_NOT_AUTH; + /* user= prints only if this stays set */ + explicit_bzero(s->user, sizeof(s->user)); session_reply(s, s->pending_tag, "NO", - "[AUTHENTICATIONFAILED] authentication failed"); + "[AUTHENTICATIONFAILED] authentication " + "failed"); break; } - + s->state = SESSION_STORE_PENDING; + setproctitle("session %u [authenticated]", s->id); break; } default: @@ -1520,7 +1892,13 @@ listener_dispatch_auth(int fd, short event, void *arg) (void)fd; } -/* SEARCH-ORACLE channel (SS8.1): at most one IMSG_SEARCH_PARSE_REQUEST/RESULT round trip is ever in flight for this process's one session, so TAILQ_FIRST(&sessions) is unambiguously it; session_id lookup elsewhere is kept only for parity with auth.c's imsg shape. */ +/* + * SEARCH-ORACLE channel: at most one + * IMSG_SEARCH_PARSE_REQUEST/RESULT round trip is ever in flight for + * this process's one session, so TAILQ_FIRST(&sessions) is + * unambiguously it; session_id lookup elsewhere is kept only for + * parity with auth.c's imsg shape. + */ void listener_dispatch_search(int fd, short event, void *arg) { @@ -1538,7 +1916,13 @@ listener_dispatch_search(int fd, short event, void *ar if ((n = imsgbuf_read(&iev->ibuf)) == -1) fatal("imsgbuf_read"); if (n == 0) { - /* SS8.1: this session's search-oracle may die independently of the session, same fail-soft shape as listener_dispatch_auth()'s channel-EOF handling -- an in-flight SEARCH gets a synthesized NO, and a later one fails fast via iev_search.ibuf.fd == -1. */ + /* + * This session's search-oracle may die + * independently of the session, same fail-soft shape + * as listener_dispatch_auth()'s channel-EOF handling -- + * an in-flight SEARCH gets a synthesized NO, and a + * later one fails fast via iev_search.ibuf.fd == -1. + */ log_warnx("search-oracle closed channel"); event_del(&iev->ev); close(iev->ibuf.fd); @@ -1551,7 +1935,8 @@ listener_dispatch_search(int fd, short event, void *ar "[UNAVAILABLE] search temporarily " "unavailable"); if (!session_dequeue_next(s)) - return; /* s torn down by a queued LOGOUT */ + /* s torn down by a queued LOGOUT */ + return; } return; } @@ -1583,7 +1968,13 @@ listener_dispatch_search(int fd, short event, void *ar "[SERVERBUG] internal error"); break; } - /* imsg_get_buf() guarantees size, not NUL termination, and this field goes straight to session_reply() via snprintf("%s"), so treat the least-trusted process's framing as untrusted, same as auth.c's username/password and parent.c's maildir. */ + /* + * imsg_get_buf() guarantees size, not NUL termination, + * and this field goes straight to session_reply() via + * snprintf("%s"), so treat the least-trusted process's + * framing as untrusted, same as auth.c's + * username/password and parent.c's maildir. + */ res.errmsg[sizeof(res.errmsg) - 1] = '\0'; if (res.rc == 0) { @@ -1592,9 +1983,9 @@ listener_dispatch_search(int fd, short event, void *ar bodylen != (size_t)res.nnodes * sizeof(struct search_node)) { log_warnx("bad " - "IMSG_SEARCH_PARSE_RESULT (nnodes " - "%u, %zu trailing bytes)", res.nnodes, - bodylen); + "IMSG_SEARCH_PARSE_RESULT " + "(nnodes %u, %zu trailing " + "bytes)", res.nnodes, bodylen); s->state = SESSION_SELECTED; session_reply(s, s->pending_tag, "NO", "[SERVERBUG] internal error"); @@ -1604,8 +1995,9 @@ listener_dispatch_search(int fd, short event, void *ar if ((nodes = malloc(bodylen)) == NULL) { log_warn("malloc SEARCH nodes"); s->state = SESSION_SELECTED; - session_reply(s, s->pending_tag, - "NO", "[SERVERBUG] internal " + session_reply(s, + s->pending_tag, "NO", + "[SERVERBUG] internal " "error"); break; } @@ -1616,8 +2008,9 @@ listener_dispatch_search(int fd, short event, void *ar "(nodes)"); free(nodes); s->state = SESSION_SELECTED; - session_reply(s, s->pending_tag, - "NO", "[SERVERBUG] internal " + session_reply(s, + s->pending_tag, "NO", + "[SERVERBUG] internal " "error"); break; } @@ -1644,7 +2037,15 @@ listener_dispatch_search(int fd, short event, void *ar } } -/* PARENT channel (fd 3): IMSG_STORE_FORK (spawn failure) and IMSG_SETUP_PEER (spawn success, imsg_get_id() has session id). No SIGHUP-driven reload here any more (SS7): this process is spawned fresh per connection and gets its own current TLS cert once at spawn time (IMSG_TLS_CERT, in listener_main()'s boot-drain loop), so it never lives long enough to need one -- see parent.c's header comment. */ +/* + * PARENT channel (fd 3): IMSG_STORE_FORK (spawn failure) and + * IMSG_SETUP_PEER (spawn success, imsg_get_id() has session id). No + * SIGHUP-driven reload here any more: this process is spawned + * fresh per connection and gets its own current TLS cert once at + * spawn time (IMSG_TLS_CERT, in listener_main()'s boot-drain loop), + * so it never lives long enough to need one -- see parent.c's header + * comment. + */ void listener_dispatch_parent(int fd, short event, void *arg) { @@ -1662,9 +2063,14 @@ listener_dispatch_parent(int fd, short event, void *ar if ((n = imsgbuf_read(&iev->ibuf)) == -1) fatal("imsgbuf_read"); if (n == 0) { - log_warnx("parent closed channel"); - event_del(&iev->ev); - return; + struct session *s = TAILQ_FIRST(&sessions); + + /* the parent is stopping us: end the session */ + log_debug("listener-worker: parent closed " + "channel, shutting down"); + if (s != NULL) + session_teardown(s, "shutdown"); + exit(0); } } @@ -1716,6 +2122,12 @@ listener_dispatch_parent(int fd, short event, void *ar imsgev_init(s->store_iev, store_fd, session_store_dispatch, s); s->state = SESSION_AUTHENTICATED; + /* + * Authenticated: stop the pre-authentication timer. + * This is the same point parent.c marks the session + * authenticated for MaxStartups. + */ + session_login_grace_disarm(s); log_debug("session %u: store peer wired", sess_id); /* RFC 9051 SS6.2.2's PLAIN example text, verbatim. */ session_reply(s, s->pending_tag, "OK", @@ -1745,9 +2157,10 @@ session_find(uint32_t id) return (NULL); } -/* Best-effort IMSG_STORE_SHUTDOWN to the store child, then closes fds, unregisters events, frees. */ +/* Best-effort IMSG_STORE_SHUTDOWN to store child; closes fds, unregisters. */ +/* reason: one of eight fixed tokens, never NULL, never attacker text */ void -session_teardown(struct session *s) +session_teardown(struct session *s, const char *reason) { if (s->store_iev != NULL) { if (imsg_compose(&s->store_iev->ibuf, IMSG_STORE_SHUTDOWN, @@ -1763,6 +2176,7 @@ session_teardown(struct session *s) } session_idle_poll_disarm(s); + session_login_grace_disarm(s); if (s->client_ev_added) event_del(&s->client_ev); @@ -1770,42 +2184,76 @@ session_teardown(struct session *s) if (s->tls_ctx != NULL) { int ret = tls_close(s->tls_ctx); - /* tls_close() closes the fd itself except when close_notify wants another round trip. */ + /* + * tls_close() closes the fd unless close_notify wants one more + * round trip. + */ if (ret == TLS_WANT_POLLIN || ret == TLS_WANT_POLLOUT) close(s->client_fd); + /* debug: a missing close_notify is routine; httpd ignores it */ else if (ret != 0) - log_warnx("session %u: tls_close: %s", s->id, + log_debug("session %u: tls_close: %s", s->id, tls_error(s->tls_ctx)); tls_free(s->tls_ctx); } else { close(s->client_fd); } - /* All NULL-safe, can still be set on a mid-stream teardown (FETCH/SEARCH/etc. in flight). */ - free(s->literal_buf); + /* + * All NULL-safe; may be set on a mid-stream teardown (FETCH/SEARCH + * etc). + */ free(s->search_matches); free(s->vanished_ranges); free(s->qresync_fetches); free(s->store_modified); free(s->pending_header_buf); - free(s->pending_body_buf); + if (s->pending_body_fd != -1) + close(s->pending_body_fd); free(s->pending_envelope_buf); free(s->pending_bodystructure_buf); - free(s->idle_known_uids); - free(s->idle_incoming_uids); - /* Any commands pipelined behind the one in flight when this session was torn down. */ + /* + * Commands pipelined behind the in-flight one when session was torn + * down. + */ while (s->cmd_queue_n > 0) free(s->cmd_queue[--s->cmd_queue_n]); TAILQ_REMOVE(&sessions, s, entry); - log_debug("session %u: closed (peer %s)", s->id, s->remote_addr); + { + struct timespec now; + const char *who = "-"; + long long dur = -1; - /* inbuf can still hold a base64 SASL response; don't hand it back to the allocator intact. */ + if (clock_gettime(CLOCK_MONOTONIC, &now) != -1 && + s->connected_at.tv_sec != 0) + dur = (long long)(now.tv_sec - + s->connected_at.tv_sec); + /* user= stays "-" until past authenticating; see listener.h */ + if (s->user[0] != '\0' && s->state != SESSION_AUTHENTICATING) + who = s->user; + /* tls= is the final state, unlike the connect line's */ + log_info("session %u: closed peer=%.200s user=%.64s " + "reason=%s tls=%s duration=%lld", s->id, + s->remote_addr, who, reason, + s->tls_active ? "yes" : "no", dur); + } + + /* + * inbuf may hold a base64 SASL response; don't hand it to allocator + * intact. + */ explicit_bzero(s, sizeof(*s)); free(s); - /* SS7: this process serves exactly this one session and never another, so exit rather than idle forever in event_dispatch() leaking a process per finished connection; parent.c's reap_child() already treats this exit as expected, matching store.c's store_shutdown(). */ + /* + * This process serves exactly this one session and never + * another, so exit rather than idle forever in event_dispatch() + * leaking a process per finished connection; parent.c's + * reap_child() already treats this exit as expected, matching + * store.c's store_shutdown(). + */ log_debug("listener-worker: session closed, exiting"); exit(0); } blob - 6fe957dc56d5fbcb2f274002f2b0ee09b78cece3 blob + 8755fed3f2e8207aa45e1e8304bd299107a67642 --- src/listener.h +++ src/listener.h @@ -1,3 +1,5 @@ +/* $OpenIMAPD$ */ + /* * Copyright (c) 2026 David Williams * @@ -14,11 +16,8 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* - * listener.h, internal, listener-process-only shared declarations, so - * listener.c's split files can all see struct session, enum - * session_state, and each other's entry points. - */ +/* listener.h: declarations private to the listener process, so */ +/* listener.c's split files can see struct session and each other. */ #ifndef LISTENER_H #define LISTENER_H @@ -28,6 +27,7 @@ #include #include +#include #include enum session_state { @@ -46,12 +46,12 @@ enum session_state { SESSION_EXPUNGING, /* IMSG_MBOX_EXPUNGE sent (EXPUNGE, or CLOSE * with silent=1), awaiting IMSG_MBOX_EXPUNGED * stream + terminal IMSG_MBOX_RESULT */ - SESSION_APPENDING, /* IMSG_MBOX_APPEND sent, awaiting the single - * terminal IMSG_MBOX_APPENDED reply. Distinct - * from the client-literal-read phase + SESSION_APPENDING, /* IMSG_MBOX_APPEND_END sent, awaiting the + * single terminal IMSG_MBOX_APPENDED reply. + * Distinct from the client-literal-read phase * (s->literal_pending) that precedes it -- * this covers only the store round trip. */ - SESSION_SEARCH_PARSING, /* SS8.1: IMSG_SEARCH_PARSE_REQUEST sent + SESSION_SEARCH_PARSING, /* IMSG_SEARCH_PARSE_REQUEST sent * to the per-connection search-oracle, * awaiting IMSG_SEARCH_PARSE_RESULT -- * precedes SESSION_SEARCHING below, a SEARCH @@ -79,81 +79,53 @@ enum session_state { * branches to * session_finish_copy_or_move(). */ - /* - * RFC 9051 SS6.3.4-SS6.3.6/SS6.3.9 additions (flat multi-mailbox - * support). All four are command-auth and never change s->state's - * SELECTED-ness; s->mbox_op_prev_state records the state to - * restore (one shared field, only one of these four is ever in - * flight per session). - */ + /* RFC 9051 SS6.3.4-SS6.3.9. All six are command-auth and never */ + /* change s->state's SELECTED-ness; mbox_op_prev_state records what */ + /* to restore, and only one is in flight per session. */ SESSION_CREATING, /* IMSG_MBOX_CREATE sent, single terminal * IMSG_MBOX_RESULT reply */ SESSION_DELETING, /* IMSG_MBOX_DELETE sent, same shape as * SESSION_CREATING */ SESSION_RENAMING, /* IMSG_MBOX_RENAME sent, same shape as * SESSION_CREATING */ - SESSION_LISTING /* IMSG_MBOX_LIST sent, awaiting the + SESSION_LISTING, /* IMSG_MBOX_LIST sent, awaiting the * IMSG_MBOX_LIST_ITEM stream + terminal * IMSG_MBOX_RESULT; branches to * session_finish_list(). */ + SESSION_SUBSCRIBING, /* IMSG_MBOX_SUBSCRIBE sent, same shape as + * SESSION_CREATING */ + SESSION_UNSUBSCRIBING /* IMSG_MBOX_UNSUBSCRIBE sent, same shape as + * SESSION_CREATING */ }; -/* - * Line-length cap for the raw CRLF-delimited read buffer below. RFC 9051 - * doesn't mandate a specific limit, but any real server needs one to - * bound memory for a client that never sends CRLF. - */ +/* Line-length cap for the raw read buffer. RFC 9051 mandates no limit, */ +/* but one is needed to bound a client that never sends CRLF. */ #define SESSION_INBUF_MAX 8192 -/* - * Bound for a client-chosen tag remembered across an async IMSG_AUTH_ - * REQUEST/IMSG_AUTH_RESULT round trip. RFC 9051 SS9 specifies no tag - * length limit; truncated via strlcpy() rather than rejected. - */ +/* Bound on a client-chosen tag held across an async round trip. RFC */ +/* 9051 SS9 sets no limit; an over-long tag is truncated, not rejected. */ #define IMAP_TAG_MAX 64 -/* - * Bound on pipelined command lines ahead of the one awaiting an async - * store round trip (RFC 9051 SS5.5 permits pipelining as long as the - * server processes them in order). A session that queues past this is - * disconnected rather than given unbounded memory. Does NOT apply to - * the pre-authentication states (see session_is_busy()). - */ +/* Bound on lines pipelined ahead of one awaiting a store round trip */ +/* (RFC 9051 SS5.5). Queueing past this disconnects the session rather */ +/* than granting unbounded memory. Not applied before authentication. */ #define SESSION_CMD_QUEUE_MAX 8 -/* - * Cap on the verbatim label text (e.g. "HEADER.FIELDS (DATE FROM)") - * stashed on struct session's pending_header_label and echoed back as - * "BODY[