commit 2a6467d3157cdcdc9f6a69080095e37a528abd2c from: David Williams date: Mon Sep 7 17:44:12 2026 UTC privsep hardening, IDLE polling, and a full security review pass Two new privilege-separated children. keymgr holds the TLS private key and performs every private-key operation on behalf of listener, which no longer has the key in its address space; listener reaches it through an OpenSSL RSA_METHOD/EC_KEY_METHOD engine override that forwards to keymgr over imsg. search-oracle parses the SEARCH grammar -- the largest and most attacker-reachable parser in the daemon -- in a per-connection process that has no descriptors, no filesystem and nothing to steal. Every child role's pledge(2) promise is narrower as a result: listener stdio recvfd sendfd inet -> stdio recvfd auth stdio rpath recvfd sendfd -> stdio rpath store stdio rpath wpath cpath flock recvfd sendfd -> stdio rpath wpath cpath flock keymgr (new) stdio recvfd search (new) stdio parent is now the only process in the daemon that can pass a descriptor at all, and search-oracle is down to bare stdio. IDLE is now a poll, and the README no longer claims otherwise. It said "real cross-session push"; what it did was tell an IDLEing session nothing until DONE. A session now rechecks its selected mailbox every "idle poll" seconds (default 5, 1-300, or 0 to disable), so mail arriving from an external MTA or another IMAP session is announced within one interval. Most polls cost two stat(2) calls and no lock: store only re-reads the index and streams UIDs when the mailbox directory or new/ has actually changed. New "startups begin N rate N full N" admission control on concurrent unauthenticated connections, following sshd_config(5)'s MaxStartups algorithm and its 10:30:100 default. A "listen" directive now selects which listeners run, not just how they are configured: a file whose only listen directive names tls port 993 binds nothing on 143, which is the deployment RFC 8314 section 3 asks for. With no listen directive at all, both are bound as before. Mailbox names are validated as UTF-8 (new utf8.c): RFC 3629 well-formedness, C1 controls refused, U+FEFF refused anywhere, per RFC 5198 Net-Unicode as far as it can be enforced without Unicode tables. The three mailbox-name parsers that had drifted apart are now one, so the mailbox and list-mailbox grammars of RFC 9051 section 9 differ in exactly one place instead of three. Correctness and robustness fixes from the review, each with a test: DELETE of the selected mailbox no longer leaves the session pointing at a mailbox that is gone; CREATE/DELETE/RENAME reject trailing garbage; INBOX is quoted in LIST output; the credential store is refused if it is world-accessible or group-writable, rather than serving every user's bcrypt hash from a 0644 file; control characters in logged names are escaped, so a mailbox name cannot forge log lines; keymgr exits when its parent dies instead of lingering. Migrated off imsg_get(), removed from OpenBSD libutil in commit 348f1fc0836b, and retired the hand-maintained "imsgev_add() after every imsg_compose()" pairing at 36 call sites in favour of an imsg close callback (imsgbuf_set_close_callback(3), added by that same commit). The read re-arm that shared the name is now imsgev_rearm_read(), so the two jobs are distinguishable. rc.d/imapd's relink step wrote a fixed /tmp path as root on every rcctl start, which is a symlink target; it now writes inside the mktemp -d directory it already creates. Found by checking the tree against the OpenBSD ports guide's security recommendations. Also: imapd.8 documents the new directives and the maildir-ownership deployment assumption; README records that keymgr depends on tls_config_use_fake_private_key() and ex_data behavior inside tls_keypair_load(), neither of which is declared in libtls's installed tls.h nor covered by any compatibility promise. commit - 845c36149fc11758db2f7d64937a70de2a56ed67 commit + 2a6467d3157cdcdc9f6a69080095e37a528abd2c blob - 7f147c923fac828c5211a4b66dfef861d9307ab8 blob + cc65af9f3e5ce618d06b770df9b41cf25029c780 --- README.md +++ README.md @@ -1,25 +1,29 @@ # OpenIMAPD -A from-scratch IMAP4rev2 ([RFC 9051](https://www.rfc-editor.org/rfc/rfc9051)) server for OpenBSD, written in traditional C in the privilege-separated tradition of `smtpd(8)`, `httpd(8)`, and `ntpd(8)`. No third-party IMAP library. +A from-scratch IMAP4rev2 ([RFC 9051](https://www.rfc-editor.org/rfc/rfc9051)) server for OpenBSD, written in C in the privilege-separated tradition of `smtpd(8)`, `httpd(8)`, and `ntpd(8)`. No third-party IMAP library. -**Status:** pre-release, version 0.1.1 Actively developed. Not a port. See [Getting the source](#getting-the-source) below for the repository. +**Status:** Pre-release, actively developed. Not a port. See [Getting the source](#getting-the-source) below for the repository. ## What it is -- **Privilege-separated**, `smtpd`-style: a root *parent* process reads configuration and binds the listening sockets; unprivileged *listener* and *auth* children handle the network and credential checks; a *store* child is forked per authenticated session, chroots into the mail spool, and drops privileges to that session's own user before ever touching a message. `pledge(2)`, `unveil(2)`, and `chroot(2)` enforce these boundaries, not just convention. +- **Privilege-separated**, `smtpd`-style, across six processes: a root *parent* reads configuration and binds the listening sockets; unprivileged *listener* and *auth* children handle the network and credential checks; *keymgr* holds the TLS private key and performs every private-key operation on request, so the process terminating TLS never has the key in its address space; a per-connection *search-oracle* parses the `SEARCH` grammar — the largest attacker-reachable parser in the daemon — in a process with no descriptors and no filesystem; and a *store* child is forked per authenticated session, chroots into the mail spool, and drops privileges to that session's own user before ever touching a message. `pledge(2)`, `unveil(2)`, and `chroot(2)` enforce these boundaries, not just convention: *parent* is the only process that can pass a file descriptor at all, and *search-oracle* runs on bare `stdio`. - **Storage**: stock maildir format (`tmp/`/`new/`/`cur/`, atomic delivery via `rename(2)`), readable with `ls` and `grep`, and natively understood by `smtpd(8)`'s own `maildir` delivery action. IMAP's extra bookkeeping (UIDs, UIDVALIDITY, per-message mod-sequences, keywords) lives in a small, `flock(2)`-guarded, line-oriented index file per mailbox, plain colon-delimited text, not a database. - **Transport**: STARTTLS on port 143 and implicit TLS on port 993 ([RFC 8314](https://www.rfc-editor.org/rfc/rfc8314)), via `libtls`. `AUTH=PLAIN` only, refused before TLS is established. ## Protocol coverage -`CAPABILITY`, `STARTTLS`, `AUTHENTICATE`, `ID`, `ENABLE`, `SELECT`, `EXAMINE`, `CREATE`, `DELETE`, `RENAME`, `LIST`, `LSUB`, `NAMESPACE`, `STATUS`, `FETCH` (including `ENVELOPE`, `BODYSTRUCTURE`, and MIME-part-addressed `BODY[]`/`BODY.PEEK[]`), `STORE`, `SEARCH`, `APPEND`, `COPY`, `MOVE`, `EXPUNGE`, `UNSELECT`, `CLOSE`, the `UID`-prefixed form of every command that supports it, `IDLE` with real cross-session push, and the [RFC 7162](https://www.rfc-editor.org/rfc/rfc7162) `CONDSTORE`/`QRESYNC` extensions. +`CAPABILITY`, `STARTTLS`, `AUTHENTICATE`, `ID`, `ENABLE`, `SELECT`, `EXAMINE`, `CREATE`, `DELETE`, `RENAME`, `LIST`, `LSUB`, `NAMESPACE`, `STATUS`, `FETCH` (including `ENVELOPE`, `BODYSTRUCTURE`, and MIME-part-addressed `BODY[]`/`BODY.PEEK[]`), `STORE`, `SEARCH`, `APPEND`, `COPY`, `MOVE`, `EXPUNGE`, `UNSELECT`, `CLOSE`, the `UID`-prefixed form of every command that supports it, `IDLE` (with the cross-session caveat noted below), and the [RFC 7162](https://www.rfc-editor.org/rfc/rfc7162) `CONDSTORE`/`QRESYNC` extensions. `SUBSCRIBE`, `UNSUBSCRIBE`, and ACL/shared-mailbox support are deliberately left out. +**`IDLE` is a poll, not a kernel-driven push.** An `IDLE`ing session rechecks its selected mailbox every `idle poll` seconds (default 5, see `imapd.conf`), so new mail — whether delivered by an external MTA or by another IMAP session — is reported within one interval rather than instantly. Most polls are two `stat(2)` calls and no lock: the store child only re-reads the index and streams UIDs when the mailbox directory or `new/` has actually been touched. Setting `idle poll 0` disables polling entirely, which restores the earlier behaviour where an `IDLE`ing session saw nothing until it sent `DONE`. + ## Requirements -OpenBSD only. This depends on ``, `pledge(2)`, `unveil(2)`, and libutil's `imsgbuf_*` API, none of which exist outside OpenBSD. Developed and tested against OpenBSD 8.0. Links against libevent, libtls/libssl/libcrypto, and libutil — all base-system libraries (see `src/Makefile`). +OpenBSD only. This depends on ``, `pledge(2)`, `unveil(2)`, and libutil's `imsgbuf_*` API, none of which exist outside OpenBSD. Developed and tested against OpenBSD 8.0. Links against libevent, libtls/libssl/libcrypto, and libutil, all base-system libraries (see `src/Makefile`). +`keymgr`, the process that isolates the TLS private key from `listener`, additionally depends on `tls_config_use_fake_private_key()` and undocumented ex_data-tagging behavior inside `tls_keypair_load()` — both unexported libtls/LibreSSL internals with no compatibility promise, and neither declared in libtls's public, installed `tls.h`. + ## Building and installing ``` blob - c805bc504412d3fbd4b2ce04c460968abe84b808 blob + 83844217b4c7c6b04ec517451e215b394640e175 --- contrib/imapduser.8 +++ contrib/imapduser.8 @@ -2,7 +2,7 @@ .\" .\" Written for the OpenIMAPD project. Public domain / no rights reserved. .\" -.Dd $Mdocdate: August 19 2026 $ +.Dd $Mdocdate: September 6 2026 $ .Dt IMAPDUSER 8 .Os .Sh NAME blob - b470a59c6c9e616124c7179097afd260205e1236 blob + 1cc25d6cde58222be9921a9f95c5ee229e4510a8 --- src/Makefile +++ src/Makefile @@ -6,10 +6,12 @@ PROG= imapd -SRCS= main.c parent.c log.c imsgev.c parse.y \ +SRCS= main.c parent.c log.c imsgev.c parse.y utf8.c \ listener.c auth_cmd.c mailbox_cmd.c append_cmd.c fetch_cmd.c \ search_cmd.c store_cmd.c store_ipc.c \ auth.c \ + keymgr.c \ + search_oracle.c \ store.c index.c mime.c envelope.c mbox_fetch.c mbox_search.c \ mbox_store.c mbox_manage.c mbox_copy.c blob - 3e52fad9d3e1bff8f2aa3c844b32c0036673d4cb blob + 0c2a64146c0476e8b02ed9b60b02b6b2ca467835 --- src/append_cmd.c +++ src/append_cmd.c @@ -85,6 +85,20 @@ parse_date_time(const char *s, int64_t *out) if (zsign == '-') zoff = -zoff; + /* + * The year is already required to be 1970 or later, but nothing bounds + * the zone -- sscanf's %c%2d%2d accepts up to +9999, and RFC 9051 SS9's + * "zone = ("+" / "-") 4DIGIT" does not constrain the value either. So + * "01-Jan-1970 00:00:00 +9959" resolves to -359940, and + * handle_mbox_append() formats the delivery timestamp straight into the + * maildir basename -- producing a file called "-359940.pid_n.host" in a + * maildir the README advertises as readable with ls(1) and grep(1), + * where any shell glob over cur/ hands that name to rm(1) as an + * option rather than a file. Out of the intended range, so refuse it. + */ + if ((int64_t)t - zoff < 0) + return (-1); + *out = (int64_t)t - zoff; return (0); } @@ -114,46 +128,13 @@ parse_append_args(char *args, struct append_parsed *ou return (-1); } - while (*p == ' ') - p++; - if (*p == '"') { - const char *start = p + 1; - char *end = strchr(start, '"'); - size_t len; - - if (end == NULL) { - *errmsg = "unterminated quoted mailbox name"; - return (-1); - } - len = (size_t)(end - start); - if (len == 0) { - *errmsg = "empty mailbox name"; - return (-1); - } - if (len >= sizeof(out->mailbox)) { - *errmsg = "mailbox name too long"; - return (-1); - } - memcpy(out->mailbox, start, len); - out->mailbox[len] = '\0'; - p = end + 1; - } else { - const char *start = p; - size_t len; - - while (*p != '\0' && *p != ' ') - p++; - len = (size_t)(p - start); - if (len == 0) { - *errmsg = "empty mailbox name"; - return (-1); - } - if (len >= sizeof(out->mailbox)) { - *errmsg = "mailbox name too long"; - return (-1); - } - memcpy(out->mailbox, start, len); - out->mailbox[len] = '\0'; + /* one parser for every mailbox argument in the tree; see mailbox_cmd.c */ + if (parse_mailbox_name(&p, out->mailbox, sizeof(out->mailbox), + errmsg) == -1) + return (-1); + if (out->mailbox[0] == '\0') { + *errmsg = "empty mailbox name"; + return (-1); } while (*p == ' ') @@ -240,6 +221,19 @@ parse_append_args(char *args, struct append_parsed *ou memcpy(digitsbuf, start, digits_len); digitsbuf[digits_len] = '\0'; + /* + * RFC 9051 SS9: literal = "{" number64 ["+"] "}", and + * number64 = 1*DIGIT -- no sign. strtoull(3) accepts one, so + * "{-1}" would arrive as ULLONG_MAX with errno untouched and + * be answered NO [LIMIT] "message too large" by cmd_append() + * rather than the BAD a syntax error deserves. Same + * first-character-is-a-digit guard listener.c's own literal + * pre-scan already applies to the same announcement. + */ + if (digitsbuf[0] < '0' || digitsbuf[0] > '9') { + *errmsg = "malformed literal octet count"; + return (-1); + } errno = 0; litlen = strtoull(digitsbuf, &digits_end, 10); if (*digits_end != '\0' || errno == ERANGE) { @@ -332,10 +326,23 @@ cmd_append(struct session *s, const char *tag, char *a s->literal_remaining = parsed.litlen; s->literal_pending = 1; - /* RFC 9051 SS4.3: only synchronizing literals need a "+" continuation; harmless but misleading to send for non-sync. */ - if (!parsed.litnonsync) - session_write(s, "+ Ready for literal data\r\n", 27); + /* + * RFC 9051 SS4.3: only synchronizing literals need a "+" continuation; + * harmless but misleading to send for non-sync. + * + * sizeof() - 1, not a hand-counted constant: this line used to pass 27 + * for a 26-byte string, which wrote the string literal's own NUL + * terminator onto the wire ahead of the tagged APPEND response. NUL is + * not a legal octet in the IMAP stream; clients mostly tolerate it, + * which is why it went unnoticed. Same idiom as listener.c's own + * session_write(s, bad, sizeof(bad) - 1) call sites. + */ + if (!parsed.litnonsync) { + static const char cont[] = "+ Ready for literal data\r\n"; + session_write(s, cont, sizeof(cont) - 1); + } + return (1); } @@ -395,11 +402,22 @@ session_finish_append(struct session *s) s->state = SESSION_APPENDING; + /* + * Same fail-soft shape send_mbox_request() now uses: a compose failure + * would otherwise leave s->state at SESSION_APPENDING with nothing in + * flight to move it back, and session_is_busy() then blocks every + * further command while the client waits for a tagged reply. + */ if (imsg_compose(&s->store_iev->ibuf, IMSG_MBOX_APPEND, 0, 0, -1, - combined, combined_len) == -1) + combined, combined_len) == -1) { log_warn("session %u: imsg_compose IMSG_MBOX_APPEND", s->id); + free(combined); + s->state = s->append_prev_state; + session_reply(s, s->pending_tag, "NO", + "[SERVERBUG] internal error"); + return (1); + } free(combined); - imsgev_add(s->store_iev); return (1); } @@ -431,8 +449,6 @@ session_handle_mbox_appended(struct session *s, appended_to_selected = (strcmp(s->append_mailbox, s->selected_mailbox) == 0); - /* RFC 9051 SS6.3.13: APPEND always adds one message, unlike EXPUNGE/CLOSE, no res->count gate needed. */ - session_notify_idle_peers(s); if (s->append_prev_state == SESSION_SELECTED && appended_to_selected) { char buf[32]; blob - 374c9f5dc398eca7c5d2b03b65e9d36cc15cf558 blob + 6e913abfb622ccd27527ba535ef21fdd7468da89 --- src/auth.c +++ src/auth.c @@ -17,6 +17,7 @@ /* auth.c, credential verification process: AUTHENTICATE PLAIN against the flat cred file. */ #include +#include #include #include @@ -51,6 +52,95 @@ 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's retrofit: does a second IMSG_AUTH_REQUEST for a session_id + * auth already resolved successfully get treated as a fresh login + * attempt, or refused? A correctly-behaving listener never sends one + * -- AUTHENTICATE is only offered from SESSION_NOT_AUTH (listener.c's + * command table), and a session leaves that state for good the moment + * auth grants it -- so this is pure defense against a compromised or + * buggy listener replaying (or fabricating a duplicate of) a grant it + * already received, mirroring keymgr.c's SS6.1 keymgr_got_init gate: + * explicit and checkable, not merely "the code that could send this + * doesn't exist yet." + * + * A fixed-size ring, not a TAILQ: auth is never told when a session + * ends (listener/store don't notify it -- only parent's + * store_children TAILQ has session lifecycle visibility, over a + * different channel), so there is no event to free an entry on. The + * alternative is unbounded growth; a ring just ages the oldest entry + * out instead. That's safe, not merely convenient: session_id is + * parent.c's monotonically increasing next_session_id (spawn_ + * connection()), minted once per accepted connection and never + * reused for the life of the daemon -- SS7 moved this counter, and + * the accept() loop that used to feed it, from listener.c to + * parent.c (see parent.c's own header comment). SS7 also makes + * reuse-within-one-process a stronger guarantee than that alone: + * each auth-worker is now spawned fresh per connection and paired + * with exactly one listener-worker for its own whole (short) life + * (parent.c's spawn_connection() again), so a single auth-worker + * can structurally never observe more than one distinct session_id + * in the first place. Kept as a ring rather than a single slot + * anyway, so this file doesn't need to change again if that ever + * stops being true. Sized well above parent.c's + * STORE_CHILD_MAX (64 concurrent sessions) so eviction shouldn't + * happen at any realistic session volume. + */ +/* + * Cap on failed authentication attempts this auth-worker will spend a + * bcrypt on, i.e. sshd_config(5)'s MaxAuthTries, whose own default this + * matches. + * + * A plain static counter IS a per-connection counter here: SS7 spawns one + * auth-worker per connection, paired with exactly one listener-worker for + * its whole (short) life (parent.c's spawn_connection()) -- the same fact + * the auth_resolved comment above relies on. + * + * Why it is needed: listener.c returns a failed session to + * SESSION_NOT_AUTH and AUTHENTICATE is ST_NOTAUTH, so a client may retry + * without limit. Each retry costs this process one bcrypt -- ~100ms of + * CPU, deliberately -- and costs the client one small packet on a + * connection it already holds. That asymmetry is backwards: bcrypt's work + * factor is meant to be paid by whoever is guessing, and without a cap an + * attacker converts cheap packets into unbounded server CPU across as + * many connections as MaxStartups allows. + * + * Past the cap, requests are refused WITHOUT calling crypt_checkpass(3), + * which is what removes the cost. The connection is not dropped -- doing + * that needs a listener-side change and a way to say so on the wire; the + * CPU asymmetry, which is the actual damage, is closed either way. + */ +#define AUTH_MAX_TRIES 6 +static unsigned int auth_failures; + +#define AUTH_RESOLVED_MAX 256 +static uint32_t auth_resolved[AUTH_RESOLVED_MAX]; +static size_t auth_resolved_next; /* ring cursor */ +static int auth_resolved_full; /* 1 once the ring has wrapped once */ + +static int +auth_session_already_resolved(uint32_t sid) +{ + size_t limit = auth_resolved_full ? AUTH_RESOLVED_MAX : auth_resolved_next; + size_t i; + + for (i = 0; i < limit; i++) { + if (auth_resolved[i] == sid) + return (1); + } + return (0); +} + +static void +auth_session_mark_resolved(uint32_t sid) +{ + auth_resolved[auth_resolved_next++] = sid; + if (auth_resolved_next == AUTH_RESOLVED_MAX) { + auth_resolved_next = 0; + auth_resolved_full = 1; + } +} + __dead void auth_main(void) { @@ -63,14 +153,13 @@ auth_main(void) char chrootdir[1024]; ssize_t n; - if (imsgbuf_init(&ibuf3, 3) == -1) - fatal("imsgbuf_init"); - imsgbuf_allow_fdpass(&ibuf3); /* for the fd-passed IMSG_SETUP_PEER peer fd below */ + /* fd-passing is allowed on this channel for the IMSG_SETUP_PEER peer fd below; see imsgev_ibuf_init()'s own comment */ + imsgev_ibuf_init(&ibuf3, 3); /* IMSG_AUTH_INIT must be read first: cred_file is needed before chroot() can be computed */ for (;;) { - if ((n = imsg_get(&ibuf3, &imsg)) == -1) - fatal("imsg_get"); + if ((n = imsgbuf_get(&ibuf3, &imsg)) == -1) + fatal("imsgbuf_get"); if (n != 0) break; if ((n = imsgbuf_read(&ibuf3)) == -1) @@ -83,6 +172,15 @@ 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 -- + * same rule as req.username/req.password below, and parent.c's own + * inbound maildir. strlcpy(3) reads its source to the NUL to compute + * its return value, so an unterminated field here would be an + * unbounded read past this stack struct, on the message that decides + * what directory this process chroot(2)s into. + */ + init.cred_file[sizeof(init.cred_file) - 1] = '\0'; imsg_free(&imsg); /* auth's own daemon-user identity; distinct from listener's _imapd. */ @@ -94,13 +192,26 @@ auth_main(void) if (strlcpy(chrootdir, init.cred_file, sizeof(chrootdir)) >= sizeof(chrootdir)) fatalx("cred_file too long: %s", init.cred_file); + /* + * Actually test what the message below claims. strrchr() finding a + * '/' only means the path HAS a directory component: "etc/creds" + * would pass and then chroot(2) relative to whatever working + * directory rc.d(8) left this daemon in. parse.y only length-checks + * the "credentials" directive, so nothing upstream enforces this. + */ + if (init.cred_file[0] != '/') + fatalx("cred_file must be an absolute path: %s", + init.cred_file); if ((slash = strrchr(chrootdir, '/')) == NULL) fatalx("cred_file must be an absolute path: %s", init.cred_file); if (strlcpy(cred_file_basename, slash + 1, sizeof(cred_file_basename)) >= sizeof(cred_file_basename)) fatalx("cred_file basename too long: %s", init.cred_file); - *slash = '\0'; + if (slash == chrootdir) + chrootdir[1] = '\0'; /* "/creds" -> chroot("/"), not chroot("") */ + else + *slash = '\0'; if (chroot(chrootdir) == -1) fatal("chroot %s", chrootdir); @@ -112,9 +223,18 @@ auth_main(void) setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid) == -1) fatal("cannot drop privileges to _imapauth"); - /* boot-time handshake: one peer (listener), then SETUP_DONE+ack */ + /* + * SS7: auth-worker's one and only peer, wired by parent.c's + * spawn_connection() via setup_peer_send() the moment both it + * and the listener-worker it's paired with exist. No + * IMSG_SETUP_DONE ack round-trip -- see parent.c's header + * comment for why spawn_connection() doesn't use one for + * per-connection wiring; this boot sequence already reads a + * fixed, statically known set of messages (just IMSG_AUTH_INIT + * above, then this) before ever touching the event loop, + * regardless of any ack. + */ peer_fd = setup_recv_one_peer(&ibuf3); - setup_recv_done_and_ack(&ibuf3); event_init(); imsgev_init(&iev_listener, peer_fd, auth_dispatch, NULL); @@ -133,8 +253,29 @@ auth_main(void) fatal("unveil lock"); } + /* + * No recvfd, no sendfd. This process receives exactly one descriptor + * in its life -- the peer fd from setup_recv_one_peer() above, which + * has already arrived by the time this line runs -- and it never + * sends one: the parent is the only process in the tree that attaches + * a descriptor to an imsg (parent.c's setup_peer_send(), + * setup_search_peer_send() and IMSG_LISTENER_SESSION_INIT are the + * only five such call sites). + * + * An earlier version of this comment kept both promises on the theory + * that an imsgbuf_allow_fdpass() channel uses sendmsg(2)/recvmsg(2) + * for all of its traffic. It does -- but that is not what the two + * promises gate. SYS_sendmsg and SYS_recvmsg are PLEDGE_STDIO + * (sys/kern/kern_pledge.c); "sendfd"/"recvfd" are checked in + * unp_internalize()/unp_externalize() (sys/kern/uipc_usrreq.c), which + * the kernel reaches only when SCM_RIGHTS is actually attached to the + * message. A plain imsg with fd == -1 needs neither. + * + * If that reasoning is wrong, pledge(2) does not degrade: a violation + * is an uncatchable SIGABRT with a core dump, and this line reverts. + */ #ifdef __OpenBSD__ - if (pledge("stdio rpath recvfd sendfd", NULL) == -1) + if (pledge("stdio rpath", NULL) == -1) fatal("pledge"); #endif @@ -159,15 +300,29 @@ auth_dispatch(int fd, short event, void *arg) if ((n = imsgbuf_read(&iev->ibuf)) == -1) fatal("imsgbuf_read"); if (n == 0) { - log_warnx("listener closed channel"); - event_del(&iev->ev); - return; + /* + * SS7: this process was spawned (parent.c's + * spawn_connection()) to serve exactly this one + * connection's listener-worker and will never serve + * another -- exit now rather than sit in + * event_dispatch() forever with nothing left to do. + * parent.c's reap_child() already documents this + * exact expectation ("left to notice its own peer + * channel EOF and exit on its own") and already + * treats an auth-worker exit as the ordinary, + * expected end of a session, not something to warn + * about. Matches store.c's store_shutdown() for the + * same reason on that per-session worker. + */ + log_debug("auth-worker: listener closed channel, " + "exiting"); + exit(0); } } for (;;) { - if ((n = imsg_get(&iev->ibuf, &imsg)) == -1) - fatal("imsg_get"); + if ((n = imsgbuf_get(&iev->ibuf, &imsg)) == -1) + fatal("imsgbuf_get"); if (n == 0) break; @@ -183,6 +338,15 @@ auth_dispatch(int fd, short event, void *arg) /* imsg_get_data() guarantees size, not NUL termination, force it */ req.username[sizeof(req.username) - 1] = '\0'; req.password[sizeof(req.password) - 1] = '\0'; + + 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)); + break; + } + memset(&res, 0, sizeof(res)); res.session_id = req.session_id; auth_verify(&req, &res); @@ -193,11 +357,12 @@ auth_dispatch(int fd, short event, void *arg) if (imsg_compose(&iev->ibuf, IMSG_AUTH_RESULT, 0, 0, -1, &res, sizeof(res)) == -1) log_warn("imsg_compose IMSG_AUTH_RESULT"); - imsgev_add(iev); if (res.ok) { struct imsg_auth_cred cred; + auth_session_mark_resolved(res.session_id); + memset(&cred, 0, sizeof(cred)); cred.session_id = res.session_id; cred.uid = res.uid; @@ -213,7 +378,6 @@ auth_dispatch(int fd, short event, void *arg) IMSG_AUTH_CRED, 0, 0, -1, &cred, sizeof(cred)) == -1) log_warn("imsg_compose IMSG_AUTH_CRED"); - imsgev_add(&iev_parent); } break; } @@ -224,8 +388,7 @@ auth_dispatch(int fd, short event, void *arg) } imsg_free(&imsg); } - /* unconditional re-arm: imsgev_init() is EV_READ not EV_PERSIST, so a pure EV_WRITE call would let it lapse */ - imsgev_add(iev); + imsgev_rearm_read(iev); (void)fd; } @@ -253,8 +416,8 @@ auth_dispatch_parent(int fd, short event, void *arg) } for (;;) { - if ((n = imsg_get(&iev->ibuf, &imsg)) == -1) - fatal("imsg_get"); + if ((n = imsgbuf_get(&iev->ibuf, &imsg)) == -1) + fatal("imsgbuf_get"); if (n == 0) break; @@ -262,10 +425,29 @@ auth_dispatch_parent(int fd, short event, void *arg) imsg_get_type(&imsg)); imsg_free(&imsg); } - imsgev_add(iev); + imsgev_rearm_read(iev); (void)fd; } +/* + * Usernames come off the network, so they reach syslog only through this: + * anything outside printable ASCII becomes '?'. listener.c is meant to + * reject a bare CR/LF in a command line, but auth is a separate process and + * does not get to assume that held. + */ +static void +auth_safe_name(const char *in, char *out, size_t outsize) +{ + size_t i; + + for (i = 0; i + 1 < outsize && in[i] != '\0'; i++) { + unsigned char c = (unsigned char)in[i]; + + out[i] = (c >= 0x20 && c < 0x7f) ? (char)c : '?'; + } + out[i] = '\0'; +} + /* always calls crypt_checkpass() with hash == NULL on unknown username, to avoid timing leaks */ static void auth_verify(struct imsg_auth_request *req, struct imsg_auth_result *res) @@ -273,7 +455,26 @@ auth_verify(struct imsg_auth_request *req, struct imsg struct cred_entry ce; const char *hash = NULL; int found; + char safename[AUTH_USERNAME_MAX]; + /* + * Budget spent: refuse without spending a bcrypt. Deliberately + * before cred_lookup(), so a client past the cap cannot even make + * this process re-read and re-scan the credential file. Logged and + * reported exactly like any other failure -- the client learns + * nothing it did not already know. + */ + if (auth_failures >= AUTH_MAX_TRIES) { + char overname[AUTH_USERNAME_MAX]; + + res->ok = 0; + auth_safe_name(req->username, overname, sizeof(overname)); + log_info("session %u: authentication failed for \"%s\" " + "(over AUTH_MAX_TRIES, not checked)", req->session_id, + overname); + return; + } + found = (cred_lookup(cred_file_basename, req->username, &ce) == 0); if (found) hash = ce.passwordhash; @@ -286,8 +487,27 @@ auth_verify(struct imsg_auth_request *req, struct imsg res->gid = ce.gid; } else { res->ok = 0; + auth_failures++; } + /* + * Nothing used to record an authentication outcome at all, so a + * password-guessing run left no trace in the logs and there was + * nothing for pf(4)/fail2ban-style tooling to key on. log_info() is + * not gated on verbosity (see log.c), so these are always emitted. + * The failure line deliberately does not distinguish "no such user" + * from "wrong password" -- that would hand back the same enumeration + * oracle crypt_checkpass(NULL) exists to close. + */ + 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); + else + log_info("session %u: authentication failed for \"%s\"", + req->session_id, safename); + explicit_bzero(&ce, sizeof(ce)); } @@ -304,12 +524,57 @@ cred_lookup(const char *path, const char *username, st return (-1); } + /* + * parent.c refuses to load the TLS private key unless it is + * root-owned and no looser than 0740; this file holds every user's + * bcrypt hash and had no such check. A flat text file people edit + * by hand very easily ends up 0644, at which point any local user + * can take the hashes away and attack them offline. + * + * Deliberately permissive about ownership and group-read, so the + * usual "root:_imapauth 0640" and "_imapauth 0400" layouts both + * pass; only world access and group-write are refused. Fails + * closed: a credential store with the wrong mode stops logins + * rather than serving them, and says so loudly. + */ + { + struct stat st; + + if (fstat(fileno(fp), &st) == -1) { + log_warn("fstat %s", path); + fclose(fp); + return (-1); + } + if (st.st_mode & (S_IROTH | S_IWOTH | S_IXOTH | S_IWGRP)) { + log_warnx("%s: insecure permissions (mode %04o), must " + "not be world-accessible or group-writable; " + "refusing all authentication until this is fixed", + path, (unsigned)(st.st_mode & 07777)); + fclose(fp); + return (-1); + } + } + while (fgets(line, sizeof(line), fp) != NULL) { - char *p = line; - char *fields[5]; - int i; - char *ep; + char *p = line; + char *fields[5]; + int i; + char *ep; + unsigned long ulval; + /* + * fgets(3) splits an over-long line, and the tail would then be + * parsed as its own entry -- a phantom credential rather than an + * error. Refuse to read the file at all rather than guess. + */ + if (strchr(line, '\n') == NULL && + strlen(line) == sizeof(line) - 1) { + log_warnx("%s: over-long line, refusing to parse the " + "credential file", path); + explicit_bzero(line, sizeof(line)); + fclose(fp); + return (-1); + } line[strcspn(line, "\n")] = '\0'; if (line[0] == '\0' || line[0] == '#') continue; @@ -334,13 +599,54 @@ cred_lookup(const char *path, const char *username, st strlcpy(out->passwordhash, fields[1], sizeof(out->passwordhash)) >= sizeof(out->passwordhash)) continue; + /* + * crypt_checkpass(3) returns SUCCESS when the stored hash + * and the supplied password are both empty + * (lib/libc/crypt/cryptutil.c's "empty password" case), so a + * blank second field is not a disabled account -- it is a + * login with an empty password. Require a bcrypt hash, for + * the same reason as the uid/gid checks below: the + * credential file should not be able to express this in the + * first place. + * + * Only the empty case needs catching. Any other non-bcrypt + * value ("!", "*", a legacy crypt string, garbage) already + * falls through crypt_checkpass()'s own "$2" test to its + * fake: label, which burns a bcrypt for timing and fails. + * + * "continue" rather than a distinct error, matching every + * other malformed-entry case here: the client must not be + * able to tell this apart from "no such user", or the + * enumeration oracle crypt_checkpass(NULL) exists to close + * comes back through the error text. + * + * To disable an account, use imapduser -d, which removes the + * line. + */ + if (fields[1][0] != '$' || fields[1][1] != '2') + continue; + /* + * strtoul(3) accepts a leading "-", so "-1" would arrive here as + * 0xffffffff, and "0" is root. parent.c refuses uid/gid 0 at the + * spawn boundary, which is the check that matters -- but the + * credential file should not be able to express either in the + * first place, and the wrap case is caught nowhere else. + */ + if (fields[2][0] < '0' || fields[2][0] > '9' || + fields[3][0] < '0' || fields[3][0] > '9') + continue; errno = 0; - out->uid = (uid_t)strtoul(fields[2], &ep, 10); - if (*ep != '\0' || errno != 0) + ulval = strtoul(fields[2], &ep, 10); + if (*ep != '\0' || errno != 0 || ulval == 0 || + ulval >= (unsigned long)(uid_t)-1) continue; - out->gid = (gid_t)strtoul(fields[3], &ep, 10); - if (*ep != '\0' || errno != 0) + out->uid = (uid_t)ulval; + errno = 0; + ulval = strtoul(fields[3], &ep, 10); + if (*ep != '\0' || errno != 0 || ulval == 0 || + ulval >= (unsigned long)(gid_t)-1) continue; + out->gid = (gid_t)ulval; if (strlcpy(out->maildir, fields[4], sizeof(out->maildir)) >= sizeof(out->maildir)) continue; @@ -348,6 +654,13 @@ 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() + * scrubs its struct cred_entry and auth_dispatch() scrubs the + * password; this buffer was the one left behind. + */ + explicit_bzero(line, sizeof(line)); fclose(fp); return (found ? 0 : -1); } blob - fa4f85eeb4c4c347226d0d001e2580ec44713e2e blob + a83bf2678a4738a483467710477fd2c2d2e0eb29 --- src/auth_cmd.c +++ src/auth_cmd.c @@ -211,12 +211,27 @@ sasl_plain_finish(struct session *s, const char *tag, session_reply(s, tag, "NO", "[SERVERBUG] internal error"); return (1); } + /* + * SS7: this connection's auth-worker may not exist -- parent.c's + * spawn_connection() tolerates that fork failing independently + * of the listener-worker's own. listener_main()'s boot-drain + * loop leaves iev_auth.ibuf.fd at -1 in that case rather than + * wiring it to a real peer (see listener.c's globals-block + * comment on iev_auth). Fail gracefully instead of composing to + * an unwired imsgev. + */ + if (iev_auth.ibuf.fd == -1) { + session_reply(s, tag, "NO", "[UNAVAILABLE] authentication " + "temporarily unavailable"); + explicit_bzero(&req, sizeof(req)); + return (1); + } + s->state = SESSION_AUTHENTICATING; if (imsg_compose(&iev_auth.ibuf, IMSG_AUTH_REQUEST, 0, 0, -1, &req, sizeof(req)) == -1) log_warn("session %u: imsg_compose IMSG_AUTH_REQUEST", s->id); - imsgev_add(&iev_auth); /* imsg_compose() already copied req, safe to scrub our stack copy */ explicit_bzero(&req, sizeof(req)); @@ -243,6 +258,7 @@ int session_handle_idle_continuation(struct session *s, const char *line) { s->idling = 0; + session_idle_poll_disarm(s); if (strcasecmp(line, "DONE") != 0) { session_reply(s, s->pending_tag, "BAD", @@ -294,6 +310,9 @@ cmd_authenticate(struct session *s, const char *tag, c } if (initial != NULL) { /* RFC 9051 SS6.2.2 initial-resp: finishes in one round trip */ + /* `initial` is the base64 of the cleartext password and points + * into s->inbuf; have the reader scrub it once consumed. */ + s->scrub_inbuf = 1; return sasl_plain_finish(s, tag, initial, 1); } blob - 51f65325e114d9eab73fcbd8cff51ff4da44736c blob + 4ece4d36298f7a478bc532382f97e19b0132e581 --- src/envelope.c +++ src/envelope.c @@ -40,7 +40,9 @@ int envbuf_append(char *buf, size_t bufsize, size_t *outlen, const char *data, size_t datalen) { - if (*outlen + datalen > bufsize) + /* subtract rather than add: "*outlen + datalen" wraps if datalen is + * ever close to SIZE_MAX, and the check would then pass */ + if (*outlen > bufsize || datalen > bufsize - *outlen) return (-1); memcpy(buf + *outlen, data, datalen); *outlen += datalen; @@ -59,21 +61,37 @@ int envbuf_append_nstring(char *buf, size_t bufsize, size_t *outlen, const char *val, size_t vallen) { + size_t save = *outlen; size_t i; if (val == NULL) return (envbuf_append_str(buf, bufsize, outlen, "NIL")); if (envbuf_append(buf, bufsize, outlen, "\"", 1) == -1) - return (-1); + goto fail; for (i = 0; i < vallen; i++) { - if ((val[i] == '"' || val[i] == '\\') && + char c = val[i]; + + /* RFC 9051 SS4.3: a quoted string is TEXT-CHAR only, which + * excludes NUL, CR and LF; substitute rather than reject so + * one odd byte in a header doesn't drop the whole field */ + if (c == '\0' || c == '\r' || c == '\n') + c = ' '; + if ((c == '"' || c == '\\') && envbuf_append(buf, bufsize, outlen, "\\", 1) == -1) - return (-1); - if (envbuf_append(buf, bufsize, outlen, &val[i], 1) == -1) - return (-1); + goto fail; + if (envbuf_append(buf, bufsize, outlen, &c, 1) == -1) + goto fail; } - return (envbuf_append(buf, bufsize, outlen, "\"", 1)); + if (envbuf_append(buf, bufsize, outlen, "\"", 1) == -1) + goto fail; + return (0); + +fail: + /* all-or-nothing: a partial append leaves an unterminated quoted + * string in the caller's buffer */ + *outlen = save; + return (-1); } /* Formats one RFC 5322 mailbox as an IMAP address tuple (RFC 9051 SS9); no group syntax, addr-adl always NIL. */ @@ -200,6 +218,9 @@ envbuf_append_one_address(char *buf, size_t bufsize, s mailboxlen -= 2; } + if (envbuf_append(addrbuf, sizeof(addrbuf), &addrlen, "(", 1) == -1) + return (-1); + if (name != NULL) { size_t j; @@ -237,12 +258,13 @@ envbuf_append_one_address(char *buf, size_t bufsize, s if (envbuf_append_nstring(addrbuf, sizeof(addrbuf), &addrlen, host, hostlen) == -1) return (-1); + if (envbuf_append(addrbuf, sizeof(addrbuf), &addrlen, ")", 1) == -1) + return (-1); - if (envbuf_append(buf, bufsize, outlen, "(", 1) == -1) - return (-1); - if (envbuf_append(buf, bufsize, outlen, addrbuf, addrlen) == -1) - return (-1); - return (envbuf_append(buf, bufsize, outlen, ")", 1)); + /* one atomic append: either the whole "(...)" tuple lands in the + * caller's buffer or none of it does (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. */ @@ -295,13 +317,14 @@ envbuf_append_address_list(char *buf, size_t bufsize, tok_len--; if (tok_len > 0) { + /* a malformed or non-fitting address leaves buf/outlen + * untouched -- envbuf_append_one_address() builds the + * whole "(...)" tuple locally before its one atomic + * append into buf -- so skipping it and continuing is + * safe */ if (envbuf_append_one_address(buf, bufsize, outlen, val + tok_start, tok_len) == 0) any = 1; - else if (*outlen > bufsize) { - return (-1); /* can't happen; envbuf_append() never overruns bufsize */ - } - /* malformed address: buf/outlen untouched on failure (addrbuf only flushed atomically) */ } } blob - c30aafc310af2638e430368765831fed1d627e96 blob + 3d9bc13cb2e5fed06610f3907760529a6f0c3fce --- src/fetch_cmd.c +++ src/fetch_cmd.c @@ -60,10 +60,16 @@ parse_nz_number(const char *str, uint32_t *out) return (0); } -/* RFC 9051 SS9 seq-range; "*" unresolved here, carried via lo_star/hi_star to store.c; backwards literal range swapped */ -int -parse_seq_range(const char *tok, uint32_t *lo, uint32_t *hi, int *lo_star, - int *hi_star) +/* + * Parses one range token -- no ':'-split ambiguity beyond the existing + * single colon, no comma -- into r. This used to be parse_seq_range()'s + * entire body, factored out so parse_sequence_set() below can reuse it + * once per comma-separated segment instead of duplicating it (parse_ + * seq_range() itself is gone now -- QRESYNC known-uids, its last + * caller, moved to parse_sequence_set() directly; see mailbox_cmd.c). + */ +static int +parse_one_seq_range(const char *tok, struct seq_range *r) { char buf[32]; char *colon; @@ -74,8 +80,8 @@ parse_seq_range(const char *tok, uint32_t *lo, uint32_ if (strlcpy(buf, tok, sizeof(buf)) >= sizeof(buf)) return (-1); - *lo_star = *hi_star = 0; - *lo = *hi = 0; + r->lo_is_star = r->hi_is_star = 0; + r->lo = r->hi = 0; if ((colon = strchr(buf, ':')) != NULL) { *colon = '\0'; @@ -87,25 +93,82 @@ parse_seq_range(const char *tok, uint32_t *lo, uint32_ } if (strcmp(loside, "*") == 0) - *lo_star = 1; - else if (parse_nz_number(loside, lo) == -1) + r->lo_is_star = 1; + else if (parse_nz_number(loside, &r->lo) == -1) return (-1); if (strcmp(hiside, "*") == 0) - *hi_star = 1; - else if (parse_nz_number(hiside, hi) == -1) + r->hi_is_star = 1; + else if (parse_nz_number(hiside, &r->hi) == -1) return (-1); - if (!*lo_star && !*hi_star && *lo > *hi) { - uint32_t tmp = *lo; + if (!r->lo_is_star && !r->hi_is_star && r->lo > r->hi) { + uint32_t tmp = r->lo; - *lo = *hi; - *hi = tmp; + r->lo = r->hi; + r->hi = tmp; } return (0); } +/* + * RFC 9051 SS9 sequence-set: (seq-number/seq-range) *("," seq-number/ + * seq-range) -- what every parse_seq_range() call site used to reject a + * comma for, rather than actually parse. Splits text on top-level commas + * (no nesting or quoting in this grammar, so a plain scan is exact) and + * parses each segment with parse_one_seq_range() above. Writes up to + * SEQSET_MAX_RANGES entries to ranges[] and the count to *nranges; + * returns -1 (with *errmsg set) if any segment is malformed, empty, or + * there are more segments than SEQSET_MAX_RANGES allows. + */ +int +parse_sequence_set(const char *text, struct seq_range ranges[SEQSET_MAX_RANGES], + uint32_t *nranges, const char **errmsg) +{ + const char *p; + uint32_t n = 0; + + if (text == NULL || *text == '\0') { + *errmsg = "empty sequence set"; + return (-1); + } + + for (p = text; ; ) { + const char *start = p; + char tok[32]; + size_t len; + + while (*p != '\0' && *p != ',') + p++; + len = (size_t)(p - start); + if (len == 0 || len >= sizeof(tok)) { + *errmsg = "invalid sequence set"; + return (-1); + } + if (n >= SEQSET_MAX_RANGES) { + *errmsg = "sequence set has too many comma-separated " + "ranges"; + return (-1); + } + memcpy(tok, start, len); + tok[len] = '\0'; + + if (parse_one_seq_range(tok, &ranges[n]) == -1) { + *errmsg = "invalid sequence set"; + return (-1); + } + n++; + + if (*p == '\0') + break; + p++; /* skip ',' */ + } + + *nranges = n; + return (0); +} + /* like strtok_r(str, " ", &savep), but space isn't a delimiter inside an unclosed '[' or '(' (RFC 9051 SS9 header-list) */ static char * fetch_att_tok(char *str, char **savep) @@ -266,6 +329,97 @@ 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). */ +static int +parse_body_peek_section_tok(const char *tok, uint32_t *attrs_inout, + char *section_part_out, size_t section_part_outsize, + int *has_partial_out, uint32_t *partial_start_out, + uint32_t *partial_count_out, int *degraded_out, const char **errmsg) +{ + const char *bracket_start = tok + strlen("BODY.PEEK["); + char *close; + char inner[SECTION_PART_MAX]; + const char *suffix; + + close = strchr(bracket_start, ']'); + if (close == NULL) { + *degraded_out = 1; + return (0); + } + if ((size_t)(close - bracket_start) >= sizeof(inner)) { + *degraded_out = 1; + return (0); + } + memcpy(inner, bracket_start, close - bracket_start); + inner[close - bracket_start] = '\0'; + suffix = close + 1; + + if (suffix[0] != '\0' && + parse_partial_suffix(suffix, has_partial_out, partial_start_out, + partial_count_out) == -1) { + *errmsg = "malformed range"; + return (-1); + } + + if (inner[0] == '\0') { + *attrs_inout |= MBOX_FETCH_BODY_WHOLE; + } else if (strcasecmp(inner, "TEXT") == 0) { + *attrs_inout |= MBOX_FETCH_BODY_TEXT; + } else if (section_part_valid(inner)) { + *attrs_inout |= MBOX_FETCH_BODY_PART; + if (strlcpy(section_part_out, inner, section_part_outsize) >= + section_part_outsize) { + *attrs_inout &= ~MBOX_FETCH_BODY_PART; + *degraded_out = 1; + return (0); + } + } else { + *degraded_out = 1; /* recognized shape, unsupported section (e.g. "2.1.TEXT") */ + 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). */ +static int +parse_body_peek_header_fields_tok(const char *tok, uint32_t *attrs_inout, + int *header_fields_not_out, char *header_fields_out, + size_t header_fields_outsize, char *header_fields_label_out, + size_t header_fields_label_outsize, const char **errmsg) +{ + size_t toklen = strlen(tok); + char inner[HEADER_FIELDS_LABEL_MAX]; + + if (toklen < strlen("BODY.PEEK[") + 1 || tok[toklen - 1] != ']') { + *errmsg = "malformed HEADER.FIELDS section"; + return (-1); + } + if (*attrs_inout & MBOX_FETCH_HEADER_FIELDS) + return (0); /* already captured one, ignore any further duplicates */ + + if (toklen - strlen("BODY.PEEK[") - 1 >= sizeof(inner)) { + *errmsg = "HEADER.FIELDS section too long"; + return (-1); + } + memcpy(inner, tok + strlen("BODY.PEEK["), + toklen - strlen("BODY.PEEK[") - 1); + inner[toklen - strlen("BODY.PEEK[") - 1] = '\0'; + + if (parse_header_fields_att(inner, header_fields_not_out, + header_fields_out, header_fields_outsize) == -1) { + *errmsg = "malformed HEADER.FIELDS section"; + return (-1); + } + if (strlcpy(header_fields_label_out, inner, + header_fields_label_outsize) >= header_fields_label_outsize) { + *errmsg = "HEADER.FIELDS section too long"; + return (-1); + } + *attrs_inout |= MBOX_FETCH_HEADER_FIELDS; + 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 */ int parse_fetch_atts(char *spec, uint32_t *attrs_out, int *degraded_out, @@ -341,86 +495,18 @@ parse_fetch_atts(char *spec, uint32_t *attrs_out, int } else if (strncasecmp(tok, "BODY.PEEK[", strlen("BODY.PEEK[")) == 0 && strncasecmp(tok, "BODY.PEEK[HEADER.FIELDS", strlen("BODY.PEEK[HEADER.FIELDS")) != 0) { - /* every other BODY.PEEK[...] shape: [], [TEXT], or [], optional <> (SS6.4.5) */ - const char *bracket_start = tok + - strlen("BODY.PEEK["); - char *close; - char inner[SECTION_PART_MAX]; - const char *suffix; - - close = strchr(bracket_start, ']'); - if (close == NULL) { - degraded = 1; /* not well-bracketed, same lenient skip as other unsupported forms */ - continue; - } - if ((size_t)(close - bracket_start) >= sizeof(inner)) { - degraded = 1; - continue; - } - memcpy(inner, bracket_start, close - bracket_start); - inner[close - bracket_start] = '\0'; - suffix = close + 1; - - if (suffix[0] != '\0' && - parse_partial_suffix(suffix, &has_partial, - &partial_start, &partial_count) == -1) { - *errmsg = "malformed range"; + if (parse_body_peek_section_tok(tok, &attrs, + section_part_out, section_part_outsize, + &has_partial, &partial_start, &partial_count, + °raded, errmsg) == -1) return (-1); - } - - if (inner[0] == '\0') { - attrs |= MBOX_FETCH_BODY_WHOLE; - } else if (strcasecmp(inner, "TEXT") == 0) { - attrs |= MBOX_FETCH_BODY_TEXT; - } else if (section_part_valid(inner)) { - attrs |= MBOX_FETCH_BODY_PART; - if (strlcpy(section_part_out, inner, - section_part_outsize) >= - section_part_outsize) { - attrs &= ~MBOX_FETCH_BODY_PART; - degraded = 1; - continue; - } - } else { - degraded = 1; /* recognized shape, unsupported section (e.g. "2.1.TEXT") */ - continue; - } } else if (strncasecmp(tok, "BODY.PEEK[HEADER.FIELDS", strlen("BODY.PEEK[HEADER.FIELDS")) == 0) { - /* prefix-matched (field-name list varies); a second HEADER.FIELDS item is silently ignored */ - size_t toklen = strlen(tok); - char inner[HEADER_FIELDS_LABEL_MAX]; - - if (toklen < strlen("BODY.PEEK[") + 1 || - tok[toklen - 1] != ']') { - *errmsg = "malformed HEADER.FIELDS section"; - return (-1); - } - if (attrs & MBOX_FETCH_HEADER_FIELDS) - continue; /* already captured one, ignore any further duplicates */ - - if (toklen - strlen("BODY.PEEK[") - 1 >= - sizeof(inner)) { - *errmsg = "HEADER.FIELDS section too long"; - return (-1); - } - memcpy(inner, tok + strlen("BODY.PEEK["), - toklen - strlen("BODY.PEEK[") - 1); - inner[toklen - strlen("BODY.PEEK[") - 1] = '\0'; - - if (parse_header_fields_att(inner, + if (parse_body_peek_header_fields_tok(tok, &attrs, header_fields_not_out, header_fields_out, - header_fields_outsize) == -1) { - *errmsg = "malformed HEADER.FIELDS section"; + header_fields_outsize, header_fields_label_out, + header_fields_label_outsize, errmsg) == -1) return (-1); - } - if (strlcpy(header_fields_label_out, inner, - header_fields_label_outsize) >= - header_fields_label_outsize) { - *errmsg = "HEADER.FIELDS section too long"; - return (-1); - } - attrs |= MBOX_FETCH_HEADER_FIELDS; } else if (strcasecmp(tok, "ENVELOPE") == 0) { attrs |= MBOX_FETCH_ENVELOPE; /* RFC 9051 SS7.5.2; no .PEEK variant, no \Seen side effect */ } else if (strcasecmp(tok, "BODY") == 0 || @@ -747,9 +833,24 @@ parse_fetch_modifiers(char *modspec, struct imsg_mbox_ "mod-sequence value"; return (-1); } + /* + * RFC 7162 SS7: chgsince-fetch-mod takes a + * mod-sequence-value, "1*DIGIT ... (1 <= n <= + * 9,223,372,036,854,775,807)". strtoull(3) accepts a + * leading sign, so "-1" would otherwise arrive as + * ULLONG_MAX with errno untouched and match nothing, + * silently. Same first-character-is-a-digit guard + * auth.c, index.c and listener.c's literal parser use. + */ + if (*valtok < '0' || *valtok > '9') { + *errmsg = "invalid CHANGEDSINCE mod-sequence"; + return (-1); + } errno = 0; req->changedsince = strtoull(valtok, &ep, 10); - if (*ep != '\0' || errno != 0) { + if (*ep != '\0' || errno != 0 || + req->changedsince == 0 || + req->changedsince > MODSEQ_MAX) { *errmsg = "invalid CHANGEDSINCE mod-sequence"; return (-1); } @@ -791,8 +892,9 @@ fetch_dispatch(struct session *s, const char *tag, cha struct imsg_mbox_fetch req; const char *seqtok; char *attspec, *modspec; - uint32_t lo, hi, attrs; - int lo_star, hi_star, rc, want_vanished = 0, degraded; + struct seq_range ranges[SEQSET_MAX_RANGES]; + uint32_t nranges, attrs; + int rc, want_vanished = 0, degraded; int header_fields_not = 0; char header_fields[HEADER_FIELDS_MAX]; char header_fields_label[HEADER_FIELDS_LABEL_MAX]; @@ -822,18 +924,11 @@ fetch_dispatch(struct session *s, const char *tag, cha args++; attspec = args; - if (strchr(seqtok, ',') != NULL) { - session_reply(s, tag, "BAD", - "comma-separated sequence sets not supported " - "issue separate FETCH commands"); + if (parse_sequence_set(seqtok, ranges, &nranges, &errmsg) == -1) { + session_reply(s, tag, "BAD", errmsg); return (1); } - if (parse_seq_range(seqtok, &lo, &hi, &lo_star, &hi_star) == -1) { - session_reply(s, tag, "BAD", "invalid sequence set"); - return (1); - } - modspec = split_trailing_modifiers(attspec); rc = parse_fetch_atts(attspec, &attrs, °raded, &header_fields_not, @@ -950,10 +1045,7 @@ fetch_dispatch(struct session *s, const char *tag, cha return (1); } - req.seq_lo = lo; - req.seq_hi = hi; - req.lo_is_star = lo_star; - req.hi_is_star = hi_star; + req.nranges = nranges; /* RFC 7162 SS3.1: MODSEQ fetch-att and CHANGEDSINCE modifier are both CONDSTORE-enabling */ if (req.attrs & MBOX_FETCH_MODSEQ) @@ -968,10 +1060,12 @@ fetch_dispatch(struct session *s, const char *tag, cha s->cmd_by_uid = by_uid; s->state = SESSION_FETCHING; - if (imsg_compose(&s->store_iev->ibuf, IMSG_MBOX_FETCH, 0, 0, -1, - &req, sizeof(req)) == -1) - log_warn("session %u: imsg_compose IMSG_MBOX_FETCH", s->id); - imsgev_add(s->store_iev); + if (!send_mbox_request(s, IMSG_MBOX_FETCH, cmdname, "IMSG_MBOX_FETCH", + &req, sizeof(req), ranges, nranges, sizeof(struct seq_range))) { + session_reply(s, tag, "NO", "[SERVERBUG] internal error"); + s->state = SESSION_SELECTED; + return (1); + } return (1); } blob - f0352b50e26676e1d971d79906d320fb43b27b6a blob + ec0bf31d729d076828fe70cb87dd5dd50c2dd6a8 --- 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: August 16 2026 $ +.Dd $Mdocdate: September 6 2026 $ .Dt IMAPD 8 .Os .Sh NAME @@ -31,7 +31,7 @@ re-executes unprivileged and .Em auth children over -.Xr imsg 3 +.Xr imsg_init 3 control channels; a .Em store child is forked per authenticated session, chroots into the mail @@ -49,6 +49,15 @@ and port 993 .Pq implicit TLS, per RFC 8314 , both via .Xr tls_init 3 . +Those are the defaults, used when +.Pa /etc/imapd.conf +contains no +.Ic listen +directive at all. +A file that contains any +.Ic listen +directive selects listeners as well as configuring them: only the +listeners it names are bound. Authentication is .Li AUTH=PLAIN only, and is refused before TLS is established; @@ -253,6 +262,22 @@ a second .Ic listen line naming a different address is a configuration error. .Pp +A +.Ic listen +directive also selects which listeners run. +If the file contains no +.Ic listen +directive, both listeners are bound on their default ports. +If it contains any, only the listeners it names are bound: a file whose +only +.Ic listen +directive is +.Pp +.Dl listen on * tls port 993 +.Pp +serves implicit TLS alone, with nothing on port 143, which is the +deployment RFC 8314 Section 3 asks for. +.Pp .Ar address must be a literal IPv4 address, a literal IPv6 address, .Ql :: @@ -267,7 +292,9 @@ Hostnames are not accepted: resolving one would add a .Nm Ns 's boot path for no benefit a single-operator personal mail server actually needs. -Since OpenBSD's IPv6 sockets are always IPv6-only +Since +.Ox Ns 's +IPv6 sockets are always IPv6-only .Pq no v4-mapped-address dual binding , unlike Linux , .Ql * binds two sockets per listener, one @@ -281,6 +308,31 @@ Mail spool root, chrooted into by the child. Defaults to .Pa /var/mail/imapd . +.Ar path +and everything directly under it +.Pq one entry per mail user's maildir +must be owned by +.Sy root +and not writable by mail users. +Each user's maildir path under +.Ar path +is validated lexically only; if it names a symlink, the +.Em store +child's +.Xr chroot 2 +follows it. +That is contained: +.Xr chroot 2 +resolves the target inside itself, and +.Xr unveil 2 +in the +.Em store +child narrows the view further. +A mail user able to create a symlink at their own maildir path would +nonetheless choose that child's starting directory within the spool. +The ownership requirement above is therefore a deployment assumption +.Nm +does not enforce in software. .It Ic credentials Ar path Credentials file (see below). Defaults to @@ -294,6 +346,95 @@ 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. +.It Ic idle poll Ar seconds +How often a session in +.Li IDLE +rechecks its selected mailbox. +.Nm +does not receive an event when mail arrives, so +.Li IDLE +is served by polling: every +.Ar seconds , +the session asks its +.Em store +child whether anything has changed, and reports new messages and +expunges if so. +New mail is therefore announced within one interval rather than +instantly, whether it was delivered by an external +.Xr smtpd 8 +or by another IMAP session. +.Pp +Polling is cheap by design. +The +.Em store +child first compares the modification times of the mailbox directory +and its +.Pa new +subdirectory against the previous look; if neither has moved it answers +immediately, taking no lock and reading nothing. +Only a mailbox that has actually been touched costs an index read, and +only one holding an unindexed delivery costs an exclusive lock. +.Pp +.Ar seconds +must be between 1 and 300 inclusive, or 0 to disable polling +altogether. +With polling disabled a session in +.Li IDLE +is told nothing until it sends +.Li DONE , +which is rarely what an operator wants. +Defaults to 5. +.It Ic startups begin Ar count Ic rate Ar percent Ic full Ar count +Admission control on concurrent connections that have not yet +authenticated, using +.Xr sshd_config 5 Ns 's +MaxStartups algorithm; see that manual for the canonical description. +.Pp +Below +.Ic begin +unauthenticated connections, +.Nm +accepts every new connection. +Between +.Ic begin +and +.Ic full +it refuses new connections with a probability rising linearly from +.Ic rate +percent at +.Ic begin +to 100 percent at +.Ic full . +At or above +.Ic full +every new connection is refused. +A connection stops counting the moment it authenticates, so the limit +bounds unfinished logins rather than established sessions. +.Pp +Both counts must be between 0 and 1000000 inclusive, +.Ar percent +between 0 and 100 inclusive, and +.Ic full +must not be smaller than +.Ic begin ; +anything else is a configuration error. +Setting +.Ic full +to 0 disables the throttle entirely and accepts unconditionally, +which is the only way to turn it off: +.Ic begin +and +.Ic rate +have no such special value. +Defaults to +.Ic begin +10 +.Ic rate +30 +.Ic full +100, matching +.Xr sshd_config 5 Ns 's +own default of 10:30:100. .It Ic attachment max Ar bytes Largest message .Nm @@ -363,6 +504,31 @@ The child .Xr chroot 2 Ns s into this directory before dropping privileges. +.It Pa imapd.uidvalidity +Per-user record, at the root of each maildir, of the highest +.Li UIDVALIDITY +ever issued to that user. +A mailbox that is created, or recreated after a +.Li DELETE , +is given a value greater than every value in this file rather than the +current time alone, so that a mailbox recreated quickly cannot be handed +a +.Li UIDVALIDITY +it has used before. +RFC 9051 Section 2.3.1.1 requires that; a bare timestamp only +approximates it, because two mailboxes created in the same second get +the same value. +.Pp +The file is created on demand, holds one decimal number, and is its own +lock. +It is removed with the maildir and needs no separate administration. +Note that +.Xr imapduser 8 Fl d +deliberately leaves a maildir in place, so an account removed and +re-added keeps its history here, which is the desired behaviour; +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. .El .Sh NETWORK .Nm @@ -377,9 +543,12 @@ by default; these, along with the listen address, are .Pa /etc/imapd.conf (or the file named by .Fl f ) -at startup, falling back to defaults for any +at startup. +A file containing no .Ic listen -directive the file omits. +directive leaves both listeners on those defaults; a file containing any +.Ic listen +directive binds only the listeners it names. IPv6 and dual-stack binding are available but not the default; see the .Ic listen on directive under FILES above. @@ -388,6 +557,7 @@ directive under FILES above. .Xr imsg_init 3 , .Xr tls_init 3 , .Xr httpd.conf 5 , +.Xr sshd_config 5 , .Xr httpd 8 , .Xr imapduser 8 , .Xr smtpd 8 @@ -408,10 +578,27 @@ directive under FILES above. .%R RFC 7162 .%T IMAP Extensions: Quick Flag Changes Resynchronization (CONDSTORE) and Quick Mailbox Resynchronization (QRESYNC) .Re +.Pp +.Rs +.%A F. Yergeau +.%D November 2003 +.%R RFC 3629 +.%T UTF-8, a transformation format of ISO 10646 +.Re +.Pp +.Rs +.%A J. Klensin +.%A M. Padlipsky +.%D March 2008 +.%R RFC 5198 +.%T Unicode Format for Network Interchange +.Re .Sh HISTORY .Nm is a from-scratch IMAP server written for the OpenIMAPD project, in -the OpenBSD privilege-separation tradition of +the +.Ox +privilege-separation tradition of .Xr smtpd 8 . .Sh CAVEATS This implementation is under active development. @@ -420,3 +607,87 @@ This implementation is under active development. and shared or multi-user mailboxes .Pq no Li ACL support are deliberately left out. +.Pp +Mailbox names are required to be well-formed UTF-8. +RFC 9051 Section 5.1 encodes them in Net-Unicode +.Pq RFC 5198 , +and requires a server to prohibit the creation of 8-bit names that do not +comply. +.Nm +enforces three of Net-Unicode's requirements and not the other three, which +is worth stating plainly rather than leaving to be discovered: +.Bl -bullet -offset indent -compact +.It +UTF-8 well-formedness per RFC 3629 is enforced. +Overlong encodings, UTF-16 surrogates and anything above U+10FFFF are +refused. +.It +The C1 controls U+0080 to U+009F are refused, as Net-Unicode requires. +C0 and DEL were already refused, which RFC 9051 Section 5.1 permits. +.It +U+FEFF is refused anywhere in a name. +Net-Unicode forbids it only at the beginning; +.Nm +is deliberately stricter, because a zero-width no-break space inside a name +produces two mailboxes no user can tell apart. +.It +Normalization to NFC is +.Em not +performed and +.Em not +required. +Two canonically equivalent spellings of the same name, such as U+00E9 and +U+0065 U+0301, are therefore two distinct mailboxes with distinct +.Li UIDVALIDITY +values. +A client that normalizes differently from another client used on the same +account will see both. +.It +Names are +.Em not +checked against a Unicode version, so a name containing an unassigned code +point is accepted. +Net-Unicode forbids that, and honouring it would require a Unicode character +database inside the daemon. +.El +.Pp +A name that fails these checks is refused with +.Li NO [CANNOT] +by +.Li CREATE , +.Li RENAME +and +.Li COPY Ns / Ns Li MOVE , +and reported as nonexistent by the commands that only ask whether a mailbox +is there. +A directory whose name fails them is skipped by +.Li LIST +and cannot be selected; the first such skip in a connection is logged, naming +the directory. +This last case can only arise for a mailbox created before these checks +existed, or created outside +.Nm +altogether. +.Pp +RFC 9051 Section 6.3.5 does not say what becomes of a session that +.Li DELETE Ns s +the mailbox it currently has selected, and states no condition under which +such a +.Li DELETE +must be refused. +.Nm +allows it and returns the session to the authenticated state, as +.Li CLOSE +does: the mailbox no longer exists, so nothing is selected. +A client that issues +.Li FETCH , +.Li STORE , +.Li SEARCH , +.Li COPY , +.Li MOVE +or +.Li EXPUNGE +afterwards is refused until it selects a mailbox again, rather than being +served +.Li INBOX +under the deleted mailbox's name. blob - 990a13c9f6bb93fc272d9a2c6c0f30821042744d blob + 3488e7c9336f34c6c5d5a011a449644f91959cfa --- src/imapd.conf.example +++ src/imapd.conf.example @@ -58,3 +58,23 @@ listen on 0.0.0.0 tls port 993 # imapd.h for the full rationale). Uncomment and adjust if your mail # routinely carries larger attachments than that. attachment max 41943040 + +# 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 +# external MTA or by another IMAP session. Most polls cost two stat(2) +# calls and no lock, so a short interval is cheap. Must be 1-300 seconds, +# or 0 to disable pushing entirely (an IDLEing session is then told +# nothing until it sends DONE). Defaults to 5. +#idle poll 5 + +# 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). +# Below "begin" open connections, every new connection is +# accepted normally. Between "begin" and "full", new connections +# are refused with linearly increasing probability, starting at +# "rate" percent at "begin" and reaching 100% at "full". At or +# above "full", every new connection is refused outright. +# Defaults to sshd_config(5)'s own default, 10:30:100. +#startups begin 10 rate 30 full 100 blob - 7ebe46f03770374d5b2bb7631a129c716bc566cb blob + 0cf2479b94bc7d44cefac9c27ab52e1c0ccfd64e --- src/imapd.h +++ src/imapd.h @@ -24,12 +24,14 @@ #include #include /* __dead */ #include +#include /* struct sockaddr_storage, socklen_t -- + * imsg_listener_session_init below */ #include #include #include -#define IMAPD_VERSION "0.1.1" +#define IMAPD_VERSION "0.1.3" /* * Process roles, selected at exec time via "-x ". See main.c. @@ -38,7 +40,11 @@ enum openimap_proc_type { PROC_PARENT, PROC_LISTENER, PROC_AUTH, - PROC_STORE + PROC_STORE, + PROC_KEYMGR, + PROC_SEARCH /* SS8: per-connection SEARCH-grammar + * parsing oracle, docs/openimap-tls-privsep- + * design.md SS8.1 */ }; /* @@ -51,14 +57,28 @@ enum imsg_type { IMSG_SETUP_PEER, IMSG_SETUP_DONE, - /* parent -> listener, at boot */ - IMSG_LISTENER_SOCKET_CLEARTEXT, /* one bound, listening fd for the - * cleartext/STARTTLS port */ - IMSG_LISTENER_SOCKET_TLS, /* same, for the implicit-TLS port */ - IMSG_TLS_CERT, - IMSG_TLS_KEY, - IMSG_LISTENER_INIT, + 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 -> 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. + */ + IMSG_LISTENER_SESSION_INIT, + /* parent -> auth, at boot */ IMSG_AUTH_INIT, @@ -66,18 +86,61 @@ 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. + */ + 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. + */ + 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. */ + 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. */ + IMSG_KEYMGR_RSA_PRIVENC, + IMSG_KEYMGR_RSA_PRIVDEC, + IMSG_KEYMGR_ECDSA_SIGN, + /* auth -> parent, per successful login */ IMSG_AUTH_CRED, /* per-session store spawn */ IMSG_STORE_FORK, IMSG_STORE_INIT, - IMSG_STORE_PEER, IMSG_STORE_SHUTDOWN, /* listener <-> store, once a session's store child is wired up */ - IMSG_MBOX_SELECT, - IMSG_MBOX_EXAMINE, + IMSG_MBOX_SELECT, /* EXAMINE too: it is a SELECT with the + * request's "readonly" field set, see + * select_or_examine() in mailbox_cmd.c */ IMSG_MBOX_SELECTED, IMSG_MBOX_FETCH, IMSG_MBOX_FETCH_META, @@ -108,7 +171,6 @@ enum imsg_type { IMSG_MBOX_DELETE, IMSG_MBOX_RENAME, IMSG_MBOX_RESULT, - IMSG_MBOX_UNSOLICITED, /* * RFC 7162 (CONDSTORE/QRESYNC) additions @@ -153,8 +215,18 @@ 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 */ - uint16_t port_cleartext; /* 143, STARTTLS */ - uint16_t port_implicit_tls; /* 993, RFC 8314 */ + /* + * 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. + */ + 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 */ char cred_file[1024]; /* auth's credential file, one * line per user, format @@ -163,6 +235,23 @@ struct openimap_config { char tls_cert_file[1024]; char tls_key_file[1024]; uint32_t bodystructure_read_max; /* "attachment 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". */ + 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(). + */ + uint32_t idle_poll_secs; }; /* @@ -175,20 +264,76 @@ struct openimap_config { /* * Boot-time config-delivery payloads. */ -struct imsg_listener_init { - char listen_addr[64]; - uint16_t port_cleartext; - uint16_t port_implicit_tls; - uint8_t n_cleartext_addrs; /* # of IMSG_LISTENER_SOCKET_ - * CLEARTEXT messages to expect, - * 1 or LISTENER_MAX_ADDRS */ - uint8_t n_tls_addrs; /* same, for _TLS */ +/* + * 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). + */ +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. + */ + uint32_t idle_poll_secs; }; struct imsg_auth_init { char cred_file[1024]; }; +/* + * 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. + * + * 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. + */ +#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 */ +}; + +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 */ +}; + struct imsg_auth_request { uint32_t session_id; char username[AUTH_USERNAME_MAX]; @@ -267,10 +412,13 @@ struct imsg_mbox_select { int qresync_has_uids; /* 0 = client omitted known-uids; * store.c then defaults to the full * UID range (SS3.2.5.1) */ - uint32_t qresync_uid_lo; /* known-uids range, single range only - * (no comma lists); ignored if + uint32_t qresync_nranges; /* count of the trailing struct + * seq_range array (RFC 9051 SS9 + * sequence-set; "*" is forbidden in + * known-uids per SS3.2.5.1, already + * rejected by parse_qresync_group()); + * ignored (and 0) if * !qresync_has_uids */ - uint32_t qresync_uid_hi; }; struct imsg_mbox_selected { @@ -430,8 +578,15 @@ struct imsg_mbox_select_vanished { /* * 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. listener.c rejects (BAD) - * an oversized section-part rather than truncating it. + * 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.) */ #define SECTION_PART_MAX 40 @@ -499,6 +654,18 @@ struct imsg_mbox_select_vanished { #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. + */ +#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 @@ -519,15 +686,42 @@ struct imsg_mbox_select_vanished { */ #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. + */ +struct seq_range { + uint32_t lo; /* 1-based, inclusive; ignored if lo_is_star */ + uint32_t hi; /* 1-based, inclusive; ignored if hi_is_star */ + int lo_is_star; + 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. + */ +#define SEQSET_MAX_RANGES 500 + struct imsg_mbox_fetch { - uint32_t seq_lo; /* 1-based, inclusive; ignored if - * lo_is_star */ - uint32_t seq_hi; /* 1-based, inclusive; ignored if - * hi_is_star */ - int lo_is_star; - int hi_is_star; /* "*" resolves against store's live - * message count, not listener's - * possibly-stale SELECT-time count */ + uint32_t nranges; /* count of the trailing struct + * seq_range array (RFC 9051 SS9 + * sequence-set); "*" resolves + * against store's live message + * count, not listener's possibly- + * stale SELECT-time count */ uint32_t attrs; /* bitmask of MBOX_FETCH_* above */ /* RFC 7162 SS3.1.4.1 CHANGEDSINCE; has_changedsince distinguishes @@ -535,8 +729,9 @@ struct imsg_mbox_fetch { int has_changedsince; uint64_t changedsince; - /* by_uid: RFC 9051 SS6.4.9 UID FETCH, resolve seq_lo/seq_hi - * against UID space. listener.c separately forces MBOX_FETCH_UID + /* by_uid: RFC 9051 SS6.4.9 UID FETCH, resolve the trailing + * sequence-set ranges against UID space rather than sequence- + * number space. listener.c separately forces MBOX_FETCH_UID * into attrs whenever this is set. * * want_vanished: RFC 7162 SS3.2.6 VANISHED modifier (only legal @@ -757,10 +952,13 @@ struct imsg_mbox_result { #define MBOX_STORE_REMOVE 2 /* -FLAGS, subtract out */ struct imsg_mbox_store { - uint32_t seq_lo; - uint32_t seq_hi; - int lo_is_star; - int hi_is_star; + uint32_t nranges; /* count of the trailing struct + * seq_range array (RFC 9051 SS9 + * sequence-set), same header-plus- + * trailing-array shape as + * imsg_mbox_fetch above; always >= 1, + * STORE always requires a + * sequence-set */ int mode; /* MBOX_STORE_* above */ int silent; /* 1 = ".SILENT", suppress the * untagged FETCH per message */ @@ -813,17 +1011,17 @@ struct imsg_mbox_store_modified { struct imsg_mbox_expunge { int silent; /* 1 for CLOSE, 0 for a real EXPUNGE command */ - /* RFC 9051 SS6.4.9's UID EXPUNGE form: only \Deleted messages whose - * UID is in [seq_lo, seq_hi] are removed. Never set together with - * silent=1 (no "UID CLOSE"). seq_lo/seq_hi/lo_is_star/hi_is_star - * mirror imsg_mbox_fetch's naming; meaningless when !by_uid (plain - * EXPUNGE takes no arguments). + /* RFC 9051 SS6.4.9's UID EXPUNGE form: only \Deleted messages in + * the trailing struct seq_range array (nranges entries, same + * header-plus-trailing-array shape as imsg_mbox_fetch above) are + * removed. Never set together with silent=1 (no "UID CLOSE"). + * Meaningless when !by_uid (plain EXPUNGE/CLOSE take no + * arguments) -- nranges is 0 and there's no trailing data in + * that case, unlike every other sequence-set-bearing imsg here, + * which always require nranges >= 1. */ int by_uid; - uint32_t seq_lo; - uint32_t seq_hi; - int lo_is_star; - int hi_is_star; + uint32_t nranges; }; struct imsg_mbox_expunged { @@ -853,10 +1051,12 @@ struct imsg_mbox_expunged { */ struct imsg_mbox_copy { int by_uid; - uint32_t seq_lo; - uint32_t seq_hi; - int lo_is_star; - int hi_is_star; + uint32_t nranges; /* count of the trailing struct + * seq_range array, same header-plus- + * trailing-array shape as + * imsg_mbox_fetch above; always >= 1, + * COPY/MOVE always require a + * sequence-set */ char destname[MBOX_NAME_MAX]; }; @@ -934,7 +1134,7 @@ struct imsg_mbox_appended { * directly as COUNT if requested). * * RFC 9051 SS6.4.4 SEARCH's search-key grammar nests arbitrarily, so it - * can't be a single fixed-size struct. listener.c's parse_search_key()/ + * can't be a single fixed-size struct. search_cmd.c's parse_search_key()/ * parse_search_key_list() compile the whole search-program into a flat * postfix array of struct search_node, sent as variable-length trailing * data after a small fixed header, same imsg_get_buf()/imsg_get_len() @@ -1005,6 +1205,47 @@ struct imsg_mbox_search { * in this imsg's trailing data */ }; +/* + * IMSG_SEARCH_PARSE_REQUEST's raw trailing bytes (already- + * buffered SEARCH argument text, post RETURN/CHARSET) are sized + * against this rather than left unbounded: generously bigger + * than any single argument list could legitimately be, since + * the whole command line it was sliced from is already capped + * at listener.h's 8192-byte SESSION_INBUF_MAX. + */ +#define SEARCH_ORACLE_ARGS_MAX 8192 + +/* + * IMSG_SEARCH_PARSE_RESULT (search-oracle -> listener): reply to + * IMSG_SEARCH_PARSE_REQUEST, echoing parse_search_key_list()'s own + * (rc, errmsg) contract (search_cmd.c) -- rc == 0: nnodes valid, + * struct search_node[nnodes] trails, same wire shape + * imsg_mbox_search above already uses; errmsg unused. rc == -1: + * BAD, errmsg set, nnodes/trailing data unused. rc == -2: NO, + * errmsg set, same. Every errmsg parse_search_key_list() and its + * helpers produce is a static string literal (search_cmd.c has no + * runtime-formatted SEARCH parse error), so + * SEARCH_ORACLE_ERRMSG_MAX only needs to cover the longest one, + * with headroom. uses_modseq mirrors struct search_parse_ctx's + * own field of the same name (search_cmd.c, opaque to every + * caller outside that file) -- RFC 7162 SS3.1: a SEARCH + * including the MODSEQ data item is a CONDSTORE-enabling + * command. Valid only when rc == 0, same as nnodes; a + * rejected parse never reaches search_dispatch()'s CONDSTORE + * check. + */ +#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 */ + int uses_modseq; /* valid when rc == 0, see this + * struct's own comment */ + char errmsg[SEARCH_ORACLE_ERRMSG_MAX]; /* valid when + * rc != 0 */ +}; + struct imsg_mbox_search_match { uint32_t seqno; uint32_t uid; @@ -1024,13 +1265,26 @@ struct imsg_mbox_search_match { * message, ascending UID order) / IMSG_MBOX_IDLE_REFRESHED (store -> * listener, terminal). * - * Used two ways by listener.c: once, synchronously after "+ idling", to - * seed s->idle_known_uids with a baseline; and again whenever - * session_notify_idle_peers() (triggered by another session's - * EXPUNGE/APPEND/MOVE) asks an idling session to recheck. Both reuse the - * same shape; store.c just reports current state via the same - * refresh_index() helper handle_mbox_select() uses, so an idle-refresh - * is as fresh as a fresh SELECT. + * 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. + * + * 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. + * + * 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. */ struct imsg_mbox_idle_uid { uint32_t uid; @@ -1038,6 +1292,16 @@ struct imsg_mbox_idle_uid { 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. + */ + int unchanged; uint32_t exists; uint32_t uidvalidity; uint32_t uidnext; @@ -1100,23 +1364,54 @@ __dead void parent_main(const char *, int, char *[], /* listener.c / auth.c / store.c take no struct openimap_config *, each * gets exactly the config it needs over its fd-3 channel instead: - * IMSG_LISTENER_INIT, IMSG_AUTH_INIT, IMSG_STORE_INIT respectively. + * IMSG_LISTENER_SESSION_INIT, IMSG_AUTH_INIT, IMSG_STORE_INIT respectively. + * search_oracle.c needs no config at all -- see its own comment. */ __dead void listener_main(void); __dead void auth_main(void); __dead void store_main(void); +__dead void keymgr_main(void); +__dead void search_oracle_main(void); +/* + * search_oracle.c's one entry point into search_cmd.c's otherwise- + * private struct search_parse_ctx, see search_oracle_parse()'s own + * comment (search_cmd.c). Declared here, not in listener.h, on + * purpose: search_oracle.c is a separate role, not part of listener.h's + * listener.c/auth_cmd.c/mailbox_cmd.c/append_cmd.c/fetch_cmd.c/ + * search_cmd.c/store_cmd.c/store_ipc.c family, and pulling that whole + * header in for one prototype is what caused search_oracle.c's own + * file-scope `static struct imsgev iev_parent` to collide with + * listener.h's unrelated `extern struct imsgev iev_parent` (listener's + * own long-lived fd-3 channel) -- same identifier, incompatible + * linkage, a real build failure on premio. search_cmd.c already + * includes this header too, so its own view of the prototype is + * unchanged by the move. + */ +int search_oracle_parse(char *, struct search_node *, uint32_t *, + int *, char *, size_t); + /* imsg helpers shared by all roles. The "handler" passed to * imsgev_init() is the libevent callback, expected to run its own - * imsgbuf_read()/imsg_get() loop (parent_dispatch_child() is the - * reference shape). + * imsgbuf_read()/imsgbuf_get() loop (parent_dispatch_child() is the + * reference shape), and to end with imsgev_rearm_read(). */ +void imsgev_ibuf_init(struct imsgbuf *, int); void imsgev_init(struct imsgev *, int, void (*)(int, short, void *), void *); void imsgev_init_from_ibuf(struct imsgev *, const struct imsgbuf *, void (*)(int, short, void *), void *); +/* You do NOT need to call this after an imsg_compose(). imsgev_init() installs + * imsgev_on_compose() as the channel's imsg close callback, so libutil arms + * EV_WRITE from inside imsg_close() on every queueing path. This is now only + * for the rare caller that must arm a channel it did not just compose on. */ void imsgev_add(struct imsgev *); +/* Same work as imsgev_add(), deliberately under a different name: this is + * the one a dispatch handler MUST call before returning. See its definition + * in imsgev.c for why the two are spelled apart. */ +void imsgev_rearm_read(struct imsgev *); + /* Boot-time setup-loop helpers: a freshly exec'd child blocks reading * fd 3 for zero or more IMSG_SETUP_PEER messages, then IMSG_SETUP_DONE, * and acks. */ blob - 556721d06f0b054a849cf2b37d31b5a2697799e4 blob + 2aa0bb8c8aa051fa5f41d67e9576286e411585cf --- src/imsgev.c +++ src/imsgev.c @@ -26,6 +26,17 @@ /* * imsgev.c, shared wrapper around imsgbuf + event(3), used by * parent.c, listener.c, auth.c, and store.c. + * + * IMSG API VERSION. This tree calls the current libutil imsg interface -- + * imsgbuf_init()/imsgbuf_read()/imsgbuf_write()/imsgbuf_flush()/ + * imsgbuf_clear() and imsgbuf_get() -- and not the older imsg_init()/ + * imsg_read()/imsg_get() spellings. That is not cosmetic: OpenBSD kept + * imsg_get() for a while as a compatibility wrapper (it called + * imsgbuf_get() and translated a success into a byte count) and has since + * removed it, so a build against current headers fails to link on that + * symbol alone. imsgbuf_get() returns 1 for a message, 0 for none and -1 on + * error; every call site in this tree tests only the 0 and -1 cases, which + * the wrapper passed through unchanged, so the switch was exact. */ #include @@ -37,23 +48,95 @@ #include "imapd.h" #include "log.h" +/* + * The one place an imsgbuf gets its imapd-wide settings. Every channel in + * the daemon goes through here, including the five that each role builds by + * hand on fd 3 before the event loop exists -- they used to open-code two of + * these three lines and omit the third, which left the two ends of the + * parent<->child channel disagreeing about the size limit. + * + * On imsgbuf_set_maxsize(3): its argument is the maximum PAYLOAD, not the + * maximum message. It adds IMSG_HEADER_SIZE before storing (libutil's + * imsgbuf_set_maxsize()), and imsgbuf_init(3) has already installed + * MAX_IMSGSIZE as the whole-message limit. So this call RAISES the limit by + * IMSG_HEADER_SIZE rather than clamping it -- which is fine, but it means + * every channel has to make the same call or the ends differ by 16 bytes. + * + * On imsgbuf_allow_fdpass(3): the parent fd-passes on each of these channels + * at spawn time (setup_peer_send(), IMSG_LISTENER_SESSION_INIT), so every + * channel needs it. Note that the parent is the ONLY process that ever + * attaches a descriptor to an imsg -- see the pledge comments in auth.c, + * keymgr.c, listener.c, search_oracle.c and store.c. + */ void +imsgev_ibuf_init(struct imsgbuf *ibuf, int fd) +{ + if (imsgbuf_init(ibuf, fd) == -1) + fatal("imsgbuf_init"); + if (imsgbuf_set_maxsize(ibuf, MAX_IMSGSIZE) == -1) + fatal("imsgbuf_set_maxsize"); + imsgbuf_allow_fdpass(ibuf); +} + +/* + * Arm EV_WRITE for a channel that has just queued a message. + * + * libutil calls this from imsg_close() (imsg.c:397-399), which is the funnel + * every queueing path goes through: imsg_compose(), imsg_composev() and + * imsg_forward() all end there. So it fires once per message queued, on every + * path, and no call site can forget it. It replaces the 36 hand-written + * "imsgev_add() after every imsg_compose()" pairings this tree used to carry. + * + * imsgbuf_set_close_callback(3) and imsgbuf_set_userdata(3) arrived in + * OpenBSD commit 348f1fc0836b (2026-09-04) -- the same commit that removed + * imsg_get() -- explicitly "to replace the bad imsgev wrappers in various + * deamons". + * + * On the early return: a single FETCH can queue hundreds of messages, and + * imsgev_add() is event_del() + event_set() + event_add(). Once EV_WRITE is + * armed the rest of a batch has nothing to do. event_pending(3) is asked + * rather than iev->events because it reports what libevent actually holds and + * so cannot go stale; smtpd uses the same idiom (usr.sbin/smtpd/control.c:362). + * Auditing every early return in all 13 dispatch handlers found them all to be + * teardown paths, so iev->events would in fact have been safe here -- this is + * belt and braces, not a fix for a known hole. + */ +static void +imsgev_on_compose(struct imsgbuf *ibuf, void *arg) +{ + struct imsgev *iev = arg; + + (void)ibuf; + + if (event_pending(&iev->ev, EV_WRITE, NULL)) + return; + imsgev_add(iev); +} + +void imsgev_init(struct imsgev *iev, int fd, void (*handler)(int, short, void *), void *data) { - if (imsgbuf_init(&iev->ibuf, fd) == -1) - fatal("imsgbuf_init"); - imsgbuf_set_maxsize(&iev->ibuf, MAX_IMSGSIZE); + imsgev_ibuf_init(&iev->ibuf, fd); - /* every channel fd-passes something at some point; allow it always */ - imsgbuf_allow_fdpass(&iev->ibuf); - iev->handler = handler; iev->data = data != NULL ? data : iev; iev->events = EV_READ; event_set(&iev->ev, fd, iev->events, iev->handler, iev->data); event_add(&iev->ev, NULL); + + /* + * After event_set()/event_add(), because the callback touches iev->ev, + * and after imsgev_ibuf_init(), because imsgbuf_init() memset()s the + * whole imsgbuf and would wipe both of these. Deliberately NOT done in + * imsgev_ibuf_init() itself: the five roles call that on fd 3 before + * any event loop exists and then compose synchronously through + * setup_recv_done_and_ack(), where a callback would reach an event that + * has never been event_set(). + */ + 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) */ @@ -70,6 +153,10 @@ imsgev_init_from_ibuf(struct imsgev *iev, const struct event_set(&iev->ev, iev->ibuf.fd, iev->events, iev->handler, iev->data); event_add(&iev->ev, NULL); + + /* see imsgev_init(); the struct copy above carried ibuf3's NULLs */ + imsgbuf_set_userdata(&iev->ibuf, iev); + 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. */ @@ -86,7 +173,36 @@ imsgev_add(struct imsgev *iev) event_add(&iev->ev, NULL); } -/* blocks for one IMSG_SETUP_PEER, returns its fd-passed fd; imsg_get() checked before imsgbuf_read() to avoid coalesced-message stalls */ +/* + * Re-arm a channel's read event at the end of its libevent dispatch handler. + * + * imsgev_init() arms EV_READ without EV_PERSIST, so libevent drops the event + * once it has fired. Every dispatch handler in this daemon must therefore + * re-arm before returning, or that channel is never read again -- a hang + * rather than a crash, and not one any single test names. + * + * This is the SAME work as imsgev_add(), on purpose, under a second name. + * imsgev_add()'s other job -- arming EV_WRITE after an imsg_compose() -- now + * belongs to imsgev_on_compose(), which libutil calls from imsg_close(). The + * two jobs used to be spelled identically at 49 call sites, which is what made + * "delete the 36 the callback replaces" a change no reviewer could check by + * reading the diff. Naming them apart is what made that diff legible; it is + * not a behaviour change, and delegating rather than duplicating keeps it from + * becoming one. + * + * Note that it still arms EV_WRITE when output is queued, because a handler + * that composed a reply and is now returning needs exactly that. + * + * See docs/Opus-5-security-review-2/Opus-5-DESIGN-imsgev-retirement.md + * sections 3 and 6. + */ +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 */ int setup_recv_one_peer(struct imsgbuf *ibuf3) { @@ -95,8 +211,8 @@ setup_recv_one_peer(struct imsgbuf *ibuf3) int fd; for (;;) { - if ((n = imsg_get(ibuf3, &imsg)) == -1) - fatal("imsg_get"); + if ((n = imsgbuf_get(ibuf3, &imsg)) == -1) + fatal("imsgbuf_get"); if (n != 0) break; if ((n = imsgbuf_read(ibuf3)) == -1) @@ -116,7 +232,7 @@ 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: imsg_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) { @@ -124,8 +240,8 @@ setup_recv_done_and_ack(struct imsgbuf *ibuf3) ssize_t n; for (;;) { - if ((n = imsg_get(ibuf3, &imsg)) == -1) - fatal("imsg_get"); + if ((n = imsgbuf_get(ibuf3, &imsg)) == -1) + fatal("imsgbuf_get"); if (n != 0) break; if ((n = imsgbuf_read(ibuf3)) == -1) blob - 6d496e3ee37c7d87346833e35d43c6e96ce2e0d4 blob + c7edf6f62c43e5467c67983c0fd8ca81f21dc298 --- src/index.c +++ src/index.c @@ -36,7 +36,56 @@ #include "log.h" #include "store_internal.h" +/* + * The index is a colon-delimited, line-oriented text file, so nothing + * written into one of its fields may contain ':', CR or LF -- an LF + * especially, since it would make the next index_save() emit a second, + * fully attacker-chosen physical line. index_append() has always + * enforced this for the basename it writes; the sites that need a + * keywords field bypass index_append() and hand-build the line, so the + * rule lives here where every one of them can share it. + */ +/* + * RFC 7162 SS7: a mod-sequence is a "Positive unsigned 63-bit integer + * (1 <= n <= 9,223,372,036,854,775,807)". Values read back from the index + * are bounded by it, as the client-facing parsers in mailbox_cmd.c, + * fetch_cmd.c and store_cmd.c bound the ones read off the wire. + */ +#define INDEX_MODSEQ_MAX INT64_MAX + int +index_field_valid(const char *field) +{ + return (field != NULL && strpbrk(field, ":\r\n") == NULL); +} + +/* + * A basename read back OUT of the index is concatenated into "new/%s" / + * "cur/%s" and handed to open(2)/stat(2)/rename(2) (mime.c, mbox_store.c). + * unveil(2) stops such a path leaving the maildir, but nothing stops it + * moving around inside it, so the format's rules are enforced on load as + * well as on the write side that already refuses these. + */ +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 */ + if (!index_field_valid(basename)) + return (0); + /* excludes "", ".", "..", and dotfiles in one test */ + if (basename[0] == '\0' || basename[0] == '.') + return (0); + for (p = (const unsigned char *)basename; *p != '\0'; p++) { + if (*p == '/' || *p < 0x20 || *p == 0x7f) + return (0); + } + return (1); +} + +int index_load(int fd, struct mbox_index *idx) { FILE *fp; @@ -61,6 +110,28 @@ index_load(int fd, struct mbox_index *idx) } while (fgets(line, sizeof(line), fp) != NULL) { + /* + * fgets(3) silently splits a line longer than the buffer, + * and the remainder is then parsed as its own record. A + * filled buffer with no '\n' in it is ambiguous by itself + * -- it's also what a LEGAL maximum-length line looks like, + * since its own trailing '\n' doesn't fit in this read. + * Peek at the next byte to tell them apart: the line's own + * '\n' (or EOF, the last line in a file missing its final + * newline) means this was exactly one record; anything else + * means fgets(3) really did split it. + */ + if (strchr(line, '\n') == NULL && + strlen(line) == sizeof(line) - 1) { + int c = fgetc(fp); + + if (c != EOF && c != '\n') { + log_warnx("session %u: over-long line in %s, " + "refusing to parse the index", session_id, + STORE_INDEX_NAME); + goto fail; + } + } line[strcspn(line, "\n")] = '\0'; if (line[0] == '\0') continue; @@ -72,42 +143,65 @@ index_load(int fd, struct mbox_index *idx) if ((colon = strchr(line, ':')) == NULL) { log_warnx("session %u: malformed index " "header: %s", session_id, line); - fclose(fp); - return (-1); + goto fail; } *colon = '\0'; + /* + * Each of the three header fields is digits or + * nothing, for the reason index_parse_line() states + * below for the UID field: strtoul(3) and strtoull(3) + * accept leading whitespace and a sign, so "-1:1:1" + * would load a UIDVALIDITY of 4294967295 and " 5:1:1" + * a UIDVALIDITY of 5, neither of which this format can + * express. Same guard auth.c, listener.c and the three + * mod-sequence parsers 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) { log_warnx("session %u: malformed " "UIDVALIDITY: %s", session_id, line); - fclose(fp); - return (-1); + goto fail; } /* RFC 7162: optional third field HIGHESTMODSEQ; NULL means older two-field header, defaults to 1 */ if ((colon2 = strchr(colon + 1, ':')) != NULL) *colon2 = '\0'; + if (colon[1] < '0' || colon[1] > '9') { + log_warnx("session %u: malformed UIDNEXT: %s", + session_id, colon + 1); + goto fail; + } errno = 0; idx->uidnext = (uint32_t)strtoul(colon + 1, &ep, 10); if (*ep != '\0' || errno != 0) { log_warnx("session %u: malformed UIDNEXT: %s", session_id, colon + 1); - fclose(fp); - return (-1); + goto fail; } if (colon2 != NULL) { + if (colon2[1] < '0' || colon2[1] > '9') { + log_warnx("session %u: malformed " + "HIGHESTMODSEQ: %s", session_id, + colon2 + 1); + goto fail; + } errno = 0; idx->highestmodseq = strtoull(colon2 + 1, &ep, 10); - if (*ep != '\0' || errno != 0) { + if (*ep != '\0' || errno != 0 || + idx->highestmodseq > INDEX_MODSEQ_MAX) { log_warnx("session %u: malformed " "HIGHESTMODSEQ: %s", session_id, colon2 + 1); - fclose(fp); - return (-1); + goto fail; } } else idx->highestmodseq = 1; @@ -122,33 +216,40 @@ index_load(int fd, struct mbox_index *idx) if (newlines == NULL) { log_warn("session %u: reallocarray index", session_id); - fclose(fp); - return (-1); + goto fail; } idx->lines = newlines; idx->cap = newcap; } if ((idx->lines[idx->nlines] = strdup(line)) == NULL) { log_warn("session %u: strdup index line", session_id); - fclose(fp); - return (-1); + goto fail; } idx->nlines++; } if (ferror(fp)) { log_warn("session %u: fgets %s", session_id, STORE_INDEX_NAME); - fclose(fp); - return (-1); + goto fail; } fclose(fp); if (first) { - idx->uidvalidity = (uint32_t)time(NULL); + idx->uidvalidity = uidvalidity_next(); idx->uidnext = 1; idx->highestmodseq = 1; + idx->fresh = 1; /* caller must persist; see the field */ } return (0); + +fail: + /* idx may hold a partial set of already-allocated lines at this + * point; index_free() is a safe no-op if it doesn't. Every caller + * (refresh_index() included) documents/relies on "-1 means idx is + * already freed" -- this is what makes that true. */ + 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. */ @@ -160,9 +261,19 @@ index_parse_line(const char *line, struct index_rec *r memset(rec, 0, sizeof(*rec)); + /* + * strtoul(3) accepts leading whitespace and a sign, so ":x:y:1" + * would parse as UID 0 and "-1:x:y:1" as UID 4294967295. The field + * is 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); + } errno = 0; rec->uid = (uint32_t)strtoul(line, &ep, 10); - if (*ep != ':') { + if (*ep != ':' || errno != 0) { log_warnx("session %u: corrupt index line: %s", session_id, line); return (-1); @@ -180,6 +291,20 @@ index_parse_line(const char *line, struct index_rec *r } memcpy(rec->basename, p, (size_t)(q - p)); rec->basename[q - p] = '\0'; + /* + * About to be pasted into "new/%s" / "cur/%s" and passed to + * open(2)/stat(2)/rename(2) by every caller. Refuse traversal, + * hidden names and control bytes here rather than relying on + * unveil(2) to catch the ones that would leave the maildir -- it + * does nothing about the ones that stay inside it. Logged by UID, + * not by basename: the basename is exactly the untrusted text that + * should not reach syslog raw. + */ + if (!index_basename_valid(rec->basename)) { + log_warnx("session %u: refusing index line with unsafe " + "basename (UID %u)", session_id, rec->uid); + return (-1); + } p = q + 1; if ((r = strchr(p, ':')) != NULL) { @@ -192,9 +317,25 @@ index_parse_line(const char *line, struct index_rec *r memcpy(rec->keywords, p, (size_t)(r - p)); rec->keywords[r - p] = '\0'; + /* + * Same rule as the UID field above, which this function has + * always enforced -- the MODSEQ field twelve lines down did + * not get it. RFC 7162 SS7 bounds a mod-sequence at + * 9,223,372,036,854,775,807, and strtoull(3)'s sign handling + * would otherwise turn "-1" into 18446744073709551615 with + * errno untouched, a value that then flows into CHANGEDSINCE + * and UNCHANGEDSINCE comparisons and out to the client as a + * MODSEQ FETCH item. + */ + if (r[1] < '0' || r[1] > '9') { + log_warnx("session %u: malformed per-message MODSEQ " + "in index line: %s", session_id, line); + return (-1); + } errno = 0; rec->modseq = strtoull(r + 1, &ep, 10); - if (*ep != '\0' || errno != 0) { + if (*ep != '\0' || errno != 0 || + rec->modseq > INDEX_MODSEQ_MAX) { log_warnx("session %u: malformed per-message MODSEQ " "in index line: %s", session_id, line); return (-1); @@ -232,6 +373,104 @@ index_max_uid(struct mbox_index *idx) return (v); } +/* + * Resolves every "*" in a parsed sequence-set (an array of struct + * seq_range, e.g. from IMSG_MBOX_FETCH's trailing array) against max -- + * the store's live index_max_uid() for a by-UID request, idx->nlines + * for a sequence-number request, same values handle_mbox_fetch() and + * friends have always resolved "*" against. + * + * RFC 9051 SS9: a seq-range is unordered ("the first sequence number + * may be smaller or larger than the second"). parse_one_seq_range() + * (fetch_cmd.c) already swaps a backwards LITERAL range at parse time, + * but skips the swap when either side is "*", since only this function + * knows what "*" resolves to -- so e.g. "5:*" on a 2-message mailbox + * arrives here as lo=5, hi=2 and is swapped to lo=2, hi=5 below, same + * as mbox_copy.c's COPY/MOVE handling has always done for this case + * (formerly duplicated there, now centralized here so every caller, + * including FETCH/STORE/UID EXPUNGE, gets the same RFC-correct + * behavior for a backwards "*"-involving range). + * + * The swap runs before the clamps below: lo is then clamped up to at + * least 1, and hi is additionally clamped down to max when clamp_hi is + * set, matching this codebase's existing per-mode behavior (sequence + * numbers can never legitimately exceed idx->nlines, but an explicit + * (non-"*") UID above the highest UID in use is left alone rather than + * clamped, since it's simply a range that won't match anything past + * the last message). Every range is kept, even one still degenerate + * after the swap and clamps (possible only when max itself is 0, i.e. + * an empty mailbox, e.g. "*:*" resolving to lo=1 hi=0 after the lo<1 + * clamp) -- seqset_contains() below correctly treats lo > hi as "never + * matches", and every caller's own scan is bounded by the same empty + * idx->nlines/no-UIDs-in-use condition, so this can't cause an + * incorrect match, only a harmless unmatchable entry. Ranges are not + * merged or sorted -- SEQSET_MAX_RANGES already bounds the count, so + * letting seqset_contains() below do one full pass per resolved range + * at each membership test is cheap enough not to be worth a merge + * step. Returns the number of ranges written to resolved[] (always + * nranges; unlike before this function grew the swap, no range is + * ever dropped). + */ +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]) +{ + uint32_t i, n = 0, lo, hi; + + for (i = 0; i < nranges; i++) { + lo = ranges[i].lo_is_star ? max : ranges[i].lo; + hi = ranges[i].hi_is_star ? max : ranges[i].hi; + if (lo > hi) { + uint32_t tmp = lo; + + lo = hi; + hi = tmp; + } + if (lo < 1) + lo = 1; + if (clamp_hi && hi > max) + hi = max; + resolved[n].lo = lo; + resolved[n].hi = hi; + resolved[n].lo_is_star = resolved[n].hi_is_star = 0; + n++; + } + return (n); +} + +/* True if val falls in any of the nresolved [lo, hi] pairs from seqset_resolve() above. */ +int +seqset_contains(const struct seq_range *resolved, uint32_t nresolved, + uint32_t val) +{ + uint32_t i; + + for (i = 0; i < nresolved; i++) { + if (val >= resolved[i].lo && val <= resolved[i].hi) + return (1); + } + return (0); +} + +/* + * Highest hi across all resolved ranges (0 if nresolved == 0), for an + * early-exit bound on an ascending scan of idx->lines: once the loop's + * position/UID exceeds this, no later line can match any range, same + * early "break" every one of these loops already had for a single + * range's hi. + */ +uint32_t +seqset_max_hi(const struct seq_range *resolved, uint32_t nresolved) +{ + uint32_t i, max = 0; + + for (i = 0; i < nresolved; i++) { + if (resolved[i].hi > max) + max = resolved[i].hi; + } + return (max); +} + /* Reports every UID in [lo, hi] absent from idx as IMSG_MBOX_SELECT_VANISHED ranges; RFC 7162 SS3.2.6 VANISHED modifier. */ void send_vanished_range(const struct mbox_index *idx, uint32_t lo, uint32_t hi, @@ -265,6 +504,16 @@ send_vanished_range(const struct mbox_index *idx, uint "IMSG_MBOX_SELECT_VANISHED", session_id); } + /* + * A UID of UINT32_MAX would wrap want to 0, after which the + * tail check below is trivially true and this function emits + * a VANISHED (EARLIER) range covering the whole UID space -- + * telling a QRESYNC client that every message in the mailbox + * is gone. Stop instead: there is nothing above this UID to + * report. + */ + if (rec.uid == UINT32_MAX) + return; want = rec.uid + 1; } @@ -312,6 +561,25 @@ index_append(struct mbox_index *idx, uint32_t uid, con char line[STORE_INDEX_LINE_MAX]; int len; + /* + * RFC 9051 SS9 makes a uniqueid an nz-number, so UID 0 is not a UID. + * Callers assign from idx->uidnext and increment it afterwards, with + * no ceiling anywhere, so an exhausted uidnext wraps to 0 and the + * appends after that silently REUSE UIDs still in the mailbox -- + * which SS2.3.1.1 forbids outright, and which breaks + * index_max_uid()'s ascending-order assumption and every "*" + * resolution built on it. Refusing here turns that into a logged + * failure at the first append past the end. The RFC's actual answer + * to running out of UIDs is to change UIDVALIDITY, which needs + * persistent state this daemon does not keep yet; see the 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) */ if (strpbrk(basename, ":\r\n") != NULL) { log_warnx("session %u: refusing index entry with unsafe " @@ -391,6 +659,18 @@ index_save(const struct mbox_index *idx) return (-1); } } + if (fflush(fp) != 0) { + log_warn("session %u: fflush %s", session_id, + STORE_INDEX_TMP_NAME); + fclose(fp); + return (-1); + } + if (fsync(fileno(fp)) == -1) { + log_warn("session %u: fsync %s", session_id, + STORE_INDEX_TMP_NAME); + fclose(fp); + return (-1); + } if (fclose(fp) != 0) { log_warn("session %u: fclose %s", session_id, STORE_INDEX_TMP_NAME); @@ -402,6 +682,21 @@ index_save(const struct mbox_index *idx) 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); + } + } return (0); } @@ -420,47 +715,62 @@ index_free(struct mbox_index *idx) /* 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 mbox_index *idx, struct imsgev *iev) + const struct seq_range *ranges, uint32_t nranges, + struct mbox_index *idx, struct imsgev *iev) { - uint32_t uid_lo, uid_hi, want, i; + struct seq_range resolved[SEQSET_MAX_RANGES]; + uint32_t nresolved, max_hi, i; if (req->qresync_has_uids) { - uid_lo = req->qresync_uid_lo; - uid_hi = req->qresync_uid_hi; + /* + * known-uids is a full RFC 9051 SS9 sequence-set + * (SS3.2.5.1), resolved/swapped/clamped the same way as + * every other UID-space consumer. "*" is forbidden in + * known-uids and already rejected by mailbox_cmd.c's + * parse_qresync_group(), so the max passed here is never + * actually consulted -- kept only for the same calling + * convention every other by-UID seqset_resolve() caller + * uses. clamp_hi is 0: a known UID above the highest one + * currently in use is exactly the case RFC 7162 SS3.2.5.1 + * wants reported VANISHED, not silently 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) */ if (idx->uidnext <= 1) return; - uid_lo = 1; - uid_hi = idx->uidnext - 1; + resolved[0].lo = 1; + resolved[0].hi = idx->uidnext - 1; + resolved[0].lo_is_star = resolved[0].hi_is_star = 0; + nresolved = 1; } - if (uid_hi < uid_lo) - return; /* empty requested range, nothing to do */ - /* single forward pass; want tracks the lowest UID not yet accounted for, a gap before it means vanished */ - want = uid_lo; - for (i = 0; i < idx->nlines && want <= uid_hi; i++) { + /* + * RFC 7162 SS3.2.6: VANISHED (EARLIER) MUST precede FETCH in the + * response stream; guaranteed not by send order here but by + * store_ipc.c's session_handle_mbox_selected(), which buffers + * both kinds separately while SESSION_SELECTING and flushes all + * VANISHED ranges before any FETCH -- the same two-pass split + * handle_mbox_fetch() already uses for its own FETCH ... + * (VANISHED) modifier, and send_vanished_range() itself is the + * exact same helper that call site uses, one resolved range at a + * time. + */ + for (i = 0; i < nresolved; i++) + send_vanished_range(idx, resolved[i].lo, resolved[i].hi, iev); + + max_hi = seqset_max_hi(resolved, nresolved); + for (i = 0; i < idx->nlines; i++) { struct index_rec rec; if (index_parse_line(idx->lines[i], &rec) == -1) continue; /* corrupt line, already logged, not reported either way */ - if (rec.uid < want) - continue; /* below the requested range, or already accounted for */ - if (rec.uid > uid_hi) + if (rec.uid > max_hi) break; + if (!seqset_contains(resolved, nresolved, rec.uid)) + continue; - if (rec.uid > want) { - struct imsg_mbox_select_vanished van; - - memset(&van, 0, sizeof(van)); - van.uid_lo = want; - van.uid_hi = rec.uid - 1; - if (imsg_compose(&iev->ibuf, IMSG_MBOX_SELECT_VANISHED, - 0, 0, -1, &van, sizeof(van)) == -1) - log_warn("session %u: imsg_compose " - "IMSG_MBOX_SELECT_VANISHED", session_id); - } - if (rec.modseq > req->qresync_modseq) { struct imsg_mbox_fetch_meta meta; char suffix[64]; @@ -481,39 +791,232 @@ qresync_send_resync(const struct imsg_mbox_select *req "IMSG_MBOX_FETCH_META (qresync)", session_id); } - - want = rec.uid + 1; } - - if (want <= uid_hi) { - struct imsg_mbox_select_vanished van; - - memset(&van, 0, sizeof(van)); - van.uid_lo = want; - van.uid_hi = uid_hi; - if (imsg_compose(&iev->ibuf, IMSG_MBOX_SELECT_VANISHED, 0, 0, - -1, &van, sizeof(van)) == -1) - log_warn("session %u: imsg_compose " - "IMSG_MBOX_SELECT_VANISHED", session_id); - } } -/* 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. */ +/* + * Takes the mailbox's index lock (op is LOCK_EX or LOCK_SH) and opens the + * index under it, filling *il. Returns 0, or -1 with nothing held. + * + * The order matters and is the whole point: the lock is taken on + * STORE_INDEX_LOCK_NAME, whose inode is stable, and the index is opened only + * afterwards, so the descriptor cannot refer to an inode that a concurrent + * index_save() has already renamed away. See STORE_INDEX_LOCK_NAME's comment + * in store_internal.h for what went wrong when the lock was taken on the + * index itself. + * + * Callers release with index_lock_release(), which is idempotent. + */ int -refresh_index(struct mbox_index *idx, int fd) +index_lock_acquire(struct index_lock *il, int op) { + il->lockfd = -1; + il->fd = -1; + + if ((il->lockfd = open(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); + close(il->lockfd); + il->lockfd = -1; + return (-1); + } + if ((il->fd = open(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); + il->lockfd = -1; + return (-1); + } + return (0); +} + +/* 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) +{ + if (il->fd != -1) { + close(il->fd); + il->fd = -1; + } + /* lock last, so no other process can take it while our index fd is open */ + if (il->lockfd != -1) { + flock(il->lockfd, LOCK_UN); + close(il->lockfd); + il->lockfd = -1; + } +} + +/* + * Issues a UIDVALIDITY for a mailbox that has none, and records it. + * + * RFC 9051 SS2.3.1.1 requires that a mailbox which loses its UIDs be given a + * UIDVALIDITY greater than the one it had before, and recommends a timestamp + * on the grounds that this "ensures that the value is unique and always + * increases". Two calls inside one second break that promise, and a client + * cannot detect it: an unchanged UIDVALIDITY is exactly how the protocol says + * "your cache is still good". + * + * So the timestamp is kept as a floor, not as the answer: the value returned + * is the later of the clock and one past the highest value this user has ever + * been issued, and the new high-water mark is written back before returning. + * The record lives at the maildir root (STORE_UIDVALIDITY_NAME) because the + * mailbox's own state is gone after DELETE -- that is what DELETE means -- so + * the memory has to outlive it. + * + * Called from index_load() with the mailbox's index lock already held. See + * STORE_UIDVALIDITY_NAME's comment for why this lock is safe to nest there, + * and for the rule that keeps it that way. + * + * Every failure degrades to the old behaviour -- a bare timestamp -- rather + * than failing the operation the caller was performing: a mailbox that opens + * with a slightly weaker UIDVALIDITY guarantee is better than a mailbox that + * will not open. One case deliberately does NOT write back: a file that + * exists and holds something unparseable is left exactly as it is, because + * overwriting it would replace a floor we failed to read with a lower one, + * which is the single worst thing this function could do. + */ +uint32_t +uidvalidity_next(void) +{ + const char *path; + char buf[32]; + struct stat st; + time_t now; + char *ep; + unsigned long parsed; + uint32_t floor = 0, val; + ssize_t n; + int fd, writeback = 1; + + /* + * The namespace is flat -- select_mailbox_dir() leaves 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; + + 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) { + log_warn("session %u: open %s (UIDVALIDITY floor); falling " + "back to a bare timestamp", session_id, path); + return (val != 0 ? val : 1); + } + if (flock(fd, LOCK_EX) == -1) { + log_warn("session %u: flock %s (UIDVALIDITY floor); falling " + "back to a bare timestamp", session_id, path); + close(fd); + return (val != 0 ? val : 1); + } + + /* + * A zero-length file is the ordinary just-created case, not damage: + * floor 0 is correct for it. 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)", + session_id, path); + writeback = 0; + } 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 */ + errno = 0; + parsed = strtoul(buf, &ep, 10); + if (buf[0] < '0' || buf[0] > '9' || *ep != '\0' || + errno != 0 || parsed > UINT32_MAX) { + log_warnx("session %u: %s holds no usable " + "UIDVALIDITY floor (%s); leaving it alone " + "and falling back to a bare timestamp", + session_id, path, buf); + writeback = 0; + } else + floor = (uint32_t)parsed; + } + } + + if (writeback) { + if (val <= floor) { + if (floor == UINT32_MAX) { + /* 4 billion issued values, or a clock past + * 2106: nothing greater is representable. */ + log_warnx("session %u: UIDVALIDITY floor is " + "exhausted (%u); reusing it", session_id, + floor); + val = UINT32_MAX; + writeback = 0; + } else + val = floor + 1; + } + } else if (val == 0) + val = 1; /* unusable clock and unusable floor */ + + if (writeback) { + n = snprintf(buf, sizeof(buf), "%u\n", val); + if (n < 0 || (size_t)n >= sizeof(buf) || + lseek(fd, 0, SEEK_SET) == -1 || + ftruncate(fd, 0) == -1 || + write(fd, buf, (size_t)n) != n) + log_warn("session %u: could not record the " + "UIDVALIDITY floor in %s; the value issued now " + "may be issued again", session_id, path); + } + + /* + * A successful floor consultation is otherwise silent, and a + * UIDVALIDITY that is quietly wrong looks exactly like one that is + * right -- the client cannot tell, which is the whole reason this + * function exists. At -v this says 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)"); + + close(fd); /* releases the flock(2) */ + return (val); +} + +/* + * Walks new/ for maildir deliveries the index does not know about yet. + * + * mutate == 0 answers only "is there at least one?", stops at the first, and + * touches neither idx nor the filesystem -- so it is safe under a SHARED + * index lock. That is the question handle_mbox_idle_refresh()'s poll asks + * every few seconds, and the answer is almost always no. + * + * mutate == 1 indexes every one it finds, which is what refresh_index() wants + * and which needs the exclusive lock its callers hold. + * + * Returns 1 if anything was found (mutate == 1: added), 0 if not, -1 on error + * -- and on error with mutate set, idx has already been index_free()'d, which + * is refresh_index()'s long-standing contract with ITS callers. + */ +static int +index_scan_new(struct mbox_index *idx, int mutate) +{ DIR *dp; struct dirent *de; + int added = 0; - if (index_load(fd, idx) == -1) - return (-1); - dp = opendir("new"); if (dp == NULL) { if (errno == ENOENT) return (0); /* no new/ yet on a never-used mailbox, not an error */ log_warn("session %u: opendir new", session_id); - index_free(idx); + if (mutate) + index_free(idx); return (-1); } while ((de = readdir(dp)) != NULL) { @@ -521,22 +1024,57 @@ refresh_index(struct mbox_index *idx, int fd) continue; /* ".", "..", and dotfiles, maildir delivery never creates the latter */ /* never index a filename with ':' or newline, would corrupt the index line format */ if (strpbrk(de->d_name, ":\r\n") != NULL) { - log_warnx("session %u: skipping new/ file with unsafe " - "name (contains ':' or newline): %s", session_id, - de->d_name); + /* + * Logged only on the mutating pass. The read-only one + * runs once per poll interval for the whole life of an + * IDLE, and one badly-named file would otherwise fill + * the log with the same line forever. + */ + if (mutate) + log_warnx("session %u: skipping new/ file " + "with unsafe name (contains ':' or " + "newline): %s", session_id, de->d_name); continue; } if (index_has_basename(idx, de->d_name)) continue; + if (!mutate) { + closedir(dp); + return (1); /* one is enough to answer the question */ + } if (index_append(idx, idx->uidnext, de->d_name) == -1) { closedir(dp); index_free(idx); return (-1); } idx->uidnext++; + added = 1; } closedir(dp); + 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. */ +int +refresh_index(struct mbox_index *idx, int fd) +{ + int added; + + if (index_load(fd, idx) == -1) + return (-1); + + if ((added = index_scan_new(idx, 1)) == -1) + return (-1); /* index_scan_new() has already freed idx */ + + /* + * Nothing new: don't pay index_save()'s cost for a no-op refresh -- + * unless the header itself is new, in which case the cost is the + * point. A freshly invented UIDVALIDITY that is never written down is + * invented again, differently, on the next call. + */ + if (!added && !idx->fresh) + return (0); + if (index_save(idx) == -1) { index_free(idx); return (-1); @@ -544,6 +1082,85 @@ refresh_index(struct mbox_index *idx, int fd) return (0); } +/* + * Cheap change probe for handle_mbox_idle_refresh(). + * + * Two stat(2) calls, no lock and no read, answering "can anything possibly + * have changed since the last look?". Both directories are needed and neither + * is redundant: + * + * "." every mutation imapd itself makes ends in index_save(), which + * creates imapd.index.tmp and rename(2)s it over imapd.index. Both + * are operations on this directory, so its mtime moves for APPEND, + * STORE, EXPUNGE, COPY and MOVE alike. + * "new" an external MTA's delivery lands a file here and touches nothing + * else until something indexes it, so "." alone would miss the one + * case IDLE exists for. + * + * The sample is taken and stored BEFORE the caller does any work, on purpose. + * Recording it afterwards would be tidier -- our own index_save() moves "."'s + * mtime, so a real change costs one extra no-op refresh on the following poll + * -- but it would also record a state that was never reported, and a write + * that landed between the work and the sample would then be invisible for + * good. One redundant refresh is a much better bug than a lost notification. + * + * The residual hole is timestamp granularity: two changes in the same + * nanosecond, either side of a sample, are indistinguishable. On OpenBSD's + * nanosecond st_mtim that is theoretical, and it is written down here rather + * than left to be rediscovered. + */ +static struct { + int valid; + ino_t dir_ino; + ino_t new_ino; + struct timespec dir_mtim; + struct timespec new_mtim; +} idle_probe; + +/* Forces the next probe to report a change; call whenever cwd changes mailbox. */ +void +idle_probe_reset(void) +{ + idle_probe.valid = 0; +} + +static int +tspec_eq(const struct timespec *a, const struct timespec *b) +{ + return (a->tv_sec == b->tv_sec && a->tv_nsec == b->tv_nsec); +} + +/* 1 = nothing can have changed since the last call; 0 = look properly. */ +static int +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. */ + idle_probe.valid = 0; + return (0); + } + if (stat("new", &nst) == -1) + memset(&nst, 0, sizeof(nst)); /* absent new/ is a stable state */ + + same = idle_probe.valid && + dst.st_ino == idle_probe.dir_ino && + nst.st_ino == idle_probe.new_ino && + tspec_eq(&dst.st_mtim, &idle_probe.dir_mtim) && + tspec_eq(&nst.st_mtim, &idle_probe.new_mtim); + + idle_probe.valid = 1; + idle_probe.dir_ino = dst.st_ino; + idle_probe.new_ino = nst.st_ino; + idle_probe.dir_mtim = dst.st_mtim; + idle_probe.new_mtim = nst.st_mtim; + + 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). */ void handle_mbox_idle_refresh(struct imsgev *iev) @@ -551,26 +1168,76 @@ handle_mbox_idle_refresh(struct imsgev *iev) struct mbox_index idx; struct imsg_mbox_idle_refreshed reply; struct imsg_mbox_idle_uid item; - int fd; + struct index_lock il = INDEX_LOCK_INIT; size_t i; + int pending; struct index_rec rec; memset(&reply, 0, sizeof(reply)); - if ((fd = open(STORE_INDEX_NAME, O_RDWR | O_CREAT, 0600)) == -1) { - log_warn("session %u: open %s", session_id, STORE_INDEX_NAME); + /* + * Cheapest question first. On an untouched mailbox this is the whole + * of the work: no lock, no index read, and no IMSG_MBOX_IDLE_UID + * stream -- which matters because that stream is one message per + * message in the mailbox, and the poll runs every few seconds. + */ + if (idle_probe_unchanged()) { + reply.ok = 1; + reply.unchanged = 1; + /* + * The whole mechanism is otherwise silent, and a poll that + * has quietly stopped firing is indistinguishable from a + * healthy daemon -- which is exactly how the dead + * cross-session push survived unnoticed. At -v these two + * lines make the poll observable: one per interval while + * nothing happens, the other when there is real work. + */ + log_debug("session %u: idle refresh: unchanged (probe: no " + "change to . or new/)", session_id); goto send; } - if (flock(fd, LOCK_EX) == -1) { - log_warn("session %u: flock %s", session_id, STORE_INDEX_NAME); - close(fd); + + /* + * Something moved, so the index must be read -- but reading is all + * the common case needs, so take the SHARED lock and escalate only if + * new/ actually holds a delivery that has to be written into the + * index. + */ + if (index_lock_acquire(&il, LOCK_SH) == -1) goto send; + if (index_load(il.fd, &idx) == -1) { + index_lock_release(&il); + goto send; } - if (refresh_index(&idx, fd) == -1) { - flock(fd, LOCK_UN); - close(fd); + if ((pending = index_scan_new(&idx, 0)) == -1) { + index_free(&idx); + index_lock_release(&il); goto send; } + /* + * idx.fresh joins pending here: index_load() has just invented a + * UIDVALIDITY for a mailbox that had no index, and writing it down + * needs the exclusive lock as much as indexing a delivery does. + */ + if (pending || idx.fresh) { + /* + * Drop the shared lock and redo the whole thing exclusively. + * Deliberately not an in-place flock(2) upgrade: that + * conversion is not atomic, so another process can slip in + * between the two states and what was read under the shared + * lock cannot be trusted afterwards. Re-reading under + * LOCK_EX costs one extra index_load() on the rare path that + * had real work to do anyway. + */ + index_free(&idx); + index_lock_release(&il); + if (index_lock_acquire(&il, LOCK_EX) == -1) + goto send; + if (refresh_index(&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) @@ -589,14 +1256,17 @@ handle_mbox_idle_refresh(struct imsgev *iev) reply.uidnext = idx.uidnext; reply.highestmodseq = idx.highestmodseq; + log_debug("session %u: idle refresh: streamed %zu uid(s), " + "highestmodseq %llu%s", session_id, idx.nlines, + (unsigned long long)idx.highestmodseq, + pending ? " (escalated to LOCK_EX, indexed new delivery)" : ""); + index_free(&idx); - flock(fd, LOCK_UN); - close(fd); + index_lock_release(&il); send: 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", session_id); - imsgev_add(iev); } blob - c4aa587b33bd334d3a01ebbe554cc1e058b1c0f4 blob + 690803c7796434b4318d87e9ef1ca9a9282726d0 --- src/listener.c +++ src/listener.c @@ -1,6 +1,22 @@ /* * Copyright (c) 2026 David Williams + * Copyright (c) 2014 Reyk Floeter + * Copyright (c) 2012 Gilles Chehade * + * The RSA_METHOD/EC_KEY_METHOD engine override below (keymgr_engine_ + * init() and everything it installs) adapts smtpd's ca.c + * (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 + * 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 + * shape is lifted from smtpd's smtp.c:187-193 (Gilles Chehade, + * Pierre-Yves Ritschard, Jacek Masiulaniec); see that function's own + * comment. + * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above * copyright notice and this permission notice appear in all copies. @@ -14,7 +30,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* listener.c, protocol/network process: client sockets, IMAP dispatch, TLS. */ +/* 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. */ #include #include @@ -26,12 +42,19 @@ #include #include #include +#include #include #include +#include #include #include #include #include +#include +#include +#include +#include + #include #include #include @@ -45,30 +68,37 @@ #include "listener.h" struct session_list sessions = TAILQ_HEAD_INITIALIZER(sessions); -static uint32_t next_session_id = 1; -struct imsgev iev_auth; /* channel to the AUTH process */ +struct imsgev iev_auth; /* channel to the AUTH process; .ibuf.fd == -1 + * if this connection's auth-worker spawn failed + * (see listener_main()'s boot-drain comment) -- + * auth_cmd.c's sasl_plain_finish() checks this + * before sending IMSG_AUTH_REQUEST */ +struct imsgev iev_search; /* channel to the search-oracle process (SS8.1); + * .ibuf.fd == -1 if this connection's oracle + * spawn failed (same boot-drain comment as + * iev_auth above) -- search_cmd.c's + * search_dispatch() checks this before sending + * IMSG_SEARCH_PARSE_REQUEST */ struct imsgev iev_parent; /* fd 3, alive for the process's lifetime */ -/* n_cleartext_fd/n_tls_fd say how many of each array's slots are live. */ -static int cleartext_fd[LISTENER_MAX_ADDRS] = { -1, -1 }; -static int tls_fd[LISTENER_MAX_ADDRS] = { -1, -1 }; -static int n_cleartext_fd, n_tls_fd; -static struct event ev_accept_cleartext[LISTENER_MAX_ADDRS]; -static struct event ev_accept_tls[LISTENER_MAX_ADDRS]; +/* + * Channel to the keymgr process: real TLS private-key operations are + * forwarded here synchronously, from inside OpenSSL's RSA_METHOD/ + * EC_KEY_METHOD callbacks (keymgr_engine_init() below), never through + * the normal event-driven imsgev dispatch -- nothing unsolicited ever + * arrives on this channel, so it's a plain struct imsgbuf, not a + * struct imsgev; 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 */ -/* Matches parent.c's send_tls_certs() read buffer size. */ +/* Matches parent.c's send_tls_cert() read buffer size. */ #define TLS_CERT_MAX 8192 -#define TLS_KEY_MAX 8192 -/* SIGHUP reload staging; listener_reload_tls() fires once both flags are set. */ -static char reload_cert_buf[TLS_CERT_MAX], reload_key_buf[TLS_KEY_MAX]; -static size_t reload_cert_len, reload_key_len; -static int reload_got_cert, reload_got_key; - struct imap_cmd_entry { const char *name; unsigned int states; /* bitmask of 1U << SESSION_* */ @@ -81,7 +111,8 @@ struct imap_cmd_entry { (1U << SESSION_SELECTING) | (1U << SESSION_SELECTED) | \ (1U << SESSION_FETCHING) | (1U << SESSION_STORING) | \ (1U << SESSION_EXPUNGING) | (1U << SESSION_APPENDING) | \ - (1U << SESSION_SEARCHING) | (1U << SESSION_STATUSING) | \ + (1U << SESSION_SEARCH_PARSING) | (1U << SESSION_SEARCHING) | \ + (1U << SESSION_STATUSING) | \ (1U << SESSION_COPYING) | (1U << SESSION_CREATING) | \ (1U << SESSION_DELETING) | (1U << SESSION_RENAMING) | \ (1U << SESSION_LISTING)) @@ -130,36 +161,374 @@ 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 internal to lib/libtls, not + * declared in the installed tls.h -- confirmed directly (checked + * lib/libtls/tls.h, the only public libtls header in openbsd_source/): + * neither it nor tls_config_set_sign_cb() (tls_internal.h) appears + * there. smtpd's smtp.c (smtp.c:54) forward-declares it the same way; + * see docs/openimap-tls-privsep-design.md SS9 on what depending on an + * unexported, no-compatibility-promise libtls internal costs. + */ +void tls_config_use_fake_private_key(struct tls_config *); + +/* + * SS5.4/SS10.2's fake-key call shape, exactly as smtp.c:187-193 uses it + * (Gilles Chehade, Pierre-Yves Ritschard, Jacek Masiulaniec -- see this + * file's header comment): tls_config_use_fake_private_key() installs + * libtls's placeholder key, which keymgr_engine_init()'s RSA_METHOD/ + * EC_KEY_METHOD override below then intercepts every operation on; + * tls_config_set_keypair_mem() supplies the real certificate with a NULL + * key. Wrapped in one function so the tls_config-building if/else-if + * chains in listener_main() (one call per branch) don't need + * restructuring around two calls. + */ +static int +keymgr_set_fake_keypair(struct tls_config *config, const char *cert_buf, + size_t cert_len) +{ + tls_config_use_fake_private_key(config); + return tls_config_set_keypair_mem(config, (const uint8_t *)cert_buf, + cert_len, NULL, 0); +} + +/* + * RSA/ECDSA privsep engine: installed once, process-wide, so every RSA/ + * EC private-key operation OpenSSL performs against this process's + * "fake" key -- inside a TLS handshake, on whatever connection happens + * to be negotiating at the time -- is intercepted here and forwarded to + * keymgr over keymgr_ibuf instead. Adapted from smtpd's ca.c + * (ca.c:289-558, Reyk Floeter, Gilles Chehade -- see this file's header + * comment); ca.c's own m_*()-based imsg framing (smtpd's message- + * abstraction layer) is replaced with imapd's native "fixed header + + * trailing raw bytes on one imsg" convention, and the imsg id field + * imapd already uses elsewhere (e.g. IMSG_SETUP_PEER's session id) takes + * the place of ca.c's own separate reqid bookkeeping. + */ + +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, bypassing the event loop -- + * called synchronously from inside an OpenSSL RSA_METHOD callback, which + * cannot itself be deferred to the normal libevent dispatch. Unlike + * ca.c's rsae_send_imsg() (ca.c:293-369), there's no "some other imsg is + * queued up, hand it to the normal dispatcher" branch: nothing besides + * these three request/reply pairs is ever multiplexed on the listener + * <->keymgr channel, keymgr never sends anything unsolicited. + */ +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) +{ + struct imsg_keymgr_sign_request req; + struct imsg_keymgr_sign_reply rep; + unsigned char combined[sizeof(req) + KEYMGR_DATA_MAX]; + struct imsg imsg; + static uint32_t reqid; + uint32_t id; + ssize_t n; + int done, ret; + + if (fromlen < 0 || (size_t)fromlen > KEYMGR_DATA_MAX) { + log_warnx("keymgr_forward_rsa: %d bytes over KEYMGR_DATA_MAX", + fromlen); + return (0); + } + + memset(&req, 0, sizeof(req)); + if (strlcpy(req.hash, hash, sizeof(req.hash)) >= sizeof(req.hash)) { + log_warnx("keymgr_forward_rsa: pubkey hash too long"); + return (0); + } + req.padding = (uint32_t)padding; + req.fromlen = (uint32_t)fromlen; + + memcpy(combined, &req, sizeof(req)); + memcpy(combined + sizeof(req), from, (size_t)fromlen); + + id = ++reqid; + if (imsg_compose(&keymgr_ibuf, type, id, 0, -1, combined, + sizeof(req) + (size_t)fromlen) == -1) { + log_warnx("keymgr_forward_rsa: imsg_compose"); + return (0); + } + if (imsgbuf_flush(&keymgr_ibuf) == -1) + fatal("keymgr_forward_rsa: imsgbuf_flush"); + + ret = 0; + done = 0; + while (!done) { + if ((n = imsgbuf_get(&keymgr_ibuf, &imsg)) == -1) + fatal("keymgr_forward_rsa: imsg_get"); + if (n == 0) { + if ((n = imsgbuf_read(&keymgr_ibuf)) == -1) + fatal("keymgr_forward_rsa: imsgbuf_read"); + if (n == 0) + fatalx("keymgr_forward_rsa: keymgr closed " + "channel"); + continue; + } + if (imsg_get_type(&imsg) != type || + imsg_get_id(&imsg) != id) { + log_warnx("keymgr_forward_rsa: unexpected reply " + "type %u id %u (wanted %u/%u)", + imsg_get_type(&imsg), imsg_get_id(&imsg), type, + id); + imsg_free(&imsg); + continue; + } + if (imsg_get_buf(&imsg, &rep, sizeof(rep)) == -1) { + log_warnx("keymgr_forward_rsa: bad reply header"); + imsg_free(&imsg); + break; + } + /* + * tosize, not KEYMGR_DATA_MAX: `to` is OpenSSL's own + * output buffer, which the caller sized RSA_size(rsa) + * -- 256 bytes for a 2048-bit key, where KEYMGR_DATA_MAX + * is 1024, sized for an 8192-bit one. Bounding by the + * wire cap rather than by the buffer we were actually + * handed would let a longer-than-expected reply overrun + * it. smtpd's ca.c sends RSA_size() across for exactly + * this reason (ca.c:319, m_add_size()); this restores + * that bound on the receiving side. + */ + if (rep.ok && rep.tolen <= tosize && + imsg_get_len(&imsg) == rep.tolen) { + if (imsg_get_buf(&imsg, to, rep.tolen) == -1) + log_warnx("keymgr_forward_rsa: bad reply " + "data"); + else + ret = (int)rep.tolen; + } + imsg_free(&imsg); + done = 1; + } + + return (ret); +} + +static ECDSA_SIG * +keymgr_forward_ecdsa(const char *hash, const unsigned char *dgst, + int dgst_len) +{ + struct imsg_keymgr_sign_request req; + struct imsg_keymgr_sign_reply rep; + unsigned char combined[sizeof(req) + KEYMGR_DATA_MAX]; + unsigned char sigbuf[KEYMGR_DATA_MAX]; + struct imsg imsg; + ECDSA_SIG *sig = NULL; + static uint32_t reqid; + uint32_t id; + ssize_t n; + int done; + const unsigned char *sigp; + + if (dgst_len < 0 || (size_t)dgst_len > KEYMGR_DATA_MAX) { + log_warnx("keymgr_forward_ecdsa: %d bytes over " + "KEYMGR_DATA_MAX", dgst_len); + return (NULL); + } + + memset(&req, 0, sizeof(req)); + if (strlcpy(req.hash, hash, sizeof(req.hash)) >= sizeof(req.hash)) { + log_warnx("keymgr_forward_ecdsa: pubkey hash too long"); + return (NULL); + } + req.fromlen = (uint32_t)dgst_len; + + memcpy(combined, &req, sizeof(req)); + memcpy(combined + sizeof(req), dgst, (size_t)dgst_len); + + id = ++reqid; + if (imsg_compose(&keymgr_ibuf, IMSG_KEYMGR_ECDSA_SIGN, id, 0, -1, + combined, sizeof(req) + (size_t)dgst_len) == -1) { + log_warnx("keymgr_forward_ecdsa: imsg_compose"); + return (NULL); + } + if (imsgbuf_flush(&keymgr_ibuf) == -1) + fatal("keymgr_forward_ecdsa: imsgbuf_flush"); + + done = 0; + while (!done) { + if ((n = imsgbuf_get(&keymgr_ibuf, &imsg)) == -1) + fatal("keymgr_forward_ecdsa: imsg_get"); + if (n == 0) { + if ((n = imsgbuf_read(&keymgr_ibuf)) == -1) + fatal("keymgr_forward_ecdsa: imsgbuf_read"); + if (n == 0) + fatalx("keymgr_forward_ecdsa: keymgr closed " + "channel"); + continue; + } + if (imsg_get_type(&imsg) != IMSG_KEYMGR_ECDSA_SIGN || + imsg_get_id(&imsg) != id) { + log_warnx("keymgr_forward_ecdsa: unexpected reply " + "type %u id %u (wanted %u/%u)", + imsg_get_type(&imsg), imsg_get_id(&imsg), + IMSG_KEYMGR_ECDSA_SIGN, id); + imsg_free(&imsg); + continue; + } + if (imsg_get_buf(&imsg, &rep, sizeof(rep)) == -1) { + log_warnx("keymgr_forward_ecdsa: bad reply header"); + imsg_free(&imsg); + break; + } + if (rep.ok && rep.tolen <= KEYMGR_DATA_MAX && + imsg_get_len(&imsg) == rep.tolen) { + if (imsg_get_buf(&imsg, sigbuf, rep.tolen) == -1) + log_warnx("keymgr_forward_ecdsa: bad reply " + "data"); + else { + sigp = sigbuf; + d2i_ECDSA_SIG(&sig, &sigp, (long)rep.tolen); + } + } + imsg_free(&imsg); + done = 1; + } + + return (sig); +} + +static int +keymgr_rsa_priv_enc(int flen, const unsigned char *from, unsigned char *to, + RSA *rsa, int padding) +{ + char *hash; + + if ((hash = RSA_get_ex_data(rsa, 0)) != NULL) + return (keymgr_forward_rsa(IMSG_KEYMGR_RSA_PRIVENC, hash, + from, flen, to, (size_t)RSA_size(rsa), padding)); + return (RSA_meth_get_priv_enc(keymgr_rsa_default)(flen, from, to, + rsa, padding)); +} + +static int +keymgr_rsa_priv_dec(int flen, const unsigned char *from, unsigned char *to, + RSA *rsa, int padding) +{ + char *hash; + + if ((hash = RSA_get_ex_data(rsa, 0)) != NULL) + return (keymgr_forward_rsa(IMSG_KEYMGR_RSA_PRIVDEC, hash, + from, flen, to, (size_t)RSA_size(rsa), padding)); + return (RSA_meth_get_priv_dec(keymgr_rsa_default)(flen, from, to, + rsa, padding)); +} + +static ECDSA_SIG * +keymgr_ecdsa_do_sign(const unsigned char *dgst, int dgst_len, + const BIGNUM *inv, const BIGNUM *rp, EC_KEY *eckey) +{ + ECDSA_SIG *(*psign_sig)(const unsigned char *, int, const BIGNUM *, + const BIGNUM *, EC_KEY *); + char *hash; + + if ((hash = EC_KEY_get_ex_data(eckey, 0)) != NULL) + return (keymgr_forward_ecdsa(hash, dgst, dgst_len)); + EC_KEY_METHOD_get_sign(keymgr_ecdsa_default, NULL, NULL, &psign_sig); + return (psign_sig(dgst, dgst_len, inv, rp, eckey)); +} + +static void +keymgr_rsa_engine_init(void) +{ + if ((keymgr_rsa_default = RSA_get_default_method()) == NULL) + fatalx("keymgr_rsa_engine_init: RSA_get_default_method"); + + if ((keymgr_rsae_method = RSA_meth_dup(keymgr_rsa_default)) == NULL) + fatalx("keymgr_rsa_engine_init: RSA_meth_dup"); + + RSA_meth_set_priv_enc(keymgr_rsae_method, keymgr_rsa_priv_enc); + RSA_meth_set_priv_dec(keymgr_rsae_method, keymgr_rsa_priv_dec); + + RSA_meth_set_flags(keymgr_rsae_method, + RSA_meth_get_flags(keymgr_rsa_default) | RSA_METHOD_FLAG_NO_CHECK); + RSA_meth_set0_app_data(keymgr_rsae_method, + RSA_meth_get0_app_data(keymgr_rsa_default)); + + RSA_set_default_method(keymgr_rsae_method); +} + +static void +keymgr_ecdsa_engine_init(void) +{ + int (*sign)(int, const unsigned char *, int, unsigned char *, + unsigned int *, const BIGNUM *, const BIGNUM *, EC_KEY *); + int (*sign_setup)(EC_KEY *, BN_CTX *, BIGNUM **, BIGNUM **); + + if ((keymgr_ecdsa_default = EC_KEY_get_default_method()) == NULL) + fatalx("keymgr_ecdsa_engine_init: EC_KEY_get_default_method"); + + if ((keymgr_ecdsae_method = EC_KEY_METHOD_new(keymgr_ecdsa_default)) + == NULL) + fatalx("keymgr_ecdsa_engine_init: EC_KEY_METHOD_new"); + + EC_KEY_METHOD_get_sign(keymgr_ecdsa_default, &sign, &sign_setup, + NULL); + EC_KEY_METHOD_set_sign(keymgr_ecdsae_method, sign, sign_setup, + keymgr_ecdsa_do_sign); + + 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). */ +static void +keymgr_engine_init(void) +{ + keymgr_rsa_engine_init(); + keymgr_ecdsa_engine_init(); +} + +/* Builds and starts this process's one and only session; defined below, forward-declared here since listener_main() calls it. */ +static void listener_start_session(uint32_t, int, int, + const struct sockaddr_storage *, socklen_t); + __dead void listener_main(void) { - struct imsgbuf ibuf3; - struct passwd *pw; - int peer_fd; - struct imsg imsg; - struct imsg_listener_init init; - ssize_t n; - char cert_buf[TLS_CERT_MAX], key_buf[TLS_KEY_MAX]; - size_t cert_len = 0, key_len = 0; - int got_cert = 0, got_key = 0, got_init = 0; - int recv_cleartext = 0, recv_tls = 0, i; + struct imsgbuf ibuf3; + struct passwd *pw; + int auth_peer_fd = -1, keymgr_peer_fd = -1; + int search_peer_fd = -1; /* SS8.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; - memset(&init, 0, sizeof(init)); + memset(&sinit, 0, sizeof(sinit)); - if (imsgbuf_init(&ibuf3, 3) == -1) - fatal("imsgbuf_init"); - imsgbuf_allow_fdpass(&ibuf3); /* receives fd-passed socket/peer messages below */ + /* fd-passing is allowed on this channel: it receives fd-passed peer/session messages below; see imsgev_ibuf_init()'s own comment */ + imsgev_ibuf_init(&ibuf3, 3); - /* boot-time handshake: one peer (auth), then SETUP_DONE+ack. */ - peer_fd = setup_recv_one_peer(&ibuf3); - setup_recv_done_and_ack(&ibuf3); - - /* Socket/cert/key/init arrive on fd 3 in any order; read synchronously before event_set(). */ - while (!got_init || !got_cert || !got_key || - recv_cleartext < init.n_cleartext_addrs || - recv_tls < init.n_tls_addrs) { - if ((n = imsg_get(&ibuf3, &imsg)) == -1) - fatal("imsg_get"); + /* + * SS7: this process is spawned fresh per connection (parent.c's + * spawn_connection()), not once at daemon boot, so the peer + * handshake is folded into the same synchronous drain loop as + * the rest of boot below rather than kept as separate blocking + * setup_recv_one_peer() calls. The auth peer in particular may + * never arrive at all -- spawn_connection() skips wiring one + * when the auth-worker itself failed to fork -- so it can't be + * a fixed, blocking "read exactly one" step the way it was when + * a listener process's whole boot depended on both peers + * existing. IMSG_SETUP_PEER is told apart by id: 0 for the auth + * peer, session_id (always >= 1) for the keymgr peer, matching + * parent.c's own setup_peer_send() calls. There is no + * IMSG_SETUP_DONE/ack step either -- see parent.c's header + * comment for why spawn_connection() doesn't use one. + */ + while (!got_cert || !got_session_init || !got_keymgr_peer) { + if ((n = imsgbuf_get(&ibuf3, &imsg)) == -1) + fatal("imsgbuf_get"); if (n == 0) { if ((n = imsgbuf_read(&ibuf3)) == -1) fatal("imsgbuf_read"); @@ -169,19 +538,64 @@ listener_main(void) continue; } switch (imsg_get_type(&imsg)) { - case IMSG_LISTENER_SOCKET_CLEARTEXT: - if (recv_cleartext >= LISTENER_MAX_ADDRS) - fatalx("listener: too many cleartext " - "listener sockets (max %d)", - LISTENER_MAX_ADDRS); - cleartext_fd[recv_cleartext++] = imsg_get_fd(&imsg); + case IMSG_SETUP_PEER: { + uint32_t id = imsg_get_id(&imsg); + int peer_fd = imsg_get_fd(&imsg); + + if (peer_fd == -1) { + log_warnx("listener: IMSG_SETUP_PEER " + "carried no fd"); + break; + } + /* + * parent sends each of these exactly once, so a + * repeat cannot happen today -- but this loop runs + * before pledge(2) and before the privilege drop, + * which makes it the one place where "trust the + * parent" is doing the most work. State the + * invariant rather than silently overwriting a + * descriptor. imsg_get_fd(3) has already passed + * responsibility for peer_fd to us (imsg_init(3): + * only UNCLAIMED descriptors are closed by + * imsg_free()), so the duplicate must be closed + * here. + */ + if (id == 0) { + if (auth_peer_fd != -1) { + log_warnx("listener: duplicate auth " + "IMSG_SETUP_PEER, ignoring"); + close(peer_fd); + break; + } + auth_peer_fd = peer_fd; + } else { + if (keymgr_peer_fd != -1) { + log_warnx("listener: duplicate keymgr " + "IMSG_SETUP_PEER, ignoring"); + close(peer_fd); + break; + } + keymgr_peer_fd = peer_fd; + got_keymgr_peer = 1; + } break; - case IMSG_LISTENER_SOCKET_TLS: - if (recv_tls >= LISTENER_MAX_ADDRS) - fatalx("listener: too many tls listener " - "sockets (max %d)", LISTENER_MAX_ADDRS); - tls_fd[recv_tls++] = imsg_get_fd(&imsg); + } + 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 */ + if (peer_fd == -1) + log_warnx("listener: IMSG_SETUP_SEARCH_PEER " + "carried no fd"); + else if (search_peer_fd != -1) { + /* already claimed above, so close it here */ + log_warnx("listener: duplicate " + "IMSG_SETUP_SEARCH_PEER, ignoring"); + close(peer_fd); + } else + search_peer_fd = peer_fd; break; + } case IMSG_TLS_CERT: cert_len = imsg_get_len(&imsg); if (cert_len > sizeof(cert_buf)) { @@ -196,27 +610,25 @@ listener_main(void) } got_cert = 1; break; - case IMSG_TLS_KEY: - key_len = imsg_get_len(&imsg); - if (key_len > sizeof(key_buf)) { - log_warnx("listener: TLS key too " - "large (%zu > %zu)", key_len, - sizeof(key_buf)); - key_len = 0; - } else if (imsg_get_data(&imsg, key_buf, - key_len) == -1) { - log_warnx("bad IMSG_TLS_KEY"); - key_len = 0; - } - got_key = 1; - break; - case IMSG_LISTENER_INIT: - if (imsg_get_data(&imsg, &init, sizeof(init)) + case IMSG_LISTENER_SESSION_INIT: + if (imsg_get_data(&imsg, &sinit, sizeof(sinit)) == -1) { - log_warnx("bad IMSG_LISTENER_INIT"); + log_warnx("bad IMSG_LISTENER_SESSION_INIT"); break; } - got_init = 1; + /* + * Refuse before claiming, unlike the two peer + * cases above: an fd left unclaimed on this imsg + * is closed by imsg_free() below (imsg_init(3)), + * so there is nothing to clean up by hand. + */ + if (client_fd != -1) { + log_warnx("listener: duplicate " + "IMSG_LISTENER_SESSION_INIT, ignoring"); + break; + } + client_fd = imsg_get_fd(&imsg); + got_session_init = 1; break; default: log_debug("listener boot: unhandled %d", @@ -225,15 +637,10 @@ listener_main(void) } imsg_free(&imsg); } - n_cleartext_fd = recv_cleartext; - n_tls_fd = recv_tls; - log_info("listening on %s:%u (cleartext, %d socket%s) and " - "%s:%u (implicit TLS, %d socket%s)", - init.listen_addr, init.port_cleartext, n_cleartext_fd, - n_cleartext_fd == 1 ? "" : "s", - init.listen_addr, init.port_implicit_tls, n_tls_fd, - n_tls_fd == 1 ? "" : "s"); + if (client_fd == -1) + fatalx("listener: IMSG_LISTENER_SESSION_INIT carried no " + "client fd"); if ((pw = getpwnam("_imapd")) == NULL) fatalx("getpwnam _imapd: no such user " @@ -250,10 +657,13 @@ listener_main(void) 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. */ + keymgr_engine_init(); + /* Failure here isn't fatal, degrades to no-TLS, checked via listener_tls_ctx == NULL below. */ - if (cert_len == 0 || key_len == 0) { - log_warnx("listener: no TLS cert/key received, TLS " - "disabled for this run"); + if (cert_len == 0) { + log_warnx("listener: no TLS cert received, TLS " + "disabled for this session"); } else if ((listener_tls_config = tls_config_new()) == NULL) { log_warnx("listener: tls_config_new failed, TLS disabled"); } else if ((listener_tls_ctx = tls_server()) == NULL) { @@ -268,9 +678,8 @@ listener_main(void) tls_config_free(listener_tls_config); listener_tls_ctx = NULL; listener_tls_config = NULL; - } else if (tls_config_set_keypair_mem(listener_tls_config, - (const uint8_t *)cert_buf, cert_len, - (const uint8_t *)key_buf, key_len) != 0) { + } else if (keymgr_set_fake_keypair(listener_tls_config, cert_buf, + cert_len) != 0) { log_warnx("listener: tls_config_set_keypair_mem: %s, " "TLS disabled", tls_config_error(listener_tls_config)); tls_free(listener_tls_ctx); @@ -287,31 +696,99 @@ listener_main(void) listener_tls_config = NULL; } else { tls_config_clear_keys(listener_tls_config); - log_info("listener: TLS configured"); + log_info("TLS configured (private-key operations " + "forwarded to keymgr)"); } - explicit_bzero(key_buf, sizeof(key_buf)); event_init(); - imsgev_init(&iev_auth, peer_fd, listener_dispatch_auth, NULL); + /* + * auth_peer_fd may be -1 (no auth-worker for this connection -- + * see this function's header comment); iev_auth.ibuf.fd is left + * at -1 in that case (its default zero-init would be fd 0, a + * real, misleading fd), and auth_cmd.c's sasl_plain_finish() + * checks that before ever composing to it. + */ + if (auth_peer_fd != -1) + imsgev_init(&iev_auth, auth_peer_fd, listener_dispatch_auth, + NULL); + else { + iev_auth.ibuf.fd = -1; + log_warnx("session %u: no auth-worker was spawned for this " + "connection, AUTHENTICATE/LOGIN will fail until a new " + "connection gets one", sinit.session_id); + } + /* + * SS8.1: search_peer_fd may be -1 (no search-oracle for this + * connection -- parent.c's spawn_connection() tolerates that fork + * failing independently of listener/auth's own); iev_search.ibuf.fd + * is left at -1 in that case, and search_cmd.c's search_dispatch() + * checks that before ever composing to it, mirroring iev_auth just + * above. + */ + if (search_peer_fd != -1) + imsgev_init(&iev_search, search_peer_fd, listener_dispatch_search, + NULL); + else { + iev_search.ibuf.fd = -1; + log_warnx("session %u: no search-oracle was spawned for this " + "connection, SEARCH will fail until a new connection gets " + "one", sinit.session_id); + } + + if (imsgbuf_init(&keymgr_ibuf, keymgr_peer_fd) == -1) + 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. */ imsgev_init_from_ibuf(&iev_parent, &ibuf3, listener_dispatch_parent, NULL); - for (i = 0; i < n_cleartext_fd; i++) { - event_set(&ev_accept_cleartext[i], cleartext_fd[i], - EV_READ | EV_PERSIST, listener_accept, (void *)0); - event_add(&ev_accept_cleartext[i], NULL); - } - for (i = 0; i < n_tls_fd; i++) { - event_set(&ev_accept_tls[i], tls_fd[i], - EV_READ | EV_PERSIST, listener_accept, (void *)1); - event_add(&ev_accept_tls[i], NULL); - } + /* + * This process's one and only session, built from what boot just + * drained above -- see listener_start_session()'s own comment. + * Must run after event_init()/the TLS setup block above: + * session_tls_start()/session_arm_client_read() register + * libevent events, and an implicit-TLS session needs + * listener_tls_ctx already set. + */ + listener_idle_poll_secs = sinit.idle_poll_secs; + listener_start_session(sinit.session_id, client_fd, + sinit.implicit_tls, &sinit.remote_ss, sinit.remote_sslen); + + /* + * SS8.1 finding: this process makes no socket(2)/connect(2)/ + * bind(2)/listen(2)/accept(2) call anywhere any more -- parent.c's + * spawn_connection() owns every one of those now (SS7), and this + * process only ever inherits an already-accepted client_fd over + * IMSG_LISTENER_SESSION_INIT. The one remaining candidate for + * needing "inet" is listener_start_session()'s getnameinfo(3) call + * just above, formatting s->remote_addr -- but NI_NUMERICHOST| + * NI_NUMERICSERV means it never touches the resolver or the + * network, just formats already-numeric address bytes already in + * hand (see that call's own comment). Dropping "inet" here on that + * basis; if this turns out wrong, pledge(2) will kill the process + * on its next getnameinfo(3) call and this line reverts. + * + * "recvfd" stays: this process receives a descriptor after this line, + * the store child's peer fd, arriving as IMSG_SETUP_PEER on the fd 3 + * channel once parent.c's store-fork handshake completes + * (listener_dispatch_parent()'s own case below). + * + * "sendfd" goes: this process never attaches a descriptor to an imsg. + * The parent is the only one in the tree that does + * (setup_peer_send(), setup_search_peer_send(), + * IMSG_LISTENER_SESSION_INIT -- five call sites, all in parent.c). + * SYS_sendmsg is PLEDGE_STDIO (sys/kern/kern_pledge.c); "sendfd" is + * checked in unp_internalize() (sys/kern/uipc_usrreq.c), which the + * kernel reaches only when SCM_RIGHTS is actually attached, so an + * imsgbuf_allow_fdpass() channel carrying only fd == -1 messages does + * not need the promise. + */ #ifdef __OpenBSD__ - if (pledge("stdio recvfd sendfd inet", NULL) == -1) + if (pledge("stdio recvfd", NULL) == -1) fatal("pledge"); #endif @@ -319,34 +796,75 @@ listener_main(void) fatalx("listener: exited event loop"); } -/* RFC 8314: on the implicit-TLS port the greeting waits for the handshake (s->pending_greeting). */ -void -listener_accept(int fd, short event, void *arg) +/* + * Builds and starts this process's one and only session, from the + * IMSG_LISTENER_SESSION_INIT payload listener_main() drained at boot + * (SS7: parent.c's spawn_connection() forks one listener-worker per + * accepted connection instead of a single long-lived listener + * accept()ing every one itself; MaxStartups admission control moved + * with it, checked in parent.c's parent_accept() before this process + * even exists -- see parent.c's own count_startups()/ + * startups_should_drop(), moved there verbatim from what used to + * live in this file). Does the same work the old accept()-driven + * listener_accept() did once accept(2) returned: construct struct + * session, format remote_addr, log, and either begin the TLS + * handshake or send the plaintext greeting -- just once, since + * there's only ever one connection for this process to serve. + */ +static void +listener_start_session(uint32_t session_id, int client_fd, int implicit_tls, + const struct sockaddr_storage *ss, socklen_t sslen) { - struct sockaddr_storage ss; - socklen_t sslen = sizeof(ss); - int client_fd; - struct session *s; + struct session *s; - (void)event; - if ((client_fd = accept(fd, (struct sockaddr *)&ss, &sslen)) == -1) { - log_warn("accept"); - return; - } - s = calloc(1, sizeof(*s)); if (s == NULL) { log_warn("calloc"); close(client_fd); - return; + /* + * SS7: nothing else will ever run in this process -- it + * exists to serve this one session, and this is the only + * call to this function. Returning would park it in + * event_dispatch() forever with no client, holding a + * parent-side open_session entry that counts against + * MaxStartups for the rest of the daemon's uptime. + * session_teardown()'s exit(0) is unreachable from here + * (there is no session to tear down), so exit directly; + * the implicit-TLS branch just below reaches the same + * outcome through session_teardown(). + */ + exit(1); } - s->id = next_session_id++; + 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 = (arg != (void *)0); /* (void *)1 == port-993 listener */ + s->implicit_tls = implicit_tls; TAILQ_INSERT_TAIL(&sessions, s, entry); - log_debug("session %u: accepted (%s)", s->id, + { + char hbuf[NI_MAXHOST], sbuf[NI_MAXSERV]; + + /* + * NI_NUMERIC*: no resolver call, no network I/O of any + * kind -- pure formatting of the already-numeric address + * bytes in ss/sslen. This is the fact listener_main()'s + * pledge() comment (SS8.1) rests dropping "inet" on; if + * that turns out wrong, this call is where pledge(2) + * would kill the process. + */ + if (getnameinfo((const struct sockaddr *)ss, sslen, hbuf, + sizeof(hbuf), sbuf, sizeof(sbuf), + NI_NUMERICHOST | NI_NUMERICSERV) == 0) + snprintf(s->remote_addr, sizeof(s->remote_addr), + ss->ss_family == AF_INET6 ? "[%s]:%s" : "%s:%s", + hbuf, sbuf); + else + strlcpy(s->remote_addr, "?", sizeof(s->remote_addr)); + } + + log_debug("session %u: accepted from %s (%s)", s->id, + s->remote_addr, s->implicit_tls ? "implicit TLS" : "cleartext/STARTTLS"); if (s->implicit_tls) { @@ -435,8 +953,8 @@ session_tls_handshake(int fd, short event, void *arg) return; } - log_warnx("session %u: tls_handshake: %s", s->id, - tls_error(s->tls_ctx)); + log_warnx("session %u: tls_handshake: %s (peer %s)", s->id, + tls_error(s->tls_ctx), s->remote_addr); session_teardown(s); } @@ -453,6 +971,70 @@ session_send_greeting(struct session *s) 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` (CRLF already stripped) ends in a NON-synchronizing + * literal announcement, "{n+}" (RFC 9051 SS4.3). Those octets are already + * in flight when the line arrives -- unlike a synchronizing "{n}", whose + * octets only follow our "+" continuation -- so the reader has to account + * for them no matter what becomes of the command that announced them. + * Octet count returned in *lenp; 0 is a legal announcement ("{0+}"). + */ +static int +line_nonsync_literal(const char *line, uint64_t *lenp) +{ + const char *open, *stop; + char digits[24], *end; + size_t len, dlen; + unsigned long long v; + + *lenp = 0; + len = strlen(line); + if (len < 4 || line[len - 1] != '}' || line[len - 2] != '+') + return (0); + stop = &line[len - 2]; /* one past the last digit */ + if ((open = memrchr(line, '{', len)) == NULL || open + 1 >= stop) + return (0); + open++; + dlen = (size_t)(stop - open); + if (dlen >= sizeof(digits) || *open < '0' || *open > '9') + return (0); /* also rejects strtoull(3)'s sign/space forms */ + memcpy(digits, open, dlen); + digits[dlen] = '\0'; + + errno = 0; + v = strtoull(digits, &end, 10); + if (*end != '\0' || errno == ERANGE) + return (0); + *lenp = (uint64_t)v; + return (1); +} + +/* + * RFC 9051 SS9: tag = 1*, i.e. no CTLs, no SP, no + * 8-bit, and none of ( ) { % * DQUOTE backslash. Only the length was + * checked before, so any other byte reached session_reply()'s "%s %s %s" + * verbatim; a tag of "+" in particular turns our own reply into what the + * client reads as a command continuation request. + */ +static int +tag_is_valid(const char *tag) +{ + const unsigned char *p; + + if (*tag == '\0') + return (0); + for (p = (const unsigned char *)tag; *p != '\0'; p++) { + if (*p <= 0x20 || *p >= 0x7f) + return (0); + if (strchr("(){%*\"\\+", (int)*p) != NULL) + return (0); + } + return (1); +} + /* Splits s->inbuf into CRLF lines (bare LF isn't one, RFC 9051 SS2.2); session_handle_line() can free *s* (LOGOUT). */ void session_dispatch_client(int fd, short event, void *arg) @@ -460,12 +1042,23 @@ 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(); see the + * re-arm at the end of this function. + * Not "want": the literal-assembly loop + * below has its own uint64_t want, and + * shadowing it draws -Wshadow. */ (void)event; + if (s->write_failed) { + /* A prior session_write() couldn't finish; see listener.h. */ + session_teardown(s); + return; + } + if (s->tls_active) { - n = tls_read(s->tls_ctx, s->inbuf + s->inbuflen, - sizeof(s->inbuf) - s->inbuflen); + 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. */ event_del(&s->client_ev); @@ -486,6 +1079,10 @@ session_dispatch_client(int fd, short event, void *arg 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(). */ + if (errno == EINTR || errno == EAGAIN || + errno == EWOULDBLOCK) + return; log_warn("session %u: read", s->id); session_teardown(s); return; @@ -500,9 +1097,50 @@ session_dispatch_client(int fd, short event, void *arg s->inbuflen += (size_t)n; for (;;) { - size_t consumed; - int alive; + uint64_t nonsync_len; + size_t consumed, linelen; + int alive; + /* + * Octets of a non-synchronizing literal whose command was + * refused (bad syntax, over the size cap, arrived while the + * session was busy, or simply wasn't APPEND). They are on + * the wire regardless, so swallow them rather than let the + * message body be parsed as further IMAP commands. + */ + if (s->literal_discard > 0) { + uint64_t take; + + take = (uint64_t)s->inbuflen < s->literal_discard ? + (uint64_t)s->inbuflen : s->literal_discard; + if (take > 0) { + memmove(s->inbuf, s->inbuf + take, + s->inbuflen - (size_t)take); + s->inbuflen -= (size_t)take; + s->literal_discard -= take; + } + if (s->literal_discard > 0) + break; /* need more data */ + /* + * The command line that announced this literal + * still ends in CRLF (RFC 9051 SS9). Nothing + * downstream wants it, and leaving it makes the + * line parser below see a zero-length line and + * answer "* BAD Empty command line" after every + * refused literal. Deliberately tolerant: if what + * follows is not CRLF then the command had more + * arguments after the literal, and the line parser + * should still get a crack at them. + */ + if (s->inbuflen >= 2 && s->inbuf[0] == '\r' && + s->inbuf[1] == '\n') { + memmove(s->inbuf, s->inbuf + 2, + s->inbuflen - 2); + s->inbuflen -= 2; + } + continue; + } + /* RFC 9051 SS4.3 literal in flight; checked before CRLF search since raw octets can contain CRLF. */ if (s->literal_pending) { uint64_t want, take; @@ -548,17 +1186,81 @@ session_dispatch_client(int fd, short event, void *arg if (crlf == NULL) break; + linelen = (size_t)(crlf - s->inbuf); + consumed = linelen + 2; + + /* + * RFC 9051 SS2.2/SS9: CR and LF appear in a command only as the + * CRLF terminator, and NUL is not an ASTRING-CHAR. This is the + * one chokepoint where that can be enforced, and enforcing it + * here is what keeps every downstream site that echoes client + * text back (FETCH's BODY[