Merge pull request #4321 from chef/service_user

fix run_as_user of windows_service

5 files changed, 119 insertions, 61 deletions

diff --git a/lib/chef/provider/service/windows.rb b/lib/chef/provider/service/windows.rb
index 355ffafc2a..f5c370246c 100644
--- a/lib/chef/provider/service/windows.rb
+++ b/lib/chef/provider/service/windows.rb
@@ -47,6 +47,8 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
 
   TIMEOUT  = 60
 
+  SERVICE_RIGHT = 'SeServiceLogonRight'
+
   def whyrun_supported?
     false
   end
@@ -78,10 +80,10 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
       Win32::Service.configure(new_config)
       Chef::Log.info "#{@new_resource} configured with #{new_config.inspect}"
 
-      # it would be nice to check if the user already has the logon privilege, but that turns out to be
-      # nontrivial.
       if new_config.has_key?(:service_start_name)
-        grant_service_logon(new_config[:service_start_name])
+        unless Chef::ReservedNames::Win32::Security.get_account_right(canonicalize_username(new_config[:service_start_name])).include?(SERVICE_RIGHT)
+          grant_service_logon(new_config[:service_start_name])
+        end
       end
 
       state = current_state
@@ -236,62 +238,15 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service
   end
 
   private
-  def make_policy_text(username)
-    text = <<-EOS
-[Unicode]
-Unicode=yes
-[Privilege Rights]
-SeServiceLogonRight = \\\\#{canonicalize_username(username)},*S-1-5-80-0
-[Version]
-signature="$CHICAGO$"
-Revision=1
-EOS
-  end
-
-  def grant_logfile_name(username)
-    Chef::Util::PathHelper.canonical_path("#{Dir.tmpdir}/logon_grant-#{clean_username_for_path(username)}-#{$$}.log", prefix=false)
-  end
-
-  def grant_policyfile_name(username)
-    Chef::Util::PathHelper.canonical_path("#{Dir.tmpdir}/service_logon_policy-#{clean_username_for_path(username)}-#{$$}.inf", prefix=false)
-  end
-
-  def grant_dbfile_name(username)
-    "#{ENV['TEMP']}\\secedit.sdb"
-  end
-
   def grant_service_logon(username)
-    logfile = grant_logfile_name(username)
-    policy_file = ::File.new(grant_policyfile_name(username), 'w')
-    policy_text = make_policy_text(username)
-    dbfile = grant_dbfile_name(username)        # this is just an audit file.
-
     begin
-      Chef::Log.debug "Policy file text:\n#{policy_text}"
-      policy_file.puts(policy_text)
-      policy_file.close   # need to flush the buffer.
-
-      # it would be nice to do this with APIs instead, but the LSA_* APIs are
-      # particularly onerous and life is short.
-      cmd = %Q{secedit.exe /configure /db "#{dbfile}" /cfg "#{policy_file.path}" /areas USER_RIGHTS SECURITYPOLICY SERVICES /log "#{logfile}"}
-      Chef::Log.debug "Granting logon-as-service privilege with: #{cmd}"
-      runner = shell_out(cmd)
-
-      if runner.exitstatus != 0
-        Chef::Log.fatal "Logon-as-service grant failed with output: #{runner.stdout}"
-        raise Chef::Exceptions::Service, <<-EOS
-Logon-as-service grant failed with policy file #{policy_file.path}.
-You can look at #{logfile} for details, or do `secedit /analyze #{dbfile}`.
-The failed command was `#{cmd}`.
-EOS
-      end
-
-      Chef::Log.info "Grant logon-as-service to user '#{username}' successful."
-
-      ::File.delete(dbfile) rescue nil
-      ::File.delete(policy_file)
-      ::File.delete(logfile) rescue nil     # logfile is not always present at end.
+      Chef::ReservedNames::Win32::Security.add_account_right(canonicalize_username(username), SERVICE_RIGHT)
+    rescue Chef::Exceptions::Win32APIError => err
+      Chef::Log.fatal "Logon-as-service grant failed with output: #{err}"
+      raise Chef::Exceptions::Service, "Logon-as-service grant failed for #{username}: #{err}"
     end
+
+    Chef::Log.info "Grant logon-as-service to user '#{username}' successful."
     true
   end
 
@@ -300,8 +255,6 @@ EOS
     username.gsub(/[\/\\. ]+/, '_')
   end
 
-  # the security policy file only seems to accept \\username, so fix .\username or .\\username.
-  # TODO: this probably has to be fixed to handle various valid Windows names correctly.
   def canonicalize_username(username)
     username.sub(/^\.?\\+/, '')
   end
diff --git a/lib/chef/win32/api.rb b/lib/chef/win32/api.rb
index 4786222bd4..de3381c5ba 100644
--- a/lib/chef/win32/api.rb
+++ b/lib/chef/win32/api.rb
@@ -147,6 +147,8 @@ class Chef
         host.typedef :long,    :LRESULT # Signed result of message processing. WinDef.h: host.typedef LONG_PTR LRESULT;
         host.typedef :pointer, :LPWIN32_FIND_DATA # Pointer to WIN32_FIND_DATA struct
         host.typedef :pointer, :LPBY_HANDLE_FILE_INFORMATION # Point to a BY_HANDLE_FILE_INFORMATION struct
+        host.typedef :pointer, :LSA_HANDLE # A handle to a Policy object
+        host.typedef :ulong,   :NTSTATUS # An NTSTATUS code returned by an LSA function call.
         host.typedef :pointer, :PBOOL # Pointer to a BOOL.
         host.typedef :pointer, :PBOOLEAN # Pointer to a BOOL.
         host.typedef :pointer, :PBYTE # Pointer to a BYTE.
@@ -174,12 +176,16 @@ class Chef
         host.typedef :pointer, :PLONG_PTR # Pointer to a LONG_PTR.
         host.typedef :pointer, :PLONG32 # Pointer to a LONG32.
         host.typedef :pointer, :PLONG64 # Pointer to a LONG64.
+        host.typedef :pointer, :PLSA_HANDLE # Pointer to an LSA_HANDLE
+        host.typedef :pointer, :PLSA_OBJECT_ATTRIBUTES # Pointer to an LSA_OBJECT_ATTRIBUTES
+        host.typedef :pointer, :PLSA_UNICODE_STRING # Pointer to LSA_UNICODE_STRING
         host.typedef :pointer, :PLUID # Pointer to a LUID.
         host.typedef :pointer, :POINTER_32 # 32-bit pointer. On a 32-bit system, this is a native pointer. On a 64-bit system, this is a truncated 64-bit pointer.
         host.typedef :pointer, :POINTER_64 # 64-bit pointer. On a 64-bit system, this is a native pointer. On a 32-bit system, this is a sign-extended 32-bit pointer.
         host.typedef :pointer, :POINTER_SIGNED # A signed pointer.
         host.typedef :pointer, :POINTER_UNSIGNED # An unsigned pointer.
         host.typedef :pointer, :PSHORT # Pointer to a SHORT.
+        host.typedef :pointer, :PSID # Pointer to an account SID
         host.typedef :pointer, :PSIZE_T # Pointer to a SIZE_T.
         host.typedef :pointer, :PSSIZE_T # Pointer to a SSIZE_T.
         host.typedef :pointer, :PSTR # Pointer to a null-terminated string of 8-bit Windows (ANSI) characters. For more information, see Character Sets Used By Fonts.
@@ -188,7 +194,6 @@ class Chef
         host.typedef :pointer, :PCRYPTPROTECT_PROMPTSTRUCT # Pointer to a CRYPTOPROTECT_PROMPTSTRUCT.
         host.typedef :pointer, :PDATA_BLOB # Pointer to a DATA_BLOB.
         host.typedef :pointer, :PTSTR # A PWSTR if UNICODE is defined, a PSTR otherwise.
-        host.typedef :pointer, :PSID
         host.typedef :pointer, :PUCHAR # Pointer to a UCHAR.
         host.typedef :pointer, :PUHALF_PTR # Pointer to a UHALF_PTR.
         host.typedef :pointer, :PUINT # Pointer to a UINT.
