BUG #15006: "make check" error if current user is "user"
The following bug has been logged on the website:
Bug reference: 15006
Logged by: Sergey Burladyan
Email address: eshkinkot@gmail.com
PostgreSQL version: 10.1
Operating system: Debian GNU/Linux 9 (stretch) in docker
Description:
I run tests in docker with current system user "user" and "make check" stop
with error:
...
updatable_views ... ok
rolenames ... FAILED
roleattributes ... ok
...
==== /home/user/postgresql-10.1/src/test/regress/regression.diffs ====
*** /home/user/postgresql-10.1/src/test/regress/expected/rolenames.out Tue
Nov 7 00:46:52 2017
--- /home/user/postgresql-10.1/src/test/regress/results/rolenames.out Thu
Jan 11 12:51:19 2018
***************
*** 42,47 ****
--- 42,48 ----
CREATE ROLE "current_user";
CREATE ROLE "session_user";
CREATE ROLE "user";
+ ERROR: role "user" already exists
CREATE ROLE current_user; -- error
ERROR: CURRENT_USER cannot be used as a role name here
LINE 1: CREATE ROLE current_user;
***************
*** 950,952 ****
--- 951,954 ----
DROP OWNED BY regress_testrol0, "Public", "current_user",
regress_testrol1, regress_testrol2, regress_testrolx CASCADE;
DROP ROLE regress_testrol0, regress_testrol1, regress_testrol2,
regress_testrolx;
DROP ROLE "Public", "None", "current_user", "session_user", "user";
+ ERROR: current user cannot be dropped======================================================================
On 01/11/2018 01:53 PM, PG Bug reporting form wrote:
The following bug has been logged on the website:
Bug reference: 15006
Logged by: Sergey Burladyan
Email address: eshkinkot@gmail.com
PostgreSQL version: 10.1
Operating system: Debian GNU/Linux 9 (stretch) in docker
Description:I run tests in docker with current system user "user" and "make check" stop
with error:
...
updatable_views ... ok
rolenames ... FAILED
roleattributes ... ok
...
Hmmm ... I'm having this issue too (my distribution uses "user"
internally for the default user). Over time I managed to convince myself
it's not really a PostgreSQL bug, but rather a strange choice of OS user
name. And I've been simply ignoring this particular regression failure
on my laptop.
But maybe we could/should fix it anyway? Most regression tests switched
to roles prefixed with regress_* so why not to do the same here?
Of course, a particularly strange distribution could still pick a
conflicting OS name, but that seems rather unlikely, I guess.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Jan 17, 2018 at 2:58 PM, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:
On 01/11/2018 01:53 PM, PG Bug reporting form wrote:
The following bug has been logged on the website:
Bug reference: 15006
Logged by: Sergey Burladyan
Email address: eshkinkot@gmail.com
PostgreSQL version: 10.1
Operating system: Debian GNU/Linux 9 (stretch) in docker
Description:I run tests in docker with current system user "user" and "make check"
stop
with error:
...
updatable_views ... ok
rolenames ... FAILED
roleattributes ... ok
...But maybe we could/should fix it anyway? Most regression tests switched
to roles prefixed with regress_* so why not to do the same here?
The point of the test seems to be to ensure that the special system
keywords, when quoted, are allowed to be used for role names. So the
choice is to make the test conditional (if the role previously exists
neither create or drop it - and since it existed it doesn't seem like its a
problem to create it anyway) or to simply not bother testing "user"
figuring that the other two roles suffice for testing this behavior.
Changing it to a unlikely-to-conflict name doesn't help since that, I'd
presume, is already being covered by some either sequence of statements.
If these tests can use pl/pgsql and DO-blocks the conditional execution of
this one should be straight-forward to incorporate...
David J.
"David G. Johnston" <david.g.johnston@gmail.com> writes:
On Wed, Jan 17, 2018 at 2:58 PM, Tomas Vondra <tomas.vondra@2ndquadrant.com>
wrote:But maybe we could/should fix it anyway? Most regression tests switched
to roles prefixed with regress_* so why not to do the same here?
The point of the test seems to be to ensure that the special system
keywords, when quoted, are allowed to be used for role names.
Exactly. Changing the names ruins the point of the test.
So the
choice is to make the test conditional (if the role previously exists
neither create or drop it - and since it existed it doesn't seem like its a
problem to create it anyway) or to simply not bother testing "user"
figuring that the other two roles suffice for testing this behavior.
I wouldn't have a big problem with just dropping this whole test stanza.
It's an out-and-out violation of our rule against not creating rolenames
not starting with "regress_", and it's not testing anything that seems
especially likely to break.
regards, tom lane
On Wed, Jan 17, 2018 at 07:21:03PM -0500, Tom Lane wrote:
I wouldn't have a big problem with just dropping this whole test stanza.
It's an out-and-out violation of our rule against not creating rolenames
not starting with "regress_", and it's not testing anything that seems
especially likely to break.
Perhaps a stupid question. What's the point behind the logic to forbid a
double-quoted "public" but to authorize a double-quoted "user"? The
whole thing looks inconsistent to me.
--
Michael
Michael Paquier <michael.paquier@gmail.com> writes:
Perhaps a stupid question. What's the point behind the logic to forbid a
double-quoted "public" but to authorize a double-quoted "user"? The
whole thing looks inconsistent to me.
Good question. There may be some backwards-compatibility considerations
here, but still this is just plain inconsistent:
regression=# create user public;
ERROR: role name "public" is reserved
LINE 1: create user public;
^
regression=# create user "public";
ERROR: role name "public" is reserved
LINE 1: create user "public";
^
regression=# create user user;
ERROR: syntax error at or near "user"
LINE 1: create user user;
^
regression=# create user "user";
CREATE ROLE
regression=# create user current_user;
ERROR: CURRENT_USER cannot be used as a role name here
LINE 1: create user current_user;
^
regression=# create user "current_user";
CREATE ROLE
I can see the point of disallowing a user named "public", because
otherwise syntax like GRANT some-privilege TO PUBLIC is just a trap
for the unwary DBA, one that could have bad security consequences.
But it's not clear to me why the same logic doesn't apply to "user",
"current_user", or "session_user", all of which are equally conflatable
with a built-in meaning in some security-relevant contexts.
BTW, you might think that those wildly different phrasings of essentially
the same error come from different places in the code, but no, they are
from adjacent lines in gram.y. WTF? This seems to be deliberately
anti-consistent.
And, to return to the original scenario, if you do
initdb -U public
it goes through just fine ... so our defenses against having such a
username have a hole in them.
Probably the OP would not be very happy if the outcome of this discussion
is that "initdb -U user" fails, but I am not seeing a principled reason
why that should be allowed.
regards, tom lane
On Wed, Jan 17, 2018 at 09:15:06PM -0500, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
Perhaps a stupid question. What's the point behind the logic to forbid a
double-quoted "public" but to authorize a double-quoted "user"? The
whole thing looks inconsistent to me.Good question. There may be some backwards-compatibility considerations
here, but still this is just plain inconsistent:<snip>
I can see the point of disallowing a user named "public", because
otherwise syntax like GRANT some-privilege TO PUBLIC is just a trap
for the unwary DBA, one that could have bad security consequences.
But it's not clear to me why the same logic doesn't apply to "user",
"current_user", or "session_user", all of which are equally conflatable
with a built-in meaning in some security-relevant contexts.
Just forgot to mention that double-quoted user names with upper-case
characters are similarly allowed should still be allowed, like:
=# CREATE ROLE "Public";
CREATE ROLE
=# CREATE ROLE "pG_as";
CREATE ROLE
So those are correctly handled now.
Worth noting also this bit (from IsReservedName), which looks correct to
me:
=# CREATE ROLE "pg_aB";
ERROR: 42939: role name "pg_aB" is reserved
DETAIL: Role names starting with "pg_" are reserved.
BTW, you might think that those wildly different phrasings of essentially
the same error come from different places in the code, but no, they are
from adjacent lines in gram.y. WTF? This seems to be deliberately
anti-consistent.
Same reaction here :)
I would have expected all the checks to be in user.c and at parsing
level.
Probably the OP would not be very happy if the outcome of this discussion
is that "initdb -U user" fails, but I am not seeing a principled reason
why that should be allowed.
Me neither.
--
Michael
Michael Paquier <michael.paquier@gmail.com> writes:
On Wed, Jan 17, 2018 at 09:15:06PM -0500, Tom Lane wrote:
Probably the OP would not be very happy if the outcome of this discussion
is that "initdb -U user" fails, but I am not seeing a principled reason
why that should be allowed.
Me neither.
Alternatively, we could consider removing all these artificial
restrictions on what a username can be, and just expecting the DBA
to know what he's doing. But what we've got right now is weirdly
inconsistent.
regards, tom lane
On 01/17/2018 06:51 PM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
On Wed, Jan 17, 2018 at 09:15:06PM -0500, Tom Lane wrote:
Probably the OP would not be very happy if the outcome of this discussion
is that "initdb -U user" fails, but I am not seeing a principled reason
why that should be allowed.Me neither.
Alternatively, we could consider removing all these artificial
restrictions on what a username can be, and just expecting the DBA
to know what he's doing. But what we've got right now is weirdly
inconsistent.
+1
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
On Wed, Jan 17, 2018 at 09:15:06PM -0500, Tom Lane wrote:
Probably the OP would not be very happy if the outcome of this discussion
is that "initdb -U user" fails, but I am not seeing a principled reason
why that should be allowed.Me neither.
Alternatively, we could consider removing all these artificial
restrictions on what a username can be, and just expecting the DBA
to know what he's doing. But what we've got right now is weirdly
inconsistent.
Have at it.
This all comes from
/messages/by-id/20141010.172740.88258644.horiguchi.kyotaro@lab.ntt.co.jp
which ended up as commit 31eae6028eca4365e7165f5f33fee1ed0486aee0 (with
a couple of fixups afterwards).
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
Of course, a particularly strange distribution could still pick a
conflicting OS name, but that seems rather unlikely, I guess.
Why the tests depend and use current OS user name? I think it's not very
good (if you does not test exactly that, of course).
If you enforce user name for temporary initdb cluster and use this
name for connection in tests, then you does not depends on OS name at
all, something like this (see attachment).
With this patch "make check-world" run successfully with OS name "user"
and "current_user".
--
Sergey Burladyan