Hi all,
I've made a refactored version of table-procexec,
hopefully with a lot less redundancy in code.
This patch adds the table-procexec backend which
is configured with a timeout of 500 milliseconds.
Currently this is hardcoded, but that is easy enough to
change and shouldnt be the holdback.
In case a table times out and the response has not reached
smtpd, this sets the table status to indicate that and
also starts an event to discard the next line coming on the socket.
After which we are "clear" for communication.
Comments would be very welcome and testing even more so.
I am not the most proficient C coder...
Cheers,
Aisha
diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y
index 832f4f2aec9..ff7b9a9a340 100644
--- a/usr.sbin/smtpd/parse.y
+++ b/usr.sbin/smtpd/parse.y
@@ -2543,13 +2543,6 @@ table : TABLE STRING STRING {
config = p+1;
}
}
- if (config != NULL && *config != '/') {
- yyerror("invalid backend parameter for table:
%s",
- $2);
- free($2);
- free($3);
- YYERROR;
- }
table = table_create(conf, backend, $2, config);
if (!table_config(table)) {
yyerror("invalid configuration file %s for
table %s",
diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile
index ef8148be8c9..46831d647dc 100644
--- a/usr.sbin/smtpd/smtpctl/Makefile
+++ b/usr.sbin/smtpd/smtpctl/Makefile
@@ -47,7 +47,7 @@ SRCS+= table.c
SRCS+= table_static.c
SRCS+= table_db.c
SRCS+= table_getpwnam.c
-SRCS+= table_proc.c
+SRCS+= table_procexec.c
SRCS+= unpack_dns.c
SRCS+= spfwalk.c
diff --git a/usr.sbin/smtpd/smtpd.h b/usr.sbin/smtpd/smtpd.h
index e6fc114d0a6..8ef80add4e7 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1663,6 +1663,7 @@ int table_regex_match(const char *, const char *);
void table_open_all(struct smtpd *);
void table_dump_all(struct smtpd *);
void table_close_all(struct smtpd *);
+const char *table_service_name(enum table_service );
/* to.c */
diff --git a/usr.sbin/smtpd/smtpd/Makefile b/usr.sbin/smtpd/smtpd/Makefile
index b31d4e42224..64e73c3bb70 100644
--- a/usr.sbin/smtpd/smtpd/Makefile
+++ b/usr.sbin/smtpd/smtpd/Makefile
@@ -62,7 +62,7 @@ SRCS+= compress_gzip.c
SRCS+= table_db.c
SRCS+= table_getpwnam.c
-SRCS+= table_proc.c
+SRCS+= table_procexec.c
SRCS+= table_static.c
SRCS+= queue_fs.c
diff --git a/usr.sbin/smtpd/table.c b/usr.sbin/smtpd/table.c
index 7328cf5df6e..81102ef90e1 100644
--- a/usr.sbin/smtpd/table.c
+++ b/usr.sbin/smtpd/table.c
@@ -35,9 +35,8 @@ struct table_backend *table_backend_lookup(const char *);
extern struct table_backend table_backend_static;
extern struct table_backend table_backend_db;
extern struct table_backend table_backend_getpwnam;
-extern struct table_backend table_backend_proc;
+extern struct table_backend table_backend_procexec;
-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 *);
@@ -48,7 +47,7 @@ static struct table_backend *backends[] = {
&table_backend_static,
&table_backend_db,
&table_backend_getpwnam,
- &table_backend_proc,
+ &table_backend_procexec,
NULL
};
@@ -67,7 +66,7 @@ table_backend_lookup(const char *backend)
return NULL;
}
-static const char *
+const char *
table_service_name(enum table_service s)
{
switch (s) {
@@ -198,10 +197,9 @@ table_create(struct smtpd *conf, const char *backend,
const char *name,
PATH_LIBEXEC"/table-%s\"", backend);
}
if (stat(path, &sb) == 0) {
- tb = table_backend_lookup("proc");
- (void)strlcpy(path, backend, sizeof(path));
+ tb = table_backend_lookup("proc-exec");
if (config) {
- (void)strlcat(path, ":", sizeof(path));
+ (void)strlcat(path, " ", sizeof(path));
if (strlcat(path, config, sizeof(path))
>= sizeof(path))
fatalx("table_create: config file path
too long");
diff --git a/usr.sbin/smtpd/table_proc.c b/usr.sbin/smtpd/table_proc.c
deleted file mode 100644
index 56893a0fb61..00000000000
--- a/usr.sbin/smtpd/table_proc.c
+++ /dev/null
@@ -1,265 +0,0 @@
-/* $OpenBSD: table_proc.c,v 1.17 2021/06/14 17:58:16 eric Exp $ */
-
-/*
- * Copyright (c) 2013 Eric Faurot <e...@openbsd.org>
- *
- * 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.
- *
- * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
- * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
- * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
- * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
- * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
- * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
- */
-
-#include <errno.h>
-#include <string.h>
-
-#include "smtpd.h"
-#include "log.h"
-
-struct table_proc_priv {
- pid_t pid;
- struct imsgbuf ibuf;
-};
-
-static struct imsg imsg;
-static size_t rlen;
-static char *rdata;
-
-extern char **environ;
-
-static void
-table_proc_call(struct table_proc_priv *p)
-{
- 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;
- }
-
- if (n == 0) {
- log_warnx("warn: table-proc: pipe closed");
- break;
- }
- }
-
- fatalx("table-proc: exiting");
-}
-
-static void
-table_proc_read(void *dst, size_t len)
-{
- 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;
-}
-
-static void
-table_proc_end(void)
-{
- if (rlen) {
- log_warnx("warn: table-proc: bogus data");
- fatalx("table-proc: exiting");
- }
- imsg_free(&imsg);
-}
-
-/*
- * API
- */
-
-static int
-table_proc_open(struct table *table)
-{
- struct table_proc_priv *priv;
- struct table_open_params op;
- int fd;
-
- priv = xcalloc(1, sizeof(*priv));
-
- fd = fork_proc_backend("table", table->t_config, table->t_name);
- if (fd == -1)
- fatalx("table-proc: exiting");
-
- imsg_init(&priv->ibuf, fd);
-
- 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);
-
- table_proc_call(priv);
- table_proc_end();
-
- table->t_handle = priv;
-
- return (1);
-}
-
-static int
-table_proc_update(struct table *table)
-{
- struct table_proc_priv *priv = table->t_handle;
- int r;
-
- imsg_compose(&priv->ibuf, PROC_TABLE_UPDATE, 0, 0, -1, NULL, 0);
-
- table_proc_call(priv);
- table_proc_read(&r, sizeof(r));
- table_proc_end();
-
- return (r);
-}
-
-static void
-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");
-
- 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;
-
- buf = imsg_create(&priv->ibuf,
- dst ? PROC_TABLE_LOOKUP : PROC_TABLE_CHECK, 0, 0,
- sizeof(s) + strlen(k) + 1);
-
- 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)
- 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();
-
- return (r);
-}
-
-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;
-
- 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)
- 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();
-
- return (r);
-}
-
-struct table_backend table_backend_proc = {
- "proc",
- K_ANY,
- NULL,
- NULL,
- NULL,
- table_proc_open,
- table_proc_update,
- table_proc_close,
- table_proc_lookup,
- table_proc_fetch,
-};
diff --git a/usr.sbin/smtpd/table_procexec.c b/usr.sbin/smtpd/table_procexec.c
new file mode 100644
index 00000000000..c991ed79192
--- /dev/null
+++ b/usr.sbin/smtpd/table_procexec.c
@@ -0,0 +1,319 @@
+/*
+ * Copyright (c) 2013 Eric Faurot <e...@openbsd.org>
+ *
+ * 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.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/types.h>
+#include <sys/queue.h>
+#include <sys/tree.h>
+#include <sys/socket.h>
+#include <sys/wait.h>
+
+#include <ctype.h>
+#include <errno.h>
+#include <event.h>
+#include <fcntl.h>
+#include <imsg.h>
+#include <paths.h>
+#include <poll.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <limits.h>
+#include <unistd.h>
+
+#include <err.h>
+#include <inttypes.h>
+
+#include "smtpd.h"
+#include "log.h"
+
+#define PROCEXEC_VERSION "1"
+#define PROCEXEC_TIMEOUT 500
+
+static int table_procexec_open(struct table *);
+static int table_procexec_update(struct table *);
+static void table_procexec_close(struct table *);
+static int table_procexec_lookup(struct table *, enum table_service, const
char *, char **);
+static int table_procexec_fetch(struct table *, enum table_service, char **);
+
+enum procexec_query;
+static int
+table_procexec_helper(struct table *, enum procexec_query, enum table_service,
const char *, char **);
+
+struct table_backend table_backend_procexec = {
+ "proc-exec",
+ K_ANY,
+ NULL,
+ NULL,
+ NULL,
+ table_procexec_open,
+ table_procexec_update,
+ table_procexec_close,
+ table_procexec_lookup,
+ table_procexec_fetch,
+};
+
+struct procexec_handle {
+ FILE *backend_w;
+ FILE *backend_r;
+ int read_fd;
+ pid_t pid;
+ int status;
+ useconds_t msec;
+};
+
+enum procexec_query {
+ T_UPDATE,
+ T_LOOKUP,
+ T_FETCH
+};
+
+static int
+table_procexec_update(struct table *t) {
+ return table_procexec_helper(t, T_UPDATE, (0), NULL, NULL);
+}
+
+static int
+table_procexec_lookup(struct table *t, enum table_service service, const char
*key, char **dst) {
+ return table_procexec_helper(t, T_LOOKUP, service, key, dst);
+}
+
+static int
+table_procexec_fetch(struct table *t, enum table_service service, char **dst){
+ return table_procexec_helper(t, T_FETCH, service, NULL, dst);
+}
+
+static void
+table_set_ready(int fd, short evt, void *arg){
+ char *line = NULL;
+ size_t len = 0;
+ ssize_t nchar;
+
+ struct procexec_handle *pe_handle = (struct procexec_handle *)arg;
+
+ nchar = getline(&line, &len, pe_handle->backend_r);
+
+ if(nchar <= 15 || strncmp(line, "TABLE-INIT|READY", 16) != 0){
+ pe_handle->status = -1;
+ }
+ else{
+ pe_handle->status = 1;
+ }
+ free(line);
+}
+
+static int
+table_procexec_open(struct table *t) {
+ struct procexec_handle *pe_handle;
+ pid_t pid;
+ int sp[2];
+
+ pe_handle = xcalloc(1, sizeof(*pe_handle));
+
+ if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, sp) == -1){
+ fatalx("procexec - socket pair: %s", t->t_name);
+ }
+
+ if ((pid = fork()) == -1) {
+ fatalx("procexec - fork: %s", t->t_name);
+ }
+
+ if (pid > 0) {
+ close(sp[0]);
+ FILE *backend_w, *backend_r;
+ if ((backend_w = fdopen(sp[1], "w")) == NULL)
+ fatalx("procexec - backend_w: %s", t->t_name);
+
+ if ((backend_r = fdopen(sp[1], "r")) == NULL)
+ fatalx("procexec - backend_r: %s", t->t_name);
+ pe_handle->read_fd = sp[1];
+
+ pe_handle->pid = pid;
+ pe_handle->backend_w = backend_w;
+ pe_handle->backend_r = backend_r;
+ pe_handle->status = 0;
+ pe_handle->msec = PROCEXEC_TIMEOUT;
+ t->t_handle = pe_handle;
+
+ fprintf(backend_w, "TABLE-INIT|%s|%d\n", PROCEXEC_VERSION,
PROCEXEC_TIMEOUT);
+ fflush(backend_w);
+
+ struct event *ev = (struct event *)xcalloc(1, sizeof(struct
event));
+ event_set(ev, sp[1], EV_READ, table_set_ready, (void
*)pe_handle);
+ event_add(ev, NULL);
+
+ return 1;
+ }
+ else {
+ close(sp[1]);
+ dup2(sp[0], STDIN_FILENO);
+ dup2(sp[0], STDOUT_FILENO);
+
+ int execr;
+ char exec[_POSIX_ARG_MAX];
+ execr = snprintf(exec, sizeof(exec), "exec %s" , t->t_config);
+ if (execr < 0 || execr >= (int) sizeof(exec))
+ fatalx("procexec - execr: %s", t->t_name);
+ execl("/bin/sh", "/bin/sh", "-c", exec, (char *)NULL);
+ }
+ return 1;
+}
+
+static void
+table_procexec_close(struct table *t) {
+ struct procexec_handle *pe_handle = (struct procexec_handle
*)t->t_handle;
+ fclose(pe_handle->backend_r);
+ fclose(pe_handle->backend_w);
+ close(pe_handle->read_fd);
+ waitpid(pe_handle->pid, 0, 0);
+ free(pe_handle);
+ pe_handle = NULL;
+}
+
+static void
+procexec_clear_io(int fd, short evt, void *arg){
+ char *line;
+ size_t len;
+ ssize_t nchar;
+
+ struct procexec_handle *pe_handle = (struct procexec_handle *)arg;
+
+ line = NULL;
+ len = 0;
+ nchar = getline(&line, &len, pe_handle->backend_r);
+ free(line);
+ pe_handle->status = 1;
+}
+
+static int
+procexec_parse(struct procexec_handle *pe_handle, uint64_t reqid, enum
procexec_query query, char **dst) {
+ char *oline, *line, *tline;
+ size_t len;
+ ssize_t nchar;
+ uint64_t resid;
+
+ oline = NULL;
+ len = 0;
+ struct pollfd pfd = { pe_handle->read_fd, POLLIN, 0 };
+ int ret = 0;
+ ret = poll(&pfd, 1, PROCEXEC_TIMEOUT);
+ if(ret == 1){
+ oline = NULL;
+ len = 0;
+ nchar = getline(&oline, &len, pe_handle->backend_r);
+ }
+ else {
+ log_trace(TRACE_IO, "io: table backend io has timed out");
+ pe_handle->status = -1;
+
+ struct event *ev = (struct event *)xcalloc(1, sizeof(struct
event));
+ event_set(ev, pe_handle->read_fd, EV_READ, procexec_clear_io,
(void *)pe_handle);
+ event_add(ev, NULL);
+
+ goto tempfail;
+ }
+ if (len == 0 || nchar < 0)
+ goto tempfail;
+ line = oline;
+
+ if(len < 13 || strncmp(line, "TABLE-RESULT|", 13) != 0)
+ goto tempfail;
+ line += 13;
+ len -= 13;
+
+ resid = strtoull(line, &tline, 16);
+ if (len < 17 || *tline != '|')
+ goto tempfail;
+ if (errno == ERANGE && resid == ULLONG_MAX)
+ goto tempfail;
+ if (reqid != resid)
+ goto tempfail;
+ line += 17;
+ len -= 17;
+
+ if(strncmp(line, "NOT-FOUND", 9) == 0)
+ goto failure;
+ if(len < 7 || strncmp(line, "FAILURE", 7) == 0 || strncmp(line,
"SUCCESS", 7) != 0)
+ goto failure;
+ line += 8;
+ len -= 8;
+ switch(query){
+ case T_UPDATE:
+ goto success;
+ break;
+ case T_LOOKUP:
+ case T_FETCH:
+ *dst = xcalloc(len, sizeof(*dst));
+ if (strlcpy(*dst, line, len) >= len){
+ free(*dst);
+ goto tempfail;
+ }
+ goto success;
+ break;
+ }
+
+ failure:
+ free(oline);
+ return 0;
+
+ tempfail:
+ free(oline);
+ return -1;
+
+ success:
+ free(oline);
+ return 1;
+}
+
+static int
+table_procexec_helper(struct table *t, enum procexec_query query, enum
table_service service, const char *key, char **dst) {
+ struct procexec_handle *pe_handle;
+ FILE* backend_w;
+ struct timeval tv;
+ uint64_t reqid;
+
+ pe_handle = (struct procexec_handle *)t->t_handle;
+ if(pe_handle == NULL || pe_handle->status != 1){
+ log_trace(TRACE_IO, "io: table backend io is %s",
(pe_handle->status == -1)?"broken":"not ready");
+ return -1;
+ }
+ backend_w = pe_handle->backend_w;
+
+ reqid = generate_uid();
+ gettimeofday(&tv, NULL);
+
+ switch(query){
+ case T_UPDATE:
+ fprintf(backend_w, "TABLE|%s|%lld.%06ld|UPDATE|%016" PRIx64
"\n",
+ PROCEXEC_VERSION, tv.tv_sec, tv.tv_usec, reqid);
+ fflush(backend_w);
+ return procexec_parse(pe_handle, reqid, query, dst);
+ break;
+ case T_LOOKUP:
+ fprintf(backend_w, "TABLE|%s|%lld.%06ld|LOOKUP|%016" PRIx64
"|%s|%s\n",
+ PROCEXEC_VERSION, tv.tv_sec, tv.tv_usec, reqid,
table_service_name(service), key);
+ fflush(backend_w);
+ return procexec_parse(pe_handle, reqid, query, dst);
+ break;
+ case T_FETCH:
+ fprintf(backend_w, "TABLE|%s|%lld.%06ld|FETCH|%016" PRIx64
"|%s\n",
+ PROCEXEC_VERSION, tv.tv_sec, tv.tv_usec, reqid,
table_service_name(service));
+ fflush(backend_w);
+ return procexec_parse(pe_handle, reqid, query, dst);
+ break;
+ }
+ return -1;
+}
