Re: [HACKERS] Re: "pg_xxx" role name restriction not applied to bootstrap superuser?

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Tom Lane (t...@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


signature.asc
Description: Digital signature

Re: [HACKERS] Re: "pg_xxx" role name restriction not applied to bootstrap superuser?

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Tom Lane (t...@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


signature.asc
Description: Digital signature

Re: [HACKERS] Re: "pg_xxx" role name restriction not applied to bootstrap superuser?

[Tom Lane]

Stephen Frost  writes:
> * Tom Lane (t...@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

Re: [HACKERS] Re: "pg_xxx" role name restriction not applied to bootstrap superuser?

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Tom Lane (t...@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
From ae3ec5c409464612754cd36372a0fc2166bc2f62 Mon Sep 17 00:00:00 2001
From: Stephen Frost 
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



signature.asc
Description: Digital signature

Re: [HACKERS] Re: "pg_xxx" role name restriction not applied to bootstrap superuser?

[Tom Lane]

Stephen Frost  writes:
> * Tom Lane (t...@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

[HACKERS] Re: "pg_xxx" role name restriction not applied to bootstrap superuser?

[Stephen Frost]

* Tom Lane (t...@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


signature.asc
Description: Digital signature