Обсуждение: TODO: GRANT/REVOKE: Allow column-level privileges

Fellow Hackers,

I've been working on this item for a little while, and I'm starting to see
some code come together.  I wanted to solicit some feedback before I got
too far along to make sure I'm on the right track.

Here's a rough overview of what I've done so far:

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

PARSER:
* modified parser to accept SQL column privs syntax
* created a PrivAttr Node which holds ( priv, attr[] ) pairs.  Currently,  it's just a list of strings.  For example,
whenyou call...
 
GRANT SELECT, UPDATE (col1, col2) ON table1, table2 to grantee;
  ... the parser creates a list of Nodes:
("select", NIL), ("update", ("col1", "col2"))

SYSTEM CATALOG:
* add "attacl aclinfo[]" column to pg_attribute table and Form_pg_attribute.* add OID column to pg_attribute.  This
permitsdependencies to be  registered correctly in pg_shdepend.* populated attacl column in existing pg_attribute
bootstrapwith NULLs* allocated an unused oid for each of the pg_attribute rows that are  bootstrapped* created an oid
indexon pg_attribute
 
* modified ExecuteGrantStmt to handle the PrivAttr structure instead of  the list of strings* modified ExecuteGrantStmt
todo a nested loop over all  (column,relation) pairs in the GRANT and find oids for all of the  attributes.
 

PSQL COMMAND LINE:
* display column privileges with  "\d+ table"

STILL LEFT TO DO:
* implement ExecGrant_Attribute() to modify pg_attribute* verify query against column privileges in addition to table
privileges*register dependencies* pg_dump column privileges
 

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

I'd welcome any feedback on the design changes I've made, or any other
potential snags I should watch out for.

Thanks.

-- kevin brintnall =~ <kbrint@rufus.net>

kevin brintnall <kbrint@rufus.net> writes:
>  * add OID column to pg_attribute.  This permits dependencies to be
>    registered correctly in pg_shdepend.

No, no ... the precedent in pg_depend is that columns are represented as
the table's OID plus a column number.  Please don't invent some random
other notation for a column, especially not one that is so expensive to
relate to the parent table.  Add a subobject ID to pg_shdepend instead.

> STILL LEFT TO DO:

My recollection is that there's quite some deal of code that assumes
pg_attribute rows are fixed-width.  You will have some issues there.
It's possible though that none of that code needs to access privileges,
in which case you'd be OK just dropping off the ACL data from the
in-memory copies of pg_attribute rows.  Another possible solution is the
pg_attrdef model, ie, keep the ACLs somewhere else.
        regards, tom lane

On Fri, Jan 20, 2006 at 07:09:46PM -0500, Tom Lane wrote:
> kevin brintnall <kbrint@rufus.net> writes:
> >  * add OID column to pg_attribute.  This permits dependencies to be
> >    registered correctly in pg_shdepend.
> 
> No, no ... the precedent in pg_depend is that columns are represented as
> the table's OID plus a column number.  Please don't invent some random
> other notation for a column, especially not one that is so expensive to
> relate to the parent table.  Add a subobject ID to pg_shdepend instead.

I was referring to the dependency that exists between a grantee and any
pg_attribute ACL entries that mention the grantee.  When the role is
dropped, the ACL entries that mention that role have to be removed.

Specifically, I propose creating an entry such as the following in
pg_shdepend for every grantee G, for every column C in which G is
mentioned:
classid    = AttributeRelationId /* 1249 */objid      = C.oidrefclassid = AuthIdRelationId /* 1260 */refobjid   =
G.oiddeptype   = 'a'    /* SHARED_DEPENDENCY_ACL */
 

Are you suggesting that the pair (reloid,attnum) is superior for
identifying a pg_attribute entry?  Are there any other possible uses for
pg_attribute.oid?

> > STILL LEFT TO DO:
> 
> My recollection is that there's quite some deal of code that assumes
> pg_attribute rows are fixed-width.  You will have some issues there.
> It's possible though that none of that code needs to access privileges,
> in which case you'd be OK just dropping off the ACL data from the
> in-memory copies of pg_attribute rows.  Another possible solution is the
> pg_attrdef model, ie, keep the ACLs somewhere else.

I'm employing the same hack^H^H^H^Hmethod that is currently used in
pg_class.

-- kevin brintnall =~ <kbrint@rufus.net>

kevin brintnall <kbrint@rufus.net> writes:
> Are you suggesting that the pair (reloid,attnum) is superior for
> identifying a pg_attribute entry?

Yes.  It's just as unique, and it makes it easy to see the relationship
between the table and its columns.  Moreover, it's what we're already
using in pg_depend.
        regards, tom lane

I think we should pay attention to the sematic of table privs and column
privs.
Here is some examples.

1. role1 GRANT table priviledge SELECT on table S to role2.   role1 REVOKE column priviledge SELECT on column S(SNO)
fromrole2.
 
2. deal with circles in GRANT graph.

"kevin brintnall" <kbrint@rufus.net> wrote
> Fellow Hackers,
>
> I've been working on this item for a little while, and I'm starting to see
> some code come together.  I wanted to solicit some feedback before I got
> too far along to make sure I'm on the right track.
>
> Here's a rough overview of what I've done so far:
>
> -----------------------------------------------------------------
>
> PARSER:
>
>  * modified parser to accept SQL column privs syntax
>
>  * created a PrivAttr Node which holds ( priv, attr[] ) pairs.  Currently,
>    it's just a list of strings.  For example, when you call...
>
> GRANT SELECT, UPDATE (col1, col2) ON table1, table2 to grantee;
>
>    ... the parser creates a list of Nodes:
>
> ("select", NIL), ("update", ("col1", "col2"))
>
> SYSTEM CATALOG:
>
>  * add "attacl aclinfo[]" column to pg_attribute table and
Form_pg_attribute.
>  * add OID column to pg_attribute.  This permits dependencies to be
>    registered correctly in pg_shdepend.
>  * populated attacl column in existing pg_attribute bootstrap with NULLs
>  * allocated an unused oid for each of the pg_attribute rows that are
>    bootstrapped
>  * created an oid index on pg_attribute
>
>  * modified ExecuteGrantStmt to handle the PrivAttr structure instead of
>    the list of strings
>  * modified ExecuteGrantStmt to do a nested loop over all
>    (column,relation) pairs in the GRANT and find oids for all of the
>    attributes.
>
> PSQL COMMAND LINE:
>
>  * display column privileges with  "\d+ table"
>
> STILL LEFT TO DO:
>
>  * implement ExecGrant_Attribute() to modify pg_attribute
>  * verify query against column privileges in addition to table privileges
>  * register dependencies
>  * pg_dump column privileges
>
> -----------------------------------------------------------------
>
> I'd welcome any feedback on the design changes I've made, or any other
> potential snags I should watch out for.
>
> Thanks.
>
> --
>  kevin brintnall =~ <kbrint@rufus.net>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>

On Thu, Jan 26, 2006 at 10:25:40PM +0800, William ZHANG wrote:
> 
> I think we should pay attention to the sematic of table privs and column
> privs.
> Here is some examples.
> 
> 1. role1 GRANT table priviledge SELECT on table S to role2.
>     role1 REVOKE column priviledge SELECT on column S(SNO) from role2.

As I understand the SQL spec, the first (table-level) GRANT you specified
would be equivalent to repeating an appropriate column-level GRANT for
every column of S.  My thought was to check the column privs and apply
this logic:
if user matches an acl for the column    .. and priv is granted, then permit    .. else priv is not granted, rejectelse
fallthrough to table privileges
 

> 2. deal with circles in GRANT graph.

Can you give an examle for how this is any different for column-level
GRANTs?

-- kevin brintnall =~ <kbrint@rufus.net>

--- kevin brintnall <kbrint@rufus.net> escreveu:

>     if user matches an acl for the column
>         .. and priv is granted, then permit
>         .. else priv is not granted, reject
>     else fall through to table privileges
> 
Wouldn't it be more cheap to test the most-common-case table privileges
first?


Euler Taveira de Oliveira
euler[at]yahoo_com_br




    
_______________________________________________________ 
Yahoo! doce lar. Faça do Yahoo! sua homepage. 
http://br.yahoo.com/homepageset.html 

Euler Taveira de Oliveira <eulerto@yahoo.com.br> writes:
> --- kevin brintnall <kbrint@rufus.net> escreveu:
>> if user matches an acl for the column
>>    .. and priv is granted, then permit
>>    .. else priv is not granted, reject
>> else fall through to table privileges

> Wouldn't it be more cheap to test the most-common-case table privileges
> first?

Also, the "reject" bit is wrong: if you have table-level privileges
then that implies privileges on all columns.  So it should be just
an additional test made after failing to find the desired table-level
privilege, and before erroring out.
        regards, tom lane

On Sun, Jan 29, 2006 at 08:16:40PM -0500, Tom Lane wrote:
> Euler Taveira de Oliveira <eulerto@yahoo.com.br> writes:
> > --- kevin brintnall <kbrint@rufus.net> escreveu:
> >> if user matches an acl for the column
> >>    .. and priv is granted, then permit
> >>    .. else priv is not granted, reject
> >> else fall through to table privileges
> 
> > Wouldn't it be more cheap to test the most-common-case table privileges
> > first?
> 
> Also, the "reject" bit is wrong: if you have table-level privileges
> then that implies privileges on all columns.  So it should be just
> an additional test made after failing to find the desired table-level
> privilege, and before erroring out.

I think that would put is in violation of the spec?  This is what I got
from SQL99 (12.2 <grant privilege statement>, General Rules):

