On Wed, Jul 07, 2010 at 07:19:55PM +0900, Yuusuke IIDA wrote:
> >>should this not be much easier by doing
> >>- cmd="su -c \"$variables\""
> >>+ cmd="su -c '$variables'"
> >> ?
> >>no escaping by sed necessary,
> >>except maybe (if you are paranoid)
> >>escaping of ' itself:
> >>sed -e "s/'/'\\\\''/"
> >
> >Good, I'd really rather avoid trying to escape stuff in the user
> >data if possible.
Then maybe we should enforce that the RESKEYs do not contain
shell meta characters at all.
Which would require the user to write a proper wrapper script
for anything more fancy than a few command line options,
and not try to inline shellscripts as oneliners into the cib.
I'd very much prefer that option.
> > Yuusuke-san, can you please test and see if
> >this works for you. Then we can perhaps advise accordingly in the
> >meta-data.
> >
> I tested this processing.
> There was not a problem.
No, you did something else. See below.
> diff -r 8cb5ba3e1d97 heartbeat/anything
> --- a/heartbeat/anything Fri Jun 25 19:54:24 2010 +0200
> +++ b/heartbeat/anything Wed Jul 07 14:42:44 2010 +0900
> @@ -46,6 +46,7 @@
> # does something and then exits.
>
> # Initialization:
> +: ${OCF_RESKEY_login_shell:="true"}
would not "preserve environment" better match the intended use case?
also there is a dedicated option to su with that name (and function).
so we would still do "su - $user -c ...", but optionally as
"su - --preserve-environment $user -c ..."
(or whatever the most "compatible" short options are accros "platforms")
You do:
> - cmd="su - $user -c \"nohup $binfile $cmdline_options >>
> $logfile 2>> $errlogfile & \"'echo \$!' "
> + cmd="su $login_shell $user -c \"nohup $binfile
> $cmdline_options >> $logfile 2>> $errlogfile & \"'echo \$!' "
> -cmdline_options="$OCF_RESKEY_cmdline_options"
> +cmdline_options='$OCF_RESKEY_cmdline_options'
which effectively places the literal, unexpanded
$OCF_RESKEY_cmdline_options into cmd, and only the eval later will
actually expand it. That is different from what I suggested.
This would be more obvious if you did
+ cmd="su $login_shell $user -c \"nohup $binfile \$OCF_RESKEY_cmdline_options
>> $logfile 2>> $errlogfile & \"'echo \$!' "
But now the debug "echo $cmd" will not actually show what command is going to
be executed,
only print the variable name...
And you still need to escape, think of
cmdline_options="; echo thanks::0:0 >> /etc/passwd; true"
does not need to be evil on purpose, maybe just accidentally contains
shell meta characters, or imbalanced quoting while figuring out how to
best write mini shellscripts in cib parameters...
leading to ugly side effects that will go unnoticed for a long time.
What I suggested was:
cmdline_options=$(echo "$OCF_RESKEY_cmdline_options" | sed -e "s/'/'\\\\''/")
(not ``; if you insist on ``, you need to
double the backslashes in sed yet again; I think)
# btw, bash would do: t="'";
cmdline_options=${OCF_RESKEY_cmdline_options//$t/$t\\$t$t}
# or use printf -v cmdline_options "%q" "$OCF_RESKEY_cmdline_options",
# and leave off the enclosing '' later...
# so many ways to do get it wrong ;-)
(actually, we would also need to escape $binfile in the same fashion)
and then put the eval into the su command, and only to word-split
cmdline_options.
something like:
su $login_shell "$user" -c "eval set -- '$cmdline_options'; nohup '$binfile'
\"\...@\" >> \"$logfile\" 2>> \"$errlogfile\" & echo \$!"
(pending typos...)
which still allows for embeded subshells, but they are much less likely
done by accident. It does no longer allow for concatenation (; more;
commands; added;) or redirection.
I could throw together an actual patch implementing these suggestions,
but as I said above, I'd much prefer to simply not do any escaping,
but require the user to write wrapper scripts if he needs something more
fancy than cmdline_options="--future --type=uncertain".
Basically
case $OCF_RESKEY_cmdline_options in
# possibly I forgot some harmless chars, add those to the whitelist.
# but rather whitelist and forget some than blacklist and forget some.
# most definetely do not whitelist: "'*?#`$()[]<>&
*[!\ a-zA-Z0-9/_.%,=+-]*)
exit $OCF_ERROR_ARGS ;;
esac
and the same for binfile.
Less potential to get it wrong in the cib.
Less confusiong quoting and escaping levels,
both in the cib and in the resource agent.
--
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com
DRBD® and LINBIT® are registered trademarks of LINBIT, Austria.
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
