Remove psql's -W option

On 21/07/18 23:58, David Fetter wrote:

Folks,

I'd like to $Subject for 12.

There are scripts it could break, but not ones that weren't already
broken in ways important to access control.

What say?

I say it should at least throw a sensible error for a few versions
before it's removed completely.

-1 on this patch
+1 for removing the "feature"
-- 
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

Hello David,

I'd like to $Subject for 12.

There are scripts it could break, but not ones that weren't already
broken in ways important to access control.

What say?

What is the rational?

I'm unsure of the logic behind removing -W (--password) but keeping -w
(--no-password), especially as the internal logic seems kept by the patch.

--
Fabien.

On 22/07/18 00:41, Fabien COELHO wrote:

Hello David,

I'd like to $Subject for 12.

There are scripts it could break, but not ones that weren't already
broken in ways important to access control.

What say?

What is the rational?

It's first on our list of things not to do:
https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_psql_-W_or_--password
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Vik Fearing <vik.fearing@2ndquadrant.com> writes:

On 22/07/18 00:41, Fabien COELHO wrote:

What is the rational?

It's first on our list of things not to do:
https://wiki.postgresql.org/wiki/Don&#39;t_Do_This#Don.27t_use_psql_-W_or_--password

As I recall, when this has been discussed in the past, people objected
because they didn't like either (1) the extra server process fork and/or
network round trip caused when a password is needed, or (2) the server
log entry that gets generated about client disconnecting without supplying
a password. (We don't log anything about it normally, but I'm not sure
that that's always true when using PAM, LDAP, connection poolers, etc.)
While those are surely niche concerns, it's not really apparent to me
what we gain by breaking them.

A possible positive reason for removing the option would be if we could
clean up this mess:
/messages/by-id/E1egDgC-000302-FN@gemulon.postgresql.org
But no fair citing that argument without presenting an actual clean-up
patch, because it's not obvious how much cleaner we could make it.

BTW, all of our client programs have this switch, so if we did agree
to remove it, this patch doesn't go nearly far enough.

regards, tom lane

PS: I found some interesting back-story here:
/messages/by-id/200712091148.54294.xzilla@users.sourceforge.net

It's first on our list of things not to do:
https://wiki.postgresql.org/wiki/Don&#39;t_Do_This#Don.27t_use_psql_-W_or_--password

As I recall, when this has been discussed in the past, people objected
because they didn't like either (1) the extra server process fork and/or
network round trip caused when a password is needed,

Looking at the protocol documentation, I cannot see why a fork would be
needed, because the password could be asked for when required at the
protocol level. If the client knew.

However, libpq does not seem to expose this logic, and/or it is not used
by "psql" which simply loops over "PQconnectdbParams", which seems to
reconnect from scratch each time (connectDBStart ends with
pqDropConnection).

Given the depth of function (PQconnectStartParams, connectDBStart,
PQconnectPool, pg_fe_sendauth, pg_password_sendauth), changing this
behavior without re-designing the whole connection functions and rewriting
the client logic, thus breaking compatibility, looks like a pain.

A possible positive reason for removing the option would be if we could
clean up this mess:
/messages/by-id/E1egDgC-000302-FN@gemulon.postgresql.org
But no fair citing that argument without presenting an actual clean-up
patch, because it's not obvious how much cleaner we could make it.

Yep.

ISTM that the only way out would be to provide a callback function that
could be used to ask for the password, and that could be called probably
from pg_fe_sendauth (?) and implement the logic currently in psql main,
and probably other clients as well.

PQsetPasswordCallback(myfunction);

And nothing else would be changed at the client level.

However the compatibility is non trivial because of the link dependency.
Maybe there could be a define so that a client could be compatible with
older lib versions, eg:

#ifdef LIBPQ_HAS_SET_PASSWORD_CALLBACK
PQsetPasswordCallback(myfunction);
#endif

Possibly this is acceptable. Not sure.

Otherwise ISTM that "-W/--password" still has some minimal value thus does
not deserve to be thrown out that quickly.

--
Fabien

On Sun, Jul 22, 2018 at 9:35 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Otherwise ISTM that "-W/--password" still has some minimal value thus does
not deserve to be thrown out that quickly.

I think I agree. I don't think this option is really hurting
anything, so I'm not quite sure why we would want to abruptly get rid
of it.

I also think your other question is a good one. It seems like the
fact that we need to reconnect -- rather than just prompting for the
password and then sending it when we get it -- is an artifact of how
libpq is designed rather than an intrinsic limitation of the protocol.

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

On Mon, Jul 23, 2018 at 11:20:46AM -0400, Robert Haas wrote:

On Sun, Jul 22, 2018 at 9:35 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:

Otherwise ISTM that "-W/--password" still has some minimal value thus does
not deserve to be thrown out that quickly.

I think I agree. I don't think this option is really hurting
anything, so I'm not quite sure why we would want to abruptly get rid
of it.

I also think your other question is a good one. It seems like the
fact that we need to reconnect -- rather than just prompting for the
password and then sending it when we get it -- is an artifact of how
libpq is designed rather than an intrinsic limitation of the protocol.

Am I understanding correctly that doing the following would be
acceptable, assuming good code quality?

- Rearrange libpq so it doesn't force this behavior.
- Deprecate the -W option uniformly in the code we ship by documenting
it and making it send warnings to stderr.

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

Robert Haas <robertmhaas@gmail.com> writes:

I also think your other question is a good one. It seems like the
fact that we need to reconnect -- rather than just prompting for the
password and then sending it when we get it -- is an artifact of how
libpq is designed rather than an intrinsic limitation of the protocol.

Well, it's a limitation of the libpq API. The problem is that it's the
application, not libpq, that's in charge of actually asking the user for
a password. Right now we inform the app that it needs to do that by
passing back a failed PGconn with appropriate state. We could imagine
passing back a PGconn with a half-finished open connection, and asking
the app to re-submit that PGconn along with a password so we could
continue the auth handshake. But it'd require changing apps to do that.

Also, doing things like that would incur the risk of exceeding
authentication_timeout while the user is typing his password. So we'd
also need some additional complexity to retry in that situation.

regards, tom lane