Thread: Fix for initdb failures on Vista
The attached patch fixes problems reported primarily on Vista, but also on some Windows 2003 and XP installations in which initdb reports that it cannot find postgres.exe. This occurs because of security-related changes implemented in Windows Vista and recent patches on older OS's. When running initdb or pg_ctl we currently create a restricted security token with the Administrators and Power Users groups (and thus their privileges) removed and re-execute the same program using the restricted token. This ensures that the process is run without potentially dangerous privileges no matter what user account it was started from. On Vista and friends however, the default DACL (list of Access Control Entries) used in the restricted token contains Administrators (the group) & System when we run as Administrator, vs. User + System when run as other users. Because we then drop Administrators, we are left with only the System ACE in the DACL, which does not allow us to use CreatePipe()/CreateProcess(). To fix this, when we create the restricted process, we initially start it in suspended mode. We modify it's DACL to explicitly add an ACE for the current user, and then resume the child process. This remains secure because administrative privileges are granted to the groups that we've dropped, not the user itself. I've tested on Vista and XP, but additional testing would be useful (Andrew, Magnus?). Please apply to head, 8.3 and 8.2 -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com The Oracle-compatible database company
Attachment
Dave Page wrote: > The attached patch fixes problems reported primarily on Vista, but > also on some Windows 2003 and XP installations in which initdb reports > that it cannot find postgres.exe. A couple of minor nitpicks: Regarding the AddUserToDaclCleanup helper function, I would suggest putting all the cleanups at the end of AddUserToDacl, jump to the cleanup section with a goto. That's a commonly used pattern to do it. One problem with the Cleanup function is that if you need to add more cleanup code (probably not likely in this case, though), you need to modify the function signature and all callers. The comment in AddUserToDacl says "This is required on Windows machines running some of Microsoft's latest security patches on XP/2K3, and on Vista/Longhorn boxes". The security patches we're talking about are not going to be the latest for very long; might want to rephrase that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Thu, Feb 21, 2008 at 03:02:07PM +0000, Dave Page wrote: > The attached patch fixes problems reported primarily on Vista, but > also on some Windows 2003 and XP installations in which initdb reports > that it cannot find postgres.exe. > > This occurs because of security-related changes implemented in Windows > Vista and recent patches on older OS's. When running initdb or pg_ctl > we currently create a restricted security token with the > Administrators and Power Users groups (and thus their privileges) > removed and re-execute the same program using the restricted token. > This ensures that the process is run without potentially dangerous > privileges no matter what user account it was started from. On Vista > and friends however, the default DACL (list of Access Control Entries) > used in the restricted token contains Administrators (the group) & > System when we run as Administrator, vs. User + System when run as > other users. Because we then drop Administrators, we are left with > only the System ACE in the DACL, which does not allow us to use > CreatePipe()/CreateProcess(). > > To fix this, when we create the restricted process, we initially start > it in suspended mode. We modify it's DACL to explicitly add an ACE for > the current user, and then resume the child process. This remains > secure because administrative privileges are granted to the groups > that we've dropped, not the user itself. > > I've tested on Vista and XP, but additional testing would be useful > (Andrew, Magnus?). Please apply to head, 8.3 and 8.2 Other than Heikkis comments: We obviously need to test-build on mingw, so if someone can do that, pleae do. If not, I'll try to get my VM up and running on it (since mingw doesn't work on my win64 box). I'm also a bit concerned that there is a whole lot of failure cases in AddUserToDacl() that all return the same, thus making it impossible to track down a problem in this code. Given that it's fairly complex interactions with the API, I think we'll want to add actual error messages to the individual failure cases. Thoughts? //Magnus
Magnus Hagander wrote: >> I've tested on Vista and XP, but additional testing would be useful >> (Andrew, Magnus?). Please apply to head, 8.3 and 8.2 >> > > Other than Heikkis comments: > > We obviously need to test-build on mingw, so if someone can do that, pleae > do. If not, I'll try to get my VM up and running on it (since mingw doesn't > work on my win64 box). > > > I'll look at testing mingw on my 32bit Vista box. cheers andrew
Dave Page wrote: > The attached patch fixes problems reported primarily on Vista, but > also on some Windows 2003 and XP installations in which initdb reports > that it cannot find postgres.exe. > > This occurs because of security-related changes implemented in Windows > Vista and recent patches on older OS's. When running initdb or pg_ctl > we currently create a restricted security token with the > Administrators and Power Users groups (and thus their privileges) > removed and re-execute the same program using the restricted token. > This ensures that the process is run without potentially dangerous > privileges no matter what user account it was started from. On Vista > and friends however, the default DACL (list of Access Control Entries) > used in the restricted token contains Administrators (the group) & > System when we run as Administrator, vs. User + System when run as > other users. Because we then drop Administrators, we are left with > only the System ACE in the DACL, which does not allow us to use > CreatePipe()/CreateProcess(). > > To fix this, when we create the restricted process, we initially start > it in suspended mode. We modify it's DACL to explicitly add an ACE for > the current user, and then resume the child process. This remains > secure because administrative privileges are granted to the groups > that we've dropped, not the user itself. > > I've tested on Vista and XP, but additional testing would be useful > (Andrew, Magnus?). Please apply to head, 8.3 and 8.2 > > This appears to work for initdb. But "make check" fails after the initdb stage, I think because pg_regress doesn't use pg_ctl to start the postmaster. The log just reads "Access is denied'" I don't have too much difficulty with that as long as we stipulate that postgres has to be built, or at least checked, as a non-privileged user (c.f. recent discussion of building RPMs as root). Alternatively, we should also patch pg_regress.c cheers andrew
On Fri, Feb 29, 2008 at 12:17:51AM -0500, Andrew Dunstan wrote: > > > Dave Page wrote: > >The attached patch fixes problems reported primarily on Vista, but > >also on some Windows 2003 and XP installations in which initdb reports > >that it cannot find postgres.exe. > > > >This occurs because of security-related changes implemented in Windows > >Vista and recent patches on older OS's. When running initdb or pg_ctl > >we currently create a restricted security token with the > >Administrators and Power Users groups (and thus their privileges) > >removed and re-execute the same program using the restricted token. > >This ensures that the process is run without potentially dangerous > >privileges no matter what user account it was started from. On Vista > >and friends however, the default DACL (list of Access Control Entries) > >used in the restricted token contains Administrators (the group) & > >System when we run as Administrator, vs. User + System when run as > >other users. Because we then drop Administrators, we are left with > >only the System ACE in the DACL, which does not allow us to use > >CreatePipe()/CreateProcess(). > > > >To fix this, when we create the restricted process, we initially start > >it in suspended mode. We modify it's DACL to explicitly add an ACE for > >the current user, and then resume the child process. This remains > >secure because administrative privileges are granted to the groups > >that we've dropped, not the user itself. > > > >I've tested on Vista and XP, but additional testing would be useful > >(Andrew, Magnus?). Please apply to head, 8.3 and 8.2 > > > > > > This appears to work for initdb. But "make check" fails after the initdb > stage, I think because pg_regress doesn't use pg_ctl to start the > postmaster. The log just reads "Access is denied'" > > I don't have too much difficulty with that as long as we stipulate that > postgres has to be built, or at least checked, as a non-privileged user > (c.f. recent discussion of building RPMs as root). Alternatively, we > should also patch pg_regress.c > I noticed that as well when looking at the code, but since I ran my tests on non-vista platforms I didn't hit the actual problem. Dave - it shuold be a simple case of adding the same line of code to the regression tests, no? Meanwhile, I'll apply what we have with my additios and cleanups per mine and Heikkis comments, because they fix the most important codepaths. //Magnus
On Fri, Feb 29, 2008 at 3:25 PM, Magnus Hagander <magnus@hagander.net> wrote: > > I noticed that as well when looking at the code, but since I ran my tests > on non-vista platforms I didn't hit the actual problem. > > Dave - it shuold be a simple case of adding the same line of code to the > regression tests, no? Yeah, that should do it. > Meanwhile, I'll apply what we have with my additios and cleanups per mine > and Heikkis comments, because they fix the most important codepaths. Thanks for putting the final polish on this. -- Dave Page EnterpriseDB UK Ltd: http://www.enterprisedb.com PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
Dave Page wrote: > On Fri, Feb 29, 2008 at 3:25 PM, Magnus Hagander <magnus@hagander.net> wrote: >> I noticed that as well when looking at the code, but since I ran my tests >> on non-vista platforms I didn't hit the actual problem. >> >> Dave - it shuold be a simple case of adding the same line of code to the >> regression tests, no? > > Yeah, that should do it. Will you provide a patch for that, or should I? (I remind you I have no box ready to reproduce it on, so it helps a lot if you can do it :-P) //Magnus
On Fri, Feb 29, 2008 at 6:41 PM, Magnus Hagander <magnus@hagander.net> wrote: > Will you provide a patch for that, or should I? (I remind you I have no > box ready to reproduce it on, so it helps a lot if you can do it :-P) Patch attached. Tested on XP and Vista. -- Dave Page EnterpriseDB UK Ltd: http://www.enterprisedb.com PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
Attachment
On Tue, Mar 04, 2008 at 03:28:30PM +0000, Dave Page wrote: > On Fri, Feb 29, 2008 at 6:41 PM, Magnus Hagander <magnus@hagander.net> wrote: > > Will you provide a patch for that, or should I? (I remind you I have no > > box ready to reproduce it on, so it helps a lot if you can do it :-P) > > Patch attached. Tested on XP and Vista. Applied to HEAD and 8.3, thanks. Does not apply to 8.2, since we don't use CreateProcessAsUser() there at all. //Magnus