Re: Patch to be verbose about being unable to read ~/.pgpasss... - Mailing list pgsql-patches

> > All regression tests pass with this case and no ABI or source
> > incompatabilities are introduced.
>
> You're still playing extremely fast and loose with that claim of "no
> incompatibilities introduced".  This may not *directly* break an
> application that uses PQsetNoticeProcessor, but it breaks the purpose
> that the app is using PQsetNoticeProcessor for, namely to intercept
> notices.  If it doesn't also override the fnoticeProc then it hasn't
> intercepted all the notices.

Those notices were sent to setderr earlier, they were already broken:
at least now there's a ghost of a chance at the app at picking up
those errors.

> What I was envisioning was a simple little subroutine that takes a
> format string and some args, formats into a work buffer, and then
> passes the formatted string to the existing notice hook.
> Essentially, moving the five lines you're unhappy about into a
> common subroutine.  This gets the job done without *any*
> modification of the application API.

*nods*

> A bigger problem with the whole approach is that there's no way for
> the application to install a notice processor before
> PasswordFromFile is invoked, because that occurs during PQconnectdb.
> So any notices emitted by same will necessarily fall into the lap of
> the default notice processor.

Which is why I thought fprinting to stderr isn't that bad of an idea
given there are only two instances inside of PasswordFromFile that
make use of the noticeProc. :)

> I didn't hear any strong objection to the idea of treating the
> weak-protection complaint as a hard error (connection failure), so
> we can fix the already-existing problem by doing that.

libpq's a library used by many many applications ... do you envison
calling abort(), exit() or _exit()?  Seems really broken to halt the
app based on FS permissions.  Imagine an ISP web based environment
with PHP using libpq to connect to PostgreSQL and an insecure .pgpass
file.  Issuing a warning should be plenty sufficient.  Since this is a
case of "mother knows best," IMHO, it'd probably be wise to only print
these warnings if a debugging variable was set inside of libpq.

> As for the issue of whether to complain about a file that exists but
> isn't readable, Peter exhibited lots of precedent for not doing so.
> I'm inclined to think we should drop that part.

*shrug* Whatever the consensus is, raising awareness isn't a bad
thing, halting execution can be fatal.  The app that carries the most
weight in terms of precedent with me is gnupg because gnupg
intentionally deals with secure content/passwords.  Here are the perm
checks that gnupg does:

*) make sure the homedir, .gnupg dir, and libdir are owned by the
 user/root
*) make sure the homedir, .gnupg dir, and libdir are "safe" for the
 user/root

It's not extensive, but, in the event of a problem, it only prints to
stderr and it doesn't bomb out (gnupg-1.2.2/g10/g10.c:885).  With
GnuPG as the extreme in terms of security and its list of checks a bit
more extensive (though not exhaustive), bombing out seems like a bad
idea.

Here's another case to not bomb out.  An admin in a php web env types:

% vi ~www/.pgpass
[enters in a .pgpass file]
% chown www ~www/.pgpass
% chmod 0600 ~www/.pgpass

Game over.  During the period of time from when the admin writes the
file to chmod 0600's the file, all database requests will be
denied... which could be substantial in a web environment, and bad for
PostgreSQL's image, esp with the talk of MySQL alienating its users
and Pg trying to pick up steam in the PHP community.

-sc


PS I really will lay off the topic, just trying to make a good case
   for a usable and secure library.  :)

--
Sean Chittenden