Date: Fri, 21 Jun 2019 12:58:56 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: allow_system_table_mods stuff
Message-ID: <20190621165855.GP2480@tamriel.snowman.net>
References: <8b00ea5e-28a7-88ba-e848-21528b632354@2ndquadrant.com>
 <CA+TgmoZfJ1Y9Zrcit9zQVTSaPugCAo=UXC8b1URJcFsqNVoxqA@mail.gmail.com>
 <20274.1561134523@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="WgMw66UcpYoL/xvb"
Content-Disposition: inline
In-Reply-To: <20274.1561134523@sss.pgh.pa.us>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--WgMw66UcpYoL/xvb
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > I kinda feel like we should prohibit DML on system catalogs, even by
> > superusers, unless you press the big red button that says "I am
> > definitely sure that I know what I'm doing."
>=20
> Keep in mind that DML-on-system-catalogs is unfortunately a really
> standard hack in extension upgrade scripts.  (If memory serves,
> some of our contrib scripts do that, and we've certainly told third
> parties that it's the only way out of some box or other.)  I don't
> think we can just shut it off.  What you seem to be proposing is to
> allow it only after
>=20
> SET allow_system_table_mods =3D on;

That's basically what my feeling is, yes.

> which would be all right except that an extension script containing
> such a command will fail outright in existing releases.  I think we
> need to be friendlier than that to extension authors who are, for the
> most part, trying to work around some deficiency of ours not theirs.

As with other cases where someone needs to do DML against the catalog
for some reason or another- we should fix that.  If there's example
cases, great!  Let's look at those and come up with a proper solution.

Other options include- letting an extension set that GUC (seems likely
that any case where this is needed is a case where the extension is
installing C functions and therefore is being run by a superuser
anyway...), or implicitly setting that GUC when we're running an
extension's script (urrggghhhh...  I don't care for that one bit, but I
like it better than letting any superuser who wishes UPDATE random bits
in the catalog).

> I'm not saying that DML-off-by-default is a bad goal to work toward;
> I'm just saying "mind the collateral damage".

Sure, makes sense.

> > A related issue is that alter_system_table_mods prohibits both stuff
> > that's probably not going to cause any big problem and stuff that is
> > almost guaranteed to make the system permanently unusable - e.g. you
> > could 'SET STORAGE' on a system catalog column, which is really pretty
> > innocuous, or you could change the oid column of pg_database to a
> > varlena type, which is guaranteed to destroy the universe.  Here
> > again, maybe some operations should be more protected than others, or
> > maybe the relatively safe things just shouldn't be subject to
> > allow_system_table_mods at all.
>=20
> Meh.  It doesn't really seem to me that distinguishing these cases
> is a productive use of code space or maintenance effort.  Superusers
> are assumed to know what they're doing, and most especially so if
> they've hit the big red button.

The direction I took the above was that we should actually be thinking
about if there are acceptable cases to be running DDL against the
catalog and, if so, specifically allow those.  I'm not convinced at the
moment that any such exist and therefore I'd rather have it denied
(unless you push the big red button) and then tell people to show us
their use case and then we can decide if it's an 'ok' thing to allow, or
what.

I'd really like to stop the cases like stackoverflow articles that
describe how to "remove" an enum value by simply modifying the catalog,
or at least make them have to add a "well, push this big red button
first that the PG people tell you never to push, and then.." to the
start.

Thanks,

Stephen

--WgMw66UcpYoL/xvb
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJdDQzPAAoJEO1sijiDR2RVEiUQALm3GlDJ19nGR1KcLLzs5pvo
4aimDnwwJpU4osBuLmhH3xHzN79vi891Ohxsc3X9gPaWkJHI8xrt/X/bHDoASuj9
dBwo3csg6fnLb+jPvxllZUkvtDM8XRLQZ22Ybie7WP6bfUOou4Jlx9M63qXnvjKM
NbM2tl4HeQT3fznO5bzz3u5Y5gzIMQ7nBt/M9XohccseOSAb8r96sWwSkq33VcJW
6qIgHy4BPmFqDMifR42LtUbXE7BIOWWfzo3GJ/yMMh/Nr1XaT1CVgNbeUPaHxSPI
Dw+SY+OWMlH5KtKNXC+g4SQ0S+aKeBBVwja9BrvbeIZPB5fVPw3xr9vV4oIR+gGA
swTZu5RURsy9omB3r7XvgvVeAvbh9VXVZkN4HxpYPRvyu+Xh2ScE3kKhXmVPLRHZ
uZPvA1o+Zg9AiTs1jrNWk6QKP0cbtLl4+pJEDHuNFUbu9milFt9SvRR+RwaUMV0Z
m5lHe4nsUQS2dIh9ZyDZl5TAGvolbvLSYNUF2vpEYMpgsYnspoeigb7oWFHH6qm0
of+DkcassM0FOvE+FYAU5AHY/Y0B28cS/ikoZsy/1cGyfz/kDpGQ5c6LKKeqR989
1mcwP2BLL2a5FUF1m12OTgH3rsMNlLWa4bPxtYXlythJ379Gjsy9ylsUlSxzkiab
05/oLGwIvJnqECrHsWrv
=Ph4m
-----END PGP SIGNATURE-----

--WgMw66UcpYoL/xvb--