Smaller access privilege changes

Started by Peter Eisentrautover 24 years ago9 messages
#1Peter Eisentraut
peter_e@gmx.net

Since there is no plan yet how to do a wholesale overhaul of the ACL
system, I'd like to stick a few improvements into the current
implementation:

* Make DELETE distinct from UPDATE privilege

* rename the internal representation: s = select, i = insert, u = update,
d = delete, R = rules

* LOCK > AccessShare will require UPDATE or DELETE. This is not a change
in effect.

* Sequence nextval and setval will require UPDATE; DELETE won't do any
longer.

* COPY FROM will require INSERT privilege. It used to require
UPDATE/DELETE, it think that is not correct..

* INSERT (the command) will require INSERT privilege. UPDATE/DELETE won't
do any longer. (Why was this there?)

* Implement SQL REFERENCES privilege: grant references on A to B will
allow user B to create a foreign key referencing table A as primary key.

I'd also like to create a regression test. That will require creating
some global users and groups in the installation where the test runs. I
think as long as we name them "regressuser1", "regressgroup2", etc. this
won't harm anyone.

Comments?

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#1)
Re: Smaller access privilege changes

Peter Eisentraut <peter_e@gmx.net> writes:

* Make DELETE distinct from UPDATE privilege

Okay.

* rename the internal representation: s = select, i = insert, u = update,
d = delete, R = rules

Since the internal representation is visible to users, I fear that a
wholesale renaming will break existing applications. Can we make this
part of the change less intrusive?

* COPY FROM will require INSERT privilege. It used to require
UPDATE/DELETE, it think that is not correct..
* INSERT (the command) will require INSERT privilege. UPDATE/DELETE won't
do any longer. (Why was this there?)

Both of these are basically there because the underlying model is "read
and write", with "append" as a limited form of "write"; so "write"
allows everything that "append" does. But if we are switching to a full
"insert/update/delete" model then this behavior should go away.

* Implement SQL REFERENCES privilege: grant references on A to B will
allow user B to create a foreign key referencing table A as primary key.

Which privilege will SELECT FOR UPDATE require, and how do you plan to
get the system to distinguish users' SELECT FOR UPDATE from the commands
issued by the foreign key triggers?

I'd also like to create a regression test. That will require creating
some global users and groups in the installation where the test runs. I
think as long as we name them "regressuser1", "regressgroup2", etc. this
won't harm anyone.

Seems reasonable, but be careful to cope with the case where these
objects already exist from a prior regression run.

regards, tom lane

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#2)
Re: Smaller access privilege changes

Tom Lane writes:

* rename the internal representation: s = select, i = insert, u = update,
d = delete, R = rules

Since the internal representation is visible to users, I fear that a
wholesale renaming will break existing applications. Can we make this
part of the change less intrusive?

I guess so. I could make r=select, a=insert, w=update, d=delete, R=rules,
x=reference. Of course we will have to break this eventually, but we
might as well put it off until then.

* Implement SQL REFERENCES privilege: grant references on A to B will
allow user B to create a foreign key referencing table A as primary key.

Which privilege will SELECT FOR UPDATE require, and how do you plan to
get the system to distinguish users' SELECT FOR UPDATE from the commands
issued by the foreign key triggers?

The REFERENCES privilege will be checked by CREATE TABLE and ALTER TABLE
(where it currently says "must be owner"). SELECT FOR UPDATE is not
affected by this and will stay the way it is.

I'd also like to create a regression test. That will require creating
some global users and groups in the installation where the test runs. I
think as long as we name them "regressuser1", "regressgroup2", etc. this
won't harm anyone.

Seems reasonable, but be careful to cope with the case where these
objects already exist from a prior regression run.

I drop them at the end of the test.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#3)
Re: Smaller access privilege changes

Peter Eisentraut <peter_e@gmx.net> writes:

Tom Lane writes:

