From 8f050d3719033643dc66e011a5bd230137269567 Mon Sep 17 00:00:00 2001 From: Andrew Saraceni Date: Sun, 10 Sep 2017 17:34:51 -0400 Subject: [PATCH] Fix SID Lookup Issues on Assorted Windows Modules (#28979) * fix sid lookup issues and update copyright/license to latest format * simplify win_owner and win_share by removing unnecessary function --- lib/ansible/modules/windows/win_acl.ps1 | 141 +++++------------- lib/ansible/modules/windows/win_acl.py | 21 +-- lib/ansible/modules/windows/win_owner.ps1 | 87 +---------- lib/ansible/modules/windows/win_owner.py | 21 +-- lib/ansible/modules/windows/win_share.ps1 | 95 ++---------- lib/ansible/modules/windows/win_share.py | 21 +-- .../targets/win_owner/tasks/main.yml | 2 +- 7 files changed, 54 insertions(+), 334 deletions(-) diff --git a/lib/ansible/modules/windows/win_acl.ps1 b/lib/ansible/modules/windows/win_acl.ps1 index b8e242bd62c..a67f6d753dc 100644 --- a/lib/ansible/modules/windows/win_acl.ps1 +++ b/lib/ansible/modules/windows/win_acl.ps1 @@ -1,113 +1,43 @@ #!powershell -# This file is part of Ansible -# # Copyright 2015, Phil Schwartz # Copyright 2015, Trond Hindenes # Copyright 2015, Hans-Joachim Kliemeck -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# WANT_JSON -# POWERSHELL_COMMON - +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +#Requires -Module Ansible.ModuleUtils.Legacy.psm1 +#Requires -Module Ansible.ModuleUtils.SID.psm1 + # win_acl module (File/Resources Permission Additions/Removal) - - -#Functions -Function UserSearch -{ - Param ([string]$accountName) - #Check if there's a realm specified - - $searchDomain = $false - $searchDomainUPN = $false - $SearchAppPools = $false - if ($accountName.Split("\").count -gt 1) - { - if ($accountName.Split("\")[0] -eq $env:COMPUTERNAME) - { - } - elseif ($accountName.Split("\")[0] -eq "IIS APPPOOL") - { - $SearchAppPools = $true - $accountName = $accountName.split("\")[1] - } - else - { - $searchDomain = $true - $accountName = $accountName.split("\")[1] +#Functions +function Get-UserSID { + param( + [String]$AccountName + ) + + $userSID = $null + $searchAppPools = $false + + if ($AccountName.Split("\").Count -gt 1) { + if ($AccountName.Split("\")[0] -eq "IIS APPPOOL") { + $searchAppPools = $true + $AccountName = $AccountName.Split("\")[1] } } - Elseif ($accountName.contains("@")) - { - $searchDomain = $true - $searchDomainUPN = $true - } - Else - { - #Default to local user account - $accountName = $env:COMPUTERNAME + "\" + $accountName - } - if (($searchDomain -eq $false) -and ($SearchAppPools -eq $false)) - { - # do not use Win32_UserAccount, because e.g. SYSTEM (BUILTIN\SYSTEM or COMPUUTERNAME\SYSTEM) will not be listed. on Win32_Account groups will be listed too - $localaccount = get-wmiobject -class "Win32_Account" -namespace "root\CIMV2" -filter "(LocalAccount = True)" | where {$_.Caption -eq $accountName} - if ($localaccount) - { - return $localaccount.SID + if ($searchAppPools) { + Import-Module -Name WebAdministration + $testIISPath = Test-Path -Path "IIS:" + if ($testIISPath) { + $appPoolObj = Get-ItemProperty -Path "IIS:\AppPools\$AccountName" + $userSID = $appPoolObj.applicationPoolSid } } - Elseif ($SearchAppPools -eq $true) - { - Import-Module WebAdministration - $testiispath = Test-path "IIS:" - if ($testiispath -eq $false) - { - return $null - } - else - { - $apppoolobj = Get-ItemProperty IIS:\AppPools\$accountName - return $apppoolobj.applicationPoolSid - } + else { + $userSID = Convert-ToSID -account_name $AccountName } - Else - { - #Search by samaccountname - $Searcher = [adsisearcher]"" - If ($searchDomainUPN -eq $false) { - $Searcher.Filter = "sAMAccountName=$($accountName)" - } - Else { - $Searcher.Filter = "userPrincipalName=$($accountName)" - } - - $result = $Searcher.FindOne() - if ($result) - { - $user = $result.GetDirectoryEntry() - - # get binary SID from AD account - $binarySID = $user.ObjectSid.Value - - # convert to string SID - return (New-Object System.Security.Principal.SecurityIdentifier($binarySID,0)).Value - } - } + return $userSID } # Need to adjust token privs when executing Set-ACL in certain cases. @@ -234,9 +164,8 @@ If (-Not (Test-Path -Path $path)) { } # Test that the user/group is resolvable on the local machine -$sid = UserSearch -AccountName ($user) -if (!$sid) -{ +$sid = Get-UserSID -AccountName $user +if (!$sid) { Fail-Json $result "$user is not a valid user or group on the host machine or domain" } @@ -260,14 +189,14 @@ Try { $InheritanceFlag = [System.Security.AccessControl.InheritanceFlags]$inherit $PropagationFlag = [System.Security.AccessControl.PropagationFlags]$propagation - + If ($type -eq "allow") { $objType =[System.Security.AccessControl.AccessControlType]::Allow } Else { $objType =[System.Security.AccessControl.AccessControlType]::Deny } - + $objUser = New-Object System.Security.Principal.SecurityIdentifier($sid) If ($path -match "^HK(CC|CR|CU|LM|U):\\") { $objACE = New-Object System.Security.AccessControl.RegistryAccessRule ($objUser, $colRights, $InheritanceFlag, $PropagationFlag, $objType) @@ -276,7 +205,7 @@ Try { $objACE = New-Object System.Security.AccessControl.FileSystemAccessRule ($objUser, $colRights, $InheritanceFlag, $PropagationFlag, $objType) } $objACL = Get-ACL $path - + # Check if the ACE exists already in the objects ACL list $match = $false @@ -300,7 +229,7 @@ Try { Break } } else { - If (($rule.FileSystemRights -eq $objACE.FileSystemRights) -And ($rule.AccessControlType -eq $objACE.AccessControlType) -And ($ruleIdentity -eq $objACE.IdentityReference) -And ($rule.IsInherited -eq $objACE.IsInherited) -And ($rule.InheritanceFlags -eq $objACE.InheritanceFlags) -And ($rule.PropagationFlags -eq $objACE.PropagationFlags)) { + If (($rule.FileSystemRights -eq $objACE.FileSystemRights) -And ($rule.AccessControlType -eq $objACE.AccessControlType) -And ($ruleIdentity -eq $objACE.IdentityReference) -And ($rule.IsInherited -eq $objACE.IsInherited) -And ($rule.InheritanceFlags -eq $objACE.InheritanceFlags) -And ($rule.PropagationFlags -eq $objACE.PropagationFlags)) { $match = $true Break } @@ -335,11 +264,11 @@ Try { # A rule didn't exist that was trying to be removed Else { Exit-Json $result "the specified rule does not exist" - } + } } } Catch { Fail-Json $result "an error occurred when attempting to $state $rights permission(s) on $path for $user - $($_.Exception.Message)" } - + Exit-Json $result diff --git a/lib/ansible/modules/windows/win_acl.py b/lib/ansible/modules/windows/win_acl.py index a5ad44662f5..d06a3185deb 100644 --- a/lib/ansible/modules/windows/win_acl.py +++ b/lib/ansible/modules/windows/win_acl.py @@ -1,27 +1,8 @@ #!/usr/bin/python -# -*- coding: utf-8 -*- - # Copyright 2015, Phil Schwartz # Copyright 2015, Trond Hindenes # Copyright 2015, Hans-Joachim Kliemeck -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# this is a windows documentation stub. actual code lives in the .ps1 -# file of the same name +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], diff --git a/lib/ansible/modules/windows/win_owner.ps1 b/lib/ansible/modules/windows/win_owner.ps1 index c7209e92853..5be18df595f 100644 --- a/lib/ansible/modules/windows/win_owner.ps1 +++ b/lib/ansible/modules/windows/win_owner.ps1 @@ -1,85 +1,9 @@ #!powershell -# This file is part of Ansible -# # Copyright 2015, Hans-Joachim Kliemeck -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -# WANT_JSON -# POWERSHELL_COMMON - -#Functions -Function UserSearch -{ - Param ([string]$accountName) - #Check if there's a realm specified - - $searchDomain = $false - $searchDomainUPN = $false - if ($accountName.Split("\").count -gt 1) - { - if ($accountName.Split("\")[0] -ne $env:COMPUTERNAME) - { - $searchDomain = $true - $accountName = $accountName.split("\")[1] - } - } - Elseif ($accountName.contains("@")) - { - $searchDomain = $true - $searchDomainUPN = $true - } - Else - { - #Default to local user account - $accountName = $env:COMPUTERNAME + "\" + $accountName - } - - if ($searchDomain -eq $false) - { - # do not use Win32_UserAccount, because e.g. SYSTEM (BUILTIN\SYSTEM or COMPUUTERNAME\SYSTEM) will not be listed. on Win32_Account groups will be listed too - $localaccount = get-wmiobject -class "Win32_Account" -namespace "root\CIMV2" -filter "(LocalAccount = True)" | where {$_.Caption -eq $accountName} - if ($localaccount) - { - return $localaccount.SID - } - } - Else - { - #Search by samaccountname - $Searcher = [adsisearcher]"" - - If ($searchDomainUPN -eq $false) { - $Searcher.Filter = "sAMAccountName=$($accountName)" - } - Else { - $Searcher.Filter = "userPrincipalName=$($accountName)" - } - - $result = $Searcher.FindOne() - if ($result) - { - $user = $result.GetDirectoryEntry() - - # get binary SID from AD account - $binarySID = $user.ObjectSid.Value - - # convert to string SID - return (New-Object System.Security.Principal.SecurityIdentifier($binarySID,0)).Value - } - } -} +#Requires -Module Ansible.ModuleUtils.Legacy.psm1 +#Requires -Module Ansible.ModuleUtils.SID.psm1 $result = @{ changed = $false @@ -97,9 +21,8 @@ If (-Not (Test-Path -Path $path)) { } # Test that the user/group is resolvable on the local machine -$sid = UserSearch -AccountName ($user) -if (-not $sid) -{ +$sid = Convert-ToSID -account_name $user +if (!$sid) { Fail-Json $result "$user is not a valid user or group on the host machine or domain" } diff --git a/lib/ansible/modules/windows/win_owner.py b/lib/ansible/modules/windows/win_owner.py index bfe7b9f1e36..6c160da7a42 100644 --- a/lib/ansible/modules/windows/win_owner.py +++ b/lib/ansible/modules/windows/win_owner.py @@ -1,25 +1,6 @@ #!/usr/bin/python -# -*- coding: utf-8 -*- - # Copyright 2015, Hans-Joachim Kliemeck -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# this is a windows documentation stub. actual code lives in the .ps1 -# file of the same name +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], diff --git a/lib/ansible/modules/windows/win_share.ps1 b/lib/ansible/modules/windows/win_share.ps1 index 617a1a3b7d3..572e78b78cb 100644 --- a/lib/ansible/modules/windows/win_share.ps1 +++ b/lib/ansible/modules/windows/win_share.ps1 @@ -1,87 +1,12 @@ #!powershell -# This file is part of Ansible - # Copyright 2015, Hans-Joachim Kliemeck -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -# WANT_JSON -# POWERSHELL_COMMON +#Requires -Module Ansible.ModuleUtils.Legacy.psm1 +#Requires -Module Ansible.ModuleUtils.SID.psm1 #Functions -Function UserSearch -{ - Param ([string]$accountName) - #Check if there's a realm specified - - $searchDomain = $false - $searchDomainUPN = $false - if ($accountName.Split("\").count -gt 1) - { - if ($accountName.Split("\")[0] -ne $env:COMPUTERNAME) - { - $searchDomain = $true - $accountName = $accountName.split("\")[1] - } - } - Elseif ($accountName.contains("@")) - { - $searchDomain = $true - $searchDomainUPN = $true - } - Else - { - #Default to local user account - $accountName = $env:COMPUTERNAME + "\" + $accountName - } - - if ($searchDomain -eq $false) - { - # do not use Win32_UserAccount, because e.g. SYSTEM (BUILTIN\SYSTEM or COMPUUTERNAME\SYSTEM) will not be listed. on Win32_Account groups will be listed too - $localaccount = get-wmiobject -class "Win32_Account" -namespace "root\CIMV2" -filter "(LocalAccount = True)" | where {$_.Caption -eq $accountName} - if ($localaccount) - { - return $localaccount.SID - } - } - Else - { - #Search by samaccountname - $Searcher = [adsisearcher]"" - - If ($searchDomainUPN -eq $false) { - $Searcher.Filter = "sAMAccountName=$($accountName)" - } - Else { - $Searcher.Filter = "userPrincipalName=$($accountName)" - } - - $result = $Searcher.FindOne() - if ($result) - { - $user = $result.GetDirectoryEntry() - - # get binary SID from AD account - $binarySID = $user.ObjectSid.Value - - # convert to string SID - return (New-Object System.Security.Principal.SecurityIdentifier($binarySID,0)).Value - } - } -} -Function NormalizeAccounts -{ +Function NormalizeAccounts { param( [parameter(valuefrompipeline=$true)] $users @@ -89,17 +14,17 @@ Function NormalizeAccounts $users = $users.Trim() If ($users -eq "") { - $splittedUsers = [Collections.Generic.List[String]] @() + $splitUsers = [Collections.Generic.List[String]] @() } Else { - $splittedUsers = [Collections.Generic.List[String]] $users.Split(",") + $splitUsers = [Collections.Generic.List[String]] $users.Split(",") } $normalizedUsers = [Collections.Generic.List[String]] @() - ForEach($splittedUser in $splittedUsers) { - $sid = UserSearch $splittedUser - If (!$sid) { - Fail-Json $result "$splittedUser is not a valid user or group on the host machine or domain" + ForEach($splitUser in $splitUsers) { + $sid = Convert-ToSID -account_name $splitUser + if (!$sid) { + Fail-Json $result "$splitUser is not a valid user or group on the host machine or domain" } $normalizedUser = (New-Object System.Security.Principal.SecurityIdentifier($sid)).Translate([System.Security.Principal.NTAccount]) diff --git a/lib/ansible/modules/windows/win_share.py b/lib/ansible/modules/windows/win_share.py index 2c3408d44bd..7772d67cb1f 100644 --- a/lib/ansible/modules/windows/win_share.py +++ b/lib/ansible/modules/windows/win_share.py @@ -1,25 +1,6 @@ #!/usr/bin/python -# -*- coding: utf-8 -*- - # Copyright 2015, Hans-Joachim Kliemeck -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# this is a windows documentation stub. actual code lives in the .ps1 -# file of the same name +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], diff --git a/test/integration/targets/win_owner/tasks/main.yml b/test/integration/targets/win_owner/tasks/main.yml index dc254fc8b06..2578da65ee9 100644 --- a/test/integration/targets/win_owner/tasks/main.yml +++ b/test/integration/targets/win_owner/tasks/main.yml @@ -45,7 +45,7 @@ path: "{{test_win_owner_path}}" user: invalid-user register: invalid_user - failed_when: invalid_user.msg != 'invalid-user is not a valid user or group on the host machine or domain' + failed_when: invalid_user.msg is not search("account_name invalid-user is not a valid account, cannot get SID.*") - name: set owner defaults check win_owner: