Re: Re: BUG #5065: pg_ctl start fails as administrator, with "could not locate matching postgres executable"

From: "Jesse Morris" <jmorris(at)coverity(dot)com>
To: "Andrew Dunstan" <andrew(at)dunslane(dot)net>, "Dave Page" <dpage(at)pgadmin(dot)org>
Cc: <pgsql-bugs(at)postgresql(dot)org>, "Magnus Hagander" <magnus(at)hagander(dot)net>
Subject: Re: Re: BUG #5065: pg_ctl start fails as administrator, with "could not locate matching postgres executable"
Date: 2009-10-19 22:21:14
Message-ID: FEFD432EA8DD2D45B110ACE93F8200FF05284C9B@sf-ex-1.migcoverity.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Andrew,

First the really important stuff: I (correctly) changed the comment
from "it's" to "its" in one place, but didn't notice the other.

As to "is doing this safe?" I'm not sure whether you are asking about my
patch, or about the general approach that I made the slight tweaks to,
but it's probably worth talking about both. I'm going to start with
describing how I understand what's going on. If my understanding of the
windows security model is wrong, then my patch might be flawed. If my
understanding of the windows security model is right, then the general
approach which I made minor changes to might be flawed.

How a thread / process interacts with the system is determined by its
security token. This token belongs to a user owner and a list of
groups. The token also has an list of rights (e.g.,
"SeTakeOwnershipPrivilege"), and a default DACL. The default DACL is
like a umask; new objects the process creates will have that DACL.

Postmaster (that's what I'm going to call postgres.exe) checks that its
token does not belong to the Administrators or Power Users groups, and
refuses to run if it is.

pg_ctl/initdb both run postmaster with a restricted token that disables
the Administrators and Power User groups. Postmaster is satisfied with
this.

However, by default on Win2k3 (and possibly elsewhere?), Administrators'
processes' default DACLs only grants access to {System, Administrators}.
That's the whole list. Postmaster spawns a child postmaster, and they
create a pipe to communicate. But the postmaster is not owned by System
and pg_ctl disabled its Administrators membership, so that pipe is not
accessible: Access denied.

On the Vista and Windows 7 machines I've looked at there's also a Login
SID (S-1-5-5-0-<rand_num>) in both the token's group membership and in
the default DACL. In that case both parent and child process can access
the created pipe and there are no problems. Cygwin, ActivePerl, and
msys all add the user that owns the process to the default DACL, so if
you run pg_ctl with one of those above you in the process tree, it will
work fine.

The code I changed was trying to add the user to the DACL. It doesn't
grant the postmaster processes any additional inherent rights, it just
says "your children processes are all accessible by the creating user
(which also has full access to the database cluster on disk anyway). I
didn't write it though; I just changed how it was doing that. I was
never able to get the original code to work at all, though Dave Page
says he can't even reproduce the failure, so I don't know what we're
doing differently. MSDN says there's a "SeAssignPrimaryTokenPrivilege"
which is required to replace a process-level token, so maybe that's why
it's wasn't working for me - but I wasn't seeing any win32 calls fail,
so I dunno.

Now: To answer your question of "What if the Administrator user is
granted additional permissions?"

The answer is the same with and without my patch. If postgres is run as
a user that has additional permissions, postgres will have additional
permissions. It is unusual to do this though; usually that is all
managed through group membership. "Administrator" is not special in
this regard; I can give the "guest" account full control over everything
if I want.

If you do want to defend against that (personally I'm indifferent), it
wouldn't be difficult to drop unnecessary permissions from the
restricted token that the daemon is already being run with. Postmaster
can verify that anything "dangerous" is not allowed from the backend
startup check. Actually, I'm not sure if *any* of these are necessary:
http://msdn.microsoft.com/en-us/library/bb530716%28VS.85%29.aspx

This wouldn't be a substitute for disabling the Administrators and Power
Users group membership; those still affect a lot of important files.

I personally don't need postgres to be more defensive, I just need it to
start successfully. So even if the above IS a good idea, someone else
will have to implement it.

And by all means, include whatever of this is correct/helpful in code
comments. I don't have to submit a separate patch for that, do I?

-Jesse

> -----Original Message-----
> From: Andrew Dunstan [mailto:andrew(at)dunslane(dot)net]
> Sent: Monday, October 19, 2009 11:03 AM
> To: Dave Page
> Cc: Jesse Morris; pgsql-bugs(at)postgresql(dot)org; Magnus Hagander
> Subject: Re: [BUGS] Re: BUG #5065: pg_ctl start fails as
administrator,
> with "could not locate matching postgres executable"
>
> It looks OK to me (modulo the incorrect changing of "its" to "it's" in
> a comment - whoever did that was trying to make it consistent, but
> unfortunately made it consistently wrong).
>
> However, I'd like a bit more comment added on just why doing this is
> safe. Would it still be safe if someone granted some dangerous
> privilege directly to the Administrator user, if that's possible?
>
> cheers
>
> andrew

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Craig Ringer 2009-10-20 03:32:49 Re:
Previous Message steffen nielsen 2009-10-19 21:04:16

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2009-10-19 22:26:14 Re: LATERAL
Previous Message marcin mank 2009-10-19 22:19:14 Re: per table random-page-cost?