2010/5/24 KaiGai Kohei kai...@ak.jp.nec.com:
(2010/05/24 22:18), Robert Haas wrote:
2010/5/24 KaiGai Koheikai...@ak.jp.nec.com:
BTW, I guess the reason why permissions on attributes are not checked here
is
that we missed it at v8.4 development.
That's a little worrying. Can you construct
(2010/07/22 8:45), Robert Haas wrote:
2010/5/24 KaiGai Koheikai...@ak.jp.nec.com:
(2010/05/24 22:18), Robert Haas wrote:
2010/5/24 KaiGai Koheikai...@ak.jp.nec.com:
BTW, I guess the reason why permissions on attributes are not checked here
is
that we missed it at v8.4 development.
That's
* Robert Haas (robertmh...@gmail.com) wrote:
I think the relevant case might be where ymj owns fk_tbl but not
pk_tbl, and has REFERENCES but not SELECT on pk_tbl.
Come to think of it, I wonder if REFERENCES on fk_tbl ought to be
sufficient to create a foreign key. Currently, it requires
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
We can find out a similar case in CreateTrigger().
If I was granted TRIGGER privilege on a certain table, I can create a new
trigger on the table without its ownership. More commonly, it allows us
to modify a certain property of the table without
On Wed, Jul 21, 2010 at 9:02 PM, Stephen Frost sfr...@snowman.net wrote:
* Robert Haas (robertmh...@gmail.com) wrote:
I think the relevant case might be where ymj owns fk_tbl but not
pk_tbl, and has REFERENCES but not SELECT on pk_tbl.
Come to think of it, I wonder if REFERENCES on fk_tbl
* Robert Haas (robertmh...@gmail.com) wrote:
On Wed, Jul 21, 2010 at 9:02 PM, Stephen Frost sfr...@snowman.net wrote:
Errr, no. If I grant you REFERENCES on my table, it means you can
create a FK to it from some other table.
Well, in that case, we should fix the fine documentation:
(2010/07/22 10:04), Stephen Frost wrote:
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
We can find out a similar case in CreateTrigger().
If I was granted TRIGGER privilege on a certain table, I can create a new
trigger on the table without its ownership. More commonly, it allows us
to modify
On Thu, May 20, 2010 at 1:33 PM, Tom Lane t...@sss.pgh.pa.us wrote:
As for committing it, I would definitely like to commit the actual
hook. If we want the hook without the contrib module that's OK with
me, although I generally feel it's useful to have examples of how
hooks can be used, which
I attached three patches for the effort.
Each patch tries to tackle one theme, so it is not unreasonable.
But the ESP security hook patch (quite tiny) depends on the DML permission
refactoring patch (relatively larger). So, Robert suggested me to reconsider
the dependency of these patches.
The
2010/6/14 KaiGai Kohei kai...@ak.jp.nec.com:
I attached three patches for the effort.
Each patch tries to tackle one theme, so it is not unreasonable.
But the ESP security hook patch (quite tiny) depends on the DML permission
refactoring patch (relatively larger). So, Robert suggested me to
* Robert Haas (robertmh...@gmail.com) wrote:
This is essentially the same patch that I wrote and posted several
weeks ago, with changes to the comments and renaming of the
identifiers. Are you trying to represent it as your own work?
Ehh, I doubt it. He had included your patch in another
(2010/06/14 21:35), Stephen Frost wrote:
* Robert Haas (robertmh...@gmail.com) wrote:
This is essentially the same patch that I wrote and posted several
weeks ago, with changes to the comments and renaming of the
identifiers. Are you trying to represent it as your own work?
Ehh, I doubt
The attached patch is a revised one for DML permission checks.
List of updates:
- Source code comments in the patched functions were revised.
- ExecCheckRTPerms() and ExecCheckRTEPerms() were moved to aclchk.c,
and renamed to chkpriv_relation_perms() and chkpriv_rte_perms().
- It took the 2nd
Tom,
* Tom Lane (t...@sss.pgh.pa.us) wrote:
I haven't dug in the SQL spec to see if that addresses
the point, but it wouldn't bother me in the least to insist that
both REFERENCES and SELECT privilege are required to create an FK.
Ok. If we require REFERENCES and SELECT privs to create an FK
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
Yes, it is entirely separate issue. I don't intend to argue whether
we can assume the default PG permission allows owner to SELECT on
the table, or not.
This actually isn't a separate issue. It's the whole crux of it, as a
matter of fact.
KaiGai Kohei kai...@ak.jp.nec.com writes:
Hmm. If both REFERENCES and SELECT privilege are required to create
a new FK constraint, why RI_Initial_Check() need to check SELECT
permission prior to SPI_execute()?
It eventually checks SELECT privilege during execution of the secondary
query. It
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
The attached patch is a revised one for DML permission checks.
This is certainly alot better.
ToDo:
- makeRangeTblEntry() stuff to allocate a RTE node with given parameter
is not yet.
I'd certainly like to see the above done, or to
* Tom Lane (t...@sss.pgh.pa.us) wrote:
Because the queries inside the triggers are done with a different
current userid.
Indeed, I figured that out eventually too. Sorry it took so long. :/
Thanks,
Stephen
signature.asc
Description: Digital signature
Stephen, thanks for comments.
The attached three patches are the revised and divided ones.
A: add makeRangeTblEntry()
B: major reworks of DML permission checks
C: add an ESP hook on the DML permission checks
(2010/05/27 0:09), Stephen Frost wrote:
KaiGai,
* KaiGai Kohei
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
Stephen, thanks for comments.
The attached three patches are the revised and divided ones.
A: add makeRangeTblEntry()
Ok, didn't actually expect that. Guess my suggestion would have been to
just use makeNode() since there wasn't
Stephen,
The 'failure' may make an impression of generic errors not only permission
denied.
How about 'error_on_violation'?
Maybe 'ereport_on_violation'? I dunno, guess one isn't really better
than the other. You need to go back and fix the comment though- you
still say 'abort' there.
(2010/05/25 12:19), Robert Haas wrote:
On Mon, May 24, 2010 at 9:27 PM, Stephen Frostsfr...@snowman.net wrote:
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
We have two options; If the checker function takes the list of
RangeTblEntry,
it will be comfortable to ExecCheckRTPerms(), but not
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
OK, the attached patch reworks it according to the way.
I havn't looked at it yet, but the hook was added to ExecCheckRTPerms(),
not RTE. This was for two main reasons- it seemed simpler to us and it
meant that any security module
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
OK, the attached patch reworks it according to the way.
Reviewing this patch, there are a whole slew of problems.
#1: REALLY BIG ISSUE- Insufficient comment updates. You've changed
function definitions in a pretty serious way as well as
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
* DoCopy() and RI_Initial_Check() were reworked to call ExecCheckRTEPerms()
with locally built RangeTblEntry.
Maybe I missed it somewhere, but we still need to address the case where
the user doesn't have those SELECT permissions that we're looking
(2010/05/25 21:44), Stephen Frost wrote:
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
OK, the attached patch reworks it according to the way.
Reviewing this patch, there are a whole slew of problems.
#1: REALLY BIG ISSUE- Insufficient comment updates. You've changed
function
(2010/05/25 22:59), Stephen Frost wrote:
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
* DoCopy() and RI_Initial_Check() were reworked to call ExecCheckRTEPerms()
with locally built RangeTblEntry.
Maybe I missed it somewhere, but we still need to address the case where
the user doesn't
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
#2: REALLY BIG ISSUE- You've added ExecutorCheckPerms_hook as part of
this patch- don't, we're in feature-freeze right now and should not be
adding hooks at this time.
The patch is intended to submit for the v9.1 development, not v9.0, isn't
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
The reason why user must have SELECT privileges on the PK/FK tables is
the validateForeignKeyConstraint() entirely calls SPI_execute() to verify
FK constraints can be established between two tables (even if fallback path).
And, the reason why
(2010/05/26 11:12), Stephen Frost wrote:
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
#2: REALLY BIG ISSUE- You've added ExecutorCheckPerms_hook as part of
this patch- don't, we're in feature-freeze right now and should not be
adding hooks at this time.
The patch is intended to submit for
Stephen Frost sfr...@snowman.net writes:
That may be the case. I'm certainly more concerned with a bug in the
existing code than any new code that we're working on. The question is-
is this actually a user-visible bug? Or do we require that a user
creating an FK needs SELECT rights on the
(2010/05/26 12:17), Tom Lane wrote:
Stephen Frostsfr...@snowman.net writes:
That may be the case. I'm certainly more concerned with a bug in the
existing code than any new code that we're working on. The question is-
is this actually a user-visible bug? Or do we require that a user
(2010/05/21 1:14), Robert Haas wrote:
In yesterday's development meeting, we talked about the possibility of
a basic SE-PostgreSQL implementation that checks permissions only for
DML. Greg Smith offered the opinion that this could provide much of
the benefit of SE-PostgreSQL for many users,
2010/5/24 KaiGai Kohei kai...@ak.jp.nec.com:
BTW, I guess the reason why permissions on attributes are not checked here is
that we missed it at v8.4 development.
That's a little worrying. Can you construct and post a test case
where this results in a user-visible failure in CVS HEAD?
The
(2010/05/24 22:18), Robert Haas wrote:
2010/5/24 KaiGai Koheikai...@ak.jp.nec.com:
BTW, I guess the reason why permissions on attributes are not checked here is
that we missed it at v8.4 development.
That's a little worrying. Can you construct and post a test case
where this results in a
(2010/05/25 4:11), Stephen Frost wrote:
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
I'd like to point out two more points are necessary to be considered
for DML permission checks in addition to ExecCheckRTPerms().
* DoCopy()
Although DoCopy() is called from standard_ProcessUtility(), it
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
postgres=# ALTER TABLE pk_tbl OWNER TO ymj;
ALTER TABLE
postgres=# ALTER TABLE fk_tbl OWNER TO ymj;
ALTER TABLE
postgres=# REVOKE ALL ON pk_tbl, fk_tbl FROM ymj;
REVOKE
postgres=# GRANT REFERENCES ON pk_tbl, fk_tbl TO ymj;
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
We have two options; If the checker function takes the list of RangeTblEntry,
it will be comfortable to ExecCheckRTPerms(), but not DoCopy(). Inversely,
if the checker function takes arguments in my patch, it will be comfortable
to DoCopy(), but
(2010/05/25 10:13), Stephen Frost wrote:
KaiGai,
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
postgres=# ALTER TABLE pk_tbl OWNER TO ymj;
ALTER TABLE
postgres=# ALTER TABLE fk_tbl OWNER TO ymj;
ALTER TABLE
postgres=# REVOKE ALL ON pk_tbl, fk_tbl FROM ymj;
REVOKE
(2010/05/25 10:27), Stephen Frost wrote:
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
We have two options; If the checker function takes the list of RangeTblEntry,
it will be comfortable to ExecCheckRTPerms(), but not DoCopy(). Inversely,
if the checker function takes arguments in my patch, it
2010/5/24 KaiGai Kohei kai...@ak.jp.nec.com:
I think we need a new SPI_*() interface which allows to run the given query
without any permission checks, because these queries are purely internal
stuff,
so we can trust the query is harmless.
[...]
I'm afraid of that the
On Mon, May 24, 2010 at 9:27 PM, Stephen Frost sfr...@snowman.net wrote:
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote:
We have two options; If the checker function takes the list of RangeTblEntry,
it will be comfortable to ExecCheckRTPerms(), but not DoCopy(). Inversely,
if the checker function
Stephen Frost sfr...@snowman.net writes:
... It makes me wonder if COPY shouldn't have been implemented using
the Executor instead, but that's, again, a completely separate topic.
It wasn't, but it wants to play like it operates in the same kind of way
as INSERT, so it needs to pick up the
In yesterday's development meeting, we talked about the possibility of
a basic SE-PostgreSQL implementation that checks permissions only for
DML. Greg Smith offered the opinion that this could provide much of
the benefit of SE-PostgreSQL for many users, while being much simpler.
In fact,
Robert Haas robertmh...@gmail.com writes:
In yesterday's development meeting, we talked about the possibility of
a basic SE-PostgreSQL implementation that checks permissions only for
DML. Greg Smith offered the opinion that this could provide much of
the benefit of SE-PostgreSQL for many
On Thu, May 20, 2010 at 12:32 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Robert Haas robertmh...@gmail.com writes:
In yesterday's development meeting, we talked about the possibility of
a basic SE-PostgreSQL implementation that checks permissions only for
DML. Greg Smith offered the opinion that
Robert Haas robertmh...@gmail.com writes:
On Thu, May 20, 2010 at 12:32 PM, Tom Lane t...@sss.pgh.pa.us wrote:
Hm, I think you need to ignore RT entries that have no requiredPerms
bits set. (Not that it matters too much, unless you were proposing to
actually commit this contrib module.)
47 matches
Mail list logo