diff options
author | vijaymmali1990 <vijay.mali@msystechnologies.com> | 2019-02-12 03:31:02 -0800 |
---|---|---|
committer | Tim Smith <tsmith@chef.io> | 2019-05-29 14:21:01 -0700 |
commit | 04a20845489b179e6e5ebd0a667242705faf710d (patch) | |
tree | 2a83e7fa7fd06ec1de4ac6ec17f6e8180d5e0848 | |
parent | 4f0ef4aa27f06834d2f54d8ea865f29badeb101f (diff) | |
download | chef-04a20845489b179e6e5ebd0a667242705faf710d.tar.gz |
Minor fixes as per the review commentsbackport_12
- Using proper verbiage for constant SUBFOLDERS_AND_FILES_ONLY
- Source
https://metacpan.org/pod/Win32::Security::ACE#dbmAceFlags
- Minor DRY up and Fixes
- Ensured chefstyle
Signed-off-by: vijaymmali1990 <vijay.mali@msystechnologies.com>
-rw-r--r-- | lib/chef/file_access_control/windows.rb | 4 | ||||
-rw-r--r-- | lib/chef/win32/api/security.rb | 2 | ||||
-rw-r--r-- | lib/chef/win32/security/ace.rb | 2 | ||||
-rw-r--r-- | spec/functional/resource/link_spec.rb | 4 | ||||
-rw-r--r-- | spec/support/shared/functional/directory_resource.rb | 4 | ||||
-rw-r--r-- | spec/support/shared/functional/file_resource.rb | 4 | ||||
-rw-r--r-- | spec/support/shared/functional/securable_resource.rb | 192 |
7 files changed, 110 insertions, 102 deletions
diff --git a/lib/chef/file_access_control/windows.rb b/lib/chef/file_access_control/windows.rb index 6937912849..a7cefdc6d7 100644 --- a/lib/chef/file_access_control/windows.rb +++ b/lib/chef/file_access_control/windows.rb @@ -90,10 +90,12 @@ class Chef target_acl.each do |target_ace| if target_ace.flags & INHERIT_ONLY_ACE == 0 self_ace = target_ace.dup + # We need flag value which is already being set in case of WRITE permissions as 3, so we will not be overwriting it with the hard coded value. self_ace.flags = 0 unless target_ace.mask == Chef::ReservedNames::Win32::API::Security::WRITE self_ace.mask = securable_object.predict_rights_mask(target_ace.mask) new_target_acl << self_ace end + # As there is no inheritence needed in case of WRITE permissions. if target_ace.mask != Chef::ReservedNames::Win32::API::Security::WRITE && target_ace.flags & (CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE) != 0 children_ace = target_ace.dup children_ace.flags |= INHERIT_ONLY_ACE @@ -220,7 +222,7 @@ class Chef when :read_execute mask |= GENERIC_READ | GENERIC_EXECUTE when :write - mask |= GENERIC_WRITE + mask |= WRITE else # Otherwise, assume it's an integer specifying the actual flags mask |= permission diff --git a/lib/chef/win32/api/security.rb b/lib/chef/win32/api/security.rb index bac4ab5450..5c3dd69c3e 100644 --- a/lib/chef/win32/api/security.rb +++ b/lib/chef/win32/api/security.rb @@ -115,7 +115,6 @@ class Chef STANDARD_RIGHTS_EXECUTE = READ_CONTROL STANDARD_RIGHTS_ALL = 0x001F0000 SPECIFIC_RIGHTS_ALL = 0x0000FFFF - # Access System Security Right ACCESS_SYSTEM_SECURITY = 0x01000000 # File/Directory Specific Rights @@ -142,6 +141,7 @@ class Chef FILE_GENERIC_WRITE = STANDARD_RIGHTS_WRITE | FILE_WRITE_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA | FILE_APPEND_DATA | SYNCHRONIZE FILE_GENERIC_EXECUTE = STANDARD_RIGHTS_EXECUTE | FILE_READ_ATTRIBUTES | FILE_EXECUTE | SYNCHRONIZE WRITE = FILE_WRITE_DATA | FILE_APPEND_DATA | FILE_WRITE_ATTRIBUTES | FILE_WRITE_EA + SUBFOLDERS_AND_FILES_ONLY = INHERIT_ONLY_ACE | CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE # Access Token Rights (for OpenProcessToken) # Access Rights for Access-Token Objects (used in OpenProcessToken) TOKEN_ASSIGN_PRIMARY = 0x0001 diff --git a/lib/chef/win32/security/ace.rb b/lib/chef/win32/security/ace.rb index ba81c44269..d593513983 100644 --- a/lib/chef/win32/security/ace.rb +++ b/lib/chef/win32/security/ace.rb @@ -113,7 +113,7 @@ class Chef struct[:AceType] = type struct[:AceFlags] = flags struct[:AceSize] = size_needed - struct[:Mask] = mask == Security::GENERIC_WRITE ? Security::WRITE : mask + struct[:Mask] = mask Chef::ReservedNames::Win32::Memory.memcpy(struct.pointer + struct.offset_of(:SidStart), sid.pointer, sid.size) ACE.new(struct.pointer) end diff --git a/spec/functional/resource/link_spec.rb b/spec/functional/resource/link_spec.rb index 4c8545e60b..d86a904098 100644 --- a/spec/functional/resource/link_spec.rb +++ b/spec/functional/resource/link_spec.rb @@ -417,11 +417,11 @@ describe Chef::Resource::Link do it_behaves_like "a securable resource without existing target" do let(:path) { target_file } - def allowed_acl(sid, expected_perms, flags = 0) + def allowed_acl(sid, expected_perms, _flags = 0) [ ACE.access_allowed(sid, expected_perms[:specific]) ] end - def denied_acl(sid, expected_perms, flags = 0) + def denied_acl(sid, expected_perms, _flags = 0) [ ACE.access_denied(sid, expected_perms[:specific]) ] end diff --git a/spec/support/shared/functional/directory_resource.rb b/spec/support/shared/functional/directory_resource.rb index c910e7c668..4fb08479e6 100644 --- a/spec/support/shared/functional/directory_resource.rb +++ b/spec/support/shared/functional/directory_resource.rb @@ -68,7 +68,7 @@ shared_examples_for "a directory resource" do def allowed_acl(sid, expected_perms, flags = 0) acl = [ ACE.access_allowed(sid, expected_perms[:specific], flags) ] if expected_perms[:generic] - acl << ACE.access_allowed(sid, expected_perms[:generic], (Chef::ReservedNames::Win32::API::Security::INHERIT_ONLY_ACE | Chef::ReservedNames::Win32::API::Security::CONTAINER_INHERIT_ACE | Chef::ReservedNames::Win32::API::Security::OBJECT_INHERIT_ACE)) + acl << ACE.access_allowed(sid, expected_perms[:generic], (Chef::ReservedNames::Win32::API::Security::SUBFOLDERS_AND_FILES_ONLY)) end acl end @@ -76,7 +76,7 @@ shared_examples_for "a directory resource" do def denied_acl(sid, expected_perms, flags = 0) acl = [ ACE.access_denied(sid, expected_perms[:specific], flags) ] if expected_perms[:generic] - acl << ACE.access_denied(sid, expected_perms[:generic], (Chef::ReservedNames::Win32::API::Security::INHERIT_ONLY_ACE | Chef::ReservedNames::Win32::API::Security::CONTAINER_INHERIT_ACE | Chef::ReservedNames::Win32::API::Security::OBJECT_INHERIT_ACE)) + acl << ACE.access_denied(sid, expected_perms[:generic], (Chef::ReservedNames::Win32::API::Security::SUBFOLDERS_AND_FILES_ONLY)) end acl end diff --git a/spec/support/shared/functional/file_resource.rb b/spec/support/shared/functional/file_resource.rb index 8aa4ffb65e..db947614b3 100644 --- a/spec/support/shared/functional/file_resource.rb +++ b/spec/support/shared/functional/file_resource.rb @@ -899,11 +899,11 @@ shared_examples_for "a configured file resource" do end # Set up the context for security tests - def allowed_acl(sid, expected_perms, flags = 0) + def allowed_acl(sid, expected_perms, _flags = 0) [ ACE.access_allowed(sid, expected_perms[:specific]) ] end - def denied_acl(sid, expected_perms, flags = 0) + def denied_acl(sid, expected_perms, _flags = 0) [ ACE.access_denied(sid, expected_perms[:specific]) ] end diff --git a/spec/support/shared/functional/securable_resource.rb b/spec/support/shared/functional/securable_resource.rb index 0a7eac442d..18e7243453 100644 --- a/spec/support/shared/functional/securable_resource.rb +++ b/spec/support/shared/functional/securable_resource.rb @@ -364,102 +364,108 @@ shared_examples_for "a securable resource without existing target" do expect(descriptor.group).to eq(arbitrary_non_default_group) end - describe "with rights and deny_rights attributes" do - - it "correctly sets :read rights" do - resource.rights(:read, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_read_perms)) - end - - it "correctly sets :read_execute rights" do - resource.rights(:read_execute, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_read_execute_perms)) - end - - it "correctly sets :write rights" do - resource.rights(:write, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_write_perms, write_flag)) - end - - it "correctly sets :modify rights" do - resource.rights(:modify, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_modify_perms)) + describe "#allowed_acl" do + context "correctly sets" do + + it ":read rights" do + resource.rights(:read, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_read_perms)) + end + + it ":read_execute rights" do + resource.rights(:read_execute, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_read_execute_perms)) + end + + it ":write rights" do + resource.rights(:write, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_write_perms, write_flag)) + end + + it ":modify rights" do + resource.rights(:modify, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_modify_perms)) + end + + it ":full_control rights" do + resource.rights(:full_control, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_full_control_perms)) + end + + it "multiple rights" do + resource.rights(:read, "Everyone") + resource.rights(:modify, "Guest") + 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 - it "correctly sets :full_control rights" do - resource.rights(:full_control, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(allowed_acl(SID.Guest, expected_full_control_perms)) - end - - it "correctly sets :read deny_rights" do - resource.deny_rights(:read, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_read_perms)) - end - - it "correctly sets :read_execute deny_rights" do - resource.deny_rights(:read_execute, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_read_execute_perms)) - end - - it "correctly sets :write deny_rights" do - resource.deny_rights(:write, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_write_perms, write_flag)) - end - - it "correctly sets :modify deny_rights" do - resource.deny_rights(:modify, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms)) - end - - it "correctly sets deny_rights" do - # deny is an ACE with full rights, but is a deny type ace, not an allow type - resource.deny_rights(:full_control, "Guest") - resource.run_action(:create) - expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_full_control_perms)) - end - - it "Sets multiple rights" do - resource.rights(:read, "Everyone") - resource.rights(:modify, "Guest") - resource.run_action(:create) - - expect(explicit_aces).to eq( - allowed_acl(SID.Everyone, expected_read_perms) + - allowed_acl(SID.Guest, expected_modify_perms) - ) - end - - it "Sets deny_rights ahead of rights" do - resource.rights(:read, "Everyone") - resource.deny_rights(:modify, "Guest") - resource.run_action(:create) - - expect(explicit_aces).to eq( - denied_acl(SID.Guest, expected_modify_perms) + - allowed_acl(SID.Everyone, expected_read_perms) - ) - end - - it "Sets deny_rights ahead of rights when specified in reverse order" do - resource.deny_rights(:modify, "Guest") - resource.rights(:read, "Everyone") - resource.run_action(:create) - - expect(explicit_aces).to eq( - denied_acl(SID.Guest, expected_modify_perms) + - allowed_acl(SID.Everyone, expected_read_perms) - ) + describe "#denied_acl" do + context "correctly sets" do + + it ":read rights" do + resource.deny_rights(:read, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_read_perms)) + end + + it ":read_execute rights" do + resource.deny_rights(:read_execute, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_read_execute_perms)) + end + + it ":write rights" do + resource.deny_rights(:write, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_write_perms, write_flag)) + end + + it ":modify rights" do + resource.deny_rights(:modify, "Guest") + resource.run_action(:create) + expect(explicit_aces).to eq(denied_acl(SID.Guest, expected_modify_perms)) + end + + it ":full_control rights" do + # deny is an ACE with full rights, but is a deny type ace, not an allow type + resource.deny_rights(:full_control, "Guest") + 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") + resource.run_action(:create) + + expect(explicit_aces).to eq( + denied_acl(SID.Guest, expected_modify_perms) + + allowed_acl(SID.Everyone, expected_read_perms) + ) + end + + it "deny_rights ahead of rights when specified in reverse order" do + resource.deny_rights(:modify, "Guest") + resource.rights(:read, "Everyone") + resource.run_action(:create) + + expect(explicit_aces).to eq( + denied_acl(SID.Guest, expected_modify_perms) + + allowed_acl(SID.Everyone, expected_read_perms) + ) + end end - end context "with a mode attribute" do |