New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit permissions - remove inheritance/lock down to admins #398

Closed
ferventcoder opened this Issue Aug 26, 2015 · 1 comment

Comments

Projects
None yet
1 participant
@ferventcoder
Member

ferventcoder commented Aug 26, 2015

This further restricts the default installation location by removing all permissions and inheritance of permissions, explicitly giving Administrator/LocalSystem to Full access, and Users are granted Read and Execute.
In prior installations, we ensured Modify access to the installing user, but that has been removed for security reasons. Should you need the previous behavior, set $env:ChocolateyInstallAllowCurrentUser="true".

  • If non-default install location, skip changing security entirely with a warning for user to do that themselves
  • If attempting to install to default location without administrative token, throw an error
  • Remove inheritance with no copy of existing inherited permissions
  • Remove all existing explicit permissions
  • Explicitly set permissions to Administrators/SYSTEM Full "(OI)(CI)(F)", Users Read/Execute "(OI)(CI)(RX)"
  • Replace ACL Owner with Administrators
  • Do not set user modify without an explicit environment variable - $env:ChocolateyInstallAllowCurrentUser="true".

I owe @jberezanski a debt of gratitude for his tireless work in providing code, examples and testing all the scenarios to find security holes.

Details

https://github.com/chocolatey/choco/blob/master/nuget/chocolatey/tools/chocolateysetup.psm1#L174-L186

http://stackoverflow.com/questions/10970957/changing-permissions-with-powershell-doesnt-propogate-to-children

A non-admin can write to the programdata folder, but they cannot modify or append to existing files. They are also not able to delete existing files they did not put there (and possibly not even those files).

Lock down the default folder even more so that by default, non-admins can not even write to the folder.

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
Member

ferventcoder commented Jan 23, 2016

Another place to see some examples of acl setting came in an interesting unrelated search http://blog.enowsoftware.com/solutions-engine/bid/185867/Powershell-Upping-your-Parameter-Validation-Game-with-Dynamic-Parameters-Part-II

@ferventcoder ferventcoder self-assigned this Jun 12, 2016

@ferventcoder ferventcoder added 3 - Done and removed 0 - Backlog labels Jun 13, 2016

@ferventcoder ferventcoder changed the title from Be explicit in permissions to Explicit permissions - remove inheritance/lock down to admins Jun 13, 2016

ferventcoder added a commit that referenced this issue Jun 13, 2016

(GH-398) Explicitly Set Permissions
Explicitly set ACEs for Chocolatey's default folder installation to
Administrators, removing inherited permissions and ensuring container
and object inheritance flow from explicit ACE settings.

Only remove inheritance for the default install folder, if being
installed elsewhere, leave inherited permissions alone.

Additionally, only set the current user to have modify access if there
is an environment variable set to allow this behavior -
`$env:ChocolateyInstallAllowCurrentUser="true"`.

Based on conversations and code examples from @jberezanski. Jakub wrote
the Get-LocalizedWellKnownPrincipalName to return the localized user
name for Well-Known SIDS.

ferventcoder added a commit that referenced this issue Jun 13, 2016

Merge branch 'stable'
* stable:
  (version) 0.9.10-rc1
  (doc) update CHANGELOG/nuspec
  (GH-398) Explicitly Set Permissions
  (doc) fix use of * to code comment

ferventcoder added a commit that referenced this issue Jun 17, 2016

(GH-398) Allow append for users on logs directory
To see errors and otherwise, allow users permission to append to log
files.

ferventcoder added a commit that referenced this issue Jun 17, 2016

(GH-398) Default install permissions enhancements
Adding to the changes in 680dc88, further secure down the default
Chocolatey installation directory.

- If not the default install path, do not make any changes. Issue a
   warning so the user knows they need to secure their own install

For the default install path:

 - If not being run from an administrative context, throw. Do not allow
    the installation.
- Remove existing explicit permissions
- Change the owner to Administrators

This work is the result of a continued conversation with @jberezanski
and thanks goes to Jakub in providing some of this code and testing the
installation for security holes.

ferventcoder added a commit that referenced this issue Jun 17, 2016

Merge branch 'stable'
* stable:
  (version) 0.9.10
  (GH-398) Default install permissions enhancements
  (doc) update CHANGELOG/nuspec
  (GH-732) reduce default webrequest timeout to 30s
  (maint) fix environmentvariable
  (GH-802) PowerShell Host - Exit codes should return
  (GH-398) Allow append for users on logs directory
  (GH-794) Warn when non-admin in default install
  (GH-793) Failing to update config - log to debug
  (GH-796) Relax upgrade log package separation
  (GH-801) Fix - bad nuspec from choco new
  (legal) add logo use policy
  (GH-795) Adjust message for missing licensed code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment