Обсуждение: Re: Please advice TODO Item pg_hba.conf

Поиск
Список
Период
Сортировка

Re: Please advice TODO Item pg_hba.conf

От
Alvaro Herrera
Дата:
Gevik Babakhani wrote:

Hi,

> 2) The file parsenodes.h is updated to support
> #define ACL_DATABASE_CONNECT

I wouldn't call it ACL_DATABASE_CONNECT, just ACL_CONNECT.  Currently we
don't have any objects other than databases that need connecting to, but
you never know.

(It should be very easy to change this if you edit the patch itself,
un-apply the old one and then re-apply the edited patch.)

> 7) File postinit.c method "ReverifyMyDatabase" is updated by following:
> First we check to make sure we are not in bootstrap processing mode.
> If not, we check to see if the connected user has ACL_DATABASE_CONNECT.
> If not, ereport(FATAL,.....)
> (Perhaps we should change the error message later)

Please make sure you use the established style for writing the ereport()
call.  The newlines you placed are not consistent with the rest of the
code.  Also the message text does not follow the guidelines (no initial
capital letter, no ending period for the primary message; the other way
around for secondary messages).  I think it should be

ereport(FATAL,    (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),         errmsg("couldn't connect to database %s",
database_name),    errdetail("User %s doesn't have the CONNECT privilege for database %s.",
user_name,database_name)));
 

Or something like that.  Not sure about the wording.  Also: are we
giving too much information so that it can be considered a security
problem?  (I don't think so, because user and database name were passed
by the user).

> 8) File dbcommands.c method "createdb" is updated by following:
> When a new database is being created we add a default ACL by 
> calling acldefault(ACL_OBJECT_DATABASE,.... and adding the default ACL
> by new_record[Anum_pg_database_datacl - 1] =
> PointerGetDatum(defaultAcl);

If I'm not mistaken, the general principle for creating objects is leave
their ACLs as NULLs.  Later, when a privilege is going to be checked, a
NULL is treated as if it contained whatever default privilege the object
class has.  So you should leave this code alone, and have the checking
code replace the default ACL when it gets a NULL (this way, it's even
more backwards compatible).

> At this moment the owner of the database CAN REVOKE himself form the
> ACL_OBJECT_DATABASE. If the implementation above is acceptable then I
> can work on this one :)

Hmm, what do you want to do about it?  ISTM the owner should be able to
revoke the privilege from himself ... (Maybe we could raise a WARNING
whenever anyone revokes the last CONNECT privilege to a database, so
that he can GRANT it to somebody before disconnecting.)


Also I'm not sure if we have discussed what's the default (initial)
privilege state.  Do we want PUBLIC to have CONNECT privilege?

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


Re: Please advice TODO Item pg_hba.conf

От
Gevik Babakhani
Дата:
Hi,

On Sun, 2006-04-23 at 17:06 -0400, Alvaro Herrera wrote:
> Gevik Babakhani wrote:
> 
> Hi,
> 
> > 2) The file parsenodes.h is updated to support
> > #define ACL_DATABASE_CONNECT
> 
> I wouldn't call it ACL_DATABASE_CONNECT, just ACL_CONNECT.  Currently we
> don't have any objects other than databases that need connecting to, but
> you never know.
> 

OK :)

> Please make sure you use the established style for writing the ereport()
> call.  The newlines you placed are not consistent with the rest of the
> code.  Also the message text does not follow the guidelines (no initial
> capital letter, no ending period for the primary message; the other way
> around for secondary messages).  I think it should be
> 
> ereport(FATAL,
>         (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>           errmsg("couldn't connect to database %s", database_name),
>          errdetail("User %s doesn't have the CONNECT privilege for database %s.",
>                      user_name, database_name)));
> 

OK. I will put the above as the error message. We can always change this
later if we wanted to.


