CREATE/ALTER ROLE PASSWORD ('value' USING 'method')
Hi all,
As discussed on the thread dedicated to SCRAM
(/messages/by-id/243d8c11-6149-a4bb-0909-136992f74b23@iki.fi),
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').
Now that password_encryption has been extended with a new value
'scram', it is a bit bothersome for the user to create roles using
different methods because password_encryption would need to be set
first:
=# SET password_encryption = 'scram';
SET
=# CREATE ROLE foorole PASSWORD 'foopass';
CREATE ROLE
=# SET password_encryption = 'md5';
SET
=# CREATE ROLE foorole2 PASSWORD 'foopass';
CREATE ROLE
What I am proposing with the patch attached is to add a new clause
(grammar is an idea from Robert), to do the same in a single command:
=# CREATE ROLE foorole3 PASSWORD ('foo' USING 'scram');
CREATE ROLE
=# CREATE ROLE foorole4 PASSWORD ('foo' USING 'md5');
CREATE ROLE
This way there is no need to enforce password_encryption prior to
define a new password. Note that like the existing clauses, this is
permissive. In short, if the value is already MD5-encrypted or
SCRAM-encrypted, then the type of the parsed value is enforced
compared to what is defined as method for this USING clause, which is
useful for bumping data.
As this needs clarification before Postgres 10, I am adding a bullet
in the TODO items. This would prove to be useful if more protocols are
added in the future.
Thoughts?
--
Michael
Attachments:
0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patchapplication/octet-stream; name=0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patchDownload
From aa3986a7511f6bea279acc335c7cb446fd8c95bd Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 8 Mar 2017 10:59:05 +0900
Subject: [PATCH] Add clause PASSWORD (val USING protocol) to CREATE/ALTER ROLE
This clause allows users to be able to enforce with which protocol
a given password is used with. if the value given is already encrypted,
the value is used as-is. This extension is useful to support future
protocols, particularly SCRAM-SHA-256.
---
doc/src/sgml/ref/alter_role.sgml | 10 +++++
doc/src/sgml/ref/create_role.sgml | 26 +++++++++++
src/backend/commands/user.c | 80 +++++++++++++++++++++++++++++++---
src/backend/parser/gram.y | 7 +++
src/test/regress/expected/password.out | 14 +++++-
src/test/regress/sql/password.sql | 9 ++++
6 files changed, 140 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index da36ad9696..43e45d504d 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -34,6 +34,7 @@ ALTER ROLE <replaceable class="PARAMETER">role_specification</replaceable> [ WIT
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
| [ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
+ | PASSWORD ( '<replaceable class="PARAMETER">password</replaceable>' USING '<replaceable class="PARAMETER">method</replaceable>' )
| VALID UNTIL '<replaceable class="PARAMETER">timestamp</replaceable>'
ALTER ROLE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>new_name</replaceable>
@@ -169,6 +170,7 @@ ALTER ROLE { <replaceable class="PARAMETER">role_specification</replaceable> | A
<term><literal>NOBYPASSRLS</literal></term>
<term><literal>CONNECTION LIMIT</literal> <replaceable class="parameter">connlimit</replaceable></term>
<term><literal>PASSWORD</> <replaceable class="parameter">password</replaceable></term>
+ <term><literal>PASSWORD</> ( '<replaceable class="parameter">password</replaceable>' USING '<replaceable class="parameter">method</replaceable>' )</term>
<term><literal>ENCRYPTED</></term>
<term><literal>UNENCRYPTED</></term>
<term><literal>VALID UNTIL</literal> '<replaceable class="parameter">timestamp</replaceable>'</term>
@@ -280,6 +282,14 @@ ALTER ROLE davide WITH PASSWORD 'hu8jmn3';
</para>
<para>
+ Change a role's password using MD5-encryption:
+
+<programlisting>
+ALTER ROLE lionel WITH PASSWORD ('hu8jmn3' USING 'md5');
+</programlisting>
+ </para>
+
+ <para>
Remove a role's password:
<programlisting>
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
index 99d1c8336c..d142166f2b 100644
--- a/doc/src/sgml/ref/create_role.sgml
+++ b/doc/src/sgml/ref/create_role.sgml
@@ -34,6 +34,7 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
| [ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
+ | PASSWORD ( '<replaceable class="PARAMETER">password</replaceable>' USING '<replaceable class="PARAMETER">method</replaceable>' )
| VALID UNTIL '<replaceable class="PARAMETER">timestamp</replaceable>'
| IN ROLE <replaceable class="PARAMETER">role_name</replaceable> [, ...]
| IN GROUP <replaceable class="PARAMETER">role_name</replaceable> [, ...]
@@ -247,6 +248,23 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
</varlistentry>
<varlistentry>
+ <term><literal>PASSWORD</> ( '<replaceable class="parameter">password</replaceable>' USING '<replaceable class="parameter">method</replaceable>' )</term>
+ <listitem>
+ <para>
+ Sets the role's password using the requested method. (A password
+ is only of use for roles having the <literal>LOGIN</literal>
+ attribute, but you can nonetheless define one for roles without it.)
+ If you do not plan to use password authentication you can omit this
+ option. The methods supported are <literal>md5</> to enforce a password
+ to be MD5-encrypted, <literal>scram</> for a SCRAM-encrypted password
+ and <literal>plain</> for an unencrypted password. If the password
+ string is already in MD5-encrypted or SCRAM-encrypted, then it is
+ stored encrypted as-is.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>VALID UNTIL</literal> '<replaceable class="parameter">timestamp</replaceable>'</term>
<listitem>
<para>
@@ -428,6 +446,14 @@ CREATE USER davide WITH PASSWORD 'jw8s0F4';
</para>
<para>
+ Create a role with a MD5-encrypted password:
+
+<programlisting>
+CREATE USER lionel WITH PASSWORD ('asdh7as' USING 'md5');
+</programlisting>
+ </para>
+
+ <para>
Create a role with a password that is valid until the end of 2004.
After one second has ticked in 2005, the password is no longer
valid.
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 14b9779144..a8729f494d 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -130,7 +130,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
if (strcmp(defel->defname, "password") == 0 ||
strcmp(defel->defname, "encryptedPassword") == 0 ||
- strcmp(defel->defname, "unencryptedPassword") == 0)
+ strcmp(defel->defname, "unencryptedPassword") == 0 ||
+ strcmp(defel->defname, "methodPassword") == 0)
{
if (dpassword)
ereport(ERROR,
@@ -144,9 +145,45 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
password_type = PASSWORD_TYPE_SCRAM;
else
password_type = PASSWORD_TYPE_MD5;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
}
else if (strcmp(defel->defname, "unencryptedPassword") == 0)
+ {
password_type = PASSWORD_TYPE_PLAINTEXT;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
+ else if (strcmp(defel->defname, "methodPassword") == 0)
+ {
+ /*
+ * This is a list of two elements, the password is first and
+ * then there is the method wanted by caller.
+ */
+ if (dpassword && dpassword->arg)
+ {
+ char *method = strVal(lsecond((List *) dpassword->arg));
+
+ password = strVal(linitial((List *) dpassword->arg));
+
+ if (strcmp(method, "md5") == 0)
+ password_type = PASSWORD_TYPE_MD5;
+ else if (strcmp(method, "plain") == 0)
+ password_type = PASSWORD_TYPE_PLAINTEXT;
+ else if (strcmp(method, "scram") == 0)
+ password_type = PASSWORD_TYPE_SCRAM;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unsupported password method %s", method)));
+ }
+ }
+ else
+ {
+ password_type = Password_encryption;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
}
else if (strcmp(defel->defname, "sysid") == 0)
{
@@ -266,8 +303,6 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
defel->defname);
}
- if (dpassword && dpassword->arg)
- password = strVal(dpassword->arg);
if (dissuper)
issuper = intVal(dissuper->arg) != 0;
if (dinherit)
@@ -539,6 +574,7 @@ AlterRole(AlterRoleStmt *stmt)
if (strcmp(defel->defname, "password") == 0 ||
strcmp(defel->defname, "encryptedPassword") == 0 ||
+ strcmp(defel->defname, "methodPassword") == 0 ||
strcmp(defel->defname, "unencryptedPassword") == 0)
{
if (dpassword)
@@ -552,9 +588,45 @@ AlterRole(AlterRoleStmt *stmt)
password_type = PASSWORD_TYPE_SCRAM;
else
password_type = PASSWORD_TYPE_MD5;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
}
else if (strcmp(defel->defname, "unencryptedPassword") == 0)
+ {
password_type = PASSWORD_TYPE_PLAINTEXT;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
+ else if (strcmp(defel->defname, "methodPassword") == 0)
+ {
+ /*
+ * This is a list of two elements, the password is first and
+ * then there is the method wanted by caller.
+ */
+ if (dpassword && dpassword->arg)
+ {
+ char *method = strVal(lsecond((List *) dpassword->arg));
+
+ if (strcmp(method, "md5") == 0)
+ password_type = PASSWORD_TYPE_MD5;
+ else if (strcmp(method, "plain") == 0)
+ password_type = PASSWORD_TYPE_PLAINTEXT;
+ else if (strcmp(method, "scram") == 0)
+ password_type = PASSWORD_TYPE_SCRAM;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unsupported password method %s", method)));
+
+ password = strVal(linitial((List *) dpassword->arg));
+ }
+ }
+ else
+ {
+ password_type = Password_encryption;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
}
else if (strcmp(defel->defname, "superuser") == 0)
{
@@ -642,8 +714,6 @@ AlterRole(AlterRoleStmt *stmt)
defel->defname);
}
- if (dpassword && dpassword->arg)
- password = strVal(dpassword->arg);
if (dissuper)
issuper = intVal(dissuper->arg);
if (dinherit)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index bb55e1c95c..66040db219 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -983,6 +983,13 @@ AlterOptRoleElem:
{
$$ = makeDefElem("password", NULL, @1);
}
+ | PASSWORD '(' Sconst USING Sconst ')'
+ {
+ $$ = makeDefElem("methodPassword",
+ (Node *)list_make2(makeString($3),
+ makeString($5)),
+ @1);
+ }
| ENCRYPTED PASSWORD Sconst
{
$$ = makeDefElem("encryptedPassword",
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index c503e43abe..8efa5d495f 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -63,6 +63,12 @@ ALTER ROLE regress_passwd4 ENCRYPTED PASSWORD 'scram-sha-256:VLK4RMaQLCvNtQ==:40
SET password_encryption = 'scram';
ALTER ROLE regress_passwd5 ENCRYPTED PASSWORD 'foo'; -- create SCRAM verifier
CREATE ROLE regress_passwd6 ENCRYPTED PASSWORD 'md53725413363ab045e20521bf36b8d8d7f'; -- encrypted with MD5, use as it is
+-- PASSWORD ('value' USING 'method')
+CREATE ROLE regress_passwd7 PASSWORD ('role_pwd7' USING 'plain');
+CREATE ROLE regress_passwd8 PASSWORD ('role_pwd8' USING 'md5');
+CREATE ROLE regress_passwd9 PASSWORD ('role_pwd9' USING 'scram');
+CREATE ROLE regress_passwd10 PASSWORD ('role_pwd10' USING 'novalue'); --error
+ERROR: unsupported password method novalue
SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
FROM pg_authid
WHERE rolname LIKE 'regress_passwd%'
@@ -75,7 +81,10 @@ SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):
regress_passwd4 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
regress_passwd5 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
regress_passwd6 | md53725413363ab045e20521bf36b8d8d7f
-(6 rows)
+ regress_passwd7 | role_pwd7
+ regress_passwd8 | md571f6e76fa74cf5716b886f35ad945663
+ regress_passwd9 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
+(9 rows)
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
@@ -83,6 +92,9 @@ DROP ROLE regress_passwd3;
DROP ROLE regress_passwd4;
DROP ROLE regress_passwd5;
DROP ROLE regress_passwd6;
+DROP ROLE regress_passwd7;
+DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
-- all entries should have been removed
SELECT rolname, rolpassword
FROM pg_authid
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index f4b3a9ac3a..5872f126ea 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -54,6 +54,12 @@ SET password_encryption = 'scram';
ALTER ROLE regress_passwd5 ENCRYPTED PASSWORD 'foo'; -- create SCRAM verifier
CREATE ROLE regress_passwd6 ENCRYPTED PASSWORD 'md53725413363ab045e20521bf36b8d8d7f'; -- encrypted with MD5, use as it is
+-- PASSWORD ('value' USING 'method')
+CREATE ROLE regress_passwd7 PASSWORD ('role_pwd7' USING 'plain');
+CREATE ROLE regress_passwd8 PASSWORD ('role_pwd8' USING 'md5');
+CREATE ROLE regress_passwd9 PASSWORD ('role_pwd9' USING 'scram');
+CREATE ROLE regress_passwd10 PASSWORD ('role_pwd10' USING 'novalue'); --error
+
SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
FROM pg_authid
WHERE rolname LIKE 'regress_passwd%'
@@ -65,6 +71,9 @@ DROP ROLE regress_passwd3;
DROP ROLE regress_passwd4;
DROP ROLE regress_passwd5;
DROP ROLE regress_passwd6;
+DROP ROLE regress_passwd7;
+DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
-- all entries should have been removed
SELECT rolname, rolpassword
--
2.12.0
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').
The parentheses seem weird ... do we really need those?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Mar 8, 2017 at 1:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
A grammar without parenthesis works as well, and I am open to suggestions.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Mar 8, 2017 at 9:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
+1
I had quick glance to patch and it looks great.
One quick comments:
+ | PASSWORD '(' Sconst USING Sconst ')'
+ {
+ $$ = makeDefElem("methodPassword",
+ (Node *)list_make2(makeString($3),
+ makeString($5)),
+ @1);
+ }methodPassword looks bit weird, can we change it to passwordMethod
or pwdEncryptMethod ?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Rushabh Lathia
On Wed, Mar 8, 2017 at 2:32 PM, Rushabh Lathia <rushabh.lathia@gmail.com> wrote:
One quick comments:
+ | PASSWORD '(' Sconst USING Sconst ')' + { + $$ = makeDefElem("methodPassword", + (Node *)list_make2(makeString($3), + makeString($5)), + @1); + }methodPassword looks bit weird, can we change it to passwordMethod
or pwdEncryptMethod ?
Using "Password" in suffix looks still better to me though for
consistency with the other ones.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 03/07/2017 08:29 PM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
+1
+ If you do not plan to use password authentication you can omit this + option. The methods supported are <literal>md5</> to enforce a password + to be MD5-encrypted, <literal>scram</> for a SCRAM-encrypted password + and <literal>plain</> for an unencrypted password. If the password
Can we please stop calling this encryption? What is being done is a form
of cryptographic hashing, not encryption.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway <mail@joeconway.com> wrote:
On 03/07/2017 08:29 PM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
+1
Seeing 3 opinions in favor of that, let's do so then. I have updated
the patch to not use parenthesis.
+ If you do not plan to use password authentication you can omit this + option. The methods supported are <literal>md5</> to enforce a password + to be MD5-encrypted, <literal>scram</> for a SCRAM-encrypted password + and <literal>plain</> for an unencrypted password. If the passwordCan we please stop calling this encryption? What is being done is a form
of cryptographic hashing, not encryption.
Yes, I agree with that for MD5, and after looking around I can see
(like here http://prosody.im/doc/plain_or_hashed) as well that
SCRAM-hashed is used. Now, there are as well references to the salt,
like in protocol.sgml:
"The salt to use when encrypting the password."
Joe, do you think that in this case using the term "hashing" would be
more appropriate? I would think so as we use it to hash the password.
The patch attached removes the parenthesis for this grammar, and uses
"hashed" instead of "encrypted" for the new documentation. For the
existing documentation, perhaps we had better just spawn a new thread,
but I am unsure of all the details yet. Opinions welcome.
--
Michael
Attachments:
0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patchapplication/octet-stream; name=0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patchDownload
From 257ed4673bb4e38340c1df9f55c97f5de8ad9838 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 9 Mar 2017 21:55:15 +0900
Subject: [PATCH] Add clause PASSWORD (val USING protocol) to CREATE/ALTER ROLE
This clause allows users to be able to enforce with which protocol
a given password is used with. if the value given is already encrypted,
the value is used as-is. This extension is useful to support future
protocols, particularly SCRAM-SHA-256.
---
doc/src/sgml/ref/alter_role.sgml | 10 +++++
doc/src/sgml/ref/create_role.sgml | 26 +++++++++++
src/backend/commands/user.c | 80 +++++++++++++++++++++++++++++++---
src/backend/parser/gram.y | 7 +++
src/test/regress/expected/password.out | 14 +++++-
src/test/regress/sql/password.sql | 9 ++++
6 files changed, 140 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index da36ad9696..257e3b96bf 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -34,6 +34,7 @@ ALTER ROLE <replaceable class="PARAMETER">role_specification</replaceable> [ WIT
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
| [ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
+ | PASSWORD '<replaceable class="PARAMETER">password</replaceable>' USING '<replaceable class="PARAMETER">method</replaceable>'
| VALID UNTIL '<replaceable class="PARAMETER">timestamp</replaceable>'
ALTER ROLE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>new_name</replaceable>
@@ -169,6 +170,7 @@ ALTER ROLE { <replaceable class="PARAMETER">role_specification</replaceable> | A
<term><literal>NOBYPASSRLS</literal></term>
<term><literal>CONNECTION LIMIT</literal> <replaceable class="parameter">connlimit</replaceable></term>
<term><literal>PASSWORD</> <replaceable class="parameter">password</replaceable></term>
+ <term><literal>PASSWORD</> '<replaceable class="parameter">password</replaceable>' USING '<replaceable class="parameter">method</replaceable>'</term>
<term><literal>ENCRYPTED</></term>
<term><literal>UNENCRYPTED</></term>
<term><literal>VALID UNTIL</literal> '<replaceable class="parameter">timestamp</replaceable>'</term>
@@ -280,6 +282,14 @@ ALTER ROLE davide WITH PASSWORD 'hu8jmn3';
</para>
<para>
+ Change a role's password using MD5 hash:
+
+<programlisting>
+ALTER ROLE lionel WITH PASSWORD 'hu8jmn3' USING 'md5';
+</programlisting>
+ </para>
+
+ <para>
Remove a role's password:
<programlisting>
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
index 99d1c8336c..54aa9c524b 100644
--- a/doc/src/sgml/ref/create_role.sgml
+++ b/doc/src/sgml/ref/create_role.sgml
@@ -34,6 +34,7 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
| [ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
+ | PASSWORD '<replaceable class="PARAMETER">password</replaceable>' USING '<replaceable class="PARAMETER">method</replaceable>'
| VALID UNTIL '<replaceable class="PARAMETER">timestamp</replaceable>'
| IN ROLE <replaceable class="PARAMETER">role_name</replaceable> [, ...]
| IN GROUP <replaceable class="PARAMETER">role_name</replaceable> [, ...]
@@ -247,6 +248,23 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
</varlistentry>
<varlistentry>
+ <term><literal>PASSWORD</> '<replaceable class="parameter">password</replaceable>' USING '<replaceable class="parameter">method</replaceable>'</term>
+ <listitem>
+ <para>
+ Sets the role's password using the requested method. (A password
+ is only of use for roles having the <literal>LOGIN</literal>
+ attribute, but you can nonetheless define one for roles without it.)
+ If you do not plan to use password authentication you can omit this
+ option. The methods supported are <literal>md5</> to enforce a password
+ to be MD5-hashed, <literal>scram</> for a SCRAM-hashed password
+ and <literal>plain</> for an non-hashed password. If the password
+ string is already in MD5-hashed or SCRAM-hashed, then it is
+ stored hashed as-is.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>VALID UNTIL</literal> '<replaceable class="parameter">timestamp</replaceable>'</term>
<listitem>
<para>
@@ -428,6 +446,14 @@ CREATE USER davide WITH PASSWORD 'jw8s0F4';
</para>
<para>
+ Create a role with a MD5-hashed password:
+
+<programlisting>
+CREATE USER lionel WITH PASSWORD 'asdh7as' USING 'md5';
+</programlisting>
+ </para>
+
+ <para>
Create a role with a password that is valid until the end of 2004.
After one second has ticked in 2005, the password is no longer
valid.
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 14b9779144..a8729f494d 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -130,7 +130,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
if (strcmp(defel->defname, "password") == 0 ||
strcmp(defel->defname, "encryptedPassword") == 0 ||
- strcmp(defel->defname, "unencryptedPassword") == 0)
+ strcmp(defel->defname, "unencryptedPassword") == 0 ||
+ strcmp(defel->defname, "methodPassword") == 0)
{
if (dpassword)
ereport(ERROR,
@@ -144,9 +145,45 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
password_type = PASSWORD_TYPE_SCRAM;
else
password_type = PASSWORD_TYPE_MD5;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
}
else if (strcmp(defel->defname, "unencryptedPassword") == 0)
+ {
password_type = PASSWORD_TYPE_PLAINTEXT;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
+ else if (strcmp(defel->defname, "methodPassword") == 0)
+ {
+ /*
+ * This is a list of two elements, the password is first and
+ * then there is the method wanted by caller.
+ */
+ if (dpassword && dpassword->arg)
+ {
+ char *method = strVal(lsecond((List *) dpassword->arg));
+
+ password = strVal(linitial((List *) dpassword->arg));
+
+ if (strcmp(method, "md5") == 0)
+ password_type = PASSWORD_TYPE_MD5;
+ else if (strcmp(method, "plain") == 0)
+ password_type = PASSWORD_TYPE_PLAINTEXT;
+ else if (strcmp(method, "scram") == 0)
+ password_type = PASSWORD_TYPE_SCRAM;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unsupported password method %s", method)));
+ }
+ }
+ else
+ {
+ password_type = Password_encryption;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
}
else if (strcmp(defel->defname, "sysid") == 0)
{
@@ -266,8 +303,6 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
defel->defname);
}
- if (dpassword && dpassword->arg)
- password = strVal(dpassword->arg);
if (dissuper)
issuper = intVal(dissuper->arg) != 0;
if (dinherit)
@@ -539,6 +574,7 @@ AlterRole(AlterRoleStmt *stmt)
if (strcmp(defel->defname, "password") == 0 ||
strcmp(defel->defname, "encryptedPassword") == 0 ||
+ strcmp(defel->defname, "methodPassword") == 0 ||
strcmp(defel->defname, "unencryptedPassword") == 0)
{
if (dpassword)
@@ -552,9 +588,45 @@ AlterRole(AlterRoleStmt *stmt)
password_type = PASSWORD_TYPE_SCRAM;
else
password_type = PASSWORD_TYPE_MD5;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
}
else if (strcmp(defel->defname, "unencryptedPassword") == 0)
+ {
password_type = PASSWORD_TYPE_PLAINTEXT;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
+ else if (strcmp(defel->defname, "methodPassword") == 0)
+ {
+ /*
+ * This is a list of two elements, the password is first and
+ * then there is the method wanted by caller.
+ */
+ if (dpassword && dpassword->arg)
+ {
+ char *method = strVal(lsecond((List *) dpassword->arg));
+
+ if (strcmp(method, "md5") == 0)
+ password_type = PASSWORD_TYPE_MD5;
+ else if (strcmp(method, "plain") == 0)
+ password_type = PASSWORD_TYPE_PLAINTEXT;
+ else if (strcmp(method, "scram") == 0)
+ password_type = PASSWORD_TYPE_SCRAM;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unsupported password method %s", method)));
+
+ password = strVal(linitial((List *) dpassword->arg));
+ }
+ }
+ else
+ {
+ password_type = Password_encryption;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
}
else if (strcmp(defel->defname, "superuser") == 0)
{
@@ -642,8 +714,6 @@ AlterRole(AlterRoleStmt *stmt)
defel->defname);
}
- if (dpassword && dpassword->arg)
- password = strVal(dpassword->arg);
if (dissuper)
issuper = intVal(dissuper->arg);
if (dinherit)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e7acc2d9a2..fbe5ba35d6 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -988,6 +988,13 @@ AlterOptRoleElem:
{
$$ = makeDefElem("password", NULL, @1);
}
+ | PASSWORD Sconst USING Sconst
+ {
+ $$ = makeDefElem("methodPassword",
+ (Node *)list_make2(makeString($2),
+ makeString($4)),
+ @1);
+ }
| ENCRYPTED PASSWORD Sconst
{
$$ = makeDefElem("encryptedPassword",
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index c503e43abe..7f18f35eb1 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -63,6 +63,12 @@ ALTER ROLE regress_passwd4 ENCRYPTED PASSWORD 'scram-sha-256:VLK4RMaQLCvNtQ==:40
SET password_encryption = 'scram';
ALTER ROLE regress_passwd5 ENCRYPTED PASSWORD 'foo'; -- create SCRAM verifier
CREATE ROLE regress_passwd6 ENCRYPTED PASSWORD 'md53725413363ab045e20521bf36b8d8d7f'; -- encrypted with MD5, use as it is
+-- PASSWORD 'value' USING 'method'
+CREATE ROLE regress_passwd7 PASSWORD 'role_pwd7' USING 'plain';
+CREATE ROLE regress_passwd8 PASSWORD 'role_pwd8' USING 'md5';
+CREATE ROLE regress_passwd9 PASSWORD 'role_pwd9' USING 'scram';
+CREATE ROLE regress_passwd10 PASSWORD 'role_pwd10' USING 'novalue'; --error
+ERROR: unsupported password method novalue
SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
FROM pg_authid
WHERE rolname LIKE 'regress_passwd%'
@@ -75,7 +81,10 @@ SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):
regress_passwd4 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
regress_passwd5 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
regress_passwd6 | md53725413363ab045e20521bf36b8d8d7f
-(6 rows)
+ regress_passwd7 | role_pwd7
+ regress_passwd8 | md571f6e76fa74cf5716b886f35ad945663
+ regress_passwd9 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
+(9 rows)
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
@@ -83,6 +92,9 @@ DROP ROLE regress_passwd3;
DROP ROLE regress_passwd4;
DROP ROLE regress_passwd5;
DROP ROLE regress_passwd6;
+DROP ROLE regress_passwd7;
+DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
-- all entries should have been removed
SELECT rolname, rolpassword
FROM pg_authid
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index f4b3a9ac3a..011be018ba 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -54,6 +54,12 @@ SET password_encryption = 'scram';
ALTER ROLE regress_passwd5 ENCRYPTED PASSWORD 'foo'; -- create SCRAM verifier
CREATE ROLE regress_passwd6 ENCRYPTED PASSWORD 'md53725413363ab045e20521bf36b8d8d7f'; -- encrypted with MD5, use as it is
+-- PASSWORD 'value' USING 'method'
+CREATE ROLE regress_passwd7 PASSWORD 'role_pwd7' USING 'plain';
+CREATE ROLE regress_passwd8 PASSWORD 'role_pwd8' USING 'md5';
+CREATE ROLE regress_passwd9 PASSWORD 'role_pwd9' USING 'scram';
+CREATE ROLE regress_passwd10 PASSWORD 'role_pwd10' USING 'novalue'; --error
+
SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
FROM pg_authid
WHERE rolname LIKE 'regress_passwd%'
@@ -65,6 +71,9 @@ DROP ROLE regress_passwd3;
DROP ROLE regress_passwd4;
DROP ROLE regress_passwd5;
DROP ROLE regress_passwd6;
+DROP ROLE regress_passwd7;
+DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
-- all entries should have been removed
SELECT rolname, rolpassword
--
2.12.0
On Thu, Mar 9, 2017 at 7:59 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
Yes, I agree with that for MD5, and after looking around I can see
(like here http://prosody.im/doc/plain_or_hashed) as well that
SCRAM-hashed is used. Now, there are as well references to the salt,
like in protocol.sgml:
"The salt to use when encrypting the password."
Joe, do you think that in this case using the term "hashing" would be
more appropriate? I would think so as we use it to hash the password.
I'm not Joe, but I think that would be more appropriate.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 03/09/2017 06:34 AM, Robert Haas wrote:
On Thu, Mar 9, 2017 at 7:59 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:Yes, I agree with that for MD5, and after looking around I can see
(like here http://prosody.im/doc/plain_or_hashed) as well that
SCRAM-hashed is used. Now, there are as well references to the salt,
like in protocol.sgml:
"The salt to use when encrypting the password."
Joe, do you think that in this case using the term "hashing" would be
more appropriate? I would think so as we use it to hash the password.I'm not Joe, but I think that would be more appropriate.
I am Joe and I agree ;-)
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
On Fri, Mar 10, 2017 at 12:00 AM, Joe Conway <mail@joeconway.com> wrote:
On 03/09/2017 06:34 AM, Robert Haas wrote:
On Thu, Mar 9, 2017 at 7:59 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:Yes, I agree with that for MD5, and after looking around I can see
(like here http://prosody.im/doc/plain_or_hashed) as well that
SCRAM-hashed is used. Now, there are as well references to the salt,
like in protocol.sgml:
"The salt to use when encrypting the password."
Joe, do you think that in this case using the term "hashing" would be
more appropriate? I would think so as we use it to hash the password.I'm not Joe, but I think that would be more appropriate.
I am Joe and I agree ;-)
OK I'll spawn a new thread on the matter.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:
On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway <mail@joeconway.com> wrote:
On 03/07/2017 08:29 PM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
+1
Seeing 3 opinions in favor of that, let's do so then. I have updated
the patch to not use parenthesis.
The regression tests only exercise the CREATE ROLE...USING version, not the
ALTER ROLE...USING version.
+ and <literal>plain</> for an non-hashed password. If the password
+ string is already in MD5-hashed or SCRAM-hashed, then it is
+ stored hashed as-is.In the last line, I think "stored as-is" sounds better.
Other than that, it looks good to me.
Cheers,
Jeff
On Fri, Mar 17, 2017 at 2:30 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway <mail@joeconway.com> wrote:
On 03/07/2017 08:29 PM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
+1
Seeing 3 opinions in favor of that, let's do so then. I have updated
the patch to not use parenthesis.The regression tests only exercise the CREATE ROLE...USING version, not the
ALTER ROLE...USING version.
Done.
+ and <literal>plain</> for an non-hashed password. If the password + string is already in MD5-hashed or SCRAM-hashed, then it is + stored hashed as-is.In the last line, I think "stored as-is" sounds better.
Okay.
Other than that, it looks good to me.
Thanks for the review. Attached is an updated patch.
--
Michael
Attachments:
0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patchapplication/octet-stream; name=0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patchDownload
From 520cad75a33e67c52c4db3f15f0b42dca139833c Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Fri, 17 Mar 2017 10:09:50 +0900
Subject: [PATCH] Add clause PASSWORD (val USING protocol) to CREATE/ALTER ROLE
This clause allows users to be able to enforce with which protocol
a given password is used with. if the value given is already encrypted,
the value is used as-is. This extension is useful to support future
protocols, particularly SCRAM-SHA-256.
---
doc/src/sgml/ref/alter_role.sgml | 10 +++++
doc/src/sgml/ref/create_role.sgml | 26 +++++++++++
src/backend/commands/user.c | 80 +++++++++++++++++++++++++++++++---
src/backend/parser/gram.y | 7 +++
src/test/regress/expected/password.out | 34 ++++++++++++++-
src/test/regress/sql/password.sql | 15 +++++++
6 files changed, 166 insertions(+), 6 deletions(-)
diff --git a/doc/src/sgml/ref/alter_role.sgml b/doc/src/sgml/ref/alter_role.sgml
index da36ad9696..257e3b96bf 100644
--- a/doc/src/sgml/ref/alter_role.sgml
+++ b/doc/src/sgml/ref/alter_role.sgml
@@ -34,6 +34,7 @@ ALTER ROLE <replaceable class="PARAMETER">role_specification</replaceable> [ WIT
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
| [ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
+ | PASSWORD '<replaceable class="PARAMETER">password</replaceable>' USING '<replaceable class="PARAMETER">method</replaceable>'
| VALID UNTIL '<replaceable class="PARAMETER">timestamp</replaceable>'
ALTER ROLE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>new_name</replaceable>
@@ -169,6 +170,7 @@ ALTER ROLE { <replaceable class="PARAMETER">role_specification</replaceable> | A
<term><literal>NOBYPASSRLS</literal></term>
<term><literal>CONNECTION LIMIT</literal> <replaceable class="parameter">connlimit</replaceable></term>
<term><literal>PASSWORD</> <replaceable class="parameter">password</replaceable></term>
+ <term><literal>PASSWORD</> '<replaceable class="parameter">password</replaceable>' USING '<replaceable class="parameter">method</replaceable>'</term>
<term><literal>ENCRYPTED</></term>
<term><literal>UNENCRYPTED</></term>
<term><literal>VALID UNTIL</literal> '<replaceable class="parameter">timestamp</replaceable>'</term>
@@ -280,6 +282,14 @@ ALTER ROLE davide WITH PASSWORD 'hu8jmn3';
</para>
<para>
+ Change a role's password using MD5 hash:
+
+<programlisting>
+ALTER ROLE lionel WITH PASSWORD 'hu8jmn3' USING 'md5';
+</programlisting>
+ </para>
+
+ <para>
Remove a role's password:
<programlisting>
diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml
index 99d1c8336c..77a9539442 100644
--- a/doc/src/sgml/ref/create_role.sgml
+++ b/doc/src/sgml/ref/create_role.sgml
@@ -34,6 +34,7 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
| [ ENCRYPTED | UNENCRYPTED ] PASSWORD '<replaceable class="PARAMETER">password</replaceable>'
+ | PASSWORD '<replaceable class="PARAMETER">password</replaceable>' USING '<replaceable class="PARAMETER">method</replaceable>'
| VALID UNTIL '<replaceable class="PARAMETER">timestamp</replaceable>'
| IN ROLE <replaceable class="PARAMETER">role_name</replaceable> [, ...]
| IN GROUP <replaceable class="PARAMETER">role_name</replaceable> [, ...]
@@ -247,6 +248,23 @@ CREATE ROLE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <replac
</varlistentry>
<varlistentry>
+ <term><literal>PASSWORD</> '<replaceable class="parameter">password</replaceable>' USING '<replaceable class="parameter">method</replaceable>'</term>
+ <listitem>
+ <para>
+ Sets the role's password using the requested method. (A password
+ is only of use for roles having the <literal>LOGIN</literal>
+ attribute, but you can nonetheless define one for roles without it.)
+ If you do not plan to use password authentication you can omit this
+ option. The methods supported are <literal>md5</> to enforce a password
+ to be MD5-hashed, <literal>scram</> for a SCRAM-hashed password
+ and <literal>plain</> for an non-hashed password. If the password
+ string is already in MD5-hashed or SCRAM-hashed, then it is
+ stored as-is.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>VALID UNTIL</literal> '<replaceable class="parameter">timestamp</replaceable>'</term>
<listitem>
<para>
@@ -428,6 +446,14 @@ CREATE USER davide WITH PASSWORD 'jw8s0F4';
</para>
<para>
+ Create a role with a MD5-hashed password:
+
+<programlisting>
+CREATE USER lionel WITH PASSWORD 'asdh7as' USING 'md5';
+</programlisting>
+ </para>
+
+ <para>
Create a role with a password that is valid until the end of 2004.
After one second has ticked in 2005, the password is no longer
valid.
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index 14b9779144..a8729f494d 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -130,7 +130,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
if (strcmp(defel->defname, "password") == 0 ||
strcmp(defel->defname, "encryptedPassword") == 0 ||
- strcmp(defel->defname, "unencryptedPassword") == 0)
+ strcmp(defel->defname, "unencryptedPassword") == 0 ||
+ strcmp(defel->defname, "methodPassword") == 0)
{
if (dpassword)
ereport(ERROR,
@@ -144,9 +145,45 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
password_type = PASSWORD_TYPE_SCRAM;
else
password_type = PASSWORD_TYPE_MD5;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
}
else if (strcmp(defel->defname, "unencryptedPassword") == 0)
+ {
password_type = PASSWORD_TYPE_PLAINTEXT;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
+ else if (strcmp(defel->defname, "methodPassword") == 0)
+ {
+ /*
+ * This is a list of two elements, the password is first and
+ * then there is the method wanted by caller.
+ */
+ if (dpassword && dpassword->arg)
+ {
+ char *method = strVal(lsecond((List *) dpassword->arg));
+
+ password = strVal(linitial((List *) dpassword->arg));
+
+ if (strcmp(method, "md5") == 0)
+ password_type = PASSWORD_TYPE_MD5;
+ else if (strcmp(method, "plain") == 0)
+ password_type = PASSWORD_TYPE_PLAINTEXT;
+ else if (strcmp(method, "scram") == 0)
+ password_type = PASSWORD_TYPE_SCRAM;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unsupported password method %s", method)));
+ }
+ }
+ else
+ {
+ password_type = Password_encryption;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
}
else if (strcmp(defel->defname, "sysid") == 0)
{
@@ -266,8 +303,6 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
defel->defname);
}
- if (dpassword && dpassword->arg)
- password = strVal(dpassword->arg);
if (dissuper)
issuper = intVal(dissuper->arg) != 0;
if (dinherit)
@@ -539,6 +574,7 @@ AlterRole(AlterRoleStmt *stmt)
if (strcmp(defel->defname, "password") == 0 ||
strcmp(defel->defname, "encryptedPassword") == 0 ||
+ strcmp(defel->defname, "methodPassword") == 0 ||
strcmp(defel->defname, "unencryptedPassword") == 0)
{
if (dpassword)
@@ -552,9 +588,45 @@ AlterRole(AlterRoleStmt *stmt)
password_type = PASSWORD_TYPE_SCRAM;
else
password_type = PASSWORD_TYPE_MD5;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
}
else if (strcmp(defel->defname, "unencryptedPassword") == 0)
+ {
password_type = PASSWORD_TYPE_PLAINTEXT;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
+ else if (strcmp(defel->defname, "methodPassword") == 0)
+ {
+ /*
+ * This is a list of two elements, the password is first and
+ * then there is the method wanted by caller.
+ */
+ if (dpassword && dpassword->arg)
+ {
+ char *method = strVal(lsecond((List *) dpassword->arg));
+
+ if (strcmp(method, "md5") == 0)
+ password_type = PASSWORD_TYPE_MD5;
+ else if (strcmp(method, "plain") == 0)
+ password_type = PASSWORD_TYPE_PLAINTEXT;
+ else if (strcmp(method, "scram") == 0)
+ password_type = PASSWORD_TYPE_SCRAM;
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("unsupported password method %s", method)));
+
+ password = strVal(linitial((List *) dpassword->arg));
+ }
+ }
+ else
+ {
+ password_type = Password_encryption;
+ if (dpassword && dpassword->arg)
+ password = strVal(dpassword->arg);
+ }
}
else if (strcmp(defel->defname, "superuser") == 0)
{
@@ -642,8 +714,6 @@ AlterRole(AlterRoleStmt *stmt)
defel->defname);
}
- if (dpassword && dpassword->arg)
- password = strVal(dpassword->arg);
if (dissuper)
issuper = intVal(dissuper->arg);
if (dinherit)
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 6316688a88..3b42e9129a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -988,6 +988,13 @@ AlterOptRoleElem:
{
$$ = makeDefElem("password", NULL, @1);
}
+ | PASSWORD Sconst USING Sconst
+ {
+ $$ = makeDefElem("methodPassword",
+ (Node *)list_make2(makeString($2),
+ makeString($4)),
+ @1);
+ }
| ENCRYPTED PASSWORD Sconst
{
$$ = makeDefElem("encryptedPassword",
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index c503e43abe..eb88f420cc 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -63,6 +63,12 @@ ALTER ROLE regress_passwd4 ENCRYPTED PASSWORD 'scram-sha-256:VLK4RMaQLCvNtQ==:40
SET password_encryption = 'scram';
ALTER ROLE regress_passwd5 ENCRYPTED PASSWORD 'foo'; -- create SCRAM verifier
CREATE ROLE regress_passwd6 ENCRYPTED PASSWORD 'md53725413363ab045e20521bf36b8d8d7f'; -- encrypted with MD5, use as it is
+-- PASSWORD 'value' USING 'method'
+CREATE ROLE regress_passwd7 PASSWORD 'role_pwd7' USING 'plain';
+CREATE ROLE regress_passwd8 PASSWORD 'role_pwd8' USING 'md5';
+CREATE ROLE regress_passwd9 PASSWORD 'role_pwd9' USING 'scram';
+CREATE ROLE regress_passwd10 PASSWORD 'role_pwd10' USING 'novalue'; --error
+ERROR: unsupported password method novalue
SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
FROM pg_authid
WHERE rolname LIKE 'regress_passwd%'
@@ -75,7 +81,30 @@ SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):
regress_passwd4 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
regress_passwd5 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
regress_passwd6 | md53725413363ab045e20521bf36b8d8d7f
-(6 rows)
+ regress_passwd7 | role_pwd7
+ regress_passwd8 | md571f6e76fa74cf5716b886f35ad945663
+ regress_passwd9 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
+(9 rows)
+
+ALTER ROLE regress_passwd7 PASSWORD 'role_pwd7' USING 'plain';
+ALTER ROLE regress_passwd8 PASSWORD 'role_pwd8' USING 'md5';
+ALTER ROLE regress_passwd9 PASSWORD 'role_pwd9' USING 'scram';
+SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
+ FROM pg_authid
+ WHERE rolname LIKE 'regress_passwd%'
+ ORDER BY rolname, rolpassword;
+ rolname | rolpassword_masked
+-----------------+---------------------------------------------------
+ regress_passwd1 | foo
+ regress_passwd2 | md5dfa155cadd5f4ad57860162f3fab9cdb
+ regress_passwd3 | md5530de4c298af94b3b9f7d20305d2a1bf
+ regress_passwd4 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
+ regress_passwd5 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
+ regress_passwd6 | md53725413363ab045e20521bf36b8d8d7f
+ regress_passwd7 | role_pwd7
+ regress_passwd8 | md571f6e76fa74cf5716b886f35ad945663
+ regress_passwd9 | scram-sha-256:<salt>:4096:<storedkey>:<serverkey>
+(9 rows)
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
@@ -83,6 +112,9 @@ DROP ROLE regress_passwd3;
DROP ROLE regress_passwd4;
DROP ROLE regress_passwd5;
DROP ROLE regress_passwd6;
+DROP ROLE regress_passwd7;
+DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
-- all entries should have been removed
SELECT rolname, rolpassword
FROM pg_authid
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index f4b3a9ac3a..6210552234 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -54,6 +54,18 @@ SET password_encryption = 'scram';
ALTER ROLE regress_passwd5 ENCRYPTED PASSWORD 'foo'; -- create SCRAM verifier
CREATE ROLE regress_passwd6 ENCRYPTED PASSWORD 'md53725413363ab045e20521bf36b8d8d7f'; -- encrypted with MD5, use as it is
+-- PASSWORD 'value' USING 'method'
+CREATE ROLE regress_passwd7 PASSWORD 'role_pwd7' USING 'plain';
+CREATE ROLE regress_passwd8 PASSWORD 'role_pwd8' USING 'md5';
+CREATE ROLE regress_passwd9 PASSWORD 'role_pwd9' USING 'scram';
+CREATE ROLE regress_passwd10 PASSWORD 'role_pwd10' USING 'novalue'; --error
+SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
+ FROM pg_authid
+ WHERE rolname LIKE 'regress_passwd%'
+ ORDER BY rolname, rolpassword;
+ALTER ROLE regress_passwd7 PASSWORD 'role_pwd7' USING 'plain';
+ALTER ROLE regress_passwd8 PASSWORD 'role_pwd8' USING 'md5';
+ALTER ROLE regress_passwd9 PASSWORD 'role_pwd9' USING 'scram';
SELECT rolname, regexp_replace(rolpassword, '(scram-sha-256):([a-zA-Z0-9+/]+==):(\d+):(\w+):(\w+)', '\1:<salt>:\3:<storedkey>:<serverkey>') as rolpassword_masked
FROM pg_authid
WHERE rolname LIKE 'regress_passwd%'
@@ -65,6 +77,9 @@ DROP ROLE regress_passwd3;
DROP ROLE regress_passwd4;
DROP ROLE regress_passwd5;
DROP ROLE regress_passwd6;
+DROP ROLE regress_passwd7;
+DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
-- all entries should have been removed
SELECT rolname, rolpassword
--
2.12.0
On Fri, Mar 17, 2017 at 10:10:59AM +0900, Michael Paquier wrote:
On Fri, Mar 17, 2017 at 2:30 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway <mail@joeconway.com> wrote:
On 03/07/2017 08:29 PM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
+1
Seeing 3 opinions in favor of that, let's do so then. I have updated
the patch to not use parenthesis.The regression tests only exercise the CREATE ROLE...USING version, not the
ALTER ROLE...USING version.Done.
+ and <literal>plain</> for an non-hashed password. If the password + string is already in MD5-hashed or SCRAM-hashed, then it is + stored hashed as-is.In the last line, I think "stored as-is" sounds better.
Okay.
Other than that, it looks good to me.
Thanks for the review. Attached is an updated patch.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Heikki,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 04/06/2017 08:33 AM, Noah Misch wrote:
On Fri, Mar 17, 2017 at 10:10:59AM +0900, Michael Paquier wrote:
On Fri, Mar 17, 2017 at 2:30 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway <mail@joeconway.com> wrote:
On 03/07/2017 08:29 PM, Tom Lane wrote:
Michael Paquier <michael.paquier@gmail.com> writes:
here is a separate thread dedicated to the following extension for
CREATE/ALTER ROLE: PASSWORD ('value' USING 'method').The parentheses seem weird ... do we really need those?
+1
Seeing 3 opinions in favor of that, let's do so then. I have updated
the patch to not use parenthesis.The regression tests only exercise the CREATE ROLE...USING version, not the
ALTER ROLE...USING version.Done.
+ and <literal>plain</> for an non-hashed password. If the password + string is already in MD5-hashed or SCRAM-hashed, then it is + stored hashed as-is.In the last line, I think "stored as-is" sounds better.
Okay.
Other than that, it looks good to me.
Thanks for the review. Attached is an updated patch.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item.
This isn't critical, SCRAM is fully functional without this new syntax.
I think we should drop this, and revisit for Postgres 11, if it feels
like we still want this then. I'll remove this from the open items list.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers