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

Commit

Permalink
Address PR feedback
Browse files Browse the repository at this point in the history 
- Rewrite HashProviderDispenser (OSX) FinalizeHashAndReset in terms of TryFinalizeHashAndReset
- Reorder _running/SetKey check in HashProviderDispenser
- Add bigger-than-needed-span test for HashAlgorithm
- Fix incorrect const usage in original and copied test
- Change HashAlgorithm.Hash to avoid throwing if HashValue is null
- Make TryComputeHash null out HashValue on success
- Cache HashSizeValue/8 into a local in TryHashFinal rather than recomputing
  • Loading branch information
@stephentoub
stephentoub committed Aug 10, 2017
1 parent 6d537ac commit 0597b42
Show file tree
Hide file tree
Showing 27 changed files with 181 additions and 182 deletions.
29 changes: 8 additions & 21 deletions src/Common/src/Internal/Cryptography/HashProviderCng.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// See the LICENSE file in the project root for more information.

using System;

using System.Diagnostics;
using Microsoft.Win32.SafeHandles;
using NTSTATUS = Interop.BCrypt.NTSTATUS;
using BCryptOpenAlgorithmProviderFlags = Interop.BCrypt.BCryptOpenAlgorithmProviderFlags;
Expand Down Expand Up @@ -68,12 +68,7 @@ public HashProviderCng(string hashAlgId, byte[] key)

