Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix SafeLsaMemoryHandle.InvalidHandle #32786

Merged
merged 10 commits into from Oct 19, 2018
Merged

Fix SafeLsaMemoryHandle.InvalidHandle #32786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ca73df9
fix SafeLsaMemoryHandle.InvalidHandle
MarcoRossignoli Oct 12, 2018
ad6e13c
address PR feedback
MarcoRossignoli Oct 12, 2018
22d1d40
update tests
MarcoRossignoli Oct 12, 2018
7359f41
address PR feedback
MarcoRossignoli Oct 12, 2018
3dd3fbf
update test
MarcoRossignoli Oct 13, 2018
1456edf
sort csproj
MarcoRossignoli Oct 13, 2018
c5bd80a
update test
MarcoRossignoli Oct 13, 2018
a2cb849
update test
MarcoRossignoli Oct 13, 2018
3f9ae05
address PR feedback
MarcoRossignoli Oct 18, 2018
171b8b3
address PR feedback
MarcoRossignoli Oct 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupNames2.cs
View file
Open in desktop
Expand Up @@ -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
);
}
}
4 changes: 2 additions & 2 deletions src/Common/src/Interop/Windows/advapi32/Interop.LsaLookupSids.cs
View file
Open in desktop
Expand Up @@ -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
);
}
}
2 changes: 1 addition & 1 deletion src/Common/src/Interop/Windows/sspicli/Interop.LsaGetLogonSessionData.cs
View file
Open in desktop
Expand Up @@ -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);
}
}
26 changes: 0 additions & 26 deletions src/System.Security.Principal.Windows/src/Microsoft/Win32/SafeHandles/SafeSecurityHandles.cs
View file
Open in desktop
Expand Up @@ -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;
Expand All @@ -44,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;
Expand All @@ -68,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
Expand Down
16 changes: 8 additions & 8 deletions src/System.Security.Principal.Windows/src/System/Security/Principal/NTAccount.cs
View file
Open in desktop
Expand Up @@ -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)
Expand Down Expand Up @@ -237,9 +237,9 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl
throw new ArgumentException(SR.Arg_EmptyCollection, nameof(sourceAccounts));
}

SafeLsaPolicyHandle LsaHandle = SafeLsaPolicyHandle.InvalidHandle;
SafeLsaMemoryHandle ReferencedDomainsPtr = SafeLsaMemoryHandle.InvalidHandle;
SafeLsaMemoryHandle SidsPtr = SafeLsaMemoryHandle.InvalidHandle;
SafeLsaPolicyHandle LsaHandle = null;
SafeLsaMemoryHandle ReferencedDomainsPtr = null;
SafeLsaMemoryHandle SidsPtr = null;

try
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -370,9 +370,9 @@ private static IdentityReferenceCollection TranslateToSids(IdentityReferenceColl
}
finally
{
LsaHandle.Dispose();
ReferencedDomainsPtr.Dispose();
MarcoRossignoli marked this conversation as resolved.
Show resolved Hide resolved
SidsPtr.Dispose();
LsaHandle?.Dispose();
ReferencedDomainsPtr?.Dispose();
SidsPtr?.Dispose();
}
}
#endregion
Expand Down
14 changes: 7 additions & 7 deletions src/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs
View file
Open in desktop
Expand Up @@ -1035,9 +1035,9 @@ 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;
SafeLsaPolicyHandle LsaHandle = null;
SafeLsaMemoryHandle ReferencedDomainsPtr = null;
SafeLsaMemoryHandle NamesPtr = null;

try
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1172,9 +1172,9 @@ private static IdentityReferenceCollection TranslateToNTAccounts(IdentityReferen
}
}

LsaHandle.Dispose();
ReferencedDomainsPtr.Dispose();
NamesPtr.Dispose();
LsaHandle?.Dispose();
ReferencedDomainsPtr?.Dispose();
NamesPtr?.Dispose();
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs
View file
Open in desktop
Expand Up @@ -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);

Expand All @@ -386,8 +386,7 @@ public override sealed string AuthenticationType
}
finally
{
if (!pLogonSessionData.IsInvalid)
pLogonSessionData.Dispose();
pLogonSessionData?.Dispose();
}
}

Expand Down
50 changes: 50 additions & 0 deletions src/System.Security.Principal.Windows/tests/NTAccount.cs
View file
Open in desktop
@@ -0,0 +1,50 @@
// 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]
public void Translate_Fail()
{
var nta = new NTAccount(Guid.NewGuid().ToString("N"));

try
{
nta.Translate(typeof(SecurityIdentifier));
}
catch (Exception e)
{
Asserts(e);
}

try
{
nta.Translate(typeof(SecurityIdentifier));
}
catch (Exception e)
{
Asserts(e);
}

return;

// Test assertions
void Asserts(Exception exception)
{
// If machine is in a domain but off line throws 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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danmosemsft @jkotas i don't know if this makes sense, maybe is extras.

}
}
}
}
}
3 changes: 2 additions & 1 deletion src/System.Security.Principal.Windows/tests/System.Security.Principal.Windows.Tests.csproj
View file
Open in desktop
@@ -1,9 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<ProjectGuid>{6C36F3AC-54A1-4021-9F5D-CDEFF7347277}</ProjectGuid>
<Configurations>netstandard-Windows_NT-Debug;netstandard-Windows_NT-Release</Configurations>
</PropertyGroup>
<ItemGroup>
<Compile Include="NTAccount.cs" />
<Compile Include="WindowsIdentityTests.cs" />
<Compile Include="WindowsPrincipalTests.cs" />
<Compile Include="WellKnownSidTypeTests.cs" />
Expand Down