From: Martijn van Duren Subject: smtpd: Give filters their own syslog tag To: tech@openbsd.org Date: Fri, 20 Feb 2026 10:18:25 +0100 EHLO, A few months ago Leo Unglaub send me an e-mail asking about the position of the mail ID inside the syslog line, more specifically that we currently prepend the filter-name before it; Making it hard to find the ID with automated tools. While I'm not a fan of automated parsing of log-files, there is something to say for having consistency in the placement, and not forcing in the filtername. I would like to propose the following diff, making use of syslog_r to give every filter their own tag. Since tags can only have alpha- and up to 32 characters (RFC3164 section 4.1.3) I made it the default to use the first 32 alpha-characters from the filter-name, and if that result is too ugly an admin can use the new tag keyword to create their own one. syslog_r isn't a portable function, but I've already discussed a couple of potential solutions with op@, so there shouldn't be any problems in that area. OK? martijn@ diff 6a338f3f70b7c314d32b889df3a82ffbbd5bf816 d28c12b27e3c9e7a32cd929c1010839a90f63a6a commit - 6a338f3f70b7c314d32b889df3a82ffbbd5bf816 commit + d28c12b27e3c9e7a32cd929c1010839a90f63a6a blob - 6b08adde4f558aa96d67233b20d869b9deb58e35 blob + 1e1739f6b7236cea0871ef42bdb8ef060cd945af --- usr.sbin/smtpd/lka.c +++ usr.sbin/smtpd/lka.c @@ -56,7 +56,8 @@ lka_imsg(struct mproc *p, struct imsg *imsg) struct msg m; union lookup lk; char buf[LINE_MAX]; - const char *tablename, *username, *password, *label, *procname; + const char *tablename, *username, *password, *label; + const char *procname, *tag; uint64_t reqid; int v; struct timeval tv; @@ -344,6 +345,7 @@ lka_imsg(struct mproc *p, struct imsg *imsg) case IMSG_LKA_PROCESSOR_FORK: m_msg(&m, imsg); m_get_string(&m, &procname); + m_get_string(&m, &tag); m_get_u32(&m, &subsystems); m_end(&m); @@ -351,7 +353,7 @@ lka_imsg(struct mproc *p, struct imsg *imsg) m_add_string(p, procname); m_close(p); - lka_proc_forked(procname, subsystems, imsg_get_fd(imsg)); + lka_proc_forked(procname, tag, subsystems, imsg_get_fd(imsg)); return; case IMSG_LKA_PROCESSOR_ERRFD: blob - 6c53c8ea65040c96de45a2360641b7e030acf17b blob + f10652c2a482e0f9fbbf3d842e7389939eb8471a --- usr.sbin/smtpd/lka_filter.c +++ usr.sbin/smtpd/lka_filter.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "smtpd.h" #include "log.h" @@ -165,6 +166,7 @@ static struct dict processors; struct processor_instance { char *name; + struct syslog_data sd; struct io *io; struct io *errfd; int ready; @@ -204,7 +206,7 @@ lka_proc_config(struct processor_instance *pi) } void -lka_proc_forked(const char *name, uint32_t subsystems, int fd) +lka_proc_forked(const char *name, const char *tag, uint32_t subsystems, int fd) { struct processor_instance *processor; @@ -215,6 +217,10 @@ lka_proc_forked(const char *name, uint32_t subsystems, processor = xcalloc(1, sizeof *processor); processor->name = xstrdup(name); + processor->sd = (struct syslog_data)SYSLOG_DATA_INIT; + /* Make sure it persists */ + tag = xstrdup(tag); + openlog_r(tag, LOG_ODELAY, LOG_MAIL, &processor->sd); processor->io = io_new(); processor->subsystems = subsystems; @@ -236,7 +242,7 @@ lka_proc_errfd(const char *name, int fd) processor->errfd = io_new(); io_set_fd(processor->errfd, fd); - io_set_callback(processor->errfd, processor_errfd, processor->name); + io_set_callback(processor->errfd, processor_errfd, processor); lka_proc_config(processor); } @@ -310,14 +316,15 @@ processor_io(struct io *io, int evt, void *arg) static void processor_errfd(struct io *io, int evt, void *arg) { - const char *name = arg; + struct processor_instance *processor = arg; char *line = NULL; ssize_t len; switch (evt) { case IO_DATAIN: - while ((line = io_getline(io, &len)) != NULL) - log_warnx("%s: %s", name, line); + while ((line = io_getline(io, &len)) != NULL) { + log_warnx_r(&processor->sd, "%s", line); + } } } blob - 7ec8ca42e18d1c57b84e27a564c23ff352c267cf blob + e9ca3ae6ff21edda39ae9dddb77d074f0b32b158 --- usr.sbin/smtpd/log.c +++ usr.sbin/smtpd/log.c @@ -24,31 +24,12 @@ #include #include +#include "log.h" + static int debug; static int verbose; const char *log_procname; -void log_init(int, int); -void log_procinit(const char *); -void log_setverbose(int); -int log_getverbose(void); -void log_warn(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_warnx(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_info(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void log_debug(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -void logit(int, const char *, ...) - __attribute__((__format__ (printf, 2, 3))); -void vlog(int, const char *, va_list) - __attribute__((__format__ (printf, 2, 0))); -__dead void fatal(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); -__dead void fatalx(const char *, ...) - __attribute__((__format__ (printf, 1, 2))); - void log_init(int n_debug, int facility) { @@ -96,21 +77,26 @@ logit(int pri, const char *fmt, ...) void vlog(int pri, const char *fmt, va_list ap) { - char *nfmt; + vlog_r(NULL, pri, fmt, ap); +} + +void +vlog_r(struct syslog_data *data, int pri, const char *fmt, va_list ap) +{ int saved_errno = errno; if (debug) { - /* best effort in out of mem situations */ - if (asprintf(&nfmt, "%s\n", fmt) == -1) { - vfprintf(stderr, fmt, ap); - fprintf(stderr, "\n"); - } else { - vfprintf(stderr, nfmt, ap); - free(nfmt); - } + if (data) + fprintf(stderr, "%s: ", data->log_tag); + vfprintf(stderr, fmt, ap); + fprintf(stderr, "\n"); fflush(stderr); - } else - vsyslog(pri, fmt, ap); + } else { + if (data) + vsyslog_r(pri, data, fmt, ap); + else + vsyslog(pri, fmt, ap); + } errno = saved_errno; } @@ -154,6 +140,16 @@ log_warnx(const char *emsg, ...) } void +log_warnx_r(struct syslog_data *data, const char *emsg, ...) +{ + va_list ap; + + va_start(ap, emsg); + vlog_r(data, LOG_ERR, emsg, ap); + va_end(ap); +} + +void log_info(const char *emsg, ...) { va_list ap; blob - 66db31ff29bf087516a4bd6748974ac5c77f16c7 blob + c8da7731c0b6f4d510f3976a97291cecf8d39560 --- usr.sbin/smtpd/log.h +++ usr.sbin/smtpd/log.h @@ -21,6 +21,8 @@ #include +struct syslog_data; + void log_init(int, int); void log_procinit(const char *); void log_setverbose(int); @@ -29,6 +31,8 @@ void log_warn(const char *, ...) __attribute__((__format__ (printf, 1, 2))); void log_warnx(const char *, ...) __attribute__((__format__ (printf, 1, 2))); +void log_warnx_r(struct syslog_data *, const char *, ...) + __attribute__((__format__ (printf, 2, 3))); void log_info(const char *, ...) __attribute__((__format__ (printf, 1, 2))); void log_debug(const char *, ...) @@ -37,6 +41,8 @@ void logit(int, const char *, ...) __attribute__((__format__ (printf, 2, 3))); void vlog(int, const char *, va_list) __attribute__((__format__ (printf, 2, 0))); +void vlog_r(struct syslog_data *, int, const char *, va_list) + __attribute__((__format__ (printf, 3, 0))); __dead void fatal(const char *, ...) __attribute__((__format__ (printf, 1, 2))); __dead void fatalx(const char *, ...) blob - b4cf1f21ddb02dce7a4911285e33eebfcf517067 blob + 64e11577147c808532be6bd0f30fc0bf81e549e2 --- usr.sbin/smtpd/parse.y +++ usr.sbin/smtpd/parse.y @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -145,6 +146,8 @@ static int host_v4(struct listen_opts *); static int host_v6(struct listen_opts *); static int host_dns(struct listen_opts *); static int interface(struct listen_opts *); +#define TAG_MAX 32 +static const char *processor_maketag(const char *); int delaytonum(char *); int is_if_in_group(const char *, const char *); @@ -450,6 +453,8 @@ PROC STRING STRING { processor = xcalloc(1, sizeof *processor); processor->command = $3; } proc_params { + if (!processor->tag) + processor->tag = processor_maketag(processor->command); dict_set(conf->sc_filter_processes_dict, $2, processor); processor = NULL; } @@ -481,6 +486,28 @@ USER STRING { } processor->chroot = $2; } +| TAG STRING { + size_t i; + + if (processor->tag) { + yyerror("tag already specified for this processor"); + free($2); + YYERROR; + } + for (i = 0; $2[i] != '\0'; i++) { + if (!isalpha($2[i])) { + yyerror("tag can only contain letters"); + free($2); + YYERROR; + } + } + if (i > TAG_MAX) { + yyerror("tag can only be %d characters", TAG_MAX); + free($2); + YYERROR; + } + processor->tag = $2; +} ; proc_params: @@ -1928,6 +1955,8 @@ FILTER STRING PROC_EXEC STRING { filter_config->proc = xstrdup($2); dict_set(conf->sc_filters_dict, $2, filter_config); } proc_params { + if (!processor->tag) + processor->tag = processor_maketag(processor->command); dict_set(conf->sc_filter_processes_dict, filter_config->proc, processor); processor = NULL; filter_config = NULL; @@ -3648,3 +3677,24 @@ config_lo_mask_source(struct listen_opts *lo) { return 0; } +/* Best effort. If it's ugly: use the tag parameter */ +static const char * +processor_maketag(const char *cmd) +{ + char path[PATH_MAX]; + int i, j; + char *tag; + + strlcpy(path, cmd, sizeof(path)); + for (i = 0; path[i] != '\0' && !isspace(path[i]); i++) + continue; + path[i] = '\0'; + tag = basename(path); + for (i = j = 0; tag[j] != '\0'; j++) { + if (isalpha(tag[j])) + tag[i++] = tag[j]; + } + tag[i] = '\0'; + + return xstrndup(tag, TAG_MAX); +} blob - a9f39543596a748985fd3011418d29ddc4ad2d1f blob + d1165d56c9dd8c0ab5bfa882dc3706f1b5c7642f --- usr.sbin/smtpd/smtpd.c +++ usr.sbin/smtpd/smtpd.c @@ -77,7 +77,7 @@ static void load_pki_tree(void); static void load_pki_keys(void); static void fork_filter_processes(void); -static void fork_filter_process(const char *, const char *, const char *, const char *, const char *, uint32_t); +static void fork_filter_process(const char *, struct filter_proc *); enum child_type { CHILD_DAEMON, @@ -1298,11 +1298,11 @@ fork_filter_processes(void) iter = NULL; while (dict_iter(env->sc_filter_processes_dict, &iter, &name, (void **)&fp)) - fork_filter_process(name, fp->command, fp->user, fp->group, fp->chroot, fp->filter_subsystem); + fork_filter_process(name, fp); } static void -fork_filter_process(const char *name, const char *command, const char *user, const char *group, const char *chroot_path, uint32_t subsystems) +fork_filter_process(const char *name, struct filter_proc *fp) { pid_t pid; struct filter_proc *processor; @@ -1312,14 +1312,14 @@ fork_filter_process(const char *name, const char *comm struct group *gr; char exec[_POSIX_ARG_MAX]; int execr; + const char *user; - if (user == NULL) - user = SMTPD_USER; + user = fp->user != NULL ? fp->user : SMTPD_USER; if ((pw = getpwnam(user)) == NULL) fatal("getpwnam"); - if (group) { - if ((gr = getgrnam(group)) == NULL) + if (fp->group) { + if ((gr = getgrnam(fp->group)) == NULL) fatal("getgrnam"); } else { @@ -1344,7 +1344,8 @@ fork_filter_process(const char *name, const char *comm close(errfd[0]); m_create(p_lka, IMSG_LKA_PROCESSOR_FORK, 0, 0, sp[1]); m_add_string(p_lka, name); - m_add_u32(p_lka, (uint32_t)subsystems); + m_add_string(p_lka, fp->tag); + m_add_u32(p_lka, (uint32_t)fp->filter_subsystem); m_close(p_lka); return; } @@ -1355,9 +1356,9 @@ fork_filter_process(const char *name, const char *comm dup2(sp[0], STDOUT_FILENO); dup2(errfd[0], STDERR_FILENO); - if (chroot_path) { - if (chroot(chroot_path) != 0 || chdir("/") != 0) - fatal("chroot: %s", chroot_path); + if (fp->chroot) { + if (chroot(fp->chroot) != 0 || chdir("/") != 0) + fatal("chroot: %s", fp->chroot); } if (setgroups(1, &gr->gr_gid) || @@ -1376,11 +1377,11 @@ fork_filter_process(const char *name, const char *comm signal(SIGHUP, SIG_DFL) == SIG_ERR) fatal("signal"); - if (command[0] == '/') - execr = snprintf(exec, sizeof(exec), "exec %s", command); + if (fp->command[0] == '/') + execr = snprintf(exec, sizeof(exec), "exec %s", fp->command); else execr = snprintf(exec, sizeof(exec), "exec %s/%s", - PATH_LIBEXEC, command); + PATH_LIBEXEC, fp->command); if (execr >= (int) sizeof(exec)) fatalx("%s: exec path too long", name); blob - 2fb14bfe1ca7b7156dcf593d1935547f99999506 blob + 8430d7024b190d575c29ae52fe1d204f51657384 --- usr.sbin/smtpd/smtpd.conf.5 +++ usr.sbin/smtpd/smtpd.conf.5 @@ -413,7 +413,7 @@ filter backed by the .Ar proc-name process. -.It Ic filter Ar filter-name Ic proc-exec Ar command +.It Ic filter Ar filter-name Ic proc-exec Ar command Op Ar options Register and execute .Qq proc filter @@ -428,6 +428,10 @@ If starts with a slash it is executed with an absolute path, otherwise it will be run from .Dq /usr/local/libexec/smtpd/ . +See +.Ic proc +for available +.Ar options . .It Ic include Qq Ar pathname Replace this directive with the content of the additional configuration file at the absolute @@ -861,7 +865,7 @@ the key length is determined automatically. The default is .Cm none , which disables DHE cipher suites. -.It Ic proc Ar proc-name Ar command +.It Ic proc Ar proc-name Ar command Op Ar options Register an external process named .Ar proc-name from @@ -875,6 +879,45 @@ If starts with a slash it is executed with an absolute path, otherwise it will be run from .Dq /usr/local/libexec/smtpd/ . +.Pp +The following options are supported: +.Bl -tag -width Ds +.It Xo +.Ic user Ar username +.Xc +The +.Ar username +under which the process is to be executed. +The default is _smtpd. +.It Xo +.Ic group Ar group +.Xc +The +.Ar group +under which the process is to be executed. +The default is the primary group of +.Ic user . +.It Xo +.Ic chroot Ar chroot +.Xc +The +.Ar chroot +under which the process is to be executed. +Care must be taken that all dependencies of the +.Ar command +are installed inside the chroot. +.It Xo +.Ic tag Ar tag +.Xc +The +.Ar tag +with which all logmessages of this processor are written to syslog. +.Ar tag +is limited to 32 letters. +The default is a best effort representation of the +.Ar command +name. +.El .It Ic queue Cm compression Store queue files in a compressed format. This may be useful to save disk space. blob - 7e03f17b7d53817029c488d8e8482b2e387f7b2a blob + b1b341ac588043afefcc737bdd0493d26d986788 --- usr.sbin/smtpd/smtpd.h +++ usr.sbin/smtpd/smtpd.h @@ -1054,6 +1054,7 @@ struct filter_proc { const char *user; const char *group; const char *chroot; + const char *tag; int errfd; enum filter_subsystem filter_subsystem; }; @@ -1353,7 +1354,7 @@ int lka(void); /* lka_proc.c */ int lka_proc_ready(void); -void lka_proc_forked(const char *, uint32_t, int); +void lka_proc_forked(const char *, const char *, uint32_t, int); void lka_proc_errfd(const char *, int); struct io *lka_proc_get_io(const char *); @@ -1711,6 +1712,7 @@ int xasprintf(char **, const char *, ...) void *xmalloc(size_t); void *xcalloc(size_t, size_t); char *xstrdup(const char *); +char *xstrndup(const char *, size_t); void *xmemdup(const void *, size_t); char *strip(char *); int io_xprint(struct io *, const char *); blob - f7df1a6d53ca6c11823bcfbc8bedc3751f301c9a blob + 8f13ff663c61f9af192730c0b09524dc0fd614ab --- usr.sbin/smtpd/util.c +++ usr.sbin/smtpd/util.c @@ -76,6 +76,17 @@ xstrdup(const char *str) return (r); } +char * +xstrndup(const char *str, size_t maxlen) +{ + char *r; + + if ((r = strndup(str, maxlen)) == NULL) + fatal("strdup"); + + return (r); +} + void * xmemdup(const void *ptr, size_t size) {