Subject: Changing references of password encryption to hashing

From: Michael Paquier <[email-masked]>
Date: Fri, 10 Mar 2017 11:16:02 +0900

Hi all,

As discussed here:
https://www.postgresql.org/message-id/[email-masked]
We are using in documentation and code comments "encryption" to define
what actually is hashing, which is confusing.

Attached is a patch for HEAD to change the documentation to match hashing.

There are a couple of things I noticed on the way:
1) There is the user-visible PQencryptPassword in libpq, which
actually hashes the password, and not encrypts it :)
2) There is as well pg_md5_encrypt() in the code, as well as there is
pg_md5_hash(). Those may be better if renamed, at least I would think
that pg_md5_encrypt should be pg_md5_password_hash, because a salt is
used as well, and the routine is dedicated to work on passwords.
Thoughts?
3) createuser also has --encrypt and --unencrypted, perhaps those
should be renamed? Honestly I don't really think that this is worth a
breakage and the option names match with the SQL commands.

I did not bother about those things in the attached, which works only
documentation and comment changes.

An open item has been added on the wiki.

Thanks,
-- 
Michael

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

---

From: Joe Conway <[email-masked]>
Date: Sat, 11 Mar 2017 14:51:52 -0800

> breakage and the option names match with the SQL commands.

My opinion is that the user visible aspects of this should be deprecated
and correct syntax provided. But perhaps that is overkill.

8<------------
    key and server key, respectively, in hexadecimal format. A password that
-   does not follow either of those formats is assumed to be unencrypted.
+   does not follow either of those formats is assumed to be in plain
format,
+   non-hashed.
   </para>
  </sect1>
8<------------
I think here, instead of "in plain format, non-hashed" it is ok to just
say "cleartext" or maybe "plaintext". Whichever is picked, it should be
used consistently.

8<------------
  <caution>
   <para>
-   To prevent unencrypted passwords from being sent across the network,
+   To prevent non-hashed passwords from being sent across the network,
8<------------
same here "non-hashed" ==> "cleartext"

8<------------
   <para>
-   Caution must be exercised when specifying an unencrypted password
+   Caution must be exercised when specifying a non-hashed password
8<------------
and here

...etc...

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

---

From: Michael Paquier <[email-masked]>
Date: Mon, 13 Mar 2017 16:11:47 +0900

> and correct syntax provided. But perhaps that is overkill.

Are ENCRYPTED and UNENCRYPTED keywords part of the SQL specification?
If yes, we had better not do 3) then. 1) and 2) would make future
back-patching harder, so I am not going to push much as I am expecting
some growling on the matter, and a doc-only patch is a step in the
good direction anyway. Extra opinions are welcome.

OK, thanks. cleartext is present in the docs already, so I have gone
for this term. Updated patch attached.
-- 
Michael

---

From: Craig Ringer <[email-masked]>
Date: Mon, 13 Mar 2017 16:48:21 +0800

> and correct syntax provided. But perhaps that is overkill.

FWIW, in my experience, pretty much nobody understands the pretty
tangled behaviour of "WITH [ENCRYPTED] PASSWORD", you have to
understand the fact table of:

* ENCRYPTED, UNENCRYPTED or neither set
* password_encryption GUC on or off
* password begins / doesn't begin with fixed string 'md5'

to fully know what will happen.

Then of course, you have to understand how all this interacts with
pg_hba.conf's 'password' and 'md5' options.

It's a right mess. Since our catalogs don't keep track of the hash
separately to the password text and use prefixes instead, and since we
need compatibility for dumps, it's hard to do a great deal about
though.

I'm not convinced that a keyword change will do much good, the whole
thing really needs a reassessment to make sure that it's clearer to
users/admins and has fewer moving parts.

So I'm in favour of fixing the docs but I'm not keen on changing the
SQL syntax in a way that just kind of papers over part of the
problems.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

---

From: Bruce Momjian <[email-masked]>
Date: Wed, 15 Mar 2017 23:19:32 -0400

> need compatibility for dumps, it's hard to do a great deal about

With SCRAM coming in PG 10, is there anything we can do to clean this up
for PG 10?

-- 
  Bruce Momjian  <[email-masked]>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

---

From: Robert Haas <[email-masked]>
Date: Thu, 16 Mar 2017 09:19:02 -0400

> SQL syntax in a way that just kind of papers over part of the

I agree.  I think that trying to design new SQL syntax at this point
is unlikely to be a good idea - we're just about out of time here, and
some people who might care about this are busy on other things, and
the deadline for patches that do new things has long since passed.
But I like the idea of trying to improve the documentation.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Joe Conway <[email-masked]>
Date: Thu, 16 Mar 2017 07:27:11 -0700

> But I like the idea of trying to improve the documentation.

Agreed. I think the documentation fixes definitely should be done, but
understand that the grammar is a longer term issue with backward
compatibility implications. Acknowledging the problem is the first step ;-)

Joe

---

From: Stephen Frost <[email-masked]>
Date: Thu, 16 Mar 2017 10:30:37 -0400

> compatibility implications. Acknowledging the problem is the first step ;-)

+1

Thanks!

Stephen

---

From: Noah Misch <[email-masked]>
Date: Thu, 6 Apr 2017 05:31:07 +0000

> > But I like the idea of trying to improve the documentation.

Seems like a good direction.

> compatibility implications. Acknowledging the problem is the first step ;-)

Most of the user-visible (including doc/) proposed changes alter material
predating v10.  Therefore, I've removed this from open item status.