Thread: [PATCH] pgpassfile connection option

Hello psql-hackers!

We thought it would be advantageous to be able to specify a 'custom'
pgpassfile within the connection string along the lines of the existing
parameters sslkey and sslcert.

Which is exactly what this very compact patch does.
The patch is minimally invasive - when no pgpassfile attribute is
provided in the connection string, the regular pgpassfile is used.
The security-measures (which are limited to checking the permissions for
0600) are kept, however we could loosen that restriciton to allow group
access as well along the lines of the ssl key file , if this is
preferred. (in case multiple users belonging to the same group would
like to connect using the same file).

The patch applies cleanly to master and compiles and runs as expected
(as there are no critical alterations).
I've not written any documentation as of now, but I'll follow up closely
if there is any interest for this patch.

notes:
  - using ~ to denote the user's home directory in the path does not
work, however $HOME works (as this is translated by bash beforehand).
  - the notation in the custom pgpassfile should follow the notation of
the 'default' pgpass files:
     hostname:port:database:username:password
  - this has only been tested on linux so far, however due to the nature
of the changes I suspect that there is nothing that could go wrong in
other environments, although I could test that as well, if deemed necessary.


I'm looking forward to any feedback,
Julian

--

Julian Markwort
Westphalian Wilhelms-University in Münster
julian.markwort@uni-muenster.de

On 09/22/2016 10:44 AM, Julian Markwort wrote:
> Hello psql-hackers!
>
> We thought it would be advantageous to be able to specify a 'custom' 
> pgpassfile within the connection string along the lines of the 
> existing parameters sslkey and sslcert.
>
> Which is exactly what this very compact patch does.
> The patch is minimally invasive - when no pgpassfile attribute is 
> provided in the connection string, the regular pgpassfile is used.
> The security-measures (which are limited to checking the permissions 
> for 0600) are kept, however we could loosen that restriciton to allow 
> group access as well along the lines of the ssl key file , if this is 
> preferred. (in case multiple users belonging to the same group would 
> like to connect using the same file).
>
> The patch applies cleanly to master and compiles and runs as expected 
> (as there are no critical alterations).
> I've not written any documentation as of now, but I'll follow up 
> closely if there is any interest for this patch.
>
> notes:
>  - using ~ to denote the user's home directory in the path does not 
> work, however $HOME works (as this is translated by bash beforehand).
>  - the notation in the custom pgpassfile should follow the notation of 
> the 'default' pgpass files:
>     hostname:port:database:username:password
>  - this has only been tested on linux so far, however due to the 
> nature of the changes I suspect that there is nothing that could go 
> wrong in other environments, although I could test that as well, if 
> deemed necessary.



I'm not necessarily opposed to this, but what is the advantage over the 
existing PGPASSFILE  environment setting mechanism?


cheers

andrew

I haven't really thought about this as I had been asked to make this 
work as an additional option to the connection parameters...
Now that I've looked at it - there is really only the benefit of saving 
the step of setting the PGPASSFILE environment variable.
However, there might be cases in which setting an environment variable 
might not be the easiest option.

Am 22.09.2016 um 17:15 schrieb Andrew Dunstan:
> I'm not necessarily opposed to this, but what is the advantage over 
> the existing PGPASSFILE  environment setting mechanism? 

On Thu, Sep 22, 2016 at 11:34 AM, Julian Markwort
<julian.markwort@uni-muenster.de> wrote:
> I haven't really thought about this as I had been asked to make this work as
> an additional option to the connection parameters...
> Now that I've looked at it - there is really only the benefit of saving the
> step of setting the PGPASSFILE environment variable.
> However, there might be cases in which setting an environment variable might
> not be the easiest option.

So, there are some security concerns here in my mind.  If a program
running under a particular user ID accepts a connection string from a
source that isn't fully trusted, the user has to accept the risk that
their .pgpass file will be used for authentication to whatever
database the program might try to connect.  However, they don't have
to accept the possibility that arbitrary local files readable by the
user ID will be used for authentication and/or disclosed; this patch
would force them to accept that risk.  That doesn't seem particularly
good.  If an adversary has enough control over my account that they
can set environment variables, it's game over: they win.  But if I
merely accept connection strings from them, I shouldn't have to worry
about anything worse than that I might be induced to connect to the
wrong thing.

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

