On 2014-12-01 at 02:39 +0000, Viktor Dukhovni wrote:
> On Mon, Dec 01, 2014 at 12:48:47AM +0100, Heiko Schlittermann wrote:
> > the above line needs to be:
> >
> > av_scanner = cmdline:{ /bin/scan %s || true; }:<trigger>:<re>
> >
> > which is quite ugly. At least it should find it's way into the spec
> > that we're using popen() and /bin/sh and that such commands will work.
> >
> > (I'm not sure about security implications from using popen() and
> > /bin/sh).
>
> It largely boils down to what might be substituted for '%s'. Instead of
> "true" I would use the ":" shell built-in:
>
> /bin/may_only_appear_to_fail || : ignore
The `%s` is directory-name created by Exim and is not influenced by
anything in the email; it's "safe".
scan_filename = string_sprintf("%s/scan/%s/%s.eml",
spool_directory, message_id, message_id);
The result of that gets truncated at the last `/` to get the
directory-name to pass to the cmdline scanner.
Since Exim configuration files must be owned by root, or a trusted user
_other_ than the Exim runtime user, `spool_directory` can't be coerced
to be a malicious value unless someone does something especially stupid
with macros. Meanwhile, `message_id` is the _Exim_ message-id, such as
`1XvGum-000F7a-TQ`, not anything from the received mail.
As to `:` vs `true` -- it's a non-issue. Syntactically they're the
same, they make no difference to parsing or safety. The only difference
between `:` and `true` is that POSIX requires `:` to be a built-in,
while `true` is only de-facto built-in. So the issue comes down to
"what should be documented as a guide for people to use"; humans not
trained in pedantry tend to not treat punctuation as significant and
colon vs semi-colon is hard to observe for said non-pedants. In this, I
am labelling anyone who is adept in shell as a de-facto pedant. This
includes myself.
So while I tend to use the >>: ${foo:=default}<< idiom myself, and think
colon makes sense for code, I _don't_ think that it makes sense to use
it in documentation of walk-through examples. We'll spend too much time
correcting peoples' problems where they "slightly" messed up the
punctuation.
Documentation should be as easy to comprehend as is practical, with the
least amount of subtlety in examples as is practical. Thus if we are
going to document how to work around return value checking, then I
strongly endorse using `true` not `:`.
That said: unless there's a reason to break working configurations, we
should be extremely reluctant to do so absent a major version bump or a
security motivation.
We may need to consider either `cmdline-checked:` or
`cmdline/options/here:` as syntaxes (syntaces?) and, for "historical
compatibility" reasons, default a bare `cmdline:` to "return code
ignored".
I can't help but think about the `ignore_status` and `temp_errors`
options on pipe transports and wonder if there's something there we can
use as a naming convention. `cmdline/no_ignore_status:` ?
-Phil
