BUG #13741: vacuumdb does not accept valid password

On Wed, Oct 28, 2015 at 6:39 AM, <brown@fastmail.com> wrote:

The following bug has been logged on the website:

Bug reference: 13741
Logged by: Eric Brown
Email address: brown@fastmail.com
PostgreSQL version: 9.5beta1
Operating system: CentOS 6.6
Description:

I am trying to vacuum a database from a remote computer. It appears that
vacuumdb is not accepting my password, though psql connects just fine.

______
browne1@reslin3 ~/Desktop> vacuumdb -z -j 24 -h reslin1.enhnet.org -p 5433
127
vacuumdb: vacuuming database "browne1"
Password:
Password:
Password:
vacuumdb: could not connect to database browne1: FATAL: password
authentication failed for user "browne1"
browne1@reslin3 ~/Desktop> psql -h reslin1.enhnet.org -p 5433
1
Password:
psql (9.5beta1)
Type "help" for help.

browne1=#
______

No. vacuumdb is accepting your password correctly. But you gave the number
of client connections as 24, to connect all these 24 clients to the
database, it is requesting
the password to enter again.

you can try with vacuumb -z -j2 to verify the same.

I feel we need to accept the password once and reuse it for all client
connections.
Provided working password can be returned for that database.

Regards,
Hari Babu
Fujitsu Australia

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

On Wed, Oct 28, 2015 at 2:53 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

On Wed, Oct 28, 2015 at 6:39 AM, <brown@fastmail.com> wrote:

The following bug has been logged on the website:

Bug reference: 13741
Logged by: Eric Brown
Email address: brown@fastmail.com
PostgreSQL version: 9.5beta1
Operating system: CentOS 6.6
Description:

I am trying to vacuum a database from a remote computer. It appears that
vacuumdb is not accepting my password, though psql connects just fine.

______
browne1@reslin3 ~/Desktop> vacuumdb -z -j 24 -h reslin1.enhnet.org -p 5433
127
vacuumdb: vacuuming database "browne1"
Password:
Password:
Password:
vacuumdb: could not connect to database browne1: FATAL: password
authentication failed for user "browne1"
browne1@reslin3 ~/Desktop> psql -h reslin1.enhnet.org -p 5433
1
Password:
psql (9.5beta1)
Type "help" for help.

browne1=#
______

No. vacuumdb is accepting your password correctly. But you gave the number
of client connections as 24, to connect all these 24 clients to the
database, it is requesting
the password to enter again.

you can try with vacuumb -z -j2 to verify the same.

I feel we need to accept the password once and reuse it for all client
connections.
Provided working password can be returned for that database.

Here I attached a patch that saves the password provided by the user
from the connectDatabase function and reuse it for connecting all clients
to the same database.

Regards,
Hari Babu
Fujitsu Australia

On Wed, Oct 28, 2015 at 4:07 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

Here I attached a patch that saves the password provided by the user
from the connectDatabase function and reuse it for connecting all clients
to the same database.

Attached a wrong patch, it is having some compilation problems,
Here is the new version with the fixed problems.

Regards,
Hari Babu
Fujitsu Australia

On Thu, Oct 29, 2015 at 9:13 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

On Wed, Oct 28, 2015 at 4:07 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

Here I attached a patch that saves the password provided by the user
from the connectDatabase function and reuse it for connecting all clients
to the same database.

Attached a wrong patch, it is having some compilation problems,
Here is the new version with the fixed problems.

I have added an entry in this CF:
https://commitfest.postgresql.org/7/417/
Let's not lose track of this patch.
--
Michael

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

On Mon, Nov 2, 2015 at 3:56 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Oct 29, 2015 at 9:13 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

On Wed, Oct 28, 2015 at 4:07 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

Here I attached a patch that saves the password provided by the user
from the connectDatabase function and reuse it for connecting all clients
to the same database.

Attached a wrong patch, it is having some compilation problems,
Here is the new version with the fixed problems.

