dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.06k stars 4.69k forks source link

[API Proposal]: Add properties to WindowsIdentity to get info about process user #81784

Open mconnew opened 1 year ago

mconnew commented 1 year ago

Background and motivation

In CoreWCF, we need to obtain a SecurityIdentifier for the process logon SID to apply permissions to a shared memory object to prevent other processes running under the same user from writing to the shared memory. This requires two steps, first getting the process token, and then getting the Token Information for the TokenGroup info for the process token. The first step can be done with managed API's today, but it's cumbersome and undocumented. I can get a process token handle which is usable for passing to the Win32 GetTokenInformation api by calling WindowsIdentity.GetCurrent(TokenAccessLevels.Query).AccessToken. The problem is if you are running on a thread where impersonation is being used, WindowsIdentity.GetCurrent().AccessToken returns the thread token handle which represents the impersonated user. This can't be used to get the token information needed to retrieve the process logon SID. You have to un-impersonate by passing SafeAccessTokenHandle.InvalidHandle to WindowsIdentity.RunImpersonated:

WindowsIdentity.RunImpersonated(SafeAccessTokenHandle.InvalidHandle, () =>
            {
                var processIdentity = WindowsIdentity.GetCurrent(TokenAccessLevels.Query);
                SafeAccessTokenHandle processToken = processIdentity.AccessToken; 
                // Use the processToken in calls to GetTokenInformation
            }

As I mentioned, passing SafeAccessTokenHandle.InvalidHandle is not documented. This is a side effect of implementation of other methods and properties which require reverting to the process user before calling some win32 api's to get various pieces of information. This is because a WindowsIdentity instance that is being accessed may represent a different user than the current thread identity, so internally it unimpersonates by passing an invalid handle before calling the win32 apis. I had to discover this "feature" by reading the code when I was looking for a way to achieve what I wanted through existing managed apis.

Once I have the WindowsIdentity for the process logon user, I need to get the SID. This is returned by the GetTokenInformation win32 api which is used by WindowsIdeneity.Groups, but it explicitly filters out SIDs with the attribute SE_GROUP_LOGON_ID here.

API Proposal

namespace System.Security.Principal;

public class WindowsIdentity
{
    public static WindowsIdentity GetCurrentProcess();
    public static WindowsIdentity GetCurrentProcess(System.Security.Principal.TokenAccessLevels desiredAccess);
    public System.Security.Principal.SecurityIdentifier? LogonSession { get; }
}

API Usage

// Retrieving the Process Logon SID
SecurityIdentifier processLogonSid = WindowsIdentity.GetCurrentProcess(TokenAccessLevels.Query).LogonSession;
// Use the logonSid as part of a dacl eg for a named pipe
DiscretionaryAcl dacl = new DiscretionaryAcl(false, false, capacity);
int accessRights = UnsafeNativeMethods.GENERIC_WRITE | UnsafeNativeMethods.GENERIC_READ;
dacl.AddAccess(AccessControlType.Allow, processLogonSid, accessRights, InheritanceFlags.None, PropagationFlags.None);

Alternative Designs

Getting the process identity could also be added to the Process class. This might be more generally useful as WindowsIdentity is Windows only, and the Process class I believe works on Linux too. To keep it cross platform, the property could be declared as IIdentity so that on Linux it could return a GenericIdentity which can provide the user name. This is how NegotiateStream handles the RemoteIdentity property in a cross platform way.

namespace System.Diagnostics;

public class Process
{
    public System.Security.Principal.IIdentity? Identity { get; }
}

You would still need the LogonSession property on WindowsIdentity to to get the logon session SID. You would then retrieve the logon side like this:

WindowsIdentity processIdentity = Process.GetCurrentProcess().Identity as WindowsIdentity;
SecurityIdentifier processLogonSid = processIdentity.LogonSession;

This is slightly less capable in one way as you can't specify the TokenAccessLevels, but there is a value of MaximumAllowed which should prevent any exceptions being thrown requesting access beyond what is allowed. In another way, it's more flexible as you can query other processes besides your own.

Risks

No risks as far as I'm aware. All new functionality would be lazily executed so there is no need for any performance regression. No change in behavior for existing apis.

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation In CoreWCF, we need to obtain a SecurityIdentifier for the process logon SID to apply permissions to a shared memory object to prevent other processes running under the same user from writing to the shared memory. This requires two steps, first getting the process token, and then getting the Token Information for the TokenGroup info for the process token. The first step can be done with managed API's today, but it's cumbersome and undocumented. I can get a process token handle which is usable for passing to the [Win32 `GetTokenInformation` api](https://learn.microsoft.com/en-us/windows/win32/api/securitybaseapi/nf-securitybaseapi-gettokeninformation) by calling `WindowsIdentity.GetCurrent(TokenAccessLevels.Query).AccessToken`. The problem is if you are running on a thread where impersonation is being used, `WindowsIdentity.GetCurrent().AccessToken` returns the thread token handle which represents the impersonated user. This can't be used to get the token information needed to retrieve the process logon SID. You have to un-impersonate by passing `SafeAccessTokenHandle.InvalidHandle` to `WindowsIdentity.RunImpersonated`: ```c# WindowsIdentity.RunImpersonated(SafeAccessTokenHandle.InvalidHandle, () => { var processIdentity = WindowsIdentity.GetCurrent(TokenAccessLevels.Query); SafeAccessTokenHandle processToken = processIdentity.AccessToken; // Use the processToken in calls to GetTokenInformation } ``` As I mentioned, passing `SafeAccessTokenHandle.InvalidHandle` is not documented. This is a side effect of implementation of other methods and properties which require reverting to the process user before calling some win32 api's to get various pieces of information. This is because a WindowsIdentity instance that is being accessed may represent a different user than the current thread identity, so internally it unimpersonates by passing an invalid handle before calling the win32 apis. I had to discover this "feature" by reading the code when I was looking for a way to achieve what I wanted through existing managed apis. Once I have the WindowsIdentity for the process logon user, I need to get the SID. This is returned by the `GetTokenInformation` win32 api which is used by `WindowsIdeneity.Groups`, but it explicitly filters out SIDs with the attribute SE_GROUP_LOGON_ID [here](https://github.com/dotnet/runtime/blob/ae70a1c51a334a5e13fa22d500f8a8390052eef5/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs#L657). ### API Proposal ```csharp namespace System.Security.Principal; public class WindowsIdentity { public static WindowsIdentity GetCurrentProcess(); public static WindowsIdentity GetCurrentProcess(System.Security.Principal.TokenAccessLevels desiredAccess); public System.Security.Principal.SecurityIdentifier? LogonSession { get; } } ``` ### API Usage ```csharp // Retrieving the Process Logon SID SecurityIdentifier processLogonSid = WindowsIdentity.GetCurrentProcess(TokenAccessLevels.Query).LogonSession; // Use the logonSid as part of a dacl eg for a named pipe DiscretionaryAcl dacl = new DiscretionaryAcl(false, false, capacity); int accessRights = UnsafeNativeMethods.GENERIC_WRITE | UnsafeNativeMethods.GENERIC_READ; dacl.AddAccess(AccessControlType.Allow, processLogonSid, accessRights, InheritanceFlags.None, PropagationFlags.None); ``` ### Alternative Designs Getting the process identity could also be added to the Process class. This might be more generally useful as WindowsIdentity is Windows only, and the Process class I believe works on Linux too. To keep it cross platform, the property could be declared as IIdentity so that on Linux it could return a GenericIdentity which can provide the user name. This is how NegotiateStream handles the RemoteIdentity property in a cross platform way. ```csharp namespace System.Diagnostics; public class Process { public System.Security.Principal.IIdentity? Identity { get; } } ``` You would still need the `LogonSession` property on `WindowsIdentity` to to get the logon session SID. You would then retrieve the logon side like this: ``` WindowsIdentity processIdentity = Process.GetCurrentProcess().Identity as WindowsIdentity; SecurityIdentifier processLogonSid = processIdentity.LogonSession; ``` This is slightly less capable in one way as you can't specify the `TokenAccessLevels`, but there is a value of `MaximumAllowed` which should prevent any exceptions being thrown requesting access beyond what is allowed. In another way, it's more flexible as you can query other processes besides your own. ### Risks No risks as far as I'm aware. All new functionality would be lazily executed so there is no need for any performance regression. No change in behavior for existing apis.
Author: mconnew
Assignees: -
Labels: `api-suggestion`, `area-System.Security`, `untriaged`
Milestone: -
bartonjs commented 1 year ago

GetCurrentProcess doesn't seem to be an appropriate name, as it's not getting a process.

GetCurrentProcessIdentity, perhaps? Or GetForCurrentProcess()?

mconnew commented 1 year ago

GetCurrentProcess doesn't seem to be an appropriate name, as it's not getting a process.

Naming is hard. I'm not strongly opinionated on names, more on the api shape.

SteveSyfuhs commented 1 year ago

Nit for nit's sake: the Windows app runtime (UWP, store, etc. etc.) gives enlightened apps their own principal identifier that is orthogonal to the user principal that launched the process. That means there's: app identity / process token identity / thread token identity. No one really cares about the first in the general case, and there isn't an NT Token associated to it, but that still changes the meaning of "process identity". Recommend putting this on Process and calling it something like GetProcessLaunchIdentity() or alternatively WindowsIdentity.GetProcessTokenIdentity(). The latter seems more technically correct, but that is certainly less platform agnostic.

LogonSession is also not a thing you want to expose here. Or at least, is technically yet another thing. You always want the token [Primary/User] SID. Sessions are also orthogonally scoped, as are Logon Ids.

As well, passing a desired access level for the process token doesn't really yield you anything useful. You will always have full impersonation rights as that token (levels are really only interesting when you're doing something on behalf of someone you're not). If you want to limit the potential risk of what a caller can do with that object instance by limiting it to a purely identity-level token, I think the better API is to always return as identity-level and then force them to call explicitly Impersonate().

mconnew commented 1 year ago

FYI I have worked with multiple teams within Microsoft who have cared about app identity. The scenario was a WCF NetNamedPipe service running on a machine, and a win32 app container running a worker process (for sandbox purposes) and giving permissions to the win32 app container process to connect to the named pipe. They had to get the app identity SID and add an allow ACL to the named pipe.
WCF in .NET Framework also gets the app identity and adds an allow acl for the scenario where you have a client and server process in the same app container who need to communicate over named pipes.

There are other API's you need to call to get all the information needed in these scenarios so I don't think it's worth solving this case without also adding types/methods to remove the need to make P/Invoke calls to get the other information too. The named pipe path needs to include some container session id as it's isolated in its own namespace. It takes 2 or 3 native calls to get this information.

jeffhandley commented 1 year ago

@SteveSyfuhs With the logic you cited regarding enlightened apps and the different layers of identity, is there risk that the logic for getting the identities will change over time?

@mconnew Do you have cross-platform needs here, or are your scenarios strictly on Windows? I'm thinking about WindowsIdentity vs. Process here, and we have WindowsIdentity set to only support Windows of course.

ghost commented 1 year ago

This issue has been marked needs-author-action and may be missing some important information.

mconnew commented 1 year ago

@jeffhandley, no cross platform needs here for my specific use case. I just need it to do some custom ACL permissions for a shared memory object which is all very win32 specific.

SteveSyfuhs commented 1 year ago

@SteveSyfuhs With the logic you cited regarding enlightened apps and the different layers of identity, is there risk that the logic for getting the identities will change over time?

Change, no. Expand to other scenarios? Certainly, always a possibility.