Bug #756: suggestion: file with password instead of $PGPASSWORD

This item:
Add file to hold passwords using PGPASSWORDFILE environment variable

Has been completed, and will be a part of the 7.3 release.

Thanks,
Rod

This functionality will be in 7.3, due out in a few months.

---------------------------------------------------------------------------

pgsql-bugs@postgresql.org wrote:

Wojciech Scigala (pg@wojtus.net) reports a bug with a severity of 4
The lower the number the more severe it is.

Short Description
suggestion: file with password instead of $PGPASSWORD

Long Description
This is not a bug-report in fact, but a suggestion of a feature.
(I couldn't find an separate email for suggestions and ideas).
As you know, many people have trouble keeping both secure (passworded) access to PG databased while allowing some unattended access for them, for example for backing up.
Enviroment variable $PGPASSWORD is very useful here, but sensitive data should not be passed via enviroment. The better way to do it is to provide a name of file which contains the password. The file's access rights (if set properly) will provide necessary security in an easy way. And that's what I'm suggesting - an introdution of new variable, say $PGPASSWORD_FILE which will point to a file with password.

Sample Code

No file was uploaded with this report

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On 4 Sep 2002 at 12:48, Rod Taylor wrote:

This item:
Add file to hold passwords using PGPASSWORDFILE environment variable

Has been completed, and will be a part of the 7.3 release.

Is there a default file name like ~/.pgpassword so that this feature
can be used without setting yet another environment variable?

cu
Reinhard

On Thu, 2002-09-05 at 05:13, Reinhard Max wrote:

On 4 Sep 2002 at 12:48, Rod Taylor wrote:

This item:
Add file to hold passwords using PGPASSWORDFILE environment variable

Has been completed, and will be a part of the 7.3 release.

Is there a default file name like ~/.pgpassword so that this feature
can be used without setting yet another environment variable?

I'm not entirely sure, but I don't believe so. You can read the 7.3
docs at developer.postgresql.org.

Rod Taylor wrote:

On Thu, 2002-09-05 at 05:13, Reinhard Max wrote:

On 4 Sep 2002 at 12:48, Rod Taylor wrote:

This item:
Add file to hold passwords using PGPASSWORDFILE environment variable

Has been completed, and will be a part of the 7.3 release.

Is there a default file name like ~/.pgpassword so that this feature
can be used without setting yet another environment variable?

I'm not entirely sure, but I don't believe so. You can read the 7.3
docs at developer.postgresql.org.

Is there a good reason for a default for this? If we have a default,
there will be no way to disable the lookups except by renaming the file.
On the other hand, no default means that people will make up their own
names for the file, and that seems bad.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Thu, 2002-09-05 at 12:14, Bruce Momjian wrote:

Rod Taylor wrote:

On Thu, 2002-09-05 at 05:13, Reinhard Max wrote:

On 4 Sep 2002 at 12:48, Rod Taylor wrote:

This item:
Add file to hold passwords using PGPASSWORDFILE environment variable

Has been completed, and will be a part of the 7.3 release.

Is there a default file name like ~/.pgpassword so that this feature
can be used without setting yet another environment variable?

I'm not entirely sure, but I don't believe so. You can read the 7.3
docs at developer.postgresql.org.

Is there a good reason for a default for this? If we have a default,
there will be no way to disable the lookups except by renaming the file.
On the other hand, no default means that people will make up their own
names for the file, and that seems bad.

My understanding is that it's a single password, not a list.

As such you would probably not want a default, as each database you
connect to will (should?) have a different password. By setting a
default file we may encourage users to use the same password throughout
all PostgreSQL databases.

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Is there a default file name like ~/.pgpassword so that this feature
can be used without setting yet another environment variable?

Is there a good reason for a default for this?

Well, most of the other packages I can think of have hard-wired
assumptions about names like "$HOME/.cvspass". If we have such a
default then there's little need for a PGPASSWORDFILE environment
variable at all. Perhaps we should go with the historical custom
and remove the env var in favor of a hardwired filename in $HOME.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Is there a default file name like ~/.pgpassword so that this feature
can be used without setting yet another environment variable?

Is there a good reason for a default for this?

Well, most of the other packages I can think of have hard-wired
assumptions about names like "$HOME/.cvspass". If we have such a
default then there's little need for a PGPASSWORDFILE environment
variable at all. Perhaps we should go with the historical custom
and remove the env var in favor of a hardwired filename in $HOME.

Yes, that is what I am thinking too. Do other packages allow you to
override the default password file name? I don't think so. I don't see
that in .ssh.

OK, next question. Is this something that can be fixed during beta. I
sure think so because if we don't we will have even more confusion for
7.4.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Rod Taylor wrote:

On Thu, 2002-09-05 at 12:14, Bruce Momjian wrote:

Rod Taylor wrote:

On Thu, 2002-09-05 at 05:13, Reinhard Max wrote:

On 4 Sep 2002 at 12:48, Rod Taylor wrote:

This item:
Add file to hold passwords using PGPASSWORDFILE environment variable

Has been completed, and will be a part of the 7.3 release.

Is there a default file name like ~/.pgpassword so that this feature
can be used without setting yet another environment variable?

I'm not entirely sure, but I don't believe so. You can read the 7.3
docs at developer.postgresql.org.

Is there a good reason for a default for this? If we have a default,
there will be no way to disable the lookups except by renaming the file.
On the other hand, no default means that people will make up their own
names for the file, and that seems bad.

My understanding is that it's a single password, not a list.

It isn't a single password. It is a file containing one password per
line with pattern matching.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

My understanding is that it's a single password, not a list.

It isn't a single password. It is a file containing one password per
line with pattern matching.

Oh, well in that case it should probably be a default spot in the home
directory.

Bruce Momjian <pgman@candle.pha.pa.us> writes:

OK, next question. Is this something that can be fixed during beta.

Yeah, I think so --- it's not forcing an initdb, so it won't be too
painful for beta testers. And once we release it will be very hard
to change the definition of the feature; better to get it right now.

regards, tom lane

On 5 Sep 2002 at 12:33, Rod Taylor wrote:

My understanding is that it's a single password, not a list.

As such you would probably not want a default, as each database you
connect to will (should?) have a different password. By setting a
default file we may encourage users to use the same password
throughout all PostgreSQL databases.

If a password file could only hold a single password, one would
constantly need to check and change the environmet variable when he
frequently changes between different databases and that would IMHO
withdraw most of the benefits of having such a file.

I would rather want to have a single file, that can hold multiple
entries and also allows commenting out entries. And why only put the
passwords into that file? The entries could be complete connection
info tuples including hostname, username, password, and maybe more
parameters, so that databases can be addressed by a nickname. I think
of something like

--- snip ---
foo password=foopass
bar hostname=foo.bar.com dbname=foodb user=max password=xyz
--- snap ---

So even the remote "bar" database could be accessed by a simple

$ psql bar

If there is still need to have the password file on another than the
default place, e.g. because the home directory is on NFS and thus
considered unsafe, the file format could simply allow some sort of
include command.

cu
Reinhard

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

OK, next question. Is this something that can be fixed during beta.

Yeah, I think so --- it's not forcing an initdb, so it won't be too
painful for beta testers. And once we release it will be very hard
to change the definition of the feature; better to get it right now.

I am on it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

OK, next question. Is this something that can be fixed during beta.

Yeah, I think so --- it's not forcing an initdb, so it won't be too
painful for beta testers. And once we release it will be very hard
to change the definition of the feature; better to get it right now.

OK, patch applied. I will update HISTORY now and send an email to
hackers outlining the change.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

+ #define PSQLHISTORY "/.psql_history"
...
! char *psql_history = (char *) malloc(strlen(home) +
! strlen(PSQLHISTORY) + 1);

! sprintf(psql_history, "%s" PSQLHISTORY, home);

This seems like a really ugly coding practice. The sprintf is hard to
read and absolutely dependent on the assumption that PSQLHISTORY
contains no %. I'd suggest this pattern:

#define PSQLHISTORY ".psql_history"

...
! char *psql_history = (char *) malloc(strlen(home) +
! strlen(PSQLHISTORY) + 2);

! sprintf(psql_history, "%s/%s", home, PSQLHISTORY);

as being easier to read and safer.

In PasswordFromFile():

+     /* Look for it in the home dir */
+     home = getenv("HOME");
+     if (home)
+     {
+         pgpassfile = malloc(strlen(home) + strlen(PGPASSFILE) + 1);
+         if (!pgpassfile)
+         {
+             fprintf(stderr, gettext("%s: out of memory\n"), pset.progname);
+             exit(EXIT_FAILURE);
+         }
+     }
+     else
+         return NULL;

libpq has no business calling exit(). How about just "return NULL" like
all the other failure cases in that routine?

regards, tom lane

OK, I will make those changes. Thanks.

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

+ #define PSQLHISTORY "/.psql_history"
...
! char *psql_history = (char *) malloc(strlen(home) +
! strlen(PSQLHISTORY) + 1);

! sprintf(psql_history, "%s" PSQLHISTORY, home);

This seems like a really ugly coding practice. The sprintf is hard to
read and absolutely dependent on the assumption that PSQLHISTORY
contains no %. I'd suggest this pattern:

#define PSQLHISTORY ".psql_history"

...
! char *psql_history = (char *) malloc(strlen(home) +
! strlen(PSQLHISTORY) + 2);

! sprintf(psql_history, "%s/%s", home, PSQLHISTORY);

as being easier to read and safer.

In PasswordFromFile():

+     /* Look for it in the home dir */
+     home = getenv("HOME");
+     if (home)
+     {
+         pgpassfile = malloc(strlen(home) + strlen(PGPASSFILE) + 1);
+         if (!pgpassfile)
+         {
+             fprintf(stderr, gettext("%s: out of memory\n"), pset.progname);
+             exit(EXIT_FAILURE);
+         }
+     }
+     else
+         return NULL;

libpq has no business calling exit(). How about just "return NULL" like
all the other failure cases in that routine?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

OK, changes made. The pasting wasn't my idea but was there as part of
the original source:

sprintf(psqlrc, "%s/.psqlrc-" PG_VERSION, home);

I thought that was the way to do it so I propogated it consistenly.

No unpropoaged. Thanks for the code review.

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

+ #define PSQLHISTORY "/.psql_history"
...
! char *psql_history = (char *) malloc(strlen(home) +
! strlen(PSQLHISTORY) + 1);

! sprintf(psql_history, "%s" PSQLHISTORY, home);

This seems like a really ugly coding practice. The sprintf is hard to
read and absolutely dependent on the assumption that PSQLHISTORY
contains no %. I'd suggest this pattern:

#define PSQLHISTORY ".psql_history"

...
! char *psql_history = (char *) malloc(strlen(home) +
! strlen(PSQLHISTORY) + 2);

! sprintf(psql_history, "%s/%s", home, PSQLHISTORY);

as being easier to read and safer.

In PasswordFromFile():

+     /* Look for it in the home dir */
+     home = getenv("HOME");
+     if (home)
+     {
+         pgpassfile = malloc(strlen(home) + strlen(PGPASSFILE) + 1);
+         if (!pgpassfile)
+         {
+             fprintf(stderr, gettext("%s: out of memory\n"), pset.progname);
+             exit(EXIT_FAILURE);
+         }
+     }
+     else
+         return NULL;

libpq has no business calling exit(). How about just "return NULL" like
all the other failure cases in that routine?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073