Thread: When and where to check for function permissions
In my current implementation for function privileges, I have the function permission check somewhere down in the executor. (To be precise, the permission is determined when the fcache is initialized, and it's checked in ExecMakeFunctionResult.) Now I remembered the way SQL99 specifies function resolution, which has the permission check before the function resolution begins. See also http://archives.postgresql.org/pgsql-hackers/2002-01/msg01120.php for the full details. This makes some sense, because normally you'd want the parser to choose only between the functions you have access to. I do have two concerns, however: 1. It would lead to confusing error messages, i.e., always "not found" instead of "permission denied". 2. It would make a great deal of more sense if the table name resolution that will be necessary in the new schema implementation were done the same way. But I have a feeling that this could get messy when rewrite rules are involved. (More generally, the whole schema resolution could get messy when rewrite rules are involved.) Any comments? -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> Now I remembered the way SQL99 specifies
> function resolution, which has the permission check before the function
> resolution begins.
That may be what the spec says, but I think the spec is completely
brain-dead in this regard and should be ignored. We do not resolve
table names that way, why should we resolve function names?
Even more to the point, what happens when someone adds or revokes
privileges that would affect already-planned queries? If I can still
call a function that is referenced by an already-planned query even
though the function's owner has revoked my right to do so, that is a
bug. On the other hand, if the query continues to "work" but silently
calls a different function than I was expecting, that's not cool either.
We did some nontrivial work awhile back to ensure that table privileges
would be checked at execution time and not before. Function privileges
*must* be handled the same way.
regards, tom lane
Curiosity got me... User 1: Create table; grant all on table to public; User 2: select * from table for update; User 1: revoke all on table from public; User 2: update table set column = column + 1; As long as User 2 holds a lock on the rows shouldn't the user have access to the rows? I'd expect the revoke statement to be blocked by the locks on those rows. -- Rod Taylor This message represents the official view of the voices in my head ----- Original Message ----- From: "Tom Lane" <tgl@sss.pgh.pa.us> To: "Peter Eisentraut" <peter_e@gmx.net> Cc: "PostgreSQL Development" <pgsql-hackers@postgresql.org> Sent: Wednesday, February 13, 2002 5:01 PM Subject: Re: [HACKERS] When and where to check for function permissions > Peter Eisentraut <peter_e@gmx.net> writes: > > Now I remembered the way SQL99 specifies > > function resolution, which has the permission check before the function > > resolution begins. > > That may be what the spec says, but I think the spec is completely > brain-dead in this regard and should be ignored. We do not resolve > table names that way, why should we resolve function names? > > Even more to the point, what happens when someone adds or revokes > privileges that would affect already-planned queries? If I can still > call a function that is referenced by an already-planned query even > though the function's owner has revoked my right to do so, that is a > bug. On the other hand, if the query continues to "work" but silently > calls a different function than I was expecting, that's not cool either. > > We did some nontrivial work awhile back to ensure that table privileges > would be checked at execution time and not before. Function privileges > *must* be handled the same way. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 5: Have you checked our extensive FAQ? > > http://www.postgresql.org/users-lounge/docs/faq.html >
"Rod Taylor" <rbt@zort.ca> writes:
> I'd expect the revoke statement to be blocked by
> the locks on those rows.
We could do it that way, but it doesn't seem like a really good idea to
me --- in essence you'd be allowing a denial-of-service attack, no?
regards, tom lane
Thought about that. But if you have a user run through your system and locks all the tables for a long time the least of my worries are the permissions. Generally it's getting that damn user disconnected then taken out back shot so that the system is moving forward again. Anyway, the point was that if Postgresql is going to be worried about users running stored plans on functions they don't have permission on, then a user shouldn't expect permission to be revoked in the middle of a transaction if they hold a lock on the item. -- Rod Taylor This message represents the official view of the voices in my head ----- Original Message ----- From: "Tom Lane" <tgl@sss.pgh.pa.us> To: "Rod Taylor" <rbt@zort.ca> Cc: "Peter Eisentraut" <peter_e@gmx.net>; "PostgreSQL Development" <pgsql-hackers@postgresql.org> Sent: Wednesday, February 13, 2002 5:29 PM Subject: Re: [HACKERS] When and where to check for function permissions > "Rod Taylor" <rbt@zort.ca> writes: > > I'd expect the revoke statement to be blocked by > > the locks on those rows. > > We could do it that way, but it doesn't seem like a really good idea to > me --- in essence you'd be allowing a denial-of-service attack, no? > > regards, tom lane >
Tom Lane writes: > Peter Eisentraut <peter_e@gmx.net> writes: > > Now I remembered the way SQL99 specifies > > function resolution, which has the permission check before the function > > resolution begins. > > That may be what the spec says, but I think the spec is completely > brain-dead in this regard and should be ignored. Why? > We do not resolve table names that way, why should we resolve function > names? We do not resolve table names at all. > Even more to the point, what happens when someone adds or revokes > privileges that would affect already-planned queries? The query plans are invalidated. Note: I'm not convinced of this idea either. But proclaiming it brain-dead isn't going to push me either way. You could say Unix shells are brain-dead, too, because they do the same thing. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
>> We do not resolve table names that way, why should we resolve function
>> names?
> We do not resolve table names at all.
The point is that a table you can't currently access isn't invisible.
It'd be even weirder if it were visible for some operation types and not
others. Among other things, that would break rules on views: you can
never insert into a view, so if we adopted the spec's viewpoint, you
could never see a view as target of INSERT and would thus never advance
to the next step of looking for a rewrite rule for it.
Here's another example that should give you pause: let's assume there's
an UPDATE access right for functions that controls whether you can
update the function definition (via CREATE OR REPLACE). Let's further
suppose that you have execute but not update access to function foo(int).
test=> select foo(2);-- works fine
test=> create or replace foo(int) ... etc etc ...;
If we follow the spec's lead, then CREATE OR REPLACE doesn't see the
existing definition at all (because it doesn't have the correct access
rights), and so instead of the expected update, you get a new function
definition. This might even appear to work, from your point of view,
if the new definition gets entered into a namespace closer to the front
of your search path than the original was. But other people will
continue to see the old definition if they don't share your path.
The implications for ambiguous-function resolution would be no less
bizarre and unhelpful.
You could *maybe* get away with visibility-depends-on-access-rights
in a world with no name search paths and no function name overloading
(although why you'd bother is not quite clear to me). In the presence
of those features, doing it the spec's way is sheer folly.
I would also ask for some positive reason why we should do this the
spec's way; what actual user benefit does it provide to make uncallable
functions invisible? I don't see one; all I see is user confusion.
> You could say Unix shells
> are brain-dead, too, because they do the same thing.
They do? How so? Last I checked, trying to execute a program I didn't
have exec rights to gave "no permissions", not "not found", and
certainly not "use the next one down the PATH instead".
regards, tom lane
On Wed, 13 Feb 2002, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > > You could say Unix shells > > are brain-dead, too, because they do the same thing. > > They do? How so? Last I checked, trying to execute a program I didn't > have exec rights to gave "no permissions", not "not found", and > certainly not "use the next one down the PATH instead". > That is only true if you give a fully qualified path to the program. For example, if you try to run the command as "ls" (instead of /bin/ls), you will get the first executable program "ls" in your path. If an administrator happened to create a file (without execute) named "ls" near the beginning of your path (say /usr/local/bin), then it would break, which it does not. If it was executable, it would get called instead by being first in the path. Granted some shells, eg. bash, will cache the path to the command on it first lookup and need to be rehashed (hash -r). -rocco