From 8380d0005224591913aa460562d118c0e5eb6f50 Mon Sep 17 00:00:00 2001 From: op Date: Tue, 7 May 2024 12:10:06 +0000 Subject: [PATCH] change the smtpd table protocol Using imsg for the "proc" table (external programs) has proven quite painful in practice since a lot of smtpd internals (structs, enums, etc..) have to be kept in sync with the various tables implementations. Instead, a filter-like protocol for tables decouples the implementations and allows to write and test tables easily. The new text-based transport protocol is documented in the (added) smtpd-tables(7) manpage. The old imsg protocol is no longer supported and existing tables have to be converted. In particular, users of opensmtpd-extras tables will need install the new opensmtpd-table-* packages. With lots of suggestions and improvements from gilles and a tweak from Philipp (philipp+openbsd [at] bureaucracy [dot] de), thanks! ok gilles --- usr.sbin/smtpd/makemap.c | 5 +- usr.sbin/smtpd/queue_proc.c | 4 +- usr.sbin/smtpd/scheduler_proc.c | 4 +- usr.sbin/smtpd/smtpd.c | 7 +- usr.sbin/smtpd/smtpd.h | 6 +- usr.sbin/smtpd/table.c | 63 ++++-- usr.sbin/smtpd/table_proc.c | 344 +++++++++++++++++--------------- 7 files changed, 249 insertions(+), 184 deletions(-) diff --git a/usr.sbin/smtpd/makemap.c b/usr.sbin/smtpd/makemap.c index b5d88a8bd46..d883389b4a4 100644 --- a/usr.sbin/smtpd/makemap.c +++ b/usr.sbin/smtpd/makemap.c @@ -1,4 +1,4 @@ -/* $OpenBSD: makemap.c,v 1.76 2024/02/11 09:24:26 op Exp $ */ +/* $OpenBSD: makemap.c,v 1.77 2024/05/07 12:10:06 op Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -57,7 +57,8 @@ enum output_type { * Stub functions so that makemap compiles using minimum object files. */ int -fork_proc_backend(const char *backend, const char *conf, const char *procname) +fork_proc_backend(const char *backend, const char *conf, const char *procname, + int do_stdout) { return (-1); } diff --git a/usr.sbin/smtpd/queue_proc.c b/usr.sbin/smtpd/queue_proc.c index a3601e5ddab..ff8fe779bd7 100644 --- a/usr.sbin/smtpd/queue_proc.c +++ b/usr.sbin/smtpd/queue_proc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: queue_proc.c,v 1.10 2024/01/20 09:01:03 claudio Exp $ */ +/* $OpenBSD: queue_proc.c,v 1.11 2024/05/07 12:10:06 op Exp $ */ /* * Copyright (c) 2013 Eric Faurot @@ -287,7 +287,7 @@ queue_proc_init(struct passwd *pw, int server, const char *conf) uint32_t version; int fd; - fd = fork_proc_backend("queue", conf, "queue-proc"); + fd = fork_proc_backend("queue", conf, "queue-proc", 0); if (fd == -1) fatalx("queue-proc: exiting"); diff --git a/usr.sbin/smtpd/scheduler_proc.c b/usr.sbin/smtpd/scheduler_proc.c index 402d36955d2..b601b905e4a 100644 --- a/usr.sbin/smtpd/scheduler_proc.c +++ b/usr.sbin/smtpd/scheduler_proc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: scheduler_proc.c,v 1.9 2021/06/14 17:58:16 eric Exp $ */ +/* $OpenBSD: scheduler_proc.c,v 1.10 2024/05/07 12:10:06 op Exp $ */ /* * Copyright (c) 2013 Eric Faurot @@ -100,7 +100,7 @@ scheduler_proc_init(const char *conf) int fd, r; uint32_t version; - fd = fork_proc_backend("scheduler", conf, "scheduler-proc"); + fd = fork_proc_backend("scheduler", conf, "scheduler-proc", 0); if (fd == -1) fatalx("scheduler-proc: exiting"); diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c index 9f9480053e5..b42a1ef79c1 100644 --- a/usr.sbin/smtpd/smtpd.c +++ b/usr.sbin/smtpd/smtpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: smtpd.c,v 1.350 2024/04/24 21:31:31 op Exp $ */ +/* $OpenBSD: smtpd.c,v 1.351 2024/05/07 12:10:06 op Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -1127,7 +1127,8 @@ load_pki_keys(void) } int -fork_proc_backend(const char *key, const char *conf, const char *procname) +fork_proc_backend(const char *key, const char *conf, const char *procname, + int do_stdout) { pid_t pid; int sp[2]; @@ -1165,6 +1166,8 @@ fork_proc_backend(const char *key, const char *conf, const char *procname) if (pid == 0) { /* child process */ dup2(sp[0], STDIN_FILENO); + if (do_stdout) + dup2(sp[0], STDOUT_FILENO); if (closefrom(STDERR_FILENO + 1) == -1) exit(1); diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h index db7a8bacdb2..0ab99d7f4f4 100644 --- a/usr.sbin/smtpd/smtpd.h +++ b/usr.sbin/smtpd/smtpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: smtpd.h,v 1.683 2024/03/02 22:40:28 op Exp $ */ +/* $OpenBSD: smtpd.h,v 1.684 2024/05/07 12:10:06 op Exp $ */ /* * Copyright (c) 2008 Gilles Chehade @@ -1620,7 +1620,7 @@ const char *proc_name(enum smtp_proc_type); const char *proc_title(enum smtp_proc_type); const char *imsg_to_str(int); void log_imsg(int, int, struct imsg *); -int fork_proc_backend(const char *, const char *, const char *); +int fork_proc_backend(const char *, const char *, const char *, int); /* srs.c */ @@ -1640,6 +1640,8 @@ struct stat_value *stat_timespec(struct timespec *); /* table.c */ +const char *table_service_name(enum table_service); +int table_service_from_name(const char *); struct table *table_find(struct smtpd *, const char *); struct table *table_create(struct smtpd *, const char *, const char *, const char *); diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c index 99db7b0a305..d24892ede64 100644 --- a/usr.sbin/smtpd/table.c +++ b/usr.sbin/smtpd/table.c @@ -1,4 +1,4 @@ -/* $OpenBSD: table.c,v 1.51 2024/01/04 09:34:03 op Exp $ */ +/* $OpenBSD: table.c,v 1.52 2024/05/07 12:10:06 op Exp $ */ /* * Copyright (c) 2013 Eric Faurot @@ -37,7 +37,6 @@ extern struct table_backend table_backend_db; extern struct table_backend table_backend_getpwnam; extern struct table_backend table_backend_proc; -static const char * table_service_name(enum table_service); static int table_parse_lookup(enum table_service, const char *, const char *, union lookup *); static int parse_sockaddr(struct sockaddr *, int, const char *); @@ -67,27 +66,59 @@ table_backend_lookup(const char *backend) return NULL; } -static const char * +const char * table_service_name(enum table_service s) { switch (s) { - case K_NONE: return "NONE"; - case K_ALIAS: return "ALIAS"; - case K_DOMAIN: return "DOMAIN"; - case K_CREDENTIALS: return "CREDENTIALS"; - case K_NETADDR: return "NETADDR"; - case K_USERINFO: return "USERINFO"; - case K_SOURCE: return "SOURCE"; - case K_MAILADDR: return "MAILADDR"; - case K_ADDRNAME: return "ADDRNAME"; - case K_MAILADDRMAP: return "MAILADDRMAP"; - case K_RELAYHOST: return "RELAYHOST"; - case K_STRING: return "STRING"; - case K_REGEX: return "REGEX"; + case K_NONE: return "none"; + case K_ALIAS: return "alias"; + case K_DOMAIN: return "domain"; + case K_CREDENTIALS: return "credentials"; + case K_NETADDR: return "netaddr"; + case K_USERINFO: return "userinfo"; + case K_SOURCE: return "source"; + case K_MAILADDR: return "mailaddr"; + case K_ADDRNAME: return "addrname"; + case K_MAILADDRMAP: return "mailaddrmap"; + case K_RELAYHOST: return "relayhost"; + case K_STRING: return "string"; + case K_REGEX: return "regex"; } return "???"; } +int +table_service_from_name(const char *service) +{ + if (!strcmp(service, "none")) + return K_NONE; + if (!strcmp(service, "alias")) + return K_ALIAS; + if (!strcmp(service, "domain")) + return K_DOMAIN; + if (!strcmp(service, "credentials")) + return K_CREDENTIALS; + if (!strcmp(service, "netaddr")) + return K_NETADDR; + if (!strcmp(service, "userinfo")) + return K_USERINFO; + if (!strcmp(service, "source")) + return K_SOURCE; + if (!strcmp(service, "mailaddr")) + return K_MAILADDR; + if (!strcmp(service, "addrname")) + return K_ADDRNAME; + if (!strcmp(service, "mailaddrmap")) + return K_MAILADDRMAP; + if (!strcmp(service, "relayhost")) + return K_RELAYHOST; + if (!strcmp(service, "string")) + return K_STRING; + if (!strcmp(service, "regex")) + return K_REGEX; + return (-1); +} + struct table * table_find(struct smtpd *conf, const char *name) { diff --git a/usr.sbin/smtpd/table_proc.c b/usr.sbin/smtpd/table_proc.c index 56893a0fb61..bd40bac6bf5 100644 --- a/usr.sbin/smtpd/table_proc.c +++ b/usr.sbin/smtpd/table_proc.c @@ -1,6 +1,7 @@ -/* $OpenBSD: table_proc.c,v 1.17 2021/06/14 17:58:16 eric Exp $ */ +/* $OpenBSD: table_proc.c,v 1.18 2024/05/07 12:10:06 op Exp $ */ /* + * Copyright (c) 2024 Omar Polo * Copyright (c) 2013 Eric Faurot * * Permission to use, copy, modify, and distribute this software for any @@ -17,85 +18,102 @@ */ #include +#include +#include #include +#include #include "smtpd.h" #include "log.h" +#define PROTOCOL_VERSION "0.1" + struct table_proc_priv { - pid_t pid; - struct imsgbuf ibuf; + FILE *in; + FILE *out; + char *line; + size_t linesize; + + /* + * The last ID used in a request. At the moment the protocol + * is synchronous from our point of view, so it's used to + * assert that the table replied with the correct ID. + */ + char lastid[16]; }; -static struct imsg imsg; -static size_t rlen; -static char *rdata; - -extern char **environ; - -static void -table_proc_call(struct table_proc_priv *p) +static char * +table_proc_nextid(struct table *table) { - ssize_t n; - - if (imsg_flush(&p->ibuf) == -1) { - log_warn("warn: table-proc: imsg_flush"); - fatalx("table-proc: exiting"); - } - - while (1) { - if ((n = imsg_get(&p->ibuf, &imsg)) == -1) { - log_warn("warn: table-proc: imsg_get"); - break; - } - if (n) { - rlen = imsg.hdr.len - IMSG_HEADER_SIZE; - rdata = imsg.data; - - if (imsg.hdr.type != PROC_TABLE_OK) { - log_warnx("warn: table-proc: bad response"); - break; - } - return; - } - - if ((n = imsg_read(&p->ibuf)) == -1 && errno != EAGAIN) { - log_warn("warn: table-proc: imsg_read"); - break; - } + struct table_proc_priv *priv = table->t_handle; + int r; - if (n == 0) { - log_warnx("warn: table-proc: pipe closed"); - break; - } - } + r = snprintf(priv->lastid, sizeof(priv->lastid), "%lld", + (unsigned long long)arc4random()); + if (r < 0 || (size_t)r >= sizeof(priv->lastid)) + fatal("table-proc: snprintf"); - fatalx("table-proc: exiting"); + return (priv->lastid); } static void -table_proc_read(void *dst, size_t len) +table_proc_send(struct table *table, const char *type, int service, + const char *param) { - if (len > rlen) { - log_warnx("warn: table-proc: bad msg len"); - fatalx("table-proc: exiting"); - } - - if (dst) - memmove(dst, rdata, len); - - rlen -= len; - rdata += len; + struct table_proc_priv *priv = table->t_handle; + struct timeval tv; + + gettimeofday(&tv, NULL); + fprintf(priv->out, "table|%s|%lld.%06ld|%s|%s", + PROTOCOL_VERSION, (long long)tv.tv_sec, (long)tv.tv_usec, + table->t_name, type); + if (service != -1) { + fprintf(priv->out, "|%s|%s", table_service_name(service), + table_proc_nextid(table)); + if (param) + fprintf(priv->out, "|%s", param); + fputc('\n', priv->out); + } else + fprintf(priv->out, "|%s\n", table_proc_nextid(table)); + + if (fflush(priv->out) == EOF) + fatal("table-proc: fflush"); } -static void -table_proc_end(void) +static const char * +table_proc_recv(struct table *table, const char *type) { - if (rlen) { - log_warnx("warn: table-proc: bogus data"); - fatalx("table-proc: exiting"); - } - imsg_free(&imsg); + struct table_proc_priv *priv = table->t_handle; + const char *l; + ssize_t linelen; + size_t len; + + if ((linelen = getline(&priv->line, &priv->linesize, priv->in)) == -1) + fatal("table-proc: getline"); + priv->line[strcspn(priv->line, "\n")] = '\0'; + l = priv->line; + + len = strlen(type); + if (strncmp(l, type, len) != 0) + goto err; + l += len; + + if (*l != '|') + goto err; + l++; + + len = strlen(priv->lastid); + if (strncmp(l, priv->lastid, len) != 0) + goto err; + l += len; + + if (*l != '|') + goto err; + return (++l); + + err: + log_warnx("warn: table-proc: failed to parse reply"); + fatalx("table-proc: exiting"); } /* @@ -106,24 +124,53 @@ static int table_proc_open(struct table *table) { struct table_proc_priv *priv; - struct table_open_params op; - int fd; + const char *s; + ssize_t len; + int service, services = 0; + int fd, fdd; priv = xcalloc(1, sizeof(*priv)); - fd = fork_proc_backend("table", table->t_config, table->t_name); + fd = fork_proc_backend("table", table->t_config, table->t_name, 1); if (fd == -1) fatalx("table-proc: exiting"); + if ((fdd = dup(fd)) == -1) { + log_warnx("warn: table-proc: dup"); + fatalx("table-proc: exiting"); + } + if ((priv->in = fdopen(fd, "r")) == NULL) + fatalx("table-proc: fdopen"); + if ((priv->out = fdopen(fdd, "w")) == NULL) + fatalx("table-proc: fdopen"); + + fprintf(priv->out, "config|smtpd-version|"SMTPD_VERSION"\n"); + fprintf(priv->out, "config|protocol|"PROTOCOL_VERSION"\n"); + fprintf(priv->out, "config|tablename|%s\n", table->t_name); + fprintf(priv->out, "config|ready\n"); + if (fflush(priv->out) == EOF) + fatalx("table-proc: fflush"); + + while ((len = getline(&priv->line, &priv->linesize, priv->in)) != -1) { + priv->line[strcspn(priv->line, "\n")] = '\0'; + + if (strncmp(priv->line, "register|", 9) != 0) + fatalx("table-proc: invalid handshake reply"); + + s = priv->line + 9; + if (!strcmp(s, "ready")) + break; + service = table_service_from_name(s); + if (service == -1 || service == K_NONE) + fatalx("table-proc: unknown service %s", s); - imsg_init(&priv->ibuf, fd); + services |= service; + } - memset(&op, 0, sizeof op); - op.version = PROC_TABLE_API_VERSION; - (void)strlcpy(op.name, table->t_name, sizeof op.name); - imsg_compose(&priv->ibuf, PROC_TABLE_OPEN, 0, 0, -1, &op, sizeof op); + if (ferror(priv->in)) + fatalx("table-proc: getline"); - table_proc_call(priv); - table_proc_end(); + if (services == 0) + fatalx("table-proc: no services registered"); table->t_handle = priv; @@ -133,16 +180,17 @@ table_proc_open(struct table *table) static int table_proc_update(struct table *table) { - struct table_proc_priv *priv = table->t_handle; - int r; + const char *r; - imsg_compose(&priv->ibuf, PROC_TABLE_UPDATE, 0, 0, -1, NULL, 0); + table_proc_send(table, "update", -1, NULL); + r = table_proc_recv(table, "update-result"); + if (!strcmp(r, "ok")) + return (1); + if (!strcmp(r, "error")) + return (0); - table_proc_call(priv); - table_proc_read(&r, sizeof(r)); - table_proc_end(); - - return (r); + log_warnx("warn: table-proc: failed parse reply"); + fatalx("table-proc: exiting"); } static void @@ -150,105 +198,85 @@ table_proc_close(struct table *table) { struct table_proc_priv *priv = table->t_handle; - imsg_compose(&priv->ibuf, PROC_TABLE_CLOSE, 0, 0, -1, NULL, 0); - if (imsg_flush(&priv->ibuf) == -1) - fatal("imsg_flush"); + if (fclose(priv->in) == EOF) + fatal("table-proc: fclose"); + if (fclose(priv->out) == EOF) + fatal("table-proc: fclose"); + free(priv->line); + free(priv); table->t_handle = NULL; } -static int -imsg_add_params(struct ibuf *buf) -{ - size_t count = 0; - - if (imsg_add(buf, &count, sizeof(count)) == -1) - return (-1); - - return (0); -} - static int table_proc_lookup(struct table *table, enum table_service s, const char *k, char **dst) { - struct table_proc_priv *priv = table->t_handle; - struct ibuf *buf; - int r; + const char *req = "lookup", *res = "lookup-result"; + const char *r; - buf = imsg_create(&priv->ibuf, - dst ? PROC_TABLE_LOOKUP : PROC_TABLE_CHECK, 0, 0, - sizeof(s) + strlen(k) + 1); + if (dst == NULL) { + req = "check"; + res = "check-result"; + } - if (buf == NULL) - return (-1); - if (imsg_add(buf, &s, sizeof(s)) == -1) - return (-1); - if (imsg_add_params(buf) == -1) - return (-1); - if (imsg_add(buf, k, strlen(k) + 1) == -1) + table_proc_send(table, req, s, k); + r = table_proc_recv(table, res); + + /* common replies */ + if (!strcmp(r, "not-found")) + return (0); + if (!strcmp(r, "error")) return (-1); - imsg_close(&priv->ibuf, buf); - - table_proc_call(priv); - table_proc_read(&r, sizeof(r)); - - if (r == 1 && dst) { - if (rlen == 0) { - log_warnx("warn: table-proc: empty response"); - fatalx("table-proc: exiting"); - } - if (rdata[rlen - 1] != '\0') { - log_warnx("warn: table-proc: not NUL-terminated"); - fatalx("table-proc: exiting"); - } - *dst = strdup(rdata); - if (*dst == NULL) - r = -1; - table_proc_read(NULL, rlen); - } - table_proc_end(); + if (dst == NULL) { + /* check op */ + if (!strncmp(r, "found", 5)) + return (1); + log_warnx("warn: table-proc: failed to parse reply"); + fatalx("table-proc: exiting"); + } - return (r); + /* lookup op */ + if (strncmp(r, "found|", 6) != 0) { + log_warnx("warn: table-proc: failed to parse reply"); + fatalx("table-proc: exiting"); + } + r += 6; + if (*r == '\0') { + log_warnx("warn: table-proc: empty response"); + fatalx("table-proc: exiting"); + } + if ((*dst = strdup(r)) == NULL) + return (-1); + return (1); } static int table_proc_fetch(struct table *table, enum table_service s, char **dst) { - struct table_proc_priv *priv = table->t_handle; - struct ibuf *buf; - int r; + const char *r; - buf = imsg_create(&priv->ibuf, PROC_TABLE_FETCH, 0, 0, sizeof(s)); - if (buf == NULL) - return (-1); - if (imsg_add(buf, &s, sizeof(s)) == -1) - return (-1); - if (imsg_add_params(buf) == -1) + table_proc_send(table, "fetch", s, NULL); + r = table_proc_recv(table, "fetch-result"); + + if (!strcmp(r, "not-found")) + return (0); + if (!strcmp(r, "error")) return (-1); - imsg_close(&priv->ibuf, buf); - - table_proc_call(priv); - table_proc_read(&r, sizeof(r)); - - if (r == 1) { - if (rlen == 0) { - log_warnx("warn: table-proc: empty response"); - fatalx("table-proc: exiting"); - } - if (rdata[rlen - 1] != '\0') { - log_warnx("warn: table-proc: not NUL-terminated"); - fatalx("table-proc: exiting"); - } - *dst = strdup(rdata); - if (*dst == NULL) - r = -1; - table_proc_read(NULL, rlen); - } - table_proc_end(); + if (strncmp(r, "found|", 6) != 0) { + log_warnx("warn: table-proc: failed to parse reply"); + fatalx("table-proc: exiting"); + } + r += 6; + if (*r == '\0') { + log_warnx("warn: table-proc: empty response"); + fatalx("table-proc: exiting"); + } - return (r); + if ((*dst = strdup(r)) == NULL) + return (-1); + return (1); } struct table_backend table_backend_proc = { -- 2.20.1