September 2, 2019 7:29 PM, "Martijn van Duren" <[email protected]> wrote:
> On 9/2/19 6:00 PM, [email protected] wrote: > >> September 2, 2019 5:55 PM, [email protected] wrote: >> >>> September 2, 2019 5:23 PM, "Martijn van Duren" >>> <[email protected]> wrote: >> >> Gilles should probably elaborate, but the way things are now is that >> system(3) is used to start the filters, allowing us to run any arbitrary >> (set of) command(s) as a filter. >> >> Since the filters now in ports are non-interactive commands I proposed >> to move them to /usr/local/libexec/smtpd, which gilles@ is a proponent >> of. This however means that all filters need to be specified by a full >> path, which is not something I would promote. >> >> Hence the proposition of this diff. >>> I don't feel comfortable adding that path to PATH, even if we're going >>> to call system() right behind. >>> >>> Why not detect if the command starts with '/' and prepend libexec directory >>> if that's not the case ? >> >> I also want to rework the command line before it's passed to system() so we >> exec and save some unnecessary processes which are only waiting for a child >> to exit its infinite loop. >> >> To do that, we are going to copy the command anyways so checking if command >> starts with a / and prepending the absolute path is going to be easy. > > Something like this? > can't test right now but comments inlined: > Index: smtpd.c > =================================================================== > RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v > retrieving revision 1.324 > diff -u -p -r1.324 smtpd.c > --- smtpd.c 26 Jul 2019 07:08:34 -0000 1.324 > +++ smtpd.c 2 Sep 2019 17:27:31 -0000 > @@ -1277,6 +1277,8 @@ fork_processor(const char *name, const c > int sp[2], errfd[2]; > struct passwd *pw; > struct group *gr; > + char exec[_POSIX_ARG_MAX]; I don't know if _POSIX_ARG_MAX is the proper define to use, I genuinely don't know so someone more knowledgeable should jump in. > + int execr; > > if (user == NULL) > user = SMTPD_USER; > @@ -1340,6 +1342,14 @@ fork_processor(const char *name, const c > signal(SIGHUP, SIG_DFL) == SIG_ERR) > err(1, "signal"); > > + if (command[0] == '/') > + execr = snprintf(exec, sizeof(exec), "exec %s", command); > + else > + execr = snprintf(exec, sizeof(exec), "exec %s/filter-%s", > + PATH_LIBEXEC, command); I don't really like that 'filter-' is implicit. So if I specify full path it's filter-rspamd, when i don't it's rspamd, which is confusing because that's also the name of a software on my system. I think people can live with typing 'filter-' and we keep it explicit. > + if (execr >= (int) sizeof(exec)) > + errx(1, "%s: exec path too long", name); > + > /* > * Wait for lka to acknowledge that it received the fd. > * This prevents a race condition between the filter sending an error > @@ -1350,7 +1360,7 @@ fork_processor(const char *name, const c > */ > if (read(STDERR_FILENO, &buf, 1) != 0) > errx(1, "lka didn't properly close write end of error socket"); > - if (system(command) == -1) > + if (system(exec) == -1) > err(1, NULL); > > /* there's no successful exit from a processor */ > Index: smtpd.conf.5 > =================================================================== > RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v > retrieving revision 1.221 > diff -u -p -r1.221 smtpd.conf.5 > --- smtpd.conf.5 17 Aug 2019 14:43:21 -0000 1.221 > +++ smtpd.conf.5 2 Sep 2019 17:27:31 -0000 > @@ -383,6 +383,11 @@ filter > .Ar filter-name > from > .Ar command . > +If > +.Ar command > +starts with a slash it is executed with an absolute path, > +else it will be prepended with > +.Dq /usr/local/libexec/smtpd/filter- . > .It Ic include Qq Ar pathname > Replace this directive with the content of the additional configuration > file at the absolute > @@ -757,6 +762,11 @@ Register an external process named > from > .Ar command . > Such processes may be used to share the same instance between multiple > filters. > +If > +.Ar command > +starts with a slash it is executed with an absolute path, > +else it will be prepended with > +.Dq /usr/local/libexec/smtpd/filter- . > .It Ic queue Cm compression > Store queue files in a compressed format. > This may be useful to save disk space.
