Обсуждение: BUG #16486: Prompted password is ignored when password specified in connection string

The following bug has been logged on the website:

Bug reference:      16486
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 13beta1
Operating system:   Ubuntu 18.04
Description:

Assuming that we have the user u1 with the password p1, the following
command:
psql -U u1 'dbname=postgres password=p1' -W
prompts for a password but the input password is effectively ignored.
So if the connection string contains an invalid password:
psql -U u1 'dbname=postgres password=p2' -W
you can enter a valid password in a prompt, but still could not connect.
The documentation says:
PGPASSWORD behaves the same as the password connection parameter.

But in fact PGPASSWORD behaves differently . The following command:
PGPASSWORD="p1" psql -U u1 'dbname=postgres' -W
prompts for a password and fails if the password is wrong.
Similarly, if the environment variable contains an invalid password, it
still can be overridden with "-W".

PG Bug reporting form <noreply@postgresql.org> writes:
> Assuming that we have the user u1 with the password p1, the following
> command:
> psql -U u1 'dbname=postgres password=p1' -W
> prompts for a password but the input password is effectively ignored.

The entire point of the -W switch is that it prompts whether or not
the password is going to be used for anything.  If you don't like that,
don't use -W.

            regards, tom lane

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Mon, Jun 8, 2020 at 2:36 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The entire point of the -W switch is that it prompts whether or not
>> the password is going to be used for anything.

> A more intuitive interpretation of -W is that it prompts for a password,
> regardless of valid values being provided by other configuration, and uses
> that password exclusively to attempt to connect to the server.
> The documentation doesn't actually say which one of those two
> interpretations is correct.

Hmm ... I had thought that the docs explained -W in more or less the same
way I did above, but I see that (at least on the psql page) things are
indeed pretty vague.  I'll see about improving that.

            regards, tom lane

I wrote:
> "David G. Johnston" <david.g.johnston@gmail.com> writes:
>> On Mon, Jun 8, 2020 at 2:36 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The entire point of the -W switch is that it prompts whether or not
>>> the password is going to be used for anything.

>> A more intuitive interpretation of -W is that it prompts for a password,
>> regardless of valid values being provided by other configuration, and uses
>> that password exclusively to attempt to connect to the server.
>> The documentation doesn't actually say which one of those two
>> interpretations is correct.

> Hmm ... I had thought that the docs explained -W in more or less the same
> way I did above, but I see that (at least on the psql page) things are
> indeed pretty vague.  I'll see about improving that.

Concretely, it looks like we need edits as attached for psql, and
likewise for all our other programs with similar options.

The thing that was really missing here IMO is the specification that a
conninfo string overrides other command-line parameters.  It's somewhat
debatable whether a -W prompt is a "command line parameter", but it
acts that way for this purpose.  In any case, I'm disinclined to document
that specific interaction, because it would amount to blessing a pretty
damfool practice, which is to make your password visible on the program's
command line.  (I wonder whether section 33.1.2 ought to specifically
caution against putting passwords into command-line conninfo strings.)

            regards, tom lane

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 42e862cf17..816406235f 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -177,6 +177,8 @@ EOF
        or <literal>postgres://</literal>), it is treated as a
        <parameter>conninfo</parameter> string. See <xref
        linkend="libpq-connstring"/> for more information.
+       Connection parameters found in the <parameter>conninfo</parameter>
+       string will override other options such as <option>--username</option>.
       </para>
       </listitem>
     </varlistentry>
@@ -498,7 +500,7 @@ EOF
      <listitem>
       <para>
        Never issue a password prompt.  If the server requires password
-       authentication and a password is not available by other means
+       authentication and a password is not available from other sources
        such as a <filename>.pgpass</filename> file, the connection
        attempt will fail.  This option can be useful in batch jobs and
        scripts where no user is present to enter a password.
@@ -518,13 +520,15 @@ EOF
       <listitem>
       <para>
        Force <application>psql</application> to prompt for a
-       password before connecting to a database.
+       password before connecting to a database, even if the password will
+       not be used.
       </para>

       <para>
-       This option is never essential, since <application>psql</application>
-       will automatically prompt for a password if the server demands
-       password authentication.  However, <application>psql</application>
+       If the server requires password authentication and a password is not
+       available from other sources such as a <filename>.pgpass</filename>
+       file, <application>psql</application> will prompt for a
+       password in any case.  However, <application>psql</application>
        will waste a connection attempt finding out that the server wants a
        password.  In some cases it is worth typing <option>-W</option> to avoid
        the extra connection attempt.

