"pg_xxx" role name restriction not applied to bootstrap superuser?
I noticed that opossum's latest buildfarm run failed, evidently because
it was set up with the user running the buildfarm named "pg_buildfarmer":
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=opossum&dt=2016-05-03%2018%3A43%3A31
That caused the bootstrap superuser's name to be "pg_buildfarmer".
initdb didn't complain about this, but it was impossible to log in:
016-05-04 02:29:00.820 BST [5729505c.26:1] LOG: connection received: host=[local]
2016-05-04 02:29:00.927 BST [5729505c.26:2] LOG: connection authorized: user=pg_buildfarmer database=postgres
2016-05-04 02:29:00.932 BST [5729505c.26:3] FATAL: invalid value for parameter "session_authorization": "pg_buildfarmer"
2016-05-04 02:29:02.260 BST [5729505e.4396:1] LOG: connection received: host=[local]
2016-05-04 02:29:02.328 BST [5729505e.4396:2] LOG: connection authorized: user=pg_buildfarmer database=postgres
2016-05-04 02:29:02.333 BST [5729505e.4396:3] FATAL: invalid value for parameter "session_authorization": "pg_buildfarmer"
2016-05-04 02:29:03.662 BST [5729505f.57cd:1] LOG: connection received: host=[local]
2016-05-04 02:29:03.731 BST [5729505f.57cd:2] LOG: connection authorized: user=pg_buildfarmer database=postgres
2016-05-04 02:29:03.735 BST [5729505f.57cd:3] FATAL: invalid value for parameter "session_authorization": "pg_buildfarmer"
I tried to duplicate this failure just now, and could not. Evidently,
the failures opossum shows above come from the "cannot set role to pg_xxx"
checks you had in check_session_authorization, which are gone as of commit
a89505fd2. So in principle opossum should succeed if it were to try
again today with the same environment.
So this seems like another reason why removing those checks was an
improvement, but I'm left with a policy question: should initdb disallow
bootstrap superuser names like "pg_xxx"? This doesn't seem quite
open-and-shut. On the one hand, if we leave it as-is, then people might
be blindsided by future additions of built-in roles. On the other,
if we forbid the case, it seems noticeably more likely that we'll break
existing setups, because "pg_something" doesn't seem like a terribly
unlikely choice for the name of the Postgres OS user. (Certainly
opossum's owner would have to fix it, so that's one example out of a
not very large sample space of buildfarm users...) Allowing a potential
conflict for the bootstrap superuser is a much narrower conflict risk
than any-old-user, so maybe it's okay to leave it as is.
Also, the failure mode if you do get an actual, rather than hypothetical,
conflict against a built-in role name isn't all that nice:
$ initdb -U pg_signal_backend
...
running bootstrap script ... FATAL: could not create unique index "pg_authid_rolname_index"
DETAIL: Key (rolname)=(pg_signal_backend) is duplicated.
...
While it's not hard to interpret this if you already know that
"pg_signal_backend" is a reserved role name, an explicit failure message
saying that the bootstrap superuser name can't begin with "pg_" would be
more user-friendly. So that's a point in favor of having initdb reject
the case.
On the whole I lean to adding a restriction, but only weakly.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
So this seems like another reason why removing those checks was an
improvement, but I'm left with a policy question: should initdb disallow
bootstrap superuser names like "pg_xxx"? This doesn't seem quite
open-and-shut. On the one hand, if we leave it as-is, then people might
be blindsided by future additions of built-in roles. On the other,
if we forbid the case, it seems noticeably more likely that we'll break
existing setups, because "pg_something" doesn't seem like a terribly
unlikely choice for the name of the Postgres OS user. (Certainly
opossum's owner would have to fix it, so that's one example out of a
not very large sample space of buildfarm users...) Allowing a potential
conflict for the bootstrap superuser is a much narrower conflict risk
than any-old-user, so maybe it's okay to leave it as is.
On the whole, I'd vote to treat the bootstrap user as a normal role and
therefore have the same restriction in place for that user also. As was
mentioned previously, it's already impossible to create schemas which
start with 'pg_', so you couldn't have a 'pg_buildfarmer' schema. I
realize that, for the buildfarm, that's not an issue, but that's a bit
of a special case.
Also, the failure mode if you do get an actual, rather than hypothetical,
conflict against a built-in role name isn't all that nice:$ initdb -U pg_signal_backend
...
running bootstrap script ... FATAL: could not create unique index "pg_authid_rolname_index"
DETAIL: Key (rolname)=(pg_signal_backend) is duplicated.
...While it's not hard to interpret this if you already know that
"pg_signal_backend" is a reserved role name, an explicit failure message
saying that the bootstrap superuser name can't begin with "pg_" would be
more user-friendly. So that's a point in favor of having initdb reject
the case.On the whole I lean to adding a restriction, but only weakly.
Agreed.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
... but I'm left with a policy question: should initdb disallow
bootstrap superuser names like "pg_xxx"?
On the whole, I'd vote to treat the bootstrap user as a normal role and
therefore have the same restriction in place for that user also.
If we're going to enforce such a restriction, I think it would be
a good thing for it to be in place in beta1.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Saturday, May 7, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.net <javascript:;>> writes:
* Tom Lane (tgl@sss.pgh.pa.us <javascript:;>) wrote:
... but I'm left with a policy question: should initdb disallow
bootstrap superuser names like "pg_xxx"?On the whole, I'd vote to treat the bootstrap user as a normal role and
therefore have the same restriction in place for that user also.If we're going to enforce such a restriction, I think it would be
a good thing for it to be in place in beta1.
I don't fathom a good reason to treat only the bootstrap user differently.
I'd second guess prohibiting pg_ generally instead of only the specific
system roles in use in a given release. Having beta1 go out with full
restrictions will at least maximize the chance of getting complaints and
insight into how prevalent the prefix is in the wild.
David J.
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
... but I'm left with a policy question: should initdb disallow
bootstrap superuser names like "pg_xxx"?On the whole, I'd vote to treat the bootstrap user as a normal role and
therefore have the same restriction in place for that user also.If we're going to enforce such a restriction, I think it would be
a good thing for it to be in place in beta1.
Makes sense.
Patch attached. I'll push this in a bit, barring objections.
Thanks!
Stephen
Attachments:
disallow_reserved_name_initdb.patchtext/x-diff; charset=us-asciiDownload
From ae3ec5c409464612754cd36372a0fc2166bc2f62 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Sun, 8 May 2016 08:35:16 -0400
Subject: [PATCH] Disallow superuser names starting with 'pg_' in initdb
As with CREATE ROLE, disallow users from specifying initial
superuser names which begin with 'pg_' in initdb.
Per discussion with Tom.
---
src/bin/initdb/initdb.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 299ddfe..7dedd8a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3562,6 +3562,12 @@ main(int argc, char *argv[])
if (strlen(username) == 0)
username = effective_user;
+ if (strncmp(username, "pg_", 3) == 0)
+ {
+ fprintf(stderr, _("%s: superuser name \"%s\" is reserved; role names can not begin with 'pg_'\n"), progname, username);
+ exit(1);
+ }
+
printf(_("The files belonging to this database system will be owned "
"by user \"%s\".\n"
"This user must also own the server process.\n\n"),
--
2.5.0
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
If we're going to enforce such a restriction, I think it would be
a good thing for it to be in place in beta1.
Makes sense.
Patch attached. I'll push this in a bit, barring objections.
Three minor wording quibbles:
* s/reserved/disallowed/ maybe? Not 100% sold on this.
* s/can not/cannot/
* use double quotes not single around pg_
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
If we're going to enforce such a restriction, I think it would be
a good thing for it to be in place in beta1.Makes sense.
Patch attached. I'll push this in a bit, barring objections.Three minor wording quibbles:
* s/reserved/disallowed/ maybe? Not 100% sold on this.
* s/can not/cannot/
* use double quotes not single around pg_
Blargh. Missed this before pushing, sorry.
Will fix.
Thanks!
Stephen
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
If we're going to enforce such a restriction, I think it would be
a good thing for it to be in place in beta1.Makes sense.
Patch attached. I'll push this in a bit, barring objections.Three minor wording quibbles:
* s/reserved/disallowed/ maybe? Not 100% sold on this.
* s/can not/cannot/
* use double quotes not single around pg_
Pushed those changes and also emailed the buildfarm owner directly
regarding the change.
Thanks!
Stephen