On 09/26/2016 07:51 PM, Robert Haas wrote:
> However, they don't have
> to accept the possibility that arbitrary local files readable by the
> user ID will be used for authentication and/or disclosed; this patch
> would force them to accept that risk.
I do agree with you, however we might have to take a look at the 
parameter sslkey's implementation here as well - There are no checks in 
place to stop you from using rogue sslkey parameters.
I'd like to suggest having both of these parameters behave in a similar 
fashion. In order to achieve safe behaviour, we could implement the use 
of environment variables prohibiting the use of user-located pgpassfiles 
and sslkeys.
How about PGSECRETSLOCATIONLOCK ?

On Tue, Oct 4, 2016 at 7:42 AM, Julian Markwort
<julian.markwort@uni-muenster.de> wrote:
> On 09/26/2016 07:51 PM, Robert Haas wrote:
>> However, they don't have
>> to accept the possibility that arbitrary local files readable by the
>> user ID will be used for authentication and/or disclosed; this patch
>> would force them to accept that risk.
>
> I do agree with you, however we might have to take a look at the parameter
> sslkey's implementation here as well - There are no checks in place to stop
> you from using rogue sslkey parameters.
> I'd like to suggest having both of these parameters behave in a similar
> fashion. In order to achieve safe behaviour, we could implement the use of
> environment variables prohibiting the use of user-located pgpassfiles and
> sslkeys.
> How about PGSECRETSLOCATIONLOCK ?

You could do something like that, I guess, but I think it might be a
good idea to wait and see if anyone else has opinions on (1) the
desirability of the basic feature, (2) the severity of the security
hazard it creates, and (3) your proposed remediation method.

So far I don't see anybody actually endorsing your proposal here,
which might mean that any patch you produce will be rejected on the
grounds that nobody has a clear use case for this feature.

Hey, everybody: chime in here...

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

05.10.2016, 20:06, Robert Haas kirjoitti:
> You could do something like that, I guess, but I think it might be a
> good idea to wait and see if anyone else has opinions on (1) the
> desirability of the basic feature, (2) the severity of the security
> hazard it creates, and (3) your proposed remediation method.

I don't think it's appropriate to compare the proposed pgpassfile option 
to sslkey: the key is never transmitted over the network anywhere.

> So far I don't see anybody actually endorsing your proposal here,
> which might mean that any patch you produce will be rejected on the
> grounds that nobody has a clear use case for this feature.
>
> Hey, everybody: chime in here...

The original patch didn't come with a description of the problem it was 
aiming to solve, but I've wanted something perhaps a bit similar in the 
past.

The pghoard backup & restore suite we maintain needs to be able to spawn 
pg_basebackup and pg_receivexlog processes and in order to avoid passing 
passwords in command-line arguments visible to everyone who can get a 
process listing we currently have pghoard edit pgpass files.

Having to edit pgpass files at all is annoying and there isn't really a 
good way to do it: we can edit the one at $HOME and hope it doesn't 
clash with the expectations of the user running the software, write it 
to a custom file somewhere else - copying the password to a location the 
user might not expect - or create a new temporary file on every invocation.

I came across some bad advice about passing passwords to libpq today: 
there was a recommendation for setting a $PASSWORD environment variable 
and including that in the connection string, using shell to expand it. 
It got me thinking that maybe such a feature could be implemented in 
libpq: have it expand unquoted $variables; for example:
  $ PASSWORD=xyz psql 'password=$PASSWORD dbname=foo'

This does have the hazard of making it very easy to accidentally use 
double quotes instead of single quotes and have the shell expand the 
variable making it visible in process listing though.

/ Oskari

My 0.02€:

Patch applies cleanly, make check ok... however AFAICS it only means that 
it compiles but it is not tested in anyway... This is is annoying. Well 
I'm not sure whether other options are tested either, but they should.

ISTM that this feature is already available through the PGPASSFILE 
environment variable, so this is really just about providing the same 
feature through a connection string option, if I'm not mistaken. I'm fine 
with that.

The documentation needs to be updated.

I'm not much concerned about security issues as discussed on the thread. 
If an attacker can control the file, then what... they can provide a 
password in my place in a file of their own? If they have the pass then it 
works, if they do not then the connection will fail... They cannot change 
the database I'm connecting to from the pass file, say to provide other 
credentials to an application. So I do not see this as worst that the 
current status.

