Hi,

On Tue, Jul 06, 2010 at 08:52:12PM +0200, Lars Ellenberg wrote:
> On Tue, Jul 06, 2010 at 06:36:14PM +0200, Dejan Muhamedagic wrote:
> > Hi Yuusuke-san,
> > 
> > On Wed, Jun 30, 2010 at 08:00:18PM +0900, Yuusuke IIDA wrote:
> > > Hi,
> > > 
> > > For anything RA, I revised it with some function addition.
> > > 
> > > The list of the change is as follows.
> > >  * I added the option which could choose whether you used a login shell 
> > > to want
> > > to let a command inherit an environment variable of Resource Agent.
> > 
> > OK, I assume that this may be useful at times. Though I'm not
> > very happy with the new parameter name, I couldn't come up with
> > another one. The big difference is, I guess, that the .profile
> > files are sourced. Perhaps to name it just "login_shell"?
> 
> the difference is that su - user clears the environment first
> (and then re-populates it from where that user usually gets his environment),
> su user (no dash) does not clear, but inherit the current environment.

OK.

> > >  * I revised it to handle an escape character in character string set by
> > > cmdline_options such as follows adequately.
> > >   --- for example: ---
> > >     primitive AAAAA ocf:heartbeat:anything \
> > >       params \
> > >         binfile="XXXXX" \
> > >         cmdline_options="-V -c \"openssl des-ede3 -d -base64 -k 'yy y'\" 
> > > -i" \
> > >   --- ---
> > 
> > Uh, this escaping gives me headache.
> 
> 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. Yuusuke-san, can you please test and see if
this works for you. Then we can perhaps advise accordingly in the
meta-data.

> As long as we do cmd="su -c \"$variable\"", it is not sufficient to
> escape \ (as the proposed patch by Yuusuke-san does), actually you'd
> need to escape ` and $ and various other things as well.
> Unless you  consider it a feature that these would be
> expanded already in the context of the eval running as root,
> not in the context of the su $user nohup.
> Which is (as it is now) a potential "root exploit",
> once you start taking "cib admin != cluster root" serious.
> which is not really sensible to do IMO, anyways. But I digres.
> 
> Hm. Maybe we should move the eval into that context, anyways?
> sort of
>  cmd="eval '${supposedly_properly_escaped_variable}'"
>  su ... -c "$cmd" ?

Hmm, I hope that the users could skip the acrobatics and do the
processing elsewhere if absolutely needed.

> But, for the record:
> 
> > The line says:
> > 
> > +cmdline_options=`... | sed 's/\\\/\\\\\\\/g' | ...`
> > 
> > How does the left side expand? Shouldn't that be an even number
> > of backslashes? The right side also has 7 backslashes.
> 
> the first "stripping" of \ is done by the shell,
> before feeding the whole thing to the `` subshell.
> And the \ quoting within `` is subtle:
>       backslash retains its literal meaning except when followed by $, `, or \
> so those \/ combinations could have been written as \\/ as well
> (if only to reduce the headache of the reader, slightly)
> but need not be.  BTW, that is one of the differences between `` and $() ...
> yep, its not pretty, but "correct", though not necessarily consistent
> between various shells and versions :(
> 
> god, I hate it when I know these useless facts from the top of my head,
> I wish I had done less shell coding ;-)

:)

Cheers,

Dejan

> > >  * Strip off the trailing clone marker.
> > >   - quotations from the following.
> > > http://hg.clusterlabs.org/pacemaker/stable-1.0/file/94515b3503b5/extra/resources/Dummy#l143
> > 
> > OK.
> > 
> > Can you please split the patch in three parts, so that we have
> > unrelated changes in signel patches.
> 
> Yes, please ;-)
> 
> -- 
> : 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/
_______________________________________________________
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/

Reply via email to