Download raw body.
smtpd: Give filters their own syslog tag
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 <inttypes.h>
#include <stdlib.h>
#include <string.h>
+#include <syslog.h>
#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 <errno.h>
#include <time.h>
+#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 <stdarg.h>
+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 <errno.h>
#include <ifaddrs.h>
#include <inttypes.h>
+#include <libgen.h>
#include <resolv.h>
#include <stdlib.h>
#include <string.h>
@@ -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)
{
smtpd: Give filters their own syslog tag