I tested the feature through python/psychopg2 by setting LD_LIBRARY_PATH 
to the dev version. I got a strange behavior wrt to errors. With "./foo" 
containing the right password it is fine.
  import psycopg2 as pg  c = pg.connect("host=localhost dbname=test user=test pgpassfile=./foo")  # ok  c.close()

Now if "./foo" contains a bad password:
  import psycopg2 as pg  c = pg.connect("host=localhost dbname=test user=test pgpassfile=./foo")  # error:
OperationalError:FATAL:  password authentication failed for user "test"  password retrieved from file "$HOME/.pgpass"
 

The reported password file is wrong. It is even more funny if ~/.pgpass 
contains the right password: the pgpassfile option *is* taken into 
account, ./foo is read and it fails, but the error message is not 
updated and points to the wrong file. The error message stuff should be 
updated to look for the pgpassfile connection string option...

-- 
Fabien.

On Tue, Oct 11, 2016 at 5:06 PM, Oskari Saarenmaa <os@ohmu.fi> wrote:
>   $ PASSWORD=xyz psql 'password=$PASSWORD dbname=foo'
>
> This does have the hazard of making it very easy to accidentally use double
> quotes instead of single quotes and have the shell expand the variable
> making it visible in process listing though.

It has the hazard that environment variables are visible in the
process listing anyway on many platforms.  On Linux, try "ps auxeww";
on MacOS X, try "ps -efEww".  At a quick glance, it seems that on both
of those platforms you have to either be root or be the same user that
owns the process, but I'm not sure that every platform will have it
locked down that tightly and even that might be more exposure than you
really want.

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

Patch applies cleanly, make check ok... however AFAICS it only means that it compiles but it is not tested in anyway... This is is annoying. Well I'm not sure whether other options are tested either, but they should.

Thanks for taking the time to review my patch! I haven't found much regarding the testing of connection options. However since there isn't anything fancy going on in PasswordFromFile() I'm not too worried about that.

The documentation needs to be updated.

I've written a couple lines now.
I aligned the definition of the connection option and the environment variable with that of other (conn.opt&env.var.) pairs and added mention of the different options to the doc of the "Password File".

The reported password file is wrong. It is even more funny if ~/.pgpass contains the right password: the pgpassfile option *is* taken into account, ./foo is read and it fails, but the error message is not updated and points to the wrong file. The error message stuff should be updated to look for the pgpassfile connection string option...

That was indeed an Error on my side, I hadn't updated the errormessages to inform you which file has been used.

So attached is an updated version of the patch.

I'd like to ask for some input on how to handle invalid files - right now no message is shown, the user just gets a password prompt as a result, however I think a message when the custom pgpassfile hasn't been found would be helpful.

Kind regards,
Julian

--

Julian Markwort
Westphalian Wilhelms-University in Münster
julian(dot)markwort(at)uni-muenster(dot)de

Hello Julian,

>> The documentation needs to be updated. 
> I've written a couple lines now.
> I aligned the definition of the connection option and the environment 
> variable with that of other (conn.opt&env.var.) pairs and added mention of 
> the different options to the doc of the "Password File".

Ok.

> [...] That was indeed an Error on my side, I hadn't updated the 
> errormessages to inform you which file has been used.
>
> So attached is an updated version of the patch.

Patch applies, compiles, "make check" ok (although the feature is not 
tested there).

Documentation compiled to html and looks ok.

I tested the feature with psql by resetting the dynamic library path.
The error message error in the previous review is fixed.

I have a problem with how the pass file name is managed: there
are three possible sources: 1) pgpassfile connection string option 2) PGPASSFILE environment variable 3) the default
(~/.pgpassor something else on windows)
 

Now function getPgPassFilename() is a misnomer, as it does not
always return the pass filename, but only in cases 2 and 3.

I think that PGPASSFILE is somehow checked twice: once explicitely
in getPgPassFilename and once because of the struct declaration
"pgpassfile". Once should be enough.

So I think some cleanup would be welcome. The mess is preexisting to your 
patch because the PGPASSFILE environment variable was managed differently 
from other options.

I would suggest that the struct gets the value (from option, environment 
or default) and is always used elsewhere. The getPgPassFilename function 
should disappear and PasswordFromFile should be simplified significantly.

> I'd like to ask for some input on how to handle invalid files - right 
> now no message is shown, the user just gets a password prompt as a 
> result, however I think a message when the custom pgpassfile hasn't been 
> found would be helpful.

