Hi Thorsten, Neil et all, I incorporated the changes in the new webrev under:
http://cr.opensolaris.org/~ulherr/pgshotstandby Please review until Wednesday April 29. Detlef Detlef Ulherr wrote: > Hi Thorsten, Neill et all > > See my replies inline. > > Detlef > > Neil Garthwaite wrote: > >> Hi Detelf, >> >> To add to Thorsten's comments here are mine. >> >> usr/src/cmd/ha-services/gds-agents/PostgreSQL/functions.ksh >> >> - lines 1266/1309/1366/1511/1565, please consider that using "su -" >> you may invoke a profile that requests a prompt. Instead you could >> consider using "su <user> -c "env <var> command etc.)". >> >> > I will give it a try > >> - line 1299, I guess the 5 second timeout for ping is enough, although >> you may want to consider making this a user defined variable. >> >> > will do so, if it is not set, then it will default to five seconds > >> usr/src/cmd/ha-services/gds-agents/PostgreSQL/functions_static.ksh >> >> - line 164, you're missing a ", this means that lines 166-89 will >> never get called. >> >> > Good catch, but the backtick finishes the statement, sometimes ksh is > more robust than expected. Anyway I will change that. > >> usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/functions.ksh >> >> - line 417 "su -" >> >> > I will give it a try. > >> usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/resilver-step1.ksh >> >> - line 218, "can can" >> - line 236, "root root" >> >> > Will fix that. > >> usr/src/cmd/ha-services/gds-agents/PostgreSQL/pgs_register.ksh >> >> - lines 88/226/382/386 you want to quote the text >> >> > Will fix that although it works without quoting. > >> usr/src/cmd/ha-services/gds-agents/PostgreSQL/pgs_smf_register.ksh >> >> - line 69, has a 2006 copyright >> >> > Will fix that. > >> Regards >> Neil >> >> On 11 Apr 2008, at 14:04, Thorsten Frueauf wrote: >> >> >> >>> Hi Detlef et al, >>> >>> here are my comments for you updated webrev: >>> >>> - general, you should verify the indention for scds_syslog messages. >>> The following line of scds_syslog should have one more level of >>> indention, if that is part of the message or args, like >>> >>> 417 scds_syslog -p daemon.err -t >>> $(syslog_tag) -m \ >>> 418 "Function: validate: The %s >>> variable is not set, but it is required" \ >>> 419 "USER" >>> >>> should really look like >>> >>> 417 scds_syslog -p daemon.err -t >>> $(syslog_tag) -m \ >>> 418 "Function: validate: The %s >>> variable is not set, but it is required" \ >>> 419 "USER" >>> >>> The source files sometimes have that right, many times not. Make >>> sure >>> all new >>> scds_syslog messages you add follow that rule - and all new code >>> within the >>> rolechg/* as well. >>> >>> > Will fix that. > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/control_pgs.ksh >>> >>> * general, still inconsistent use of rm vs ${RM} >>> >>> > Will fix that. > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/functions.ksh >>> - line 1260, use "${CP} -p" instead of just "${CP}", that >>> will ensure that the file is not created with the current >>> umask >>> of user root. >>> >>> > Agreed > >>> - line 1261, can be removed if you use "${CP} -p" in line 1260 >>> >>> > Agreed > >>> - line 1272, why don't you need the chown from line 1260 >>> before >>> this call? >>> >>> > I the smf context, the start command is run as the postgres user, so no > chown required here, it is done in the pgs_smf_register. > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/functions_static >>> - line 535, typo fuction -> function >>> >>> > Will fix that > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/pgs_register.ksh >>> - line 24, copyright needs to be 2008 >>> >>> > Will fix that. > >>> - the code line 81-103 and 219-237 seems to do the exact same >>> thing. >>> Could you deduplicate it by calling it in a generic >>> portion that >>> is true for global and non-global zone usage? >>> >>> > Will do that > >>> - line 383, displaying the passphrase on the screen is not a >>> secure >>> way to get the passphrase from the user. Disable echo to the >>> tty and ask for the passphrase twice, compare both inputs >>> and >>> only >>> succeed if they are identical. The passphrase must not be >>> displayed >>> on the screen. >>> >>> >>> > Will fix that. > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/ >>> control_rolechg.ksh >>> - general - since this is new code, make sure every command >>> you >>> call has the full path prefixed. e.g. line 93, 134, 158 call >>> just rm >>> instead of /bin/rm. >>> Note that e.g. Indiana has no longer /bin or /usr/bin as >>> first in >>> $PATH - so you might experience undesired effects. Same is >>> true >>> if PATH for thr root user is changed. >>> >>> >>> > Will fix that > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/functions.ksh >>> usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/resilver- >>> step1.ksh >>> usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/resilver- >>> step2.ksh >>> usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/ >>> rolechg_config.ksh >>> - line 27 or 28, ident string is not expanded, while the ident >>> string >>> on some other new files are. Make sure this is correct. >>> >>> > This is correct, it is because of the change history > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/resilver- >>> step1.ksh >>> usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/resilver- >>> step2.ksh >>> - generic, why do you check "if [ -d /etc/cluster ]"? >>> would checking with "clinfo" for cluster mode not being >>> better? >>> >>> > Not in this case, the difference between the directory test and clinfo > is, that clinfo reports false if a node is not booted in the cluster, > while the directory test is for a configured cluster. When a user first > copies the scripts and edits it, the validation should result in a > correct script regardless whether or not the nod is currently booted in > cluster mode or not. > >>> - generic, I assume it is important that resilver-step2.ksh is >>> only called after resilver-step1.ksh has been run. >>> Therefor I suggest resilver-step1.ksh creates a file within >>> /tmp or /var/tmp which resilver-step2.ksh looks for, only >>> if it exists it proceeds, otherwise it aborts or warns. >>> resilver-step2.ksh would then remove that file after a >>> successful >>> run. >>> >>> > Good suggestion. > >>> - usr/src/cmd/ha-services/gds-agents/PostgreSQL/rolechg/ >>> rolechg_register.ksh >>> - line 61, I know currently there is nothing to do for a >>> probe, >>> but I would recommend to call /usr/bin/true from the >>> control_rolechg >>> command and add a switch for the probe option. That way, >>> if for >>> some reason you find out it would be good to do something >>> during >>> probe, this can be introduced via patch without requiring >>> the >>> user >>> to re-register. >>> >>> > Will do that > >>> - line 136, either use ${CP} (which you need to setup), or use >>> /bin/cp >>> >>> > Will do that > >>> - line 152, same as line 136, this time either ${RM} or /bin/ >>> rm >>> >>> > Will do that > >>> Greets >>> Thorsten >>> >>> >>> Detlef Ulherr wrote: >>> >>> >>>> Hi Thorsten et all >>>> >>>> The changes are made and available in the current webrev under >>>> http://cr.opensolaris.org/~ulherr/pgshotstandby >>>> >>>> Comments are appreciated >>>> >>>> Detlef >>>> >>>> Detlef Ulherr wrote: >>>> >>>> >>>>> Hi Thorsten, >>>>> >>>>> I will combine Tims and your suggestion. >>>>> >>>>> Detlef >>>>> >>>>> Thorsten Frueauf wrote: >>>>> >>>>> >>>>> >>>>>> Hi Detlef, Tim et al, >>>>>> >>>>>> I would agree if we are in an interactive user session. We are not. >>>>>> >>>>>> Said customers likely also have in their policy that a password or >>>>>> passphrase must not be written down or stored in cleartext. Now >>>>>> what? >>>>>> >>>>>> Anyway, I gave my review comment and reasons as of why I would >>>>>> recommend against such an option completely. >>>>>> >>>>>> I hope others add their own or other potential customer >>>>>> perspectives. >>>>>> >>>>>> In any case, the existing code does not satisfy basic security >>>>>> concerns. I would also recommend against an variable option for the >>>>>> passphrases within pgs_config for the reasons I cited way below. If >>>>>> you still want to use such a passphrase, ask for it during >>>>>> registration and store it right away in the destination and make >>>>>> sure >>>>>> the permission and owner is set _before_ you write it in there (not >>>>>> after). >>>>>> >>>>>> Greets >>>>>> Thorsten >>>>>> >>>>>> Detlef Ulherr wrote: >>>>>> >>>>>> >>>>>> >>>>>>> Hi Thorsten, Tim et all >>>>>>> >>>>>>> I second Tims position, if you search in google how to set up >>>>>>> ssh you >>>>>>> find the advice "never go without a passphrase" So it is likely >>>>>>> that we >>>>>>> hit the scenario where a customer does not allow the >>>>>>> passwordless login >>>>>>> without a passphrase. >>>>>>> >>>>>>> My former employer (names do not belong in public discussions) >>>>>>> was one >>>>>>> of these. The existence of the passphrases was checked and >>>>>>> enforced at >>>>>>> audits. To pass the audit without a passphrase it needed a very >>>>>>> good >>>>>>> reason. >>>>>>> >>>>>>> Detlef >>>>>>> >>>>>>> Tim Read - Staff Engineer Solaris Availability Engineering wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> Thorsten, >>>>>>>> >>>>>>>> I agree, security by obscurity won't work. But if customer have a >>>>>>>> policy of requiring passphrases, this module will not be able to >>>>>>>> handle that situation. We cannot insist that users change their >>>>>>>> security standards. >>>>>>>> >>>>>>>> I think we could make it quite difficult for the intermediate >>>>>>>> file >>>>>>>> to be intercepted. It all depends on the scheduling between: >>>>>>>> >>>>>>>> root# echo "password" > cleartext_password >>>>>>>> root# chmod 400 cleartext_password >>>>>>>> [A] >>>>>>>> root# chown postgres cleartext_password >>>>>>>> root# /bin/su postgres -c "( /bin/cat cleartext_password; \ >>>>>>>> /bin/rm cleartext_password) | some command" >>>>>>>> [B] >>>>>>>> >>>>>>>> It's not inconceivable that a tight loop could pick this up >>>>>>>> between >>>>>>>> [A] and [B], though the sudden spike in CPU usage might give the >>>>>>>> perpetrator away. >>>>>>>> >>>>>>>> Away, I don't think adding this feature makes it any worse, but >>>>>>>> it >>>>>>>> does allow us to comply with customers requirements here should >>>>>>>> they >>>>>>>> have one. >>>>>>>> >>>>>>>> Since this is an open process, we should see what others think, >>>>>>>> after all, this is about meeting user requirements. >>>>>>>> >>>>>>>> Regards, >>>>>>>> >>>>>>>> Tim >>>>>>>> --- >>>>>>>> >>>>>>>> >>>>>>>> Thorsten Frueauf wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> Hi Tim et al, >>>>>>>>> >>>>>>>>> the current implementation in the webrev is surly different from >>>>>>>>> what you describe below. >>>>>>>>> >>>>>>>>> But anyway, security by obscurity is not going to work, since >>>>>>>>> the >>>>>>>>> agent code is a) open source and b) available for anyone local >>>>>>>>> to >>>>>>>>> the system within /opt/SUNWscPostgreSQL/*. >>>>>>>>> >>>>>>>>> Thus if you assume a local compromised system it would be an >>>>>>>>> easy >>>>>>>>> task to now also intercept the moment where the password file is >>>>>>>>> made available. >>>>>>>>> >>>>>>>>> The passwordless access via ssh using authorized keys is a >>>>>>>>> common >>>>>>>>> used technique. If this solution requires ssh in that form I >>>>>>>>> am not >>>>>>>>> sure what other option there is for the user. Adding the ssh >>>>>>>>> passphrase does not increase security in this case - since we >>>>>>>>> are >>>>>>>>> not interactive with a user. >>>>>>>>> >>>>>>>>> I did not write the best practice documents I referred to, but >>>>>>>>> if >>>>>>>>> /usr/ucb/ps works in the way described when assuming that the >>>>>>>>> local >>>>>>>>> PostgreSQL user is breached, this is all you need, since the ssh >>>>>>>>> agent is setup as this user. >>>>>>>>> >>>>>>>>> In summary I am not sure of the option to store the passphrase >>>>>>>>> in >>>>>>>>> cleartext adds any further to the security, but it may pretend a >>>>>>>>> level of security that is not there. >>>>>>>>> >>>>>>>>> Greets >>>>>>>>> Thorsten >>>>>>>>> >>>>>>>>> Tim Read - Staff Engineer Solaris Availability Engineering >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> Thorsten, >>>>>>>>>> >>>>>>>>>> May be I should re-check the code that Detlef has as I >>>>>>>>>> recommended >>>>>>>>>> that he use passphrases. My original idea was that the >>>>>>>>>> passphrase >>>>>>>>>> would be transiently made available to the Postgres user >>>>>>>>>> account >>>>>>>>>> by some controlling root script that wrote it into a Postgres >>>>>>>>>> owned (chmod 400) file. The Postgres user, probably via su, >>>>>>>>>> would >>>>>>>>>> then consume that passphrase and then delete the file. Thus the >>>>>>>>>> file would be present for no more than a few seconds, if that. >>>>>>>>>> >>>>>>>>>> The (limited?) advantage is that if the Postgres has been >>>>>>>>>> compromised locally, it doesn't mean that the account is >>>>>>>>>> compromised remotely unless they know of this mechanism and/or >>>>>>>>>> manage to intercept the file. >>>>>>>>>> >>>>>>>>>> I believe the link below has an inaccuracy: /usr/ucb/ps -aeww >>>>>>>>>> command does not expose the environment of other users to each >>>>>>>>>> other. Only root can view anyone's environment and individual >>>>>>>>>> users can only see theirs. This is exactly what you would >>>>>>>>>> expect. >>>>>>>>>> May be this has changed since the document was written. >>>>>>>>>> >>>>>>>>>> In summary, I don't think all users are comfortable with >>>>>>>>>> accounts >>>>>>>>>> having passwordless ssh. We should not force this requirement >>>>>>>>>> on >>>>>>>>>> them. >>>>>>>>>> >>>>>>>>>> Regards, >>>>>>>>>> >>>>>>>>>> Tim >>>>>>>>>> --- >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Thorsten Frueauf wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>>> The passphrase is optional, and the code works perfect with >>>>>>>>>>>> the >>>>>>>>>>>> authorized keys alone. so it is up to the user to configure >>>>>>>>>>>> the >>>>>>>>>>>> passphrase. The passphrase was added to the code upon >>>>>>>>>>>> feedback >>>>>>>>>>>> as an >>>>>>>>>>>> otional configuration. The administrator who prefers to go >>>>>>>>>>>> with >>>>>>>>>>>> authorized keys and passwordless login between the PostgreSQL >>>>>>>>>>>> users is >>>>>>>>>>>> free to do so. if he wants to add a passphrase to his keys he >>>>>>>>>>>> can do so >>>>>>>>>>>> as well. so it is all about choice >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> Maybe Tim or you can explain as of what additional security >>>>>>>>>>> the >>>>>>>>>>> passphrase is supposed to offer, if the passphrase is stored >>>>>>>>>>> in >>>>>>>>>>> clear text readable by root and the PostgreSQL user. How is >>>>>>>>>>> that >>>>>>>>>>> better from a passwordless access realized through the >>>>>>>>>>> authorized >>>>>>>>>>> keys for the PostgreSQL user? >>>>>>>>>>> >>>>>>>>>>> The authorized key mechanism already needs access to the >>>>>>>>>>> PostgreSQL user .ssh settings. Only then the passwordless >>>>>>>>>>> access >>>>>>>>>>> will work. >>>>>>>>>>> >>>>>>>>>>> Now if already the PostgreSQL user got breached, when using >>>>>>>>>>> the >>>>>>>>>>> ssh passphrase, it then is also available to the one that >>>>>>>>>>> breached the PostgreSQL user. To me there is no additional >>>>>>>>>>> security gain, but being able to use the passphrase adds an >>>>>>>>>>> additional concern. >>>>>>>>>>> >>>>>>>>>>> I would strongly recommend to not use this new >>>>>>>>>>> implementation you >>>>>>>>>>> added for the ssh passphrase. >>>>>>>>>>> >>>>>>>>>>> Maybe have a look at >>>>>>>>>>> http://opensolaris.org/os/community/arc/bestpractices/passwords-cli/ >>>>>>>>>>> and >>>>>>>>>>> http://opensolaris.org/os/community/arc/bestpractices/passwords-files/ >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> The best way is really to avoid that headache completely. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> I see the problem here, that the passphrase is readable at >>>>>>>>>>>> least >>>>>>>>>>>> for >>>>>>>>>>>> root or the PostgreSQL user. But every simple encryption is >>>>>>>>>>>> easy >>>>>>>>>>>> breakable, and does not add very much as well. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> Thats why I recommend to not introduce such a mechanism. It is >>>>>>>>>>> not necessary and does not add security to the authorized key >>>>>>>>>>> option. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> I will restrict the permissions of the parameter file to the >>>>>>>>>>>> PostgreSQL. >>>>>>>>>>>> It has to be the PostgreSQL user sorry, otherwise I then >>>>>>>>>>>> would >>>>>>>>>>>> require a >>>>>>>>>>>> passwordless login for the root user which I really want to >>>>>>>>>>>> avoid. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> Will send my comments for the missing files as soon as I >>>>>>>>>>>>> can. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>> Since the default permission for the pgs_config is 644, I >>>>>>>>>>> would >>>>>>>>>>> assume that any user making a copy will inherit this >>>>>>>>>>> permission. >>>>>>>>>>> And for registration the passphrase will be readable in there. >>>>>>>>>>> And the copy will likely stay in mode 644 as well. >>>>>>>>>>> >>>>>>>>>>> Introducing the ssh passphrase adds more headache than you >>>>>>>>>>> want >>>>>>>>>>> while I don't see the value add. >>>>>>>>>>> >>>>>>>>>>> Reading the cited comments from Tim (I did not see his >>>>>>>>>>> original >>>>>>>>>>> mail) I can also not see where he asked for a ssh passphrase >>>>>>>>>>> implementation. >>>>>>>>>>> >>>>>>>>>>> I am not sure why he says "Having passwordless login is >>>>>>>>>>> probably >>>>>>>>>>> not a good idea." - maybe we can hear the real concern for >>>>>>>>>>> this >>>>>>>>>>> scenario? >>>>>>>>>>> >>>>>>>>>>> Greets >>>>>>>>>>> Thorsten >>>>>>>>>>> >>>>>>>>>>> >>> -- >>> ~ >>> ~ >>> ~ >>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> Sitz der Gesellschaft: >>> Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten >>> Amtsgericht Muenchen: HRB 161028 >>> Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. Roland >>> Boemer >>> Vorsitzender des Aufsichtsrates: Martin Haering >>> >>> ~ >>> ~ >>> ~ >>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> _______________________________________________ >>> ha-clusters-discuss mailing list >>> ha-clusters-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/ha-clusters-discuss >>> >>> >> _______________________________________________ >> ha-clusters-discuss mailing list >> ha-clusters-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/ha-clusters-discuss >> >> > > -- ***************************************************************************** Detlef Ulherr Staff Engineer Tel: (++49 6103) 752-248 Availability Engineering Fax: (++49 6103) 752-167 Sun Microsystems GmbH Amperestra?e 6 mailto:detlef.ulherr at sun.com 63225 Langen http://www.sun.de/ ***************************************************************************** Sitz der Gesellschaft: Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten Amtsgericht Muenchen: HRB 161028 Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer Vorsitzender des Aufsichtsrates: Martin Haering *****************************************************************************
