Thread: Re: [HACKERS] Postgres acl (fwd)

Re: [HACKERS] Postgres acl (fwd)

From
Bruce Momjian
Date:
Forwarded message:
> > I believe I found a bug. If a user other than the postgres superuser is
> > given permission to create databases, then he should be able to destroy
> > the databases he creates. Currently he can't, at least in version 6.2.1
> > complied for SunOS 5.5. Only the poostgres superuser can delete
> > databases. If otherusers try they get the following error message:
> >
> > "WARN:pg_database: Permission denied.
> > destroydb: database destroy failed on tmpdb."
> >
> > eventhough this user is the database admin for tmpdb as shown in the
> > pd_database table.
> >
> >
>
> Here is the fix.  This bug has been around for a while:
>
> ---------------------------------------------------------------------------
>
> *** ./aclchk.c.orig    Tue Jan  6 00:10:25 1998
> --- ./aclchk.c    Tue Jan  6 00:18:40 1998
> ***************
> *** 410,416 ****
>            * pg_database table, there is still additional permissions
>            * checking in dbcommands.c
>            */
> !         if (mode & ACL_AP)
>               return ACLCHECK_OK;
>       }
>
> --- 410,416 ----
>            * pg_database table, there is still additional permissions
>            * checking in dbcommands.c
>            */
> !         if ((mode & ACL_WR) || (mode & ACL_AP))
>               return ACLCHECK_OK;
>       }

I am now thinking about this patch, and I don't think I like it.  The
original code allowed APPEND-only for users who can create databases,
but no DELETE.  The patch gives them DELETE permission, so they can
destroy their database, but they could issue the command:

    select from pg_database

and destroy everyone's.  'drop database' does checkes, but the acl check
is done in the executor, and it doesn't know if the the checks have been
performed or not.

Can someone who has permission to create databases be trusted not to
delete others?  If we say no, how do we make sure they can change
pg_database rows on only databases that they own?

--
Bruce Momjian
maillist@candle.pha.pa.us

Re: [HACKERS] Postgres acl (fwd)

From
The Hermit Hacker
Date:
On Tue, 6 Jan 1998, Bruce Momjian wrote:

> Can someone who has permission to create databases be trusted not to
> delete others?  If we say no, how do we make sure they can change
> pg_database rows on only databases that they own?

    deleting a database is accomplished using 'drop database', no?
Can the code for that not be modified to see whether the person dropping
the database is the person that owns it *or* pgsuperuser?



Re: [HACKERS] Postgres acl (fwd)

From
Bruce Momjian
Date:
>
> On Tue, 6 Jan 1998, Bruce Momjian wrote:
>
> > Can someone who has permission to create databases be trusted not to
> > delete others?  If we say no, how do we make sure they can change
> > pg_database rows on only databases that they own?
>
>     deleting a database is accomplished using 'drop database', no?
> Can the code for that not be modified to see whether the person dropping
> the database is the person that owns it *or* pgsuperuser?

It already does the check, but issues an SQL from the C code to delete
from pg_database.  I believe any user who can create a database can
issue the same SQL command from psql, bypassing the drop database
checks, no?

--
Bruce Momjian
maillist@candle.pha.pa.us

Re: [HACKERS] Postgres acl (fwd)

From
The Hermit Hacker
Date:
On Tue, 6 Jan 1998, Bruce Momjian wrote:

> >
> > On Tue, 6 Jan 1998, Bruce Momjian wrote:
> >
> > > Can someone who has permission to create databases be trusted not to
> > > delete others?  If we say no, how do we make sure they can change
> > > pg_database rows on only databases that they own?
> >
> >     deleting a database is accomplished using 'drop database', no?
> > Can the code for that not be modified to see whether the person dropping
> > the database is the person that owns it *or* pgsuperuser?
>
> It already does the check, but issues an SQL from the C code to delete
> from pg_database.  I believe any user who can create a database can
> issue the same SQL command from psql, bypassing the drop database
> checks, no?

    Okay, I understand what you mean here...so I guess the next
question is should system tables be directly modifyable by non-superuser?

    For instance, we have a 'drop database' SQL command...can we
restrict 'delete from pg_database' to just superuser, while leaving 'drop
database' open to those with createdb privileges?  Same with 'create
user', and, possible, a 'create group' command instead of 'insert into
pg_group'?



Re: [HACKERS] Postgres acl (fwd)

From
Kevin Witten
Date:
Bruce Momjian wrote:
>
> Forwarded message:
> > > I believe I found a bug. If a user other than the postgres superuser is
> > > given permission to create databases, then he should be able to destroy
> > > the databases he creates. Currently he can't, at least in version 6.2.1
> > > complied for SunOS 5.5. Only the poostgres superuser can delete
> > > databases. If otherusers try they get the following error message:
> > >
> > > "WARN:pg_database: Permission denied.
> > > destroydb: database destroy failed on tmpdb."
> > >
> > > eventhough this user is the database admin for tmpdb as shown in the
> > > pd_database table.
> > >
> > >
> >
> > Here is the fix.  This bug has been around for a while:
> >
> > ---------------------------------------------------------------------------
> >
> > *** ./aclchk.c.orig   Tue Jan  6 00:10:25 1998
> > --- ./aclchk.c        Tue Jan  6 00:18:40 1998
> > ***************
> > *** 410,416 ****
> >                * pg_database table, there is still additional permissions
> >                * checking in dbcommands.c
> >                */
> > !             if (mode & ACL_AP)
> >                       return ACLCHECK_OK;
> >       }
> >
> > --- 410,416 ----
> >                * pg_database table, there is still additional permissions
> >                * checking in dbcommands.c
> >                */
> > !             if ((mode & ACL_WR) || (mode & ACL_AP))
> >                       return ACLCHECK_OK;
> >       }
>
> I am now thinking about this patch, and I don't think I like it.  The
> original code allowed APPEND-only for users who can create databases,
> but no DELETE.  The patch gives them DELETE permission, so they can
> destroy their database, but they could issue the command:
>
>         select from pg_database
>
> and destroy everyone's.  'drop database' does checkes, but the acl check
> is done in the executor, and it doesn't know if the the checks have been
> performed or not.
>
> Can someone who has permission to create databases be trusted not to
> delete others?  If we say no, how do we make sure they can change
> pg_database rows on only databases that they own?
>
> --
> Bruce Momjian
> maillist@candle.pha.pa.us


Can't you check to see if they own the database before you let them
delete the row in pg_database. If a row is deleted from pg_database, it
is disallowed unless the userid is the same as the datdba field in that
row?

Re: [HACKERS] Postgres acl (fwd)

From
Bruce Momjian
Date:
>
> On Tue, 6 Jan 1998, Bruce Momjian wrote:
>
> > >
> > > On Tue, 6 Jan 1998, Bruce Momjian wrote:
> > >
> > > > Can someone who has permission to create databases be trusted not to
> > > > delete others?  If we say no, how do we make sure they can change
> > > > pg_database rows on only databases that they own?
> > >
> > >     deleting a database is accomplished using 'drop database', no?
> > > Can the code for that not be modified to see whether the person dropping
> > > the database is the person that owns it *or* pgsuperuser?
> >
> > It already does the check, but issues an SQL from the C code to delete
> > from pg_database.  I believe any user who can create a database can
> > issue the same SQL command from psql, bypassing the drop database
> > checks, no?
>
>     Okay, I understand what you mean here...so I guess the next
> question is should system tables be directly modifyable by non-superuser?
>
>     For instance, we have a 'drop database' SQL command...can we
> restrict 'delete from pg_database' to just superuser, while leaving 'drop
> database' open to those with createdb privileges?  Same with 'create
> user', and, possible, a 'create group' command instead of 'insert into
> pg_group'?

Yes, we must replace the SQL commands in commands/dbcommands.c with
lower-level C table access routines so we do not have to go to the
executor, where the access permissions are checked.

--
Bruce Momjian
maillist@candle.pha.pa.us