From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
cc: Fujii Masao <masao.fujii@gmail.com>,
 Michael Paquier <michael.paquier@gmail.com>,
 Haribabu Kommi <kommi.haribabu@gmail.com>, brown@fastmail.com,
 "pgsql-bugs@postgresql.org" <pgsql-bugs@postgresql.org>
Subject: Re: BUG #13741: vacuumdb does not accept valid password
In-reply-to: <20151112170303.GN614468@alvherre.pgsql>
References: 
 <CAB7nPqQOianRojz+_=kdYg4caeYuP9057ODTgCFoAHSp0t9ccw@mail.gmail.com>
 <20151102192626.GG6104@alvherre.pgsql>
 <CAJrrPGe6+q7V8mssFV5nHXpisZqTLOwd=434hF3q1tyDjoGmew@mail.gmail.com>
 <CAB7nPqQTPujBV9a1bZMNGQ1VAUTAYfZTNnM6Yb6c2TZHva_MvQ@mail.gmail.com>
 <CAJrrPGeXuav-OYjJYKwSaLt+aOU+emH4a5XZS3XfMHNfanUmnw@mail.gmail.com>
 <CAJrrPGetAZVhE=0YJY5idTYtSt2o1LCkkC9aQaw81kJg2RyWuw@mail.gmail.com>
 <20151104021618.GP6104@alvherre.pgsql>
 <CAB7nPqSazOE_bB7koqHOY2gYM6vPs9e3D158ZCjxyJ61p1Ra8Q@mail.gmail.com>
 <CAB7nPqTq5PfQbMMCqJ4KRM8Q1ppOgVu4WziuEzm=3TNUN5wUoA@mail.gmail.com>
 <CAHGQGwHx0BvxTDpo2xXS--gEJFQjjrTP4BZY8wRzUfPYakzEgQ@mail.gmail.com>
 <20151112170303.GN614468@alvherre.pgsql>
Comments: In-reply-to Alvaro Herrera <alvherre@2ndquadrant.com>
 message dated "Thu, 12 Nov 2015 14:03:03 -0300"
Date: Thu, 12 Nov 2015 14:41:03 -0500
Message-ID: <15139.1447357263@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-bugs-owner@postgresql.org

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> Fujii Masao wrote:
>> ISTM that the attached simpler patch can fix the problem.
>> But maybe I'm missing something...

> Hmm, this is a very good and simple idea that looks like it should do
> the trick.  I would just add a comment explaining why we're using a
> static.

NAK.  This changes the behavior of connectDatabase() for *all* users
of that function, not only vacuumdb.  But the proposed behavioral
change is only appropriate for calling programs in which only a single
host/port/database target is used per execution.  In other contexts,
reusing the prior password is not just inappropriate but could actually
create security issues.  (It's possible that this behavior would be okay
for all existing callers, but that doesn't mean we should put in a
security gotcha for future uses.)

We could make this approach work if connectDatabase() remembered all
the parameters internally, and only tried to reuse the password when
they all match.  Or maybe it'd be better to alter the API so the caller
can say whether to try to reuse a saved password or not.  But I'm not sure
whether either of those answers is cleaner than the previous patch.

(BTW, I notice that pg_dumpall.c has a version of connectDatabase in
which the "static" trick is already being used, sans any documentation.
That's okay for pg_dumpall, but might be an issue if anyone copies-and-
pastes that version somewhere else ... and in any case it's fair to ask
why that version hasn't been merged with common.c.)

			regards, tom lane


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