> > 8) File dbcommands.c method "createdb" is updated by following:
> > When a new database is being created we add a default ACL by 
> > calling acldefault(ACL_OBJECT_DATABASE,.... and adding the default ACL
> > by new_record[Anum_pg_database_datacl - 1] =
> > PointerGetDatum(defaultAcl);
> 
> If I'm not mistaken, the general principle for creating objects is leave
> their ACLs as NULLs.  Later, when a privilege is going to be checked, a
> NULL is treated as if it contained whatever default privilege the object
> class has.  So you should leave this code alone, and have the checking
> code replace the default ACL when it gets a NULL (this way, it's even
> more backwards compatible).

Personally I think this would create an conflict only in case of the
CONNECT privilege. If the ACL is NULL and we treat NULL as default and
the CONNECT privilege is part of default privileges then how do we
distinguish between someone NOT HAVING THE CONNECT PRIVILEGE to connect
to a certain database. This was the reason I thought when someone
connects to a database NULL ACL will not do because you cannot know the
user connecting does or does not have the CONNECT privilege. The problem
is I think when you revoke a privilege from ACL, the current code
regarding this actually removes/deletes the privilege from the ACL bits.
> 
> > At this moment the owner of the database CAN REVOKE himself form the
> > ACL_OBJECT_DATABASE. If the implementation above is acceptable then I
> > can work on this one :)
> 
> Hmm, what do you want to do about it?  ISTM the owner should be able to
> revoke the privilege from himself ... (Maybe we could raise a WARNING
> whenever anyone revokes the last CONNECT privilege to a database, so
> that he can GRANT it to somebody before disconnecting.)

Personally I think it would be better for the database owner not have
the option to REVOKE himself from the CONNECTION privilege of his own
database. 



> Also I'm not sure if we have discussed what's the default (initial)
> privilege state.  Do we want PUBLIC to have CONNECT privilege?

If I where a DBA I would rather explicitly give people connect
permission to my database rather than having to explicitly block them
from connecting to the database. It is like all the doors are closed to
the public unless the doorman opens the door for the person he knows.
But then again this is my personal opinion. We can always discuss this
of course.




Re: Please advice TODO Item pg_hba.conf

От
Martijn van Oosterhout
Дата:
On Sun, Apr 23, 2006 at 11:40:08PM +0200, Gevik Babakhani wrote:
> > Also I'm not sure if we have discussed what's the default (initial)
> > privilege state.  Do we want PUBLIC to have CONNECT privilege?
>
> If I where a DBA I would rather explicitly give people connect
> permission to my database rather than having to explicitly block them
> from connecting to the database. It is like all the doors are closed to
> the public unless the doorman opens the door for the person he knows.
> But then again this is my personal opinion. We can always discuss this
> of course.

Backward compatability. The code has to end up working exactly as the
system behaves now (i.e. pg_hba.conf controls everything). Maybe a
release or two down the track we can change the default, but right now
you can't.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> From each according to his ability. To each according to his ability to litigate.

Re: Please advice TODO Item pg_hba.conf

От
Alvaro Herrera
Дата:
Gevik Babakhani wrote:

> > If I'm not mistaken, the general principle for creating objects is leave
> > their ACLs as NULLs.  Later, when a privilege is going to be checked, a
> > NULL is treated as if it contained whatever default privilege the object
> > class has.  So you should leave this code alone, and have the checking
> > code replace the default ACL when it gets a NULL (this way, it's even
> > more backwards compatible).
> 
> Personally I think this would create an conflict only in case of the
> CONNECT privilege. If the ACL is NULL and we treat NULL as default and
> the CONNECT privilege is part of default privileges then how do we
> distinguish between someone NOT HAVING THE CONNECT PRIVILEGE to connect
> to a certain database. This was the reason I thought when someone
> connects to a database NULL ACL will not do because you cannot know the
> user connecting does or does not have the CONNECT privilege. The problem
> is I think when you revoke a privilege from ACL, the current code
> regarding this actually removes/deletes the privilege from the ACL bits.

I don't understand.  The code should look like this:

if (acl in pg_database == NULL)acl = acldefault
elseacl = acl in pg_database
if (has_permission(acl, user, ACL_CONNECT))can connect
elsecan't connect


To revoke a privilege you do this:

if (acl in pg_datbase == NULL)acl = acldefault
elseacl = acl in pg_database
newacl = revoke_privilege_from(acl)
store newacl in pg_database



You can very easily know if someone does have the connect privilege,
even if the ACL is null, because the NULL acl has a very well-defined
meaning (only OWNER can connect, or maybe OWNER and PUBLIC can connect,
or whatever).


