Re: [HACKERS] Escaping : in .pgpass - code or docs bug?

2011-12-22 Thread Robert Haas
On Mon, Dec 19, 2011 at 2:42 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Yeah, it is a pretty old bug -- this code was clearly written by some
 rookie that didn't know very well what he was doing.

Hey, I got that joke.

I fixed this in master.  I'm not going to bother with anything else
unless someone else feels motivated to make it happen.

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

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


Re: [HACKERS] Escaping : in .pgpass - code or docs bug?

2011-12-22 Thread Alvaro Herrera

Excerpts from Robert Haas's message of jue dic 22 15:03:36 -0300 2011:
 
 On Mon, Dec 19, 2011 at 2:42 PM, Alvaro Herrera
 alvhe...@commandprompt.com wrote:
  Yeah, it is a pretty old bug -- this code was clearly written by some
  rookie that didn't know very well what he was doing.
 
 Hey, I got that joke.
 
 I fixed this in master.  I'm not going to bother with anything else
 unless someone else feels motivated to make it happen.

Thanks! :-)

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Escaping : in .pgpass - code or docs bug?

2011-12-19 Thread Robert Haas
On Sat, Dec 17, 2011 at 3:27 AM, Ross Reedstrom reeds...@rice.edu wrote:
 On Fri, Dec 16, 2011 at 02:55:09PM +, Richard Huxton wrote:
 According to the docs [1], you should escape embedded colons in
 .pgpass (fair enough). Below is PG 9.1.1

 user = te:st, db = te:st, password = te:st

     $ cat ~/.pgpass
     *:*:te:st:te:st:te:st
     $ psql91 -U te:st -d te:st
     te:st=

     $ cat ~/.pgpass
     *:*:te\:st:te\:st:te:st
     $ psql91 -U te:st -d te:st
     te:st=

     $ cat ~/.pgpass
     *:*:te\:st:te\:st:te\:st
     $ psql91 -U te:st -d te:st
     psql: FATAL:  password authentication failed for user te:st
     password retrieved from file /home/richardh/.pgpass

 I'm a bit puzzled how it manages without the escaping in the first
 case. There's a lack of consistency though that either needs
 documenting or fixing.

 Hmm, seems the code in fe-connect.c that reads the password out of .pgpass 
 does this:

    if ((t = pwdfMatchesString(t, hostname)) == NULL ||
                        (t = pwdfMatchesString(t, port)) == NULL ||
                        (t = pwdfMatchesString(t, dbname)) == NULL ||
                        (t = pwdfMatchesString(t, username)) == NULL)
  [...]

 pwdfMatchesString 'eats' the stringbuffer until the next unmatched character 
 or
 unescaped colon.  If it falls out the bottom of that, the rest of the line is
 returned as the candidate password.

 Since the code that does the backslash detection is in pwdfMatchesString(), 
 and
 the password never goes through that function, the escapes are not cleaned up.

 This should either be fixed by changing the documentation to say to not escape
 colons or backslashes in the password part, only, or modify this function
 (PasswordFromFile) to silently unescape the password string. It already copies
 it.

My vote is for a doc correction in the back-branches and a behavior
change in master.

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

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


Re: [HACKERS] Escaping : in .pgpass - code or docs bug?

2011-12-19 Thread Richard Huxton

On 19/12/11 16:48, Robert Haas wrote:

On Sat, Dec 17, 2011 at 3:27 AM, Ross Reedstromreeds...@rice.edu  wrote:

This should either be fixed by changing the documentation to say to not escape
colons or backslashes in the password part, only, or modify this function
(PasswordFromFile) to silently unescape the password string. It already copies
it.


My vote is for a doc correction in the back-branches and a behavior
change in master.


Seems sensible - presumably mentioning this will be corrected in 9.2?

It's clearly not what you'd call urgent since nobody else seems to 
have noticed before now.


--
  Richard Huxton
  Archonet Ltd

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


Re: [HACKERS] Escaping : in .pgpass - code or docs bug?

2011-12-19 Thread Alvaro Herrera

Excerpts from Richard Huxton's message of lun dic 19 16:33:31 -0300 2011:
 On 19/12/11 16:48, Robert Haas wrote:
  On Sat, Dec 17, 2011 at 3:27 AM, Ross Reedstromreeds...@rice.edu  wrote:
  This should either be fixed by changing the documentation to say to not 
  escape
  colons or backslashes in the password part, only, or modify this function
  (PasswordFromFile) to silently unescape the password string. It already 
  copies
  it.
 
  My vote is for a doc correction in the back-branches and a behavior
  change in master.
 
 Seems sensible - presumably mentioning this will be corrected in 9.2?
 
 It's clearly not what you'd call urgent since nobody else seems to 
 have noticed before now.