I have added an entry in this CF:
https://commitfest.postgresql.org/7/417/
Let's not lose track of this patch.

Regarding this patch, wouldn't it be clearer to pass the password as a
variable of connectDatabase()? Then we could use NULL at the first
call of connectDatabase so as we enforce the prompt if requested by
the user. For successive calls of connectDatabase for each worker, we
then fetch the password from the parent connection using that:
if (PQconnectionUsedPassword(con))
password = PQpass(conn);
And pass it as an argument of connectDatabase. In short, I think that
this approach would make a more portable routine because one could
enforce a password at the first call of connectDatabase() without
having to save it once.
--
Michael

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

On Mon, Nov 2, 2015 at 6:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Mon, Nov 2, 2015 at 3:56 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Oct 29, 2015 at 9:13 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

On Wed, Oct 28, 2015 at 4:07 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

Here I attached a patch that saves the password provided by the user
from the connectDatabase function and reuse it for connecting all clients
to the same database.

Attached a wrong patch, it is having some compilation problems,
Here is the new version with the fixed problems.

I have added an entry in this CF:
https://commitfest.postgresql.org/7/417/
Let's not lose track of this patch.

Thanks for adding the patch to commitfest.

Regarding this patch, wouldn't it be clearer to pass the password as a
variable of connectDatabase()? Then we could use NULL at the first
call of connectDatabase so as we enforce the prompt if requested by
the user. For successive calls of connectDatabase for each worker, we
then fetch the password from the parent connection using that:
if (PQconnectionUsedPassword(con))
password = PQpass(conn);
And pass it as an argument of connectDatabase. In short, I think that
this approach would make a more portable routine because one could
enforce a password at the first call of connectDatabase() without
having to save it once.

Thanks for the review.
Here I attached modified patch that gets the password from parent connection
if exists and use it for the child connection also.

Regards,
Hari Babu
Fujitsu Australia

On Mon, Nov 2, 2015 at 8:10 PM, Haribabu Kommi wrote:

Here I attached modified patch that gets the password from parent connection
if exists and use it for the child connection also.

Meh? Why the parent connection? You could simply have the password as
an argument of connectDatabase, per se the attached. That looks just
more simple.
--
Michael

Michael Paquier wrote:

On Mon, Nov 2, 2015 at 8:10 PM, Haribabu Kommi wrote:

Here I attached modified patch that gets the password from parent connection
if exists and use it for the child connection also.

Meh? Why the parent connection? You could simply have the password as
an argument of connectDatabase, per se the attached. That looks just
more simple.

Thanks! This is almost there, but there's an important missing piece --
vacuumdb --all will ask you for a password for each database. I think
vacuum_one_database needs to receive a password from its caller too.

(Also, please use pg_strdup.)

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On Tue, Nov 3, 2015 at 6:26 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

Michael Paquier wrote:

On Mon, Nov 2, 2015 at 8:10 PM, Haribabu Kommi wrote:

Here I attached modified patch that gets the password from parent connection
if exists and use it for the child connection also.

Meh? Why the parent connection? You could simply have the password as
an argument of connectDatabase, per se the attached. That looks just
more simple.

Thanks! This is almost there, but there's an important missing piece --
vacuumdb --all will ask you for a password for each database. I think
vacuum_one_database needs to receive a password from its caller too.

Here I attached a separate patch to handle the reuse of password for
vacuumdb -all
case. The same behavior exists in all supported branches.

(Also, please use pg_strdup.)

The simple_prompt function in connectDatabase allocates memory for password
using malloc. Because of this reason he used strdup instead of pstrdup to avoid
adding a separate code to handle both the cases.

Regards,
Hari Babu
Fujitsu Australia

On Tue, Nov 3, 2015 at 6:10 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

On Tue, Nov 3, 2015 at 6:26 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:

Michael Paquier wrote:

On Mon, Nov 2, 2015 at 8:10 PM, Haribabu Kommi wrote:

Here I attached modified patch that gets the password from parent connection
if exists and use it for the child connection also.

Meh? Why the parent connection? You could simply have the password as
an argument of connectDatabase, per se the attached. That looks just
more simple.

Thanks! This is almost there, but there's an important missing piece --
vacuumdb --all will ask you for a password for each database. I think
vacuum_one_database needs to receive a password from its caller too.

Argh, right.

Here I attached a separate patch to handle the reuse of password for
vacuumdb -all
case. The same behavior exists in all supported branches.

Sure. Still you don't actually need a double pointer as you do. You
could just reuse the password from the connection obtained via
connectMaintenanceDatabase and pass the password from this connection
as the argument to vacuum_one_database. Something like the attached
seems more elegant IMO.

(Also, please use pg_strdup.)

The simple_prompt function in connectDatabase allocates memory for password
using malloc. Because of this reason he used strdup instead of pstrdup to avoid
adding a separate code to handle both the cases.

Yep. That's the deal.
--
Michael

On Wed, Nov 4, 2015 at 12:06 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Tue, Nov 3, 2015 at 6:10 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

Here I attached a separate patch to handle the reuse of password for
vacuumdb -all
case. The same behavior exists in all supported branches.

Sure. Still you don't actually need a double pointer as you do. You
could just reuse the password from the connection obtained via
connectMaintenanceDatabase and pass the password from this connection
as the argument to vacuum_one_database. Something like the attached
seems more elegant IMO.

Why I used a double pointer is to support the scenario like the following.
- There is no password requirement for Postgres, template1 and
maintenance db that is provided by the user.
- But there is a password requirement for user databases.
- If user doesn't provide the password during connection to
Maintenance database, later it prompts for
password while connecting to user database.
- Without the double pointer, further on for every database, it
prompts for the password and also
the case of --analyze-in-stages prompts for password for the all the stages.

Regards,
Hari Babu
Fujitsu Australia

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

On Wed, Nov 4, 2015 at 11:24 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

On Wed, Nov 4, 2015 at 12:06 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Tue, Nov 3, 2015 at 6:10 AM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

Here I attached a separate patch to handle the reuse of password for
vacuumdb -all
case. The same behavior exists in all supported branches.

Sure. Still you don't actually need a double pointer as you do. You
could just reuse the password from the connection obtained via
connectMaintenanceDatabase and pass the password from this connection
as the argument to vacuum_one_database. Something like the attached
seems more elegant IMO.

Why I used a double pointer is to support the scenario like the following.
- There is no password requirement for Postgres, template1 and
maintenance db that is provided by the user.
- But there is a password requirement for user databases.
- If user doesn't provide the password during connection to
Maintenance database, later it prompts for
password while connecting to user database.
- Without the double pointer, further on for every database, it
prompts for the password and also
the case of --analyze-in-stages prompts for password for the all the stages.

And one more thing, the vacuumdb password behavior is present in back branches
also, is it worth back patching the vacuumdb fix to all supported
branches and apply
the jobs connection fix only to master and 9.5?

Regards,
Hari Babu
Fujitsu Australia

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

Haribabu Kommi wrote:

On Wed, Nov 4, 2015 at 11:24 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

And one more thing, the vacuumdb password behavior is present in back branches
also, is it worth back patching the vacuumdb fix to all supported
branches and apply
the jobs connection fix only to master and 9.5?

Given the lack of complaints, I doubt it's worth the destabilization
risk. Let's just patch 9.5 and be done with it.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

On Wed, Nov 4, 2015 at 9:24 AM, Haribabu Kommi wrote:

On Wed, Nov 4, 2015 at 12:06 AM, Michael Paquier wrote:
Why I used a double pointer is to support the scenario like the following.
- There is no password requirement for Postgres, template1 and
maintenance db that is provided by the user.
- But there is a password requirement for user databases.
- If user doesn't provide the password during connection to
Maintenance database, later it prompts for
password while connecting to user database.
- Without the double pointer, further on for every database, it
prompts for the password and also
the case of --analyze-in-stages prompts for password for the all the stages.