> Personally I think it would be better for the database owner not have
> the option to REVOKE himself from the CONNECTION privilege of his own
> database. 

Why?  A table owner can revoke privileges from himself.

alvherre=# create user foo;
CREATE ROLE
alvherre=# grant create on schema public to foo;
GRANT
alvherre=# set session AUTHORIZATION foo;
SET
alvherre=> create table foo (a int);
CREATE TABLE
alvherre=> revoke all on foo from foo;
REVOKE
alvherre=> \z foo
Privilegios para base de datos «alvherre»Schema | Nombre | Tipo  | Privilegios 
--------+--------+-------+-------------public | foo    | tabla | {}
(1 fila)

alvherre=> select * from foo;
ERROR:  permiso denegado para la relación foo
(english: permission denied ...)


> > Also I'm not sure if we have discussed what's the default (initial)
> > privilege state.  Do we want PUBLIC to have CONNECT privilege?
> 
> If I where a DBA I would rather explicitly give people connect
> permission to my database rather than having to explicitly block them
> from connecting to the database. It is like all the doors are closed to
> the public unless the doorman opens the door for the person he knows.
> But then again this is my personal opinion. We can always discuss this
> of course.

I understand your point, but we give a lot of privileges by default (I
think we give CREATE on the PUBLIC schema, for example).  You can
propose to change that behavior, but I feel that's a different
discussion than what you are working on ATM.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


Re: Please advice TODO Item pg_hba.conf

От
Gevik Babakhani
Дата:
Hi,

> if (acl in pg_database == NULL)
>     acl = acldefault
> else
>     acl = acl in pg_database
> if (has_permission(acl, user, ACL_CONNECT))
>     can connect
> else
>     can't connect
> 
> 
> To revoke a privilege you do this:
> 
> if (acl in pg_datbase == NULL)
>     acl = acldefault
> else
>     acl = acl in pg_database
> newacl = revoke_privilege_from(acl)
> store newacl in pg_database

Perfect, I see it now :) My error was to actually add the "acldefault"
when the acl was null. 

> 
> > Personally I think it would be better for the database owner not have
> > the option to REVOKE himself from the CONNECTION privilege of his own
> > database. 
> 
> Why?  A table owner can revoke privileges from himself.

Of course a TABLE owner can revoke privileges from himself. But why
would a DATABASE owner want to lock himself out from CONNECTING to his
database. Perhaps there is a legitimate reason for this but it doesn't
make sense. Right? I see it this way: Why should I lockout myself from
my own house and throw the keys away. (I am a man of simple words and
examples, I must apologize.)

> I understand your point, but we give a lot of privileges by default (I
> think we give CREATE on the PUBLIC schema, for example).  You can
> propose to change that behavior, but I feel that's a different
> discussion than what you are working on ATM.
> 

Agreed.



Re: Please advice TODO Item pg_hba.conf

От
Alvaro Herrera
Дата:
Gevik Babakhani wrote:

> > > Personally I think it would be better for the database owner not have
> > > the option to REVOKE himself from the CONNECTION privilege of his own
> > > database. 
> > 
> > Why?  A table owner can revoke privileges from himself.
> 
> Of course a TABLE owner can revoke privileges from himself. But why
> would a DATABASE owner want to lock himself out from CONNECTING to his
> database.

I don't know :-)  If it doesn't make sense for somebody, then she won't
do it.

It's not like we are going out of our way to allow somebody to revoke
the privileges from oneself.  We are just keeping the thing as simple as
possible.  As I said, maybe a reasonable option would be to raise a
WARNING when somebody revoked the last CONNECT privilege.  So you grant
the privilege to somebody else and the revoke yours.

> Perhaps there is a legitimate reason for this but it doesn't
> make sense. Right? I see it this way: Why should I lockout myself from
> my own house and throw the keys away. (I am a man of simple words and
> examples, I must apologize.)

Maybe you've given a copy of the keys to somebody else.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: Please advice TODO Item pg_hba.conf

