Please review pull request #482: (#10299) Puppet.features.root? should be true when running as LocalSystem opened by (joshcooper)
Description:
Previously, on Windows 2003 and earlier, Puppet.features.root? was
implemented by checking if the current user was a member of the local
Administrators group. However, many accounts, e.g. LocalSystem, are
implicit members of this group, so Puppet.features.root? would
incorrectly return false. This led to puppet not being able to find
its default configuration directory, among other things.
Conversely, a process can be executing using a restricted token, so
while the user may be a member of the Administrators group, the
process will be running with less privileges, and
Puppet.features.root? would incorrectly return true.
This commit uses CheckTokenMembership to determine if the local
Administrators group SID is both present and enabled in the calling
thread's access token.
The behavior on Vista/2008 is unchanged. The calling thread's token
must be currently elevated.
- Opened: Thu Feb 09 23:48:34 UTC 2012
- Based on: puppetlabs:2.7.x (e5e3b21ceff7361c96a5b191d5e2fff718f34c1c)
- Requested merge: joshcooper:ticket/2.7.x/10299-LocalSystem-not-root (e094195901aa77e5183845eb781435d722054d92)
Diff follows:
diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb
index 82524d0..c7bab23 100644
--- a/lib/puppet/util/suidmanager.rb
+++ b/lib/puppet/util/suidmanager.rb
@@ -55,18 +55,8 @@ def groups=(grouplist)
def self.root?
return Process.uid == 0 unless Puppet.features.microsoft_windows?
- require 'sys/admin'
- require 'win32/security'
- require 'facter'
-
- majversion = Facter.value(:kernelmajversion)
- return false unless majversion
-
- # if Vista or later, check for unrestricted process token
- return Win32::Security.elevated_security? unless majversion.to_f < 6.0
-
- group = Sys::Admin.get_group("Administrators", :sid => Win32::Security::SID::BuiltinAdministrators)
- group and group.members.index(Sys::Admin.get_login) != nil
+ require 'puppet/util/windows/user'
+ Puppet::Util::Windows::User.admin?
end
# Runs block setting uid and gid if provided then restoring original ids
diff --git a/lib/puppet/util/windows.rb b/lib/puppet/util/windows.rb
index 53b2380..a5ff82a 100644
--- a/lib/puppet/util/windows.rb
+++ b/lib/puppet/util/windows.rb
@@ -1,4 +1,5 @@
module Puppet::Util::Windows
require 'puppet/util/windows/error'
require 'puppet/util/windows/security'
+ require 'puppet/util/windows/user'
end
diff --git a/lib/puppet/util/windows/user.rb b/lib/puppet/util/windows/user.rb
new file mode 100644
index 0000000..6722a0e
--- /dev/null
+++ b/lib/puppet/util/windows/user.rb
@@ -0,0 +1,44 @@
+require 'puppet/util/windows'
+
+require 'win32/security'
+
+module Puppet::Util::Windows::User
+ include Windows::Security
+ extend Windows::Security
+
+ def admin?
+ require 'facter'
+
+ majversion = Facter.value(:kernelmajversion)
+ return false unless majversion
+
+ # if Vista or later, check for unrestricted process token
+ return Win32::Security.elevated_security? unless majversion.to_f < 6.0
+
+ # otherwise 2003 or less
+ check_token_membership
+ end
+ module_function :admin?
+
+ def check_token_membership
+ sid = 0.chr * 80
+ size = [80].pack('L')
+ member = 0.chr * 4
+
+ unless CreateWellKnownSid(WinBuiltinAdministratorsSid, nil, sid, size)
+ raise Puppet::Util::Windows::Error.new("Failed to create administrators SID")
+ end
+
+ unless IsValidSid(sid)
+ raise Puppet::Util::Windows::Error.new("Invalid SID")
+ end
+
+ unless CheckTokenMembership(nil, sid, member)
+ raise Puppet::Util::Windows::Error.new("Failed to check membership")
+ end
+
+ # Is administrators SID enabled in calling thread's access token?
+ member.unpack('L')[0] == 1
+ end
+ module_function :check_token_membership
+end
diff --git a/spec/integration/util/windows/user_spec.rb b/spec/integration/util/windows/user_spec.rb
new file mode 100755
index 0000000..4e622e4
--- /dev/null
+++ b/spec/integration/util/windows/user_spec.rb
@@ -0,0 +1,59 @@
+#!/usr/bin/env rspec
+
+require 'spec_helper'
+
+describe "Puppet::Util::Windows::User", :if => Puppet.features.microsoft_windows? do
+ describe "2003 without UAC" do
+ before :each do
+ Facter.stubs(:value).with(:kernelmajversion).returns("5.2")
+ end
+
+ it "should be an admin if user's token contains the Administrators SID" do
+ Puppet::Util::Windows::User.expects(:check_token_membership).returns(true)
+ Win32::Security.expects(:elevated_security?).never
+
+ Puppet::Util::Windows::User.should be_admin
+ end
+
+ it "should not be an admin if user's token doesn't contain the Administrators SID" do
+ Puppet::Util::Windows::User.expects(:check_token_membership).returns(false)
+ Win32::Security.expects(:elevated_security?).never
+
+ Puppet::Util::Windows::User.should_not be_admin
+ end
+
+ it "should raise an exception if we can't check token membership" do
+ Puppet::Util::Windows::User.expects(:check_token_membership).raises(Win32::Security::Error, "Access denied.")
+ Win32::Security.expects(:elevated_security?).never
+
+ lambda { Puppet::Util::Windows::User.should raise_error(Win32::Security::Error, /Access denied./) }
+ end
+ end
+
+ describe "2008 with UAC" do
+ before :each do
+ Facter.stubs(:value).with(:kernelmajversion).returns("6.0")
+ end
+
+ it "should be an admin if user is running with elevated privileges" do
+ Win32::Security.stubs(:elevated_security?).returns(true)
+ Puppet::Util::Windows::User.expects(:check_token_membership).never
+
+ Puppet::Util::Windows::User.should be_admin
+ end
+
+ it "should not be an admin if user is not running with elevated privileges" do
+ Win32::Security.stubs(:elevated_security?).returns(false)
+ Puppet::Util::Windows::User.expects(:check_token_membership).never
+
+ Puppet::Util::Windows::User.should_not be_admin
+ end
+
+ it "should raise an exception if the process fails to open the process token" do
+ Win32::Security.stubs(:elevated_security?).raises(Win32::Security::Error, "Access denied.")
+ Puppet::Util::Windows::User.expects(:check_token_membership).never
+
+ lambda { Puppet::Util::Windows::User.should raise_error(Win32::Security::Error, /Access denied./) }
+ end
+ end
+end
diff --git a/spec/unit/util/suidmanager_spec.rb b/spec/unit/util/suidmanager_spec.rb
index 575762f..7581e10 100755
--- a/spec/unit/util/suidmanager_spec.rb
+++ b/spec/unit/util/suidmanager_spec.rb
@@ -249,61 +249,16 @@
end
describe "on Microsoft Windows", :if => Puppet.features.microsoft_windows? do
- describe "2003 without UAC" do
- before :each do
- Facter.stubs(:value).with(:kernelmajversion).returns("5.2")
- end
-
- it "should be root if user is a member of the Administrators group" do
- Sys::Admin.stubs(:get_login).returns("Administrator")
- Sys::Group.stubs(:members).returns(%w[Administrator])
-
- Win32::Security.expects(:elevated_security?).never
- Puppet::Util::SUIDManager.should be_root
- end
-
- it "should not be root if the process is running as Guest" do
- Sys::Admin.stubs(:get_login).returns("Guest")
- Sys::Group.stubs(:members).returns([])
+ it "should be root if user is privileged" do
+ Puppet::Util::Windows::User.stubs(:admin?).returns true
- Win32::Security.expects(:elevated_security?).never
- Puppet::Util::SUIDManager.should_not be_root
- end
-
- it "should raise an exception if the process fails to open the process token" do
- Win32::Security.stubs(:elevated_security?).raises(Win32::Security::Error, "Access denied.")
- Sys::Admin.stubs(:get_login).returns("Administrator")
- Sys::Group.expects(:members).never
-
- lambda { Puppet::Util::SUIDManager.should raise_error(Win32::Security::Error, /Access denied./) }
- end
+ Puppet::Util::SUIDManager.should be_root
end
- describe "2008 with UAC" do
- before :each do
- Facter.stubs(:value).with(:kernelmajversion).returns("6.0")
- end
-
- it "should be root if user is running with elevated privileges" do
- Win32::Security.stubs(:elevated_security?).returns(true)
- Sys::Admin.expects(:get_login).never
-
- Puppet::Util::SUIDManager.should be_root
- end
-
- it "should not be root if user is not running with elevated privileges" do
- Win32::Security.stubs(:elevated_security?).returns(false)
- Sys::Admin.expects(:get_login).never
+ it "should not be root if user is not privileged" do
+ Puppet::Util::Windows::User.stubs(:admin?).returns false
- Puppet::Util::SUIDManager.should_not be_root
- end
-
- it "should raise an exception if the process fails to open the process token" do
- Win32::Security.stubs(:elevated_security?).raises(Win32::Security::Error, "Access denied.")
- Sys::Admin.expects(:get_login).never
-
- lambda { Puppet::Util::SUIDManager.should raise_error(Win32::Security::Error, /Access denied./) }
- end
+ Puppet::Util::SUIDManager.should_not be_root
end
end
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
