There's no way to tell what security new files should be created with

[AnyOldName3]

In my VFS, I've got what seems to be a better implementation of GetFileSecurity and SetFileSecurity than the Mirror sample includes, meaning that I get closer results to what winfstest expects in the 10.t (security and permissions) test set. However, it attempts to create new files with specific DACLs, and this information is not relayed to me via Dokan.NET, so I'm limited in what I can do to resolve this.

If a FileSystemSecurity object was passed to CreateFile, then I could get this set of tests to pass (and provide the necessary changes to the mirror sample).

[Liryna]

In my VFS, I've got what seems to be a better implementation of GetFileSecurity and SetFileSecurity than the Mirror sample includes

Ah congratulation ! If you could improve also the mirror and make a PR, it would be very appreciate ❤️

It is true that currently this is not passed by c# createfile 😮 ! https://github.com/dokan-dev/dokan-dotnet/blob/master/DokanNet/DokanOperationProxy.cs#L272

The param is type of PDOKAN_IO_SECURITY_CONTEXT and would need to be converted in c#
https://github.com/dokan-dev/dokany/blob/de4e1408c498b069ada734d0ca9b5c2de501d14d/sys/public.h#L164 and SecurityContext->AccessState.SecurityDescriptor (https://github.com/dokan-dev/dokany/blob/0fb2ddd0a139b8834d9648b0c0cfa13a8216e982/samples/dokan_mirror/mirror.c#L201) convert to FileSecurity for being used during FileStream

Haven't look yet if this can be made or not. Do you think you can look into it ?

[AnyOldName3]

I'm currently busy with being behind on a final-year university project (the thing I'm making my VFS for), so much as I'd like to, I don't have a huge amount of time to go investigating a perfect solution to all the interesting issues of open-source projects I find, unfortunately. It might go on my long list of things to do once I graduate.

Additionally, I'm having issues unsetting the SE_DACL_AUTO_INHERITED security descriptor control flag from within C#. It lets me change it for FileSystemSecurity objects, but when I write them to disk and read them back, the flag hasn't changed. I'm getting the impression that this is because this flag can only be modified by a separate function in the raw Win32 API, and that function isn't exposed to C#.

I'm also not sure how big an issue this actually is. I think the effective access I'm getting is the same, but it's enough to upset winfstest, and enough for me to feel it's not quite ready for a PR.

Maybe it would be a good idea for Dokan.NET to provide helper functions which wrap behaviour which VFSes are likely to need, but can't get in C#.

[Liryna]

@AnyOldName3 Helper sounds to be a good idea
@magol do you have an idea how FileSecurity can be set when FileStream is used ?

[magol]

@Liryna, @AnyOldName3
I know nothing about file security, but I'll try to to learn and see what I can do.

[Liryna]

@magol will try to help you on this if you have any questions or if find some time 😢

[magol]

@Liryna I havn't test it, but can FileStream.SetAccessControl be used for setting FileSecurity in FileStream?

The question is how we get the FileSecurity to CreateFile. I guess we have to change the API in IDokanOperations.

[Liryna]

Yes, exact @magol ! There is also directly the constructor that has the param: https://msdn.microsoft.com/en-us/library/ms143397(v=vs.110).aspx

Yes we will need to change the IDokanOperations as you said.

There is already SetFileSecurityProxy that make the conversion. We should probably just move it to a helper and use it in ZwCreateFileProxy to create the proper FileSystemSecurity 👍
https://github.com/dokan-dev/dokan-dotnet/blob/master/DokanNet/DokanOperationProxy.cs#L1097-L1124

[magol]

If we change the IDokanOperations, it is a breaking change, and that should only be done in a major release. One solution is maybe to make a temporary interface containing this changes. In that way, the users can select to continue to use IDokanOperations as it is, or start to use the new interface if they want to use the new functionality. And when we release a new mayor version, we remove the temporary interface and update IDokanOperations.

I have looked at SetFileSecurityProxy and how it create a FileSystemSecurity. I have following questions:

