PowerShell / Win32-OpenSSH

Win32 port of OpenSSH
7.37k stars 758 forks source link

Errors from permission repair PowerShell utilities. #1880

Open rkitover opened 2 years ago

rkitover commented 2 years ago

"OpenSSH for Windows" version 8.0.0.0

Server OperatingSystem Windows 11 insider preview dev channel

Client OperatingSystem Windows 11 insider preview dev channel

What is failing

./fixuserfilepermissions.ps1

produces the following output:

  [*] ~\.ssh\config

'blotus\None' should not have access to '~\.ssh\config'..
Shall I remove this access?
[Y] Yes  [A] Yes to All  [N] No  [L] No to All  [S] Suspend  [?] Help (default is "Y"): A
Exception calling "RemoveAccessRule" with "1" argument(s): "This access control list is not in canonical form and
therefore cannot be modified."
At C:\tools\cygwin\tmp\powershell-permissions-repair-scripts\OpenSSHUtils.psm1:524 char:20
+                 if(-not ($acl.RemoveAccessRule($ace)))
+                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : InvalidOperationException

Exception calling "RemoveAccessRule" with "1" argument(s): "This access control list is not in canonical form and
therefore cannot be modified."
At C:\tools\cygwin\tmp\powershell-permissions-repair-scripts\OpenSSHUtils.psm1:524 char:20
+                 if(-not ($acl.RemoveAccessRule($ace)))
+                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : InvalidOperationException

      Repaired permissions

  [*] C:\Users\rkitover\.ssh\id_rsa
      looks good

  [*] C:\Users\rkitover\.ssh\id_rsa.pub
      looks good

   Done.
repair-authorizedkeypermission -file ~/.ssh/authorized_keys

produces the following output:

  [*] ~/.ssh/authorized_keys

'NT AUTHORITY\SYSTEM' has the following access to '~/.ssh/authorized_keys': 'Deny'-'ExecuteFile'.
Shall I make it Allow FullControl?
[Y] Yes  [A] Yes to All  [N] No  [L] No to All  [S] Suspend  [?] Help (default is "Y"): A
Exception calling "SetAccessRule" with "1" argument(s): "This access control list is not in canonical form and
therefore cannot be modified."
At C:\tools\cygwin\tmp\powershell-permissions-repair-scripts\OpenSSHUtils.psm1:433 char:17
+                 $acl.SetAccessRule($ace)
+                 ~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : InvalidOperationException

'NT AUTHORITY\SYSTEM' now has FullControl access to '~/.ssh/authorized_keys'.
Exception calling "RemoveAccessRule" with "1" argument(s): "This access control list is not in canonical form and
therefore cannot be modified."
At C:\tools\cygwin\tmp\powershell-permissions-repair-scripts\OpenSSHUtils.psm1:524 char:20
+                 if(-not ($acl.RemoveAccessRule($ace)))
+                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : InvalidOperationException

Exception calling "RemoveAccessRule" with "1" argument(s): "This access control list is not in canonical form and
therefore cannot be modified."
At C:\tools\cygwin\tmp\powershell-permissions-repair-scripts\OpenSSHUtils.psm1:524 char:20
+                 if(-not ($acl.RemoveAccessRule($ace)))
+                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : InvalidOperationException

      Repaired permissions

Expected output

no errors

Actual output

see above

bagajjal commented 2 years ago

@tgauth , please have a look

tgauth commented 2 years ago

@rkitover, can you run icacls <filepath> /verify for the config & authorized_keys files in the .ssh folder? Based on this, it may be a file corruption issue.

rkitover commented 2 years ago

I fixed them manually with icacls. I am not exactly sure who or what the cause was.

According to that thread this fixes the issue:

$acl = Get-Acl $path
Set-Acl $path $acl

do you think it may be worthwhile to add this to the module?

rkitover commented 2 years ago

Actually I just tried that and it does not work in my case.

Here is the output of the script:

  [*] ~\.ssh\config

'blotus\None' should not have access to '~\.ssh\config'..
Shall I remove this access?
[Y] Yes  [A] Yes to All  [N] No  [L] No to All  [S] Suspend  [?] Help (default is "Y"): A
MethodInvocationException: Exception calling "RemoveAccessRule" with "1" argument(s): "This access control list is not in canonical form and
therefore cannot be modified."

MethodInvocationException: Exception calling "RemoveAccessRule" with "1" argument(s): "This access control list is not in canonical form and
therefore cannot be modified."

So I tried running:

get-acl .config | set-acl .config

, but the result from the script is exactly the same.

tgauth commented 2 years ago

Was this still the script output after fixing the files manually with icacls?

My understanding of the thread is that, for a corrupted file, the command is: get-acl <uncorrupted file with similar permissions> | set-acl .config

rkitover commented 2 years ago

Was this still the script output after fixing the files manually with icacls?

No, I did this last time, now this happened again to this file.

Someone or something is screwing up the ACLs on my files.

This particular file is not important to the operation of ssh, but since I saw this error again I thought I'd bring it up.

According to this post in the thread, set-acl will reorder ACLs correctly to fix this corruption. I made a copy of the file to test. Here is what I see:

set-acl test

So it actually does not do that.

Of course if I overwrite the ACLs from another file or edit them manually it will fix the problem.

I think we need to add something to these scripts to handle this situation.

rkitover commented 2 years ago

I made a copy of the ACL for you test, restoring it reproduces the issue.

This is the process I used to save/restore the ACL:

save/restore ACL

This is the contents of config-acl.txt:

config.bak

D:PAI(D;;DTRC;;;S-1-0-0)(A;;FA;;;S-1-5-21-3866810369-3403323307-2869509466-1001)(D;;WP;;;SY)(D;;WP;;;BA)(A;;0x120080;;;S-1-5-21-3866810369-3403323307-2869509466-513)(A;;0x1201ff;;;SY)(A;;0x1201ff;;;BA)(A;;0x120080;;;WD)S:PAINO_ACCESS_CONTROL
tgauth commented 2 years ago

Thanks for the example - I am able to repro by replacing the two local user SIDs with my own.

As far as incorporating a fix into the scripts, I don't know of a good way to use icacls. That leads me to implementing a try-catch for this error and replacing the non-canonical ACL with some pre-defined, standard ACL, with the user's confirmation. In the case of a user's config file, the ACL would be full access for the system/admin & user. I'm open to other ideas on implementation.

rkitover commented 2 years ago

Sounds good to me. In most cases these ACLs would look about the same anyway (based on the user) right?

ejouellette commented 2 years ago

I fixed them manually with icacls. I am not exactly sure who or what the cause was.

I think I may have an answer, or at least some helpful info.

At first I thought the out of order ACL list on config.bak file may have been caused by copying the config file from ~/.ssh/config to ~/.ssh/config.bak in a Cygwin or Msys2 (mounted path with acl on) environment.

For a test, I started out with a ~.ssh\config file which was generated in Windows. running "icacls ~.ssh\config /verify" in PowerShell is totally successful (Successfully processed 1 files; Failed processing 0 files).

Next, I copied my ~.ssh\config to ~.ssh\config.bak in Windows Explorer (or PowerShell). Running "icacls ~.ssh\config.bak /verify" in PowerShell is also successful.

Then, I deleted ~/.ssh/config.bak, and copied ~/.ssh/config to ~/.ssh/config.bak in Msys2 (on a mounted directory with acl on) or in Cygwin bash shell prompt. Running "icacls ~.ssh\config.bak /verify" in PowerShell is also successful.

Next, I created a new file in Msys2 or Cygwin bash shell prompt: "touch foo" Running "icacls .\foo /verify" in PowerShell gives the error: .\foo: Ace entries not in canonical order. Successfully processed 0 files; Failed processing 0 files

