3
\$\begingroup\$

In my program I need to impersonate users on the domain network to perform various tasks. Obviously I can only be one user at a time, and there are multiple threads that would like to be either the same or a different user.

I thought it would be neat to generalize the solution, so if you know of a better way to impersonate multiple users that's great, but it's not what I'm looking for in this question. (Note, impersonation is not per-process, so you don't need this for impersonation and I'm not using it anymore)

Can anyone think of a cleaner way of doing it? I really like that I can wrap all accesses in a using block, but I don't like that the underlying class has to implement IPretendDisposable and structure it's Dispose method with an if statement like that.

public interface IPretendDispose : IDisposable
{
    Func<bool> DisposeHandler { get; set; }
}

public abstract class SharedDisposable<TSharedObject, TKey> where TSharedObject : IPretendDispose
{
    protected abstract TSharedObject Create(TKey key);

    private TSharedObject _currentSharedObject;
    private TKey _currentKey;
    private readonly ReaderWriterLockSlim _domainLockSlim = new ReaderWriterLockSlim();

    public TSharedObject GetObject(TKey key)
    {
        var earlyReturn = false;
        _domainLockSlim.EnterReadLock();
        try
        {
            if (key.Equals(_currentKey))
            {
                earlyReturn = true;
                return _currentSharedObject;
            }
        }
        finally
        {
            if (!earlyReturn)
            {
                _domainLockSlim.ExitReadLock();
            }
        }

        //We use this flag otherwise we won't have exited the write lock before we enter the read lock.
        //It is ok to enter the read lock while we are still in the upgradeablereadlock. That's how you downgrade.
        var success = false;
        _domainLockSlim.EnterUpgradeableReadLock();
        try
        {
            //We've waited for our chance to change the instance.
            //First check if another waiting thread made the change for us.
            //Like double-checked locking
            if (key.Equals(_currentKey))
            {
                success = true;
                return _currentSharedObject;
            }

            _domainLockSlim.EnterWriteLock();
            try
            {
                if (_currentSharedObject != null)
                {
                    _currentSharedObject.DisposeHandler = () => true;
                    _currentSharedObject.Dispose();
                }

                _currentKey = key;
                _currentSharedObject = Create(key);
                _currentSharedObject.DisposeHandler = () =>
                {
                    _domainLockSlim.ExitReadLock();
                    return false;
                };

                success = true;
                return _currentSharedObject;
            }
            finally
            {
                //The spot that needs to execute before 
                _domainLockSlim.ExitWriteLock();
            }
        }
        finally
        {
            if (success)
            {
                _domainLockSlim.EnterReadLock();
            }

            _domainLockSlim.ExitUpgradeableReadLock();
        }
    }
}

public class DomainImpersonator : IPretendDispose
{
    private readonly WindowsImpersonationContext _impersonationContext;
    public DomainImpersonator(string userName, string domainName, string password)
    {
        var token = IntPtr.Zero;
        var tokenDuplicate = IntPtr.Zero;

        try
        {
            if (RevertToSelf()
                && LogonUser(userName, domainName, password, Logon32LogonInteractive, Logon32ProviderDefault, ref token) != 0
                && DuplicateToken(token, 2, ref tokenDuplicate) != 0)
            {
                var tempWindowsIdentity = new WindowsIdentity(tokenDuplicate);
                _impersonationContext = tempWindowsIdentity.Impersonate();
            }
            else
            {
                throw new Win32Exception(Marshal.GetLastWin32Error());
            }
        }
        finally
        {
            if (token != IntPtr.Zero)
            {
                CloseHandle(token);
            }

            if (tokenDuplicate != IntPtr.Zero)
            {
                CloseHandle(tokenDuplicate);
            }
        }
    }

    public Func<bool> DisposeHandler { get; set; }
    public void Dispose()
    {
        if (DisposeHandler == null || DisposeHandler())
        {
            _impersonationContext?.Undo();
        }
    }

    [DllImport("advapi32.dll", SetLastError = true)]
    private static extern int LogonUser(
        string lpszUserName,
        string lpszDomain,
        string lpszPassword,
        int dwLogonType,
        int dwLogonProvider,
        ref IntPtr phToken);

    [DllImport("advapi32.dll", CharSet = CharSet.Auto, SetLastError = true)]
    private static extern int DuplicateToken(
        IntPtr hToken,
        int impersonationLevel,
        ref IntPtr hNewToken);

    [DllImport("advapi32.dll", CharSet = CharSet.Auto, SetLastError = true)]
    private static extern bool RevertToSelf();

    [DllImport("kernel32.dll", CharSet = CharSet.Auto)]
    private static extern bool CloseHandle(IntPtr handle);

    private const int Logon32LogonInteractive = 2;
    private const int Logon32ProviderDefault = 0;
}

public class SharedDomainImpersonator : SharedDisposable<DomainImpersonator, Tuple<string, string>>
{
    //Assume this is set safely before-hand
    public readonly Dictionary<Tuple<string, string>, string> DomainImpersonationProfiles = new Dictionary<Tuple<string, string>, string>();
    protected override DomainImpersonator Create(Tuple<string, string> key)
    {
        string password;
        if (!DomainImpersonationProfiles.TryGetValue(key, out password))
        {
            throw new Exception("This won't break the locking mechanism.");
        }

        return new DomainImpersonator(key.Item1, key.Item2, password);
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ I'm a little confused by what you are trying. I think I have a different pattern that is more SRPy, but not sure. You are allow multiple threads with the current user to work but different users need to wait until the threads are done? \$\endgroup\$
    – dmoonfire
    Jan 17, 2018 at 21:05
  • 1
    \$\begingroup\$ Yes, threads that want to impersonate a different user wait (since impersonation is process-wide (I think)). Say I own a single bucket that can contain paint. We can dip our brushes in the bucket but, the paint comes in tubes that we can't dip into. If I'm painting blue, everyone else who wants to paint blue is welcome to join me. But if someone wants to paint red, they have to wait until we are done and they can empty the bucket and refill it with red. \$\endgroup\$ Jan 17, 2018 at 21:13
  • \$\begingroup\$ Can you show a very simple use case? \$\endgroup\$ Jan 18, 2018 at 6:55

2 Answers 2

2
\$\begingroup\$

Using dmoonfire's idea of returning a new object, I created an interface which will contain the value and itself implement the disposal that releases the read-lock. This way you can still do (using var obj = Impersonator.GetObject()) to both get your value and enter the lock, you simply now use obj.Value in your code. This gets rid of IPretendDispose
I've also changed the ordering inside the write-lock to avoid the failure case where _currentKey gets updated but then _currentSharedObject's creation fails.

As an aside, for domain principal impersonation this is entirely unnecessary. Impersonation is on a per-thread basis, so at most you need a [ThreadStatic] variable or to check that you're already impersonating the correct user.

public interface IWrappedDisposable<out T> : IDisposable
{
    T Value { get; }
}

public abstract class SharedDisposable<TSharedObject, TKey> where TSharedObject : IDisposable
{
    private class DisposableWrapper : IWrappedDisposable<TSharedObject>
    {
        private readonly ReaderWriterLockSlim _readLock;
        public TSharedObject Value { get; }

        public DisposableWrapper(TSharedObject value, ReaderWriterLockSlim readLock)
        {
            Value = value;
            _readLock = readLock;
        }

        public void Dispose()
        {
            _readLock.ExitReadLock();
        }
    }

    protected abstract TSharedObject Create(TKey key);

    private TSharedObject _currentSharedObject;
    private TKey _currentKey;
    private readonly ReaderWriterLockSlim _sharedObjectLock = new ReaderWriterLockSlim();

    public IWrappedDisposable<TSharedObject> GetObject(TKey key)
    {
        var earlyReturn = false;
        _sharedObjectLock.EnterReadLock();
        try
        {
            if (key.Equals(_currentKey))
            {
                earlyReturn = true;
                return new DisposableWrapper(_currentSharedObject, _sharedObjectLock);
            }
        }
        finally
        {
            if (!earlyReturn)
            {
                _sharedObjectLock.ExitReadLock();
            }
        }

        //We use this flag otherwise we won't have exited the write lock before we enter the read lock.
        //It is ok to enter the read lock while we are still in the upgradeablereadlock. That's how you downgrade.
        var success = false;
        _sharedObjectLock.EnterUpgradeableReadLock();
        try
        {
            //We've waited for our chance to change the instance.
            //First check if another waiting thread made the change for us.
            //Like double-checked locking
            if (key.Equals(_currentKey))
            {
                success = true;
                return new DisposableWrapper(_currentSharedObject, _sharedObjectLock);
            }

            _sharedObjectLock.EnterWriteLock();
            try
            {
                var oldObject = _currentSharedObject;

                _currentSharedObject = Create(key);
                _currentKey = key;

                oldObject?.Dispose();

                success = true;
                return new DisposableWrapper(_currentSharedObject, _sharedObjectLock);
            }
            finally
            {
                //The spot that needs to execute before the succes check
                _sharedObjectLock.ExitWriteLock();
            }
        }
        finally
        {
            if (success)
            {
                _sharedObjectLock.EnterReadLock();
            }

            _sharedObjectLock.ExitUpgradeableReadLock();
        }
    }
}

public class DomainImpersonator : IDisposable
{
    private readonly WindowsImpersonationContext _impersonationContext;
    public DomainImpersonator(string userName, string domainName, string password)
    {
        var token = IntPtr.Zero;
        var tokenDuplicate = IntPtr.Zero;

        try
        {
            if (RevertToSelf()
                && LogonUser(userName, domainName, password, Logon32LogonInteractive, Logon32ProviderDefault, ref token) != 0
                && DuplicateToken(token, 2, ref tokenDuplicate) != 0)
            {
                var tempWindowsIdentity = new WindowsIdentity(tokenDuplicate);
                _impersonationContext = tempWindowsIdentity.Impersonate();
            }
            else
            {
                throw new Win32Exception(Marshal.GetLastWin32Error());
            }
        }
        finally
        {
            if (token != IntPtr.Zero)
            {
                CloseHandle(token);
            }

            if (tokenDuplicate != IntPtr.Zero)
            {
                CloseHandle(tokenDuplicate);
            }
        }
    }

    public void Dispose()
    {
        _impersonationContext?.Undo();
    }

    [DllImport("advapi32.dll", SetLastError = true)]
    private static extern int LogonUser(
        string lpszUserName,
        string lpszDomain,
        string lpszPassword,
        int dwLogonType,
        int dwLogonProvider,
        ref IntPtr phToken);

    [DllImport("advapi32.dll", CharSet = CharSet.Auto, SetLastError = true)]
    private static extern int DuplicateToken(
        IntPtr hToken,
        int impersonationLevel,
        ref IntPtr hNewToken);

    [DllImport("advapi32.dll", CharSet = CharSet.Auto, SetLastError = true)]
    private static extern bool RevertToSelf();

    [DllImport("kernel32.dll", CharSet = CharSet.Auto)]
    private static extern bool CloseHandle(IntPtr handle);

    private const int Logon32LogonInteractive = 2;
    private const int Logon32ProviderDefault = 0;
}

public class SharedDomainImpersonator : SharedDisposable<DomainImpersonator, Tuple<string, string>>
{
    //Assume this is set safely before-hand
    public readonly Dictionary<Tuple<string, string>, string> DomainImpersonationProfiles = new Dictionary<Tuple<string, string>, string>();
    protected override DomainImpersonator Create(Tuple<string, string> key)
    {
        string password;
        if (!DomainImpersonationProfiles.TryGetValue(key, out password))
        {
            throw new Exception("This won't break the locking mechanism.");
        }

        return new DomainImpersonator(key.Item1, key.Item2, password);
    }
}
\$\endgroup\$
1
\$\begingroup\$

I use a locking pattern that let's me use using with various ReaderWriterLockSlim classes.

ReaderWriterLockSlim locker;

using (new ReadLock(locker)) { }

That idea would let you have a simple IDisposable that only gets the shared lock, but then put the rest of your code inside the using () {} without having other classes be aware of it. (Makes Single Responsibility Principle happy.)

With that pattern, you would create a simple SharedKeyLock:

namespace SharedLock
{
    using System;
    using System.Collections.Generic;
    using System.Threading;
    using System.Threading.Tasks;

    public class Class1
    {
        public static void Main()
        {
            // Set up the shared lock.
            var shared = new SharedKeyLock<string>();
            Console.WriteLine("Set up the string-based shared lock.");

            // Perform the serial test.
            Console.WriteLine("Serial Test:");

            using (shared.Lock("Bob"))
            {
                Console.WriteLine("Serial: Bob 1");
            }

            using (shared.Lock("Bob"))
            {
                Console.WriteLine("Serial: Bob 2");
            }

            using (shared.Lock("Steve"))
            {
                Console.WriteLine("Serial: Steve 1");
            }

            // Multi-threaded.
            var tasks = new List<Task>
            {
                Task.Run(
                    () =>
                    {
                        using (shared.Lock("Bob"))
                        {
                            Console.WriteLine("Threaded: Bob 1 Start");
                            Thread.Sleep(5000);
                            Console.WriteLine("Threaded: Bob 1 Stop");
                        }
                    }),
                Task.Run(
                    () =>
                    {
                        using (shared.Lock("Bob"))
                        {
                            Console.WriteLine("Threaded: Bob 2 Start");
                            Thread.Sleep(500); // Shorter!
                            Console.WriteLine("Threaded: Bob 2 Stop");
                        }
                    }),
                Task.Run(
                    () =>
                    {
                        using (shared.Lock("Steve"))
                        {
                            Console.WriteLine("Threaded: Steve 1 Start");
                            Thread.Sleep(5000);
                            Console.WriteLine("Threaded: Steve 1 Stop");
                        }
                    }),
                Task.Run(
                    () =>
                    {
                        using (shared.Lock("Gary"))
                        {
                            Console.WriteLine("Threaded: Gary 1 Start");
                            Thread.Sleep(5000);
                            Console.WriteLine("Threaded: Gary 1 Stop");
                        }
                    }),
            };

            Task.WaitAll(tasks.ToArray());

            // For debugging.
            Console.Write("Press return to continue> ");
            Console.ReadLine();
        }
    }

    /// <summary>
    /// Implements the manager for a shared key where multiple threads with the same key are allowed
    /// to run at the same time but different keys have to wait.
    /// </summary>
    /// <typeparam name="TKey">The type of the key.</typeparam>
    public class SharedKeyLock<TKey>
        where TKey : IEquatable<TKey>
    {
        /// <summary>
        /// Controls access when we have to change current or the lock.
        /// </summary>
        private readonly ReaderWriterLockSlim currentLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);

        /// <summary>
        /// Keeps track of how many threads are currently using the key.
        /// </summary>
        private int currentCount;

        /// <summary>
        /// Keeps track of the currently processing key.
        /// </summary>
        private TKey currentKey;

        public IDisposable Lock(TKey key)
        {
            // Do this in an infinite loop. This has the possibility of being a tight loop, so
            // we have a short sleep with the continue.
            while (true)
            {
                // Use a read lock to do a quick test to see if we can continue forward.
                // This is used when the current key is the key being processed.
                using (new ReadLock(this.currentLock))
                {
                    if (key.Equals(this.currentKey))
                    {
                        return new KeyLock<TKey>(this);
                    }
                }

                // Get an upgradable lock to see if we can acquire the lock.
                using (new UpgradableLock(this.currentLock))
                {
                    // Check again to see if we are the current key. Because of locking, this can
                    // happen between the two loops easily.
                    if (key.Equals(this.currentKey))
                    {
                        return new KeyLock<TKey>(this);
                    }

                    // We aren't the current key and we know no one else is using it. For everything
                    // else, we have to upgrade to a write lock.
                    using (new WriteLock(this.currentLock))
                    {
                        // We don't have to retest the current key because nothing else will get
                        // through the above upgradable lock. First we check to see if there is something
                        // else currently holding the shared lock. If there is, try again.
                        if (this.currentCount > 0)
                        {
                            Thread.Sleep(10);
                            continue;
                        }

                        // If we got this far, there is no thread using the current lock which means we can
                        // steal it.
                        this.currentKey = key;

                        // Return an instance to the key.
                        return new KeyLock<TKey>(this);
                    }
                }
            }
        }

        /// <summary>
        /// Represents an instance of a lock with a specific key.
        /// </summary>
        private class KeyLock<TLockKey> : IDisposable
            where TLockKey : IEquatable<TLockKey>
        {
            private readonly SharedKeyLock<TLockKey> sharedLock;

            internal KeyLock(SharedKeyLock<TLockKey> sharedLock)
            {
                this.sharedLock = sharedLock;
                Interlocked.Increment(ref this.sharedLock.currentCount);
            }

            public void Dispose()
            {
                Interlocked.Decrement(ref this.sharedLock.currentCount);
            }
        }
    }

    /// <summary>
    /// Defines a ReaderWriterLockSlim read-only lock.
    /// </summary>
    public class ReadLock : IDisposable
    {
        private readonly ReaderWriterLockSlim readerWriterLockSlim;

        public ReadLock(ReaderWriterLockSlim readerWriterLockSlim)
        {
            this.readerWriterLockSlim = readerWriterLockSlim;
            readerWriterLockSlim.EnterReadLock();
        }

        public void Dispose()
        {
            this.readerWriterLockSlim.ExitReadLock();
        }
    }

    public class UpgradableLock : IDisposable
    {
        private readonly ReaderWriterLockSlim readerWriterLockSlim;

        public UpgradableLock(ReaderWriterLockSlim readerWriterLockSlim)
        {
            this.readerWriterLockSlim = readerWriterLockSlim;
            readerWriterLockSlim.EnterUpgradeableReadLock();
        }

        public void Dispose()
        {
            this.readerWriterLockSlim.ExitUpgradeableReadLock();
        }
    }

    public class WriteLock : IDisposable
    {
        private readonly ReaderWriterLockSlim readerWriterLockSlim;

        public WriteLock(ReaderWriterLockSlim readerWriterLockSlim)
        {
            this.readerWriterLockSlim = readerWriterLockSlim;
            readerWriterLockSlim.EnterWriteLock();
        }

        public void Dispose()
        {
            this.readerWriterLockSlim.ExitWriteLock();
        }
    }
}

The ReadLock, UpgradableLock, and WriteLock are just pulled in from my library. It's MIT, doesn't matter. Basically with the above classes, you can do this:

var shared = new SharedKeyLock<string>();

using (shared.Lock(SomeStringKey)) {
    // Write code knowing it will only run if you have the same key.
}

This ends up being SRP-friendly because the code you are running doesn't have to be aware of its locking status.

\$\endgroup\$
5
  • 2
    \$\begingroup\$ I don't know, this doesn't look right especially with the infinite loop and the sleep :-| \$\endgroup\$
    – t3chb0t
    Jan 18, 2018 at 5:06
  • 1
    \$\begingroup\$ The while (true) statement is because you can have multiple different keys trying to enter that initial loop at time (Bob, Steve, Gary). Once the first one gets through (say Bob), Steve and Gary are still trying to get the lock but only one will (say Steve), that means Gary needs to try again. One approach is to create a max wait time which gives you a break if it doesn't work, but the pattern of ReaderWriterLockSlim is that Enter blocks forever, TryEnter does not. So there could be a safer TryEntry that has a max wait time. \$\endgroup\$
    – dmoonfire
    Jan 18, 2018 at 5:20
  • \$\begingroup\$ The sleep is to prevent a hard-tight loop. Again, if you have a lot of different keys waiting, they have the potential of looping through the read/upgradable loop quickly. I just put a short sleep to make it more reasonable. You could switch to a manual reset slim implementation which would get rid of that but it was hard for an off-the-cuff implementation. :) \$\endgroup\$
    – dmoonfire
    Jan 18, 2018 at 5:21
  • 3
    \$\begingroup\$ To me this feels worse. You've wrapped the lock entrances into using sure, but at the cost of introducing a loop. Then you're returning IDisposable instead TSharedObject. That gets rid of IPretendDispose, but now you've lost the using for TSharedObject, which is kind of the central idea of the implementation. \$\endgroup\$ Jan 18, 2018 at 6:22
  • \$\begingroup\$ Fair enough. You can get rid of the sleep part of the loop using manual reset (instead of sleep, use a wait and then trigger it in the Dispose of the inner class) but the spin lock is a bit harder without jumping through hoops. As for the shared object, I figured if you have a key (say Windows identity), then it would be a separate class to translate that key into whatever you wanted. That way, you have two SRP classes: one to handle the concurrency based on a key, the other to create/manage your shared object from that key knowing it won't have an invalid concurrency. \$\endgroup\$
    – dmoonfire
    Jan 18, 2018 at 7:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.