Thread: Remove psql's -W option
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? 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
Attachment
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'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: https://www.postgresql.org/message-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: https://www.postgresql.org/message-id/flat/200712091148.54294.xzilla%40users.sourceforge.net
>> 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 > > 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: > https://www.postgresql.org/message-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