Running "icacls .\foo" results in: .\foo NULL SID:(DENY)(Rc,S,X,DC) ENVY-TS17\techie:(R,W,D,WDAC,WO) ENVY-TS17\None:(DENY)(S,X) NT AUTHORITY\SYSTEM:(DENY)(S,X) BUILTIN\Administrators:(DENY)(S,X) BUILTIN\Users:(DENY)(S,X) ENVY-TS17\None:(RX) NT AUTHORITY\SYSTEM:(RX,W) BUILTIN\Administrators:(RX,W) BUILTIN\Users:(RX) Everyone:(R)

So, there seems to be an issue with Msys2 and Cygwin ACL file permissions on newly created files. BTW I'm running the latest Msys2 version as of today (3/28/2022). Hope this helps.

rkitover commented 2 years ago

@2Torr This is very interesting, thank you.

I do use MSYS2 and Cygwin occasionally, but the default mounts are noacl and I never changed them to acl mounts, and I do not see the issue you describe.

Regardless, we should follow up on this with the Cygwin project (which is the upstream for MSYS2.)

rkitover commented 2 years ago

@2Torr I just verified on another computer that you pinpointed the source of the issue exactly.

I maintain this script:

https://github.com/rkitover/windows-alt-sshd-msys2

. And in this script an operation modifies ~/.ssh/config which can be the Windows one with db_home: windows in /etc/nssswitch.conf.

In this script, it does a touch, chmod 600 and does a cat >>config to append blocks to it, and all of this seems to trigger the ACL screwing up that you noticed in ACL mounts even though my mounts are noacl.

I will fix my script but I will not promise to follow up on the issue in the Cygwin runtime if it's still there, but will try to if possible.

rkitover commented 2 years ago

@tgauth I am wondering, has your fix been committed? Now I have a way to reproduce the corruption, and I am still getting an error on OpenSSH_for_Windows_8.6p1 which is the --prerelease version from Chocolatey.

In any case, the URL to fetch the script I'm using in my script is:

https://raw.githubusercontent.com/PowerShell/openssh-portable/latestw_all/contrib/win32/openssh/FixUserFilePermissions.ps1

The error I get now is different however, now I get:

  [*] ~\.ssh\config

'Все' should not have access to '~\.ssh\config'..
Shall I remove this access?
[Y] Yes  [A] Yes to All  [N] No  [L] No to All  [S] Suspend  [?] Help (default is "Y"): A
MethodInvocationException: Exception calling "RemoveAccessRule" with "1" argument(s): "This access control list is not in canonical form and therefore cannot be modified."

      Repaired permissions

  [*] C:\Users\rkitover\.ssh\id_rsa
      looks good

  [*] C:\Users\rkitover\.ssh\id_rsa.pub
      looks good

   Done.

, the difference seems to be that the error appears only once.

tgauth commented 2 years ago

@rkitover, it has not.

There were some adjacent changes to OpenSSHUtils.psm1 that might be causing the modification to the error, but I'd have to look into that a little closer to be sure.

rkitover commented 2 years ago

@2Torr This seems even weirder now, it looks like the ACLs are wrong on my ~/.ssh directory, and even PowerShell creates new files with the wrong ACLs. My screenshot program is broken, so I'm pasting a console log:

v PWSH{W} ~\.ssh
rkitover@epycwin > icacls config
config NT AUTHORITY\СИСТЕМА:(F)
       BUILTIN\Администраторы:(F)
       EPYCWIN\rkitover:(F)

Successfully processed 1 files; Failed processing 0 files
v PWSH{W} ~\.ssh
rkitover@epycwin > cpi config config.bak
v PWSH{W} ~\.ssh
rkitover@epycwin > icacls config.bak
config.bak NULL SID:(DENY)(Rc,DC)
           BUILTIN\Администраторы:(F)
           NT AUTHORITY\СИСТЕМА:(DENY)(X)
           BUILTIN\Администраторы:(DENY)(X)
           EPYCWIN\rkitover:(Rc,S,RA)
           NT AUTHORITY\СИСТЕМА:(RX,W,DC)
           BUILTIN\Администраторы:(RX,W,DC)
           Все:(Rc,S,RA)

