Skip to content

Comments

Ticket/2.7.x/9186 windows file security#97

Merged
nicklewis merged 1 commit intopuppetlabs:2.7.xfrom
joshcooper:ticket/2.7.x/9186-windows-file-security
Sep 14, 2011
Merged

Ticket/2.7.x/9186 windows file security#97
nicklewis merged 1 commit intopuppetlabs:2.7.xfrom
joshcooper:ticket/2.7.x/9186-windows-file-security

Conversation

@joshcooper
Copy link
Contributor

The Puppet::Util::WindowsSecurity module maps POSIX owner, group, and
modes to the Windows security model, and back.

The primary goal of this mapping is to ensure that owner, group, and
modes can be round-tripped in a consistent and deterministic
way. Otherwise, Puppet might think file resources are out-of-sync
every time it runs. A secondary goal is to provide equivalent
permissions for common use-cases. For example, setting the owner to
"Administrators", group to "Users", and mode to 750 (which also denies
access to everyone else.)

There are some well-known problems mapping windows and POSIX
permissions due to differences between the two security models. Search
for "POSIX permission mapping leak". In POSIX, access to a file is
determined solely based on the most specific class (user, group,
other). So a mode of 460 would deny write access to the owner even if
they are a member of the group. But in Windows, the entire access
control list is walked until the user is explicitly denied or allowed
(denied take precedence, and if neither occurs they are denied). As a
result, a user could be allowed access based on their group
membership. To solve this problem, other people have used deny access
control entries to more closely model POSIX, but this introduces a lot
of complexity.

In general, this implementation only supports "typical" permissions,
where group permissions are a subset of user, and other permissions
are a subset of group, e.g. 754, but not 467. However, there are some
Windows quirks to be aware of.

  • The owner can be either a user or group SID, and most system files
    are owned by the Administrators group.
  • The group can be either a user or group SID.
  • Unexpected results can occur if the owner and group are the
    same, but the user and group classes are different, e.g. 750. In
    this case, it is not possible to allow write access to the owner,
    but not the group. As a result, the actual permissions set on the
    file would be 770.
  • In general, only privileged users can set the owner, group, or
    change the mode for files they do not own. In 2003, the user must
    be a member of the Administrators group. In Vista/2008, the user
    must be running with elevated privileges.
  • A file/dir can be deleted by anyone with the DELETE access right
    OR by anyone that has the FILE_DELETE_CHILD access right for the
    parent. See http://support.microsoft.com/kb/238018. But on Unix,
    the user must have write access to the file/dir AND execute access
    to all of the parent path components.
  • Many access control entries are inherited from parent directories,
    and it is common for file/dirs to have more than 3 entries,
    e.g. Users, Power Users, Administrators, SYSTEM, etc, which cannot
    be mapped into the 3 class POSIX model. The get_mode method will
    set the S_IEXTRA bit flag indicating that an access control entry
    was found whose SID is neither the owner, group, or other. This
    enables Puppet to detect when file/dirs are out-of-sync,
    especially those that Puppet did not create, but is attempting
    to manage.
  • On Unix, the owner and group can be modified without changing the
    mode. But on Windows, an access control entry specifies which SID
    it applies to. As a result, the set_owner and set_group methods
    automatically rebuild the access control list based on the new
    (and different) owner or group.

@joshcooper
Copy link
Contributor Author

I've made additional changes and repushed. I moved the Windows Error into its own class, created a puppet/util/windows directory hold the error and the security classes, and eliminated code duplication when getting/setting owner and group sids. Also eliminated the use of the win32/file gem as it caused more problems than it solved.

The Puppet::Util::Windows::Security module maps POSIX owner, group,
and modes to the Windows security model, and back.

The primary goal of this mapping is to ensure that owner, group, and
modes can be round-tripped in a consistent and deterministic
way. Otherwise, Puppet might think file resources are out-of-sync
every time it runs. A secondary goal is to provide equivalent
permissions for common use-cases. For example, setting the owner to
"Administrators", group to "Users", and mode to 750 (which also denies
access to everyone else.)

There are some well-known problems mapping windows and POSIX
permissions due to differences between the two security models. Search
for "POSIX permission mapping leak". In POSIX, access to a file is
determined solely based on the most specific class (user, group,
other). So a mode of 460 would deny write access to the owner even if
they are a member of the group. But in Windows, the entire access
control list is walked until the user is explicitly denied or allowed
(denied take precedence, and if neither occurs they are denied). As a
result, a user could be allowed access based on their group
membership. To solve this problem, other people have used deny access
control entries to more closely model POSIX, but this introduces a lot
of complexity.

In general, this implementation only supports "typical" permissions,
where group permissions are a subset of user, and other permissions
are a subset of group, e.g. 754, but not 467.  However, there are some
Windows quirks to be aware of.

* The owner can be either a user or group SID, and most system files
  are owned by the Administrators group.
* The group can be either a user or group SID.
* Unexpected results can occur if the owner and group are the
  same, but the user and group classes are different, e.g. 750. In
  this case, it is not possible to allow write access to the owner,
  but not the group. As a result, the actual permissions set on the
  file would be 770.
* In general, only privileged users can set the owner, group, or
  change the mode for files they do not own. In 2003, the user must
  be a member of the Administrators group. In Vista/2008, the user
  must be running with elevated privileges.
* A file/dir can be deleted by anyone with the DELETE access right
  OR by anyone that has the FILE_DELETE_CHILD access right for the
  parent. See http://support.microsoft.com/kb/238018. But on Unix,
  the user must have write access to the file/dir AND execute access
  to all of the parent path components.
* Many access control entries are inherited from parent directories,
  and it is common for file/dirs to have more than 3 entries,
  e.g. Users, Power Users, Administrators, SYSTEM, etc, which cannot
  be mapped into the 3 class POSIX model. The get_mode method will
  set the S_IEXTRA bit flag indicating that an access control entry
  was found whose SID is neither the owner, group, or other. This
  enables Puppet to detect when file/dirs are out-of-sync,
  especially those that Puppet did not create, but is attempting
  to manage.
* On Unix, the owner and group can be modified without changing the
  mode. But on Windows, an access control entry specifies which SID
  it applies to. As a result, the set_owner and set_group methods
  automatically rebuild the access control list based on the new
  (and different) owner or group.

We are not using the win32-file gem, because it modifies several File
methods, e.g. dirname, basename, to return paths with
backslashes. This breaks other parts of the code, notably Pathname, as
calling File.basename("c:/") returns "c:\\", which prevents Pathname
from finding the root of the drive. Instead, we call the
GetFileAttributes and SetFileAttributes Win32 API directly.

This commit also introduces a Puppet::Util::Windows::Error class that
extends Puppet::Error, and provides access to the Win32 last error
code and localized Windows error message.
nicklewis added a commit that referenced this pull request Sep 14, 2011
…-security

Ticket/2.7.x/9186 windows file security
@nicklewis nicklewis merged commit 9e725e4 into puppetlabs:2.7.x Sep 14, 2011
Iristyle pushed a commit to Iristyle/puppet that referenced this pull request Sep 9, 2014
(RE-929) update build_defaults for pe-puppet
melissa pushed a commit to melissa/puppet that referenced this pull request Mar 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants