Обсуждение: What's the point of allow_system_table_mods?
Hi, I'm not quite clear what the goal of allow_system_table_mods is. Obviously, it's extremely dangerous to target catalogs with DDL. But at the same time we allow DML to catalog tables without any sort of restriction. I also don't understand what's achieved by having allow_system_table_mods be PGC_POSTMASTER. If anything it seems to make it more likely to resort to a) leaving it enabled all the time b) use DML to modify catalogs. Wouldn't it be more sensible to disallow all catalog modifications unless allow_system_table_mods was enabled, and make allow_system_table_mods PGC_SUSET and GUC_DISALLOW_IN_FILE? Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> I'm not quite clear what the goal of allow_system_table_mods
> is. Obviously, it's extremely dangerous to target catalogs with DDL. But
> at the same time we allow DML to catalog tables without any sort of
> restriction.
The last is not true, see pg_class_aclmask().
> I also don't understand what's achieved by having
> allow_system_table_mods be PGC_POSTMASTER.
True. Possibly there was some confusion with ignore_system_indexes,
which probably *should* be PGC_POSTMASTER: if you think the system
indexes are corrupt then they're corrupt for everybody.
> Wouldn't it be more sensible to disallow all catalog modifications
> unless allow_system_table_mods was enabled, and make
> allow_system_table_mods PGC_SUSET and GUC_DISALLOW_IN_FILE?
I'm on board with the second part of that but not the first.
DDL on the system catalogs is significantly more dangerous
than DML, so I think that having an extra layer of protection
for it is a good idea.
regards, tom lane
Hi, On 2019-05-09 12:22:39 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > I'm not quite clear what the goal of allow_system_table_mods > > is. Obviously, it's extremely dangerous to target catalogs with DDL. But > > at the same time we allow DML to catalog tables without any sort of > > restriction. > > The last is not true, see pg_class_aclmask(). You mean because it's restricted to superusers? > > I also don't understand what's achieved by having > > allow_system_table_mods be PGC_POSTMASTER. > > True. Possibly there was some confusion with ignore_system_indexes, > which probably *should* be PGC_POSTMASTER: if you think the system > indexes are corrupt then they're corrupt for everybody. Hm, but it's pretty useful to be able to verify if system index corruption is to blame, by enabling ignore_system_indexes in one session. Don't really see us gaining anything by forcing it to be done system-wide. > > Wouldn't it be more sensible to disallow all catalog modifications > > unless allow_system_table_mods was enabled, and make > > allow_system_table_mods PGC_SUSET and GUC_DISALLOW_IN_FILE? > > I'm on board with the second part of that but not the first. > DDL on the system catalogs is significantly more dangerous > than DML, so I think that having an extra layer of protection > for it is a good idea. Why is it so much more dangerous? I've seen plenty of corrupted clusters due to people doing DML against the catalogs. I'm OK with adding separate GUCs for both, if we want to do that, but I do think we shouldn't allow updating the catalogs wthout having having the superuser explicitly opt into that. We need superusers permissions for a lot of pretty routine tasks (say creating an extension with C functions) - so that also being the permission to do dangerous things like UPDATE to pg_class etc, isn't great. Greetings, Andres Freund
>>>>> "Andres" == Andres Freund <andres@anarazel.de> writes: Andres> Why is it so much more dangerous? I've seen plenty of corrupted Andres> clusters due to people doing DML against the catalogs. I'm OK Andres> with adding separate GUCs for both, if we want to do that, but Andres> I do think we shouldn't allow updating the catalogs wthout Andres> having having the superuser explicitly opt into that. Be aware that a nonzero number of extensions (postgis especially) do catalog DML in their install or update scripts. While you might well think they shouldn't do that, in practice there is usually no viable alternative. -- Andrew (irc:RhodiumToad)
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Andres" == Andres Freund <andres@anarazel.de> writes:
> Andres> Why is it so much more dangerous? I've seen plenty of corrupted
> Andres> clusters due to people doing DML against the catalogs. I'm OK
> Andres> with adding separate GUCs for both, if we want to do that, but
> Andres> I do think we shouldn't allow updating the catalogs wthout
> Andres> having having the superuser explicitly opt into that.
> Be aware that a nonzero number of extensions (postgis especially) do
> catalog DML in their install or update scripts.
I believe we've done that in some contrib update scripts, as well.
> While you might well
> think they shouldn't do that, in practice there is usually no viable
> alternative.
In principle, if the thing is SUSET, we could have such extension scripts
set it temporarily. But it would be a compatibility hazard -- a script
with such a SET command in it would fail in older branches.
What exactly is the motivation for changing this now, after 20 years?
regards, tom lane
Hi, On 2019-05-10 19:51:10 +0100, Andrew Gierth wrote: > >>>>> "Andres" == Andres Freund <andres@anarazel.de> writes: > > Andres> Why is it so much more dangerous? I've seen plenty of corrupted > Andres> clusters due to people doing DML against the catalogs. I'm OK > Andres> with adding separate GUCs for both, if we want to do that, but > Andres> I do think we shouldn't allow updating the catalogs wthout > Andres> having having the superuser explicitly opt into that. > > Be aware that a nonzero number of extensions (postgis especially) do > catalog DML in their install or update scripts. While you might well > think they shouldn't do that, in practice there is usually no viable > alternative. Sure, but if it's a SUSET GUC that'd not be a huge problem, would it? They'd need to locally set it, which, sure. But it'd also be a good way to signal such things to readers. Greetings, Andres Freund
Hi, On 2019-05-10 15:00:18 -0400, Tom Lane wrote: > What exactly is the motivation for changing this now, after 20 years? That I've seen enough corruption and other hard to investigate issues related to manual catalog modifications to make me complain. Note that other have complained about this before, too. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2019-05-10 15:00:18 -0400, Tom Lane wrote:
>> What exactly is the motivation for changing this now, after 20 years?
> That I've seen enough corruption and other hard to investigate issues
> related to manual catalog modifications to make me complain. Note that
> other have complained about this before, too.
So, if the problem is that cowboy DBAs are making ill-advised manual
changes, how is a SUSET GUC going to stop them from doing that?
They'll just turn it on and make the same ill-advised change, especially
after they see us and other people doing exactly that in extensions.
If you're arguing that the changes were accidental, it seems like the real
answer to that is "stop using superuser unnecessarily". I don't think
that adding training wheels to superuser is really a great idea in the
long run. I remember wars back in the last century about whether rm
should be hacked to disallow "rm -rf /" even to superusers. The eventual
consensus was "no", and this seems about the same.
regards, tom lane
Hi, On 2019-05-10 15:48:49 -0400, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2019-05-10 15:00:18 -0400, Tom Lane wrote: > >> What exactly is the motivation for changing this now, after 20 years? > > > That I've seen enough corruption and other hard to investigate issues > > related to manual catalog modifications to make me complain. Note that > > other have complained about this before, too. > > So, if the problem is that cowboy DBAs are making ill-advised manual > changes, how is a SUSET GUC going to stop them from doing that? > They'll just turn it on and make the same ill-advised change, especially > after they see us and other people doing exactly that in extensions. Having to figure out what that GUC is called, looking in the documentation to do so, seing that there's a large WARNING about it does reduce risk. > If you're arguing that the changes were accidental, it seems like the real > answer to that is "stop using superuser unnecessarily". I don't think > that adding training wheels to superuser is really a great idea in the > long run. Well, we require superuser for a lot of operations with a vastly lower risk. Like CREATE EXTENSION postgres_fdw etc.. So that's not really a feasible answer. Nor do we provide a setup, where non-superuser admin roles exist by default. > I remember wars back in the last century about whether rm > should be hacked to disallow "rm -rf /" even to superusers. The eventual > consensus was "no", and this seems about the same. Note that rm has prohibited this for a *long* time now (and not just linux, solaris too). And you've argued for disallowing most DDL against catalogs... I don't think it's always obvious to users how dangerous such operations can be. Greetings, Andres Freund