Re: [HACKERS] Column-Level Privileges
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > [ column privs cleanup patch ] > > Applied with revisions, as per previous messages. Great, thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
Stephen Frost writes: > [ column privs cleanup patch ] Applied with revisions, as per previous messages. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
On Tue, Feb 3, 2009 at 7:04 PM, Tom Lane wrote:
> Stephen Frost writes:
>> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>>> * Some of the information_schema views are specified to respond to
>>> per-column privileges; the column_privileges and columns views
>>> certainly need work now to meet spec, and there might be others.
>
>> Done.
>
> I looked through the spec a bit. If I'm reading it right, these
> views should show columns that you have either table-level or
> column-level privilege for:
>column_privileges
>columns
>key_column_usage
>role_column_grants
>
> What's more, these views should show you tables/views that you have
> column privilege on any column of, even if you haven't got any
> full-table privileges:
>tables
>table_constraints
>table_privileges
>views
>
> I thought about handling the tests for the latter by exposing
> pg_attribute_aclcheck_all() as a function named something like
> has_any_column_privilege(). However, that would amount to forcing a
> nestloop-with-inner-indexscan join to pg_attribute for any table you
> didn't have full-table privileges for; also it would bloat the syscache
> in a database with lots of tables. It might be better to expose that
> join explicitly and let the planner try to decide what to do. OTOH
> I don't think the planner would be very smart about avoiding the join
> if you do have full-table privileges. Either way you slice it it could
> be really slow :-(
>
> Comments, better ideas? Does anyone think I misread the spec?
Perhaps there should be a column in pg_class that indicates, in
essence, whether there is any point in checking pg_attribute. Call it
relaclpartial aclitem[]. Whenever user U has column-level privileges
P and Q on a subset of the columns in the table, put {U=PQ} in
relaclpartial.
...Robert
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> * Some of the information_schema views are specified to respond to >> per-column privileges; the column_privileges and columns views >> certainly need work now to meet spec, and there might be others. > Done. I looked through the spec a bit. If I'm reading it right, these views should show columns that you have either table-level or column-level privilege for: column_privileges columns key_column_usage role_column_grants What's more, these views should show you tables/views that you have column privilege on any column of, even if you haven't got any full-table privileges: tables table_constraints table_privileges views I thought about handling the tests for the latter by exposing pg_attribute_aclcheck_all() as a function named something like has_any_column_privilege(). However, that would amount to forcing a nestloop-with-inner-indexscan join to pg_attribute for any table you didn't have full-table privileges for; also it would bloat the syscache in a database with lots of tables. It might be better to expose that join explicitly and let the planner try to decide what to do. OTOH I don't think the planner would be very smart about avoiding the join if you do have full-table privileges. Either way you slice it it could be really slow :-( Comments, better ideas? Does anyone think I misread the spec? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
Tom, all,
In the attached patch-
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> * Some of the information_schema views are specified to respond to
> per-column privileges; the column_privileges and columns views
> certainly need work now to meet spec, and there might be others.
Done.
> * It might be appropriate to let the pg_stats view expose stats for
> columns you have select privilege for, even if you haven't got it
> across the whole table.
Done.
> * We probably ought to invent has_column_privilege SQL functions
> analogous to has_table_privilege; this is not just for completeness,
> but is probably necessary to finish the above items.
Done.
> * ISTM that COPY with a column list should succeed if you have
> SELECT or INSERT privilege on just the mentioned columns.
Done.
> * Perhaps it would be appropriate to let LOCK TABLE succeed if you have
> proper permissions on at least one column of the table. However, it's
> bad enough that LOCK TABLE examines permissions before locking the table
> now; I don't think it ought to be grovelling through the columns without
> lock. So this might be a place to leave well enough alone.
Left alone.
Thanks,
Stephen
Index: src/backend/catalog/information_schema.sql
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/information_schema.sql,v
retrieving revision 1.50
diff -c -r1.50 information_schema.sql
*** src/backend/catalog/information_schema.sql 20 Jan 2009 09:10:20 - 1.50
--- src/backend/catalog/information_schema.sql 3 Feb 2009 03:17:26 -
***
*** 507,523
UNION ALL
SELECT 0::oid, 'PUBLIC'
) AS grantee (oid, rolname),
! (SELECT 'SELECT' UNION ALL
! SELECT 'INSERT' UNION ALL
! SELECT 'UPDATE' UNION ALL
! SELECT 'REFERENCES') AS pr (type)
WHERE a.attrelid = c.oid
AND c.relnamespace = nc.oid
AND a.attnum > 0
AND NOT a.attisdropped
AND c.relkind IN ('r', 'v')
! AND aclcontains(c.relacl,
makeaclitem(grantee.oid, u_grantor.oid, pr.type, false))
AND (pg_has_role(u_grantor.oid, 'USAGE')
OR pg_has_role(grantee.oid, 'USAGE')
--- 507,523
UNION ALL
SELECT 0::oid, 'PUBLIC'
) AS grantee (oid, rolname),
! (VALUES ('SELECT'),
! ('INSERT'),
! ('UPDATE'),
! ('REFERENCES')) AS pr (type)
WHERE a.attrelid = c.oid
AND c.relnamespace = nc.oid
AND a.attnum > 0
AND NOT a.attisdropped
AND c.relkind IN ('r', 'v')
! AND aclcontains(a.attacl,
makeaclitem(grantee.oid, u_grantor.oid, pr.type, false))
AND (pg_has_role(u_grantor.oid, 'USAGE')
OR pg_has_role(grantee.oid, 'USAGE')
***
*** 677,683
OR has_table_privilege(c.oid, 'SELECT')
OR has_table_privilege(c.oid, 'INSERT')
OR has_table_privilege(c.oid, 'UPDATE')
!OR has_table_privilege(c.oid, 'REFERENCES') );
GRANT SELECT ON columns TO PUBLIC;
--- 677,687
OR has_table_privilege(c.oid, 'SELECT')
OR has_table_privilege(c.oid, 'INSERT')
OR has_table_privilege(c.oid, 'UPDATE')
!OR has_table_privilege(c.oid, 'REFERENCES')
!OR has_column_privilege(c.oid, a.attnum, 'SELECT')
!OR has_column_privilege(c.oid, a.attnum, 'INSERT')
!OR has_column_privilege(c.oid, a.attnum, 'UPDATE')
!OR has_column_privilege(c.oid, a.attnum, 'REFERENCES'));
GRANT SELECT ON columns TO PUBLIC;
Index: src/backend/catalog/system_views.sql
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/system_views.sql,v
retrieving revision 1.58
diff -c -r1.58 system_views.sql
*** src/backend/catalog/system_views.sql 1 Jan 2009 17:23:37 - 1.58
--- src/backend/catalog/system_views.sql 3 Feb 2009 03:17:26 -
***
*** 137,143
FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
! WHERE has_table_privilege(c.oid, 'select');
REVOKE ALL on pg_statistic FROM public;
--- 137,144
FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
! WHERE has_table_privilege(c.oid, 'select') OR
! has_column_privilege(c.oid, a.attnum, 'select');
REVOKE ALL on pg_statistic FROM public;
Index: src/backend/comm
Re: [HACKERS] Column-Level Privileges
Tom, et al, * Tom Lane (t...@sss.pgh.pa.us) wrote: > There are still some significant loose ends though: Apologies for not having this finished already, been kind of caught up in some discussions. :) > * Some of the information_schema views are specified to respond to > per-column privileges; the column_privileges and columns views > certainly need work now to meet spec, and there might be others. Done. > * It might be appropriate to let the pg_stats view expose stats for > columns you have select privilege for, even if you haven't got it > across the whole table. Done. > * We probably ought to invent has_column_privilege SQL functions > analogous to has_table_privilege; this is not just for completeness, > but is probably necessary to finish the above items. Done. > * ISTM that COPY with a column list should succeed if you have > SELECT or INSERT privilege on just the mentioned columns. Currently working on this one, doesn't look too bad, but I'm not going to get it finished tonight. Once I've got this done, hopefully tomorrow, I'll send in a patch against HEAD for all of the above. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
On Thu, Jan 22, 2009 at 07:03:45PM -0500, Tom Lane wrote:
> BTW, something else I'd meant to bring up for discussion is whether
> anyone likes the formatting of column privileges in \dp:
>
> regression=# create table foo(bar int, baz int);
> CREATE TABLE
> regression=# grant select on foo to joe;
> GRANT
> regression=# grant insert(bar), update(baz) on foo to joe;
> GRANT
> regression=# \dp foo
> Access privileges
> Schema | Name | Type | Access privileges | Column access privileges
> +--+---+---+--
> public | foo | table | postgres=arwdDxt/postgres | bar:
>: joe=r/postgres: joe=a/postgres
>: baz:
>: joe=w/postgres
> (1 row)
>
> (The colons after the column names are something I added on my own
> authority to Stephen's original.)
>
> This seems a bit ASCII-art-ish to me; it certainly wouldn't be
> readily parsable by programs. Now that's not really the design goal
> for \d output, and I don't have a better suggestion offhand, but
> still... anyone got a better idea?
Apart from enclosing braces and commas in between, it looks like JSON.
Maybe adding those in would help :)
regression=# \dp foo
Access privileges
Schema | Name | Type | Access privileges | Column access privileges
+--+---+---+--
public | foo | table | postgres=arwdDxt/postgres | {
: joe=r/postgres: bar:
: joe=a/postgres,
: baz:
: joe=w/postgres
: }
(1 row)
Cheers,
David.
--
David Fetter http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fet...@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Well, the examples I've looked at fit in 80 columns, but it's true that > all the identifiers involved were pretty short. The alternative I think > you're suggesting is Yeah, I see that now. I guess you'd need a column identifier wider than 'Column Access Privileges' or so, which is almost asking for trouble already, or a combination of grantee+privs+grantor greater than around the same, which would require rolenames of >9 characters for grantee and grantor, which is probably not that common. The new stuff added to split the ACL across lines is pretty nice. > which is definitely more compact horizontally, but I think it's harder > to follow. It's also *less* compact vertically, which is not a > negligible consideration either. yea, I'd rather we provide more information on a given row than add additional rows, but I also tend to run my DB-work terminals at 220x70 or so, which seems to indicate I'm in the minority. :) Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
Stephen Frost writes: > One thing that just occured to me is that we could, should we want to, > move the column-level privs over into the 'Access privileges' column by > just adding them on after the table-level privs. We would want to make > sure the table-level privs come first and maybe have some seperator to > indicate that the following are column-level privs. > That might make the display nicer on 80-col systems, though personally > I like larger windows. :) Well, the examples I've looked at fit in 80 columns, but it's true that all the identifiers involved were pretty short. The alternative I think you're suggesting is Access privileges Schema | Name | Type | Access privileges +--+---+--- public | foo | table | postgres=arwdDxt/postgres : joe=r/postgres : bar: : joe=a/postgres : baz: : joe=w/postgres (1 row) which is definitely more compact horizontally, but I think it's harder to follow. It's also *less* compact vertically, which is not a negligible consideration either. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
* Tom Lane (t...@sss.pgh.pa.us) wrote: > BTW, something else I'd meant to bring up for discussion is whether > anyone likes the formatting of column privileges in \dp: Well, I kinda like it, but that's not an entirely unbiased opinion. ;) > Access privileges > Schema | Name | Type | Access privileges | Column access privileges > +--+---+---+-- > public | foo | table | postgres=arwdDxt/postgres | bar: >: joe=r/postgres: joe=a/postgres >: baz: >: joe=w/postgres > (1 row) > > (The colons after the column names are something I added on my own > authority to Stephen's original.) sure, makes sense. > This seems a bit ASCII-art-ish to me; it certainly wouldn't be readily > parsable by programs. Now that's not really the design goal for \d > output, and I don't have a better suggestion offhand, but still... > anyone got a better idea? One thing that just occured to me is that we could, should we want to, move the column-level privs over into the 'Access privileges' column by just adding them on after the table-level privs. We would want to make sure the table-level privs come first and maybe have some seperator to indicate that the following are column-level privs. That might make the display nicer on 80-col systems, though personally I like larger windows. :) A couple of things I didn't particularly like: I don't like having to have a separate command to show column-level privs, and I don't really like displaying the column-level privs after the regular \dp output for tables. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
BTW, something else I'd meant to bring up for discussion is whether anyone likes the formatting of column privileges in \dp: regression=# create table foo(bar int, baz int); CREATE TABLE regression=# grant select on foo to joe; GRANT regression=# grant insert(bar), update(baz) on foo to joe; GRANT regression=# \dp foo Access privileges Schema | Name | Type | Access privileges | Column access privileges +--+---+---+-- public | foo | table | postgres=arwdDxt/postgres | bar: : joe=r/postgres: joe=a/postgres : baz: : joe=w/postgres (1 row) (The colons after the column names are something I added on my own authority to Stephen's original.) This seems a bit ASCII-art-ish to me; it certainly wouldn't be readily parsable by programs. Now that's not really the design goal for \d output, and I don't have a better suggestion offhand, but still... anyone got a better idea? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Applied with revisions. The main externally visible change is that I > implemented per-column REFERENCES privilege, since that's required by > spec. I did some heavy revision of the parsing support too, as per > previous dicussions, and editorial cleanup and bugfixing elsewhere. Great! Glad to hear it, and thanks for the updates and handling REFERENCES. > There are still some significant loose ends though: [...] I'll work on these and plan to finish them by Monday. > * Perhaps it would be appropriate to let LOCK TABLE succeed if you have > proper permissions on at least one column of the table. However, it's > bad enough that LOCK TABLE examines permissions before locking the table > now; I don't think it ought to be grovelling through the columns without > lock. So this might be a place to leave well enough alone. Agreed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
On Thu, Jan 22, 2009 at 3:29 PM, Tom Lane wrote: > > * We probably ought to invent has_column_privilege SQL functions > analogous to has_table_privilege; this is not just for completeness, > but is probably necessary to finish the above items. > +1 > * ISTM that COPY with a column list should succeed if you have > SELECT or INSERT privilege on just the mentioned columns. > +1 > * Perhaps it would be appropriate to let LOCK TABLE succeed if you have > proper permissions on at least one column of the table. However, it's > bad enough that LOCK TABLE examines permissions before locking the table > now; I don't think it ought to be grovelling through the columns without > lock. So this might be a place to leave well enough alone. > +1 -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL AsesorÃa y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
Stephen Frost writes: > Attached is an updated patch for column-level privileges. Applied with revisions. The main externally visible change is that I implemented per-column REFERENCES privilege, since that's required by spec. I did some heavy revision of the parsing support too, as per previous dicussions, and editorial cleanup and bugfixing elsewhere. There are still some significant loose ends though: * Some of the information_schema views are specified to respond to per-column privileges; the column_privileges and columns views certainly need work now to meet spec, and there might be others. * It might be appropriate to let the pg_stats view expose stats for columns you have select privilege for, even if you haven't got it across the whole table. * We probably ought to invent has_column_privilege SQL functions analogous to has_table_privilege; this is not just for completeness, but is probably necessary to finish the above items. * ISTM that COPY with a column list should succeed if you have SELECT or INSERT privilege on just the mentioned columns. * Perhaps it would be appropriate to let LOCK TABLE succeed if you have proper permissions on at least one column of the table. However, it's bad enough that LOCK TABLE examines permissions before locking the table now; I don't think it ought to be grovelling through the columns without lock. So this might be a place to leave well enough alone. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
Stephen Frost wrote: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> On looking closer, though, it's *still* messy and unobvious :-(. >> There is no single place in the parser where we have the complete >> multi-level query tree available in a convenient form for this sort of >> postprocessing. > > That's unfortunate. :/ > >> I've thought of a less painful variant of my third option: instead of >> making a permanent addition to RangeTblEntry, we can have a transient >> data structure attached to ParseState that lets us find the JoinExpr >> nodes for already-parsed joins. I'm going to try that next. > > Sounds reasonable. I'd be happy to help if there's anything useful I > can do at this point. I also think it can be a reasonable approach. However, as an aside, it will not be a help for SE-PostgreSQL, because it checks Query tree *after* it passed through the rewriter stage, so ParseState is already released. :-( http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/src/backend/security/sepgsql/proxy.c#395 QueryRewrite() -> pgacePostQueryRewrite() -> sepgsqlPostQueryRewrite() -> walkQueryHelper() -> walkVarHelper() -> wholeRefJoinWalker() Yes, it is an optional facility and we assume performance is not first priority for SE-PostgreSQL users. However, if its duration of life has been expanded to the tail of rewriter, I would be also happy. Thanks, -- KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
* Tom Lane (t...@sss.pgh.pa.us) wrote: > On looking closer, though, it's *still* messy and unobvious :-(. > There is no single place in the parser where we have the complete > multi-level query tree available in a convenient form for this sort of > postprocessing. That's unfortunate. :/ > I've thought of a less painful variant of my third option: instead of > making a permanent addition to RangeTblEntry, we can have a transient > data structure attached to ParseState that lets us find the JoinExpr > nodes for already-parsed joins. I'm going to try that next. Sounds reasonable. I'd be happy to help if there's anything useful I can do at this point. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
KaiGai Kohei writes: > Stephen Frost wrote: >> * Tom Lane (t...@sss.pgh.pa.us) wrote: >>> On the whole I think we have to go back to the original plan of >>> recursively searching the query's expressions after we've finished all >>> the transformations (and have a completed jointree to refer to). >> >> Honestly, I like this approach. > I agree with Stephen's opinion. > Indeed, the walker approach requires additional steps during query > parsing, but the code obviousness is a significant factor from the > point of view of security. On looking closer, though, it's *still* messy and unobvious :-(. There is no single place in the parser where we have the complete multi-level query tree available in a convenient form for this sort of postprocessing. I've thought of a less painful variant of my third option: instead of making a permanent addition to RangeTblEntry, we can have a transient data structure attached to ParseState that lets us find the JoinExpr nodes for already-parsed joins. I'm going to try that next. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
Stephen Frost wrote: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> On the whole I think we have to go back to the original plan of >> recursively searching the query's expressions after we've finished all >> the transformations (and have a completed jointree to refer to). This >> is slightly annoying on the grounds of adding parsing overhead that's >> completely useless unless per-column privileges are in use. On the >> other hand, none of the workable alternatives are exactly overhead-free >> either. >> >> Comments? > > Honestly, I like this approach. There is some additional overhead > during parsing, but it seems cleaner and more robust. Also, hopefully > in most cases where people are concerned about parse time they're > preparing their queries. If it's warrented, we could try doing > benchmarks to see how bad the impact is and if we need to do something > different. It doesn't strike me as likely to be a significant amount of > overhead though. I agree with Stephen's opinion. Indeed, the walker approach requires additional steps during query parsing, but the code obviousness is a significant factor from the point of view of security. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
* Tom Lane (t...@sss.pgh.pa.us) wrote: > ... btw, what is the reasoning behind the special cases for SELECT FOR > UPDATE in execMain.c? Basically, because the original logic allowed SELECT-FOR-UPDATE if you only had SELECT rights, which wasn't right. > If there actually is a need to treat SELECT FOR UPDATE specially, then > this code is quite wrong because it will also fire on a plain UPDATE > (assuming the UPDATE reads any existing column values, which it usually > would). Offhand though I don't see why we can't just use code that is > symmetric with the SELECT case: if requiredPerms includes UPDATE but > there are no columns called out for UPDATE, then allow it if we have > UPDATE on any column. I agree, this makes alot more sense to me. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
* Tom Lane (t...@sss.pgh.pa.us) wrote: > On the whole I think we have to go back to the original plan of > recursively searching the query's expressions after we've finished all > the transformations (and have a completed jointree to refer to). This > is slightly annoying on the grounds of adding parsing overhead that's > completely useless unless per-column privileges are in use. On the > other hand, none of the workable alternatives are exactly overhead-free > either. > > Comments? Honestly, I like this approach. There is some additional overhead during parsing, but it seems cleaner and more robust. Also, hopefully in most cases where people are concerned about parse time they're preparing their queries. If it's warrented, we could try doing benchmarks to see how bad the impact is and if we need to do something different. It doesn't strike me as likely to be a significant amount of overhead though. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Column-Level Privileges
... btw, what is the reasoning behind the special cases for SELECT FOR UPDATE in execMain.c? /* Check if this is SELECT-FOR-UPDATE and handle * accordingly. */ if(remainingPerms & ACL_UPDATE && pg_attribute_aclcheck_all(relOid, userid, ACL_UPDATE, ACLMASK_ALL) != ACLCHECK_OK) aclcheck_error(ACLCHECK_NO_PRIV, ACL_KIND_CLASS, get_rel_name(relOid)); If there actually is a need to treat SELECT FOR UPDATE specially, then this code is quite wrong because it will also fire on a plain UPDATE (assuming the UPDATE reads any existing column values, which it usually would). Offhand though I don't see why we can't just use code that is symmetric with the SELECT case: if requiredPerms includes UPDATE but there are no columns called out for UPDATE, then allow it if we have UPDATE on any column. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column-Level Privileges
Stephen Frost writes: > Attached is an updated patch for column-level privileges. I'm working on getting this committed. I've run into a major stumbling block in the parse-time marking of columns for SELECT privileges: the do-it-as-the-Vars-get-transformed approach basically doesn't work as-is. The problem comes from whole-row Vars referencing JOIN relations. We already talked about the need to mark column 0 as referenced for a whole-row Var, but if the Var is referencing a join then the correct thing is to recursively mark column 0 of the two input relations. (The existing patch simply crashes in this case ...) The problem is that there is no reliable way to identify the two input relations given only the join RTE. The normal thing we do is to dig in the query jointree for the JoinExpr, but during parse analysis the jointree is still being built and we don't have access to it :-(. The failure case would be where an upper JOIN/ON clause contains a whole-row reference to a contained JOIN relation. I considered a couple of alternatives: * Modify the recursion in transformFromClauseItem so that we can somehow get at the partially-built jointree for the current join item. Ugly and probably fragile. * Depend on the join's joinaliasvars list to contain references to both sides of the join. This fails in various corner cases, for instance LEFT NATURAL JOIN where every column of the righthand rel is a common column. * Add the left and right input RT indexes to join RTEs, so that we can get hold of them without needing to search the jointree. Kind of annoying to do this just for the one usage, mainly because there's a lot of infrastructure needed to update such entries during rewriter/planner manipulations. (Which would all be 100% useless anyway if the fields are only examined by the parser, but I dislike the idea of having RTE fields that aren't valid after parse time ...) On the whole I think we have to go back to the original plan of recursively searching the query's expressions after we've finished all the transformations (and have a completed jointree to refer to). This is slightly annoying on the grounds of adding parsing overhead that's completely useless unless per-column privileges are in use. On the other hand, none of the workable alternatives are exactly overhead-free either. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Column-Level Privileges
All, Attached is an updated patch for column-level privileges. This is a very minor change to use get_rel_name(), so the main point of this is to update the general community on the status of the patch. It was pointed out to me that some folks havn't been able to follow along and so aren't sure of the status. Overall, I feel this patch is definitely ready for another review. Markus, Alvaro, as the official CommitFest Reviewers, I'd really like your feedback on this version. Comments are welcome from others too, of course. Changes since the November patch: - column-level privileges are now respected during JOINs, including NATURAL JOINs and JOINs with USING clauses, per the SQL specification, many thanks to KaiGai and Tom for that! - Regression tests have been added and are reasonably extensive, but more testing is always good, please test and comment if you're interested in this capability! - Documentation has been added - psql and pg_dump, support has been added - The code has been cleaned up, bits of code refactored into seperate functions (pg_attribute_aclcheck_all, ExecGrant_Attribute), follows coding practices better - execMain is now cleaner in how it handles permissions that aren't applicable to columns - Dependency handling has been fixed - Interfaces have been made cleaner between acl.c and aclchk.c, aclchk.c no longer knows the deep innards of an Acl - Comments added about impact of attacl being added to pg_attribute - Priv node renamed to AccessPriv - RTE variables cols_sel and cols_mod changed to Bitmapsets - outfuncs support added for AccessPriv - readfuncs now support Bitmapsets - error-handling for GRANT CREATE (col1) improved - system columns now handled when explicitly requested - Other minor changes/bug fixes I'm not aware of any outstanding issues at this point. The changes recently have been pretty minor, and I really feel like things have settled down a great deal with this patch. Thanks! Stephen colprivs_2009011502.diff.gz Description: Binary data signature.asc Description: Digital signature
Re: [HACKERS] column level privileges
Jaime Casanova wrote: On Tue, Apr 1, 2008 at 5:40 PM, Andrew Dunstan <[EMAIL PROTECTED]> wrote: Apologies if this gets duplicated - original seems to have been dropped due to patch size - this time I am sending it gzipped. just for the record, this patch doesn't apply cleanly to CVS I'm not at all surprised. As I said in the original post: I'm going to see how much bitrot there is and see what changes are necessary to get it to apply. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] column level privileges
On Tue, Apr 1, 2008 at 5:40 PM, Andrew Dunstan <[EMAIL PROTECTED]> wrote: > > Apologies if this gets duplicated - original seems to have been dropped due > to patch size - this time I am sending it gzipped. > just for the record, this patch doesn't apply cleanly to CVS -- regards, Jaime Casanova -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] column level privileges
Postgres does not backport features, so you would need to retrofit the patch to 8.3 yourself, or pay / persuade somebody else to do that for you. That should not be too hard, as it was in fact developed late in the 8.3 cycle. Before you jump on it as suiting your needs, read carefully. In particular, take note of the fact that it is SQL92 privileges, which specifically do NOT include SELECT restrictions. cheers andrew sanjay sharma wrote: It would be great help to me, and I am sure for many other people too who are working with security solutions, if this feature is released as patch before 8.4 release. Sanjay Sharma > Date: Tue, 1 Apr 2008 22:02:30 -0400 > From: [EMAIL PROTECTED] > To: [EMAIL PROTECTED] > CC: pgsql-hackers@postgresql.org > Subject: Re: [HACKERS] column level privileges > > > > The earliest will be 8.4, which is many many months away. > > It should be possible to produce a patch for 8.3 if you're interested. > > cheers > > andrew > > sanjay sharma wrote: > > Hello Andrew, > > > > When do you expect this patch to go in production and available for > > public use? I would keep an eye for its release. > > > > Sanjay Sharma > > > > > Date: Tue, 1 Apr 2008 18:40:24 -0400 > > > From: [EMAIL PROTECTED] > > > To: pgsql-hackers@postgresql.org > > > Subject: [HACKERS] column level privileges > > > > > > > > > Apologies if this gets duplicated - original seems to have been dropped > > > due to patch size - this time I am sending it gzipped. > > > > > > cheers > > > > > > andrew > > > > > > Original Message > > > Subject: column level privileges > > > Date: Tue, 01 Apr 2008 08:32:25 -0400 > > > From: Andrew Dunstan <[EMAIL PROTECTED]> > > > To: Patches (PostgreSQL) <[EMAIL PROTECTED]> > > > > > > > > > > > > This patch by Golden Lui was his work for the last Google SoC. I was > > his > > > mentor for the project. I have just realised that he didn't send his > > > final patch to the list. > > > > > > I guess it's too late for the current commit-fest, but it really needs > > > to go on a patch queue (my memory on this was jogged by Tom's recent > > > mention of $Subject). > > > > > > I'm going to see how much bitrot there is and see what changes are > > > necessary to get it to apply. > > > > > > cheers > > > > > > andrew > > > > > > > > > - > > > Here is a README for the whole patch. > > > > > > According to the SQL92 standard, there are four levels in the privilege > > > hierarchy, i.e. database, tablespace, table, and column. Most > > commercial > > > DBMSs support all the levels, but column-level privilege is hitherto > > > unaddressed in the PostgreSQL, and this patch try to implement it. > > > > > > What this patch have done: > > > 1. The execution of GRANT/REVOKE for column privileges. Now only > > > INSERT/UPDATE/REFERENCES privileges are supported, as SQL92 specified. > > > SELECT privilege is now not supported. This part includes: > > > 1.1 Add a column named 'attrel' in pg_attribute catalog to store > > > column privileges. Now all column privileges are stored, no matter > > > whether they could be implied from table-level privilege. > > > 1.2 Parser for the new kind of GRANT/REVOKE commands. > > > 1.3 Execution of GRANT/REVOKE for column privileges. Corresponding > > > column privileges will be added/removed automatically if no column is > > > specified, as SQL standard specified. > > > 2. Column-level privilege check. > > > Now for UPDATE/INSERT/REFERENCES privilege, privilege check will be > > > done ONLY on column level. Table-level privilege check was done in the > > > function InitPlan. Now in this patch, these three kind of privilege are > > > checked during the parse phase. > > > 2.1 For UPDATE/INSERT commands. Privilege check is done in the > > > function transformUpdateStmt/transformInsertStmt. > > > 2.2 For REFERENCES, privilege check is done in the function > > > ATAddForeignKeyConstraint. This function will be called whenever a > > > foreign key constraint is added, like create table, alter table, etc. > > > 2.3 For COPY command, INSERT privilege is check in the function > > > DoCopy. SELECT command is checked in DoCopy too. > > > 3. While adding a new
Re: [HACKERS] column level privileges
It would be great help to me, and I am sure for many other people too who are working with security solutions, if this feature is released as patch before 8.4 release. Sanjay Sharma> Date: Tue, 1 Apr 2008 22:02:30 -0400> From: [EMAIL PROTECTED]> To: [EMAIL PROTECTED]> CC: pgsql-hackers@postgresql.org> Subject: Re: [HACKERS] column level privileges> > > > The earliest will be 8.4, which is many many months away.> > It should be possible to produce a patch for 8.3 if you're interested.> > cheers> > andrew> > sanjay sharma wrote:> > Hello Andrew,> > > > When do you expect this patch to go in production and available for > > public use? I would keep an eye for its release.> > > > Sanjay Sharma> >> > > Date: Tue, 1 Apr 2008 18:40:24 -0400> > > From: [EMAIL PROTECTED]> > > To: pgsql-hackers@postgresql.org> > > Subject: [HACKERS] column level privileges> > >> > >> > > Apologies if this gets duplicated - original seems to have been dropped> > > due to patch size - this time I am sending it gzipped.> > >> > > cheers> > >> > > andrew> > >> > > Original Message > > > Subject: column level privileges> > > Date: Tue, 01 Apr 2008 08:32:25 -0400> > > From: Andrew Dunstan <[EMAIL PROTECTED]>> > > To: Patches (PostgreSQL) <[EMAIL PROTECTED]>> > >> > >> > >> > > This patch by Golden Lui was his work for the last Google SoC. I was > > his> > > mentor for the project. I have just realised that he didn't send his> > > final patch to the list.> > >> > > I guess it's too late for the current commit-fest, but it really needs> > > to go on a patch queue (my memory on this was jogged by Tom's recent> > > mention of $Subject).> > >> > > I'm going to see how much bitrot there is and see what changes are> > > necessary to get it to apply.> > >> > > cheers> > >> > > andrew> > >> > >> > > -> > > Here is a README for the whole patch.> > >> > > According to the SQL92 standard, there are four levels in the privilege> > > hierarchy, i.e. database, tablespace, table, and column. Most > > commercial> > > DBMSs support all the levels, but column-level privilege is hitherto> > > unaddressed in the PostgreSQL, and this patch try to implement it.> > >> > > What this patch have done:> > > 1. The execution of GRANT/REVOKE for column privileges. Now only> > > INSERT/UPDATE/REFERENCES privileges are supported, as SQL92 specified.> > > SELECT privilege is now not supported. This part includes:> > > 1.1 Add a column named 'attrel' in pg_attribute catalog to store> > > column privileges. Now all column privileges are stored, no matter> > > whether they could be implied from table-level privilege.> > > 1.2 Parser for the new kind of GRANT/REVOKE commands.> > > 1.3 Execution of GRANT/REVOKE for column privileges. Corresponding> > > column privileges will be added/removed automatically if no column is> > > specified, as SQL standard specified.> > > 2. Column-level privilege check.> > > Now for UPDATE/INSERT/REFERENCES privilege, privilege check will be> > > done ONLY on column level. Table-level privilege check was done in the> > > function InitPlan. Now in this patch, these three kind of privilege are> > > checked during the parse phase.> > > 2.1 For UPDATE/INSERT commands. Privilege check is done in the> > > function transformUpdateStmt/transformInsertStmt.> > > 2.2 For REFERENCES, privilege check is done in the function> > > ATAddForeignKeyConstraint. This function will be called whenever a> > > foreign key constraint is added, like create table, alter table, etc.> > > 2.3 For COPY command, INSERT privilege is check in the function> > > DoCopy. SELECT command is checked in DoCopy too.> > > 3. While adding a new column to a table using ALTER TABLE command, set> > > appropriate privilege for the new column according to privilege already> > > granted on the table.> > > 4. Allow pg_dump and pg_dumpall to dump in/out column privileges.> > > 5. Add a column named objsubid in pg_shdepend catalog to record ACL> > > dependencies between column and roles.> > > 6. modify the grammar of ECPG to support column level privileges.> > > 7. change psql's \z (\d
Re: [HACKERS] column level privileges
The earliest will be 8.4, which is many many months away. It should be possible to produce a patch for 8.3 if you're interested. cheers andrew sanjay sharma wrote: Hello Andrew, When do you expect this patch to go in production and available for public use? I would keep an eye for its release. Sanjay Sharma > Date: Tue, 1 Apr 2008 18:40:24 -0400 > From: [EMAIL PROTECTED] > To: pgsql-hackers@postgresql.org > Subject: [HACKERS] column level privileges > > > Apologies if this gets duplicated - original seems to have been dropped > due to patch size - this time I am sending it gzipped. > > cheers > > andrew > > Original Message > Subject: column level privileges > Date: Tue, 01 Apr 2008 08:32:25 -0400 > From: Andrew Dunstan <[EMAIL PROTECTED]> > To: Patches (PostgreSQL) <[EMAIL PROTECTED]> > > > > This patch by Golden Lui was his work for the last Google SoC. I was his > mentor for the project. I have just realised that he didn't send his > final patch to the list. > > I guess it's too late for the current commit-fest, but it really needs > to go on a patch queue (my memory on this was jogged by Tom's recent > mention of $Subject). > > I'm going to see how much bitrot there is and see what changes are > necessary to get it to apply. > > cheers > > andrew > > > - > Here is a README for the whole patch. > > According to the SQL92 standard, there are four levels in the privilege > hierarchy, i.e. database, tablespace, table, and column. Most commercial > DBMSs support all the levels, but column-level privilege is hitherto > unaddressed in the PostgreSQL, and this patch try to implement it. > > What this patch have done: > 1. The execution of GRANT/REVOKE for column privileges. Now only > INSERT/UPDATE/REFERENCES privileges are supported, as SQL92 specified. > SELECT privilege is now not supported. This part includes: > 1.1 Add a column named 'attrel' in pg_attribute catalog to store > column privileges. Now all column privileges are stored, no matter > whether they could be implied from table-level privilege. > 1.2 Parser for the new kind of GRANT/REVOKE commands. > 1.3 Execution of GRANT/REVOKE for column privileges. Corresponding > column privileges will be added/removed automatically if no column is > specified, as SQL standard specified. > 2. Column-level privilege check. > Now for UPDATE/INSERT/REFERENCES privilege, privilege check will be > done ONLY on column level. Table-level privilege check was done in the > function InitPlan. Now in this patch, these three kind of privilege are > checked during the parse phase. > 2.1 For UPDATE/INSERT commands. Privilege check is done in the > function transformUpdateStmt/transformInsertStmt. > 2.2 For REFERENCES, privilege check is done in the function > ATAddForeignKeyConstraint. This function will be called whenever a > foreign key constraint is added, like create table, alter table, etc. > 2.3 For COPY command, INSERT privilege is check in the function > DoCopy. SELECT command is checked in DoCopy too. > 3. While adding a new column to a table using ALTER TABLE command, set > appropriate privilege for the new column according to privilege already > granted on the table. > 4. Allow pg_dump and pg_dumpall to dump in/out column privileges. > 5. Add a column named objsubid in pg_shdepend catalog to record ACL > dependencies between column and roles. > 6. modify the grammar of ECPG to support column level privileges. > 7. change psql's \z (\dp) command to support listing column privileges > for tables and views. If \z(\dp) is run with a pattern, column > privileges are listed after table level privileges. > 8. Regression test for column-level privileges. I changed both > privileges.sql and expected/privileges.out, so regression check is now > all passed. > > Best wishes > Dong > -- > Guodong Liu > Database Lab, School of EECS, Peking University > Room 314, Building 42, Peking University, Beijing, 100871, China > > Exclusive Marriage Proposals! Find UR life partner at Shaadi.com Try it! <http://ss1.richmedia.in/recurl.asp?pid=430> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] column level privileges
Hello Andrew, When do you expect this patch to go in production and available for public use? I would keep an eye for its release. Sanjay Sharma> Date: Tue, 1 Apr 2008 18:40:24 -0400> From: [EMAIL PROTECTED]> To: pgsql-hackers@postgresql.org> Subject: [HACKERS] column level privileges> > > Apologies if this gets duplicated - original seems to have been dropped > due to patch size - this time I am sending it gzipped.> > cheers> > andrew> > Original Message > Subject: column level privileges> Date: Tue, 01 Apr 2008 08:32:25 -0400> From: Andrew Dunstan <[EMAIL PROTECTED]>> To: Patches (PostgreSQL) <[EMAIL PROTECTED]>> > > > This patch by Golden Lui was his work for the last Google SoC. I was his > mentor for the project. I have just realised that he didn't send his > final patch to the list.> > I guess it's too late for the current commit-fest, but it really needs > to go on a patch queue (my memory on this was jogged by Tom's recent > mention of $Subject).> > I'm going to see how much bitrot there is and see what changes are > necessary to get it to apply.> > cheers> > andrew> > > -> Here is a README for the whole patch.> > According to the SQL92 standard, there are four levels in the privilege > hierarchy, i.e. database, tablespace, table, and column. Most commercial > DBMSs support all the levels, but column-level privilege is hitherto > unaddressed in the PostgreSQL, and this patch try to implement it.> > What this patch have done:> 1. The execution of GRANT/REVOKE for column privileges. Now only > INSERT/UPDATE/REFERENCES privileges are supported, as SQL92 specified. > SELECT privilege is now not supported. This part includes:> 1.1 Add a column named 'attrel' in pg_attribute catalog to store > column privileges. Now all column privileges are stored, no matter > whether they could be implied from table-level privilege.> 1.2 Parser for the new kind of GRANT/REVOKE commands.> 1.3 Execution of GRANT/REVOKE for column privileges. Corresponding > column privileges will be added/removed automatically if no column is > specified, as SQL standard specified.> 2. Column-level privilege check.> Now for UPDATE/INSERT/REFERENCES privilege, privilege check will be > done ONLY on column level. Table-level privilege check was done in the > function InitPlan. Now in this patch, these three kind of privilege are > checked during the parse phase.> 2.1 For UPDATE/INSERT commands. Privilege check is done in the > function transformUpdateStmt/transformInsertStmt.> 2.2 For REFERENCES, privilege check is done in the function > ATAddForeignKeyConstraint. This function will be called whenever a > foreign key constraint is added, like create table, alter table, etc.> 2.3 For COPY command, INSERT privilege is check in the function > DoCopy. SELECT command is checked in DoCopy too.> 3. While adding a new column to a table using ALTER TABLE command, set > appropriate privilege for the new column according to privilege already > granted on the table.> 4. Allow pg_dump and pg_dumpall to dump in/out column privileges.> 5. Add a column named objsubid in pg_shdepend catalog to record ACL > dependencies between column and roles.> 6. modify the grammar of ECPG to support column level privileges.> 7. change psql's \z (\dp) command to support listing column privileges > for tables and views. If \z(\dp) is run with a pattern, column > privileges are listed after table level privileges.> 8. Regression test for column-level privileges. I changed both > privileges.sql and expected/privileges.out, so regression check is now > all passed.> > Best wishes> Dong> -- > Guodong Liu> Database Lab, School of EECS, Peking University> Room 314, Building 42, Peking University, Beijing, 100871, China> > _ Technology : Catch up on updates on the latest Gadgets, Reviews, Gaming and Tips to use technology etc. http://computing.in.msn.com/
[HACKERS] column level privileges
Apologies if this gets duplicated - original seems to have been dropped due to patch size - this time I am sending it gzipped. cheers andrew Original Message Subject:column level privileges Date: Tue, 01 Apr 2008 08:32:25 -0400 From: Andrew Dunstan <[EMAIL PROTECTED]> To: Patches (PostgreSQL) <[EMAIL PROTECTED]> This patch by Golden Lui was his work for the last Google SoC. I was his mentor for the project. I have just realised that he didn't send his final patch to the list. I guess it's too late for the current commit-fest, but it really needs to go on a patch queue (my memory on this was jogged by Tom's recent mention of $Subject). I'm going to see how much bitrot there is and see what changes are necessary to get it to apply. cheers andrew - Here is a README for the whole patch. According to the SQL92 standard, there are four levels in the privilege hierarchy, i.e. database, tablespace, table, and column. Most commercial DBMSs support all the levels, but column-level privilege is hitherto unaddressed in the PostgreSQL, and this patch try to implement it. What this patch have done: 1. The execution of GRANT/REVOKE for column privileges. Now only INSERT/UPDATE/REFERENCES privileges are supported, as SQL92 specified. SELECT privilege is now not supported. This part includes: 1.1 Add a column named 'attrel' in pg_attribute catalog to store column privileges. Now all column privileges are stored, no matter whether they could be implied from table-level privilege. 1.2 Parser for the new kind of GRANT/REVOKE commands. 1.3 Execution of GRANT/REVOKE for column privileges. Corresponding column privileges will be added/removed automatically if no column is specified, as SQL standard specified. 2. Column-level privilege check. Now for UPDATE/INSERT/REFERENCES privilege, privilege check will be done ONLY on column level. Table-level privilege check was done in the function InitPlan. Now in this patch, these three kind of privilege are checked during the parse phase. 2.1 For UPDATE/INSERT commands. Privilege check is done in the function transformUpdateStmt/transformInsertStmt. 2.2 For REFERENCES, privilege check is done in the function ATAddForeignKeyConstraint. This function will be called whenever a foreign key constraint is added, like create table, alter table, etc. 2.3 For COPY command, INSERT privilege is check in the function DoCopy. SELECT command is checked in DoCopy too. 3. While adding a new column to a table using ALTER TABLE command, set appropriate privilege for the new column according to privilege already granted on the table. 4. Allow pg_dump and pg_dumpall to dump in/out column privileges. 5. Add a column named objsubid in pg_shdepend catalog to record ACL dependencies between column and roles. 6. modify the grammar of ECPG to support column level privileges. 7. change psql's \z (\dp) command to support listing column privileges for tables and views. If \z(\dp) is run with a pattern, column privileges are listed after table level privileges. 8. Regression test for column-level privileges. I changed both privileges.sql and expected/privileges.out, so regression check is now all passed. Best wishes Dong -- Guodong Liu Database Lab, School of EECS, Peking University Room 314, Building 42, Peking University, Beijing, 100871, China pg_colpriv_version_0.4.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