Yeah, it is a pretty old bug -- this code was clearly written by some
rookie that didn't know very well what he was doing.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Escaping : in .pgpass - code or docs bug?

2011-12-17 Thread Ross Reedstrom
On Fri, Dec 16, 2011 at 02:55:09PM +, Richard Huxton wrote:
 According to the docs [1], you should escape embedded colons in
 .pgpass (fair enough). Below is PG 9.1.1
 
 user = te:st, db = te:st, password = te:st
 
 $ cat ~/.pgpass
 *:*:te:st:te:st:te:st
 $ psql91 -U te:st -d te:st
 te:st=
 
 $ cat ~/.pgpass
 *:*:te\:st:te\:st:te:st
 $ psql91 -U te:st -d te:st
 te:st=
 
 $ cat ~/.pgpass
 *:*:te\:st:te\:st:te\:st
 $ psql91 -U te:st -d te:st
 psql: FATAL:  password authentication failed for user te:st
 password retrieved from file /home/richardh/.pgpass
 
 I'm a bit puzzled how it manages without the escaping in the first
 case. There's a lack of consistency though that either needs
 documenting or fixing.

Hmm, seems the code in fe-connect.c that reads the password out of .pgpass does 
this:

if ((t = pwdfMatchesString(t, hostname)) == NULL ||
(t = pwdfMatchesString(t, port)) == NULL ||
(t = pwdfMatchesString(t, dbname)) == NULL ||
(t = pwdfMatchesString(t, username)) == NULL)
 [...]

pwdfMatchesString 'eats' the stringbuffer until the next unmatched character or
unescaped colon.  If it falls out the bottom of that, the rest of the line is
returned as the candidate password.

Since the code that does the backslash detection is in pwdfMatchesString(), and
the password never goes through that function, the escapes are not cleaned up.

This should either be fixed by changing the documentation to say to not escape
colons or backslashes in the password part, only, or modify this function
(PasswordFromFile) to silently unescape the password string. It already copies
it. 

Perhaps something like (WARNING! untested code, rusty C skills):


*** src/interfaces/libpq/fe-connect.c.orig  2011-12-16 17:44:29.265913914 
-0600
--- src/interfaces/libpq/fe-connect.c   2011-12-16 17:46:46.137913871 -0600
***
*** 4920,4925 
--- 4920,4933 
continue;
ret = strdup(t);
fclose(fp);
+ 
+ /* unescape any residual escaped colons */
+ t = ret;
+ while (t[0]) {
+ if (t[0] == '\\'  (t[1] == ':' || t[1] == '\\'))
+ strcpy(t,t+1);
+ t++;
+ 
return ret;
}
  
This would be backward compatible, in that existing working passwords would
continue to work, unless they happen to contain exactly the string '\:' or
'\\', then they'd need to double the backslash.

Ross
-- 
Ross Reedstrom, Ph.D. reeds...@rice.edu
Systems Engineer  Admin, Research Scientistphone: 713-348-6166
Connexions  http://cnx.orgfax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E  F888 D3AE 810E 88F0 BEDE

 
 
 [1] http://www.postgresql.org/docs/9.1/static/libpq-pgpass.html
 
 -- 
   Richard Huxton
   Archonet Ltd
 
 -- 
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers
 

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


[HACKERS] Escaping : in .pgpass - code or docs bug?

2011-12-16 Thread Richard Huxton
According to the docs [1], you should escape embedded colons in .pgpass 
(fair enough). Below is PG 9.1.1


user = te:st, db = te:st, password = te:st

$ cat ~/.pgpass
*:*:te:st:te:st:te:st
$ psql91 -U te:st -d te:st
te:st=

$ cat ~/.pgpass
*:*:te\:st:te\:st:te:st
$ psql91 -U te:st -d te:st
te:st=

$ cat ~/.pgpass
*:*:te\:st:te\:st:te\:st
$ psql91 -U te:st -d te:st
psql: FATAL:  password authentication failed for user te:st
password retrieved from file /home/richardh/.pgpass

I'm a bit puzzled how it manages without the escaping in the first case. 
There's a lack of consistency though that either needs documenting or 
fixing.



[1] http://www.postgresql.org/docs/9.1/static/libpq-pgpass.html

--
  Richard Huxton
  Archonet Ltd

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