From ca73df900b8a0b4e6896cad90c1049c77943bef9 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 12 Oct 2018 15:12:40 +0200 Subject: [PATCH 01/10] fix SafeLsaMemoryHandle.InvalidHandle --- .../Win32/SafeHandles/SafeSecurityHandles.cs | 4 +--- .../tests/NTAccount.cs | 20 +++++++++++++++++++ ...em.Security.Principal.Windows.Tests.csproj | 3 ++- 3 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 src/System.Security.Principal.Windows/tests/NTAccount.cs diff --git a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs index 4af4233e3e90..f61481cc8ed0 100644 --- a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs +++ b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs @@ -12,8 +12,6 @@ internal sealed class SafeLsaMemoryHandle : SafeBuffer { private SafeLsaMemoryHandle() : base(true) { } - private static SafeLsaMemoryHandle _invalidHandle = new SafeLsaMemoryHandle(IntPtr.Zero); - // 0 is an Invalid Handle internal SafeLsaMemoryHandle(IntPtr handle) : base(true) { @@ -24,7 +22,7 @@ internal static SafeLsaMemoryHandle InvalidHandle { get { - return _invalidHandle; + return new SafeLsaMemoryHandle(IntPtr.Zero); } } diff --git a/src/System.Security.Principal.Windows/tests/NTAccount.cs b/src/System.Security.Principal.Windows/tests/NTAccount.cs new file mode 100644 index 000000000000..4c4fe4af5bbf --- /dev/null +++ b/src/System.Security.Principal.Windows/tests/NTAccount.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.ComponentModel; +using Xunit; + +namespace System.Security.Principal.Windows.Tests +{ + public class NTAccountTest + { + [Fact(Skip = "This test needs a machine in a domain but off line.")] + public void Translate_Fail() + { + var nta = new NTAccount("foobar"); + Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); + Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); + } + } +} diff --git a/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj b/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj index d91f571cd627..98b66fd060fc 100644 --- a/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj +++ b/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj @@ -1,4 +1,4 @@ - + {6C36F3AC-54A1-4021-9F5D-CDEFF7347277} netstandard-Windows_NT-Debug;netstandard-Windows_NT-Release @@ -7,6 +7,7 @@ + From ad6e13c72fe18708195187cc984f1bb94270c098 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 12 Oct 2018 16:45:25 +0200 Subject: [PATCH 02/10] address PR feedback --- .../src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs | 4 +++- .../src/System/Security/Principal/NTAccount.cs | 2 -- .../src/System/Security/Principal/SID.cs | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs index f61481cc8ed0..4af4233e3e90 100644 --- a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs +++ b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs @@ -12,6 +12,8 @@ internal sealed class SafeLsaMemoryHandle : SafeBuffer { private SafeLsaMemoryHandle() : base(true) { } + private static SafeLsaMemoryHandle _invalidHandle = new SafeLsaMemoryHandle(IntPtr.Zero); + // 0 is an Invalid Handle internal SafeLsaMemoryHandle(IntPtr handle) : base(true) { @@ -22,7 +24,7 @@ internal static SafeLsaMemoryHandle InvalidHandle { get { - return new SafeLsaMemoryHandle(IntPtr.Zero); + return _invalidHandle; } } diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs index 6b02969198a7..fbb8452cdfca 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs @@ -371,8 +371,6 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl finally { LsaHandle.Dispose(); - ReferencedDomainsPtr.Dispose(); - SidsPtr.Dispose(); } } #endregion diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs index e27c2f989514..b691682b6868 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs @@ -1173,8 +1173,6 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen } LsaHandle.Dispose(); - ReferencedDomainsPtr.Dispose(); - NamesPtr.Dispose(); } } From 22d1d40d73060a4377241dd3439d144decac213e Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 12 Oct 2018 16:50:31 +0200 Subject: [PATCH 03/10] update tests --- src/System.Security.Principal.Windows/tests/NTAccount.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/System.Security.Principal.Windows/tests/NTAccount.cs b/src/System.Security.Principal.Windows/tests/NTAccount.cs index 4c4fe4af5bbf..575f0e0eb8cd 100644 --- a/src/System.Security.Principal.Windows/tests/NTAccount.cs +++ b/src/System.Security.Principal.Windows/tests/NTAccount.cs @@ -10,11 +10,18 @@ namespace System.Security.Principal.Windows.Tests public class NTAccountTest { [Fact(Skip = "This test needs a machine in a domain but off line.")] - public void Translate_Fail() + public void Translate_Fail_Domain_Offline() { var nta = new NTAccount("foobar"); Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); } + + [Fact] + public void Translate_Fail() + { + var nta = new NTAccount("foobar"); + Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); + } } } From 7359f41383be73c45b7329ed0f6575855d176f32 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 12 Oct 2018 18:02:15 +0200 Subject: [PATCH 04/10] address PR feedback --- .../src/System/Security/Principal/NTAccount.cs | 17 +++++++++++++++-- .../src/System/Security/Principal/SID.cs | 15 ++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs index fbb8452cdfca..949ceac97cdf 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs @@ -176,7 +176,7 @@ internal static IdentityReferenceCollection Translate(IdentityReferenceCollectio return Result; } - + internal static IdentityReferenceCollection Translate(IdentityReferenceCollection sourceAccounts, Type targetType, out bool someFailed) { if (sourceAccounts == null) @@ -370,7 +370,20 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl } finally { - LsaHandle.Dispose(); + if (!LsaHandle.IsInvalid) + { + LsaHandle.Dispose(); + } + + if (!ReferencedDomainsPtr.IsInvalid) + { + ReferencedDomainsPtr.Dispose(); + } + + if (!SidsPtr.IsInvalid) + { + SidsPtr.Dispose(); + } } } #endregion diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs index b691682b6868..e9c25b67b995 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs @@ -1172,7 +1172,20 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen } } - LsaHandle.Dispose(); + if (!LsaHandle.IsInvalid) + { + LsaHandle.Dispose(); + } + + if (!ReferencedDomainsPtr.IsInvalid) + { + ReferencedDomainsPtr.Dispose(); + } + + if (!NamesPtr.IsInvalid) + { + NamesPtr.Dispose(); + } } } From 3dd3fbf3b4fa0a81ef5024bd9da99bb34b7398d6 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 13 Oct 2018 09:22:54 +0200 Subject: [PATCH 05/10] update test --- .../tests/NTAccount.cs | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/src/System.Security.Principal.Windows/tests/NTAccount.cs b/src/System.Security.Principal.Windows/tests/NTAccount.cs index 575f0e0eb8cd..21617f75e245 100644 --- a/src/System.Security.Principal.Windows/tests/NTAccount.cs +++ b/src/System.Security.Principal.Windows/tests/NTAccount.cs @@ -9,19 +9,40 @@ namespace System.Security.Principal.Windows.Tests { public class NTAccountTest { - [Fact(Skip = "This test needs a machine in a domain but off line.")] - public void Translate_Fail_Domain_Offline() - { - var nta = new NTAccount("foobar"); - Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); - Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); - } - [Fact] public void Translate_Fail() { - var nta = new NTAccount("foobar"); - Assert.Throws(() => nta.Translate(typeof(SecurityIdentifier))); + var nta = new NTAccount(Guid.NewGuid().ToString("N")); + + try + { + nta.Translate(typeof(SecurityIdentifier)); + } + catch (Exception ex) + { + // If machine is in a domain but off line throws Win32Exception + Assert.True(ex is IdentityNotMappedException || ex is Win32Exception); + if (ex is Win32Exception win32Exception) + { + // ERROR_TRUSTED_RELATIONSHIP_FAILURE: The trust relationship between this workstation and the primary domain failed. + Assert.Equal(1789, win32Exception.NativeErrorCode); + } + } + + try + { + nta.Translate(typeof(SecurityIdentifier)); + } + catch (Exception ex) + { + // If machine is in a domain but off line throws Win32Exception + Assert.True(ex is IdentityNotMappedException || ex is Win32Exception); + if (ex is Win32Exception win32Exception) + { + // ERROR_TRUSTED_RELATIONSHIP_FAILURE: The trust relationship between this workstation and the primary domain failed. + Assert.Equal(1789, win32Exception.NativeErrorCode); + } + } } } } From 1456edfc418fb92dad099b5c9ecb165ca9689c4e Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 13 Oct 2018 09:26:25 +0200 Subject: [PATCH 06/10] sort csproj --- .../tests/System.Security.Principal.Windows.Tests.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj b/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj index 98b66fd060fc..e2b00270f274 100644 --- a/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj +++ b/src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj @@ -4,10 +4,10 @@ netstandard-Windows_NT-Debug;netstandard-Windows_NT-Release + - From c5bd80ab31685a3ac9535f4179e21713d474c3ba Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 13 Oct 2018 09:29:53 +0200 Subject: [PATCH 07/10] update test --- .../tests/NTAccount.cs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/System.Security.Principal.Windows/tests/NTAccount.cs b/src/System.Security.Principal.Windows/tests/NTAccount.cs index 21617f75e245..b5fb6df2d08c 100644 --- a/src/System.Security.Principal.Windows/tests/NTAccount.cs +++ b/src/System.Security.Principal.Windows/tests/NTAccount.cs @@ -18,26 +18,27 @@ public void Translate_Fail() { nta.Translate(typeof(SecurityIdentifier)); } - catch (Exception ex) + catch (Exception e) { - // If machine is in a domain but off line throws Win32Exception - Assert.True(ex is IdentityNotMappedException || ex is Win32Exception); - if (ex is Win32Exception win32Exception) - { - // ERROR_TRUSTED_RELATIONSHIP_FAILURE: The trust relationship between this workstation and the primary domain failed. - Assert.Equal(1789, win32Exception.NativeErrorCode); - } + Asserts(e); } try { nta.Translate(typeof(SecurityIdentifier)); } - catch (Exception ex) + catch (Exception e) + { + Asserts(e); + } + + return; + + void Asserts(Exception exception) { // If machine is in a domain but off line throws Win32Exception - Assert.True(ex is IdentityNotMappedException || ex is Win32Exception); - if (ex is Win32Exception win32Exception) + Assert.True(exception is IdentityNotMappedException || exception is Win32Exception); + if (exception is Win32Exception win32Exception) { // ERROR_TRUSTED_RELATIONSHIP_FAILURE: The trust relationship between this workstation and the primary domain failed. Assert.Equal(1789, win32Exception.NativeErrorCode); From a2cb8494d8b21de8c42b3115e9978f6b3ad06a9b Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Sat, 13 Oct 2018 09:33:47 +0200 Subject: [PATCH 08/10] update test --- src/System.Security.Principal.Windows/tests/NTAccount.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/System.Security.Principal.Windows/tests/NTAccount.cs b/src/System.Security.Principal.Windows/tests/NTAccount.cs index b5fb6df2d08c..16a044d744ba 100644 --- a/src/System.Security.Principal.Windows/tests/NTAccount.cs +++ b/src/System.Security.Principal.Windows/tests/NTAccount.cs @@ -34,6 +34,7 @@ public void Translate_Fail() return; + // Test assertions void Asserts(Exception exception) { // If machine is in a domain but off line throws Win32Exception From 3f9ae056d33fe66b6965b2d16414108d3f5b6a03 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Thu, 18 Oct 2018 11:45:30 +0200 Subject: [PATCH 09/10] address PR feedback --- .../Windows/advapi32/Interop.LsaLookupNames2.cs | 4 ++-- .../Windows/advapi32/Interop.LsaLookupSids.cs | 4 ++-- .../Win32/SafeHandles/SafeSecurityHandles.cs | 10 ---------- .../src/System/Security/Principal/NTAccount.cs | 17 +++++------------ .../src/System/Security/Principal/SID.cs | 17 +++++------------ 5 files changed, 14 insertions(+), 38 deletions(-) diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupNames2.cs b/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupNames2.cs index 41b65e29a689..dcb76bec8252 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupNames2.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupNames2.cs @@ -16,8 +16,8 @@ internal static partial class Advapi32 int flags, int count, UNICODE_STRING[] names, - ref SafeLsaMemoryHandle referencedDomains, - ref SafeLsaMemoryHandle sids + out SafeLsaMemoryHandle referencedDomains, + out SafeLsaMemoryHandle sids ); } } diff --git a/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupSids.cs b/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupSids.cs index 7e067d63348d..576d8e53bd74 100644 --- a/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupSids.cs +++ b/src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupSids.cs @@ -15,8 +15,8 @@ internal static partial class Advapi32 SafeLsaPolicyHandle handle, int count, IntPtr[] sids, - ref SafeLsaMemoryHandle referencedDomains, - ref SafeLsaMemoryHandle names + out SafeLsaMemoryHandle referencedDomains, + out SafeLsaMemoryHandle names ); } } diff --git a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs index 4af4233e3e90..2067356f2c28 100644 --- a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs +++ b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs @@ -12,22 +12,12 @@ internal sealed class SafeLsaMemoryHandle : SafeBuffer { private SafeLsaMemoryHandle() : base(true) { } - private static SafeLsaMemoryHandle _invalidHandle = new SafeLsaMemoryHandle(IntPtr.Zero); - // 0 is an Invalid Handle internal SafeLsaMemoryHandle(IntPtr handle) : base(true) { SetHandle(handle); } - internal static SafeLsaMemoryHandle InvalidHandle - { - get - { - return _invalidHandle; - } - } - override protected bool ReleaseHandle() { return Interop.Advapi32.LsaFreeMemory(handle) == 0; diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs index 949ceac97cdf..3872583f26f0 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs @@ -238,8 +238,8 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl } SafeLsaPolicyHandle LsaHandle = SafeLsaPolicyHandle.InvalidHandle; - SafeLsaMemoryHandle ReferencedDomainsPtr = SafeLsaMemoryHandle.InvalidHandle; - SafeLsaMemoryHandle SidsPtr = SafeLsaMemoryHandle.InvalidHandle; + SafeLsaMemoryHandle ReferencedDomainsPtr = null; + SafeLsaMemoryHandle SidsPtr = null; try { @@ -287,7 +287,7 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl someFailed = false; uint ReturnCode; - ReturnCode = Interop.Advapi32.LsaLookupNames2(LsaHandle, 0, sourceAccounts.Count, Names, ref ReferencedDomainsPtr, ref SidsPtr); + ReturnCode = Interop.Advapi32.LsaLookupNames2(LsaHandle, 0, sourceAccounts.Count, Names, out ReferencedDomainsPtr, out SidsPtr); // // Make a decision regarding whether it makes sense to proceed @@ -375,15 +375,8 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl LsaHandle.Dispose(); } - if (!ReferencedDomainsPtr.IsInvalid) - { - ReferencedDomainsPtr.Dispose(); - } - - if (!SidsPtr.IsInvalid) - { - SidsPtr.Dispose(); - } + ReferencedDomainsPtr?.Dispose(); + SidsPtr?.Dispose(); } } #endregion diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs index e9c25b67b995..93f9f80a2238 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs @@ -1036,8 +1036,8 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen IntPtr[] SidArrayPtr = new IntPtr[sourceSids.Count]; GCHandle[] HandleArray = new GCHandle[sourceSids.Count]; SafeLsaPolicyHandle LsaHandle = SafeLsaPolicyHandle.InvalidHandle; - SafeLsaMemoryHandle ReferencedDomainsPtr = SafeLsaMemoryHandle.InvalidHandle; - SafeLsaMemoryHandle NamesPtr = SafeLsaMemoryHandle.InvalidHandle; + SafeLsaMemoryHandle ReferencedDomainsPtr = null; + SafeLsaMemoryHandle NamesPtr = null; try { @@ -1072,7 +1072,7 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen someFailed = false; uint ReturnCode; - ReturnCode = Interop.Advapi32.LsaLookupSids(LsaHandle, sourceSids.Count, SidArrayPtr, ref ReferencedDomainsPtr, ref NamesPtr); + ReturnCode = Interop.Advapi32.LsaLookupSids(LsaHandle, sourceSids.Count, SidArrayPtr, out ReferencedDomainsPtr, out NamesPtr); // // Make a decision regarding whether it makes sense to proceed @@ -1177,15 +1177,8 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen LsaHandle.Dispose(); } - if (!ReferencedDomainsPtr.IsInvalid) - { - ReferencedDomainsPtr.Dispose(); - } - - if (!NamesPtr.IsInvalid) - { - NamesPtr.Dispose(); - } + ReferencedDomainsPtr?.Dispose(); + NamesPtr?.Dispose(); } } From 171b8b38bbb9e6bd66a99c4a2592262c039b6cd6 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Thu, 18 Oct 2018 14:47:50 +0200 Subject: [PATCH 10/10] address PR feedback --- .../sspicli/Interop.LsaGetLogonSessionData.cs | 2 +- .../Win32/SafeHandles/SafeSecurityHandles.cs | 16 ---------------- .../src/System/Security/Principal/NTAccount.cs | 8 ++------ .../src/System/Security/Principal/SID.cs | 8 ++------ .../System/Security/Principal/WindowsIdentity.cs | 7 +++---- 5 files changed, 8 insertions(+), 33 deletions(-) diff --git a/src/Common/src/Interop/Windows/sspicli/Interop.LsaGetLogonSessionData.cs b/src/Common/src/Interop/Windows/sspicli/Interop.LsaGetLogonSessionData.cs index 621a9011fa17..67d630a48864 100644 --- a/src/Common/src/Interop/Windows/sspicli/Interop.LsaGetLogonSessionData.cs +++ b/src/Common/src/Interop/Windows/sspicli/Interop.LsaGetLogonSessionData.cs @@ -11,6 +11,6 @@ internal static partial class Interop internal static partial class SspiCli { [DllImport(Interop.Libraries.SspiCli, SetLastError = true)] - internal static extern int LsaGetLogonSessionData(ref LUID LogonId, ref SafeLsaReturnBufferHandle ppLogonSessionData); + internal static extern int LsaGetLogonSessionData(ref LUID LogonId, out SafeLsaReturnBufferHandle ppLogonSessionData); } } diff --git a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs index 2067356f2c28..3db37143d65c 100644 --- a/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs +++ b/src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs @@ -34,14 +34,6 @@ internal SafeLsaPolicyHandle(IntPtr handle) : base(true) SetHandle(handle); } - internal static SafeLsaPolicyHandle InvalidHandle - { - get - { - return new SafeLsaPolicyHandle(IntPtr.Zero); - } - } - override protected bool ReleaseHandle() { return Interop.Advapi32.LsaClose(handle) == 0; @@ -58,14 +50,6 @@ internal SafeLsaReturnBufferHandle(IntPtr handle) : base(true) SetHandle(handle); } - internal static SafeLsaReturnBufferHandle InvalidHandle - { - get - { - return new SafeLsaReturnBufferHandle(IntPtr.Zero); - } - } - override protected bool ReleaseHandle() { // LsaFreeReturnBuffer returns an NTSTATUS diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs index 3872583f26f0..8d515f01d9f4 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs @@ -237,7 +237,7 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl throw new ArgumentException(SR.Arg_EmptyCollection, nameof(sourceAccounts)); } - SafeLsaPolicyHandle LsaHandle = SafeLsaPolicyHandle.InvalidHandle; + SafeLsaPolicyHandle LsaHandle = null; SafeLsaMemoryHandle ReferencedDomainsPtr = null; SafeLsaMemoryHandle SidsPtr = null; @@ -370,11 +370,7 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl } finally { - if (!LsaHandle.IsInvalid) - { - LsaHandle.Dispose(); - } - + LsaHandle?.Dispose(); ReferencedDomainsPtr?.Dispose(); SidsPtr?.Dispose(); } diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs index 93f9f80a2238..09435371977a 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs @@ -1035,7 +1035,7 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen IntPtr[] SidArrayPtr = new IntPtr[sourceSids.Count]; GCHandle[] HandleArray = new GCHandle[sourceSids.Count]; - SafeLsaPolicyHandle LsaHandle = SafeLsaPolicyHandle.InvalidHandle; + SafeLsaPolicyHandle LsaHandle = null; SafeLsaMemoryHandle ReferencedDomainsPtr = null; SafeLsaMemoryHandle NamesPtr = null; @@ -1172,11 +1172,7 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen } } - if (!LsaHandle.IsInvalid) - { - LsaHandle.Dispose(); - } - + LsaHandle?.Dispose(); ReferencedDomainsPtr?.Dispose(); NamesPtr?.Dispose(); } diff --git a/src/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs b/src/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs index b75b0ade684e..2db1c964e269 100644 --- a/src/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs +++ b/src/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs @@ -372,10 +372,10 @@ public override sealed string AuthenticationType if (authId.LowPart == Interop.LuidOptions.ANONYMOUS_LOGON_LUID) return string.Empty; // no authentication, just return an empty string - SafeLsaReturnBufferHandle pLogonSessionData = SafeLsaReturnBufferHandle.InvalidHandle; + SafeLsaReturnBufferHandle pLogonSessionData = null; try { - int status = Interop.SspiCli.LsaGetLogonSessionData(ref authId, ref pLogonSessionData); + int status = Interop.SspiCli.LsaGetLogonSessionData(ref authId, out pLogonSessionData); if (status < 0) // non-negative numbers indicate success throw GetExceptionFromNtStatus(status); @@ -386,8 +386,7 @@ public override sealed string AuthenticationType } finally { - if (!pLogonSessionData.IsInvalid) - pLogonSessionData.Dispose(); + pLogonSessionData?.Dispose(); } }