От
Gevik Babakhani
Дата:
> I don't know :-)  If it doesn't make sense for somebody, then she won't
> do it.
> 
> It's not like we are going out of our way to allow somebody to revoke
> the privileges from oneself.  We are just keeping the thing as simple as
> possible.  As I said, maybe a reasonable option would be to raise a
> WARNING when somebody revoked the last CONNECT privilege.  So you grant
> the privilege to somebody else and the revoke yours.
> 

Thinking about the above... is actually a brilliant option.  (always
listing to your coach they say..) 



Re: Please advice TODO Item pg_hba.conf

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Gevik Babakhani wrote:
>> At this moment the owner of the database CAN REVOKE himself form the
>> ACL_OBJECT_DATABASE. If the implementation above is acceptable then I
>> can work on this one :)

> Hmm, what do you want to do about it?  ISTM the owner should be able to
> revoke the privilege from himself ...

Agreed.  You can revoke all your own ordinary privileges on a table,
for example, and it would be just weird to not be able to do so for
a database.

> (Maybe we could raise a WARNING
> whenever anyone revokes the last CONNECT privilege to a database, so
> that he can GRANT it to somebody before disconnecting.)

Since (a) you could connect to a different DB, eg template1, and restore
the privilege; and (b) superusers will get past this check anyhow,
I see no need for special pushups to prevent revoking CONNECT.

> Also I'm not sure if we have discussed what's the default (initial)
> privilege state.  Do we want PUBLIC to have CONNECT privilege?

It must, else the behavior is not backwards compatible.
        regards, tom lane


Re: Please advice TODO Item pg_hba.conf

От
Tom Lane
Дата:
Gevik Babakhani <pgdev@xs4all.nl> writes:
> On Sun, 2006-04-23 at 17:06 -0400, Alvaro Herrera wrote:
>> If I'm not mistaken, the general principle for creating objects is leave
>> their ACLs as NULLs.

> Personally I think this would create an conflict only in case of the
> CONNECT privilege. If the ACL is NULL and we treat NULL as default and
> the CONNECT privilege is part of default privileges then how do we
> distinguish between someone NOT HAVING THE CONNECT PRIVILEGE to connect
> to a certain database.

You're not following Alvaro's point.  The code's behavior is that a NULL
ACL is interpreted as being the default ACL for the object type.
Whether people would be allowed to connect would depend on what we set
as the default privilege state for the CONNECT privilege.  But since
we are going to grant it to PUBLIC by default (no, that's not open to
debate), people will succeed in connecting to a database with NULL ACL.
That's just like they can succeed in creating schemas in a database with
NULL ACL today.

Revoking privileges from an object with NULL ACL doesn't leave it NULL
(try it and see, preferably on an object where the default privileges
include some for PUBLIC --- databases or functions will do).
        regards, tom lane


Re: Please advice TODO Item pg_hba.conf

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Gevik Babakhani wrote:
>> Of course a TABLE owner can revoke privileges from himself. But why
>> would a DATABASE owner want to lock himself out from CONNECTING to his
>> database.

> I don't know :-)  If it doesn't make sense for somebody, then she won't
> do it.

> It's not like we are going out of our way to allow somebody to revoke
> the privileges from oneself.  We are just keeping the thing as simple as
> possible.

There is a good, defensible reason for this: the behavior of
security-related commands should be as simple and unsurprising as
possible.  Weird special cases added in the name of improving usability
are likely to do the opposite.  What would you expectREVOKE CONNECT ON DATABASE foo FROM foo_owner
to do, if not revoke his connect privileges?  Failing to do so could
be called a security vulnerability.
        regards, tom lane


Re: Please advice TODO Item pg_hba.conf

От
Gevik Babakhani
Дата:
Tom,

> the behavior of
> security-related commands should be as simple and unsurprising as
> possible.  Weird special cases added in the name of improving usability
> are likely to do the opposite. 

Golden rule.

Thank you :)




Re: TODO Item: ACL_CONNECT

От
Alvaro Herrera
Дата:
Gevik Babakhani wrote:

> To my surprise the code you described above was already there :) 
> function aclchk.c:pg_database_aclmask:1696

Sure, that sort of was my point :-)

> If the above is okay and correct. Then I guess for simple systems one
> could only enter the line below in pg_hba.conf 
> "host/hostssel    all     all    (whatever IP)   (whatever option)"

Ok, good.  This is what people was aiming for initially, I hope.  What
do people think, particularly those who wanted to manage pg_hba.conf via
SQL commands?

> New test patch:
> http://www.xs4all.nl/~gevik/patch/patch-0.2.diff

Without looking at the surrounding code, I'm a bit wary of the fact that
in ReverifyMyDatabase, pg_database_aclcheck is called with GetUserId()
but the error message is emitted with the user_name that was passed as
parameter instead.  The inconsistency could prove painful in the future;
maybe it's OK, but if it is, you should declare it in the surrounding
comments.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


TODO Item: ACL_CONNECT

От
Gevik Babakhani
Дата:
Hi

> I don't understand.  The code should look like this:
> 
> if (acl in pg_database == NULL)
>     acl = acldefault
> else
>     acl = acl in pg_database
> if (has_permission(acl, user, ACL_CONNECT))
>     can connect
> else
>     can't connect
> 

To my surprise the code you described above was already there :) 
function aclchk.c:pg_database_aclmask:1696

snip...if (isNull){    /* No ACL, so build default ACL */    acl = acldefault(ACL_OBJECT_DATABASE, ownerId);
aclDatum= (Datum) 0;}
 

However the original acldefault:case:ACL_OBJECT_DATABASE only had
ACL_CREATE_TEMP as default for PUBLIC. I thought by adding ACL_CONNECT
to the world_owner makes connecting to a database available for public,
which is the required behavior as discussed yesterday. 

Original...
case ACL_OBJECT_DATABASE:world_default = ACL_CREATE_TEMP /* NO_RIGHTS! */owner_default =
ACL_ALL_RIGHTS_DATABASE;break;

Proposed....
case ACL_OBJECT_DATABASE:world_default = ACL_CREATE_TEMP | ACL_CONNECT; /* NO_RIGHTS! */owner_default =
ACL_ALL_RIGHTS_DATABASE;break;

Would the above be correct?
The following is how I tested the code above.


1. make new new compile/install and initdb.

2. run createdb <enter> (database pgdev is created)

3. psql <enter> (login with user pgdev to pgdev)

4. create role user1 login; and then quit.

5. psql -U user1 -d pgdev (login success. this is the backwardcompatible and the required behavior I guess we wanted)

6. quit and login with psql like step in 3

7. GRANT CONNECTION ON DATABASE pgdev to pgdev; 
(this would overwrite the ACL NULL. The public ACL still exists.)
REVOKE CONNECTION ON DATABASE pgdev from PUBLIC; and the quit
(public cannot login to pgdev anymore :) only the owner )

8. psql -U user1 -d pgdev (login fails this time 

psql: FATAL:  couldn't connect to database pgdev
DETAIL:  User user1 doesn't have the CONNECTION privilege for database
pgdev.

)

9. quit and login with psql like step in 3
GRANT CONNECTION ON DATABASE pgdev to user1; and quit.

10. psql -U user1 -d pgdev (login success and the {user1=c/pgdev}
is added to the ACL)

* end test *************************

If the above is okay and correct. Then I guess for simple systems one
could only enter the line below in pg_hba.conf 
"host/hostssel    all     all    (whatever IP)   (whatever option)"

and by granting ACL_CONNECT to roles could keep 
the pg_hba.conf simple and short.

New test patch:
http://www.xs4all.nl/~gevik/patch/patch-0.2.diff




Re: TODO Item: ACL_CONNECT

От
Gevik Babakhani
Дата:
> Ok, good.  This is what people was aiming for initially, I hope.  What
> do people think, particularly those who wanted to manage pg_hba.conf via
> SQL commands?

I guess for this one more people have to play with the new
functionality. 

> Without looking at the surrounding code, I'm a bit wary of the fact that
> in ReverifyMyDatabase, pg_database_aclcheck is called with GetUserId()
> but the error message is emitted with the user_name that was passed as
> parameter instead.  The inconsistency could prove painful in the future;
> maybe it's OK, but if it is, you should declare it in the surrounding
> comments.

I have added proper comment for that.

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

