From 6b3e395f7cc184a34cd8c13daf13b1e836853987 Mon Sep 17 00:00:00 2001 From: antiduh Date: Tue, 24 Jun 2014 19:41:19 +0000 Subject: [PATCH] Implemented SafeHandle usage for Context et al. The usage isn't actually safe yet, since I still reference the rawHandle without doing safe (CER) reference counting on the handle. --- Contexts/ClientContext.cs | 32 +++++++++++++++++++++----------- Contexts/Context.cs | 32 +++++++++++++++----------------- Contexts/ContextNativeMethods.cs | 22 +++++++++++----------- Contexts/ServerContext.cs | 16 ++++++---------- SspiHandle.cs | 19 +++++++++++++++++++ 5 files changed, 72 insertions(+), 49 deletions(-) diff --git a/Contexts/ClientContext.cs b/Contexts/ClientContext.cs index 437052d..4d91e57 100644 --- a/Contexts/ClientContext.cs +++ b/Contexts/ClientContext.cs @@ -25,9 +25,6 @@ namespace NSspi.Contexts public SecurityStatus Init( byte[] serverToken, out byte[] outToken ) { - long prevContextHandle = base.ContextHandle; - long newContextHandle = 0; - long expiry = 0; SecurityStatus status; @@ -38,11 +35,11 @@ namespace NSspi.Contexts SecureBuffer serverBuffer; SecureBufferAdapter serverAdapter; - if ( (serverToken != null) && (prevContextHandle == 0) ) + if ( (serverToken != null) && ( this.ContextHandle.IsInvalid ) ) { throw new InvalidOperationException( "Out-of-order usage detected - have a server token, but no previous client token had been created." ); } - else if ( (serverToken == null) && (prevContextHandle != 0) ) + else if ( (serverToken == null) && ( this.ContextHandle.IsInvalid == false ) ) { throw new InvalidOperationException( "Must provide the server's response when continuing the init process." ); } @@ -55,9 +52,24 @@ namespace NSspi.Contexts serverBuffer = new SecureBuffer( serverToken, BufferType.Token ); } + // Some notes on handles and invoking InitializeSecurityContext + // - The first time around, the phContext parameter (the 'old' handle) is a null pointer to what + // would be an RawSspiHandle, to indicate this is the first time it's being called. + // The phNewContext is a pointer (reference) to an RawSspiHandle struct of where to write the + // new handle's values. + // - The next time you invoke ISC, it takes a pointer to the handle it gave you last time in phContext, + // and takes a pointer to where it should write the new handle's values in phNewContext. + // - After the first time, you can provide the same handle to both parameters. From MSDN: + // "On the second call, phNewContext can be the same as the handle specified in the phContext + // parameter." + // It will overwrite the handle you gave it with the new handle value. + // - All handle structures themselves are actually *two* pointer variables, eg, 64 bits on 32-bit + // Windows, 128 bits on 64-bit Windows. + // - So in the end, on a 64-bit machine, we're passing a 64-bit value (the pointer to the struct) that + // points to 128 bits of memory (the struct itself) for where to write the handle numbers. using ( outAdapter = new SecureBufferAdapter( outTokenBuffer ) ) { - if ( prevContextHandle == 0 ) + if ( this.ContextHandle.IsInvalid ) { status = ContextNativeMethods.InitializeSecurityContext_1( ref this.Credential.Handle.rawHandle, @@ -68,7 +80,7 @@ namespace NSspi.Contexts SecureBufferDataRep.Network, IntPtr.Zero, 0, - ref newContextHandle, + ref this.ContextHandle.rawHandle, outAdapter.Handle, ref this.finalAttribs, ref expiry @@ -80,14 +92,14 @@ namespace NSspi.Contexts { status = ContextNativeMethods.InitializeSecurityContext_2( ref this.Credential.Handle.rawHandle, - ref prevContextHandle, + ref this.ContextHandle.rawHandle, this.serverPrinc, this.requestedAttribs, 0, SecureBufferDataRep.Network, serverAdapter.Handle, 0, - ref newContextHandle, + ref this.ContextHandle.rawHandle, outAdapter.Handle, ref this.finalAttribs, ref expiry @@ -114,8 +126,6 @@ namespace NSspi.Contexts throw new SSPIException( "Failed to invoke InitializeSecurityContext for a client", status ); } - base.ContextHandle = newContextHandle; - return status; } } diff --git a/Contexts/Context.cs b/Contexts/Context.cs index 58b9873..eb2cc1e 100644 --- a/Contexts/Context.cs +++ b/Contexts/Context.cs @@ -16,6 +16,8 @@ namespace NSspi { this.Credential = cred; + this.ContextHandle = new SafeContextHandle(); + this.disposed = false; } @@ -26,7 +28,7 @@ namespace NSspi protected Credential Credential { get; private set; } - public long ContextHandle { get; protected set; } + public SafeContextHandle ContextHandle { get; protected set; } public string AuthorityName { @@ -59,10 +61,10 @@ namespace NSspi this.Credential.Dispose(); } - long contextHandleCopy = this.ContextHandle; - ContextNativeMethods.DeleteSecurityContext( ref contextHandleCopy ); + // TODO SAFE_CER + ContextNativeMethods.DeleteSecurityContext( ref this.ContextHandle.rawHandle ); - this.ContextHandle = 0; + this.ContextHandle.Dispose(); this.disposed = true; } @@ -90,8 +92,6 @@ namespace NSspi SecureBuffer paddingBuffer; SecureBufferAdapter adapter; - long contextHandle = this.ContextHandle; - SecurityStatus status; byte[] result; @@ -103,8 +103,9 @@ namespace NSspi using( adapter = new SecureBufferAdapter( new[] { trailerBuffer, dataBuffer, paddingBuffer } ) ) { + // TODO SAFE_CER status = ContextNativeMethods.EncryptMessage( - ref contextHandle, + ref this.ContextHandle.rawHandle, 0, adapter.Handle, 0 @@ -154,9 +155,7 @@ namespace NSspi SecureBuffer dataBuffer; SecureBuffer paddingBuffer; SecureBufferAdapter adapter; - - long contextHandle = this.ContextHandle; - + SecurityStatus status; byte[] result = null; int remaining; @@ -221,8 +220,9 @@ namespace NSspi using( adapter = new SecureBufferAdapter( new [] { trailerBuffer, dataBuffer, paddingBuffer } ) ) { + // TODO SAFE_CER status = ContextNativeMethods.DecryptMessage( - ref contextHandle, + ref this.ContextHandle.rawHandle, adapter.Handle, 0, 0 @@ -243,11 +243,11 @@ namespace NSspi internal SecPkgContext_Sizes QueryBufferSizes() { SecPkgContext_Sizes sizes = new SecPkgContext_Sizes(); - long contextHandle = this.ContextHandle; SecurityStatus status; + // TODO SAFE_CER status = ContextNativeMethods.QueryContextAttributes_Sizes( - ref contextHandle, + ref this.ContextHandle.rawHandle, ContextQueryAttrib.Sizes, ref sizes ); @@ -263,7 +263,6 @@ namespace NSspi internal string QueryContextString(ContextQueryAttrib attrib) { SecPkgContext_String stringAttrib; - long contextHandle; SecurityStatus status; string result; @@ -274,10 +273,9 @@ namespace NSspi stringAttrib = new SecPkgContext_String(); - contextHandle = this.ContextHandle; - + // TODO SAFE_CER status = ContextNativeMethods.QueryContextAttributes_String( - ref contextHandle, + ref this.ContextHandle.rawHandle, attrib, ref stringAttrib ); diff --git a/Contexts/ContextNativeMethods.cs b/Contexts/ContextNativeMethods.cs index 9b5b86d..d0d3ce7 100644 --- a/Contexts/ContextNativeMethods.cs +++ b/Contexts/ContextNativeMethods.cs @@ -36,7 +36,7 @@ namespace NSspi.Contexts IntPtr inputBuffer, ContextAttrib requestedAttribs, SecureBufferDataRep dataRep, - ref long newContextHandle, + ref RawSspiHandle newContextHandle, IntPtr outputBuffer, ref ContextAttrib outputAttribs, ref long expiry @@ -52,11 +52,11 @@ namespace NSspi.Contexts )] public static extern SecurityStatus AcceptSecurityContext_2( ref RawSspiHandle credHandle, - ref long oldContextHandle, + ref RawSspiHandle oldContextHandle, IntPtr inputBuffer, ContextAttrib requestedAttribs, SecureBufferDataRep dataRep, - ref long newContextHandle, + ref RawSspiHandle newContextHandle, IntPtr outputBuffer, ref ContextAttrib outputAttribs, ref long expiry @@ -112,7 +112,7 @@ namespace NSspi.Contexts SecureBufferDataRep dataRep, IntPtr inputBuffer, int reserved2, - ref long newContextHandle, + ref RawSspiHandle newContextHandle, IntPtr outputBuffer, ref ContextAttrib contextAttribs, ref long expiry @@ -127,14 +127,14 @@ namespace NSspi.Contexts )] public static extern SecurityStatus InitializeSecurityContext_2( ref RawSspiHandle credentialHandle, - ref long previousHandle, + ref RawSspiHandle previousHandle, string serverPrincipleName, ContextAttrib requiredAttribs, int reserved1, SecureBufferDataRep dataRep, IntPtr inputBuffer, int reserved2, - ref long newContextHandle, + ref RawSspiHandle newContextHandle, IntPtr outputBuffer, ref ContextAttrib contextAttribs, ref long expiry @@ -147,7 +147,7 @@ namespace NSspi.Contexts CharSet = CharSet.Unicode, SetLastError = true )] - public static extern SecurityStatus DeleteSecurityContext( ref long contextHandle ); + public static extern SecurityStatus DeleteSecurityContext( ref RawSspiHandle contextHandle ); [DllImport( "Secur32.dll", @@ -157,7 +157,7 @@ namespace NSspi.Contexts SetLastError = true )] public static extern SecurityStatus EncryptMessage( - ref long contextHandle, + ref RawSspiHandle contextHandle, int qualityOfProtection, IntPtr bufferDescriptor, int sequenceNumber @@ -172,7 +172,7 @@ namespace NSspi.Contexts SetLastError = true )] public static extern SecurityStatus DecryptMessage( - ref long contextHandle, + ref RawSspiHandle contextHandle, IntPtr bufferDescriptor, int sequenceNumber, int qualityOfProtection @@ -184,7 +184,7 @@ namespace NSspi.Contexts CallingConvention = CallingConvention.Winapi, CharSet = CharSet.Unicode )] public static extern SecurityStatus QueryContextAttributes_Sizes( - ref long contextHandle, + ref RawSspiHandle contextHandle, ContextQueryAttrib attrib, ref SecPkgContext_Sizes sizes ); @@ -195,7 +195,7 @@ namespace NSspi.Contexts CallingConvention = CallingConvention.Winapi, CharSet = CharSet.Unicode )] public static extern SecurityStatus QueryContextAttributes_String( - ref long contextHandle, + ref RawSspiHandle contextHandle, ContextQueryAttrib attrib, ref SecPkgContext_String names ); diff --git a/Contexts/ServerContext.cs b/Contexts/ServerContext.cs index 3cf0847..2b82e02 100644 --- a/Contexts/ServerContext.cs +++ b/Contexts/ServerContext.cs @@ -24,21 +24,17 @@ namespace NSspi.Contexts SecureBuffer clientBuffer = new SecureBuffer( clientToken, BufferType.Token ); SecureBuffer outBuffer = new SecureBuffer( new byte[12288], BufferType.Token ); - long oldContextHandle = base.ContextHandle; - long newContextHandle = 0; - SecurityStatus status; long expiry = 0; SecureBufferAdapter clientAdapter; SecureBufferAdapter outAdapter; - using ( clientAdapter = new SecureBufferAdapter( clientBuffer ) ) { using ( outAdapter = new SecureBufferAdapter( outBuffer ) ) { - if ( oldContextHandle == 0 ) + if( this.ContextHandle.IsInvalid ) { status = ContextNativeMethods.AcceptSecurityContext_1( ref this.Credential.Handle.rawHandle, @@ -46,7 +42,7 @@ namespace NSspi.Contexts clientAdapter.Handle, requestedAttribs, SecureBufferDataRep.Network, - ref newContextHandle, + ref this.ContextHandle.rawHandle, outAdapter.Handle, ref this.finalAttribs, ref expiry @@ -56,15 +52,17 @@ namespace NSspi.Contexts { status = ContextNativeMethods.AcceptSecurityContext_2( ref this.Credential.Handle.rawHandle, - ref oldContextHandle, + ref this.ContextHandle.rawHandle, clientAdapter.Handle, requestedAttribs, SecureBufferDataRep.Network, - ref newContextHandle, + ref this.ContextHandle.rawHandle, outAdapter.Handle, ref this.finalAttribs, ref expiry ); + + } } } @@ -96,8 +94,6 @@ namespace NSspi.Contexts throw new SSPIException( "Failed to call AcceptSecurityContext", status ); } - base.ContextHandle = newContextHandle; - return status; } } diff --git a/SspiHandle.cs b/SspiHandle.cs index 7d5ebef..6ee7872 100644 --- a/SspiHandle.cs +++ b/SspiHandle.cs @@ -6,6 +6,7 @@ using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; using System.Text; using System.Threading.Tasks; +using NSspi.Contexts; namespace NSspi { @@ -83,4 +84,22 @@ namespace NSspi return status == SecurityStatus.OK; } } + + public class SafeContextHandle : SafeSspiHandle + { + public SafeContextHandle() + : base() + { } + + protected override bool ReleaseHandle() + { + SecurityStatus status = ContextNativeMethods.DeleteSecurityContext( + ref base.rawHandle + ); + + base.ReleaseHandle(); + + return status == SecurityStatus.OK; + } + } }