OWNER TO on all objects
Hi,
This is a preview patch - DON'T COMMIT IT TO HEAD!
What I've done in this patch is add the following:
ALTER AGGREGATE / OWNER TO
ALTER CONVERSION / OWNER TO
ALTER FUNCTION / OWNER TO
ALTER OPERATOR / OWNER TO
ALTER OPERATOR CLASS / OWNER TO
ALTER SCHEMA / OWNER TO
ALTER TYPE / OWNER TO
That means we can change the owner of all objects.
Next, I modified pg_dump to remove all SET SESSION AUTHORIZATION
commands for object creation. (I left them in on the COPY commands).
Then I made it so that pg_dump will output an OWNER TO statement after
every object creation.
This means that pg_dump can dump a restorable dump in cases where, say,
a super user created a language, and then had their superuser privs
dropped, or when a user has created a table, but has then had their
create privileges removed.
At the moment, i'm happy with how it dumps and reloads the regression
database, and i'm working on adding tests for all OWNER TO in the
regression suite. Full doc updates are already included.
Please review and give me feedback! The patch is large, but not at all
complex :)
Some questions:
* Do we need the set session auth for COPY commands still?
* Are there any subtle implications of changing owners that I haven't
realised? I know that it will affect SECURITY DEFINER for functions,
but I put that in the docs.
* Is doing this ok: ObjectIdGetDatum(typTup->typrelid)
* Is there any reason there is no RENAME TO command for operators?
Chris
Attachments:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
Then I made it so that pg_dump will output an OWNER TO statement after
every object creation.
Perhaps better to put these out towards the end of the dump, not right
after the creation of the object? Or is that what you're doing?
I would envision the safest procedure as creating all objects, loading
all data, etc, then all ALTER OWNERs, then all GRANT/REVOKEs.
* Do we need the set session auth for COPY commands still?
Not if you still own the table while loading into it (see above point).
However, this all assumes a complete dump/restore. Consider data-only
restores. Consider partial restores using pg_restore's options for
that. What happens then? It'd likely be appropriate to issue set
session auth during scenarios involving pre-existing objects.
* Is there any reason there is no RENAME TO command for operators?
Lack of round tuits, no doubt.
regards, tom lane
Perhaps better to put these out towards the end of the dump, not right
after the creation of the object? Or is that what you're doing?
I just inserted the ALTER OWNER statement between the CREATE and the
GRANTs. Why do you want them at the end of the dump?
I would envision the safest procedure as creating all objects, loading
all data, etc, then all ALTER OWNERs, then all GRANT/REVOKEs.
I don't yet understand your reasoning for wanting this all at the end...
Not if you still own the table while loading into it (see above point).
Can we not load as superuser?
However, this all assumes a complete dump/restore. Consider data-only
restores. Consider partial restores using pg_restore's options for
that. What happens then? It'd likely be appropriate to issue set
session auth during scenarios involving pre-existing objects.
OK, i will test all those situations... What scenarios did you have in
mind?
Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
I just inserted the ALTER OWNER statement between the CREATE and the
GRANTs. Why do you want them at the end of the dump?
So that the initial owner is still owner when he does COPY, ALTER TABLE
ADD PRIMARY KEY, etc etc. Else you're gonna have problems.
The regression database is next to useless as a testbed for this,
btw, since all the objects in it are owned by the superuser anyway.
regards, tom lane
So that the initial owner is still owner when he does COPY, ALTER TABLE
ADD PRIMARY KEY, etc etc. Else you're gonna have problems.
I was thinking of doing all COPY and ALTER as superuser as well...
Or are you trying to make it work when run as non-super? Which is won't
since ALTER OWNER will require superuser.
ie. the entire script always runs as a single user, most usefully a
superuser.
Chris
Christopher Kings-Lynne wrote:
* Is there any reason there is no RENAME TO command for operators?
That might change the precedence of the operator and get you in a big
mess with stored expressions everywhere.
Christopher Kings-Lynne wrote:
Then I made it so that pg_dump will output an OWNER TO statement
after every object creation.
I'd prefer it if OWNER TO were only added if it is actually necessary
(or there be some option to turn it off). I don't want to edit the
entire dump file if I want to restore the database into another SQL
database.
That might change the precedence of the operator and get you in a big
mess with stored expressions everywhere.
What if you could only do it on non-system operators?
Chris
I'd prefer it if OWNER TO were only added if it is actually necessary
(or there be some option to turn it off). I don't want to edit the
entire dump file if I want to restore the database into another SQL
database.
There is the existing --no-owner option, which this patch respects, same
as old pg_dump.
It's not possible to dump it if necessary, as it's not possible at
restore time to know the user they are restoring as I guess.
The old pg_dump would output a session auth, and then wouldn't bother
changing it until necessary. However, with ALTER OWNER, I have to dump
it for every object, even if it's the same user.
I guess I could optimise so that if the owner matches the superuser
specified with the -S option, I wouldn't bother dumping the ALTER?
Chris
Peter Eisentraut <peter_e@gmx.net> writes:
Christopher Kings-Lynne wrote:
* Is there any reason there is no RENAME TO command for operators?
That might change the precedence of the operator
... true ...
and get you in a big mess with stored expressions everywhere.
Not with respect to compiled expressions. It could conceivably break
stored function source texts and application-generated queries, but
those are broken a fortiori by the new operator name.
So I don't think this objection has a lot of weight.
regards, tom lane
Christopher Kings-Lynne wrote:
The old pg_dump would output a session auth, and then wouldn't bother
changing it until necessary. However, with ALTER OWNER, I have to
dump it for every object, even if it's the same user.
Well, the advantage of SET SESSION AUTHORIZATION is that it is SQL
compliant, whereas ALTER OWNER is not. So I'm in favor of changing
nothing. The examples you listed in you original mail, where the
privilege to do something was later dropped so the original state is
not reproducible, are to me examples that the privilege system is
flawed. You could use ALTER OWNER in those cases only, because those
states are not SQL compliant anyway.
Tom Lane wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
Christopher Kings-Lynne wrote:
* Is there any reason there is no RENAME TO command for operators?
That might change the precedence of the operator
... true ...
and get you in a big mess with stored expressions everywhere.
Not with respect to compiled expressions. It could conceivably break
stored function source texts and application-generated queries, but
those are broken a fortiori by the new operator name.So I don't think this objection has a lot of weight.
IIRC, it was the objection that you put forth when I last attempted to
do it... The question is perhaps not so much whether we can get away
with it, it's whether the behavior is reasonable and consistent for
users that don't know implementation details.
Peter Eisentraut <peter_e@gmx.net> writes:
Tom Lane wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
That might change the precedence of the operator
So I don't think this objection has a lot of weight.
IIRC, it was the objection that you put forth when I last attempted to
do it...
Was it? I vaguely remember objecting to something on the basis of
possible precedence changes, but I don't recall it being RENAME
OPERATOR.
The question is perhaps not so much whether we can get away
with it, it's whether the behavior is reasonable and consistent for
users that don't know implementation details.
Agreed. Probably the main point here is the inconsistency in behavior
for stored rules and constraint/default expressions (which would track
the operator rename and would stay parenthesized correctly), versus
stored function source code (which would not). I don't think it would
surprise anyone that the query texts embedded in their app code don't
magically update ;-) but for the stuff that's "all stored in the
database" they might expect consistency.
On the other hand, the same inconsistency already exists for table and
column names, and I've not heard great squawks about it. So I withdraw
any previous objection I might have made to RENAME OPERATOR. It's not
obvious that it's more dangerous than any other rename.
regards, tom lane
Well, the advantage of SET SESSION AUTHORIZATION is that it is SQL
compliant, whereas ALTER OWNER is not. So I'm in favor of changing
nothing.
That, however is a highly theoretical, and quite non-practical
"solution". It leaves many of the world's postgresql database
non-upgradable and "fixing" postgres so that revoking someone's create
privilege dropped all their tables is _madness_. You can't but agree
that the SQL spec is totally broken in that respect. They've broken the
underlying orthogonality of their permissions system.
I think Tom even may have mentioned that the SQL rules about that sort
of thing only seem to apply to domains or something anyway...
I mean, if I (as a PostgreSQL developer) cannot upgrade my _own_
database then how does anyone else have a chance?
Chris
Peter Eisentraut <peter_e@gmx.net> writes:
Well, the advantage of SET SESSION AUTHORIZATION is that it is SQL
compliant, whereas ALTER OWNER is not. So I'm in favor of changing
nothing.
That's a fair point, but you have to admit that it's a bit abstract
while Chris has a real problem he needs to solve. Our dumps are awfully
low on the SQL-compliance scale anyway :-(
The examples you listed in you original mail, where the
privilege to do something was later dropped so the original state is
not reproducible, are to me examples that the privilege system is
flawed.
Sure, but we're not fixing the privilege system this time round (unless
you have work in progress you haven't mentioned ;-)). In any case this
answer is no help for dumping existing databases with "illegal"
configurations, and newer pg_dumps will still be expected to cope with
those.
You could use ALTER OWNER in those cases only, because those
states are not SQL compliant anyway.
Is it really possible for pg_dump to detect such cases and decide which
method to use? I'd be in favor of this if it were practical to do,
but it sounds suspiciously close to AI-complete ... especially when
considering scenarios involving pg_restore into an existing database ...
This brings up a question for Chris, which is whether he's implemented
this in a way that forces the decision at pg_dump time, or whether
it is made during pg_restore. I would definitely agree that we need
to postpone the choice of which to do till pg_restore. In other words,
a dump archive should only show object ownerships and not prejudge
how those ownerships will get set during the restore session.
regards, tom lane
That's a fair point, but you have to admit that it's a bit abstract
while Chris has a real problem he needs to solve. Our dumps are awfully
low on the SQL-compliance scale anyway :-(
We could keep around an option for dumping the auth statements instead
of alter statements perhaps.
Sure, but we're not fixing the privilege system this time round (unless
you have work in progress you haven't mentioned ;-)).
I don't think any fix to privs system which creates a situation where
you cannot drop CREATE privilege from someone who owns a table is jsut
silly.
This brings up a question for Chris, which is whether he's implemented
this in a way that forces the decision at pg_dump time, or whether
it is made during pg_restore. I would definitely agree that we need
to postpone the choice of which to do till pg_restore. In other words,
a dump archive should only show object ownerships and not prejudge
how those ownerships will get set during the restore session.
I've implented it exactly like comments are implemented. I just created
a dumpOwner() function that adds an archive entry to the current object.
It appears in the pg_dump.c basically wherever a dumpComment()
appears, but always before the dumpAcls() if there is one.
What we need to do is decide on the exact semantics of how this is going
to work, and then I can make all the (hopefully small) changes required
to make it work.
OK, are these the requirements? Please comment.
* I fix ALTER OWNER to allow it to work if you are NOT a superuser, but
ARE the existing owner. This makes it work just like set session auth
and means that if your dump includes only stuff you own, it will still work.
* Text mode dumps
- I issue alter owner after every object creation, suppressing ALL
session auths, including COPY
- I keep a switch to disable alter owner and dump set session instead.
Is this really necessary?
- The -S option only affects enabling and disabling triggers and i
don't have to worry about it
- The only difference is data-only dumps - we still need set session
auth? Actually, no - read the next point.
- How does the above point affect full dumps that include schema and
data? In my proposal, the copy commands will run as the user running
the script, not the table owner anymore. Presumably, the user running
the script is a superuser. Given that it is possible for a table owner
to revoke their own INSERT privilege on their table, the existing
behaviour is broken anyway.
- The --no-owner option means no alter owner or session auth
statements are dumped.
- pg_dump currently in the case when the owner of a table no longer
exists, dumps SET SESSION AUTHORIZATION DEFAULT. I will simply omit the
ALTER OWNER command.
* Custom format dumps
- OK, I admit I have little experience with this format.
- The alter owner objects will be stored as toc entries just like
comment on objects.
- They should pop back out of the archive when creating a text dump
from a binary one, identical to the text format.
- With respect to Tom's question about restore-time option - how is it
different to now?? A that moment, we have the pg_restore -O option to
not restore the session auth commands - what needs to change? I just
won't output the ALTER OWNER commands so everything will be owned by
whoever runs pg_restore.
Does that seem like the way to go?
Chris
- How does the above point affect full dumps that include schema and
data? In my proposal, the copy commands will run as the user running
the script, not the table owner anymore. Presumably, the user running
the script is a superuser. Given that it is possible for a table owner
to revoke their own INSERT privilege on their table, the existing
behaviour is broken anyway.
Yes, confirmed. Current pg_dump is quite broken in the situation where
a table owner revokes their own INSERT privilege. It attempts to run
COPY as the owner of the table. Only a superuser is guaranteed to be
able to always insert. We should fix that.
Chris
There is one other consideration, and that is that current pg_dump likes
to set session_auth to user of object before outputting drop command,
when '-c' is specificed.
I propose that we eliminate that set session_auth as well. If the user
running the script is the owner of that object or a superuser, then they
will be able to drop it. If they are neither, then the drop will fail,
but the session_authorization would have failed anyway!
Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
This brings up a question for Chris, which is whether he's implemented
this in a way that forces the decision at pg_dump time, or whether
it is made during pg_restore.
I've implented it exactly like comments are implemented. I just created
a dumpOwner() function that adds an archive entry to the current object.
It appears in the pg_dump.c basically wherever a dumpComment()
appears, but always before the dumpAcls() if there is one.
I think this is wrong, primarily because it's gonna be seriously
incompatible with existing dump files. The existing technique is
that each TOC entry says who owns the object. You should use that
information and not have to rely on new additions to the file format.
* I fix ALTER OWNER to allow it to work if you are NOT a superuser, but
ARE the existing owner.
No, you don't. That allows non-superusers to give away object
ownership, which is well-established as a security hole; Unix
filesystems stopped doing it years ago.
- How does the above point affect full dumps that include schema and
data? In my proposal, the copy commands will run as the user running
the script, not the table owner anymore. Presumably, the user running
the script is a superuser. Given that it is possible for a table owner
to revoke their own INSERT privilege on their table, the existing
behaviour is broken anyway.
This is why GRANT/REVOKE has to be postponed to the end. I think it
would be a lot simpler and more reliable if you also postponed ALTER
OWNER.
* Custom format dumps
- OK, I admit I have little experience with this format.
Then you have fundamentally failed to grok pg_dump, and you should
rethink everything you've done to date. The way things work is that
EVERYTHING effectively goes through a custom dump. pg_dump in text
mode is really pg_dump followed by pg_restore with the intermediate
TOC just kept in memory temporarily. Therefore, any time you have done
something that you don't know how to convert into pg_restore behavior,
it's because you were hacking the wrong place. Everything you need to
know about an object *must* go through the TOC representation and then
be converted to text at the restore side.
- With respect to Tom's question about restore-time option - how is it
different to now?? A that moment, we have the pg_restore -O option to
not restore the session auth commands - what needs to change? I just
won't output the ALTER OWNER commands so everything will be owned by
whoever runs pg_restore.
I think there needs to be a restore-side switch that chooses whether
to emit ALTER OWNER or SET SESSION AUTH commands. This is probably
just for pro-forma SQL compliance, unless Peter has some brilliant
insight about how to avoid ALTER OWNER.
regards, tom lane
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
There is one other consideration, and that is that current pg_dump likes
to set session_auth to user of object before outputting drop command,
when '-c' is specificed.
I propose that we eliminate that set session_auth as well. If the user
running the script is the owner of that object or a superuser, then they
will be able to drop it. If they are neither, then the drop will fail,
but the session_authorization would have failed anyway!
Seems reasonable. The -c option is pretty badly thought out anyway :-(
regards, tom lane
I think this is wrong, primarily because it's gonna be seriously
incompatible with existing dump files. The existing technique is
that each TOC entry says who owns the object. You should use that
information and not have to rely on new additions to the file format.
Hrm. OK, i might be able to do that, but constructing the ALTER OWNER
commands will be difficult I think. Each TOC entry seems to have the
'OPERATOR (int4, none)' or 'TABLE blah' string in it from memory, so I
assume I can pull that out. I had failed to consider restoring from
existing dump files actually.
* I fix ALTER OWNER to allow it to work if you are NOT a superuser, but
ARE the existing owner.No, you don't. That allows non-superusers to give away object
ownership, which is well-established as a security hole; Unix
filesystems stopped doing it years ago.
I worded that badly. I meant "allow a user to change the owner of
something to what it already is". ie. Just make the no-op allowed by
everyone. session_auth already does this. This means I can make text
mode dumps that have ALTER OWNER in them and then even if you are not a
superuser, so long as you own everything, you're ok.
- How does the above point affect full dumps that include schema and
data? In my proposal, the copy commands will run as the user running
the script, not the table owner anymore. Presumably, the user running
the script is a superuser. Given that it is possible for a table owner
to revoke their own INSERT privilege on their table, the existing
behaviour is broken anyway.This is why GRANT/REVOKE has to be postponed to the end. I think it
would be a lot simpler and more reliable if you also postponed ALTER
OWNER.
Or we just always run the COPYs as the person executing the script, ie.
remove session_auth from the COPY commands.
Then you have fundamentally failed to grok pg_dump, and you should
rethink everything you've done to date. The way things work is that
EVERYTHING effectively goes through a custom dump. pg_dump in text
mode is really pg_dump followed by pg_restore with the intermediate
TOC just kept in memory temporarily. Therefore, any time you have done
something that you don't know how to convert into pg_restore behavior,
it's because you were hacking the wrong place. Everything you need to
know about an object *must* go through the TOC representation and then
be converted to text at the restore side.
I'm well aware of how it works - but compared to text format, i don't
have as much experience. I have done a fair bit of pg_dump hacking in
my time... All my changes work perfectly with pg_restore and the binary
dump format. I can pg_dump my production db using custom and plain text
and there is no diff between the plain text and the plain text extracted
from the binary dump. I can also reload that dump and dump it again,
and keep cycling it with no diff - without moving the grants/owners to
the end.
I will have to spend some time investigating how to collect up the
grants and stuff and move them to the end, if you still feel it is
necessary.
- With respect to Tom's question about restore-time option - how is it
different to now?? A that moment, we have the pg_restore -O option to
not restore the session auth commands - what needs to change? I just
won't output the ALTER OWNER commands so everything will be owned by
whoever runs pg_restore.I think there needs to be a restore-side switch that chooses whether
to emit ALTER OWNER or SET SESSION AUTH commands. This is probably
just for pro-forma SQL compliance, unless Peter has some brilliant
insight about how to avoid ALTER OWNER.
Yes, well if I change it how you suggested in your first paragraph
(which has to happen for backwards compatibility), then this wouldn't
seem too hard.
Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
No, you don't. That allows non-superusers to give away object
ownership, which is well-established as a security hole; Unix
filesystems stopped doing it years ago.
I worded that badly. I meant "allow a user to change the owner of
something to what it already is". ie. Just make the no-op allowed by
everyone. session_auth already does this.
Ah. Okay, no objection to that. (In fact I believe we put in the
special case for session_auth for exactly the same reason.)
regards, tom lane
I worded that badly. I meant "allow a user to change the owner of
something to what it already is". ie. Just make the no-op allowed by
everyone. session_auth already does this.Ah. Okay, no objection to that. (In fact I believe we put in the
special case for session_auth for exactly the same reason.)
Actually, do I make it that anyone can do a no-op user change, or can
only the user who is the existing owner do the no-op? It's a very tiny
different and probably won't make much difference but perhaps it's
better to make it a bit tighter check? What do you think?
Chris
- How does the above point affect full dumps that include schema and
data? In my proposal, the copy commands will run as the user running
the script, not the table owner anymore. Presumably, the user running
the script is a superuser. Given that it is possible for a table owner
to revoke their own INSERT privilege on their table, the existing
behaviour is broken anyway.This is why GRANT/REVOKE has to be postponed to the end. I think it
would be a lot simpler and more reliable if you also postponed ALTER
OWNER.
Ok, I have thought about this a bit and I have decided that you are
right in that grant revoke MUST be deferred to the end of the file.
The reason is that there are two conditions under which we expect a dump
restore to work. First is if you are restoring as a superuser second is
if you are restoring as a user who owns ALL the objects in the file.
Restoring as superuser will work so long as we use alter owner instead
of changing the current user.
The second is more tricky due to the fact that an object owner can
revoke their own privileges from their objects. If a user does this, it
cannot be guaranteed that they will be able to restore unless all grants
are done last and all objects are initially created as that user.
Consider revoking your own REFERENCES privilege, as an example.
Hence, I will have to make it that all existing dumping is done, then
all the ownership changes will be done, then all the grants will be done.
Does that seem reasonable?
Chris
I think this is wrong, primarily because it's gonna be seriously
incompatible with existing dump files. The existing technique is
that each TOC entry says who owns the object. You should use that
information and not have to rely on new additions to the file format.
This is why GRANT/REVOKE has to be postponed to the end. I think it
would be a lot simpler and more reliable if you also postponed ALTER
OWNER.
OK, implementing this is nasty. How do I collect up all the ACLs from
EXISTING custom archives and move them to the end?? This is hard
because ACLs are just dependents on their parent object and cannot be
sorted on their own to the end of the dump.
Since the dumping process outputs to stdout as it goes along, I'd have
to create some big in-memory string of all acls and owners collected so
far. That seems bad.
The alternative is to scan the entire archive twice. On the second scan
I would only output owner and acl commands.
Another option is to simply not bother fixing old custom dumps. They
could just still restore exactly how they would have without any changes
from me. I would add new TOC types to the 7.5 pg_dump that could be
sorted to the end...
What do I do?
Chris
Any change someone who knows (or who can declare that we not fix
existing dumps) comment on this?
Chris
Christopher Kings-Lynne wrote:
Show quoted text
I think this is wrong, primarily because it's gonna be seriously
incompatible with existing dump files. The existing technique is
that each TOC entry says who owns the object. You should use that
information and not have to rely on new additions to the file format.This is why GRANT/REVOKE has to be postponed to the end. I think it
would be a lot simpler and more reliable if you also postponed ALTER
OWNER.OK, implementing this is nasty. How do I collect up all the ACLs from
EXISTING custom archives and move them to the end?? This is hard
because ACLs are just dependents on their parent object and cannot be
sorted on their own to the end of the dump.Since the dumping process outputs to stdout as it goes along, I'd have
to create some big in-memory string of all acls and owners collected so
far. That seems bad.The alternative is to scan the entire archive twice. On the second scan
I would only output owner and acl commands.Another option is to simply not bother fixing old custom dumps. They
could just still restore exactly how they would have without any changes
from me. I would add new TOC types to the 7.5 pg_dump that could be
sorted to the end...What do I do?
Chris
---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?