I agree that it is currently unconvincing. I'm unclear about the correct 
level of messages that should be displayed for a library. I think that it 
should be pretty silent by default if all is well but that some 
environment variable should be able to put a more verbose level...

Libpq connection options seem not to be tested anywhere. There should be 
TAP tests in src/interfaces/libpq. This remark is independent of your 
patch.

-- 
Fabien.

Thanks for your quick response, Fabien
> I think that PGPASSFILE is somehow checked twice
yup, that was unintended redundancy...
> I would suggest that the struct gets the value (from option, 
> environment or default) and is always used elsewhere. The 
> getPgPassFilename function should disappear and PasswordFromFile 
> should be simplified significantly. 
I agree, however that's easier said than done because the "default" is 
only constant by its definition, not by its location.
There is no "default" location as the home directory depends not only on 
the system but also on the user. Afaics we can't entirely get rid of a 
function to get the location of the default .pgpass file.

Kind regards,
Julian

>> I would suggest that the struct gets the value (from option, environment or 
>> default) and is always used elsewhere. The getPgPassFilename function 
>> should disappear and PasswordFromFile should be simplified significantly. 
> I agree, however that's easier said than done because the "default" is only 
> constant by its definition, not by its location.
> There is no "default" location as the home directory depends not only on the 
> system but also on the user. Afaics we can't entirely get rid of a function 
> to get the location of the default .pgpass file.

Hmmm... I thought to put the default if not set from the connection option 
or the environment variable, say in connectOptions2(), just before getting 
the password if it is needed? That is ensure that the pgpassfile field is 
not NULL before calling PasswordFromFile, so that PasswordFromFile does 
not have to do anything and then the error message can rely on the 
pgpassfile field?

-- 
Fabien.

Alright, here goes another one:
1. Cleaned up the clutter with getPgPassFilename - the function is now
named fillDefaultPGPassFile() and only does exactly that.
2. Since a connection option "pgpassfile" or environment variable
"PGPASSFILE" are picked up in conninfo_add_defaults() or in case a
password was needed, but neither a pgpassfile connection option or
environment variable were set, we'd have filled the conn->pgpassfile
field with the "default" ~/.pgpass stuff.
Thus, when returning with an error, if conn->pgpassfile was set and a
password was necessary, we must have tried that pgpassfile, so i got rid
of the field "dot_pgpass_used" in  the pg_conn struct and the pgpassfile
string is always used in the error message.
3. Going on, I renamed "dot_pg_pass_warning()" to "PGPassFileWarning()"

Kind regards,
Julian Markwort

Welp, I was in head over heels, sorry for my messy email...

*2. No more "dot_pgpass_used" - we fill the conn->pgpassfile field with 
any options that have been provided (connection parameter, environment 
variable, "default" ~/.pgpass) and in case there has been an error with 
the authentification, we only use conn->pgpassfile in the error message.

Fabien Coelho wrote:
> I agree that it is currently unconvincing. I'm unclear about the 
> correct level of messages that should be displayed for a library. I 
> think that it should be pretty silent by default if all is well but 
> that some environment variable should be able to put a more verbose 
> level... 
fe_connect.c in general seems very talkative about it's errors - even if 
it's only about files not beeing found, so I'll include an error message 
for that case next time.

Julian

Hello Julian,

> Alright, here goes another one:

Patch v3 applies, make check ok, feature tested on Linux, one small issue 
found, see below.

> 1. Cleaned up the clutter with getPgPassFilename - the function is now named 
> fillDefaultPGPassFile() and only does exactly that.

Ok.

> 2. Since a connection option "pgpassfile" or environment variable 
> "PGPASSFILE" are picked up in conninfo_add_defaults() or in case a password 
> was needed, but neither a pgpassfile connection option or environment 
> variable were set, we'd have filled the conn->pgpassfile field with the 
> "default" ~/.pgpass stuff.

Ok.

> Thus, when returning with an error, if conn->pgpassfile was set and a 
> password was necessary, we must have tried that pgpassfile, so i got rid of 
> the field "dot_pgpass_used"

No, you should not have done that, because it changes a feature which was 
to warn *only* when the password was coming from file.

> in the pg_conn struct and the pgpassfile string is always used in the 
> error message.
 sh> touch dot_pass_empty sh> LD_LIBRARY_PATH=./src/interfaces/libpq \    psql "dbname=test host=localhost user=test
