Re: [HACKERS] GRANT/REVOKE: Allow column-level privileges

2006-01-29 Thread kevin brintnall
On Sun, Jan 29, 2006 at 08:16:40PM -0500, Tom Lane wrote:
> Euler Taveira de Oliveira <[EMAIL PROTECTED]> writes:
> > --- kevin brintnall <[EMAIL PROTECTED]> 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 , General Rules):

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

4) For every privilege descriptor in CPD whose action is SELECT without a
   column name or method name, privilege descriptors are also created and
   added to CPD for each column C in O for which A holds the corresponding
   privilege with grant option. For each such column, a privilege
   descriptor is created that specifies the identical , the
   identical , 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 , General Rules, 15-18)

15) SELECT with neither  nor  specifies the SELECT privilege on all columns of T including any
 ^
columns subsequently added to T and implies a table privilege descriptor
^^^
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 ... ?

-- 
 kevin brintnall =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [HACKERS] GRANT/REVOKE: Allow column-level privileges

2006-01-29 Thread kevin brintnall
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, reject
else fall through 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 =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [HACKERS] TODO: GRANT/REVOKE: Allow column-level privileges

2006-01-20 Thread kevin brintnall
On Fri, Jan 20, 2006 at 07:09:46PM -0500, Tom Lane wrote:
> kevin brintnall <[EMAIL PROTECTED]> 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.oid
refclassid = AuthIdRelationId /* 1260 */
refobjid   = G.oid
deptype= '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 =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


[HACKERS] TODO: GRANT/REVOKE: Allow column-level privileges

2006-01-20 Thread kevin brintnall
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 =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


[HACKERS] restrict column-level GRANTs to a single relation?

2006-01-19 Thread kevin brintnall
Fellow hackers,

I'm curious about the best way to handle something like this:

GRANT SELECT (col1, col2, col3) ON table1, table2 TO grantee;

Is it reasonable to restrict this to a single relation, and throw an error
if multiple relations are specified?  That would require the preceding
grant to be specified as:

GRANT SELECT (col1, col2, col3) ON table1 TO grantee;
GRANT SELECT (col1, col2, col3) ON table2 TO grantee;

The SQL standards don't seem to mandate the first form (unless I
misread?)..  Do y'all think this is a reasonable compromise?

-- 
 kevin brintnall =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match


Re: [HACKERS] GRANT/REVOKE column-level privileges

2006-01-13 Thread kevin brintnall
On Fri, Jan 13, 2006 at 10:04:10AM -0500, Tom Lane wrote:
> Martijn van Oosterhout  writes:
> > Umm, yes. You also need to add the column to the contents of
> > pg_attribute, give the attribute a number, increase the number of
> > attributes as stored in pg_class, update the #define that gives the
> > attribute count, change the macro that gives the size of the
> > pg_attribute structure (ATTRIBUTE_TUPLE_SIZE) and update all the places
> > that create the structure to store a null or something else in that
> > column.

I did all that, with the exception of the relnatts entry in pg_class.  I
omitted my full diff for brevity.

> > At that, I think I missed some steps but this should get you a bit
> > further...
> 
> It'd be worthwhile to look into the CVS history to study past commits
> that have added columns to pg_attribute.  Adding columns to any of
> the core system catalogs is generally a PITA ... not impossible,
> but there are plenty of details to take care of.

Thank you.  That is a good suggestion.

-- 
 kevin brintnall =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


[HACKERS] GRANT/REVOKE column-level privileges

2006-01-13 Thread kevin brintnall
Has anyone else taken a look at this?  I thought I'd play around with the
system catalog and see if I couldn't put an ACL column into pg_attribute:

It ended up generating the following BKI line:

insert ( 1249 attacl 1034 -1 -1 18 1 -1 -1 f x i f f f t 0 _null_ )

And the ROW certainly appears to be in pg_attribute:

template1=# select * from pg_attribute where attrelid=1249 and 
attnum=18;
-[ RECORD 1 ]-+---
attrelid  | 1249
attname   | attacl
atttypid  | 1034
attstattarget | -1
attlen| -1
attnum| 18
attndims  | 1
attcacheoff   | -1
atttypmod | -1
attbyval  | f
attstorage| x
attalign  | i
attnotnull| f
atthasdef | f
attisdropped  | f
attislocal| t
attinhcount   | 0

 no attacl column though!

However, the COLUMN doesn't appear to the parser:

[EMAIL PROTECTED]/test=# select attacl from pg_attribute;
ERROR:  column "attacl" does not exist

-

For better or worse, I tried the idea from pg_class where the attacl[]
comes at the end of the CATALOG(pg_attribute):

*** include/catalog/pg_attribute.h  15 Oct 2005 02:49:42 -  1.119
--- include/catalog/pg_attribute.h  13 Jan 2006 09:29:06 -
***
*** 37,44 
--- 37,50 
   *
   *If you change the following, make sure you change the structs 
for
   *system attributes in catalog/heap.c also.
   * 
+  *This structure is actually variable-length (the last attribute 
is
+  *a POSTGRES array).  Hence, sizeof(FormData_pg_attribute) 
does not
+  *necessarily match the actual length of the structure.  
Furthermore
+  *attacl may be a NULL field.  Hence, you MUST use heap_getattr()
+  *to get the attacl field ... and don't forget to check isNull.
+  * 
   */
  #define AttributeRelationId  1249
  
  CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS
***
*** 148,161 
--- 154,174 
boolattislocal;
  
/* Number of times inherited from direct parent relation(s) */
int4attinhcount;
+ 
+   /*
+* attacl may or may not be present, see note above!
+*/
+   aclitem attacl[1];  /* we declare this just for the 
catalog */
+ 
  } FormData_pg_attribute;
  
  /*
   * someone should figure out how to do this properly. (The problem is
   * the size of the C struct is not the same as the size of the tuple
   * because of alignment padding at the end of the struct.)
+  * This includes only the fixed part of the tuple (not the attacl).
   */
  #define ATTRIBUTE_TUPLE_SIZE \
(offsetof(FormData_pg_attribute,attinhcount) + sizeof(int4))


-

What is causing the parser not to be able to see that attacl is a valid
column?  Have I missed something in the relcache?  Or is the pg_class hack
(with its relacl[] on the end of the struct) truly not going to work with
pg_attribute?

Ideas?

-- 
 kevin brintnall =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


[HACKERS] TODO: Allow INET + INT4 to increment the host part of the address

2006-01-08 Thread kevin brintnall
Hi..

I want to work on the following TODO item:

Allow INET + INT4 to increment the host part of the address, or
throw an error on overflow.

I plan to add a '+' operator to the system catalog.  Is there a way to
reserve/request an OID for this purpose?  I was thinking about using 1265,
but I don't want to conflict with any existing work.  Is there a process
for getting an OID (or is this it? :)

-- 
 kevin brintnall =~ <[EMAIL PROTECTED]>

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org