puppetlabs/puppet — pull request #2211 by joshcooper (2014-01-02 21:10:20+00:00)
Title: (PUP-1255) Don't be sensitive to pseudo-inherited ACEs

Previously, the test would fail on Windows 2012, as the access control
entries in the Temp directory's DACL are missing the INHERITED_ACE bit:

> setacl -on temp -ot file -actn list -lst "f:tab;w:d,s,o,g;i:y"
temp

Owner: WIN-61G7FSKJ7JI\josh
Group: WIN-61G7FSKJ7JI\None
DACL(not_protected):
NT AUTHORITY\SYSTEM full allow container_inherit+object_inherit
BUILTIN\Administrators full allow container_inherit+object_inherit
WIN-61G7FSKJ7JI\josh full allow container_inherit+object_inherit

Note how none of the entries granting 'full' access are "inherited".

The test creates two empty temp files (the source is generated in the
call to Puppet::Util.replace_file) each with permissions:

WIN-61G7FSKJ7JI\josh
WIN-61G7FSKJ7JI\None
NT AUTHORITY\SYSTEM:(I) 0x1f01ff
BUILTIN\Administrators:(I) 0x1f01ff
WIN-61G7FSKJ7JI\josh:(I) 0x1f01ff

Each ACE is inherited and grants full access as expected. But when the Win32
API `ReplaceFile` is called, it prepends non-inherited ACEs to the destination
file:

WIN-61G7FSKJ7JI\josh
WIN-61G7FSKJ7JI\None
NT AUTHORITY\SYSTEM: 0x1f01ff
BUILTIN\Administrators: 0x1f01ff
WIN-61G7FSKJ7JI\josh: 0x1f01ff
NT AUTHORITY\SYSTEM:(I) 0x1f01ff
BUILTIN\Administrators:(I) 0x1f01ff
WIN-61G7FSKJ7JI\josh:(I) 0x1f01ff

While the merging behavior is unexpected, it doesn't alter the effective
security of the file.

This commit marks the directory as protected, which breaks inheritance
at that point. As a result, ReplaceFile behaves as expected.

Also note that if the Temp ACLs are reset recursively:

icacls Temp /reset /t

Then the merging behavior does not occur and the test passes (without
this commit).

See also pseudo-inherited ACEs:
http://helgeklein.com/setacl/faq/#what-are-pseudo-protected-acls-and-pseudo-inherited-aces-setacl-2-1