Successfully processed 1 files; Failed processing 0 files
v PWSH{W} ~\.ssh
rkitover@epycwin > icacls config.bak /verify
config.bak: Ace entries not in canonical order.
Successfully processed 0 files; Failed processing 0 files
v PWSH{W} ~\.ssh
rkitover@epycwin > icacls .
. NULL SID:(DENY)(Rc,DC)
  EPYCWIN\rkitover:(F)
  NT AUTHORITY\СИСТЕМА:(DENY)(X)
  BUILTIN\Администраторы:(DENY)(X)
  EPYCWIN\rkitover:(Rc,S,RA)
  NT AUTHORITY\СИСТЕМА:(RX,W,DC)
  BUILTIN\Администраторы:(RX,W,DC)
  Все:(Rc,S,RA)
  NULL SID:(OI)(CI)(IO)(DENY)(Rc,DC)
  СОЗДАТЕЛЬ-ВЛАДЕЛЕЦ:(OI)(CI)(IO)(F)
  NT AUTHORITY\СИСТЕМА:(OI)(CI)(IO)(DENY)(X)
  BUILTIN\Администраторы:(OI)(CI)(IO)(DENY)(X)
  ГРУППА-СОЗДАТЕЛЬ:(OI)(CI)(IO)(Rc,S,RA)
  NT AUTHORITY\СИСТЕМА:(OI)(CI)(IO)(RX,W,DC)
  BUILTIN\Администраторы:(OI)(CI)(IO)(RX,W,DC)
  Все:(OI)(CI)(IO)(Rc,S,RA)

Successfully processed 1 files; Failed processing 0 files
v PWSH{W} ~\.ssh
rkitover@epycwin >
rkitover commented 2 years ago

@2Torr Once I did get-acl id_rsa.pub | set-acl . all my problems went away and my script works correctly as well.

ejouellette commented 2 years ago

@2Torr I just verified on another computer that you pinpointed the source of the issue exactly.

I maintain this script:

https://github.com/rkitover/windows-alt-sshd-msys2

. And in this script an operation modifies ~/.ssh/config which can be the Windows one with db_home: windows in /etc/nssswitch.conf.

In this script, it does a touch, chmod 600 and does a cat >>config to append blocks to it, and all of this seems to trigger the ACL screwing up that you noticed in ACL mounts even though my mounts are noacl.

I will fix my script but I will not promise to follow up on the issue in the Cygwin runtime if it's still there, but will try to if possible.

@rkitover Sorry for my delayed reply due to tax season. Thank you very much for looking into this issue. Yes, I've used your "windows-alt-sshd-msys2" script before and think it's awesome!

@2Torr This seems even weirder now, it looks like the ACLs are wrong on my ~/.ssh directory, and even PowerShell creates new files with the wrong ACLs. My screenshot program is broken, so I'm pasting a console log.

If your ~/.ssh directory was created / touched by MSYS2 or Cygwin, then its ACL list would no longer be in canonical form. If permission inheritance is enabled for all child objects of your ~/.ssh directory, then even PowerShell would create files in that folder with a non-canonical ACL list..

@2Torr Once I did get-acl id_rsa.pub | set-acl . all my problems went away and my script works correctly as well.

Cool! Glad you found a work-around for the issue. Maybe at some point in the future, people can look into possibly improving the default ACL used by MSYS2 and Cygwin? I read thru the docs for Cygwin, and think there may be a way to provide a better default ACL list.

rkitover commented 2 years ago

Cool! Glad you found a work-around for the issue.

I'm not sure I have yet, I have to check if my script triggers this condition somehow in one of its invocations and see if I can find a work-around. I might just rewrite that part of the script or the whole script in powershell.

Maybe at some point in the future, people can look into possibly improving the default ACL used by MSYS2 and Cygwin? I read thru the docs for Cygwin, and think there may be a way to provide a better default ACL list.

Yeah we should definitely follow up on this. I will post back here if I find the exact behavior that causes this in my case.