public sealed override unsafe void AppendHashData(ReadOnlySpan<byte> source)
{
NTSTATUS ntStatus;
fixed (byte* pRgb = &source.DangerousGetPinnableReference())
{
ntStatus = Interop.BCrypt.BCryptHashData(_hHash, pRgb, source.Length, 0);

}
NTSTATUS ntStatus = Interop.BCrypt.BCryptHashData(_hHash, source, source.Length, 0);
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
{
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
Expand All @@ -82,30 +77,22 @@ public sealed override unsafe void AppendHashData(ReadOnlySpan<byte> source)

public sealed override byte[] FinalizeHashAndReset()
{
byte[] hash = new byte[_hashSize];
NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(_hHash, hash, hash.Length, 0);
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
{
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
}

ResetHashObject();
var hash = new byte[_hashSize];
bool success = TryFinalizeHashAndReset(hash, out int bytesWritten);
Debug.Assert(success);
Debug.Assert(hash.Length == bytesWritten);
return hash;
}

public override unsafe bool TryFinalizeHashAndReset(Span<byte> destination, out int bytesWritten)
public override bool TryFinalizeHashAndReset(Span<byte> destination, out int bytesWritten)
{
if (destination.Length < _hashSize)
{
bytesWritten = 0;
return false;
}

NTSTATUS ntStatus;
fixed (byte* ptr = &destination.DangerousGetPinnableReference())
{
ntStatus = Interop.BCrypt.BCryptFinishHash(_hHash, ptr, _hashSize, 0);
}
NTSTATUS ntStatus = Interop.BCrypt.BCryptFinishHash(_hHash, destination, _hashSize, 0);
if (ntStatus != NTSTATUS.STATUS_SUCCESS)
{
throw Interop.BCrypt.CreateCryptographicException(ntStatus);
Expand Down
20 changes: 18 additions & 2 deletions src/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Digest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,27 @@ internal static partial class AppleCrypto
[DllImport(Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_DigestCreate")]
internal static extern SafeDigestCtxHandle DigestCreate(PAL_HashAlgorithm algorithm, out int cbDigest);

internal static unsafe int DigestUpdate(SafeDigestCtxHandle ctx, ReadOnlySpan<byte> pbData, int cbData)
{
fixed (byte* pbDataPtr = &pbData.DangerousGetPinnableReference())
{
return DigestUpdate(ctx, pbDataPtr, cbData);
}
}

[DllImport(Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_DigestUpdate")]
internal static extern unsafe int DigestUpdate(SafeDigestCtxHandle ctx, byte* pbData, int cbData);
private static extern unsafe int DigestUpdate(SafeDigestCtxHandle ctx, byte* pbData, int cbData);

internal static unsafe int DigestFinal(SafeDigestCtxHandle ctx, Span<byte> pbOutput, int cbOutput)
{
fixed (byte* pbOutputPtr = &pbOutput.DangerousGetPinnableReference())
{
return DigestFinal(ctx, pbOutputPtr, cbOutput);
}
}

[DllImport(Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_DigestFinal")]
internal static extern unsafe int DigestFinal(SafeDigestCtxHandle ctx, byte* pbOutput, int cbOutput);
private static extern unsafe int DigestFinal(SafeDigestCtxHandle ctx, byte* pbOutput, int cbOutput);
}
}

Expand Down
22 changes: 19 additions & 3 deletions src/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.Hmac.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,29 @@ internal static partial class AppleCrypto
internal static extern SafeHmacHandle HmacCreate(PAL_HashAlgorithm algorithm, ref int cbDigest);

[DllImport(Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_HmacInit")]
internal static extern unsafe int HmacInit(SafeHmacHandle ctx, byte* pbKey, int cbKey);
internal static extern unsafe int HmacInit(SafeHmacHandle ctx, [In] byte[] pbKey, int cbKey);

internal static unsafe int HmacUpdate(SafeHmacHandle ctx, ReadOnlySpan<byte> pbData, int cbData)
{
fixed (byte* pbDataPtr = &pbData.DangerousGetPinnableReference())
{
return HmacUpdate(ctx, pbDataPtr, cbData);
}
}

[DllImport(Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_HmacUpdate")]
internal static extern unsafe int HmacUpdate(SafeHmacHandle ctx, byte* pbData, int cbData);
private static extern unsafe int HmacUpdate(SafeHmacHandle ctx, byte* pbData, int cbData);

internal static unsafe int HmacFinal(SafeHmacHandle ctx, ReadOnlySpan<byte> pbOutput, int cbOutput)
{
fixed (byte* pbOutputPtr = &pbOutput.DangerousGetPinnableReference())
{
return HmacFinal(ctx, pbOutputPtr, cbOutput);
}
}

[DllImport(Libraries.AppleCryptoNative, EntryPoint = "AppleCryptoNative_HmacFinal")]
internal static extern unsafe int HmacFinal(SafeHmacHandle ctx, byte* pbOutput, int cbOutput);
private static extern unsafe int HmacFinal(SafeHmacHandle ctx, byte* pbOutput, int cbOutput);
}
}

Expand Down
10 changes: 9 additions & 1 deletion src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EVP.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@ internal static partial class Crypto
[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpDigestReset")]
internal extern static int EvpDigestReset(SafeEvpMdCtxHandle ctx, IntPtr type);

internal static unsafe int EvpDigestUpdate(SafeEvpMdCtxHandle ctx, ReadOnlySpan<byte> d, int cnt)
{
fixed (byte* dPtr = &d.DangerousGetPinnableReference())
{
return EvpDigestUpdate(ctx, dPtr, cnt);
}
}

[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpDigestUpdate")]
internal extern static unsafe int EvpDigestUpdate(SafeEvpMdCtxHandle ctx, byte* d, int cnt);
private extern static unsafe int EvpDigestUpdate(SafeEvpMdCtxHandle ctx, byte* d, int cnt);

[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_EvpDigestFinalEx")]
internal extern static unsafe int EvpDigestFinalEx(SafeEvpMdCtxHandle ctx, byte* md, ref uint s);
Expand Down
10 changes: 9 additions & 1 deletion src/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Hmac.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,16 @@ internal static partial class Crypto
[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_HmacReset")]
internal extern static int HmacReset(SafeHmacCtxHandle ctx);

internal static unsafe int HmacUpdate(SafeHmacCtxHandle ctx, ReadOnlySpan<byte> data, int len)
{
fixed (byte* dataPtr = &data.DangerousGetPinnableReference())
{
return HmacUpdate(ctx, dataPtr, len);
}
}

[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_HmacUpdate")]
internal extern static unsafe int HmacUpdate(SafeHmacCtxHandle ctx, byte* data, int len);
private extern static unsafe int HmacUpdate(SafeHmacCtxHandle ctx, byte* data, int len);

[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_HmacFinal")]
internal extern static unsafe int HmacFinal(SafeHmacCtxHandle ctx, byte* data, ref int len);
Expand Down
10 changes: 5 additions & 5 deletions src/Common/src/Interop/Windows/BCrypt/Interop.BCryptFinishHash.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,23 @@
// 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;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;

internal partial class Interop
{
internal partial class BCrypt
{
internal static unsafe NTSTATUS BCryptFinishHash(SafeBCryptHashHandle hHash, byte[] pbOutput, int cbOutput, int dwFlags)
internal static unsafe NTSTATUS BCryptFinishHash(SafeBCryptHashHandle hHash, Span<byte> pbOutput, int cbOutput, int dwFlags)
{
fixed (byte* ptr = pbOutput)
fixed (byte* pbOutputPtr = &pbOutput.DangerousGetPinnableReference())
{
return BCryptFinishHash(hHash, ptr, cbOutput, dwFlags);
return BCryptFinishHash(hHash, pbOutputPtr, cbOutput, dwFlags);
}
}

[DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)]
internal static unsafe extern NTSTATUS BCryptFinishHash(SafeBCryptHashHandle hHash, byte* pbOutput, int cbOutput, int dwFlags);
private static unsafe extern NTSTATUS BCryptFinishHash(SafeBCryptHashHandle hHash, byte* pbOutput, int cbOutput, int dwFlags);
}
}

11 changes: 9 additions & 2 deletions src/Common/src/Interop/Windows/BCrypt/Interop.BCryptHashData.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Diagnostics;
using System.Runtime.InteropServices;

using Microsoft.Win32.SafeHandles;
Expand All @@ -12,8 +11,16 @@ internal partial class Interop
{
internal partial class BCrypt
{
internal static unsafe NTSTATUS BCryptHashData(SafeBCryptHashHandle hHash, ReadOnlySpan<byte> pbInput, int cbInput, int dwFlags)
{
fixed (byte* pbInputPtr = &pbInput.DangerousGetPinnableReference())
{
return BCryptHashData(hHash, pbInputPtr, cbInput, dwFlags);
}
}

[DllImport(Libraries.BCrypt, CharSet = CharSet.Unicode)]
internal static extern unsafe NTSTATUS BCryptHashData(SafeBCryptHashHandle hHash, byte* pbInput, int cbInput, int dwFlags);
private static extern unsafe NTSTATUS BCryptHashData(SafeBCryptHashHandle hHash, byte* pbInput, int cbInput, int dwFlags);
}
}

98 changes: 22 additions & 76 deletions ...m.Security.Cryptography.Algorithms/src/Internal/Cryptography/HashProviderDispenser.OSX.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,36 +86,22 @@ internal AppleHmacProvider(Interop.AppleCrypto.PAL_HashAlgorithm algorithm, byte
HashSizeInBytes = hashSizeInBytes;
}

public override unsafe void AppendHashData(ReadOnlySpan<byte> data)
public override void AppendHashData(ReadOnlySpan<byte> data)
{
if (!_running)
{
SetKey();
}

int ret;

fixed (byte* pData = &data.DangerousGetPinnableReference())
{
ret = Interop.AppleCrypto.HmacUpdate(_ctx, pData, data.Length);
}

if (ret != 1)
if (Interop.AppleCrypto.HmacUpdate(_ctx, data, data.Length) != 1)
{
throw new CryptographicException();
}
}

private unsafe void SetKey()
private void SetKey()
{
int ret;

fixed (byte* pbKey = _key)
{
ret = Interop.AppleCrypto.HmacInit(_ctx, pbKey, _key.Length);
}

if (ret != 1)
if (Interop.AppleCrypto.HmacInit(_ctx, _key, _key.Length) != 1)
{
throw new CryptographicException();
}
Expand All @@ -125,47 +111,27 @@ private unsafe void SetKey()

public override unsafe byte[] FinalizeHashAndReset()
{
if (!_running)
{
SetKey();
}

byte[] output = new byte[HashSizeInBytes];
int ret;

fixed (byte* pbOutput = output)
{
ret = Interop.AppleCrypto.HmacFinal(_ctx, pbOutput, output.Length);
}

if (ret != 1)
{
throw new CryptographicException();
}

_running = false;
var output = new byte[HashSizeInBytes];
bool success = TryFinalizeHashAndReset(output, out int bytesWritten);
Debug.Assert(success);
Debug.Assert(bytesWritten == output.Length);
return output;
}

public override unsafe bool TryFinalizeHashAndReset(Span<byte> destination, out int bytesWritten)
public override bool TryFinalizeHashAndReset(Span<byte> destination, out int bytesWritten)
{
if (!_running)
{
SetKey();
}

if (destination.Length < HashSizeInBytes)
{
bytesWritten = 0;
return false;
}

int ret;
fixed (byte* pbOutput = &destination.DangerousGetPinnableReference())
if (!_running)
{
ret = Interop.AppleCrypto.HmacFinal(_ctx, pbOutput, destination.Length);
SetKey();
}
if (ret != 1)

if (Interop.AppleCrypto.HmacFinal(_ctx, destination, destination.Length) != 1)
{
throw new CryptographicException();
}
Expand Down Expand Up @@ -214,54 +180,34 @@ internal AppleDigestProvider(Interop.AppleCrypto.PAL_HashAlgorithm algorithm)
HashSizeInBytes = hashSizeInBytes;
}

public override unsafe void AppendHashData(ReadOnlySpan<byte> data)
public override void AppendHashData(ReadOnlySpan<byte> data)
{
int ret;

fixed (byte* pData = &data.DangerousGetPinnableReference())
{
ret = Interop.AppleCrypto.DigestUpdate(_ctx, pData, data.Length);
}

int ret = Interop.AppleCrypto.DigestUpdate(_ctx, data, data.Length);
if (ret != 1)
{
Debug.Assert(ret == 0, $"DigestUpdate return value {ret} was not 0 or 1");
throw new CryptographicException();
}
}

public override unsafe byte[] FinalizeHashAndReset()
public override byte[] FinalizeHashAndReset()
{
byte[] hash = new byte[HashSizeInBytes];
int ret;

fixed (byte* pHash = hash)
{
ret = Interop.AppleCrypto.DigestFinal(_ctx, pHash, hash.Length);
}

if (ret != 1)
{
Debug.Assert(ret == 0, $"DigestFinal return value {ret} was not 0 or 1");
throw new CryptographicException();
}

var hash = new byte[HashSizeInBytes];
bool success = TryFinalizeHashAndReset(hash, out int bytesWritten);
Debug.Assert(success);
Debug.Assert(bytesWritten == hash.Length);
return hash;
}

public override unsafe bool TryFinalizeHashAndReset(Span<byte> destination, out int bytesWritten)
public override bool TryFinalizeHashAndReset(Span<byte> destination, out int bytesWritten)
{
if (destination.Length < HashSizeInBytes)
{
bytesWritten = 0;
return false;
}

int ret;
fixed (byte* pHash = &destination.DangerousGetPinnableReference())
{
ret = Interop.AppleCrypto.DigestFinal(_ctx, pHash, destination.Length);
}
int ret = Interop.AppleCrypto.DigestFinal(_ctx, destination, destination.Length);
if (ret != 1)
{
Debug.Assert(ret == 0, $"DigestFinal return value {ret} was not 0 or 1");
Expand Down
Loading

0 comments on commit 0597b42

Please sign in to comment.