I guess the next step is to check for the last ACL_CONNECT privilege as
discussed below.


> At this moment the owner of the database CAN REVOKE himself form the
> ACL_OBJECT_DATABASE. If the implementation above is acceptable then I
> can work on this one :)

Hmm, what do you want to do about it?  ISTM the owner should be able to
revoke the privilege from himself ... (Maybe we could raise a WARNING
whenever anyone revokes the last CONNECT privilege to a database, so
that he can GRANT it to somebody before disconnecting.)



Re: TODO Item: ACL_CONNECT

От
Gevik Babakhani
Дата:
Hi,

The following is also added to a new patch.

> > At this moment the owner of the database CAN REVOKE himself form the
> > ACL_OBJECT_DATABASE. If the implementation above is acceptable then I
> > can work on this one :)
> 
> Hmm, what do you want to do about it?  ISTM the owner should be able to
> revoke the privilege from himself ... (Maybe we could raise a WARNING
> whenever anyone revokes the last CONNECT privilege to a database, so
> that he can GRANT it to somebody before disconnecting.)
> 

If one is going to revoke the last ACL_CONNECT, a warning is going to
issued then that part of the REVOKE gets canceled.

Can this patch be tested for assurance?
What is the next step after that? Should I send the patch to the
pgsql-patches list?

http://www.xs4all.nl/~gevik/patch/patch-0.4.diff

Regards,
Gevik.



Re: TODO Item: ACL_CONNECT

От
Alvaro Herrera
Дата:
Gevik Babakhani wrote:

> If one is going to revoke the last ACL_CONNECT, a warning is going to
> issued then that part of the REVOKE gets canceled.

Humm, no, the WARNING is issued but the REVOKE is executed anyway.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: TODO Item: ACL_CONNECT

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Gevik Babakhani wrote:
>> If one is going to revoke the last ACL_CONNECT, a warning is going to
>> issued then that part of the REVOKE gets canceled.

> Humm, no, the WARNING is issued but the REVOKE is executed anyway.

Why are we debating this?  It won't get accepted anyway, because the
whole thing is silly.  Show me one other object type that we issue
such warnings for, or anyone else who has even suggested that we should.
        regards, tom lane


Re: TODO Item: ACL_CONNECT

От
Gevik Babakhani
Дата:
Hi 

On Mon, 2006-04-24 at 23:07 -0400, Alvaro Herrera wrote:
> Gevik Babakhani wrote:
> 
> > If one is going to revoke the last ACL_CONNECT, a warning is going to
> > issued then that part of the REVOKE gets canceled.
> 
> Humm, no, the WARNING is issued but the REVOKE is executed anyway.

I have tested this by applying the patch-0.4.diff of a new src tree.

[gevik@voyager ~]$ createdb
CREATE DATABASE
[gevik@voyager ~]$ psql
Welcome to psql 8.2devel, the PostgreSQL interactive terminal.

Type:  \copyright for distribution terms      \h for help with SQL commands      \? for help with psql commands      \g
orterminate with semicolon to execute query      \q to quit
 

gevik=# revoke connection on database gevik from gevik;
REVOKE
gevik=# revoke connection on database gevik from public;
WARNING:  The revoke statement or at least one part of it cannot be
completed on database gevik
DETAIL:  At least one database connection privilege should be granted
for this database
REVOKE
gevik=# select datname,datacl from pg_catalog.pg_database; datname  |           datacl
-----------+----------------------------postgres  |gevik     | {=Tc/gevik,gevik=CT/gevik}template1 |
{=c/gevik,gevik=CTc/gevik}template0| {=c/gevik,gevik=CTc/gevik}
 
(4 rows)

gevik=#




Re: TODO Item: ACL_CONNECT

От
Gevik Babakhani
Дата:
On Mon, 2006-04-24 at 23:16 -0400, Tom Lane wrote:
> Why are we debating this?  It won't get accepted anyway, because the
> whole thing is silly.  Show me one other object type that we issue
> such warnings for, or anyone else who has even suggested that we should.

So, I am very much confused. What do I do now. Do you mean the whole
thing won't get accepted and I should stop developing the TODO item? or
just strip the warning part.



Re: TODO Item: ACL_CONNECT

