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
*****************************************************************************
