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.
- usr/src/cmd/ha-services/gds-agents/PostgreSQL/control_pgs.ksh
* general, still inconsistent use of rm vs ${RM}
- 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.
- line 1261, can be removed if you use "${CP} -p" in line 1260
- line 1272, why don't you need the chown from line 1260 before
this call?
- usr/src/cmd/ha-services/gds-agents/PostgreSQL/functions_static
- line 535, typo fuction -> function
- usr/src/cmd/ha-services/gds-agents/PostgreSQL/pgs_register.ksh
- line 24, copyright needs to be 2008
- 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?
- 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.
- 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.
- 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.
- 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?
- 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.
- 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.
- line 136, either use ${CP} (which you need to setup), or use
/bin/cp
- line 152, same as line 136, this time either ${RM} or /bin/rm
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
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
