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

From: Ross Reedstrom <reedstrm(at)rice(dot)edu>
To: Richard Huxton <dev(at)archonet(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: Escaping ":" in .pgpass - code or docs bug?
Date: 2011-12-17 08:27:54
Message-ID: 20111217082754.GB30887@rice.edu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nikhil Sontakke 2011-12-17 09:03:42 Re: Review: Non-inheritable check constraints
Previous Message Tom Lane 2011-12-17 07:25:35 Re: WIP: SP-GiST, Space-Partitioned GiST