pgpassfile=./dot_pass_empty"Password: BAD_PASSWORD_TYPED psql: FATAL:  password authentication failed for user "test"
passwordretrieved from file "./dot_pass_empty"
 

The warning is wrong, the password was typed directly, not retrieved from 
a file. The "dot_pgpass_used" boolean is still required to avoid that.

> 3. Going on, I renamed "dot_pg_pass_warning()" to "PGPassFileWarning()"

This makes sense, its name is not necessarily ".pgpass".

-- 
Fabien.

Am 01.11.2016 um 15:14 schrieb Fabien COELHO:<br /><blockquote cite="mid:alpine.DEB.2.20.1611011504491.9978@lancre"
type="cite"><br/><blockquote style="color: #000000;" type="cite">Thus, when returning with an error, if
conn->pgpassfilewas set and a password was necessary, we must have tried that pgpassfile, so i got rid of the field
"dot_pgpass_used"<br /></blockquote><br /> No, you should not have done that, because it changes a feature which was to
warn<b class="moz-txt-star"><span class="moz-txt-tag">*</span>only<span class="moz-txt-tag">*</span></b> when the
passwordwas coming from file. <br /><br /> The warning is wrong, the password was typed directly, not retrieved from a
file.The "dot_pgpass_used" boolean is still required to avoid that. </blockquote><br /> I see... I was too focused on
lookingfor things to declutter, that I missed that case. I'll address that in the next revision.<br /> 

I've updated my patch to work with the changes introduced to libpq by allowing multiple hosts.
On Fabien's recommendations, I've kept the variable dot_pgpassfile_used, however I renamed that to pgpassfile_used, to keep a consistent naming scheme.
I'm still not sure about the amount of error messages produced by libpq, I think it would be ideal to report an error while accessing a file provided in the connection string, however I would not report that same type of error when the .pgpass file has been tried to retrieve a password.
(Else, you'd get an error message on every connection string that doesn't specify a pgpassfile or password, since .pgpass will be checked every time, before prompting the user to input the password)

regards,
Julian Markwort

All,

* Robert Haas (robertmhaas@gmail.com) wrote:
> You could do something like that, I guess, but I think it might be a
> good idea to wait and see if anyone else has opinions on (1) the
> desirability of the basic feature, (2) the severity of the security
> hazard it creates, and (3) your proposed remediation method.
[...]
> Hey, everybody: chime in here...

The feature strikes me as pretty reasonable to have and the pghoard
example shows that it can be quite handy in some circumstances.  I don't
see much merit behind the security concern raised- the file in question
would have to have the correct format and you would have to be
connecting to a system listed in that file for any disclosure to happen,
no?  As such, I don't know that any remediation is necessary for this.

Thanks!

Stephen

Hello Julian,

> I've updated my patch to work with the changes introduced to libpq by 
> allowing multiple hosts.

Ok. Patch applies cleanly, compiles & checks (although yet again the 
feature is not tested).

Feature tested and works for me, although I'm not sure how the multi-host 
warning about pgpassfile works, but I could not find a wrong warning.

Independently of your patch, while testing I concluded that the multi-host 
feature and documentation should be improved:
 - If a connection fails, it does not say for which host/port.
  sh> PGPASSFILE=$HOME/.pgpass.test \      LD_LIBRARY_PATH=$PWD/src/interfaces/libpq \      psql -d
"host=host1,host2,host3user=test dbname=test"  psql: FATAL:  password authentication failed for user "test"  password
retrievedfrom file "$HOME/.pgpass.test"
 
 - doc says "If multiple host names are specified, each will be tried in   turn in the order given.".
  In fact they are tried in turn if the network connection fails, but not  if the connection fails for some other
reasonsuch as the auth.  The documentation is not precise enough.
 

> On Fabien's recommendations, I've kept the variable dot_pgpassfile_used, 
> however I renamed that to pgpassfile_used, to keep a consistent naming 
> scheme.

Ok.

> I'm still not sure about the amount of error messages produced by libpq, 
> I think it would be ideal to report an error while accessing a file 
> provided in the connection string, however I would not report that same 
> type of error when the .pgpass file has been tried to retrieve a 
> password. (Else, you'd get an error message on every connection string 
> that doesn't specify a pgpassfile or password, since .pgpass will be 
> checked every time, before prompting the user to input the password)