• SetFileSecurityProxy uses rawSecurityDescriptorLength to copy the right number of bytes, but ZwCreateFileProxy do not have this information. How do I know the size?
• Where should the conversion be done?
  • If we do it in ZwCreateFileProxy we have to make a decision if it should use DirectorySecurity or FileSecurity.
  • If we let the user to make the conversion in IDokanOperations.CreateFile, we have to create a function in DokanHelper to help the user to convert. But then we have to use IntPtr for the parameter to IDokanOperations.CreateFile, and I don't like to send a raw pointer to the user.
  • A variation of the previous option is to encapsulate the IntPtr in a struct and give that to IDokanOperations.CreateFile. In that struct, it can be functions to convert to DirectorySecurity or FileSecurity.

[magol]

@Liryna
I'm a noob when it comes to PInvoke and file security, so my attempts to implementation it is probably pretty naive.
Unfortunately it does not work, it crashes on Marshal.PtrToStructure.

Can you please take a look and see if you can see what is wrong.
Please also tell me what you think of my proposed implementation.

[Liryna]

Hi @magol ,

Thank you for looking into it 👍

I have made some changes on your structs to make marshal work:

 [StructLayout(LayoutKind.Sequential, Pack = 8)]
    public struct DOKAN_ACCESS_STATE
    {
        public byte SecurityEvaluated;
        public byte GenerateAudit;
        public byte GenerateOnClose;
        public byte AuditPrivileges;
        public uint Flags;
        public uint RemainingDesiredAccess;
        public uint PreviouslyGrantedAccess;
        public uint OriginalDesiredAccess;
        public IntPtr SecurityDescriptor;
        public UNICODE_STRING ObjectName;
        public UNICODE_STRING ObjectType;
    }

UNICODE_STRING:

    /// <summary>
    /// http://www.pinvoke.net/default.aspx/Structures/UNICODE_STRING.html
    /// </summary>
    [StructLayout(LayoutKind.Sequential)]
    public struct UNICODE_STRING : IDisposable
    {
        public ushort Length;
        public ushort MaximumLength;
        private IntPtr buffer;

        public UNICODE_STRING(string s)
        {
            Length = (ushort)(s.Length * 2);
            MaximumLength = (ushort)(Length + 2);
            buffer = Marshal.StringToHGlobalUni(s);
        }

        public void Dispose()
        {
            Marshal.FreeHGlobal(buffer);
            buffer = IntPtr.Zero;
        }

        public override string ToString()
        {
            return Marshal.PtrToStringUni(buffer);
        }
    }

But an error happen during call of DokanFileSystemSecurity constructor when calling GetSecurityDescriptorLength

[magol]

@Liryna Thanks for fixing the struct.
I have not test GetSecurityDescriptorLength as I did not get a correct Security descriptior to test with. I don't even know if it is the right approach.

Is it any other way to get the size?

[Liryna]

@magol your call seems to be alright but during debug I seen that the point was null and the documentation of GetSecurityDescriptorLength say The pointer is assumed to be valid. that why probably there is a crash.

A null check should be added and see if with a normal call GetSecurityDescriptorLength pass

[kyanha]

You could change the API in IDokanOperations, but I think there might be a better way to do it: make a method in DokanFileInfo that returns the correct ACL to create the file with. (This is defined as 'the ACL created by SeAssignSecurity() called from the kernel driver'.) This would go alongside the GetRequestor() method that's already there. I realize that this further separates the C# API from the C/C++ API, but honestly there's a lot more that should be altered if it's really supposed to be using C# idioms.

(My goal with suggesting this is to decrease the number of breaking changes that filesystem creators need to deal with. I've already got one breaking change that I can't actually fix right now, because I use Keybase and they're dragging their heels on updating KeybaseFS to a newer version of Dokan.)

Anyway, do you think the creation of another method (or two) in DokanFileInfo might be an acceptable approach to address this particular issue, until 2.0 comes out?

[kyanha]

@magol @Liryna I have determined that my prior suggestion is unworkable, as the kernel driver does not have the parent/inheritable ACL at the time it needs to call SeAssignSecurity.