3) For every privilege descriptor in CPD whose action is INSERT, UPDATE,  or REFERENCES without a column name,
privilegedescriptors are also  created and added to CPD for each column C in O for which A holds the  corresponding
privilegewith grant option. For each such column, a  privilege descriptor is created that specifies the identical
<grantee>, the identical <action>, object C, and grantor A. 
 

4) For every privilege descriptor in CPD whose action is SELECT without a  column name or method name, privilege
descriptorsare also created and  added to CPD for each column C in O for which A holds the corresponding  privilege
withgrant option. For each such column, a privilege  descriptor is created that specifies the identical <grantee>, the
identical<action>, object C, and grantor A. 
 

As I read it, granting a table-level privilege is equivalent to repeating
the appropriate column-level privilege for all columns.  In other words:

For this table:
CREATE TABLE tab (c1 int, c2 int, c3 int);

This statement:GRANT SELECT ON tab TO grantee;

...also implies:
GRANT SELECT (c1) ON tab TO grantee;GRANT SELECT (c2) ON tab TO grantee;GRANT SELECT (c3) ON tab TO grantee;

This means that after the following, the grantee should have no privileges
on tab.c1 (but should retain them on tab.c2, tab.c3):
GRANT SELECT ON tab TO grantee;REVOKE SELECT (c1) ON tab FROM grantee;

If we want to consult the relation ACL first, then we have to convert any
relation-level GRANTs to column-level GRANTs once any of the column
privileges are REVOKEd.  However, this prevents us from seeing that the
grantee ever had table privileges, and we'll be in violation of the spec
when we go to add new columns:

(SQL99, 10.5 <privileges>, General Rules, 15-18)

15) SELECT with neither <privilege column list> nor <privilege method   list> specifies the SELECT privilege on all
columnsof T including any                                                            ^^^^^^^^^^^^^   columns
subsequentlyadded to T and implies a table privilege descriptor   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^   and one or more
columnprivilege descriptors. If T is a table of a   structured type TY, then SELECT also specifies the SELECT privilege
onall   methods of the type TY, including any methods subsequently added to the   type TY, and implies one or more
table/methodprivilege descriptors. 
 

Aside from checking the column acl first, I'm not sure how we can conform
to the spec.  Does anyone have a better way to handle this internally,
while still producing correct results?
GRANT SELECT ON tab TO grantee;REVOKE SELECT (c1) ON tab FROM grantee;

It's possible I'm just mis-understanding SQL99 ... ?

-- kevin brintnall =~ <kbrint@rufus.net>

>> 2. deal with circles in GRANT graph.
>
> Can you give an examle for how this is any different for column-level
> GRANTs?

When judging if there are any circles in the grant graph, we can represent
table priviledges as column priviledges, thus make things easier. I have not
think hard enought to figure out a better algorithm.

Another problem is, should we allow any circles to be formed when executing
GRANTs?  Say:   <grantor1, grantee1, object1, priviledge1, with grant option>,   <grantee1, grantee2, object1,
priviledge1,with grant option>,   <grantee2, grantor1, object1, priviledge1, with grant option>,
 
should the third GRANT be executed successfuly?
I remember that MSSQL 2000 and ORACLE 9i are different.

William ZHANG

"kevin brintnall" <kbrint@rufus.net>
...
> (SQL99, 10.5 <privileges>, General Rules, 15-18)
>
> 15) SELECT with neither <privilege column list> nor <privilege method
>    list> specifies the SELECT privilege on all columns of T including any
>                                                             ^^^^^^^^^^^^^
>    columns subsequently added to T and implies a table privilege 
> descriptor
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

That is exactly what I wanted to point out.
We should record in some place that the grantee will have SELECT priviledge
on any newly created columns. e.g.   GRANT SELECT ON tab TO grantee;
can be represented as   <tab, grantee, table priviledge SELECT>
after REVOKE SELECT (c1) ON tab FROM grantee;   <tab, grantee, pseduo table priviledge SELECT>   <tab/c2, grantee,
columnpriviledge SELECT>   <tab/c3, grantee, column priviledge SELECT>
 
when ALTER TABLE tab ADD COLUMN c4 CHAR(8);
we can use   <tab, grantee, pseduo table priviledge SELECT>
to deduce:   <tab/c4, grantee, column priviledge SELECT>

>    and one or more column privilege descriptors. If T is a table of a
>    structured type TY, then SELECT also specifies the SELECT privilege on 
> all
>    methods of the type TY, including any methods subsequently added to the
>    type TY, and implies one or more table/method privilege descriptors.
>
> Aside from checking the column acl first, I'm not sure how we can conform
> to the spec.  Does anyone have a better way to handle this internally,
> while still producing correct results?
>
> GRANT SELECT ON tab TO grantee;
> REVOKE SELECT (c1) ON tab FROM grantee;
>
> It's possible I'm just mis-understanding SQL99 ... ?

"William ZHANG" <uniware@zedware.org> writes:
> Another problem is, should we allow any circles to be formed when executing
> GRANTs?

This is already prohibited.
        regards, tom lane