Yeah, I thought a bit about that while writing the last patch but
that's quite narrow, no? So I didn't think it was worth doing, the
approach to request a password only after connecting to the
maintenance database instead of relying on the first one given out by
user looked more simple and solid to me, and is able to cover all the
test scenarios either way as one can specify the maintenance database
he wishes to connect to.
Thoughts from other folks?
--
Michael

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

On Wed, Nov 4, 2015 at 11:16 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Haribabu Kommi wrote:

On Wed, Nov 4, 2015 at 11:24 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

And one more thing, the vacuumdb password behavior is present in back branches
also, is it worth back patching the vacuumdb fix to all supported
branches and apply
the jobs connection fix only to master and 9.5?

Given the lack of complaints, I doubt it's worth the destabilization
risk. Let's just patch 9.5 and be done with it.

Fine for me. --all is supported for ages.
--
Michael

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

On Wed, Nov 4, 2015 at 4:30 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Nov 4, 2015 at 11:16 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Haribabu Kommi wrote:

On Wed, Nov 4, 2015 at 11:24 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

And one more thing, the vacuumdb password behavior is present in back branches
also, is it worth back patching the vacuumdb fix to all supported
branches and apply
the jobs connection fix only to master and 9.5?

Given the lack of complaints, I doubt it's worth the destabilization
risk. Let's just patch 9.5 and be done with it.

Fine for me. --all is supported for ages.

OK, so attached is a patch aimed at master and 9.5. I reused the
suggestion of Haribabu to not rely completely on the maintenance
database after testing with a couple of database, some of them using
md5 and others trust. That's just more portable this way, and user
just needs to specify the password once to be done even with vacuumdb
--all.
Thoughts?
--
Michael

On Fri, Nov 6, 2015 at 5:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Nov 4, 2015 at 4:30 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Nov 4, 2015 at 11:16 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Haribabu Kommi wrote:

On Wed, Nov 4, 2015 at 11:24 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

And one more thing, the vacuumdb password behavior is present in back branches
also, is it worth back patching the vacuumdb fix to all supported
branches and apply
the jobs connection fix only to master and 9.5?

Given the lack of complaints, I doubt it's worth the destabilization
risk. Let's just patch 9.5 and be done with it.

Fine for me. --all is supported for ages.

OK, so attached is a patch aimed at master and 9.5. I reused the
suggestion of Haribabu to not rely completely on the maintenance
database after testing with a couple of database, some of them using
md5 and others trust. That's just more portable this way, and user
just needs to specify the password once to be done even with vacuumdb
--all.
Thoughts?

Thanks for the patch. Yes, reusing the password helps for --analyze-in-stages
for single database case also along with -all.

Regards,
Hari Babu
Fujitsu Australia

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

On Fri, Nov 6, 2015 at 3:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Nov 4, 2015 at 4:30 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Wed, Nov 4, 2015 at 11:16 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

Haribabu Kommi wrote:

On Wed, Nov 4, 2015 at 11:24 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:

And one more thing, the vacuumdb password behavior is present in back branches
also, is it worth back patching the vacuumdb fix to all supported
branches and apply
the jobs connection fix only to master and 9.5?

Given the lack of complaints, I doubt it's worth the destabilization
risk. Let's just patch 9.5 and be done with it.

Fine for me. --all is supported for ages.

OK, so attached is a patch aimed at master and 9.5. I reused the
suggestion of Haribabu to not rely completely on the maintenance
database after testing with a couple of database, some of them using
md5 and others trust. That's just more portable this way, and user
just needs to specify the password once to be done even with vacuumdb
--all.
Thoughts?

ISTM that the attached simpler patch can fix the problem.
But maybe I'm missing something...

Regards,

--
Fujii Masao

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.

--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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