On 6/9/21 5:19 AM, Gilles CHEHADE wrote:
Hi,

I wrote table_procexec (despite the copyright which I copy-pasted and forgot to 
replace author) so just providing a bit of insight:
Ah, I did not know that. I will fix the header in the next patch
table_procexec was written as a proof of concept for a new table protocol 
inspired by the filter protocol to make it easier to write privsep table 
backends using any language or library available without requiring dependencies 
in the daemon.

I implemented it as a table backend because it was the easiest way to show a 
working implementation of the protocol without making changes in the daemon,
the table_procexec backend sits in between the daemon and “new” tables to 
translate old protocol queries into new protocol queries and new protocol 
results into old protocol results,
but the intent was to move the underlying implementation of the table API to 
the protocol and not retain this table proxy.
Do you mean that ALL table backends should be replaced by the new model? Yes, it should be possible to do that but I don't know if that should be done simultaneously as introducing procexec.
Not my call but I don’t think what you’re proposing is the proper way to 
integrate it.

There are a few downsides in plugging it this way but the most annoying one is 
that instead of starting a table with a configuration parameter, this is 
tricking it into executing the table_procexec and using the configuration path 
as the path to the “new” table, which means that you have no way to provide the 
new table with a configuration for itself.
I don't know if this is true. I was able to provide parameters to the procexec executable via

   $ cat /etc/mail/smtpd.conf
   #       $OpenBSD: smtpd.conf,v 1.14 2019/11/26 20:14:38 gilles Exp $

   # This is the smtpd server system-wide configuration file.
   # See smtpd.conf(5) for more information.

   table paliases file:/etc/mail/aliases
   table aliases "proc-exec:/usr/local/bin/aliases_procexec
   root.t...@bsd.ac"
   listen on socket

   # To accept external mail, replace with: listen on all
   #
   listen on lo0

   action "local_mail" mbox alias <aliases>
   action "outbound" relay
   action "bsd.ac" relay host smtp://10.7.0.1

   # Uncomment the following to accept external mail for domain
   "example.org"
   #
   # match from any for domain "example.org" action "local_mail"
   match from local for local action "local_mail"
   match from local for domain "bsd.ac" action "bsd.ac"
   match from local for any action "outbound"


   $ cat /usr/local/bin/aliases_procexec
   #!/bin/ksh

   user="${1:-r...@bsd.ac}"

   while read line
   do
      reqid="$(echo $line | awk -F'|' '{ print $5; }')"
      reply="TABLE-RESULT|$reqid|FOUND|$user"
      echo $reply
   done < /dev/stdin
   exit 0

Does this not work as providing configuration parameters?

So if we replace all backends by procexec model it will be possible to have configuration such as

   table aliasesA file /etc/mail/aliases
   table aliasesB db /etc/mail/aliases.db
   table aliasesC /usr/local/bin/aliases_procexec "root.t...@bsd.ac"
   table <name>  <executable> <configuration>

where the 'file', 'db' can be executables in /usr/libexec/smtpd or absolute paths.

This may be a possible thing to do but maybe it can be done after procexec is tested a bit.

Hopefully I've addressed the proper concerns.

Best,
Aisha


Gilles





On 8 Jun 2021, at 23:04, Aisha Tammy <openbsd.t...@aisha.cc> wrote:

Hi,
  I've attached a slightly updated patch for the procexec.
Ping for someone to take a look :)

Cheers,
Aisha

diff --git a/usr.sbin/smtpd/smtpctl/Makefile b/usr.sbin/smtpd/smtpctl/Makefile
index ef8148be8c9..2e8beff1ad1 100644
--- a/usr.sbin/smtpd/smtpctl/Makefile
+++ b/usr.sbin/smtpd/smtpctl/Makefile
@@ -48,6 +48,7 @@ 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 be934112103..221f24fbdc4 100644
--- a/usr.sbin/smtpd/smtpd.h
+++ b/usr.sbin/smtpd/smtpd.h
@@ -1656,6 +1656,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..debfc8d8ab7 100644
--- a/usr.sbin/smtpd/smtpd/Makefile
+++ b/usr.sbin/smtpd/smtpd/Makefile
@@ -63,6 +63,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 1d82d88b81a..0c67d205065 100644
--- a/usr.sbin/smtpd/table.c
+++ b/usr.sbin/smtpd/table.c
@@ -46,8 +46,8 @@ 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 *);
@@ -59,6 +59,7 @@ static struct table_backend *backends[] = {
        &table_backend_db,
        &table_backend_getpwnam,
        &table_backend_proc,
+       &table_backend_procexec,
        NULL
};

@@ -77,7 +78,7 @@ table_backend_lookup(const char *backend)
        return NULL;
}

-static const char *
+const char *
table_service_name(enum table_service s)
{
        switch (s) {
diff --git a/usr.sbin/smtpd/table_procexec.c b/usr.sbin/smtpd/table_procexec.c
new file mode 100644
index 00000000000..88bfc435fb3
--- /dev/null
+++ b/usr.sbin/smtpd/table_procexec.c
@@ -0,0 +1,346 @@
+/*
+ * 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 <ctype.h>
+#include <errno.h>
+#include <event.h>
+#include <fcntl.h>
+#include <imsg.h>
+#include <paths.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 PROTOCOL_VERSION "1"
+
+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 **);
+
+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;
+       pid_t   pid;
+};
+
+static int
+table_procexec_open(struct table *t) {
+       struct procexec_handle *pe_handle;
+       pid_t pid;
+       int sp[2];
+       int execr;
+       char exec[_POSIX_ARG_MAX];
+
+       if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, sp) == -1){
+               fatalx("procexec - socket pair: %s", t->t_name);
+       }
+
+       pe_handle = xcalloc(1, sizeof(*pe_handle));
+
+       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->pid = pid;
+               pe_handle->backend_w = backend_w;
+               pe_handle->backend_r = backend_r;
+               t->t_handle = pe_handle;
+               return 1;
+       }
+       else {
+               close(sp[1]);
+               dup2(sp[0], STDIN_FILENO);
+               dup2(sp[0], STDOUT_FILENO);
+
+               if (t->t_config[0] != '/')
+                       fatalx("procexec - config needs absolute path: %s", 
t->t_name);
+               execr = snprintf(exec, sizeof(exec), "exec %s" , t->t_config);
+               if (execr >= (int) sizeof(exec))
+                       fatalx("procexec - execr: %s", t->t_name);
+               execl("/bin/sh", "/bin/sh", "-c", exec, (char *)NULL);
+               fatalx("procexec: %s", t->t_name);
+       }
+}
+
+static void
+table_procexec_close(struct table *t){
+       if(t->t_handle == NULL)
+               return;
+       kill(((struct procexec_handle *)t->t_handle)->pid, SIGTERM);
+       free(t->t_handle);
+       t->t_handle = NULL;
+}
+
+static int
+table_procexec_update(struct table *t)
+{
+       struct timeval tv;
+       uint64_t reqid;
+       char *line = NULL;
+       size_t linecap = 0;
+       ssize_t linelen;
+       uint64_t reqid_res;
+       char *qid = NULL;
+       char *ep = NULL;
+
+       reqid = generate_uid();
+       gettimeofday(&tv, NULL);
+
+       FILE* backend_w = ((struct procexec_handle *)t->t_handle)->backend_w;
+       FILE* backend_r = ((struct procexec_handle *)t->t_handle)->backend_r;
+
+       fprintf(backend_w, "TABLE|%s|%lld.%06ld|UPDATE|%016"PRIx64"\n",
+               PROTOCOL_VERSION,
+               tv.tv_sec, tv.tv_usec, reqid);
+       fflush(backend_w);
+
+       linelen = getline(&line, &linecap, backend_r);
+       if (linelen == 0)
+               return 0;
+       line[strcspn(line, "\n")] = '\0';
+
+       if (strncmp(line, "TABLE-RESULT|", 13) != 0)
+               return -1;
+       line += 13;
+
+       qid = line;
+       reqid_res = strtoull(qid, &ep, 16);
+       if (qid[0] == '\0' || *ep != '|')
+               return -1;
+       if (errno == ERANGE && reqid_res == ULLONG_MAX)
+               return -1;
+       if (reqid != reqid_res)
+               return -1;
+
+       line = ep+1;
+
+       return strcmp(line, "UPDATED") == 0;
+}
+
+/*
+static int
+table_procexec_check(int service, struct dict *params, const char *key)
+{
+       struct timeval tv;
+       uint64_t reqid;
+       char *line = NULL;
+       size_t linecap = 0;
+       ssize_t linelen;
+       uint64_t reqid_res;
+       char *qid = NULL;
+       char *ep = NULL;
+
+       reqid = generate_uid();
+       gettimeofday(&tv, NULL);
+
+       fprintf(backend_w, "TABLE|%s|%lld.%06ld|CHECK|%016"PRIx64"|%s|%s\n",
+               PROTOCOL_VERSION,
+               tv.tv_sec, tv.tv_usec, reqid, service_to_name(service), key);
+       fflush(backend_w);
+
+       linelen = getline(&line, &linecap, backend_r);
+       if (linelen == 0)
+               return 0;
+       line[strcspn(line, "\n")] = '\0';
+
+       if (strncmp(line, "TABLE-RESULT|", 13) != 0)
+               return -1;
+       line += 13;
+
+       qid = line;
+       reqid_res = strtoull(qid, &ep, 16);
+       if (qid[0] == '\0' || *ep != '|')
+               return -1;
+       if (errno == ERANGE && reqid_res == ULLONG_MAX)
+               return -1;
+       if (reqid != reqid_res)
+               return -1;
+
+       line = ep+1;
+
+       if (strcmp(line, "FAILURE") == 0)
+               return -1;
+
+       if (strcmp(line, "NOT-FOUND") == 0)
+               return 0;
+
+       if (strcmp(line, "FOUND") == 0)
+               return 1;
+
+       return -1;
+}
+*/
+
+static int
+table_procexec_lookup(struct table *t, enum table_service service, const char 
*key, char **dst) {
+       struct timeval tv;
+       uint64_t reqid;
+       char *line = NULL;
+       size_t linecap = 0;
+       ssize_t linelen;
+       uint64_t reqid_res;
+       char *qid = NULL;
+       char *ep = NULL;
+       size_t sz = 0;
+
+       FILE* backend_w = ((struct procexec_handle *)t->t_handle)->backend_w;
+       FILE* backend_r = ((struct procexec_handle *)t->t_handle)->backend_r;
+
+       reqid = generate_uid();
+       gettimeofday(&tv, NULL);
+
+       fprintf(backend_w, "TABLE|%s|%lld.%06ld|LOOKUP|%016"PRIx64"|%s|%s\n",
+               PROTOCOL_VERSION,
+               tv.tv_sec, tv.tv_usec, reqid, table_service_name(service), key);
+       fflush(backend_w);
+
+       linelen = getline(&line, &linecap, backend_r);
+       if (linelen == 0)
+               return 0;
+       line[strcspn(line, "\n")] = '\0';
+
+       if (strncmp(line, "TABLE-RESULT|", 13) != 0)
+               return -1;
+       line += 13;
+
+       qid = line;
+       reqid_res = strtoull(qid, &ep, 16);
+       if (qid[0] == '\0' || *ep != '|')
+               return -1;
+       if (errno == ERANGE && reqid_res == ULLONG_MAX)
+               return -1;
+       if (reqid != reqid_res)
+               return -1;
+
+       line = ep+1;
+
+       if (strcmp(line, "FAILURE") == 0)
+               return -1;
+
+       if (strcmp(line, "NOT-FOUND") == 0)
+               return 0;
+
+       if (strncmp(line, "FOUND|", 6) == 0) {
+               line = line + 6;
+               sz = strlen(line) + 1;
+               *dst = xcalloc(sz, sizeof(*dst));
+               if (strlcpy(*dst, line, sz) >= sz){
+                       free(*dst);
+                       return -1;
+               }
+               return 1;
+       }
+       return -1;
+}
+
+static int
+table_procexec_fetch(struct table *t, enum table_service service, char **dst){
+       struct timeval tv;
+       uint64_t reqid;
+       char *line = NULL;
+       size_t linecap = 0;
+       ssize_t linelen;
+       uint64_t reqid_res;
+       char *qid = NULL;
+       char *ep = NULL;
+       size_t sz = 0;
+
+       FILE* backend_w = ((struct procexec_handle *)t->t_handle)->backend_w;
+       FILE* backend_r = ((struct procexec_handle *)t->t_handle)->backend_r;
+
+       reqid = generate_uid();
+       gettimeofday(&tv, NULL);
+
+       fprintf(backend_w, "TABLE|%s|%lld.%06ld|FETCH|%016"PRIx64"|%s\n",
+               PROTOCOL_VERSION,
+               tv.tv_sec, tv.tv_usec, reqid, table_service_name(service));
+       fflush(backend_w);
+
+       linelen = getline(&line, &linecap, backend_r);
+       if (linelen == 0)
+               return 0;
+       line[strcspn(line, "\n")] = '\0';
+
+       if (strncmp(line, "TABLE-RESULT|", 13) != 0)
+               return -1;
+       line += 13;
+
+       qid = line;
+       reqid_res = strtoull(qid, &ep, 16);
+       if (qid[0] == '\0' || *ep != '|')
+               return -1;
+       if (errno == ERANGE && reqid_res == ULLONG_MAX)
+               return -1;
+       if (reqid != reqid_res)
+               return -1;
+
+       line = ep+1;
+
+       if (strcmp(line, "FAILURE") == 0)
+               return -1;
+
+       if (strcmp(line, "NOT-FOUND") == 0)
+               return 0;
+
+       if (strncmp(line, "FOUND|", 6) == 0) {
+               sz = strlen(line) + 1;
+               *dst = xcalloc(sz, sizeof(*dst));
+               if (strlcpy(*dst, line+6, sz) >= sz){
+                       free(*dst);
+                       return -1;
+               }
+               return 1;
+       }
+       return -1;
+}


Reply via email to