Yep. This issue is independent of your patch as it is already there.
There could be a boolean set when the pass file is explicitely provided 
that could trigger a warning only in this case.

A few detailed comments:

Beware of spacing: . "if(" -> "if (" (2 instances) . " =='\0')" -> " == '\0')" (at least one instance)

Elsewhere:
 + if (conn->pgpassfile_used && conn->password_needed && conn->result && +     conn->pgpassfile &&
conn->pgpassfile[0]!='\0'&&
 

ISTM that if pgpassfile_used is true then pgpassfile is necessarily 
defined, so the second line tests are redundant? Or am I missing 
something?

-- 
Fabien.

Fabien Coelho wrote:

A few detailed comments:

Beware of spacing:
 . "if(" -> "if (" (2 instances)
 . " =='\0')" -> " == '\0')" (at least one instance)

Elsewhere:

 + if (conn->pgpassfile_used && conn->password_needed && conn->result &&
 +     conn->pgpassfile && conn->pgpassfile[0]!='\0' &&

ISTM that if pgpassfile_used is true then pgpassfile is necessarily defined, so the second line tests are redundant? Or am I missing something?

I've adressed those spacing errors.
You are right, if pgpassfile_used is true, it SHOULD be defined, I just like to be careful whenever I'm working with strings. But I guess in this scenario I can trust the caller and omit those checks.

 > Independently of your patch, while testing I concluded that the multi-host feature and documentation should be improved:

 > - If a connection fails, it does not say for which host/port.

Thanks I will re-test same.

> In fact they are tried in turn if the network connection fails, but not

> if the connection fails for some other reason such as the auth.

> The documentation is not precise enough.

If we fail due to authentication, then I think we should notify the client instead of trying next host. I think it is expected genuine user have right credentials for making the connection. So it's better if we notify same to client immediately rather than silently ignoring them. Otherwise the host node which the client failed to connect will be permanently unavailable until client corrects itself. But I agree documentation and error message as you said need improvements. I will try to correct same

I agree with those criticisms of the multi-host feature and notifying the client in case of an authentification error rather than trying other hosts seems sensible to me.
But I think fixes for those should be part of different patches, as this patch's aim was only to expand the existing pgpassfile functionality to be used with a parameter.

regards,
Julian

Hello Julian,

> I've adressed those spacing errors.

Ok.

> You are right, if pgpassfile_used is true, it SHOULD be defined, I just like 
> to be careful whenever I'm working with strings. But I guess in this scenario 
> I can trust the caller and omit those checks.

Good.

Patch looks ok, applies, compiles & checks, and tested manually.

I've switch in the CF to "ready for committer", and we'll see what the 
next level thinks about it:-)

> [...] I agree with those criticisms of the multi-host feature and 
> notifying the client in case of an authentification error rather than 
> trying other hosts seems sensible to me.

Sure. I complained about the fuzzy documentation & imprecise warning 
message because I stumbled upon that while testing.

> But I think fixes for those should be part of different patches, as this 
> patch's aim was only to expand the existing pgpassfile functionality to 
> be used with a parameter.

Yes.

-- 
Fabien.

Fabien COELHO <coelho@cri.ensmp.fr> writes:
> I've switch in the CF to "ready for committer", and we'll see what the 
> next level thinks about it:-)

Pushed with a few adjustments:

* It seemed to me that the appropriate parameter name is "passfile"
not "pgpassfile".  In all but a couple of legacy cases, the environment
variable's name is the parameter name plus a PG prefix; therefore,
with the environment variable already named PGPASSFILE, we have to use
"passfile" for consistency.  Putting a PG prefix on a connection parameter
name is rather pointless anyway, since there's no larger namespace that
it might have a conflict in.

* The error handling in the submitted patch was pretty shoddy; it didn't
check for malloc failure at all, and it didn't report a suitable error on
pqGetHomeDirectory failure either (which is worth doing, if it's going to
be a reason for connection failure).  I ended up concluding that the
easiest way to fix that involved just inlining getPgPassFilename into the
one remaining call site.

* I also got rid of the assignment of DefaultPassword to conn->pgpass;
that wasn't doing anything very useful anymore.  AFAICS, the only visible
effect was to make PQpass() return an empty string not NULL, but we can
make that happen without paying a malloc/free cycle for it.

* Minor comment and docs cleanups.
        regards, tom lane