Обсуждение: Re: [HACKERS] PGPASSWORD and client tools

Here's a patch that I think (hope) does this right, by using the file
pointed to by the environment var PGPASSFILE, if set, in preference to
$HOME/.pgpass. I assume that at this stage it would be held over for 8.1
as a new feature - if not I'll put together some docco in a hurry.

cheers

andrew



Andrew Dunstan wrote:

>
>
> Tom Lane wrote:
>
>> Andrew Dunstan <andrew@dunslane.net> writes:
>>
>>
>>> How about an environment variable that points to a .pgpass type file.
>>>
>>
>>
>> You can do that today: point $HOME at some temp directory or other.
>> AFAIR pg_dump doesn't make any other use of $HOME ...
>>
>>
>>
>>> Or we could even play games with PGPASSWORD - if it names an
>>> existing file that satisfies the .pgpass criteria then it will be
>>> taken as the location of the .pgpass file instead of $HOME/.pgpass -
>>> otherwise its value will be considered to be the password itself.
>>>
>>
>>
>> Gaack... if you want a separate variable, we can talk about that, but
>> let's not overload PGPASSWORD like that.  Consider even just the
>> implications of whether libpq error messages should echo back the
>> "filename" ...
>>
>>
>>
>>
>
> Yeah. as usual you're right :-)
>
> So let's go woth PGPASSFILE
>
> cheers
>
> andrew
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
>
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /home/cvsmirror/pgsql-server/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.284
diff -c -r1.284 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    29 Aug 2004 05:07:00 -0000    1.284
--- src/interfaces/libpq/fe-connect.c    5 Oct 2004 21:47:49 -0000
***************
*** 3096,3101 ****
--- 3096,3102 ----
      FILE       *fp;
      char       *pgpassfile;
      char       *home;
+     char       *passfile_env;
      struct stat stat_buf;

  #define LINELEN NAMEDATALEN*5
***************
*** 3113,3137 ****
      if (port == NULL)
          port = DEF_PGPORT_STR;

!     /*
!      * Look for it in the home dir. We don't use get_home_path() so we
!      * don't pull path.c into our library.
!      */
!     if (!(home = getenv(HOMEDIR)))
!         return NULL;
!
!     pgpassfile = malloc(strlen(home) + 1 + strlen(PGPASSFILE) + 1);
!     if (!pgpassfile)
      {
!         fprintf(stderr, libpq_gettext("out of memory\n"));
!         return NULL;
      }

  #ifndef WIN32
!     sprintf(pgpassfile, "%s/%s", home, PGPASSFILE);
  #else
!     sprintf(pgpassfile, "%s\\%s", home, PGPASSFILE);
  #endif

      /* If password file cannot be opened, ignore it. */
      if (stat(pgpassfile, &stat_buf) == -1)
--- 3114,3153 ----
      if (port == NULL)
          port = DEF_PGPORT_STR;

!     if ((passfile_env = getenv("PGPASSFILE")) != NULL &&
!         strlen(passfile_env) > 0)
      {
!         /* use the literal path from the environment, if set */
!         pgpassfile = strdup(passfile_env);
!         if (!pgpassfile)
!         {
!             fprintf(stderr, libpq_gettext("out of memory\n"));
!             return NULL;
!         }
      }
+     else
+     {

+         /*
+          * Look for it in the home dir. We don't use get_home_path() so we
+          * don't pull path.c into our library.
+          */
+         if (!(home = getenv(HOMEDIR)))
+             return NULL;
+
+         pgpassfile = malloc(strlen(home) + 1 + strlen(PGPASSFILE) + 1);
+         if (!pgpassfile)
+         {
+             fprintf(stderr, libpq_gettext("out of memory\n"));
+             return NULL;
+         }
+
  #ifndef WIN32
!         sprintf(pgpassfile, "%s/%s", home, PGPASSFILE);
  #else
!         sprintf(pgpassfile, "%s\\%s", home, PGPASSFILE);
  #endif
+     }

      /* If password file cannot be opened, ignore it. */
      if (stat(pgpassfile, &stat_buf) == -1)
***************
*** 3140,3145 ****
--- 3156,3173 ----
          return NULL;
      }

+     /* Must be a plain file, or we warn and ignore it */
+
+     if (! S_ISREG(stat_buf.st_mode))
+     {
+         fprintf(stderr,
+                 libpq_gettext("WARNING: Password file %s is not a plain file.\n"),
+                 pgpassfile);
+         free(pgpassfile);
+         return NULL;
+
+     }
+
  #ifndef WIN32
      /* If password file is insecure, alert the user and ignore it. */
      if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))

This has been saved for the 8.1 release:

    http:/momjian.postgresql.org/cgi-bin/pgpatches2

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