От
Alvaro Herrera
Дата:
Gevik Babakhani wrote:
> On Mon, 2006-04-24 at 23:16 -0400, Tom Lane wrote:
> > Why are we debating this?  It won't get accepted anyway, because the
> > whole thing is silly.  Show me one other object type that we issue
> > such warnings for, or anyone else who has even suggested that we should.

No other object type has the ability to require you to stop the server
and start a standalone backend to fix the mistake, which is what makes
this thing unique.


> So, I am very much confused. What do I do now. Do you mean the whole
> thing won't get accepted and I should stop developing the TODO item? or
> just strip the warning part.

Tom is referring to the WARNING.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


Re: TODO Item: ACL_CONNECT

От
Alvaro Herrera
Дата:
Gevik Babakhani wrote:
> Hi 
> 
> On Mon, 2006-04-24 at 23:07 -0400, Alvaro Herrera wrote:
> > Gevik Babakhani wrote:
> > 
> > > If one is going to revoke the last ACL_CONNECT, a warning is going to
> > > issued then that part of the REVOKE gets canceled.
> > 
> > Humm, no, the WARNING is issued but the REVOKE is executed anyway.
> 
> I have tested this by applying the patch-0.4.diff of a new src tree.

My point is that the behavior you describe is broken.  The warning
should be issued but the command should be executed anyway.  That way
the user executing it knows that he has locked everyone out of the
database (but he can lock everyone out of the database if he wants to.)

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


Re: TODO Item: ACL_CONNECT

От
Stephen Frost
Дата:
* Alvaro Herrera (alvherre@commandprompt.com) wrote:
> Gevik Babakhani wrote:
> > On Mon, 2006-04-24 at 23:16 -0400, Tom Lane wrote:
> > > Why are we debating this?  It won't get accepted anyway, because the
> > > whole thing is silly.  Show me one other object type that we issue
> > > such warnings for, or anyone else who has even suggested that we should.
>
> No other object type has the ability to require you to stop the server
> and start a standalone backend to fix the mistake, which is what makes
> this thing unique.

Eh?  Isn't that the case if you manage to remove the superuser bit from
everyone?  Yet it's allowed, I'm not even sure there's a warning..  In
any case, what we do there can serve as precedent.
Thanks,
    Stephen

Re: TODO Item: ACL_CONNECT

От
Alvaro Herrera
Дата:
Stephen Frost wrote:
> * Alvaro Herrera (alvherre@commandprompt.com) wrote:
> > Gevik Babakhani wrote:
> > > On Mon, 2006-04-24 at 23:16 -0400, Tom Lane wrote:
> > > > Why are we debating this?  It won't get accepted anyway, because the
> > > > whole thing is silly.  Show me one other object type that we issue
> > > > such warnings for, or anyone else who has even suggested that we should.
> > 
> > No other object type has the ability to require you to stop the server
> > and start a standalone backend to fix the mistake, which is what makes
> > this thing unique.
> 
> Eh?  Isn't that the case if you manage to remove the superuser bit from
> everyone?  Yet it's allowed, I'm not even sure there's a warning..  In
> any case, what we do there can serve as precedent.

Hmm, true.  Maybe we could raise a warning in that case as well :-)

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.


Re: TODO Item: ACL_CONNECT

От
Gevik Babakhani
Дата:
On Tue, 2006-04-25 at 08:53 -0400, Alvaro Herrera wrote:
> Gevik Babakhani wrote:

> My point is that the behavior you describe is broken.  The warning
> should be issued but the command should be executed anyway.  That way
> the user executing it knows that he has locked everyone out of the
> database (but he can lock everyone out of the database if he wants to.)

This is no problem :)  I will remove the cancellation part and change
the warning. 

Could someone please provide a suitable warning message :) 



Re: TODO Item: ACL_CONNECT

От
Tom Lane
Дата:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> No other object type has the ability to require you to stop the server
> and start a standalone backend to fix the mistake, which is what makes
> this thing unique.

This one doesn't either.  I already pointed out two reasons why not:
1. you can connect to a different database (eg template1 or postgres)
and fix the problem from there.
2. the restriction won't be enforced against superusers anyway.
        regards, tom lane