5

My C# code uses impersonation by calling Win32 functions via P/Invoke

internal class Win32Native
{
    [DllImport("advapi32.dll", SetLastError = true)]
    public static extern int ImpersonateLoggedOnUser(IntPtr token);

    [DllImport("advapi32.dll", SetLastError = true)]
    public static extern int RevertToSelf();
}

try {
    var token = obtainTokenFromLogonUser();
    Win32Native.ImpersonateLoggedOnUser( token );
    throw new Exception(); // this is for simulation
    Win32Native.RevertToSelf()
} catch( Exception e ) {
    LogException( e );
    throw;
}

also I have AppDomain.CurrentDomain.UnhandledException handler installed that also logs all unhandled exception.

I'm sure that the code that logs exceptions works fine both with and without impersonation.

Now the problem is that in the above code it looks like catch is not entered and UnhandledException is also not called. The only trace of the exception is an entry in the Event Viewer.

If I add a finally like this:

try {
    var token = obtainTokenFromLogonUser();
    Win32Native.ImpersonateLoggedOnUser( token );
    try {
       throw new Exception(); // this is for simulation
    } finally {
        Win32Native.RevertToSelf()
    }
} catch( Exception e ) {
    LogException( e );
    throw;
}

then the exception is logged okay both from catch and from UnhandledException handler.

What's happening? Does the thread being impersonated prevent usual exception handling?

Amicable
  • 3,115
  • 3
  • 49
  • 77
sharptooth
  • 167,383
  • 100
  • 513
  • 979
  • if you put a break-point at `LogException`: do you get there? – Marc Gravell Mar 15 '13 at 10:02
  • @MarcGravell: I don't know, there's no debugger where this is reproduced. I realise that there may be some problem with `LogException()` itself, but so far the code it relies on worked fine both impersonated and not impersonated. – sharptooth Mar 15 '13 at 10:04
  • Taking a guess here: since .NET keeps track of the execution context, the exception handling might realize that the context has changed and is preventing the exception handler to execute unless the revert is executed first. Not doing so would lead to an elevation of privileges if the code were to run with the impersonated user... – Roman Gruber Mar 15 '13 at 12:26
  • Out of interest - why are you going the native route rather than using the built in support in `WindowsIdentity`? – Damien_The_Unbeliever Mar 15 '13 at 13:26
  • @Damien_The_Unbeliever: That's the best I happened to find with Google. – sharptooth Mar 15 '13 at 13:29
  • This kind of code will fail to trigger an program crash when it runs on Vista or Win7 in 32-bit mode on a 64-bit operating system. And is activated by an event handler that is triggered by a Windows message. The Load event is a common one. We can't see the context of this code. – Hans Passant Mar 15 '13 at 13:44

2 Answers2

3

Without seeing the code of the LogException, I can't be sure, but it could be that whatever you are doing in there is doing so in the context of the impersonated / loggedon user and that that user does not have user rights to be doing whatever it is you are doing e.g. writing to a file etc and that the code then crashes at that point. The fact that your code works only after you have "Reverted to Self" seems to bear this out.

So what I'm trying to say it, chances are your exception is actually being caught by the catch, but that the LogException is failing due to it trying to do work in context of an incorrect user.

To test this out, inside your LogException, try force your current context to "Self" before attempting any logging and see whether the 1st snippet now starts working.

1

Turns out the problem was with logging code. At some point we added code that retrieves current process start time (Process.StartTime) and that code yields Access denied when called from an impersonated thread.

Access is denied
System.ComponentModel.Win32Exception
at System.Diagnostics.Process.GetProcessHandle(Int32 access, Boolean throwIfExited)
at System.Diagnostics.Process.GetProcessTimes()
at System.Diagnostics.Process.get_StartTime()
//our logging code here

Logging code also called System.Diagnostics.Trace.WriteLine() before invoking code with Process.StartTime and writes through "trace listeners" succeeded, only writes through the latter code failed.

So there's no difference in catching exceptions with or without impersonation. It's even possible to install an exception filter so that it is invoked in security context of impersonated code which can possibly result in privileges elevation. Details for the latter here.

sharptooth
  • 167,383
  • 100
  • 513
  • 979