Andrew Dunstan wrote:
>
> Here's a patch that I think (hope) does this right, by using the file
> pointed to by the environment var PGPASSFILE, if set, in preference to
> $HOME/.pgpass. I assume that at this stage it would be held over for 8.1
> as a new feature - if not I'll put together some docco in a hurry.
>
> cheers
>
> andrew
>
>
>
> Andrew Dunstan wrote:
>
> >
> >
> > Tom Lane wrote:
> >
> >> Andrew Dunstan <andrew@dunslane.net> writes:
> >>
> >>
> >>> How about an environment variable that points to a .pgpass type file.
> >>>
> >>
> >>
> >> You can do that today: point $HOME at some temp directory or other.
> >> AFAIR pg_dump doesn't make any other use of $HOME ...
> >>
> >>
> >>
> >>> Or we could even play games with PGPASSWORD - if it names an
> >>> existing file that satisfies the .pgpass criteria then it will be
> >>> taken as the location of the .pgpass file instead of $HOME/.pgpass -
> >>> otherwise its value will be considered to be the password itself.
> >>>
> >>
> >>
> >> Gaack... if you want a separate variable, we can talk about that, but
> >> let's not overload PGPASSWORD like that.  Consider even just the
> >> implications of whether libpq error messages should echo back the
> >> "filename" ...
> >>
> >>
> >>
> >>
> >
> > Yeah. as usual you're right :-)
> >
> > So let's go woth PGPASSFILE
> >
> > cheers
> >
> > andrew
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> >


>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

--
  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

I will add documentation for it.

Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

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


Andrew Dunstan wrote:
>
> Here's a patch that I think (hope) does this right, by using the file
> pointed to by the environment var PGPASSFILE, if set, in preference to
> $HOME/.pgpass. I assume that at this stage it would be held over for 8.1
> as a new feature - if not I'll put together some docco in a hurry.
>
> cheers
>
> andrew
>
>
>
> Andrew Dunstan wrote:
>
> >
> >
> > Tom Lane wrote:
> >
> >> Andrew Dunstan <andrew@dunslane.net> writes:
> >>
> >>
> >>> How about an environment variable that points to a .pgpass type file.
> >>>
> >>
> >>
> >> You can do that today: point $HOME at some temp directory or other.
> >> AFAIR pg_dump doesn't make any other use of $HOME ...
> >>
> >>
> >>
> >>> Or we could even play games with PGPASSWORD - if it names an
> >>> existing file that satisfies the .pgpass criteria then it will be
> >>> taken as the location of the .pgpass file instead of $HOME/.pgpass -
> >>> otherwise its value will be considered to be the password itself.
> >>>
> >>
> >>
> >> Gaack... if you want a separate variable, we can talk about that, but
> >> let's not overload PGPASSWORD like that.  Consider even just the
> >> implications of whether libpq error messages should echo back the
> >> "filename" ...
> >>
> >>
> >>
> >>
> >
> > Yeah. as usual you're right :-)
> >
> > So let's go woth PGPASSFILE
> >
> > cheers
> >
> > andrew
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> >


>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

--
  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

I have applied this patch, but restructured it to better work in our
code.  Patch attached.  I also added documentation.

Another new addition is that we now will check to see that the password
file is a regular file and not a symlink or something.  This was part of
your patch for PGPASSFILE but I extended it to ~/.pgpass too.

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

Andrew Dunstan wrote:
>
> Here's a patch that I think (hope) does this right, by using the file
> pointed to by the environment var PGPASSFILE, if set, in preference to
> $HOME/.pgpass. I assume that at this stage it would be held over for 8.1
> as a new feature - if not I'll put together some docco in a hurry.
>
> cheers
>
> andrew
>
>
>
> Andrew Dunstan wrote:
>
> >
> >
> > Tom Lane wrote:
> >
> >> Andrew Dunstan <andrew@dunslane.net> writes:
> >>
> >>
> >>> How about an environment variable that points to a .pgpass type file.
> >>>
> >>
> >>
> >> You can do that today: point $HOME at some temp directory or other.
> >> AFAIR pg_dump doesn't make any other use of $HOME ...
> >>
> >>
> >>
> >>> Or we could even play games with PGPASSWORD - if it names an
> >>> existing file that satisfies the .pgpass criteria then it will be
> >>> taken as the location of the .pgpass file instead of $HOME/.pgpass -
> >>> otherwise its value will be considered to be the password itself.
> >>>
> >>
> >>
> >> Gaack... if you want a separate variable, we can talk about that, but
> >> let's not overload PGPASSWORD like that.  Consider even just the
> >> implications of whether libpq error messages should echo back the
> >> "filename" ...
> >>
> >>
> >>
> >>
> >
> > Yeah. as usual you're right :-)
> >
> > So let's go woth PGPASSFILE
> >
> > cheers
> >
> > andrew
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> >


>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

