
iidayuus at intellilink
Jul 7, 2010, 3:19 AM
Post #5 of 6
(616 views)
Permalink
|
Hi, I divided a patch. (2010/07/07 17:59), Dejan Muhamedagic wrote: > 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. > I tested this processing. There was not a problem. Many thanks, Yuusuke. >> 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 [at] lists >> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev >> Home Page: http://linux-ha.org/ > _______________________________________________________ > Linux-HA-Dev: Linux-HA-Dev [at] lists > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev > Home Page: http://linux-ha.org/ > -- ---------------------------------------- METRO SYSTEMS CO., LTD YuusukeIida Mail: iidayuus [at] intellilink ----------------------------------------
|