Hello Tom,
09.06.2020 04:59, Tom Lane wrote:
> The thing that was really missing here IMO is the specification that a
> conninfo string overrides other command-line parameters.  It's somewhat
> debatable whether a -W prompt is a "command line parameter", but it
> acts that way for this purpose.  In any case, I'm disinclined to document
> that specific interaction, because it would amount to blessing a pretty
> damfool practice, which is to make your password visible on the program's
> command line.  (I wonder whether section 33.1.2 ought to specifically
> caution against putting passwords into command-line conninfo strings.)
I don't think that the main issue is with blessing such a practice,
because the documentation already says about PGPASSWORD:
Use of this environment variable is not recommended for security
reasons, as some operating systems allow non-root users to see process
environment variables via ps; ...
So using the PGPASSWORD is not blessed either.
And I wonder, what if we had, say PGCONNURI environment variable? Should
the password specified inside be not overridable too?

By the way, similar behaviour could also be observed with PGSERVICE:
echo -e "[mydb]\nhost=localhost\nuser=u1\npassword=p1\n" >/tmp/service.conf
PGSERVICEFILE=/tmp/service.conf PGSERVICE="mydb" psql -W
In this case psql effectively ignores the input password too. So the
visibility of the password is not main criteria to make it dominant.

Best regards,
Alexander

On Mon, Jun  8, 2020 at 09:59:18PM -0400, Tom Lane wrote:
> I wrote:
> > Hmm ... I had thought that the docs explained -W in more or less the same
> > way I did above, but I see that (at least on the psql page) things are
> > indeed pretty vague.  I'll see about improving that.
> 
> Concretely, it looks like we need edits as attached for psql, and
> likewise for all our other programs with similar options.
> 
> The thing that was really missing here IMO is the specification that a
> conninfo string overrides other command-line parameters.  It's somewhat
> debatable whether a -W prompt is a "command line parameter", but it
> acts that way for this purpose.  In any case, I'm disinclined to document
> that specific interaction, because it would amount to blessing a pretty
> damfool practice, which is to make your password visible on the program's
> command line.  (I wonder whether section 33.1.2 ought to specifically
> caution against putting passwords into command-line conninfo strings.)

I used a later version of Tom's patch to add documentation of how
connection strings can override command-line options, plus it has the
password description improvements.

I would like to apply this to all supported versions.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

On Wed, Sep  2, 2020 at 09:54:37AM +0200, Oleksandr Shulgin wrote:
> On Tue, Sep 1, 2020 at 10:38 PM Bruce Momjian <bruce@momjian.us> wrote:
> 
> 
>     I used a later version of Tom's patch to add documentation of how
>     connection strings can override command-line options, plus it has the
>     password description improvements.
> 
>     I would like to apply this to all supported versions.
> 
> 
> Not related to the proposed change, this sentence is the same before and after,
> but makes me wonder:
> 
> > However, <application>psql</application>
>         will waste a connection attempt finding out that the server wants a
>         password.  In some cases it is worth typing <option>-W</option> to
> avoid
>         the extra connection attempt.
> 
> AFAIK, there is nothing in the protocol that forces psql to make an extra
> connection attempt.  If libpq provided a means to pass a password-prompt
> callback, no extra connection would be needed I think?

That is true.  I don't think we want the backend to be kept open waiting
for someone to type a password, which is why, I think, we don't do it
that way.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

Bruce Momjian <bruce@momjian.us> writes:
> On Wed, Sep  2, 2020 at 09:54:37AM +0200, Oleksandr Shulgin wrote:
>> AFAIK, there is nothing in the protocol that forces psql to make an extra
>> connection attempt.  If libpq provided a means to pass a password-prompt
>> callback, no extra connection would be needed I think?

> That is true.  I don't think we want the backend to be kept open waiting
> for someone to type a password, which is why, I think, we don't do it
> that way.

Yeah, I think this is an ancient value judgement.  Sure, it's pretty
questionable whether spawning a backend a second time is better than
tying one up waiting for a human to type a password.  (It's going to
be spending a lot of time waiting for that human later on, anyway.)
The real point IMO is that libpq's connection API is already
unreasonably complicated, and providing a callback of this sort
would make it even messier.  A different client library might make
a different choice, perhaps.

            regards, tom lane

Bruce Momjian <bruce@momjian.us> writes:
> I used a later version of Tom's patch to add documentation of how
> connection strings can override command-line options, plus it has the
> password description improvements.

>          <option>--all</option>) is not used, the database name is read
>          from the environment variable <envar>PGDATABASE</envar>.  If
>          that is not set, the user name specified for the connection is
> -        used.
> +        used.  <link linkend="libpq-connstring">Connction strings</link>
> +        can also be used for connection specifications, and these can
> +        override other command-line options.
>         </para>
>        </listitem>
>       </varlistentry>

* Spellcheck (not "Connction" please)

* I do not like the wording "can override", because that's just impossibly
vague.  It leaves the reader wondering whether there are cases where that
doesn't happen.

I'd suggest wording more like

    The <replaceable>dbname</replaceable> can be a
    <link ...>connection string</link>.  If so, connection parameters
    found in the connection string will override any conflicting command
    line options, such as <option>--username</option>.

            regards, tom lane

