Skip to content
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

SeAssignSecurity has no idea what the parent ACL is, so can't create a correct ACL in return #705

Open
kyanha opened this issue Jun 24, 2018 · 11 comments

Comments

@kyanha
Copy link
Contributor

kyanha commented Jun 24, 2018

This is a request for a feature that needs to be implemented to get security working 100%.

There is no user-mode component that I've been able to find that performs the work of the kernel routine SeAssignSecurity. This means that there's no user-mode component that takes the parent ACL and the requested (or token-default) ACL, and returns the correct ACL taking inheritance into account.

However, the only use of SeAssignSecurity in the kernel driver shows up in create.c, and it doesn't provide the parent's ACL to the function. Its comment says, "we don't keep track of parents, this will have to be handled in user mode."

What I'm given to understand is that this attempts to punt the responsibility from kernel mode (where the only facility necessary to fulfill the responsibility is) to user mode (where there is nothing that can even emulate the facility necessary to fulfill that responsibility). I can understand why the driver doesn't track parents, but that means that the only system-provided library routine to handle the problem is unavailable.

As a result of this, there are four potential paths forward:

  1. The driver can start tracking parents' ACLs,
  2. The library can define another callout to userspace for "get ACL for the parent of this name" (without necessarily having a handle open to said parent -- be it a call to GetFileSecurity or another routine),
  3. The driver can define another IOCTL for the user mode to submit the parent ACL and default/requested ACL to obtain the correct ACL (which may require the user to pass a pointer to a valid page that the kernel routine then writes to, with a pointer-to-int that contains either the size of the allocation on input or the minimum size to hold the entire returned ACL on output), or
  4. Someone can create, debug, and maintain a user-mode ACL inheritance routine.

I'm not sure how long it would take to do the first one. And unfortunately, I have absolutely -zero- faith in my ability to code correctly in kernelspace, so I cannot volunteer to make it happen. The second is frustrating, in that it defines an additional routine for the user to implement. I don't like the idea of overloading GetFileSecurity with the semantic of "oh yeah, and this can be called without having an open handle to it (so you can't guarantee that you're going to have a user context set on it, nor will you get a Cleanup or Close on it)". The third option is probably the least frustrating, but again I don't know how to code correctly in the kernel, so I can't write the PR -- at least, not without a huge amount of coaching. The fourth option will take a long while and a huge amount of frustration to get right.

@kyanha
Copy link
Contributor Author

kyanha commented Jun 24, 2018

This causes a bad interaction with my suggestion on dokan-dev/dokan-dotnet#139 , namely that said suggestion won't work because of it.

@amarcionek
Copy link

@kyanha, I've run into a similar problem as you in the past with our IFS driver. In fact, you can see I tried to find a user mode equivalent to SeAssignSecurity some 10 years ago!

For what its worth, I can comment on how we solved this. We essentially did your option 2, although not exactly as you state. When a CREATE request comes in, we call into user-mode using essentially a stat call on what would be the entry itself. If it exists, we do nothing. If it doesn't exist, then the response to the 'stat' call contains a security descriptor (SD) taken from some parent of what will be that newly created file and a boolean indicating that a new SD needs to be formed. The kernel then looks at the boolean, if true calls SeAssignSecurity using the parent and incoming SD, and uses the resulting SD with the call to create the file in user mode. Its been sufficient, although probably not extremely efficient.

I've considered your option 3 there but haven't implemented it. It would be nice to have an IOCTL that user mode can use to send an SD and a parent SD and get a resulting SD back. We have use cases where files can come into our file system through other methods (e.g. scanning some other share) that don't go through the kernel. I think the only tricky part would be the SubjectSecurityContext For requests through the driver, you'd have to pass that up into the user mode and then pass it back down, but that wouldn't work elsewhere. Or, you could up with some default context, that the kernel always holds on to. But I'm not an expert in this area to know what is 'allowed.'

@kyanha
Copy link
Contributor Author

kyanha commented Aug 17, 2018

@amarcionek, the SubjectSecurityContext is kind of available to the user code, via the function DokanOpenRequestorToken(). (In Dokan.Net, it's available via DokanFileInfo.GetRequestor().) I'm not sure how the kernel could obtain the SSC from the token, though -- that needs a bit more investigation. Maybe the best approximation for anything that doesn't return a security context would be to return the SYSTEM security context, but I don't know.

If you need to figure out the security to apply based on scanning some other share, I think it might be best to construct a context based on the owner and group of the file that you're bringing in, then pass that to SeAssignSecurity?

Do you happen to have the results of NTFS (as opposed to the filesystems you've implemented) related to different things in security contexts on IRP_MJ_CREATE available? That would likely help determine the best way to proceed.

@kyanha
Copy link
Contributor Author

kyanha commented Oct 1, 2018

@amarcionek Okay, I've got a better idea of what's going on in kernel-land now.

The same mechanism that makes the SubjectSecurityContext available to one IOCTL would make it available to every IOCTL; as a result, there's no need to pass the context up to user mode and then back to kernel mode. Unfortunately, this reduces the applicability of being able to simply send an IOCTL down and get a proper response if it's not coming from a context through the kernel. I'm considering a second IOCTL that doesn't require a handle to file opened through the share, that allocates and populates a new SECURITY_CONTEXT structure [something that the kernel provides no mechanism for doing] just to make SeAssignSecurity work for arbitrary contexts.

@amarcionek
Copy link

I think that's fair. MSFT says that file system developers don't need to care what's in a security descriptor, yet in this case, it sort of matters, if you don't have an owner group and need to use the token. The key lines in the definition of SeAssignSecurity come from the comments section where it says in the absence of an owner/group, the caller's token is used. I have never seen a security descriptor at that point in the code NOT have an owner. But in the absence of one, if I had to choose an account for that owner, I'd say the local administrators group would be appropriate.

@kyanha
Copy link
Contributor Author

kyanha commented Oct 2, 2018

File system developers don't need to care what's in a security descriptor if and only if they're not operating outside of the preconceived notions that Microsoft has written its design and documentation around. As soon as Dokany pushed filesystem implementation into user mode, that constraint flew out the window.

I've got an accepted/triaged/assigned bug over at https://github.com/MicrosoftDocs/windows-driver-docs-ddi/issues/258 to get documentation on RtlSetSaclSecurityDescriptor, so that SACLs can be properly inherited as well.

I'm intending to have a place to provide an owner/group in the secondary IOCTL's passed-in structure, so that assumptions need not be made by the driver and instead can be foisted off on the filesystem developer. I agree that Administrators would be reasonable in a lot of circumstances, but the issue is more that Dokany exists because kernel-mode filesystem development is also reasonable in a lot of circumstances. But only "a lot of", not "all". :)

@kyanha
Copy link
Contributor Author

kyanha commented Oct 8, 2019

My bug over at MicrosoftDocs/windows-driver-docs-ddi#258 was decided not to be documented. This is... annoying. But I might be able to do something to inherit SACLs anyway, not quite sure yet.

@DDoSolitary
Copy link
Member

FWIW, I managed to get security working using only user space code, when trying to write a memfs using Dokan.

What I did:

  • Manually create a security descriptor for the root directory.
  • Keep track of parent in user space.
  • Use CreatePrivateObjectSecurity to handle inheritance. Just feed it: 1) parent security descriptor 2) creator's security descriptor from DOKAN_IO_SECURITY_CONTEXT.AccessState.SecurityDescriptor 3) user token from DokanOpenRequestorToken

Code: https://github.com/dokan-dev/dokan-rust/tree/master/dokan/examples/memfs

It is written in Rust, but should be easy to understand.

@kyanha
Copy link
Contributor Author

kyanha commented Jul 13, 2020

I didn't even see CreatePrivateObjectSecurity as an available API. x.x This is the user-mode ACL creator/manipulator that I was looking for, to do the work of SeAssignSecurity. Thank you so much for finding it, @DDoSolitary!

(Documentation of this API should probably be put into the various integration libraries, to avoid anyone else wasting time looking in the wrong areas for something Microsoft has already created.)

@kyanha
Copy link
Contributor Author

kyanha commented Jul 17, 2020

Hey @DDoSolitary , how much of the work that you did in Rust is related to using CreatePrivateObjectSecurity and not CreatePrivateObjectSecurityEx? The Ex version has flags to automatically implement inheritance on directories.

@DDoSolitary
Copy link
Member

DDoSolitary commented Jul 18, 2020

@kyanha Actually, I used the Ex version at the beginning. However, after some experiments, I found that the auto inherit flags don't matter in this case. CreatePrivateObjectSecurity will inherit from parent's access control entries, and your SetFileSecurity function will be called whenever these inherited entries need update, no matter these flags are set or not.

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