Recognizing superuser in pg_hba.conf
It can sometimes be useful to match against a superuser in pg_hba.conf.
For example, one could imagine wanting to reject nonsuperuser from a
particular database.
This used to be possible by creating an empty role and matching against
that, but that functionality was removed (a long time ago) by commit
94cd0f1ad8a.
Adding another keyword can break backwards compatibility, of course. So
that is an issue that needs to be discussed, but I don't imagine too
many people are using role names "superuser" and "nonsuperuser". Those
who are will have to quote them.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachments:
hba_superuser.0001.patchtext/x-patch; charset=UTF-8; name=hba_superuser.0001.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 5f1eec78fb..c0d1e00266 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -250,7 +250,11 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
<para>
Specifies which database user name(s) this record
matches. The value <literal>all</literal> specifies that it
- matches all users. Otherwise, this is either the name of a specific
+ matches all users.
+ The value <literal>superuser</literal> specifies that it matches all
+ superusers. The value <literal>nonsuperuser</literal> specifies that it
+ matches no superusers.
+ Otherwise, this is either the name of a specific
database user, or a group name preceded by <literal>+</literal>.
(Recall that there is no real distinction between users and groups
in <productname>PostgreSQL</productname>; a <literal>+</literal> mark really means
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index b6de92783a..a7a59a0510 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -596,6 +596,16 @@ check_role(const char *role, Oid roleid, List *tokens)
if (is_member(roleid, tok->string + 1))
return true;
}
+ else if (token_is_keyword(tok, "superuser"))
+ {
+ if (superuser_arg(roleid))
+ return true;
+ }
+ else if (token_is_keyword(tok, "nonsuperuser"))
+ {
+ if (!superuser_arg(roleid))
+ return true;
+ }
else if (token_matches(tok, role) ||
token_is_keyword(tok, "all"))
return true;
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
It can sometimes be useful to match against a superuser in pg_hba.conf.
Seems like a reasonable desire.
Adding another keyword can break backwards compatibility, of course. So
that is an issue that needs to be discussed, but I don't imagine too
many people are using role names "superuser" and "nonsuperuser". Those
who are will have to quote them.
I'm not very happy about the continuing creep of pseudo-reserved database
and user names in pg_hba.conf. I wish we'd adjust the notation so that
these keywords are syntactically distinct from ordinary names. Given
the precedent that "+" and "@" prefixes change what an identifier means,
maybe we could use "*" or some other punctuation character as a keyword
prefix? We'd have to give grandfather exceptions to the existing
keywords, at least for a while, but we could say that new ones won't be
recognized without the prefix.
regards, tom lane
On 28/12/2019 19:07, Tom Lane wrote:
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
It can sometimes be useful to match against a superuser in pg_hba.conf.
Seems like a reasonable desire.
Adding another keyword can break backwards compatibility, of course. So
that is an issue that needs to be discussed, but I don't imagine too
many people are using role names "superuser" and "nonsuperuser". Those
who are will have to quote them.I'm not very happy about the continuing creep of pseudo-reserved database
and user names in pg_hba.conf. I wish we'd adjust the notation so that
these keywords are syntactically distinct from ordinary names. Given
the precedent that "+" and "@" prefixes change what an identifier means,
maybe we could use "*" or some other punctuation character as a keyword
prefix? We'd have to give grandfather exceptions to the existing
keywords, at least for a while, but we could say that new ones won't be
recognized without the prefix.
I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.
I will submit two patches, one that recognizes the sigil for all the
other keywords, and then an update of this patch.
--
Vik
On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:
these keywords are syntactically distinct from ordinary names. Given
the precedent that "+" and "@" prefixes change what an identifier means,
maybe we could use "*" or some other punctuation character as a keyword
prefix? We'd have to give grandfather exceptions to the existing
keywords, at least for a while, but we could say that new ones won't be
recognized without the prefix.I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.
I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:
I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.
I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.
I don't have any particular objection to '&' if people prefer that.
But ':' seems like it would introduce confusion with the
variable-substitution notation used in psql and some other places.
It's not that hard to imagine that somebody might want a
variable-substitution notation in pg_hba.conf someday, so we should
leave syntax room for one, and ':' seems like a likely choice
for it (although I suppose a case could be made for '$' too).
regards, tom lane
On Sun, Dec 29, 2019 at 11:31 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't have any particular objection to '&' if people prefer that.
But ':' seems like it would introduce confusion with the
variable-substitution notation used in psql and some other places.It's not that hard to imagine that somebody might want a
variable-substitution notation in pg_hba.conf someday, so we should
leave syntax room for one, and ':' seems like a likely choice
for it (although I suppose a case could be made for '$' too).
Well, as I say, I don't care very much... I hope we can agree on
something and move forward.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 29/12/2019 17:31, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:
I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.I don't have any particular objection to '&' if people prefer that.
I wrote the patch so I got to decide. :-) I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.
It turns out that my original patch didn't really change, all the meat
is in the keywords patch. The superuser patch is to be applied on top
of the keywords patch.
--
Vik Fearing
Attachments:
hba_keywords.0001.patchtext/x-patch; charset=UTF-8; name=hba_keywords.0001.patchDownload
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 55694c4368..add25b2b43 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -8953,8 +8953,8 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<entry><structfield>text</structfield></entry>
<entry>
Host name or IP address, or one
- of <literal>all</literal>, <literal>samehost</literal>,
- or <literal>samenet</literal>, or null for local connections
+ of <literal>&all</literal>, <literal>&samehost</literal>,
+ or <literal>&samenet</literal>, or null for local connections
</entry>
</row>
<row>
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 5f1eec78fb..96fc4b01e9 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -81,11 +81,26 @@
A record is made
up of a number of fields which are separated by spaces and/or tabs.
Fields can contain white space if the field value is double-quoted.
- Quoting one of the keywords in a database, user, or address field (e.g.,
- <literal>all</literal> or <literal>replication</literal>) makes the word lose its special
- meaning, and just match a database, user, or host with that name.
</para>
+ <note>
+ <para>
+ As of version 13, keywords are preceded by the character "&".
+ For compatibility, the following legacy keywords are still accepted
+ on their own:
+ <literal>all</literal>, <literal>replication</literal>,
+ <literal>samegroup</literal>, <literal>samehost</literal>,
+ <literal>samenet</literal>, <literal>samerole</literal>,
+ <literal>sameuser</literal>.
+ </para>
+
+ <para>
+ Quoting one of these legacy keywords in a database, user, or address
+ field makes the word lose its special meaning, and just match a
+ database, user, or host with that name.
+ </para>
+ </note>
+
<para>
Each record specifies a connection type, a client IP address range
(if relevant for the connection type), a database name, a user name,
@@ -221,18 +236,18 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
<listitem>
<para>
Specifies which database name(s) this record matches. The value
- <literal>all</literal> specifies that it matches all databases.
- The value <literal>sameuser</literal> specifies that the record
+ <literal>&all</literal> specifies that it matches all databases.
+ The value <literal>&sameuser</literal> specifies that the record
matches if the requested database has the same name as the
- requested user. The value <literal>samerole</literal> specifies that
+ requested user. The value <literal>&samerole</literal> specifies that
the requested user must be a member of the role with the same
- name as the requested database. (<literal>samegroup</literal> is an
- obsolete but still accepted spelling of <literal>samerole</literal>.)
+ name as the requested database. (<literal>&samegroup</literal> is an
+ obsolete but still accepted spelling of <literal>&samerole</literal>.)
Superusers are not considered to be members of a role for the
- purposes of <literal>samerole</literal> unless they are explicitly
+ purposes of <literal>&samerole</literal> unless they are explicitly
members of the role, directly or indirectly, and not just by
virtue of being a superuser.
- The value <literal>replication</literal> specifies that the record
+ The value <literal>&replication</literal> specifies that the record
matches if a physical replication connection is requested (note that
replication connections do not specify any particular database).
Otherwise, this is the name of
@@ -249,7 +264,7 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
<listitem>
<para>
Specifies which database user name(s) this record
- matches. The value <literal>all</literal> specifies that it
+ matches. The value <literal>&all</literal> specifies that it
matches all users. Otherwise, this is either the name of a specific
database user, or a group name preceded by <literal>+</literal>.
(Recall that there is no real distinction between users and groups
@@ -312,9 +327,9 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
</para>
<para>
- You can also write <literal>all</literal> to match any IP address,
- <literal>samehost</literal> to match any of the server's own IP
- addresses, or <literal>samenet</literal> to match any address in any
+ You can also write <literal>&all</literal> to match any IP address,
+ <literal>&samehost</literal> to match any of the server's own IP
+ addresses, or <literal>&samenet</literal> to match any address in any
subnet that the server is directly connected to.
</para>
@@ -699,40 +714,40 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
# connections).
#
# TYPE DATABASE USER ADDRESS METHOD
-local all all trust
+local &all &all trust
# The same using local loopback TCP/IP connections.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all 127.0.0.1/32 trust
+host &all &all 127.0.0.1/32 trust
# The same as the previous line, but using a separate netmask column
#
# TYPE DATABASE USER IP-ADDRESS IP-MASK METHOD
-host all all 127.0.0.1 255.255.255.255 trust
+host &all &all 127.0.0.1 255.255.255.255 trust
# The same over IPv6.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all ::1/128 trust
+host &all &all ::1/128 trust
# The same using a host name (would typically cover both IPv4 and IPv6).
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all localhost trust
+host &all &all localhost trust
# Allow any user from any host with IP address 192.168.93.x to connect
# to database "postgres" as the same user name that ident reports for
# the connection (typically the operating system user name).
#
# TYPE DATABASE USER ADDRESS METHOD
-host postgres all 192.168.93.0/24 ident
+host postgres &all 192.168.93.0/24 ident
# Allow any user from host 192.168.12.10 to connect to database
# "postgres" if the user's password is correctly supplied.
#
# TYPE DATABASE USER ADDRESS METHOD
-host postgres all 192.168.12.10/32 scram-sha-256
+host postgres &all 192.168.12.10/32 scram-sha-256
# Allow any user from hosts in the example.com domain to connect to
# any database if the user's password is correctly supplied.
@@ -742,8 +757,8 @@ host postgres all 192.168.12.10/32 scram-sha-256
# authentication.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all mike .example.com md5
-host all all .example.com scram-sha-256
+host &all mike .example.com md5
+host &all &all .example.com scram-sha-256
# In the absence of preceding "host" lines, these three lines will
# reject all connections from 192.168.54.1 (since that entry will be
@@ -754,9 +769,9 @@ host all all .example.com scram-sha-256
# encrypted GSSAPI connections) are allowed, but only from 192.168.12.10.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all 192.168.54.1/32 reject
-hostgssenc all all 0.0.0.0/0 gss
-host all all 192.168.12.10/32 gss
+host &all &all 192.168.54.1/32 reject
+hostgssenc &all &all 0.0.0.0/0 gss
+host &all &all 192.168.12.10/32 gss
# Allow users from 192.168.x.x hosts to connect to any database, if
# they pass the ident check. If, for example, ident says the user is
@@ -765,7 +780,7 @@ host all all 192.168.12.10/32 gss
# "omicron" that says "bryanh" is allowed to connect as "guest1".
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all 192.168.0.0/16 ident map=omicron
+host &all &all 192.168.0.0/16 ident map=omicron
# If these are the only three lines for local connections, they will
# allow local users to connect only to their own databases (databases
@@ -775,15 +790,15 @@ host all all 192.168.0.0/16 ident map=omicro
# are required in all cases.
#
# TYPE DATABASE USER ADDRESS METHOD
-local sameuser all md5
-local all @admins md5
-local all +support md5
+local &sameuser &all md5
+local &all @admins md5
+local &all +support md5
# The last two lines above can be combined into a single line:
-local all @admins,+support md5
+local &all @admins,+support md5
# The database column can also use lists and file names:
-local db1,db2,@demodbs all md5
+local db1,db2,@demodbs &all md5
</programlisting>
</example>
</sect1>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index b6de92783a..da18c389e5 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -64,8 +64,20 @@ typedef struct check_network_data
bool result; /* set to true if match */
} check_network_data;
+/*
+ * The following keywords are accepted without the keyword sigil for legacy
+ * reasons. They may become normal words at some point in the future.
+ */
+#define token_is_legacy_keyword(t, k) (!t->quoted && ( \
+ strcmp(t->string, "all") == 0 || \
+ strcmp(t->string, "replication") == 0 || \
+ strcmp(t->string, "samegroup") == 0 || \
+ strcmp(t->string, "samehost") == 0 || \
+ strcmp(t->string, "samenet") == 0 || \
+ strcmp(t->string, "samerole") == 0 || \
+ strcmp(t->string, "sameuser") == 0))
-#define token_is_keyword(t, k) (!t->quoted && strcmp(t->string, k) == 0)
+#define token_is_keyword(t, k) ((t->string[0] == '&' && strcmp(t->string+1, k) == 0) || token_is_legacy_keyword(t, k))
#define token_matches(t, k) (strcmp(t->string, k) == 0)
/*
@@ -2472,7 +2484,7 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
* Flatten HbaToken list to string list. It might seem that we
* should re-quote any quoted tokens, but that has been rejected
* on the grounds that it makes it harder to compare the array
- * elements to other system catalogs. That makes entries like
+ * elements to other system catalogs. That makes entries with legacy keywords like
* "all" or "samerole" formally ambiguous ... but users who name
* databases/roles that way are inflicting their own pain.
*/
diff --git a/src/backend/libpq/pg_hba.conf.sample b/src/backend/libpq/pg_hba.conf.sample
index c853e36232..d08aba497f 100644
--- a/src/backend/libpq/pg_hba.conf.sample
+++ b/src/backend/libpq/pg_hba.conf.sample
@@ -21,12 +21,12 @@
# "hostssl" is an SSL-encrypted TCP/IP socket, and "hostnossl" is a
# plain TCP/IP socket.
#
-# DATABASE can be "all", "sameuser", "samerole", "replication", a
-# database name, or a comma-separated list thereof. The "all"
-# keyword does not match "replication". Access to replication
+# DATABASE can be &all, &sameuser, &samerole, &replication, a
+# database name, or a comma-separated list thereof. The &all
+# keyword does not match &replication. Access to replication
# must be enabled in a separate record (see example below).
#
-# USER can be "all", a user name, a group name prefixed with "+", or a
+# USER can be &all, a user name, a group name prefixed with "+", or a
# comma-separated list thereof. In both the DATABASE and USER fields
# you can also write a file name prefixed with "@" to include names
# from a separate file.
@@ -53,11 +53,18 @@
# section in the documentation for a list of which options are
# available for which authentication methods.
#
+# As of version 13, keywords are preceded by the character "&".
+# For compatibility, the following legacy keywords are still accepted on
+# their own:
+# all,
+# replication,
+# sameuser, samegroup, samerole,
+# samehost, samenet
+#
# Database and user names containing spaces, commas, quotes and other
-# special characters must be quoted. Quoting one of the keywords
-# "all", "sameuser", "samerole" or "replication" makes the name lose
-# its special character, and just match a database or username with
-# that name.
+# special characters must be quoted. Quoting one of the legacy keywords
+# from the previous paragraph makes the name lose its special character,
+# and just match a database or username with that name.
#
# This file is read on server startup and when the server receives a
# SIGHUP signal. If you edit the file on a running system, you have to
@@ -77,13 +84,13 @@
# TYPE DATABASE USER ADDRESS METHOD
@remove-line-for-nolocal@# "local" is for Unix domain socket connections only
-@remove-line-for-nolocal@local all all @authmethodlocal@
+@remove-line-for-nolocal@local &all &all @authmethodlocal@
# IPv4 local connections:
-host all all 127.0.0.1/32 @authmethodhost@
+host &all &all 127.0.0.1/32 @authmethodhost@
# IPv6 local connections:
-host all all ::1/128 @authmethodhost@
+host &all &all ::1/128 @authmethodhost@
# Allow replication connections from localhost, by a user with the
# replication privilege.
-@remove-line-for-nolocal@local replication all @authmethodlocal@
-host replication all 127.0.0.1/32 @authmethodhost@
-host replication all ::1/128 @authmethodhost@
+@remove-line-for-nolocal@local &replication &all @authmethodlocal@
+host &replication &all 127.0.0.1/32 @authmethodhost@
+host &replication &all ::1/128 @authmethodhost@
hba_superuser.0002.patchtext/x-patch; charset=UTF-8; name=hba_superuser.0002.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 96fc4b01e9..36cdf180c4 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -265,7 +265,11 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
<para>
Specifies which database user name(s) this record
matches. The value <literal>&all</literal> specifies that it
- matches all users. Otherwise, this is either the name of a specific
+ matches all users.
+ The value <literal>&superuser</literal> specifies that it matches all
+ superusers. The value <literal>&nonsuperuser</literal> specifies that it
+ matches no superusers.
+ Otherwise, this is either the name of a specific
database user, or a group name preceded by <literal>+</literal>.
(Recall that there is no real distinction between users and groups
in <productname>PostgreSQL</productname>; a <literal>+</literal> mark really means
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index da18c389e5..087680be64 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -608,6 +608,16 @@ check_role(const char *role, Oid roleid, List *tokens)
if (is_member(roleid, tok->string + 1))
return true;
}
+ else if (token_is_keyword(tok, "superuser"))
+ {
+ if (superuser_arg(roleid))
+ return true;
+ }
+ else if (token_is_keyword(tok, "nonsuperuser"))
+ {
+ if (!superuser_arg(roleid))
+ return true;
+ }
else if (token_matches(tok, role) ||
token_is_keyword(tok, "all"))
return true;
On 29/12/2019 23:10, Vik Fearing wrote:
On 29/12/2019 17:31, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:
I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.I don't have any particular objection to '&' if people prefer that.
I wrote the patch so I got to decide. :-) I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.It turns out that my original patch didn't really change, all the meat
is in the keywords patch. The superuser patch is to be applied on top
of the keywords patch.
I missed a few places in the tap tests. New keywords patch attached,
superuser patch unchanged.
--
Vik Fearing
Attachments:
hba_keywords.0002.patchtext/x-patch; charset=UTF-8; name=hba_keywords.0002.patchDownload
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 55694c4368..add25b2b43 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -8953,8 +8953,8 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<entry><structfield>text</structfield></entry>
<entry>
Host name or IP address, or one
- of <literal>all</literal>, <literal>samehost</literal>,
- or <literal>samenet</literal>, or null for local connections
+ of <literal>&all</literal>, <literal>&samehost</literal>,
+ or <literal>&samenet</literal>, or null for local connections
</entry>
</row>
<row>
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 5f1eec78fb..96fc4b01e9 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -81,11 +81,26 @@
A record is made
up of a number of fields which are separated by spaces and/or tabs.
Fields can contain white space if the field value is double-quoted.
- Quoting one of the keywords in a database, user, or address field (e.g.,
- <literal>all</literal> or <literal>replication</literal>) makes the word lose its special
- meaning, and just match a database, user, or host with that name.
</para>
+ <note>
+ <para>
+ As of version 13, keywords are preceded by the character "&".
+ For compatibility, the following legacy keywords are still accepted
+ on their own:
+ <literal>all</literal>, <literal>replication</literal>,
+ <literal>samegroup</literal>, <literal>samehost</literal>,
+ <literal>samenet</literal>, <literal>samerole</literal>,
+ <literal>sameuser</literal>.
+ </para>
+
+ <para>
+ Quoting one of these legacy keywords in a database, user, or address
+ field makes the word lose its special meaning, and just match a
+ database, user, or host with that name.
+ </para>
+ </note>
+
<para>
Each record specifies a connection type, a client IP address range
(if relevant for the connection type), a database name, a user name,
@@ -221,18 +236,18 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
<listitem>
<para>
Specifies which database name(s) this record matches. The value
- <literal>all</literal> specifies that it matches all databases.
- The value <literal>sameuser</literal> specifies that the record
+ <literal>&all</literal> specifies that it matches all databases.
+ The value <literal>&sameuser</literal> specifies that the record
matches if the requested database has the same name as the
- requested user. The value <literal>samerole</literal> specifies that
+ requested user. The value <literal>&samerole</literal> specifies that
the requested user must be a member of the role with the same
- name as the requested database. (<literal>samegroup</literal> is an
- obsolete but still accepted spelling of <literal>samerole</literal>.)
+ name as the requested database. (<literal>&samegroup</literal> is an
+ obsolete but still accepted spelling of <literal>&samerole</literal>.)
Superusers are not considered to be members of a role for the
- purposes of <literal>samerole</literal> unless they are explicitly
+ purposes of <literal>&samerole</literal> unless they are explicitly
members of the role, directly or indirectly, and not just by
virtue of being a superuser.
- The value <literal>replication</literal> specifies that the record
+ The value <literal>&replication</literal> specifies that the record
matches if a physical replication connection is requested (note that
replication connections do not specify any particular database).
Otherwise, this is the name of
@@ -249,7 +264,7 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
<listitem>
<para>
Specifies which database user name(s) this record
- matches. The value <literal>all</literal> specifies that it
+ matches. The value <literal>&all</literal> specifies that it
matches all users. Otherwise, this is either the name of a specific
database user, or a group name preceded by <literal>+</literal>.
(Recall that there is no real distinction between users and groups
@@ -312,9 +327,9 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
</para>
<para>
- You can also write <literal>all</literal> to match any IP address,
- <literal>samehost</literal> to match any of the server's own IP
- addresses, or <literal>samenet</literal> to match any address in any
+ You can also write <literal>&all</literal> to match any IP address,
+ <literal>&samehost</literal> to match any of the server's own IP
+ addresses, or <literal>&samenet</literal> to match any address in any
subnet that the server is directly connected to.
</para>
@@ -699,40 +714,40 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
# connections).
#
# TYPE DATABASE USER ADDRESS METHOD
-local all all trust
+local &all &all trust
# The same using local loopback TCP/IP connections.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all 127.0.0.1/32 trust
+host &all &all 127.0.0.1/32 trust
# The same as the previous line, but using a separate netmask column
#
# TYPE DATABASE USER IP-ADDRESS IP-MASK METHOD
-host all all 127.0.0.1 255.255.255.255 trust
+host &all &all 127.0.0.1 255.255.255.255 trust
# The same over IPv6.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all ::1/128 trust
+host &all &all ::1/128 trust
# The same using a host name (would typically cover both IPv4 and IPv6).
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all localhost trust
+host &all &all localhost trust
# Allow any user from any host with IP address 192.168.93.x to connect
# to database "postgres" as the same user name that ident reports for
# the connection (typically the operating system user name).
#
# TYPE DATABASE USER ADDRESS METHOD
-host postgres all 192.168.93.0/24 ident
+host postgres &all 192.168.93.0/24 ident
# Allow any user from host 192.168.12.10 to connect to database
# "postgres" if the user's password is correctly supplied.
#
# TYPE DATABASE USER ADDRESS METHOD
-host postgres all 192.168.12.10/32 scram-sha-256
+host postgres &all 192.168.12.10/32 scram-sha-256
# Allow any user from hosts in the example.com domain to connect to
# any database if the user's password is correctly supplied.
@@ -742,8 +757,8 @@ host postgres all 192.168.12.10/32 scram-sha-256
# authentication.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all mike .example.com md5
-host all all .example.com scram-sha-256
+host &all mike .example.com md5
+host &all &all .example.com scram-sha-256
# In the absence of preceding "host" lines, these three lines will
# reject all connections from 192.168.54.1 (since that entry will be
@@ -754,9 +769,9 @@ host all all .example.com scram-sha-256
# encrypted GSSAPI connections) are allowed, but only from 192.168.12.10.
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all 192.168.54.1/32 reject
-hostgssenc all all 0.0.0.0/0 gss
-host all all 192.168.12.10/32 gss
+host &all &all 192.168.54.1/32 reject
+hostgssenc &all &all 0.0.0.0/0 gss
+host &all &all 192.168.12.10/32 gss
# Allow users from 192.168.x.x hosts to connect to any database, if
# they pass the ident check. If, for example, ident says the user is
@@ -765,7 +780,7 @@ host all all 192.168.12.10/32 gss
# "omicron" that says "bryanh" is allowed to connect as "guest1".
#
# TYPE DATABASE USER ADDRESS METHOD
-host all all 192.168.0.0/16 ident map=omicron
+host &all &all 192.168.0.0/16 ident map=omicron
# If these are the only three lines for local connections, they will
# allow local users to connect only to their own databases (databases
@@ -775,15 +790,15 @@ host all all 192.168.0.0/16 ident map=omicro
# are required in all cases.
#
# TYPE DATABASE USER ADDRESS METHOD
-local sameuser all md5
-local all @admins md5
-local all +support md5
+local &sameuser &all md5
+local &all @admins md5
+local &all +support md5
# The last two lines above can be combined into a single line:
-local all @admins,+support md5
+local &all @admins,+support md5
# The database column can also use lists and file names:
-local db1,db2,@demodbs all md5
+local db1,db2,@demodbs &all md5
</programlisting>
</example>
</sect1>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index b6de92783a..da18c389e5 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -64,8 +64,20 @@ typedef struct check_network_data
bool result; /* set to true if match */
} check_network_data;
+/*
+ * The following keywords are accepted without the keyword sigil for legacy
+ * reasons. They may become normal words at some point in the future.
+ */
+#define token_is_legacy_keyword(t, k) (!t->quoted && ( \
+ strcmp(t->string, "all") == 0 || \
+ strcmp(t->string, "replication") == 0 || \
+ strcmp(t->string, "samegroup") == 0 || \
+ strcmp(t->string, "samehost") == 0 || \
+ strcmp(t->string, "samenet") == 0 || \
+ strcmp(t->string, "samerole") == 0 || \
+ strcmp(t->string, "sameuser") == 0))
-#define token_is_keyword(t, k) (!t->quoted && strcmp(t->string, k) == 0)
+#define token_is_keyword(t, k) ((t->string[0] == '&' && strcmp(t->string+1, k) == 0) || token_is_legacy_keyword(t, k))
#define token_matches(t, k) (strcmp(t->string, k) == 0)
/*
@@ -2472,7 +2484,7 @@ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
* Flatten HbaToken list to string list. It might seem that we
* should re-quote any quoted tokens, but that has been rejected
* on the grounds that it makes it harder to compare the array
- * elements to other system catalogs. That makes entries like
+ * elements to other system catalogs. That makes entries with legacy keywords like
* "all" or "samerole" formally ambiguous ... but users who name
* databases/roles that way are inflicting their own pain.
*/
diff --git a/src/backend/libpq/pg_hba.conf.sample b/src/backend/libpq/pg_hba.conf.sample
index c853e36232..d08aba497f 100644
--- a/src/backend/libpq/pg_hba.conf.sample
+++ b/src/backend/libpq/pg_hba.conf.sample
@@ -21,12 +21,12 @@
# "hostssl" is an SSL-encrypted TCP/IP socket, and "hostnossl" is a
# plain TCP/IP socket.
#
-# DATABASE can be "all", "sameuser", "samerole", "replication", a
-# database name, or a comma-separated list thereof. The "all"
-# keyword does not match "replication". Access to replication
+# DATABASE can be &all, &sameuser, &samerole, &replication, a
+# database name, or a comma-separated list thereof. The &all
+# keyword does not match &replication. Access to replication
# must be enabled in a separate record (see example below).
#
-# USER can be "all", a user name, a group name prefixed with "+", or a
+# USER can be &all, a user name, a group name prefixed with "+", or a
# comma-separated list thereof. In both the DATABASE and USER fields
# you can also write a file name prefixed with "@" to include names
# from a separate file.
@@ -53,11 +53,18 @@
# section in the documentation for a list of which options are
# available for which authentication methods.
#
+# As of version 13, keywords are preceded by the character "&".
+# For compatibility, the following legacy keywords are still accepted on
+# their own:
+# all,
+# replication,
+# sameuser, samegroup, samerole,
+# samehost, samenet
+#
# Database and user names containing spaces, commas, quotes and other
-# special characters must be quoted. Quoting one of the keywords
-# "all", "sameuser", "samerole" or "replication" makes the name lose
-# its special character, and just match a database or username with
-# that name.
+# special characters must be quoted. Quoting one of the legacy keywords
+# from the previous paragraph makes the name lose its special character,
+# and just match a database or username with that name.
#
# This file is read on server startup and when the server receives a
# SIGHUP signal. If you edit the file on a running system, you have to
@@ -77,13 +84,13 @@
# TYPE DATABASE USER ADDRESS METHOD
@remove-line-for-nolocal@# "local" is for Unix domain socket connections only
-@remove-line-for-nolocal@local all all @authmethodlocal@
+@remove-line-for-nolocal@local &all &all @authmethodlocal@
# IPv4 local connections:
-host all all 127.0.0.1/32 @authmethodhost@
+host &all &all 127.0.0.1/32 @authmethodhost@
# IPv6 local connections:
-host all all ::1/128 @authmethodhost@
+host &all &all ::1/128 @authmethodhost@
# Allow replication connections from localhost, by a user with the
# replication privilege.
-@remove-line-for-nolocal@local replication all @authmethodlocal@
-host replication all 127.0.0.1/32 @authmethodhost@
-host replication all ::1/128 @authmethodhost@
+@remove-line-for-nolocal@local &replication &all @authmethodlocal@
+host &replication &all 127.0.0.1/32 @authmethodhost@
+host &replication &all ::1/128 @authmethodhost@
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index 5985130e3d..237a5ab53f 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -29,7 +29,7 @@ sub reset_pg_hba
my $hba_method = shift;
unlink($node->data_dir . '/pg_hba.conf');
- $node->append_conf('pg_hba.conf', "local all all $hba_method");
+ $node->append_conf('pg_hba.conf', "local &all &all $hba_method");
$node->reload;
return;
}
diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl
index c4b335c45f..8d54f60710 100644
--- a/src/test/authentication/t/002_saslprep.pl
+++ b/src/test/authentication/t/002_saslprep.pl
@@ -25,7 +25,7 @@ sub reset_pg_hba
my $hba_method = shift;
unlink($node->data_dir . '/pg_hba.conf');
- $node->append_conf('pg_hba.conf', "local all all $hba_method");
+ $node->append_conf('pg_hba.conf', "local &all &all $hba_method");
$node->reload;
return;
}
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index e3eb052160..108fd70243 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -197,7 +197,7 @@ sub test_access
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{host all all $hostaddr/32 gss map=mymap});
+ qq{host &all &all $hostaddr/32 gss map=mymap});
$node->restart;
test_access($node, 'test1', 'SELECT true', 2, '', 'fails without ticket');
@@ -233,7 +233,7 @@ test_access(
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{hostgssenc all all $hostaddr/32 gss map=mymap});
+ qq{hostgssenc &all &all $hostaddr/32 gss map=mymap});
$node->restart;
test_access(
@@ -255,7 +255,7 @@ test_access($node, "test1", 'SELECT true', 2, "gssencmode=disable",
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{hostnogssenc all all $hostaddr/32 gss map=mymap});
+ qq{hostnogssenc &all &all $hostaddr/32 gss map=mymap});
$node->restart;
test_access(
@@ -279,7 +279,7 @@ test_access(
truncate($node->data_dir . '/pg_ident.conf', 0);
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{host all all $hostaddr/32 gss include_realm=0});
+ qq{host &all &all $hostaddr/32 gss include_realm=0});
$node->restart;
test_access(
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index f8941144f5..f1eb2e8cfc 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -174,7 +174,7 @@ note "simple bind";
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapprefix="uid=" ldapsuffix=",dc=example,dc=net"}
+ qq{local &all &all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapprefix="uid=" ldapsuffix=",dc=example,dc=net"}
);
$node->restart;
@@ -190,7 +190,7 @@ note "search+bind";
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn"}
+ qq{local &all &all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn"}
);
$node->restart;
@@ -206,7 +206,7 @@ note "multiple servers";
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapserver="$ldap_server $ldap_server" ldapport=$ldap_port ldapbasedn="$ldap_basedn"}
+ qq{local &all &all ldap ldapserver="$ldap_server $ldap_server" ldapport=$ldap_port ldapbasedn="$ldap_basedn"}
);
$node->restart;
@@ -222,7 +222,7 @@ note "LDAP URLs";
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapurl="$ldap_url/$ldap_basedn?uid?sub"});
+ qq{local &all &all ldap ldapurl="$ldap_url/$ldap_basedn?uid?sub"});
$node->restart;
$ENV{"PGPASSWORD"} = 'wrong';
@@ -239,7 +239,7 @@ note "search filters";
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn" ldapsearchfilter="(|(uid=\$username)(mail=\$username))"}
+ qq{local &all &all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn" ldapsearchfilter="(|(uid=\$username)(mail=\$username))"}
);
$node->restart;
@@ -252,7 +252,7 @@ note "search filters in LDAP URLs";
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapurl="$ldap_url/$ldap_basedn??sub?(|(uid=\$username)(mail=\$username))"}
+ qq{local &all &all ldap ldapurl="$ldap_url/$ldap_basedn??sub?(|(uid=\$username)(mail=\$username))"}
);
$node->restart;
@@ -266,7 +266,7 @@ test_access($node, 'test2@example.net', 0, 'search filter finds by mail');
# override. It might be useful in a case like this.
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapurl="$ldap_url/$ldap_basedn??sub" ldapsearchfilter="(|(uid=\$username)(mail=\$username))"}
+ qq{local &all &all ldap ldapurl="$ldap_url/$ldap_basedn??sub" ldapsearchfilter="(|(uid=\$username)(mail=\$username))"}
);
$node->restart;
@@ -278,7 +278,7 @@ note "diagnostic message";
# note bad ldapprefix with a question mark that triggers a diagnostic message
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapprefix="?uid=" ldapsuffix=""}
+ qq{local &all &all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapprefix="?uid=" ldapsuffix=""}
);
$node->restart;
@@ -290,7 +290,7 @@ note "TLS";
# request StartTLS with ldaptls=1
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn" ldapsearchfilter="(uid=\$username)" ldaptls=1}
+ qq{local &all &all ldap ldapserver=$ldap_server ldapport=$ldap_port ldapbasedn="$ldap_basedn" ldapsearchfilter="(uid=\$username)" ldaptls=1}
);
$node->restart;
@@ -300,7 +300,7 @@ test_access($node, 'test1', 0, 'StartTLS');
# request LDAPS with ldapscheme=ldaps
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapserver=$ldap_server ldapscheme=ldaps ldapport=$ldaps_port ldapbasedn="$ldap_basedn" ldapsearchfilter="(uid=\$username)"}
+ qq{local &all &all ldap ldapserver=$ldap_server ldapscheme=ldaps ldapport=$ldaps_port ldapbasedn="$ldap_basedn" ldapsearchfilter="(uid=\$username)"}
);
$node->restart;
@@ -310,7 +310,7 @@ test_access($node, 'test1', 0, 'LDAPS');
# request LDAPS with ldapurl=ldaps://...
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapurl="$ldaps_url/$ldap_basedn??sub?(uid=\$username)"}
+ qq{local &all &all ldap ldapurl="$ldaps_url/$ldap_basedn??sub?(uid=\$username)"}
);
$node->restart;
@@ -320,7 +320,7 @@ test_access($node, 'test1', 0, 'LDAPS with URL');
# bad combination of LDAPS and StartTLS
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf',
- qq{local all all ldap ldapurl="$ldaps_url/$ldap_basedn??sub?(uid=\$username)" ldaptls=1}
+ qq{local &all &all ldap ldapurl="$ldaps_url/$ldap_basedn??sub?(uid=\$username)" ldaptls=1}
);
$node->restart;
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 270bd6c856..b39d8cf25a 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -390,7 +390,7 @@ sub set_replication_conf
if ($TestLib::windows_os)
{
print $hba
- "host replication all $test_localhost/32 sspi include_realm=1 map=regress\n";
+ "host &replication &all $test_localhost/32 sspi include_realm=1 map=regress\n";
}
close $hba;
return;
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 297b8fbd6f..650fe38767 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1065,10 +1065,10 @@ config_sspi_auth(const char *pgdata, const char *superuser_name)
exit(2);
}
CW(fputs("# Configuration written by config_sspi_auth()\n", hba) >= 0);
- CW(fputs("host all all 127.0.0.1/32 sspi include_realm=1 map=regress\n",
+ CW(fputs("host &all &all 127.0.0.1/32 sspi include_realm=1 map=regress\n",
hba) >= 0);
if (have_ipv6)
- CW(fputs("host all all ::1/128 sspi include_realm=1 map=regress\n",
+ CW(fputs("host &all &all ::1/128 sspi include_realm=1 map=regress\n",
hba) >= 0);
CW(fclose(hba) == 0);
hba_superuser.0002.patchtext/x-patch; charset=UTF-8; name=hba_superuser.0002.patchDownload
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 96fc4b01e9..36cdf180c4 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -265,7 +265,11 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceable
<para>
Specifies which database user name(s) this record
matches. The value <literal>&all</literal> specifies that it
- matches all users. Otherwise, this is either the name of a specific
+ matches all users.
+ The value <literal>&superuser</literal> specifies that it matches all
+ superusers. The value <literal>&nonsuperuser</literal> specifies that it
+ matches no superusers.
+ Otherwise, this is either the name of a specific
database user, or a group name preceded by <literal>+</literal>.
(Recall that there is no real distinction between users and groups
in <productname>PostgreSQL</productname>; a <literal>+</literal> mark really means
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index da18c389e5..087680be64 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -608,6 +608,16 @@ check_role(const char *role, Oid roleid, List *tokens)
if (is_member(roleid, tok->string + 1))
return true;
}
+ else if (token_is_keyword(tok, "superuser"))
+ {
+ if (superuser_arg(roleid))
+ return true;
+ }
+ else if (token_is_keyword(tok, "nonsuperuser"))
+ {
+ if (!superuser_arg(roleid))
+ return true;
+ }
else if (token_matches(tok, role) ||
token_is_keyword(tok, "all"))
return true;
On Mon, Dec 30, 2019 at 11:56:17AM +0100, Vik Fearing wrote:
On 29/12/2019 23:10, Vik Fearing wrote:
On 29/12/2019 17:31, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:
I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.I don't have any particular objection to '&' if people prefer that.
I wrote the patch so I got to decide. :-)� I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.It turns out that my original patch didn't really change, all the meat
is in the keywords patch.� The superuser patch is to be applied on top
of the keywords patch.I missed a few places in the tap tests.� New keywords patch attached,
superuser patch unchanged.--
Vik Fearing
Patches apply cleanly to 0ce38730ac72029f3f2c95ae80b44f5b9060cbcc, and
include documentation. They could use an example of the new
capability, possibly included in the sample pg_hba.conf, e.g.
host &all &superuser 0.0.0.0/0 reject
or similar.
The feature works as described, and is useful. I have thus far been
unable to make it crash.
I haven't used intentionally hostile strings to test it, as I didn't
see those as an important attack surface. This is because by the time
someone hostile can write to pg_hba.conf, they've got all the control
they need to manipulate the entire node, including root exploits.
I've marked this as Ready for Committer.
Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Greetings,
* Vik Fearing (vik.fearing@2ndquadrant.com) wrote:
On 29/12/2019 23:10, Vik Fearing wrote:
On 29/12/2019 17:31, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:
I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.I don't have any particular objection to '&' if people prefer that.
I wrote the patch so I got to decide. :-) I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.It turns out that my original patch didn't really change, all the meat
is in the keywords patch. The superuser patch is to be applied on top
of the keywords patch.I missed a few places in the tap tests. New keywords patch attached,
superuser patch unchanged.
We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?
Thanks,
Stephen
On 02/01/2020 20:52, Stephen Frost wrote:
Greetings,
* Vik Fearing (vik.fearing@2ndquadrant.com) wrote:
On 29/12/2019 23:10, Vik Fearing wrote:
On 29/12/2019 17:31, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:
I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.I don't have any particular objection to '&' if people prefer that.
I wrote the patch so I got to decide. :-)� I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.It turns out that my original patch didn't really change, all the meat
is in the keywords patch.� The superuser patch is to be applied on top
of the keywords patch.I missed a few places in the tap tests.� New keywords patch attached,
superuser patch unchanged.We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?
This is an argument against the superusers patch, but surely you are not
suggesting we add a pg_all role that contains all users?� And what about
the keywords that aren't for users?
--
Vik Fearing
Stephen Frost <sfrost@snowman.net> writes:
We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?
Meh. If the things aren't actually roles, I think this'd just
add confusion. Or were you proposing to implement them as roles?
I'm not sure if that would be practical in every case.
regards, tom lane
"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Stephen Frost <sfrost@snowman.net> writes:
We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix
when we could just declare that 'pg_superusers' is a role to which
all superusers are members? Or something along those lines?
Tom> Meh. If the things aren't actually roles, I think this'd just add
Tom> confusion. Or were you proposing to implement them as roles? I'm
Tom> not sure if that would be practical in every case.
In fact my original suggestion when this idea was discussed on IRC was
to remove the current superuser flag and turn it into a role; but the
issue then is that role membership is inherited and superuserness
currently isn't, so that's a more intrusive change.
--
Andrew (irc:RhodiumToad)
Greetings,
On Thu, Jan 2, 2020 at 15:04 Tom Lane <tgl@sss.pgh.pa.us> wrote:
Stephen Frost <sfrost@snowman.net> writes:
We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?Meh. If the things aren't actually roles, I think this'd just
add confusion. Or were you proposing to implement them as roles?
I'm not sure if that would be practical in every case.
Having them as roles might be interesting though I don’t think it would be
required. As for your argument, surely we aren’t going to make
“&superusers” an actual role with this, so you have to accept that’s what
there isn’t a real role either way. I don’t really care for this idea of
making up new syntax that people have to learn, understand, train others
on, etc.
The pg_ prefix makes it clear that it’s a system role... literally by
definition.
As for Vik’s thought about “pg_all”- I hadn’t been thinking we would do
that (“all” is already accepted there anyway and trying to deprecate that
seems unlikely to result in ever actually removing it because that’s the
kind of thing we will argue about and never do..), but it seems like an
interesting idea. Using “public” is maybe another interesting thought there
since that’s the same thing and also reserved...
Thanks,
Stephen
Show quoted text
## Stephen Frost (sfrost@snowman.net):
We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?
Taking this idea one step further (back?): with any non-trivial
number of (user-)roles in the database, DBAs would be well advised
to use group(-role)s for privilege management anyways. It's not
to unreasonable to grant SUPERUSER through a group, too. Although
I'm not sure we'd need a new pg_superuser role here, we're not
inventing a new set of object privileges as in e.g. pg_monitor;
the DBA can just create their own superuser group.
Is there really a need to add more features, or would it be sufficient
to make the applications of group roles more prominent in the docs?
(I've seen way too many cases in which people where granting privileges
to individual users when they should have used groups, so I might
be biased).
Regards,
Christoph
--
Spare Space
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> Meh. If the things aren't actually roles, I think this'd just add
Tom> confusion. Or were you proposing to implement them as roles? I'm
Tom> not sure if that would be practical in every case.
In fact my original suggestion when this idea was discussed on IRC was
to remove the current superuser flag and turn it into a role; but the
issue then is that role membership is inherited and superuserness
currently isn't, so that's a more intrusive change.
To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.
It's possible that role-ifying everything and then allowing "!role"
in the pg_hba.conf syntax would be enough. Not sure though.
More generally, allowing inheritance of superuser scares me a bit
from a security standpoint. I wouldn't mind turning all the other
legacy role properties into grantable roles, but I *like* the fact
that that one is special.
regards, tom lane
Greetings,
On Thu, Jan 2, 2020 at 15:50 Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> Meh. If the things aren't actually roles, I think this'd just add
Tom> confusion. Or were you proposing to implement them as roles? I'm
Tom> not sure if that would be practical in every case.In fact my original suggestion when this idea was discussed on IRC was
to remove the current superuser flag and turn it into a role; but the
issue then is that role membership is inherited and superuserness
currently isn't, so that's a more intrusive change.To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.
Sorry- why do we need that..? The first match for a pg_hba line wins, so
you can define all the access methods that superuser accounts are allowed
to use first, then a “reject” line for superuser accounts, and then
whatever else you want after that.
More generally, allowing inheritance of superuser scares me a bit
from a security standpoint. I wouldn't mind turning all the other
legacy role properties into grantable roles, but I *like* the fact
that that one is special.
Requiring an extra “set role whatever;” is good to make sure the user
really understands they’re running as superuser, but it doesn’t really
improve actual security at all since there’s no way to require a password
or anything. That superuser-ness isn’t inherited but membership in the
“postgres” or other role-that-owns-everything role is actually strikes me
as less than ideal... the whole allow system table mods thing kinda helps
with that since you need that extra step to actually change most things but
it’s still not great imv. I can’t get too excited about trying to improve
this though since I’d expect material changes to improve security to be
beat back with backwards incompatibility concerns.
Thanks,
Stephen
Show quoted text
Stephen Frost <sfrost@snowman.net> writes:
On Thu, Jan 2, 2020 at 15:50 Tom Lane <tgl@sss.pgh.pa.us> wrote:
To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.
Sorry- why do we need that..? The first match for a pg_hba line wins, so
you can define all the access methods that superuser accounts are allowed
to use first, then a “reject” line for superuser accounts, and then
whatever else you want after that.
Seems kind of awkward. Or more to the point: you can already do whatever
you want in pg_hba.conf, as long as you're willing to be verbose enough
(and, perhaps, willing to maintain group memberships to fit your needs).
The discussion here, IMO, is about offering useful shorthands.
So a facility like "!role" seems potentially useful. Maybe it's not
really, but I don't think we should reject it just because there's
a verbose and non-obvious way to get the same result.
regards, tom lane
Greetings,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
On Thu, Jan 2, 2020 at 15:50 Tom Lane <tgl@sss.pgh.pa.us> wrote:
To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.Sorry- why do we need that..? The first match for a pg_hba line wins, so
you can define all the access methods that superuser accounts are allowed
to use first, then a “reject” line for superuser accounts, and then
whatever else you want after that.Seems kind of awkward. Or more to the point: you can already do whatever
you want in pg_hba.conf, as long as you're willing to be verbose enough
(and, perhaps, willing to maintain group memberships to fit your needs).
Sure it's awkward, but it's how people actually deal with these things
today. I'm not against improving on that situation but I also don't
hear tons of complaints about it either, so I do think we should be
thoughtful when it comes to making changes here.
The discussion here, IMO, is about offering useful shorthands.
In general, I'm alright with that idea, but I do want to make sure we're
really being thoughtful when it comes to inventing new syntax that will
only work in this one place and will have to be handled specially by any
tools or anything that wants to generate or look at this.
What are we going to have be displayed through pg_hba_file_rules() for
this '!role' or whatever else, in the 'user_name' column? (Also, ugh,
I find calling that column 'user_name' mildly offensive considering that
function was added well after roles, and it's not like it really meant
'user name' even before then..).
Yes, I'm sure we could just have it be the text '!role' and make
everyone who cares have to parse out that field, in SQL, to figure out
who it really applies to and basically just make everyone deal with it
but I remain skeptical about if it's really a particularly good
approach.
So a facility like "!role" seems potentially useful. Maybe it's not
really, but I don't think we should reject it just because there's
a verbose and non-obvious way to get the same result.
I don't agree that it's "non-obvious" that if you have a config file
where "first match wins" that things which don't match the first line
are, by definition, NOT whatever that first line was and then fall
through to the next, where you could use 'reject' if you want. In fact,
I've always kinda figured that's what 'reject' was for, though I'll
admit that it's been around for far longer than I've been involved in
the project (sadly, I hadn't discovered PG yet back in 1998).
Thanks,
Stephen
So it's not clear to me whether we have any meeting of the minds
on wanting this patch. In the meantime, though, the cfbot
reports that the patch breaks the ssl tests. Why is that?
regards, tom lane
On 06/01/2020 17:03, Tom Lane wrote:
So it's not clear to me whether we have any meeting of the minds
on wanting this patch. In the meantime, though, the cfbot
reports that the patch breaks the ssl tests. Why is that?
I have no idea. I cannot reproduce the failure locally.
--
Vik Fearing
Vik Fearing <vik.fearing@2ndquadrant.com> writes:
On 06/01/2020 17:03, Tom Lane wrote:
So it's not clear to me whether we have any meeting of the minds
on wanting this patch. In the meantime, though, the cfbot
reports that the patch breaks the ssl tests. Why is that?
I have no idea. I cannot reproduce the failure locally.
Hm, it blows up pretty thoroughly for me too, on a RHEL6 box.
Are you sure you're running that test -- check-world doesn't do it?
At least in the 001_ssltests test, the failures seem to all look
like this in the TAP test's log file:
psql: error: could not connect to server: could not initiate GSSAPI security context: Unspecified GSS failure. Minor code may provide more information
could not initiate GSSAPI security context: Credentials cache file '/tmp/krb5cc_502' not found
There are no matching entries in the postmaster log file, so this
seems to be strictly a client-side failure.
(Are we *really* putting security credentials in /tmp ???)
regards, tom lane
On 2020-01-06 17:03, Tom Lane wrote:
So it's not clear to me whether we have any meeting of the minds
on wanting this patch.
This fairly far-ranging syntax reorganization of pg_hba.conf doesn't
appeal to me. pg_hba.conf is complicated enough conceptually for users,
but AFAICT nobody ever complained about the syntax or the lexical
structure specifically. Assigning meaning to randomly chosen special
characters, moreover in a security-relevant file, seems like the wrong
way to go.
Moreover, this thread has morphed from what it says in the subject line
to changing the syntax of pg_hba.conf in a somewhat fundamental way. So
at the very least someone should post a comprehensive summary of what is
being proposed, instead of just attaching patches that implement
whatever was discussed across the thread.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 08/01/2020 23:13, Peter Eisentraut wrote:
On 2020-01-06 17:03, Tom Lane wrote:
So it's not clear to me whether we have any meeting of the minds
on wanting this patch.This fairly far-ranging syntax reorganization of pg_hba.conf doesn't
appeal to me. pg_hba.conf is complicated enough conceptually for
users, but AFAICT nobody ever complained about the syntax or the
lexical structure specifically. Assigning meaning to randomly chosen
special characters, moreover in a security-relevant file, seems like
the wrong way to go.Moreover, this thread has morphed from what it says in the subject
line to changing the syntax of pg_hba.conf in a somewhat fundamental
way. So at the very least someone should post a comprehensive summary
of what is being proposed, instead of just attaching patches that
implement whatever was discussed across the thread.
What is being proposed is what is in the Subject and the original
patch. The other patch is because Tom didn't like "the continuing creep
of pseudo-reserved database and user names" so I wrote a patch to mark
such reserved names and rebased my original patch on top of it. Only
the docs changed in the rebase. The original patch (or its rebase) is
what I am interested in.
--
Vik Fearing
On Wed, 8 Jan 2020 at 23:55, Vik Fearing <vik.fearing@2ndquadrant.com>
wrote:
On 08/01/2020 23:13, Peter Eisentraut wrote:
On 2020-01-06 17:03, Tom Lane wrote:
So it's not clear to me whether we have any meeting of the minds
on wanting this patch.This fairly far-ranging syntax reorganization of pg_hba.conf doesn't
appeal to me. pg_hba.conf is complicated enough conceptually for
users, but AFAICT nobody ever complained about the syntax or the
lexical structure specifically. Assigning meaning to randomly chosen
special characters, moreover in a security-relevant file, seems like
the wrong way to go.Moreover, this thread has morphed from what it says in the subject
line to changing the syntax of pg_hba.conf in a somewhat fundamental
way. So at the very least someone should post a comprehensive summary
of what is being proposed, instead of just attaching patches that
implement whatever was discussed across the thread.What is being proposed is what is in the Subject and the original
patch. The other patch is because Tom didn't like "the continuing creep
of pseudo-reserved database and user names" so I wrote a patch to mark
such reserved names and rebased my original patch on top of it. Only
the docs changed in the rebase. The original patch (or its rebase) is
what I am interested in.
Hopefully there will be no danger of me gaining access if I have a crafted
rolename?
postgres=# create role "&backdoor";
CREATE ROLE
--
Simon Riggs http://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Solutions for the Enterprise
Simon Riggs <simon@2ndquadrant.com> writes:
On Wed, 8 Jan 2020 at 23:55, Vik Fearing <vik.fearing@2ndquadrant.com>
wrote:What is being proposed is what is in the Subject and the original
patch. The other patch is because Tom didn't like "the continuing creep
of pseudo-reserved database and user names" so I wrote a patch to mark
such reserved names and rebased my original patch on top of it. Only
the docs changed in the rebase. The original patch (or its rebase) is
what I am interested in.
Hopefully there will be no danger of me gaining access if I have a crafted
rolename?
postgres=# create role "&backdoor";
CREATE ROLE
Well, the existence of such a role name wouldn't by itself cause any
change in the way that pg_hba.conf is parsed. If you could then
persuade a superuser to insert a pg_hba.conf line that is trying
to match your username, the line might do something else than what the
superuser expected, which is bad. But the *exact* same hazard applies
to proposals based on inventing pseudo-reserved keywords (by which
I mean things that look like names, but aren't reserved words, so that
somebody could create a role name matching them). Either way, an
uninformed superuser could be tricked.
What I'm basically objecting to is the pseudo-reservedness. If we
don't want to dodge that with special syntax, we should dodge it
by making sure the keywords are actually reserved names. In other
words, add a "pg_" prefix, as somebody else suggested upthread.
I don't personally find that prettier than a punctuation prefix,
but I can live with it if other people do.
BTW, although that solution works for the immediate need of
keywords that have to be distinguished from role names, it doesn't
currently scale to keywords for the database column, because we
don't treat "pg_" as a reserved prefix for database names:
regression=# create role pg_zit;
ERROR: role name "pg_zit" is reserved
DETAIL: Role names starting with "pg_" are reserved.
regression=# create database pg_zit;
CREATE DATABASE
Should we do so, or wait till there's an immediate need to?
regards, tom lane
Greetings,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
What I'm basically objecting to is the pseudo-reservedness. If we
don't want to dodge that with special syntax, we should dodge it
by making sure the keywords are actually reserved names. In other
words, add a "pg_" prefix, as somebody else suggested upthread.
Yes, that was my suggestion, and it was also my change a few major
versions ago that actually reserved the "pg_" prefix for roles.
BTW, although that solution works for the immediate need of
keywords that have to be distinguished from role names, it doesn't
currently scale to keywords for the database column, because we
don't treat "pg_" as a reserved prefix for database names:regression=# create role pg_zit;
ERROR: role name "pg_zit" is reserved
DETAIL: Role names starting with "pg_" are reserved.
regression=# create database pg_zit;
CREATE DATABASEShould we do so, or wait till there's an immediate need to?
I seem to recall (but it was years ago, so I might be wrong) advocating
that we should reserve the 'pg_' prefix for *all* object types. All I
can recall is that there wasn't much backing for the idea (though I also
don't recall any specific objection, and it's also quite possible that
there was simply no response to the idea).
For my 2c, I'd *much* rather we reserve it across the board, and sooner
than later, since that would hopefully reduce the impact on people. The
only justification for *not* reserving it is if we KNOW that we'll never
need a special one of those, but, well, we're well past that for
database names already- look at the fact that we've got a "replication"
one, for example. Maybe we can't ever un-reserve that, but I like the
idea of reserving "pg_" for database names and then having
"pg_replication" be allowed to mean replication connections and then
encouraging users to use that instead.
Thanks,
Stephen
On Thu, Jan 9, 2020 at 10:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
What I'm basically objecting to is the pseudo-reservedness. If we
don't want to dodge that with special syntax, we should dodge it
by making sure the keywords are actually reserved names.
You know, as I was reading this email, I got to thinking: aren't we
engineering a solution to a problem for which we already have a
solution?
The documentation says:
"Quoting one of the keywords in a database, user, or address field
(e.g., all or replication) makes the word lose its special character,
and just match a database, user, or host with that name."
So if you've writing a pg_hba.conf file that contains a specific role
name, and you want to make sure it doesn't get confused with a current
or future keyword, just quote it. If you don't quote it, make sure to
RTFM at the time and when upgrading.
If you want to argue that this isn't the cleanest possible solution to
the problem, I think I would agree. If we were doing this over again,
we could probably design a better syntax for pg_hba.conf, perhaps one
where all specific role names have to be quoted and anything that's
not quoted is expected to be a keyword. But, as it is, nothing's
really broken here, and practical confusion is unlikely. If someone
has a role named "superuser", then it's probably a superuser account,
and the worst that will happen is that we'll match all superuser
accounts rather than only that one. If someone has a non-superuser
account called "superuser", or if they have an account named
"nonsuperuser," then, uh, that's lame, and if this patch causes them
to improve their choice of role names, that's good. If it causes them
to use quotes, that's also good.
But I think I'm coming around to the view that we're making what ought
to be a simple change complicated, and we should just not do that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Jan 9, 2020 at 10:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
What I'm basically objecting to is the pseudo-reservedness. If we
don't want to dodge that with special syntax, we should dodge it
by making sure the keywords are actually reserved names.
...
But I think I'm coming around to the view that we're making what ought
to be a simple change complicated, and we should just not do that.
The problem is that we keep deciding that okay, it probably won't hurt
anybody if this particular thing-that-ought-to-be-a-reserved-word isn't
really reserved. Your exercise in justifying that for "superuser" is
not unlike every other previous argument about this. Sooner or later
that's going to fail, and somebody's going to have a security problem
because they didn't know that a particular name has magic properties
in a particular context. (Which, indeed, maybe it didn't have when
they chose it.) Claiming they should have known better isn't where
I want to be when that happens.
I don't want to keep going down that path. These things are effectively
reserved words, and they need to act like reserved words, so that you
get an error if you misuse them. Silently doing something else than
what (one reasonable reading of) a pg_hba.conf entry seems to imply
is *bad*.
regards, tom lane
On Thu, Jan 9, 2020 at 11:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
The problem is that we keep deciding that okay, it probably won't hurt
anybody if this particular thing-that-ought-to-be-a-reserved-word isn't
really reserved. Your exercise in justifying that for "superuser" is
not unlike every other previous argument about this. Sooner or later
that's going to fail, and somebody's going to have a security problem
because they didn't know that a particular name has magic properties
in a particular context. (Which, indeed, maybe it didn't have when
they chose it.) Claiming they should have known better isn't where
I want to be when that happens.
But, again, we already *have* a way of solving this problem: use
quotes. As Simon pointed out, your proposed solution isn't really a
solution at all, because & can appear in role names. It probably
won't, but there probably also won't be a role name that matches
either of these keywords, so it's just six of one, half a dozen of the
other. The thing that really solves it is quoting.
Now I admit that if we decide pg_hba.conf keywords have to start with
"pg_" and prevent names beginning with "pg_" from being used as object
names, then we'd have TWO ways of distinguishing between a keyword and
an object name. But I don't think TMTOWTDI is the right design
principle here.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Thu, Jan 9, 2020 at 11:06 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
The problem is that we keep deciding that okay, it probably won't hurt
anybody if this particular thing-that-ought-to-be-a-reserved-word isn't
really reserved.
But, again, we already *have* a way of solving this problem: use
quotes. As Simon pointed out, your proposed solution isn't really a
solution at all, because & can appear in role names.
I'm not sure that the pg_hba.conf parser allows that without quotes,
but in any case it's irrelevant to the proposal to use a pg_ prefix.
We don't allow non-built-in role names to be spelled that way,
quoted or not.
Now I admit that if we decide pg_hba.conf keywords have to start with
"pg_" and prevent names beginning with "pg_" from being used as object
names, then we'd have TWO ways of distinguishing between a keyword and
an object name. But I don't think TMTOWTDI is the right design
principle here.
The principle I'm concerned about is not letting a configuration file
that was perfectly fine in version N silently become a security hazard
in version N+1. The only way I will accept your proposal is if we
change the pg_hba.conf parser to *require* quotes around every
role and DB name that's not meant to be a keyword, so that people get
used to that requirement. But I doubt that idea will fly.
regards, tom lane
Greetings,
* Robert Haas (robertmhaas@gmail.com) wrote:
But, again, we already *have* a way of solving this problem: use
quotes. As Simon pointed out, your proposed solution isn't really a
solution at all, because & can appear in role names. It probably
won't, but there probably also won't be a role name that matches
either of these keywords, so it's just six of one, half a dozen of the
other. The thing that really solves it is quoting.
I really just can't agree with the idea that:
"&superuser"
and
&superuser
in pg_hba.conf should mean materially different things and have far
reaching security differences. Depending on quoting in pg_hba.conf for
this distinction is an altogether bad idea.
Now I admit that if we decide pg_hba.conf keywords have to start with
"pg_" and prevent names beginning with "pg_" from being used as object
names, then we'd have TWO ways of distinguishing between a keyword and
an object name. But I don't think TMTOWTDI is the right design
principle here.
There is a *really* big difference here though which makes this not "two
ways to do the same thing"- you *can't* create a user starting with
"pg_". You *can* create a user with an '&' in it. If we prevented you
from being able to create users with '&' in it then I'd be more open to
the idea of using '&' to mean something special in pg_hba, and then it
really would be two different ways to do the same thing, but that's not
actually what's being proposed here.
Thanks,
Stephen
Hi,
I see this patch is marked as RFC since 12/30, but there seems to be
quite a lot of discussion about the syntax, keywords and how exactly to
identify the superuser. So I'll switch it back to needs review, which I
think is a better representation of the current state.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
I see this patch is marked as RFC since 12/30, but there seems to be
quite a lot of discussion about the syntax, keywords and how exactly to
identify the superuser. So I'll switch it back to needs review, which I
think is a better representation of the current state.
Somebody switched it to RFC again, despite the facts that
(a) there is absolutely no consensus about what syntax to use
(and some of the proposals imply very different patches),
(b) there's been no discussion at all since the last CF, and
(c) the patch is still failing in the cfbot (src/test/ssl fails).
While resolving (c) would seem to be the author's problem, I don't
think it's worth putting effort into that detail until we have
some meeting of the minds about (a). So I'll put this back to
"needs review".
regards, tom lane
On 30 Mar 2020, at 20:28, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
I see this patch is marked as RFC since 12/30, but there seems to be
quite a lot of discussion about the syntax, keywords and how exactly to
identify the superuser. So I'll switch it back to needs review, which I
think is a better representation of the current state.Somebody switched it to RFC again, despite the facts that
(a) there is absolutely no consensus about what syntax to use
(and some of the proposals imply very different patches),(b) there's been no discussion at all since the last CF, and
(c) the patch is still failing in the cfbot (src/test/ssl fails).
While resolving (c) would seem to be the author's problem, I don't
think it's worth putting effort into that detail until we have
some meeting of the minds about (a). So I'll put this back to
"needs review".
Since there hasn't been any more progress on this since the last CF, and the
fact that the outcome may result in a completely different patch, I'm inclined
to mark this returned with feedback rather than have it linger. The discussion
can continue and the entry be re-opened.
Thoughts?
cheers ./daniel
On 7/2/20 3:14 PM, Daniel Gustafsson wrote:
On 30 Mar 2020, at 20:28, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
I see this patch is marked as RFC since 12/30, but there seems to be
quite a lot of discussion about the syntax, keywords and how exactly to
identify the superuser. So I'll switch it back to needs review, which I
think is a better representation of the current state.Somebody switched it to RFC again, despite the facts that
(a) there is absolutely no consensus about what syntax to use
(and some of the proposals imply very different patches),(b) there's been no discussion at all since the last CF, and
(c) the patch is still failing in the cfbot (src/test/ssl fails).
While resolving (c) would seem to be the author's problem, I don't
think it's worth putting effort into that detail until we have
some meeting of the minds about (a). So I'll put this back to
"needs review".Since there hasn't been any more progress on this since the last CF, and the
fact that the outcome may result in a completely different patch, I'm inclined
to mark this returned with feedback rather than have it linger. The discussion
can continue and the entry be re-opened.Thoughts?
No objection.
--
Vik Fearing