--
  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
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.182
diff -c -c -r1.182 libpq.sgml
*** doc/src/sgml/libpq.sgml    4 Jun 2005 20:42:41 -0000    1.182
--- doc/src/sgml/libpq.sgml    10 Jun 2005 02:55:48 -0000
***************
*** 3713,3718 ****
--- 3713,3729 ----
  <listitem>
  <para>
  <indexterm>
+  <primary><envar>PGPASSFILE</envar></primary>
+ </indexterm>
+ <envar>PGPASSFILE</envar>
+ specifies the name of the password file to use for lookups.
+ If not set, it defaults to <filename>~/.pgpass</>
+ (see <xref linkend="libpq-pgpass">).
+ </para>
+ </listitem>
+ <listitem>
+ <para>
+ <indexterm>
   <primary><envar>PGSERVICE</envar></primary>
  </indexterm>
  <envar>PGSERVICE</envar>
***************
*** 3902,3913 ****
  </indexterm>

  <para>
! The file <filename>.pgpass</filename> in a user's home directory is a file
! that can contain passwords to be used if the connection requires a
! password (and no password has been specified otherwise).
! On Microsoft Windows the file is named
! <filename>%APPDATA%\postgresql\pgpass.conf</> (where <filename>%APPDATA%</>
! refers to the Application Data subdirectory in the user's profile).
  </para>

  <para>
--- 3913,3925 ----
  </indexterm>

  <para>
! The file <filename>.pgpass</filename> in a user's home directory or the
! file referenced by <envar>PGPASSFILE</envar> can contain passwords to
! be used if the connection requires a password (and no password has been
! specified  otherwise). On Microsoft Windows the file is named
! <filename>%APPDATA%\postgresql\pgpass.conf</> (where
! <filename>%APPDATA%</> refers to the Application Data subdirectory in
! the user's profile).
  </para>

  <para>
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.307
diff -c -c -r1.307 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    4 Jun 2005 20:42:43 -0000    1.307
--- src/interfaces/libpq/fe-connect.c    10 Jun 2005 02:55:52 -0000
***************
*** 3217,3225 ****
  PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
  {
      FILE       *fp;
-     char        homedir[MAXPGPATH];
      char        pgpassfile[MAXPGPATH];
      struct stat stat_buf;

  #define LINELEN NAMEDATALEN*5
      char        buf[LINELEN];
--- 3217,3225 ----
  PasswordFromFile(char *hostname, char *port, char *dbname, char *username)
  {
      FILE       *fp;
      char        pgpassfile[MAXPGPATH];
      struct stat stat_buf;
+     char       *passfile_env;

  #define LINELEN NAMEDATALEN*5
      char        buf[LINELEN];
***************
*** 3236,3250 ****
      if (port == NULL)
          port = DEF_PGPORT_STR;

!     if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
!         return NULL;

!     snprintf(pgpassfile, sizeof(pgpassfile), "%s/%s", homedir, PGPASSFILE);

      /* If password file cannot be opened, ignore it. */
      if (stat(pgpassfile, &stat_buf) == -1)
          return NULL;

  #ifndef WIN32
      /* If password file is insecure, alert the user and ignore it. */
      if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
--- 3236,3273 ----
      if (port == NULL)
          port = DEF_PGPORT_STR;

!     if ((passfile_env = getenv("PGPASSFILE")) != NULL)
!     {
!         /* use the literal path from the environment, if set */
!         StrNCpy(pgpassfile, passfile_env, MAXPGPATH);
!         if (!pgpassfile)
!         {
!             fprintf(stderr, libpq_gettext("out of memory\n"));
!             return NULL;
!         }
!     }
!     else
!     {
!         char        homedir[MAXPGPATH];

!         if (!pqGetHomeDirectory(homedir, sizeof(homedir)))
!             return NULL;
!         snprintf(pgpassfile, sizeof(pgpassfile), "%s/%s", homedir, PGPASSFILE);
!     }

      /* If password file cannot be opened, ignore it. */
      if (stat(pgpassfile, &stat_buf) == -1)
          return NULL;

+     if (!S_ISREG(stat_buf.st_mode))
+     {
+         fprintf(stderr,
+                 libpq_gettext("WARNING: Password file %s is not a plain file.\n"),
+                 pgpassfile);
+         free(pgpassfile);
+         return NULL;
+     }
+
  #ifndef WIN32
      /* If password file is insecure, alert the user and ignore it. */
      if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))

Bruce Momjian <pgman@candle.pha.pa.us> writes:
>       char        pgpassfile[MAXPGPATH];

> !         if (!pgpassfile)
> !         {
> !             fprintf(stderr, libpq_gettext("out of memory\n"));
> !             return NULL;
> !         }

Waste of code, eh?

            regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >       char        pgpassfile[MAXPGPATH];
>
> > !         if (!pgpassfile)
> > !         {
> > !             fprintf(stderr, libpq_gettext("out of memory\n"));
> > !             return NULL;
> > !         }
>
> Waste of code, eh?

Never hurts to be too careful.  (removed)  ;-)

--
  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