Jira (PUP-9068) Windows admin? check should consider group membership

2 views
Skip to first unread message

Ethan Brown (JIRA)

unread,
Aug 15, 2018, 1:29:03 PM8/15/18
to puppe...@googlegroups.com
Ethan Brown created an issue
 
Puppet / Improvement PUP-9068
Windows admin? check should consider group membership
Issue Type: Improvement Improvement
Affects Versions: PUP 5.5.3
Assignee: Unassigned
Components: Windows
Created: 2018/08/15 10:28 AM
Fix Versions: PUP 5.3.z, PUP 5.5.z
Labels: windows ntfs security
Priority: Normal Normal
Reporter: Ethan Brown

In PA-2019, the installer was changed to lay down permissions differently so that ProgramData generally has Administrators: (F) and SYSTEM: (F) set recursively.

It's possible to create an "administrative" user based on their token privileges, but without actually making them part of the Administrators group. The check inside Puppet at for elevated_security? at https://github.com/puppetlabs/puppet/blob/e7839794a1d7d393e6716927764c1276494123c2/lib/puppet/util/windows/process.rb#L183-L205 will then pass, despite the user not being in Administrators.

If such a user is assigned to the Puppet service, then pandemonium ensues, given how permissions are set on ProgramData\PuppetLabs.

The admin? check should be altered to ensure the user is part of Administrators or not. This determines where data can be written for that user.

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Glenn Sarti (JIRA)

unread,
Aug 21, 2018, 7:14:03 PM8/21/18
to puppe...@googlegroups.com
Glenn Sarti commented on Improvement PUP-9068
 
Re: Windows admin? check should consider group membership

Ethan Brown Can you add the instructions on how to create an admin user without admin group. This seems very edgecae-y

Erick Banks (JIRA)

unread,
Sep 4, 2018, 7:41:03 PM9/4/18
to puppe...@googlegroups.com
Erick Banks updated an issue
 
Change By: Erick Banks
Sprint: Windows Grooming Hopper

Erick Banks (JIRA)

unread,
Sep 4, 2018, 7:42:02 PM9/4/18
to puppe...@googlegroups.com

Ethan Brown (JIRA)

unread,
Sep 6, 2018, 10:31:03 PM9/6/18
to puppe...@googlegroups.com
Ethan Brown commented on Improvement PUP-9068
 
Re: Windows admin? check should consider group membership

There's a nice PowerShell module that does all the heavy lifting with privilege assignment at https://gallery.technet.microsoft.com/scriptcenter/Grant-Revoke-Query-user-26e259b0

Through the process of elimination, I was able to determine the single token privilege necessary to "trick" our code - namely SeImpersonatePrivilege

# create the user
net user testadmin Admin123 /add
# grant the impersonation privilege
Grant-UserRight -Account testadmin -Right SeImpersonatePrivilege
 
# verify user rights - should return only the SeImpersonatePrivilege
Get-UserRightsGrantedToAccount testadmin
 
# use psexec to launch a cmd process and navigate to a directory with Puppet installed, for instance C:\source\puppetlabs-scheduled_task>
# run ruby and show elevated is on
bundle exec ruby -e "require 'puppet'; puts Puppet::Util::Windows::Process.elevated_security?"
# true

For reference, the account should also display something similar to the following for whoami /all

USER INFORMATION
----------------
 
User Name                SID
======================== ============================================
vagrant-2008r2\testadmin S-1-5-21-271343509-1886877197-423808128-4919
 
 
GROUP INFORMATION
-----------------
 
Group Name                                           Type             SID                                          Attributes
==================================================== ================ ============================================ ==================================================
Everyone                                             Well-known group S-1-1-0                                      Mandatory group, Enabled by default, Enabled group
VAGRANT-2008R2\g45991a14-ee2d-48f6-925c-6ea809a5f994 Alias            S-1-5-21-271343509-1886877197-423808128-4735 Mandatory group, Enabled by default, Enabled group
VAGRANT-2008R2\TestGroup-PUP8231                     Alias            S-1-5-21-271343509-1886877197-423808128-4699 Mandatory group, Enabled by default, Enabled group
BUILTIN\Users                                        Alias            S-1-5-32-545                                 Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\INTERACTIVE                             Well-known group S-1-5-4                                      Mandatory group, Enabled by default, Enabled group
CONSOLE LOGON                                        Well-known group S-1-2-1                                      Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\Authenticated Users                     Well-known group S-1-5-11                                     Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\This Organization                       Well-known group S-1-5-15                                     Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\Local account                           Well-known group S-1-5-113                                    Mandatory group, Enabled by default, Enabled group
NT AUTHORITY\NTLM Authentication                     Well-known group S-1-5-64-10                                  Mandatory group, Enabled by default, Enabled group
Mandatory Label\High Mandatory Level                 Label            S-1-16-12288                                 Mandatory group, Enabled by default, Enabled group
 
 
PRIVILEGES INFORMATION
----------------------
 
Privilege Name                Description                               State
============================= ========================================= ========
SeChangeNotifyPrivilege       Bypass traverse checking                  Enabled
SeImpersonatePrivilege        Impersonate a client after authentication Enabled
SeIncreaseWorkingSetPrivilege Increase a process working set            Disabled
 
C:\Windows\system32>

For reference, with the privilege removed, the output of whoami /all is nearly identical except for the SeImpersonatePrivilege assignment above and the Medium Mandatory Level set instead of the High Mandatory Level above.