* rename the internal representation: s = select, i = insert, u = update,
d = delete, R = rules

Since the internal representation is visible to users, I fear that a
wholesale renaming will break existing applications. Can we make this
part of the change less intrusive?

I guess so. I could make r=select, a=insert, w=update, d=delete, R=rules,
x=reference. Of course we will have to break this eventually, but we
might as well put it off until then.

My thought exactly. If we were going straight to full SQL compliance
then I wouldn't worry, but I don't like the idea of breaking apps now
and then breaking them some more later.

A different tack is to go ahead and make the change now, but try to
ensure we won't have to change the coding again when we do the rest of
the SQL protection model. Do you know what is still missing given this
change?

Seems reasonable, but be careful to cope with the case where these
objects already exist from a prior regression run.

I drop them at the end of the test.

What if the prior test crashed or was aborted by the user midway
through? Cleaning up at the end of the test is good, but I think
it'd be wise for pg_regress to also drop these users/groups before
it starts the run.

regards, tom lane

#5Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#2)
Re: Smaller access privilege changes

Peter Eisentraut <peter_e@gmx.net> writes:

* Make DELETE distinct from UPDATE privilege

Okay.

* rename the internal representation: s = select, i = insert, u = update,
d = delete, R = rules

Since the internal representation is visible to users, I fear that a
wholesale renaming will break existing applications. Can we make this
part of the change less intrusive?

If we are voting, I kind of like the newer letters. The old ones made
little sense except to QUEL users.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#6Oliver Elphick
olly@lfix.co.uk
In reply to: Bruce Momjian (#5)
Re: Smaller access privilege changes

Peter Eisentraut wrote:

Since there is no plan yet how to do a wholesale overhaul of the ACL
system, I'd like to stick a few improvements into the current
implementation:

* COPY FROM will require INSERT privilege. It used to require
UPDATE/DELETE, it think that is not correct..

COPY FROM should require either INSERT or UPDATE or both according to
what rows are being copied. If a copied primary key already exists,
that will be an update. I don't see any reason to give COPY FROM any
special treatment - surely it should succeed or fail according to
whether what it is trying to do is within the user's privileges.

--
Oliver Elphick Oliver.Elphick@lfix.co.uk
Isle of Wight http://www.lfix.co.uk/oliver
PGP: 1024R/32B8FAA1: 97 EA 1D 47 72 3F 28 47 6B 7E 39 CC 56 E4 C1 47
GPG: 1024D/3E1D0C1C: CA12 09E0 E8D5 8870 5839 932A 614D 4C34 3E1D 0C1C
========================================
"I will praise thee; for I am fearfully and wonderfully
made..." Psalms 139:14

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Oliver Elphick (#6)
Re: Smaller access privilege changes

Oliver Elphick writes:

COPY FROM should require either INSERT or UPDATE or both according to
what rows are being copied. If a copied primary key already exists,
that will be an update.

No, it will be an error.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#5)
Re: Smaller access privilege changes

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Peter Eisentraut <peter_e@gmx.net> writes:

* rename the internal representation: s = select, i = insert, u = update,
d = delete, R = rules

If we are voting, I kind of like the newer letters.

Actually, I like 'em too ... *if* they're the final set. I'm just
concerned about the idea of breaking user applications for an
intermediate stop on the way to full SQL92 privilege specs. It'd be
especially nasty if we found ourselves using non-intuitive coding for
the full SQL specs so as to stay compatible with an intermediate stop.

regards, tom lane

#9Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#4)
Re: Smaller access privilege changes

Tom Lane writes:

A different tack is to go ahead and make the change now, but try to
ensure we won't have to change the coding again when we do the rest of
the SQL protection model. Do you know what is still missing given this
change?

I don't think this will end up being the user-visible interface. We
should have a view, perhaps as part of the standard information_schema,
that users can query for privilege information. So for now I'll leave the
old letters the way they are and just add a few new ones.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter