Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Heikki Linnakangas]

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  wrote:

On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier 
wrote:


On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway  wrote:

On 03/07/2017 08:29 PM, Tom Lane wrote:

Michael Paquier  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 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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Noah Misch]

On Fri, Mar 17, 2017 at 10:10:59AM +0900, Michael Paquier wrote:
> On Fri, Mar 17, 2017 at 2:30 AM, Jeff Janes  wrote:
> > On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier 
> > wrote:
> >>
> >> On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway  wrote:
> >> > On 03/07/2017 08:29 PM, Tom Lane wrote:
> >> >> Michael Paquier  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 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] 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] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Michael Paquier]

On Fri, Mar 17, 2017 at 2:30 AM, Jeff Janes  wrote:
> On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier 
> wrote:
>>
>> On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway  wrote:
>> > On 03/07/2017 08:29 PM, Tom Lane wrote:
>> >> Michael Paquier  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 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


0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Jeff Janes]

On Thu, Mar 9, 2017 at 4:59 AM, Michael Paquier 
wrote:

> On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway  wrote:
> > On 03/07/2017 08:29 PM, Tom Lane wrote:
> >> Michael Paquier  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 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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Michael Paquier]

On Fri, Mar 10, 2017 at 12:00 AM, Joe Conway  wrote:
> On 03/09/2017 06:34 AM, Robert Haas wrote:
>> On Thu, Mar 9, 2017 at 7:59 AM, Michael Paquier
>>  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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Joe Conway]

On 03/09/2017 06:34 AM, Robert Haas wrote:
> On Thu, Mar 9, 2017 at 7:59 AM, Michael Paquier
>  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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Robert Haas]

On Thu, Mar 9, 2017 at 7:59 AM, Michael Paquier
 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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Michael Paquier]

On Thu, Mar 9, 2017 at 1:17 AM, Joe Conway  wrote:
> On 03/07/2017 08:29 PM, Tom Lane wrote:
>> Michael Paquier  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 md5 to enforce a 
>> password
>> +to be MD5-encrypted, scram for a SCRAM-encrypted 
>> password
>> +and 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.

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


0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Joe Conway]

On 03/07/2017 08:29 PM, Tom Lane wrote:
> Michael Paquier  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 md5 to enforce a 
> password
> +to be MD5-encrypted, scram for a SCRAM-encrypted password
> +and 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



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Michael Paquier]

On Wed, Mar 8, 2017 at 2:32 PM, Rushabh Lathia  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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Rushabh Lathia]

On Wed, Mar 8, 2017 at 9:59 AM, Tom Lane  wrote:

> Michael Paquier  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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Michael Paquier]

On Wed, Mar 8, 2017 at 1:29 PM, Tom Lane  wrote:
> Michael Paquier  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

Re: [HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Tom Lane]

Michael Paquier  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

[HACKERS] CREATE/ALTER ROLE PASSWORD ('value' USING 'method')

[Michael Paquier]

Hi all,

As discussed on the thread dedicated to SCRAM
(https://www.postgresql.org/message-id/243d8c11-6149-a4bb-0909-136992f74...@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


0001-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers