As of PG 8.3, libpq allows a conninfo string to be passed in via the
dbName parameter of PQsetdbLogin. This is to allow access to conninfo
facilities in old programs that are still using PQsetdbLogin (including
most of our own standard clients ... ahem). For instance

psql "service = foo"

Andrew Dunstan pointed out a possible security hole in this: it will
allow people to do

psql "dbname = mydb password = mypassword"

which would leave their password exposed on the program's command line.

While we cannot absolutely prevent client apps from doing stupid things,
it seems like it might be a good idea to prevent passwords from being
passed in through dbName. The attached patch (which depends on some
pretty-recent changes in CVS HEAD) accomplishes this.

Anybody think this is good, bad, or silly? Does the issue need
explicit documentation, and if so where and how?

regards, tom lane


Index: fe-connect.c
================================================== =================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.354
diff -c -r1.354 fe-connect.c
*** fe-connect.c 9 Dec 2007 19:01:40 -0000 1.354
--- fe-connect.c 11 Dec 2007 02:46:22 -0000
***************
*** 599,604 ****
--- 599,618 ----
{
if (!connectOptions1(conn, dbName))
return conn;
+
+ /*
+ * We disallow supplying a password through dbName, because a large
+ * number of applications allow dbName to be set from a command-line
+ * parameter, and putting a password on your command line is a horrid
+ * idea from a security point of view.
+ */
+ if (conn->pgpass_from_client)
+ {
+ conn->status = CONNECTION_BAD;
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("password must not be set within database name parameter\n"));
+ return conn;
+ }
}
else
{


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

"Joshua D. Drake" <jd@commandprompt.com> writes:
> Tom Lane wrote:
>> As of PG 8.3, libpq allows a conninfo string to be passed in via the
>> dbName parameter of PQsetdbLogin.

> I didn't even know we could do that. I always use the shell variable
> option instead. Does anyone actually use the facility?

Well, not yet, because it's new in 8.3 ...

regards, tom lane

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

Tom Lane wrote:
> As of PG 8.3, libpq allows a conninfo string to be passed in via the
> dbName parameter of PQsetdbLogin. This is to allow access to conninfo
> facilities in old programs that are still using PQsetdbLogin (including
> most of our own standard clients ... ahem). For instance
>
> psql "service = foo"
>
> Andrew Dunstan pointed out a possible security hole in this: it will
> allow people to do
>
> psql "dbname = mydb password = mypassword"
>
> which would leave their password exposed on the program's command line.
>
> While we cannot absolutely prevent client apps from doing stupid things,
> it seems like it might be a good idea to prevent passwords from being
> passed in through dbName. The attached patch (which depends on some
> pretty-recent changes in CVS HEAD) accomplishes this.
>
> Anybody think this is good, bad, or silly? Does the issue need

I didn't even know we could do that. I always use the shell variable
option instead. Does anyone actually use the facility?

> explicit documentation, and if so where and how?

I think it should just throw a syntax error, this isn't covered as an
ability in the man page. I doubt anyone is honestly using this that
isn't smart enough to just figure out it isn't supported.

Joshua D. Drake



---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Anybody think this is good, bad, or silly? Does the issue need
> explicit documentation, and if so where and how?

I'm going to have to vote 'silly' on this one. While I agree that in
general we should discourage, and not provide explicit command-line
options for, passing a password on the command-line, I don't feel that
it makes sense to explicitly complicate things to prevent it.

Just my 2c,

Thanks,

Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHXgDkrzgMPqB3kigRAtzOAJ9py/ppmwfD5Fv4yN/pRxaiWKjHgQCfSXCr
qIFaJ/T1D+CG+NSHoDTpCcQ=
=3mE0
-----END PGP SIGNATURE-----

Tom Lane wrote:
> "Joshua D. Drake" <jd@commandprompt.com> writes:
>> Tom Lane wrote:
>>> As of PG 8.3, libpq allows a conninfo string to be passed in via the
>>> dbName parameter of PQsetdbLogin.
>
>> I didn't even know we could do that. I always use the shell variable
>> option instead. Does anyone actually use the facility?
>
> Well, not yet, because it's new in 8.3 ...

Yeah, let's not do that. Like you said, "While we cannot absolutely
prevent client apps from doing stupid things, it seems like it might be
a good idea to prevent passwords from being passed in through dbName. "

To me... this is something that if we allow, people will use it, and we
will end up removing it, realizing it is a bad idea. There are plenty of
other ways to pass the password in a more sane way.

Sincerely,

Joshua D. Drake


>
> regards, tom lane
>


---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Stephen Frost wrote:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>
>> Anybody think this is good, bad, or silly? Does the issue need
>> explicit documentation, and if so where and how?
>>
>
> I'm going to have to vote 'silly' on this one. While I agree that in
> general we should discourage, and not provide explicit command-line
> options for, passing a password on the command-line, I don't feel that
> it makes sense to explicitly complicate things to prevent it.
>
>
>

It's a matter of being consistent. If we think such a facility shouldn't
be provided on security grounds, then we shouldn't allow it via a
backdoor, ISTM.

cheers

andrew

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

Andrew Dunstan <andrew@dunslane.net> writes:
> Stephen Frost wrote:
>> I'm going to have to vote 'silly' on this one.

> It's a matter of being consistent. If we think such a facility shouldn't
> be provided on security grounds, then we shouldn't allow it via a
> backdoor, ISTM.

Well, the problem with this approach is that libpq has no real means
of knowing whether a string it's been passed was exposed on the command
line or not. dbName might be secure, and for that matter the conninfo
string passed to PQconnectdb might be insecure. Should we put in
arbitrary restrictions on the basis of hypotheses about where these
different arguments came from?

It's also worth noting that we haven't removed the PGPASSWORD
environment variable, even though that's demonstrably insecure on some
platforms.

I'm actually inclined to vote with Stephen that this is a silly change.
I just put up the patch to show the best way of doing it if we're gonna
do it ...

regards, tom lane

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

On Mon, Dec 10, 2007 at 10:47:19PM -0500, Tom Lane wrote:
> Andrew Dunstan <andrew@dunslane.net> writes:
> > Stephen Frost wrote:
> >> I'm going to have to vote 'silly' on this one.
>
> > It's a matter of being consistent. If we think such a facility shouldn't
> > be provided on security grounds, then we shouldn't allow it via a
> > backdoor, ISTM.
>
> Well, the problem with this approach is that libpq has no real means
> of knowing whether a string it's been passed was exposed on the command
> line or not. dbName might be secure, and for that matter the conninfo
> string passed to PQconnectdb might be insecure. Should we put in
> arbitrary restrictions on the basis of hypotheses about where these
> different arguments came from?
>
> It's also worth noting that we haven't removed the PGPASSWORD
> environment variable, even though that's demonstrably insecure on some
> platforms.
>
> I'm actually inclined to vote with Stephen that this is a silly change.
> I just put up the patch to show the best way of doing it if we're gonna
> do it ...

+1 on the silly.

If we want to prevent it for psql, we should actually prevent it *in* psql,
not in libpq. There are an infinite number of scenarios where it's
perfectly safe to put the password there... If we want to do it share, we
should add a function like PQSanitizeConnectionString() that will remove
it, that can be called from those client apps that may be exposing it.

There are also platforms that don't show the full commandline to other
users - or even other processes - that aren't affected, of course.

//Magnus

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

Magnus Hagander wrote:
> On Mon, Dec 10, 2007 at 10:47:19PM -0500, Tom Lane wrote:

> If we want to prevent it for psql, we should actually prevent it *in* psql,
> not in libpq. There are an infinite number of scenarios where it's
> perfectly safe to put the password there... If we want to do it share, we
> should add a function like PQSanitizeConnectionString() that will remove
> it, that can be called from those client apps that may be exposing it.
>
> There are also platforms that don't show the full commandline to other
> users - or even other processes - that aren't affected, of course.

One idea is to have psql "hide" the password on the ps status. That way
it becomes less of a security issue. It would still be a problem on
certain operating systems, but at least several common platforms would
be covered.

--
Alvaro Herrera http://www.flickr.com/photos/alvherre/
Officer Krupke, what are we to do?
Gee, officer Krupke, Krup you! (West Side Story, "Gee, Officer Krupke")

---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

Alvaro Herrera wrote:
> Magnus Hagander wrote:
>> On Mon, Dec 10, 2007 at 10:47:19PM -0500, Tom Lane wrote:
>
>> If we want to prevent it for psql, we should actually prevent it *in* psql,
>> not in libpq. There are an infinite number of scenarios where it's
>> perfectly safe to put the password there... If we want to do it share, we
>> should add a function like PQSanitizeConnectionString() that will remove
>> it, that can be called from those client apps that may be exposing it.
>>
>> There are also platforms that don't show the full commandline to other
>> users - or even other processes - that aren't affected, of course.
>
> One idea is to have psql "hide" the password on the ps status. That way
> it becomes less of a security issue. It would still be a problem on
> certain operating systems, but at least several common platforms would
> be covered.

There would still be race condition. It would still be visible until
psql hides it. In a way that would be even worse, because it wouldn't be
obvious to an administrator that there's a problem because the password
wouldn't be visible in ps output, but hackers know about stuff like that.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq