Bug#311546: postgresql-7.4: changing postgres uid breaks pg_ctrcluster
retitle 311546 postgresql-common: pg_ctlcluster should verify that the cluster uid/gid exist severity 311546 minor reassign postgresql-common thanks Hi Alban! [EMAIL PROTECTED] [2005-06-01 20:17 +0200]: pg_ctrlcluster get the postmaster uid and gid from the cluster. It was 31:32 when i created it. I had deleted and recreated the postgres account to update it (you removed its shell and maybe other things so not to miss any part i used the postinst adduser command to recreate it). The new uid/gid where (fake ones) 122:122. D'oh, please don't do such things. System account IDs must not be modified since that breaks all files that are owned by them. You can modify all the other properties (GECOS, shell, whatever) without any problem, but if you change the IDs, you also have to change the owner and group of all files which are owned by the account, and additionally stop all running servers before (they also run under the postgres account). Thus running the postgresql-7.4 init script fired pg_ctrcluster which retrieved the old uid/gid from the cluster (31:32) and changed the process uid/gid to match the cluster one before calling pg_ctl. pg_ctl was running with an inexistant account. Yes, this can lead to all sorts of weirdnesses. I will add a test to pg_ctlcluster that will refuse to start the postmaster if the account is not valid to prevent such failures in the future. And in pg_ctl , l.114 : if [ `$PGPATH/pg_id -u` -eq 0 ] then echo $CMDNAME: cannot be run as root 12 (...) Right, this could be made much more robust with if [ `$PGPATH/pg_id -u` = 0 ] echo This user does not exists . You may have changed the uid of the account used to create this database cluster. You should dump, drop and reinit a the cluster. fi I will implement something like that in pg_ctlcluster in Perl. Maybe all this is too much for a special case which may not appear often as the problem never arise as far as i searched for this error. Please close this bug if you think so. Well, I will close it when I added the check to make the error more obvious. Thanks, Martin -- Martin Pitt http://www.piware.de Ubuntu Developer http://www.ubuntulinux.org Debian Developerhttp://www.debian.org signature.asc Description: Digital signature
Bug#311546: postgresql-7.4: changing postgres uid breaks pg_ctrcluster
Le jeudi 02 juin 2005 à 13:35 +0200, Martin Pitt a écrit : [EMAIL PROTECTED] [2005-06-01 20:17 +0200]: pg_ctrlcluster get the postmaster uid and gid from the cluster. It was 31:32 when i created it. I had deleted and recreated the postgres account to update it (you removed its shell and maybe other things so not to miss any part i used the postinst adduser command to recreate it). The new uid/gid where (fake ones) 122:122. D'oh, please don't do such things. System account IDs must not be modified since that breaks all files that are owned by them. You can modify all the other properties (GECOS, shell, whatever) without any problem, but if you change the IDs, you also have to change the owner and group of all files which are owned by the account, and additionally stop all running servers before (they also run under the postgres account). Thus running the postgresql-7.4 init script fired pg_ctrcluster which retrieved the old uid/gid from the cluster (31:32) and changed the process uid/gid to match the cluster one before calling pg_ctl. pg_ctl was running with an inexistant account. Yes, this can lead to all sorts of weirdnesses. I will add a test to pg_ctlcluster that will refuse to start the postmaster if the account is not valid to prevent such failures in the future. That s way better than my hack . I also though it would be great to manage it at a higher level (less bloat then adding check in every part of the code that deal with uid, better do it at the entry point). Though i still have a comment ... And in pg_ctl , l.114 : if [ `$PGPATH/pg_id -u` -eq 0 ] then echo $CMDNAME: cannot be run as root 12 (...) Right, this could be made much more robust with if [ `$PGPATH/pg_id -u` = 0 ] == would be posix complient. Though i am against the use for test ([]) when testing a command output (instead of a variable). I am addicted to : if `$PGPATH/pg_id -u` ; or use of a temp variable to store the command output. Easier to debug :) (it is pretty hard to debug command output in if [ `command` ] ) echo This user does not exists . You may have changed the uid of the account used to create this database cluster. You should dump, drop and reinit a the cluster. fi I will implement something like that in pg_ctlcluster in Perl. Maybe all this is too much for a special case which may not appear often as the problem never arise as far as i searched for this error. Please close this bug if you think so. Well, I will close it when I added the check to make the error more obvious. I am used to always putting quote around shell variables though this time i found it great you did not. Else i would never have found out why pg_ctl acted in a weird way. At least pg_ctl fails if it runs with a broken uid. With the quote it would not. I feel that the fact this code break if the process uid is invalid is a good thing ... with quote it will think that .../pg_id: Success in sterr is a valid uid when 0 is not. Maybe the check at pg_ctlcluster level is enough :) Thank you for the quick followup. Alban
Bug#311546: postgresql-7.4: changing postgres uid breaks pg_ctrcluster
Hi Alban! Alban browaeys [2005-06-02 19:51 +0200]: Yes, this can lead to all sorts of weirdnesses. I will add a test to pg_ctlcluster that will refuse to start the postmaster if the account is not valid to prevent such failures in the future. That s way better than my hack . I also though it would be great to manage it at a higher level (less bloat then adding check in every part of the code that deal with uid, better do it at the entry point). Though i still have a comment ... This is already fixed in arch head, BTW. if [ `$PGPATH/pg_id -u` = 0 ] == would be posix complient. Though i am against the use for test ([]) when testing a command output (instead of a variable). I am addicted to : if `$PGPATH/pg_id -u` ; But this tests the exit code, not the output. Could be right, of course, but I did not check. I am used to always putting quote around shell variables though this time i found it great you did not. I'm used to that, too. BTW, I did not write pg_ctl, that's upstream code. :-) I wrote the perl infrastructure around it (postgresql-common). Maybe the check at pg_ctlcluster level is enough :) Yes, I agree. Have a nice day and thanks for the report! Martin -- Martin Pitt http://www.piware.de Ubuntu Developer http://www.ubuntulinux.org Debian Developerhttp://www.debian.org signature.asc Description: Digital signature
Bug#311546: postgresql-7.4: changing postgres uid breaks pg_ctrcluster
Package: postgresql-7.4
Version: 1:7.4.8-4
Severity: normal
pg_ctrlcluster get the postmaster uid and gid from the cluster.
It was 31:32 when i created it.
I had deleted and recreated the postgres account to update it
(you removed its shell and maybe other things so not to miss any
part i used the postinst adduser command to recreate it).
The new uid/gid where (fake ones) 122:122.
Thus running the postgresql-7.4 init script fired pg_ctrcluster
which retrieved the old uid/gid from the cluster (31:32) and
changed the process uid/gid to match the cluster one before
calling pg_ctl.
pg_ctl was running with an inexistant account. This leads to this
error:
* Starting PostgreSQL 7.4 database server: main
/usr/lib/postgresql/7.4/bin/pg_id: Success
/usr/lib/postgresql/7.4/bin/pg_ctl: line 114: [: -eq: unary operator expected
the problem is that in pg_id code, l.105:
if (!pw)
{
perror(argv[0]);
exit(1);
}
perror returns pg_id program name and success as (!pw) is true
and no other errors happened.
And in pg_ctl , l.114 :
if [ `$PGPATH/pg_id -u` -eq 0 ]
then
echo $CMDNAME: cannot be run as root 12
(...)
here `$PGPATH/pg_id -u` is not a integer anymore but .../pg_id:
Success.
A quick hack to let it work would be to use:
if $PGPATH/pg_id -u;
then
echo $CMDNAME: cannot be run as root 12
though it would be better if it errors in case the cluster is in
fact owned by no account. I though about:
uid=$( $PGPATH/pg_id -u 2/dev/null);
$status=$?
if [ $status -eq 0 ]
then
if [ $uid -eq 0 ];
then
echo $CMDNAME: cannot be run as root 12
(...)
exit 1
fi
else
echo This user does not exists . You may have changed the uid
of the account used to create this database cluster. You should
dump, drop and reinit a the cluster.
fi
This is a matter of talks as i wonder if pg_id should not return
the uid even if the account does not exists.
uid=31(invalid user)
and let us manage the error in pg_ctl as we wish.
Maybe all this is too much for a special case which may not
appear often as the problem never arise as far as i searched for
this error. Please close this bug if you think so.
Regards
Alban
-- System Information:
Debian Release: 3.1
APT prefers unstable
APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)
Shell: /bin/sh linked to /bin/bash
Kernel: Linux 2.6.12-rc5usb-serial
Locale: [EMAIL PROTECTED], [EMAIL PROTECTED] (charmap=UTF-8)
Versions of packages postgresql-7.4 depends on:
ii libc6 2.3.5-1 GNU C Library: Shared libraries an
ii libcomerr2 1.37+1.38-WIP-0509-1 common error description library
ii libkrb531.3.6-3 MIT Kerberos runtime libraries
ii libpam0g0.76-22 Pluggable Authentication Modules l
ii libpq3 1:7.4.8-4PostgreSQL C client library
ii libreadline44.3-15 GNU readline and history libraries
ii libssl0.9.7 0.9.7g-1 SSL shared libraries
ii postgresql-client-7 1:7.4.8-4front-end programs for PostgreSQL
ii postgresql-common 12 manager for PostgreSQL database cl
ii zlib1g 1:1.2.2-4compression library - runtime
-- no debconf information
--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]
