From be28ba90bde1fe0240f313151e3d564ce57e8b29 Mon Sep 17 00:00:00 2001 From: Chris Doherty Date: Tue, 21 Oct 2014 14:09:04 -0700 Subject: [PATCH] Enable Windows services to run as a different user (CHEF-4921). This adds :run_as_user and :run_as_password attributes to the windows_service resource. If a logon user is specified, the resource will (on every run) grant the logon-as-service privilege to that user, using secedit.exe. --- .../application/windows_service_manager.rb | 10 +- lib/chef/provider/service.rb | 2 +- lib/chef/provider/service/windows.rb | 100 +++++++++++++++++- lib/chef/resource/windows_service.rb | 18 ++++ .../resource/windows_service_spec.rb | 97 +++++++++++++++++ spec/functional/win32/service_manager_spec.rb | 60 +---------- spec/support/chef_helpers.rb | 6 +- .../shared/functional/win32_service.rb | 60 +++++++++++ spec/unit/provider/service/windows_spec.rb | 94 ++++++++++++++++ 9 files changed, 383 insertions(+), 64 deletions(-) create mode 100644 spec/functional/resource/windows_service_spec.rb create mode 100644 spec/support/shared/functional/win32_service.rb diff --git a/lib/chef/application/windows_service_manager.rb b/lib/chef/application/windows_service_manager.rb index 30810c51f28..de8ed657c22 100644 --- a/lib/chef/application/windows_service_manager.rb +++ b/lib/chef/application/windows_service_manager.rb @@ -16,7 +16,9 @@ # limitations under the License. # -require 'win32/service' +if RUBY_PLATFORM =~ /mswin|mingw32|windows/ + require 'win32/service' +end require 'chef/config' require 'mixlib/cli' @@ -88,6 +90,8 @@ def initialize(service_options) @service_display_name = service_options[:service_display_name] @service_description = service_options[:service_description] @service_file_path = service_options[:service_file_path] + @service_start_name = service_options[:run_as_user] + @password = service_options[:run_as_password] end def run(params = ARGV) @@ -116,7 +120,9 @@ def run(params = ARGV) # and we don't want that, so we need to override the service type. :service_type => ::Win32::Service::SERVICE_WIN32_OWN_PROCESS, :start_type => ::Win32::Service::SERVICE_AUTO_START, - :binary_path_name => cmd + :binary_path_name => cmd, + :service_start_name => @service_start_name, + :password => @password, ) puts "Service '#{@service_name}' has successfully been installed." end diff --git a/lib/chef/provider/service.rb b/lib/chef/provider/service.rb index 968f9bff9c0..75da2ddb31a 100644 --- a/lib/chef/provider/service.rb +++ b/lib/chef/provider/service.rb @@ -150,7 +150,7 @@ def restart_service end def reload_service - raise Chef::Exceptions::UnsupportedAction, "#{self.to_s} does not support :restart" + raise Chef::Exceptions::UnsupportedAction, "#{self.to_s} does not support :reload" end protected diff --git a/lib/chef/provider/service/windows.rb b/lib/chef/provider/service/windows.rb index d4c272354e8..ba53f0a3c32 100644 --- a/lib/chef/provider/service/windows.rb +++ b/lib/chef/provider/service/windows.rb @@ -20,6 +20,7 @@ require 'chef/provider/service/simple' if RUBY_PLATFORM =~ /mswin|mingw32|windows/ + require 'chef/win32/error' require 'win32/service' end @@ -29,6 +30,7 @@ class Chef::Provider::Service::Windows < Chef::Provider::Service provides :windows_service, os: "windows" include Chef::Mixin::ShellOut + include Chef::ReservedNames::Win32::API::Error rescue LoadError #Win32::Service.get_start_type AUTO_START = 'auto start' @@ -67,6 +69,22 @@ def load_current_resource def start_service if Win32::Service.exists?(@new_resource.service_name) + # reconfiguration is idempotent, so just do it. + new_config = { + service_name: @new_resource.service_name, + service_start_name: @new_resource.run_as_user, + password: @new_resource.run_as_password, + }.reject { |k,v| v.nil? || v.length == 0 } + + 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]) + end + state = current_state if state == RUNNING Chef::Log.debug "#{@new_resource} already started - nothing to do" @@ -79,7 +97,17 @@ def start_service shell_out!(@new_resource.start_command) else spawn_command_thread do - Win32::Service.start(@new_resource.service_name) + begin + Win32::Service.start(@new_resource.service_name) + rescue SystemCallError => ex + if ex.errno == ERROR_SERVICE_LOGON_FAILED + Chef::Log.error ex.message + raise Chef::Exceptions::Service, + "Service #{@new_resource} did not start due to a logon failure (error #{ERROR_SERVICE_LOGON_FAILED}): possibly the specified user '#{@new_resource.run_as_user}' does not have the 'log on as a service' privilege, or the password is incorrect." + else + raise ex + end + end end wait_for_state(RUNNING) end @@ -209,6 +237,76 @@ def action_configure_startup 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. + end + true + end + + # remove characters that make for broken or wonky filenames. + def clean_username_for_path(username) + 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 + def current_state Win32::Service.status(@new_resource.service_name).current_state end diff --git a/lib/chef/resource/windows_service.rb b/lib/chef/resource/windows_service.rb index 2aec4d6304d..8090adceb0f 100644 --- a/lib/chef/resource/windows_service.rb +++ b/lib/chef/resource/windows_service.rb @@ -37,6 +37,8 @@ def initialize(name, run_context=nil) @resource_name = :windows_service @allowed_actions.push(:configure_startup) @startup_type = :automatic + @run_as_user = "" + @run_as_password = "" end def startup_type(arg=nil) @@ -48,6 +50,22 @@ def startup_type(arg=nil) :equal_to => [ :automatic, :manual, :disabled ] ) end + + def run_as_user(arg=nil) + set_or_return( + :run_as_user, + arg, + :kind_of => [ String ] + ) + end + + def run_as_password(arg=nil) + set_or_return( + :run_as_password, + arg, + :kind_of => [ String ] + ) + end end end end diff --git a/spec/functional/resource/windows_service_spec.rb b/spec/functional/resource/windows_service_spec.rb new file mode 100644 index 00000000000..00285f47aa1 --- /dev/null +++ b/spec/functional/resource/windows_service_spec.rb @@ -0,0 +1,97 @@ +# +# Author:: Chris Doherty () +# Copyright:: Copyright (c) 2014 Chef Software, Inc. +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +require 'spec_helper' + +describe Chef::Resource::WindowsService, :windows_only, :system_windows_service_gem_only do + + include_context "using Win32::Service" + + let(:username) { "service_spec_user"} + let(:qualified_username) { ".\\#{username}"} + let(:password) { "1a2b3c4X!&narf"} + + let(:user_resource) { + r = Chef::Resource::User.new(username, run_context) + r.username(username) + r.password(password) + r.comment("temp spec user") + r + } + + let(:global_service_file_path) { + "#{ENV['WINDIR']}\\temp\\#{File.basename(test_service[:service_file_path])}" + } + + let(:service_params) { + test_service.merge( { + run_as_user: qualified_username, + run_as_password: password, + service_name: "spec_service_#{$$}_#{rand(1000)}", + service_display_name: "windows_service test service", + service_description: "Test service for running the windows_service functional spec.", + service_file_path: global_service_file_path, + } ) + } + + let(:manager) { + Chef::Application::WindowsServiceManager.new(service_params) + } + + let(:service_resource) { + r = Chef::Resource::WindowsService.new(service_params[:service_name], run_context) + [:run_as_user, :run_as_password].each { |prop| r.send(prop, service_params[prop]) } + r + } + + before { + user_resource.run_action(:create) + + # the service executable has to be outside the current user's home + # directory in order for the logon user to execute it. + FileUtils::copy_file(test_service[:service_file_path], global_service_file_path) + + # if you don't make the file executable by the service user, you'll get + # the not-very-helpful "service did not respond fast enough" error. + + # #mode may break in a post-Windows 8.1 release, and have to be replaced + # with the rights stuff in the file resource. + file = Chef::Resource::File.new(global_service_file_path, run_context) + file.mode("0777") + + file.run_action(:create) + + manager.run(%w{--action install}) + } + + after { + user_resource.run_action(:remove) + manager.run(%w{--action uninstall}) + File.delete(global_service_file_path) + } + + describe "logon as a service" do + it "successfully runs a service as another user" do + service_resource.run_action(:start) + end + + it "raises an exception when it can't grant the logon privilege" do +# service_resource.run_action(:start) + end + end +end diff --git a/spec/functional/win32/service_manager_spec.rb b/spec/functional/win32/service_manager_spec.rb index fd21e7d82ee..d2474deace2 100644 --- a/spec/functional/win32/service_manager_spec.rb +++ b/spec/functional/win32/service_manager_spec.rb @@ -24,7 +24,7 @@ # # ATTENTION: # This test creates a windows service for testing purposes and runs it -# as Local System on windows boxes. +# as Local System (or an otherwise specified user) on windows boxes. # This test will fail if you run the tests inside a Windows VM by # sharing the code from your host since Local System account by # default can't see the mounted partitions. @@ -35,61 +35,7 @@ describe "Chef::Application::WindowsServiceManager", :windows_only, :system_windows_service_gem_only do - # Some helper methods. - - def test_service_exists? - ::Win32::Service.exists?("spec-service") - end - - def test_service_state - ::Win32::Service.status("spec-service").current_state - end - - def service_manager - Chef::Application::WindowsServiceManager.new(test_service) - end - - def cleanup - # Uninstall if the test service is installed. - if test_service_exists? - - # We can only uninstall when the service is stopped. - if test_service_state != "stopped" - ::Win32::Service.send("stop", "spec-service") - while test_service_state != "stopped" - sleep 1 - end - end - - ::Win32::Service.delete("spec-service") - end - - # Delete the test_service_file if it exists - if File.exists?(test_service_file) - File.delete(test_service_file) - end - - end - - - # Definition for the test-service - - let(:test_service) { - { - :service_name => "spec-service", - :service_display_name => "Spec Test Service", - :service_description => "Service for testing Chef::Application::WindowsServiceManager.", - :service_file_path => File.expand_path(File.join(File.dirname(__FILE__), '../../support/platforms/win32/spec_service.rb')) - } - } - - # Test service creates a file for us to verify that it is running. - # Since our test service is running as Local System we should look - # for the file it creates under SYSTEM temp directory - - let(:test_service_file) { - "#{ENV['SystemDrive']}\\windows\\temp\\spec_service_file" - } + include_context "using Win32::Service" context "with invalid service definition" do it "throws an error when initialized with no service definition" do @@ -190,7 +136,7 @@ def cleanup ["pause", "resume"].each do |action| it "#{action} => should raise error" do - expect {service_manager.run(["-a", action])}.to raise_error(::Win32::Service::Error) + expect { service_manager.run(["-a", action]) }.to raise_error(SystemCallError) end end diff --git a/spec/support/chef_helpers.rb b/spec/support/chef_helpers.rb index 237543748c3..851b1dce0a3 100644 --- a/spec/support/chef_helpers.rb +++ b/spec/support/chef_helpers.rb @@ -67,15 +67,15 @@ def has_diff? # win32/service gem. windows_service_manager tests create a windows # service that starts with the system ruby and requires this gem. def system_windows_service_gem? - windows_service_gem_check_command = "ruby -e 'require \"win32/daemon\"' > /dev/null 2>&1" + windows_service_gem_check_command = %q{ruby -r "win32/daemon" -e ":noop"} if defined?(Bundler) Bundler.with_clean_env do # This returns true if the gem can be loaded - system windows_service_gem_check_command + system(windows_service_gem_check_command) end else # This returns true if the gem can be loaded - system windows_service_gem_check_command + system(windows_service_gem_check_command) end end diff --git a/spec/support/shared/functional/win32_service.rb b/spec/support/shared/functional/win32_service.rb new file mode 100644 index 00000000000..7dd19204182 --- /dev/null +++ b/spec/support/shared/functional/win32_service.rb @@ -0,0 +1,60 @@ + +require 'chef/application/windows_service_manager' + +shared_context "using Win32::Service" do + # Some helper methods. + + def test_service_exists? + ::Win32::Service.exists?("spec-service") + end + + def test_service_state + ::Win32::Service.status("spec-service").current_state + end + + def service_manager + Chef::Application::WindowsServiceManager.new(test_service) + end + + def cleanup + # Uninstall if the test service is installed. + if test_service_exists? + + # We can only uninstall when the service is stopped. + if test_service_state != "stopped" + ::Win32::Service.send("stop", "spec-service") + while test_service_state != "stopped" + sleep 1 + end + end + + ::Win32::Service.delete("spec-service") + end + + # Delete the test_service_file if it exists + if File.exists?(test_service_file) + File.delete(test_service_file) + end + + end + + + # Definition for the test-service + + let(:test_service) { + { + :service_name => "spec-service", + :service_display_name => "Spec Test Service", + :service_description => "Service for testing Chef::Application::WindowsServiceManager.", + :service_file_path => File.expand_path(File.join(File.dirname(__FILE__), '../../platforms/win32/spec_service.rb')) + } + } + + # Test service creates a file for us to verify that it is running. + # Since our test service is running as Local System we should look + # for the file it creates under SYSTEM temp directory + + let(:test_service_file) { + "#{ENV['SystemDrive']}\\windows\\temp\\spec_service_file" + } +end diff --git a/spec/unit/provider/service/windows_spec.rb b/spec/unit/provider/service/windows_spec.rb index e4b0714d22a..784a2232b25 100644 --- a/spec/unit/provider/service/windows_spec.rb +++ b/spec/unit/provider/service/windows_spec.rb @@ -18,6 +18,7 @@ # require 'spec_helper' +require 'mixlib/shellout' describe Chef::Provider::Service::Windows, "load_current_resource" do before(:each) do @@ -38,6 +39,7 @@ allow(Win32::Service).to receive(:config_info).with(@new_resource.service_name).and_return( double("ConfigStruct", :start_type => "auto start")) allow(Win32::Service).to receive(:exists?).and_return(true) + allow(Win32::Service).to receive(:configure).and_return(Win32::Service) end it "should set the current resources service name to the new resources service name" do @@ -131,6 +133,26 @@ expect(@new_resource.updated_by_last_action?).to be_falsey end + describe "running as a different account" do + let(:old_run_as_user) { @new_resource.run_as_user } + let(:old_run_as_password) { @new_resource.run_as_password } + + before { + @new_resource.run_as_user(".\\wallace") + @new_resource.run_as_password("Wensleydale") + } + + after { + @new_resource.run_as_user(old_run_as_user) + @new_resource.run_as_password(old_run_as_password) + } + + it "should call #grant_service_logon if the :run_as_user and :run_as_password attributes are present" do + expect(Win32::Service).to receive(:start) + expect(@provider).to receive(:grant_service_logon).and_return(true) + @provider.start_service + end + end end @@ -364,4 +386,76 @@ expect { @provider.send(:set_startup_type, :fire_truck) }.to raise_error(Chef::Exceptions::ConfigurationError) end end + + shared_context "testing private methods" do + + let(:private_methods) { + described_class.private_instance_methods + } + + before { + described_class.send(:public, *private_methods) + } + + after { + described_class.send(:private, *private_methods) + } + end + + describe "grant_service_logon" do + include_context "testing private methods" + + let(:username) { "unit_test_user" } + let(:success_string) { "The task has completed successfully.\r\nSee logfile etc." } + let(:failure_string) { "Look on my works, ye Mighty, and despair!" } + let(:command) { + dbfile = @provider.grant_dbfile_name(username) + policyfile = @provider.grant_policyfile_name(username) + logfile = @provider.grant_logfile_name(username) + + %Q{secedit.exe /configure /db "#{dbfile}" /cfg "#{policyfile}" /areas USER_RIGHTS SECURITYPOLICY SERVICES /log "#{logfile}"} + } + let(:shellout_env) { {:environment=>{"LC_ALL"=>"en_US.UTF-8"}} } + + before { + expect_any_instance_of(described_class).to receive(:shell_out).with(command).and_call_original + expect_any_instance_of(Mixlib::ShellOut).to receive(:run_command).and_return(nil) + } + + after { + # only needed for the second test. + ::File.delete(@provider.grant_policyfile_name(username)) rescue nil + ::File.delete(@provider.grant_logfile_name(username)) rescue nil + ::File.delete(@provider.grant_dbfile_name(username)) rescue nil + } + + it "calls Mixlib::Shellout with the correct command string" do + expect_any_instance_of(Mixlib::ShellOut).to receive(:exitstatus).and_return(0) + expect(@provider.grant_service_logon(username)).to equal true + end + + it "raises an exception when the grant command fails" do + expect_any_instance_of(Mixlib::ShellOut).to receive(:exitstatus).and_return(1) + expect_any_instance_of(Mixlib::ShellOut).to receive(:stdout).and_return(failure_string) + expect { @provider.grant_service_logon(username) }.to raise_error(Chef::Exceptions::Service) + end + end + + describe "cleaning usernames" do + include_context "testing private methods" + + it "correctly reformats usernames to create valid filenames" do + expect(@provider.clean_username_for_path("\\\\problem username/oink.txt")).to eq("_problem_username_oink_txt") + expect(@provider.clean_username_for_path("boring_username")).to eq("boring_username") + end + + it "correctly reformats usernames for the policy file" do + expect(@provider.canonicalize_username(".\\maryann")).to eq("maryann") + expect(@provider.canonicalize_username("maryann")).to eq("maryann") + + expect(@provider.canonicalize_username("\\\\maryann")).to eq("maryann") + expect(@provider.canonicalize_username("mydomain\\\\maryann")).to eq("mydomain\\\\maryann") + expect(@provider.canonicalize_username("\\\\mydomain\\\\maryann")).to eq("mydomain\\\\maryann") + end + end end