Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Obsolete SecureString ctors #5

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ internal static SafeLocalAllocHandle LocalAlloc(int cb)
return h;
}

internal static SafeLocalAllocHandle LocalAlloc(nuint cb)
{
var h = new SafeLocalAllocHandle();
h.SetHandle(Marshal.AllocHGlobal(unchecked((nint)cb)));
h.Initialize(cb);
return h;
}

// 0 is an Invalid Handle
internal SafeLocalAllocHandle(IntPtr handle) : base(true)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
using System.Security;
using Xunit;

#pragma warning disable OBS0001 // SecureString ctors are obsolete

namespace System.Diagnostics.Tests
{
public class EventLogSessionTests : FileCleanupTestBase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
using Microsoft.Win32.SafeHandles;
using Xunit;

#pragma warning disable OBS0001 // SecureString ctors are obsolete

namespace System.Diagnostics.Tests
{
public class ProcessStartInfoTests : ProcessTestBase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
using Xunit;
using Xunit.Sdk;

#pragma warning disable OBS0001 // SecureString ctors are obsolete

namespace System.Diagnostics.Tests
{
public partial class ProcessTests : ProcessTestBase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.ComponentModel;
using System.Security;

#pragma warning disable OBS0001 // SecureString ctor is obsolete

namespace System.Management
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,8 @@ public SecureString SecurePassword
{
get
{
string? str = _password as string;
if (str != null)
{
return MarshalToSecureString(str);
}
SecureString? sstr = _password as SecureString;
return sstr != null ? sstr.Copy() : new SecureString();
return (_password as SecureString)?.Copy()
?? MarshalToSecureString(_password as string);
}
set
{
Expand Down Expand Up @@ -175,8 +170,9 @@ private string MarshalToString(SecureString sstr)
return result;
}

private unsafe SecureString MarshalToSecureString(string str)
private unsafe SecureString MarshalToSecureString(string? str)
{
#pragma warning disable OBS0001 // SecureString ctor is obsolete
if (string.IsNullOrEmpty(str))
{
return new SecureString();
Expand All @@ -186,6 +182,7 @@ private unsafe SecureString MarshalToSecureString(string str)
{
return new SecureString(ptr, str.Length);
}
#pragma warning restore OBS0001
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.Security;
using Xunit;

#pragma warning disable OBS0001 // SecureString ctors are obsolete

namespace System.Net.Primitives.Functional.Tests
{
public static partial class NetworkCredentialTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;

using Xunit;

#pragma warning disable OBS0001 // SecureString ctors are obsolete

namespace System.Net.Security.Tests
{
using Configuration = System.Net.Test.Common.Configuration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,7 @@
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SafeBuffer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SafeHandle.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SEHException.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\ShroudedBuffer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\StructLayoutAttribute.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SuppressGCTransitionAttribute.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\TypeIdentifierAttribute.cs" />
Expand Down Expand Up @@ -1561,8 +1562,8 @@
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\Loader\LibraryNameVariation.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\MemoryFailPoint.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshal.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\ShroudedBuffer.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\Versioning\VersioningHelper.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Security\SecureString.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Thread.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\TimerQueue.Windows.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\TimeZoneInfo.Win32.cs" />
Expand Down Expand Up @@ -1772,8 +1773,8 @@
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\Loader\LibraryNameVariation.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\MemoryFailPoint.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\Marshal.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\ShroudedBuffer.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Runtime\Versioning\VersioningHelper.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Security\SecureString.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Thread.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\TimerQueue.Unix.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\TimeZoneInfo.Unix.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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.

namespace System.Runtime.InteropServices
{
internal sealed partial class ShroudedBufferHandle
{
internal IntPtr AllocateCore()
=> Marshal.AllocHGlobal((nint)_cbData);

private bool ReleaseHandleCore()
{
Marshal.FreeHGlobal(handle);
return true;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// 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.

namespace System.Runtime.InteropServices
{
internal sealed partial class ShroudedBufferHandle
{
private static readonly IntPtr _hHeap = GetOrCreateHeap();

internal IntPtr AllocateCore()
=> Interop.HeapAlloc(_hHeap, 0, _cbData);

private static IntPtr GetOrCreateHeap()
{
// In 64-bit processes where the virtual address space is larger,
// we use our own private heap to store the shrouded data. This
// somewhat isolates the shrouded data's storage space from that of
// the rest of the application, providing limited mitigation of a
// use-after-free elsewhere in the application accidentally pointing
// to an address used by a shrouded buffer instance.

IntPtr hHeap = (IntPtr.Size == 8) ? Interop.HeapCreate(0, 0, 0) : Interop.GetProcessHeap();
if (hHeap == IntPtr.Zero)
{
Marshal.ThrowExceptionForHR(Marshal.GetHRForLastWin32Error());
Environment.FailFast("Couldn't get heap information.");
}

return hHeap;
}

private bool ReleaseHandleCore()
{
return Interop.HeapFree(_hHeap, 0, handle);
}

private static class Interop
{
private const string KERNEL32_LIB = "kernel32.dll";

[DllImport(KERNEL32_LIB, CallingConvention = CallingConvention.Winapi, SetLastError = true)]
internal static extern IntPtr GetProcessHeap();

[DllImport(KERNEL32_LIB, CallingConvention = CallingConvention.Winapi, SetLastError = true)]
internal static extern IntPtr HeapCreate(uint flOptions, nuint dwInitialSize, nuint dwMaximumSize);

[DllImport(KERNEL32_LIB, CallingConvention = CallingConvention.Winapi, SetLastError = false)]
internal static extern IntPtr HeapAlloc(IntPtr hHeap, uint dwFlags, nuint dwBytes);

[DllImport(KERNEL32_LIB, CallingConvention = CallingConvention.Winapi, SetLastError = true)]
internal static extern bool HeapFree(IntPtr hHeap, uint dwFlags, IntPtr lpMem);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
// 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 Internal.Runtime.CompilerServices;
using Microsoft.Win32.SafeHandles;

namespace System.Runtime.InteropServices
{
/*
* Represents data which should be "shrouded" from the rest of the application.
* Shrouded data is sensitive data which should be difficult to *accidentally*
* disclose, even accounting for some types of application bugs. For example,
* we don't want accidental ArrayPool misuse to disclose the contents of shrouded
* buffers. But there's no effort to thwart *intentional* disclosure of these
* contents, such as through a debugger or memory dump utility.
*
* Some design philosophies and mitigations provided by this type:
*
* 1) The public surface area of this API works solely in Span<T>, not string
* or T[] or anything else. This gives the caller the ability to use pre-
* pinned memory if they so desire.
*
* 2) The virtual address space used to back the shrouded contents should be
* isolated from the virtual address space used by the rest of the
* framework or application code where practical. This helps avoid the
* case where the app has a dangling pointer or object reference that can
* be used to access the shrouded data.
*
* 3) The data is immutable once shrouded. If a caller wishes to mutate the
* contents, they must create a new instance. Immutability allows this
* object to be used by multiple callers simultaneously. (The Dispose
* method is not thread-safe.)
*
* 4) No reference is ever provided to the raw backing contents. This allows
* some future-proofing of the type, such as allowing the actual contents
* to be stored in a different process. (Think lsass / lsaiso.)
*
* *** THIS TYPE MAKES NO SECURITY GUARANTEES. ***
*
* This type is intended only to prevent accidental disclosure of the shrouded
* contents. If protection against an active adversary is warranted, the
* machine administrator should take additional steps such as: isolating the
* process handling the sensitive data into its own service account, enabling
* extra runtime safety checks, disabling memory dump collection, enabling OS-
* wide hardware-backed safety mechanisms, and restricting access to the host.
*/
public unsafe class ShroudedBuffer<T> : IDisposable where T : unmanaged
{
private readonly ShroudedBufferHandle _hnd;

/// <summary>
/// Creates a new <see cref="ShroudedBuffer{T}"/> from the provided contents.
/// </summary>
/// <param name="contents">The contents to copy into the new instance.</param>
/// <remarks>
/// The newly-returned <see cref="ShroudedBuffer{T}"/> instance maintains its
/// own copy of the data separate from <paramref name="contents"/>.
/// </remarks>
public ShroudedBuffer(ReadOnlySpan<T> contents)
{
// multiplication below will never overflow
_hnd = new ShroudedBufferHandle((nuint)Unsafe.SizeOf<T>() * (uint)contents.Length);
Length = contents.Length;

bool refAdded = false;
try
{
_hnd.DangerousAddRef(ref refAdded);
contents.CopyTo(new Span<T>((void*)_hnd.DangerousGetHandle(), Length));
}
finally
{
if (refAdded)
{
_hnd.DangerousRelease();
}
}
}

/// <summary>
/// Returns the length (in elements) of this buffer.
/// </summary>
public int Length { get; }

/// <summary>
/// Copies the contents of this <see cref="ShroudedBuffer{T}"/> instance to
/// a destination buffer.
/// </summary>
/// <param name="destination">
/// The destination buffer which should receive the contents.
/// This buffer must be at least <see cref="Length"/> elements in length.
/// </param>
/// <exception cref="ArgumentException">
/// <paramref name="destination"/>'s length is smaller than <see cref="Length"/>.
/// </exception>
/// <exception cref="ObjectDisposedException">
/// This instance has already been disposed.
/// </exception>
public void CopyTo(Span<T> destination)
{
bool refAdded = false;
try
{
_hnd.DangerousAddRef(ref refAdded);
ReadOnlySpan<T> source = new ReadOnlySpan<T>((void*)_hnd.DangerousGetHandle(), Length);
source.CopyTo(destination);
}
finally
{
if (refAdded)
{
_hnd.DangerousRelease();
}
}
}

internal ShroudedBuffer<T> DeepClone()
{
bool refAdded = false;
try
{
_hnd.DangerousAddRef(ref refAdded);
return new ShroudedBuffer<T>(new ReadOnlySpan<T>((void*)_hnd.DangerousGetHandle(), Length));
}
finally
{
if (refAdded)
{
_hnd.DangerousRelease();
}
}
}

/// <summary>
/// Disposes of this instance, including any unmanaged resources.
/// The contents will no longer be accessible once the instance is disposed.
/// </summary>
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

/// <summary>
/// Disposes of this instance, including any unmanaged resources.
/// The contents will no longer be accessible once the instance is disposed.
/// </summary>
/// <param name="disposing">
/// <see langword="true"/> if this instance is being disposed through a call
/// to the <see cref="Dispose"/> method; <see langword="false"/> if this instance
/// is being finalized.
/// </param>
protected virtual void Dispose(bool disposing)
{
_hnd.Dispose();
}
}

internal sealed partial class ShroudedBufferHandle : SafeHandleZeroOrMinusOneIsInvalid
{
private readonly nuint _cbData; // used to clear memory before freeing

// allocates memory; does not guarantee zero-init
internal ShroudedBufferHandle(nuint cbData)
: base(ownsHandle: true)
{
_cbData = cbData;
SetHandle(AllocateCore());

if (IsInvalid)
{
throw new OutOfMemoryException();
}
}

protected override unsafe bool ReleaseHandle()
{
// zero the memory before releasing it
SpanHelpers.ClearWithoutReferences(ref Unsafe.AsRef<byte>((void*)handle), _cbData);
return ReleaseHandleCore();
}
}
}
Loading