Mandatory Label\Medium Mandatory Level               Label            S-1-16-8192                                  Mandatory group, Enabled by default, Enabled group

There's a good tidbit about high integrity tokens not required to be in the Administrators group at https://stackoverflow.com/a/30970434
Another real-world example is at https://peter.hahndorf.eu/blog/elevate-nonadmin.html

The MSDN documentation for the integrity mechanism is at https://msdn.microsoft.com/en-us/library/bb625963.aspx

Erick Banks (JIRA)

unread,
Sep 10, 2018, 2:18:03 PM9/10/18
to puppe...@googlegroups.com

Erick Banks (JIRA)

unread,
Sep 10, 2018, 2:18:04 PM9/10/18
to puppe...@googlegroups.com
Erick Banks updated an issue
Change By: Erick Banks
Fix Version/s: PUP 5.3.z

Kenn Hussey (JIRA)

unread,
Sep 24, 2018, 10:00:25 AM9/24/18
to puppe...@googlegroups.com

Kenn Hussey (JIRA)

unread,
Sep 24, 2018, 8:00:04 PM9/24/18
to puppe...@googlegroups.com
Kenn Hussey commented on Improvement PUP-9068
 
Re: Windows admin? check should consider group membership

Ethan Brown are you still planning to get this in for Puppet 6.0.1?

Kenn Hussey (JIRA)

unread,
Sep 25, 2018, 4:53:05 PM9/25/18
to puppe...@googlegroups.com
Kenn Hussey updated an issue
Change By: Kenn Hussey
Fix Version/s: PUP 6.0.1
Fix Version/s: PUP 6.0.z

Kenn Hussey (JIRA)

unread,
Sep 25, 2018, 4:53:06 PM9/25/18
to puppe...@googlegroups.com

Keiran Haggerty (JIRA)

unread,
Sep 25, 2018, 7:22:04 PM9/25/18
to puppe...@googlegroups.com
Keiran Haggerty assigned an issue to Unassigned
Change By: Keiran Haggerty
Assignee: Ethan Brown

Keiran Haggerty (JIRA)

unread,
Sep 25, 2018, 7:23:05 PM9/25/18
to puppe...@googlegroups.com
Keiran Haggerty updated an issue
Change By: Keiran Haggerty
Fix Version/s: PUP 6.0.z
Fix Version/s: PUP 6.0.2

Keiran Haggerty (JIRA)

unread,
Sep 25, 2018, 7:25:03 PM9/25/18
to puppe...@googlegroups.com

Keiran Haggerty (JIRA)

unread,
Sep 25, 2018, 7:25:04 PM9/25/18
to puppe...@googlegroups.com
Keiran Haggerty updated an issue
Change By: Keiran Haggerty
Sprint: Windows Hopper 2018-10-3

Keiran Haggerty (JIRA)

unread,
Sep 25, 2018, 7:26:05 PM9/25/18
to puppe...@googlegroups.com

Glenn Sarti (JIRA)

unread,
Oct 1, 2018, 8:10:06 PM10/1/18
to puppe...@googlegroups.com
Glenn Sarti commented on Improvement PUP-9068

Ethan Brown So after pairing with Erick Banks this is probably not an issue.

The elevated_security? method you posted is only called by Puppet::Util::Windows::User.admin? which already does the elevated AND group membership checks.

This is then called by Puppet::Util::SUIDManager.root? to determine if a user is a root. I also verified the correct behaviour of Puppet::Util::Windows::User.admin? using the restricted token set you posted earlier i.e. it was always false.

Given this I don't think there's anything to do for this ticket. Closing as Won't Do.

Glenn Sarti (JIRA)

unread,
Oct 1, 2018, 9:03:03 PM10/1/18
to puppe...@googlegroups.com

Glenn Sarti (JIRA)

unread,
Oct 1, 2018, 9:04:05 PM10/1/18
to puppe...@googlegroups.com
Glenn Sarti assigned an issue to Unassigned
 
Change By: Glenn Sarti
Assignee: Glenn Sarti

Michael Lombardi (JIRA)

unread,
Oct 1, 2018, 9:34:06 PM10/1/18
to puppe...@googlegroups.com

Ethan Brown (JIRA)

unread,
Oct 3, 2018, 12:53:07 PM10/3/18
to puppe...@googlegroups.com
Ethan Brown commented on Improvement PUP-9068

https://github.com/puppetlabs/puppet/commits/abda86cec25e62e6a2fb80150294469e1031d3fa includes the merged commit

Last known good build of puppet-agent on 5.5.x:
Built at: 1 Oct 2018 22:09:08
PUPPET_AGENT_VERSION: 5.5.6.141.gdb6e53b
PUPPET_AGENT_COMMIT: db6e53b11c698e42a163be82f3f603ea6d122668
PUPPET_AGENT_SHORT_COMMIT: db6e53b11
FACTER_COMMIT: 8dd59ecdfbbf9a0c24f3257c960fb95feb241c9c
PUPPET_COMMIT: abda86cec25e62e6a2fb80150294469e1031d3fa
HIERA_COMMIT: 715ae4039e4cc7d248ccd9a1cf74c65d8b7f6226
PXPAGENT_COMMIT: b23ec4b6b114e766ce183fd311be047cd4dcf735

Josh Cooper (JIRA)

unread,
Jul 15, 2019, 7:02:04 PM7/15/19
to puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
Fix Version/s: PUP 5.5.z
Fix Version/s: PUP 5.5.7
Reply all
Reply to author
Forward
0 new messages