Thread: [PATCH] pg_autovacuum commandline password hiding.
Hi
I'm not sure if you've done this for a later version of pg_autovacuum.
I'm using what came with postgres-7.4.6. For database security on
a shared server (~800 logins) it's best to set the superuser password
and not allow passwordless connections. The only thing is that
pg_autovacuum keeps the password supplied on the commandline so
anyone that does a 'ps' can get the database superuser password.
--- pg_autovacuum.c.orig Mon Apr 18 08:08:27 2005
+++ pg_autovacuum.c Mon Apr 18 07:57:59 2005
@@ -879,7 +879,8 @@
args->user = optarg;
break;
case 'P':
- args->password = optarg;
+ args->password = strdup(optarg);
+ for (c = 0; optarg[c]; optarg[c++] = 'x');
break;
case 'H':
args->host = optarg;
I hope that this is a worthwhile patch.
Ian
--
Ian Freislich
Ian FREISLICH wrote: > I'm not sure if you've done this for a later version of pg_autovacuum. > I'm using what came with postgres-7.4.6. For database security on > a shared server (~800 logins) it's best to set the superuser password > and not allow passwordless connections. The only thing is that > pg_autovacuum keeps the password supplied on the commandline so > anyone that does a 'ps' can get the database superuser password. Is this portable? Considering the hoops that backend/utils/misc/ps_status.c jumps through to do something similar for the postmaster, I would guess not. BTW, I would suggest using ~/.pgpass, as that should be secure on all platforms. -Neil
Neil Conway wrote: > Ian FREISLICH wrote: > > I'm not sure if you've done this for a later version of pg_autovacuum. > > I'm using what came with postgres-7.4.6. For database security on > > a shared server (~800 logins) it's best to set the superuser password > > and not allow passwordless connections. The only thing is that > > pg_autovacuum keeps the password supplied on the commandline so > > anyone that does a 'ps' can get the database superuser password. > > Is this portable? Considering the hoops that > backend/utils/misc/ps_status.c jumps through to do something similar for > the postmaster, I would guess not. > > BTW, I would suggest using ~/.pgpass, as that should be secure on all > platforms. I have no idea. It works on FreeBSD and Linux which are the two platform I have access to. Does pg_autovacuum use ~/.pgpass? Ian -- Ian Freislich
Ian FREISLICH wrote: > Does pg_autovacuum use ~/.pgpass? Yes (any libpq-based app will). -Neil
Ian FREISLICH <if@hetzner.co.za> writes:
> ... The only thing is that
> pg_autovacuum keeps the password supplied on the commandline so
> anyone that does a 'ps' can get the database superuser password.
Which is exactly why we don't (and won't) provide such a switch.
Use ~/.pgpass instead.
regards, tom lane
> -----Original Message----- > From: pgsql-patches-owner@postgresql.org > [mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Tom Lane > Sent: 24 May 2005 15:17 > To: Ian FREISLICH > Cc: pgsql-patches@postgresql.org > Subject: Re: [PATCHES] [PATCH] pg_autovacuum commandline > password hiding. > > Ian FREISLICH <if@hetzner.co.za> writes: > > ... The only thing is that > > pg_autovacuum keeps the password supplied on the commandline so > > anyone that does a 'ps' can get the database superuser password. > > Which is exactly why we don't (and won't) provide such a switch. Err, yes we do: root@zankou:~# ~postgres/bin/pg_autovacuum -h usage: pg_autovacuum [-D] Daemonize (Detach from tty and run in the background) [-d] debug (debug level=0,1,2,3; default=0) [-s] sleep base value (default=300) [-S] sleep scaling factor (default=2.000000) [-v] vacuum base threshold (default=1000) [-V] vacuum scaling factor (default=2.000000) [-a] analyze base threshold (default=500) [-A] analyze scaling factor (default=1.000000) [-L] logfile (default=none) [-c] vacuum_cost_delay (default=none) [-C] vacuum_cost_page_hit (default=none) [-m] vacuum_cost_page_miss (default=none) [-n] vacuum_cost_page_dirty (default=none) [-l] vacuum_cost_limit (default=none) [-U] username (libpq default) [-P] password (libpq default) [-H] host (libpq default) [-p] port (libpq default) [-h] help (Show this output) :-( Regards, Dave.
"Dave Page" <dpage@vale-housing.co.uk> writes:
>> Which is exactly why we don't (and won't) provide such a switch.
> Err, yes we do:
Um, sorry, I totally misread Ian's patch as a proposal that we add a
password switch (I hate unidiffs ;-)).
I would argue actually that this switch is a horrible idea and we
must take it out entirely. The method Ian proposes for hiding the
password after reading it is certainly not portable in the slightest,
and even if we could make it work on all platforms (which we can't)
I don't think it would be good enough, because there would still be
a window where the superuser password was exposed to view before
we could wipe it out.
psql, pg_dump, etc allow password specification from stdin and from
.pgpass, never on the command line. There is a reason why they are all
designed like that. pg_autovacuum hasn't been studied carefully enough
I guess, because we should never have let a security hole like this get
by us.
regards, tom lane
Tom Lane wrote: >psql, pg_dump, etc allow password specification from stdin and from >.pgpass, never on the command line. There is a reason why they are all >designed like that. pg_autovacuum hasn't been studied carefully enough >I guess, because we should never have let a security hole like this get >by us. > > > > I agree. And while we're on the topic, my patch from last year to allow setting an alternative location for the pgpass file via the environment seems to be lingering in the pgpatches2 queue. I know some clients use the environment to pass the password directly (also very insecure) because they can't specify the passfile location. cheers andrew
> -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > Sent: 24 May 2005 16:02 > To: Dave Page > Cc: Ian FREISLICH; pgsql-patches@postgresql.org > Subject: Re: [PATCHES] [PATCH] pg_autovacuum commandline > password hiding. > > "Dave Page" <dpage@vale-housing.co.uk> writes: > >> Which is exactly why we don't (and won't) provide such a switch. > > > Err, yes we do: > > Um, sorry, I totally misread Ian's patch as a proposal that we add a > password switch (I hate unidiffs ;-)). :-) > I would argue actually that this switch is a horrible idea and we > must take it out entirely. The method Ian proposes for hiding the > password after reading it is certainly not portable in the slightest, > and even if we could make it work on all platforms (which we can't) > I don't think it would be good enough, because there would still be > a window where the superuser password was exposed to view before > we could wipe it out. Agreed. The attached patch should do the trick. Regards, Dave
Attachment
Dave Page wrote: >>-----Original Message----- >>From: Tom Lane [mailto:tgl@sss.pgh.pa.us] >> >> >>I would argue actually that this switch is a horrible idea and we >>must take it out entirely. The method Ian proposes for hiding the >>password after reading it is certainly not portable in the slightest, >>and even if we could make it work on all platforms (which we can't) >>I don't think it would be good enough, because there would still be >>a window where the superuser password was exposed to view before >>we could wipe it out. >> >> > >Agreed. The attached patch should do the trick. > From a cursory glance at the patch it looks OK to me. I don't have time to test it right now. Sorry about this, I should have removed the username / password switches a while ago.
Tom Lane wrote:
> psql, pg_dump, etc allow password specification from stdin and from
> .pgpass, never on the command line. There is a reason why they are all
> designed like that. pg_autovacuum hasn't been studied carefully enough
> I guess, because we should never have let a security hole like this get
> by us.
It has certainly been observed that this is a security problem in the
past. In fact, the pg_autovacuum documentation makes that clear:
-P password: Password pg_autovacuum will use to connect with. *WARNING*
This option is insecure. When installed as a Windows Service, this
option will be stored in plain text in the registry. When used with
most Unix variants, other users will be able to see the argument to
the "-P" option via ps(1). The ~/.pgpass file can be used to
specify a password more securely.
I think the reason there is at least some value in having this switch
for pg_autovacuum is that pg_autovacuum is almost exclusively used in a
situation in which the password can't be specified on the command-line
(which is not the case for most of the other command-line tools). Sure,
it isn't secure, but the documentation makes that clear, and security is
not important to everyone -- I can certainly envision users for whom the
command-line flag is convenient.
-Neil
Neil Conway wrote: > I think the reason there is at least some value in having this switch > for pg_autovacuum is that pg_autovacuum is almost exclusively used in a > situation in which the password can't be specified on the command-line Sorry, thinko: I meant interactively via the terminal. -Neil
Neil Conway <neilc@samurai.com> writes:
> Neil Conway wrote:
>> I think the reason there is at least some value in having this switch
>> for pg_autovacuum is that pg_autovacuum is almost exclusively used in a
>> situation in which the password can't be specified on the command-line
> Sorry, thinko: I meant interactively via the terminal.
Right. I don't think it'd be worth the trouble to implement the
equivalent of -W (get the password from stdin), since as you say
the use-case for that is pretty tiny for autovacuum.
The question at hand is whether we want to support an obvious security
hole. The argument that "some people will not care" applies with at
least as much force to psql or pg_dump, which at least have the grace
to not hang around and advertise their command-line parameters forever.
I think that using -P for pg_autovacuum is just plain stupid, even on a
nominally secure single-user box. If you believe your box is secure,
why are you using password auth for local connections in the first
place? Might as well set it up as "trust". You certainly shouldn't
imagine that the password is securing anything when an always-on daemon
is advertising it to the world in its command line.
regards, tom lane
Tom Lane wrote: > The question at hand is whether we want to support an obvious security > hole. The argument that "some people will not care" applies with at > least as much force to psql or pg_dump, which at least have the grace > to not hang around and advertise their command-line parameters forever. > I think that using -P for pg_autovacuum is just plain stupid, even on a > nominally secure single-user box. Assuming that command-line parameters are actually globally visible on your platform, which isn't necessarily the case. Anyway, I basically agree that the legitimate use-case for this feature is pretty small, and it is probably worth removing. However, I don't think it is urgent (there are plenty of other ways to shoot yourself in the foot), and shouldn't be backpatched -- people may be using this functionality. -Neil
Neil Conway <neilc@samurai.com> writes:
> Tom Lane wrote:
>> I think that using -P for pg_autovacuum is just plain stupid, even on a
>> nominally secure single-user box.
> Assuming that command-line parameters are actually globally visible on
> your platform, which isn't necessarily the case.
I don't offhand know of any Unix platforms where they cannot be found
out --- you may have to use non-default ps arguments, but you can see
'em. I do know of platforms where they cannot be hidden (Solaris for
instance).
> Anyway, I basically agree that the legitimate use-case for this feature
> is pretty small, and it is probably worth removing. However, I don't
> think it is urgent (there are plenty of other ways to shoot yourself in
> the foot), and shouldn't be backpatched -- people may be using this
> functionality.
My inclination is to remove this from HEAD (where it may be moot anyway
by the time 8.1 freezes) and from 8.0.
regards, tom lane
Tom Lane wrote: > I don't offhand know of any Unix platforms where they cannot be found > out I don't know which platforms it is secure/insecure on, but I can certainly imagine secure systems where ps(1) data in general is viewed as sensitive and thus not made globally visible. > My inclination is to remove this from HEAD (where it may be moot anyway > by the time 8.1 freezes) and from 8.0. I don't think there is sufficient justification for removing this feature and breaking users of a stable release series. The documentation states this feature is likely insecure, so presumably people who are using it are aware of the consequences. -Neil
Neil Conway <neilc@samurai.com> writes:
> Tom Lane wrote:
>> I don't offhand know of any Unix platforms where they cannot be found
>> out
> I don't know which platforms it is secure/insecure on, but I can
> certainly imagine secure systems where ps(1) data in general is viewed
> as sensitive and thus not made globally visible.
It's imaginable, but can you point to any real examples? The historical
tradition is that command-line parameters are visible, and therefore
Unix programs are invariably designed to not expose security information
on the command line, and therefore there is no security motivation to
hide command lines. It's a tight little cause-and-effect loop.
Unfortunately, pg_autovacuum didn't get the word, and so we are creating
an opportunity for people to shoot themselves in the foot. I think
that's a bug to be fixed.
> I don't think there is sufficient justification for removing this
> feature and breaking users of a stable release series.
"Breaking" obviously-insecure usages is exactly the intention.
regards, tom lane
Tom Lane wrote: > Neil Conway <neilc@samurai.com> writes: >>I don't know which platforms it is secure/insecure on, but I can >>certainly imagine secure systems where ps(1) data in general is viewed >>as sensitive and thus not made globally visible. > > > It's imaginable, but can you point to any real examples? FreeBSD's MAC (security.mac.seeotheruids.enabled sysctl) and the Openwall Linux kernel patch are the first examples I found, but I didn't spend long searching. >>I don't think there is sufficient justification for removing this >>feature and breaking users of a stable release series. > > "Breaking" obviously-insecure usages is exactly the intention. But it's not "obviously-insecure". In some situations it is perfectly secure (or security isn't important), but there are better alternatives (e.g. using trust authentication, as you suggest). -Neil