From 6d348be80bdbb6a3fc090177cae815dfc031e663 Mon Sep 17 00:00:00 2001 From: mwrock Date: Fri, 11 Sep 2020 12:22:22 -0700 Subject: allow the use of SIDs in windows securable resources Signed-off-by: mwrock --- lib/chef/file_access_control/windows.rb | 6 +- .../shared/functional/securable_resource.rb | 139 ++++++++++++++++----- 2 files changed, 115 insertions(+), 30 deletions(-) diff --git a/lib/chef/file_access_control/windows.rb b/lib/chef/file_access_control/windows.rb index 118eae179c..9e5f07428c 100644 --- a/lib/chef/file_access_control/windows.rb +++ b/lib/chef/file_access_control/windows.rb @@ -112,7 +112,11 @@ class Chef def get_sid(value) if value.is_a?(String) - SID.from_account(value) + begin + Security.convert_string_sid_to_sid(value) + rescue Chef::Exceptions::Win32APIError + SID.from_account(value) + end elsif value.is_a?(SID) value else diff --git a/spec/support/shared/functional/securable_resource.rb b/spec/support/shared/functional/securable_resource.rb index 26f2b1cc5a..f933237583 100644 --- a/spec/support/shared/functional/securable_resource.rb +++ b/spec/support/shared/functional/securable_resource.rb @@ -242,48 +242,100 @@ shared_examples_for "a securable resource with existing target" do include_context "use Windows permissions" describe "when setting owner" do - before do - resource.owner(SID.admin_account_name) - resource.run_action(:create) - end - - it "should set the owner" do - expect(descriptor.owner).to eq(SID.Administrator) + context "with user name" do + before do + resource.owner(SID.admin_account_name) + resource.run_action(:create) + end + + it "should set the owner" do + expect(descriptor.owner).to eq(SID.Administrator) + end + + it "is marked as updated only if changes are made" do + expect(resource.updated_by_last_action?).to eq(expect_updated?) + end end - it "is marked as updated only if changes are made" do - expect(resource.updated_by_last_action?).to eq(expect_updated?) + context "with SID" do + before do + resource.owner(SID.Administrator.to_s) + resource.run_action(:create) + end + + it "should set the owner" do + expect(descriptor.owner).to eq(SID.Administrator) + end + + it "is marked as updated only if changes are made" do + expect(resource.updated_by_last_action?).to eq(expect_updated?) + end end end describe "when setting group" do - before do - resource.group("Administrators") - resource.run_action(:create) - end - - it "should set the group" do - expect(descriptor.group).to eq(SID.Administrators) + context "with group name" do + before do + resource.group("Administrators") + resource.run_action(:create) + end + + it "should set the group" do + expect(descriptor.group).to eq(SID.Administrators) + end + + it "is marked as updated only if changes are made" do + expect(resource.updated_by_last_action?).to eq(expect_updated?) + end end - it "is marked as updated only if changes are made" do - expect(resource.updated_by_last_action?).to eq(expect_updated?) + context "with group SID" do + before do + resource.group(SID.Administrators.to_s) + resource.run_action(:create) + end + + it "should set the group" do + expect(descriptor.group).to eq(SID.Administrators) + end + + it "is marked as updated only if changes are made" do + expect(resource.updated_by_last_action?).to eq(expect_updated?) + end end end describe "when setting rights and deny_rights" do - before do - resource.deny_rights(:modify, "Guest") - resource.rights(:read, "Guest") - resource.run_action(:create) - end + context "with user name" do + before do + resource.deny_rights(:modify, "Guest") + resource.rights(:read, "Guest") + resource.run_action(:create) + end - it "should set the rights and deny_rights" do - expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms) + allowed_acl(SID.Guest, expected_read_perms)) + it "should set the rights and deny_rights" do + expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms) + allowed_acl(SID.Guest, expected_read_perms)) + end + + it "is marked as updated only if changes are made" do + expect(resource.updated_by_last_action?).to eq(expect_updated?) + end end - it "is marked as updated only if changes are made" do - expect(resource.updated_by_last_action?).to eq(expect_updated?) + context "with SID" do + before do + resource.deny_rights(:modify, SID.Guest.to_s) + resource.rights(:read, SID.Guest.to_s) + resource.run_action(:create) + end + + it "should set the rights and deny_rights" do + expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms) + allowed_acl(SID.Guest, expected_read_perms)) + end + + it "is marked as updated only if changes are made" do + expect(resource.updated_by_last_action?).to eq(expect_updated?) + end end end end @@ -302,12 +354,18 @@ shared_examples_for "a securable resource without existing target" do expect(descriptor.owner).to eq(SID.default_security_object_owner) end - it "sets owner when owner is specified" do + it "sets owner when owner is specified by name" do resource.owner "Guest" resource.run_action(:create) expect(descriptor.owner).to eq(SID.Guest) end + it "sets owner when owner is specified by SID" do + resource.owner SID.Guest.to_s + resource.run_action(:create) + expect(descriptor.owner).to eq(SID.Guest) + end + it "fails to set owner when owner has invalid characters" do expect { resource.owner 'Lance "The Nose" Glindenberry III' }.to raise_error(Chef::Exceptions::ValidationFailed) end @@ -339,12 +397,18 @@ shared_examples_for "a securable resource without existing target" do expect(descriptor.group).to eq(SID.default_security_object_group) end - it "sets group when group is specified" do + it "sets group when group is specified by name" do resource.group "Everyone" resource.run_action(:create) expect(descriptor.group).to eq(SID.Everyone) end + it "sets group when group is specified by SID" do + resource.group SID.Everyone.to_s + resource.run_action(:create) + expect(descriptor.group).to eq(SID.Everyone) + end + it "fails to set group when group has invalid characters" do expect { resource.group 'Lance "The Nose" Glindenberry III' }.to raise_error(Chef::Exceptions::ValidationFailed) end @@ -406,6 +470,17 @@ shared_examples_for "a securable resource without existing target" do allowed_acl(SID.Guest, expected_modify_perms) ) end + + it "multiple rights with SID" do + resource.rights(:read, SID.Everyone.to_s) + resource.rights(:modify, SID.Guest.to_s) + resource.run_action(:create) + + expect(explicit_aces).to eq( + allowed_acl(SID.Everyone, expected_read_perms) + + allowed_acl(SID.Guest, expected_modify_perms) + ) + end end end @@ -443,6 +518,12 @@ shared_examples_for "a securable resource without existing target" do expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_full_control_perms)) end + it "using SID" do + resource.deny_rights(:full_control, SID.Guest.to_s) + resource.run_action(:create) + expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_full_control_perms)) + end + it "deny_rights ahead of rights" do resource.rights(:read, "Everyone") resource.deny_rights(:modify, "Guest") -- cgit v1.2.1