Skip to content

2.0 beta winfstest GetFileSecurity/SetFileSecurity failures #607

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
bailey27 opened this issue Oct 22, 2017 · 4 comments
Closed

2.0 beta winfstest GetFileSecurity/SetFileSecurity failures #607

bailey27 opened this issue Oct 22, 2017 · 4 comments

Comments

@bailey27
Copy link
Contributor

bailey27 commented Oct 22, 2017

I was testing the 2.0 beta, and I noticed that even when I run mirror as administrator that when I run winfstest (from either an admin cmd prompt or as a normal user), I get winfstest failures similar to what I would expect in Dokany 1.x if I were not running mirror as administrator.

I was using an old version of winfstest. I tried the latest from dimov-cz/winfstest, but I couldn't get it to work.

So I got @Liryna 's fork of winfstest and built it, and it works.

When I run it as I described above, I see


C:\git\winfstest\TestSuite>run-winfstest.bat . m:\
C:\git\winfstest\TestSuite\\.\t\base\00.t  ok
C:\git\winfstest\TestSuite\\.\t\base\01.t  ok
C:\git\winfstest\TestSuite\\.\t\base\02.t  ok
C:\git\winfstest\TestSuite\\.\t\base\03.t  ok
C:\git\winfstest\TestSuite\\.\t\base\04.t  ok
C:\git\winfstest\TestSuite\\.\t\base\05.t  ok
C:\git\winfstest\TestSuite\\.\t\base\06.t  ok
C:\git\winfstest\TestSuite\\.\t\base\07.t  ok
C:\git\winfstest\TestSuite\\.\t\base\08.t  ok
C:\git\winfstest\TestSuite\\.\t\base\09.t  ok
C:\git\winfstest\TestSuite\\.\t\base\10.t
not ok 2 - expect "GetFileSecurity m:\3e16bcea DACL_SECURITY_INFORMATION" <lambda> - result [{'Sddl': 'D:AI(A;ID;FA;;;BA)(A;ID;FA;;;SY)(A;ID;0x1200a9;;;BU)(A;ID;0x1301bf;;;AU)'}]
not ok 10 - expect "DeleteFile m:\3e16bcea" 0 - got ERROR_ACCESS_DENIED
not ok 11 - expect "CreateDirectory m:\3e16bcea D:P(A;;GA;;;WD)" 0 - got ERROR_ALREADY_EXISTS
not ok 12 - expect "GetFileSecurity m:\3e16bcea DACL_SECURITY_INFORMATION" 0 - got ERROR_ACCESS_DENIED
not ok 13 - expect "SetFileSecurity m:\3e16bcea DACL_SECURITY_INFORMATION D:P(A;;GA;;;WD)(A;;GR;;;SY)" 0 - got ERROR_ACCESS_DENIED
not ok 14 - expect "GetFileSecurity m:\3e16bcea DACL_SECURITY_INFORMATION" 0 - got ERROR_ACCESS_DENIED
not ok 15 - expect "RemoveDirectory m:\3e16bcea" 0 - got ERROR_DIRECTORY
not ok 7/15
C:\git\winfstest\TestSuite\\.\t\streams\00.t  ok
C:\git\winfstest\TestSuite\\.\t\streams\01.t  ok
C:\git\winfstest\TestSuite\\.\t\streams\02.t  ok

total ................................. ok 493/500 - not ok 7/500

Is this a known issue? I looked through the open issues and didn't see anything.

@Liryna
Copy link
Member

Liryna commented Oct 22, 2017

Hi @bailey27 ,

Yes, this is a known issue #307 (comment)
It can be fixed easily but another issue related is pending to find an answer (look at the parent folder to see if we have rights to remove file if access denied is returned).

If you want to see the pending list of tasks for the 2.0, I try to keep it updated here: #307 (comment)

@Liryna Liryna added this to the 2.0.0 milestone Nov 19, 2017
@Liryna Liryna added the Bug label Nov 19, 2017
@Liryna
Copy link
Member

Liryna commented Dec 31, 2017

Hi @bailey27 ,

I have committed the fix that I was talking in my previous comment. 2d68b50
there is still an error when deleting the file that is read only.

This case is missing currently.
Before Dokany library was creating a parent open to see if it gives us rights to delete children https://github.com/dokan-dev/dokany/blob/master/dokan/create.c#L272
For the 2.x.x, it is probably better to handle the check directly in the mirror.

@Corillian Do you remember the purpose of MirrorCreateNewSecurity https://github.com/dokan-dev/dokany/blob/Corillian-asyncio/samples/dokan_mirror/mirror.c#L832 ?
It seems to get the parent file security directory for the new file. Did you also plan to use it to check the parents' rights to delete ?

I tried to make this code work without success even with AddSeSecurityNamePrivilege

  PSECURITY_DESCRIPTOR *ParentSecurity = NULL;
  DWORD test = GetNamedSecurityInfo(
    L"C:\\test", SE_FILE_OBJECT,
    BACKUP_SECURITY_INFORMATION, // give us everything
    NULL, NULL, NULL, NULL, ParentSecurity);

Since MirrorGetParentSecurity is always failing for me https://github.com/dokan-dev/dokany/blob/Corillian-asyncio/samples/dokan_mirror/mirror.c#L569

@Corillian
Copy link

@Liryna The parent directory descriptor is required to create a child descriptor, presumably because the child descriptor inherits the parent's properties except where explicitly overridden: https://msdn.microsoft.com/en-us/library/windows/desktop/aa376405(v=vs.85).aspx

@Liryna
Copy link
Member

Liryna commented Jan 3, 2022

No longer the case with the new 2.0.0

@Liryna Liryna closed this as completed Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants