Skip to content

Questions / Problems with function Set-SecureFileACL #719

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

Closed
DarwinJS opened this issue May 13, 2017 · 6 comments
Closed

Questions / Problems with function Set-SecureFileACL #719

DarwinJS opened this issue May 13, 2017 · 6 comments

Comments

@DarwinJS
Copy link

DarwinJS commented May 13, 2017

On this page: https://github.com/PowerShell/Win32-OpenSSH/wiki/Security-protection-of-various-files-in-win32-openssh

This code is published:

function Set-SecureFileACL
{
    param(
        [string]$FilePath,
        [System.Security.Principal.NTAccount]$Owner = $null
        )

    $myACL = Get-ACL -Path $FilePath
    $myACL.SetAccessRuleProtection($True, $True)
    Set-Acl -Path $FilePath -AclObject $myACL

    $myACL = Get-ACL $FilePath
    $actualOwner = $null
    if($owner -eq $null)
    {
        $actualOwner = New-Object System.Security.Principal.NTAccount($($env:USERDOMAIN), $($env:USERNAME))
    }
    else
    {
        $actualOwner = $Owner
    }

    $myACL.SetOwner($actualOwner)

    if($myACL.Access)
    {
        $myACL.Access | % {
            if(-not ($myACL.RemoveAccessRule($_)))
            {
                throw "failed to remove access of $($_.IdentityReference.Value) rule in setup "
            }
        }
    }

    $objACE = New-Object System.Security.AccessControl.FileSystemAccessRule `
        ($actualOwner, "FullControl", "None", "None", "Allow")
    $myACL.AddAccessRule($objACE)
    Set-Acl -Path $FilePath -AclObject $myACL
}

I have some questions about it:

  1. Shouldn't $myACL.SetAccessRuleProtection($True, $True) be $myACL.SetAccessRuleProtection($True, $False)' in order to disable inherited permissions? (https://msdn.microsoft.com/en-us/library/system.security.accesscontrol.objectsecurity.setaccessruleprotection(v=vs.110).aspx)
  2. The line if(-not ($myACL.RemoveAccessRule($_))) generates an exception and never falls through to the if block. I believe it needs "Try / Catch" to actually allow the offending ID to be emited.
  3. On Windows 10, I am getting the below exception for accounts "APPLICATION PACKAGE AUTHORITY\ALL APPLICATION PACKAGES" and "APPLICATION PACKAGE AUTHORITY\ALL RESTRICTED APPLICATION PACKAGES". This happens even when I change the line mentioned in Interactive commands not supported #1 to $myACL.SetAccessRuleProtection($True, $False)'

HOWEVER, these two users do not appear in the ACLs I see in Explorer - and it keeps getting the error every time. I don't know if these identities originally had permissions did or not. It's almost like there is some type of virtual ACL?

"Exception             : System.Management.Automation.MethodInvocationException: Exception calling "RemoveAccessRule" with "1" argument(s): "Some or all identity references could not be
                        translated." ---> System.Security.Principal.IdentityNotMappedException: Some or all identity references could not be translated.
                           at System.Security.Principal.NTAccount.Translate(IdentityReferenceCollection sourceAccounts, Type targetType, Boolean forceSuccess)
                           at System.Security.Principal.NTAccount.Translate(Type targetType)
                           at System.Security.AccessControl.CommonObjectSecurity.ModifyAccess(AccessControlModification modification, AccessRule rule, Boolean& modified)
                           at System.Security.AccessControl.CommonObjectSecurity.RemoveAccessRule(AccessRule rule)
                           at CallSite.Target(Closure , CallSite , Object , Object )
                           --- End of inner exception stack trace ---
                           at System.Management.Automation.ExceptionHandlingOps.ConvertToMethodInvocationException(Exception exception, Type typeToThrow, String methodName, Int32 numArgs,
                        MemberInfo memberInfo)
                           at CallSite.Target(Closure , CallSite , Object , Object )
                           at System.Management.Automation.Interpreter.DynamicInstruction`3.Run(InterpretedFrame frame)
                           at System.Management.Automation.Interpreter.EnterTryCatchFinallyInstruction.Run(InterpretedFrame frame)"
@DarwinJS
Copy link
Author

DarwinJS commented May 13, 2017

I have a work around - but I'm not sure if its implications.

I am also creating this code to work in the chocolatey package - so a little concerned that there are other similar scenarios lurking - so I'm still very interested in why this happens and if there is another class of "special user accounts" that might cause similar errors with this particular approach to removing acls.

I replaced the code:

          if(-not ($myACL.RemoveAccessRule($_)))
          {
            throw "failed to remove access of $($_.IdentityReference.Value) rule in setup "
          }

With:

          $IdentityToProcess = $_.IdentityReference.Value
          Try 
          {
            $result = $myACL.RemoveAccessRule($_)
          }
          Catch [System.Management.Automation.MethodInvocationException]
          {
            #Ignore "IdentityNotMappedException" for "APPLICATION PACKAGE AUTHORITY\*" identities (and others)
            If ($_.FullyQualifiedErrorId -ne 'IdentityNotMappedException')
            {
              Throw "Error trying to remove access of $IdentityToProcess"
            }
          }
          Catch 
          {
            Throw "Error trying to remove access of $IdentityToProcess"
          }

I tried some methods other than ".RemoveAccessRule" (e.g. RemoveAccessRuleSpecific) and they get the same error on the same user ids.

@DarwinJS
Copy link
Author

Could someone on the dev team test the permissions scripts on Windows 7 / Server 2008?

At first I started getting "The security identifier is not allowed to be the owner of this object" for granting ownership of the server keys to "NT SERVICE\sshd" (only on Windows 7) and researching a work around led to the following rabbit holes:

https://learn-powershell.net/2014/06/24/changing-ownership-of-file-or-folder-using-powershell/
https://www.sysadmins.lv/blog-ru/strannosti-set-acl.aspx
https://social.technet.microsoft.com/Forums/scriptcenter/en-US/87679d43-04d5-4894-b35b-f37a6f5558cb/solved-how-to-take-ownership-and-change-permissions-for-blocked-files-and-folders-in-powershell?forum=winserverpowershell

I tried an implementation of the code in that last post and couldn't get it to work. The error changes to "Attempted to perform an unauthorized operation."

I also tried reordering to put the ownership change AFTER the granting of full permissions to "NT SERVICE\sshd" and AFTER the revoking of inherited permissions - no luck.

@manojampalam
Copy link
Contributor

@bingbing8 is following up

@manojampalam
Copy link
Contributor

@DarwinJS we've pulled down 0.0.13.0. We coordinate with you on the work needed before we push it out again.

@bingbing8
Copy link
Contributor

bingbing8 commented May 15, 2017

@DarwinJS, Thanks for your feedback. yes, the line should be $myACL.SetAccessRuleProtection($True, $False). I am not able to repro the exception on win10 yet. When you print out $_.IdentityReference.Value, what type of account it is? Does it exist at HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\ProfileList? Can you try this API $acl.GetAccessRules($true,$true, [System.Security.Principal.NTAccount]) with different parameters https://msdn.microsoft.com/en-us/library/system.security.accesscontrol.commonobjectsecurity.getaccessrules(v=vs.110).aspx and see if there is difference? Also does icacls report those two rules?

@DarwinJS
Copy link
Author

@bingbing8 - thanks for your reply.

The strange user account is happening only on Nano.

Given that the "Domain" is "APPLICATION PACKAGE AUTHORITY" for both accounts, I was guessing that it is some type of virtual account - maybe having to do with PackageManagement in general or Nano Server Packages as a primary method of updating the OS?

I will try the method call you suggest when I get some more time to focus on this.

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

No branches or pull requests

3 participants