diff --git a/lib/chef/win32/api/security.rb b/lib/chef/win32/api/security.rb
index 4c352a3554..4353c488f8 100644
--- a/lib/chef/win32/api/security.rb
+++ b/lib/chef/win32/api/security.rb
@@ -207,6 +207,21 @@ class Chef
         LOGON32_PROVIDER_WINNT40 = 2;
         LOGON32_PROVIDER_WINNT50 = 3;
 
+        # LSA access policy
+        POLICY_VIEW_LOCAL_INFORMATION = 0x00000001
+        POLICY_VIEW_AUDIT_INFORMATION = 0x00000002
+        POLICY_GET_PRIVATE_INFORMATION = 0x00000004
+        POLICY_TRUST_ADMIN = 0x00000008
+        POLICY_CREATE_ACCOUNT = 0x00000010
+        POLICY_CREATE_SECRET = 0x00000020
+        POLICY_CREATE_PRIVILEGE = 0x00000040
+        POLICY_SET_DEFAULT_QUOTA_LIMITS = 0x00000080
+        POLICY_SET_AUDIT_REQUIREMENTS = 0x00000100
+        POLICY_AUDIT_LOG_ADMIN = 0x00000200
+        POLICY_SERVER_ADMIN = 0x00000400
+        POLICY_LOOKUP_NAMES = 0x00000800
+        POLICY_NOTIFICATION = 0x00001000
+
         ###############################################
         # Win32 API Bindings
         ###############################################
@@ -381,6 +396,23 @@ class Chef
           end
         end
 
+        # https://msdn.microsoft.com/en-us/library/windows/desktop/ms721829(v=vs.85).aspx
+        class LSA_OBJECT_ATTRIBUTES < FFI::Struct
+          layout :Length, :ULONG,
+                 :RootDirectory, :HANDLE,
+                 :ObjectName, :pointer,
+                 :Attributes, :ULONG,
+                 :SecurityDescriptor, :PVOID,
+                 :SecurityQualityOfService, :PVOID
+        end
+
+        # https://msdn.microsoft.com/en-us/library/windows/desktop/ms721841(v=vs.85).aspx
+        class LSA_UNICODE_STRING < FFI::Struct
+          layout :Length, :USHORT,
+                 :MaximumLength, :USHORT,
+                 :Buffer, :PWSTR
+        end
+
         ffi_lib "advapi32"
 
         safe_attach_function :AccessCheck, [:pointer, :HANDLE, :DWORD, :pointer, :pointer, :pointer, :pointer, :pointer], :BOOL
@@ -415,6 +447,12 @@ class Chef
         safe_attach_function :LookupPrivilegeNameW, [ :LPCWSTR, :PLUID, :LPWSTR, :LPDWORD ], :BOOL
         safe_attach_function :LookupPrivilegeDisplayNameW, [ :LPCWSTR, :LPCWSTR, :LPWSTR, :LPDWORD, :LPDWORD ], :BOOL
         safe_attach_function :LookupPrivilegeValueW, [ :LPCWSTR, :LPCWSTR, :PLUID ], :BOOL
+        safe_attach_function :LsaAddAccountRights, [ :pointer, :pointer, :pointer, :ULONG ], :NTSTATUS
+        safe_attach_function :LsaClose, [ :LSA_HANDLE ], :NTSTATUS
+        safe_attach_function :LsaEnumerateAccountRights, [ :LSA_HANDLE, :PSID, :PLSA_UNICODE_STRING, :PULONG ], :NTSTATUS
+        safe_attach_function :LsaFreeMemory, [ :PVOID ], :NTSTATUS
+        safe_attach_function :LsaNtStatusToWinError, [ :NTSTATUS ], :ULONG
+        safe_attach_function :LsaOpenPolicy, [ :PLSA_UNICODE_STRING, :PLSA_OBJECT_ATTRIBUTES, :DWORD, :PLSA_HANDLE ], :NTSTATUS
         safe_attach_function :MakeAbsoluteSD, [ :pointer, :pointer, :LPDWORD, :pointer, :LPDWORD, :pointer, :LPDWORD, :pointer, :LPDWORD, :pointer, :LPDWORD], :BOOL
         safe_attach_function :MapGenericMask, [ :PDWORD, :PGENERICMAPPING ], :void
         safe_attach_function :OpenProcessToken, [ :HANDLE, :DWORD, :PHANDLE ], :BOOL
diff --git a/lib/chef/win32/error.rb b/lib/chef/win32/error.rb
index 2175608eeb..c638773d2a 100644
--- a/lib/chef/win32/error.rb
+++ b/lib/chef/win32/error.rb
@@ -57,8 +57,7 @@ class Chef
       # nil::: always returns nil when it does not raise
       # === Raises
       # Chef::Exceptions::Win32APIError:::
-      def self.raise!(message = nil)
-        code = get_last_error
+      def self.raise!(message = nil, code = get_last_error)
         msg = format_message(code).strip
         formatted_message = ""
         formatted_message << message if message
diff --git a/lib/chef/win32/security.rb b/lib/chef/win32/security.rb
index bc80517d80..38694c9fec 100644
--- a/lib/chef/win32/security.rb
+++ b/lib/chef/win32/security.rb
@@ -104,6 +104,22 @@ class Chef
         end
       end
 
+      def self.add_account_right(name, privilege)
+        privilege_pointer = FFI::MemoryPointer.new LSA_UNICODE_STRING, 1
+        privilege_lsa_string = LSA_UNICODE_STRING.new(privilege_pointer)
+        privilege_lsa_string[:Buffer] = FFI::MemoryPointer.from_string(privilege.to_wstring)
+        privilege_lsa_string[:Length] = privilege.length * 2
+        privilege_lsa_string[:MaximumLength] = (privilege.length + 1) * 2
+
+        with_lsa_policy(name) do |policy_handle, sid|
+          result = LsaAddAccountRights(policy_handle.read_pointer, sid, privilege_pointer, 1)
+          win32_error = LsaNtStatusToWinError(result)
+          if win32_error != 0
+            Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
+          end
+        end
+      end
+
       def self.adjust_token_privileges(token, privileges)
         token = token.handle if token.respond_to?(:handle)
         old_privileges_size = FFI::Buffer.new(:long).write_long(privileges.size_with_privileges)
@@ -165,6 +181,29 @@ class Chef
         end
       end
 
+      def self.get_account_right(name)
+        privileges = []
+        privilege_pointer = FFI::MemoryPointer.new(:pointer)
+        privilege_length = FFI::MemoryPointer.new(:ulong)
+
+        with_lsa_policy(name) do |policy_handle, sid|
+          result = LsaEnumerateAccountRights(policy_handle.read_pointer, sid, privilege_pointer, privilege_length)
+          win32_error = LsaNtStatusToWinError(result)
+          return [] if win32_error == 2 # FILE_NOT_FOUND - No rights assigned
+          if win32_error != 0
+            Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
+          end
+
+          privilege_length.read_ulong.times do |i|
+            privilege = LSA_UNICODE_STRING.new(privilege_pointer.read_pointer + i * LSA_UNICODE_STRING.size)
+            privileges << privilege[:Buffer].read_wstring
+          end
+          LsaFreeMemory(privilege_pointer)
+        end
+
+        privileges
+      end
+
       def self.get_ace(acl, index)
         acl = acl.pointer if acl.respond_to?(:pointer)
         ace = FFI::Buffer.new :pointer
@@ -548,6 +587,30 @@ class Chef
         end
       end
 
+      def self.with_lsa_policy(username)
+        sid = lookup_account_name(username)[1]
+
+        access = 0
+        access |= POLICY_CREATE_ACCOUNT
+        access |= POLICY_LOOKUP_NAMES
+
+        policy_handle = FFI::MemoryPointer.new(:pointer)
+        result = LsaOpenPolicy(nil, LSA_OBJECT_ATTRIBUTES.new, access, policy_handle)
+        win32_error = LsaNtStatusToWinError(result)
+        if win32_error != 0
+          Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
+        end
+
+        begin
+          yield policy_handle, sid.pointer
+        ensure
+          win32_error = LsaNtStatusToWinError(LsaClose(policy_handle.read_pointer))
+          if win32_error != 0
+            Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
+          end
+        end
+      end
+
       def self.with_privileges(*privilege_names)
         # Set privileges
         token = open_current_process_token(TOKEN_READ | TOKEN_ADJUST_PRIVILEGES)