Escaping ":" in .pgpass - code or docs bug?

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

On Fri, Dec 16, 2011 at 02:55:09PM +0000, 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. reedstrm(at)rice(dot)edu
Systems Engineer & Admin, Research Scientist phone: 713-348-6166
Connexions http://cnx.org fax: 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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Sat, Dec 17, 2011 at 3:27 AM, Ross Reedstrom <reedstrm(at)rice(dot)edu> wrote:
> On Fri, Dec 16, 2011 at 02:55:09PM +0000, 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

On 19/12/11 16:48, Robert Haas wrote:
> On Sat, Dec 17, 2011 at 3:27 AM, Ross Reedstrom<reedstrm(at)rice(dot)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

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 Reedstrom<reedstrm(at)rice(dot)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 <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Mon, Dec 19, 2011 at 2:42 PM, Alvaro Herrera
<alvherre(at)commandprompt(dot)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

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
> <alvherre(at)commandprompt(dot)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 <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support