On Wed, Sep  2, 2020 at 03:47:40PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > I used a later version of Tom's patch to add documentation of how
> > connection strings can override command-line options, plus it has the
> > password description improvements.
> 
> >          <option>--all</option>) is not used, the database name is read
> >          from the environment variable <envar>PGDATABASE</envar>.  If
> >          that is not set, the user name specified for the connection is
> > -        used.
> > +        used.  <link linkend="libpq-connstring">Connction strings</link>
> > +        can also be used for connection specifications, and these can
> > +        override other command-line options.
> >         </para>
> >        </listitem>
> >       </varlistentry>
> 
> * Spellcheck (not "Connction" please)
> 
> * I do not like the wording "can override", because that's just impossibly
> vague.  It leaves the reader wondering whether there are cases where that
> doesn't happen.
> 
> I'd suggest wording more like
> 
>     The <replaceable>dbname</replaceable> can be a
>     <link ...>connection string</link>.  If so, connection parameters
>     found in the connection string will override any conflicting command
>     line options, such as <option>--username</option>.

Sure, makes sense.  Updated patch attached.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

On Thu, Sep  3, 2020 at 03:05:38PM -0400, Bruce Momjian wrote:
> On Wed, Sep  2, 2020 at 03:47:40PM -0400, Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > I used a later version of Tom's patch to add documentation of how
> > > connection strings can override command-line options, plus it has the
> > > password description improvements.
> > 
> > >          <option>--all</option>) is not used, the database name is read
> > >          from the environment variable <envar>PGDATABASE</envar>.  If
> > >          that is not set, the user name specified for the connection is
> > > -        used.
> > > +        used.  <link linkend="libpq-connstring">Connction strings</link>
> > > +        can also be used for connection specifications, and these can
> > > +        override other command-line options.
> > >         </para>
> > >        </listitem>
> > >       </varlistentry>
> > 
> > * Spellcheck (not "Connction" please)
> > 
> > * I do not like the wording "can override", because that's just impossibly
> > vague.  It leaves the reader wondering whether there are cases where that
> > doesn't happen.
> > 
> > I'd suggest wording more like
> > 
> >     The <replaceable>dbname</replaceable> can be a
> >     <link ...>connection string</link>.  If so, connection parameters
> >     found in the connection string will override any conflicting command
> >     line options, such as <option>--username</option>.
> 
> Sure, makes sense.  Updated patch attached.

Patch applied to all supported versions;  thanks for the report.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

On Fri, Oct 02, 2020 at 10:19:58PM -0400, Bruce Momjian wrote:
> On Thu, Sep  3, 2020 at 03:05:38PM -0400, Bruce Momjian wrote:
> > On Wed, Sep  2, 2020 at 03:47:40PM -0400, Tom Lane wrote:
> > > Bruce Momjian <bruce@momjian.us> writes:
> > > > I used a later version of Tom's patch to add documentation of how
> > > > connection strings can override command-line options, plus it has the
> > > > password description improvements.
> > > 
> > > >          <option>--all</option>) is not used, the database name is read
> > > >          from the environment variable <envar>PGDATABASE</envar>.  If
> > > >          that is not set, the user name specified for the connection is
> > > > -        used.
> > > > +        used.  <link linkend="libpq-connstring">Connction strings</link>
> > > > +        can also be used for connection specifications, and these can
> > > > +        override other command-line options.
> > > >         </para>
> > > >        </listitem>
> > > >       </varlistentry>
> > > 
> > > * Spellcheck (not "Connction" please)
...
> > Sure, makes sense.  Updated patch attached.
> 
> Patch applied to all supported versions;  thanks for the report.

Unfortunately that included the typo Tom pointed out :(

On Mon, Nov  2, 2020 at 12:33:33AM -0600, Justin Pryzby wrote:
> On Fri, Oct 02, 2020 at 10:19:58PM -0400, Bruce Momjian wrote:
> > On Thu, Sep  3, 2020 at 03:05:38PM -0400, Bruce Momjian wrote:
> > > On Wed, Sep  2, 2020 at 03:47:40PM -0400, Tom Lane wrote:
> > > > Bruce Momjian <bruce@momjian.us> writes:
> > > > > I used a later version of Tom's patch to add documentation of how
> > > > > connection strings can override command-line options, plus it has the
> > > > > password description improvements.
> > > > 
> > > > >          <option>--all</option>) is not used, the database name is read
> > > > >          from the environment variable <envar>PGDATABASE</envar>.  If
> > > > >          that is not set, the user name specified for the connection is
> > > > > -        used.
> > > > > +        used.  <link linkend="libpq-connstring">Connction strings</link>
> > > > > +        can also be used for connection specifications, and these can
> > > > > +        override other command-line options.
> > > > >         </para>
> > > > >        </listitem>
> > > > >       </varlistentry>
> > > > 
> > > > * Spellcheck (not "Connction" please)
> ...
> > > Sure, makes sense.  Updated patch attached.
> > 
> > Patch applied to all supported versions;  thanks for the report.
> 
> Unfortunately that included the typo Tom pointed out :(

Oops, I missed that.  Fixed, thanks.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee