Skip site navigation (1) Skip section navigation (2)

Re: [BUGS] Bug#333854: pg_group file update problems

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Dennis Vshivkov <walrus(at)amur(dot)ru>, 333854(at)bugs(dot)debian(dot)org,submit(at)bugs(dot)debian(dot)org,PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [BUGS] Bug#333854: pg_group file update problems
Date: 2005-10-26 13:54:48
Message-ID: 200510261354.j9QDsmc26752@candle.pha.pa.us (view raw or flat)
Thread:
Lists: pgsql-bugspgsql-patches
This bug will be fixed in the next 8.0.X release.  I have not fixed
7.4.X because the risk/benefit does not warrant it, and 8.1 does not
have this problem.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> 
> I have developed a small patch to 8.0.X that fixes your reported
> problem:
> 
> 	test=> CREATE GROUP g1;
> 	CREATE GROUP
> 
> 	test=> CREATE USER u1 IN GROUP g1;
> 	CREATE USER
> 	test=> \! cat /u/pg/data/global/pg_group
> 	"g1"    "u1"
> 
> 	test=> CREATE USER u2 IN GROUP g1;
> 	CREATE USER
> 	test=> \! cat /u/pg/data/global/pg_group
> 	"g1"    "u1" "u2"
> 
> 	test=> ALTER USER u2 RENAME TO u3;
> 	ALTER USER
> 	test=> \! cat /u/pg/data/global/pg_group
> 	"g1"    "u1" "u3"
> 
> In the patch, notice the old comment that suggests we might need to use
> CommandCounterIncrement().
> 
> This sesms safe to apply to back branches.
> 
> ---------------------------------------------------------------------------
> 
> Dennis Vshivkov wrote:
> > Package: postgresql-8.0
> > Version: 8.0.3-13
> > Severity: important
> > Tags: patch, upstream
> > 
> > Here's the problem:
> > 
> > db=# CREATE GROUP g1;
> > CREATE GROUP
> > db=# CREATE USER u1 IN GROUP g1;                        (1)
> > CREATE USER
> > 
> > # cat /var/lib/postgresql/8.0/main/global/pg_group
> > #
> > 
> > The file gets rewritten, but the group `g1' line does not get
> > added to the file.  Continue:
> > 
> > db=# CREATE USER u2 IN GROUP g1;                        (2)
> > CREATE USER
> > 
> > # cat /var/lib/postgresql/8.0/main/global/pg_group
> > "g1"    "u1"
> > #
> > 
> > Now the line is there, but it lacks the latest member.  Consider
> > this also:
> > 
> > db=# ALTER USER u2 RENAME TO u3;                        (3)
> > ALTER USER
> > 
> > # cat /var/lib/postgresql/8.0/main/global/pg_group
> > "g1"    "u1" "u2"
> > #
> > 
> > The problem is that the code that updates pg_group file resolves
> > group membership through the system user catalogue cache.  The
> > file update happens shortly before the commit, but the caches
> > only see updates after the commit.  Because of this, new users
> > or changes in users' names often do not make it to pg_group.
> > That leads to mysterious authentication failures subsequently.
> > The problem can also have security implications for certain
> > pg_hba.conf arrangements.
> > 
> > The attached `98-6-pg_group-stale-data-fix.patch' makes the code
> > in question access the system user table directly and thus fixes
> > the cases (1) and (2), however (3) is doubly ill: the user
> > renaming code does not even trigger a pg_group file update.
> > Hence the other patch, `98-5-rename-user-update-pg_group.patch'.
> > 
> > A byproduct of the main fix is removal of an unlikely system
> > cache reference leak which happens if a group member name
> > contains a newline.
> > 
> > The problems were found and the fixes were done for PostgreSQL
> > 8.0.3 release.  The flaws seem intact in 8.0.4 source code, too.
> > 
> > Hope this helps.
> > 
> > -- 
> > /Awesome Walrus <walrus(at)amur(dot)ru>
> 
> [ Attachment, skipping... ]
> 
> [ Attachment, skipping... ]
> 
> > 
> > ---------------------------(end of broadcast)---------------------------
> > TIP 4: Have you searched our list archives?
> > 
> >                http://archives.postgresql.org
> 
> -- 
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 359-1001
>   +  If your life is a hard drive,     |  13 Roberts Road
>   +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

> Index: src/backend/commands/user.c
> ===================================================================
> RCS file: /cvsroot/pgsql/src/backend/commands/user.c,v
> retrieving revision 1.147
> diff -c -c -r1.147 user.c
> *** src/backend/commands/user.c	31 Dec 2004 21:59:42 -0000	1.147
> --- src/backend/commands/user.c	14 Oct 2005 15:42:17 -0000
> ***************
> *** 175,183 ****
>   
>   	/*
>   	 * Read pg_group and write the file.  Note we use SnapshotSelf to
> ! 	 * ensure we see all effects of current transaction.  (Perhaps could
> ! 	 * do a CommandCounterIncrement beforehand, instead?)
>   	 */
>   	scan = heap_beginscan(grel, SnapshotSelf, 0, NULL);
>   	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
>   	{
> --- 175,183 ----
>   
>   	/*
>   	 * Read pg_group and write the file.  Note we use SnapshotSelf to
> ! 	 * ensure we see all effects of current transaction.
>   	 */
> + 	CommandCounterIncrement();
>   	scan = heap_beginscan(grel, SnapshotSelf, 0, NULL);
>   	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
>   	{
> ***************
> *** 781,786 ****
> --- 781,787 ----
>   	 * Set flag to update flat password file at commit.
>   	 */
>   	user_file_update_needed();
> + 	group_file_update_needed();
>   }
>   
>   
> ***************
> *** 1200,1205 ****
> --- 1201,1207 ----
>   	 * Set flag to update flat password file at commit.
>   	 */
>   	user_file_update_needed();
> + 	group_file_update_needed();
>   }
>   
>   
> ***************
> *** 1286,1291 ****
> --- 1288,1294 ----
>   	heap_close(rel, NoLock);
>   
>   	user_file_update_needed();
> + 	group_file_update_needed();
>   }
>   
>   

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman(at)candle(dot)pha(dot)pa(dot)us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

In response to

pgsql-bugs by date

Next:From: Bruce MomjianDate: 2005-10-26 13:59:33
Subject: Re: [BUGS] BUG #1993: Adding/subtracting negative time intervals
Previous:From: Bruce MomjianDate: 2005-10-26 13:03:01
Subject: Re: BUG #1993: Adding/subtracting negative time intervals

pgsql-patches by date

Next:From: Michael PaesoldDate: 2005-10-26 14:07:46
Subject: Re: expanded \df+ display broken in beta4
Previous:From: Bruce MomjianDate: 2005-10-26 12:52:55
Subject: Re: PQescapeIdentifier

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group