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.

Sign up for GitHub

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

Enable Rfc2898DeriveBytes on Browser WASM #71768

Merged
merged 4 commits into from
Jul 9, 2022
Merged

Enable Rfc2898DeriveBytes on Browser WASM #71768

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6906aa2
Enable Rfc2898DeriveBytes on Browser WASM
eerhardt Jul 6, 2022
7e3672f
PR feedback
eerhardt Jul 7, 2022
31d287e
Mark HKDF as supported on Browser and enable tests
eerhardt Jul 7, 2022
559eef9
Fix tests to exclude MD5
eerhardt Jul 8, 2022
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
11 changes: 11 additions & 0 deletions ...n/src/Interop/Browser/System.Security.Cryptography.Native.Browser/Interop.SubtleCrypto.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,16 @@ internal static unsafe partial int EncryptDecrypt(
int input_len,
byte* output_buffer,
int output_len);

[LibraryImport(Libraries.CryptoNative, EntryPoint = "SystemCryptoNativeBrowser_DeriveBits")]
internal static unsafe partial int DeriveBits(
byte* password_buffer,
int password_len,
byte* salt_buffer,
int salt_len,
int iterations,
SimpleDigest hashAlgorithm,
byte* output_buffer,
int output_len);
}
}
41 changes: 19 additions & 22 deletions src/libraries/Common/src/System/Security/Cryptography/HashOneShotHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,28 +98,25 @@ internal static int MacData(
ReadOnlySpan<byte> source,
Span<byte> destination)
{
if (Helpers.HasHMAC)
{
if (hashAlgorithm == HashAlgorithmName.SHA256)
{
return HMACSHA256.HashData(key, source, destination);
}
else if (hashAlgorithm == HashAlgorithmName.SHA1)
{
return HMACSHA1.HashData(key, source, destination);
}
else if (hashAlgorithm == HashAlgorithmName.SHA512)
{
return HMACSHA512.HashData(key, source, destination);
}
else if (hashAlgorithm == HashAlgorithmName.SHA384)
{
return HMACSHA384.HashData(key, source, destination);
}
else if (hashAlgorithm == HashAlgorithmName.MD5)
{
return HMACMD5.HashData(key, source, destination);
}
if (hashAlgorithm == HashAlgorithmName.SHA256)
{
return HMACSHA256.HashData(key, source, destination);
}
else if (hashAlgorithm == HashAlgorithmName.SHA1)
{
return HMACSHA1.HashData(key, source, destination);
}
else if (hashAlgorithm == HashAlgorithmName.SHA512)
{
return HMACSHA512.HashData(key, source, destination);
}
else if (hashAlgorithm == HashAlgorithmName.SHA384)
{
return HMACSHA384.HashData(key, source, destination);
}
else if (Helpers.HasMD5 && hashAlgorithm == HashAlgorithmName.MD5)
{
return HMACMD5.HashData(key, source, destination);
}

throw new CryptographicException(SR.Format(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithm.Name));
Expand Down
12 changes: 2 additions & 10 deletions src/libraries/Common/src/System/Security/Cryptography/Helpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,7 @@ namespace Internal.Cryptography
internal static partial class Helpers
{
[UnsupportedOSPlatformGuard("browser")]
internal static bool HasSymmetricEncryption { get; } =
#if NETCOREAPP
!OperatingSystem.IsBrowser();
#else
true;
#endif

[UnsupportedOSPlatformGuard("browser")]
internal static bool HasHMAC { get; } =
internal static bool HasNonAesSymmetricEncryption =>
#if NETCOREAPP
!OperatingSystem.IsBrowser();
#else
Expand All @@ -37,7 +29,7 @@ internal static partial class Helpers
#if NETCOREAPP
[UnsupportedOSPlatformGuard("android")]
[UnsupportedOSPlatformGuard("browser")]
public static bool IsRC2Supported => !OperatingSystem.IsAndroid();
public static bool IsRC2Supported => !OperatingSystem.IsAndroid() && !OperatingSystem.IsBrowser();
#else
public static bool IsRC2Supported => true;
#endif
Expand Down
64 changes: 22 additions & 42 deletions src/libraries/Common/src/System/Security/Cryptography/PasswordBasedEncryption.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,29 @@ internal static unsafe int Decrypt(
{
Debug.Assert(destination.Length >= encryptedData.Length);

if (!Helpers.HasSymmetricEncryption)
// Don't check that algorithmIdentifier.Parameters is set here.
// Maybe some future PBES3 will have one with a default.

if (algorithmIdentifier.Algorithm == Oids.PasswordBasedEncryptionScheme2)
{
return Pbes2Decrypt(
algorithmIdentifier.Parameters,
password,
passwordBytes,
encryptedData,
destination);
}

if (!Helpers.HasNonAesSymmetricEncryption)
{
throw new CryptographicException(
SR.Format(
SR.Cryptography_UnknownAlgorithmIdentifier,
algorithmIdentifier.Algorithm));
}

// Don't check that algorithmIdentifier.Parameters is set here.
// Maybe some future PBES3 will have one with a default.

HashAlgorithmName digestAlgorithmName;
SymmetricAlgorithm? cipher = null;
SymmetricAlgorithm cipher;

bool pkcs12 = false;

Expand Down Expand Up @@ -131,13 +141,6 @@ internal static unsafe int Decrypt(
cipher.KeySize = 40;
pkcs12 = true;
break;
case Oids.PasswordBasedEncryptionScheme2:
return Pbes2Decrypt(
algorithmIdentifier.Parameters,
password,
passwordBytes,
encryptedData,
destination);
default:
throw new CryptographicException(
SR.Format(
Expand All @@ -146,7 +149,6 @@ internal static unsafe int Decrypt(
}

Debug.Assert(digestAlgorithmName.Name != null);
Debug.Assert(cipher != null);

using (cipher)
{
Expand Down Expand Up @@ -237,14 +239,6 @@ internal static void InitiateEncryption(
{
Debug.Assert(pbeParameters != null);

if (!Helpers.HasSymmetricEncryption)
{
throw new CryptographicException(
SR.Format(
SR.Cryptography_UnknownAlgorithmIdentifier,
pbeParameters.EncryptionAlgorithm));
}

isPkcs12 = false;

switch (pbeParameters.EncryptionAlgorithm)
Expand All @@ -264,7 +258,7 @@ internal static void InitiateEncryption(
cipher.KeySize = 256;
encryptionAlgorithmOid = Oids.Aes256Cbc;
break;
case PbeEncryptionAlgorithm.TripleDes3KeyPkcs12:
case PbeEncryptionAlgorithm.TripleDes3KeyPkcs12 when Helpers.HasNonAesSymmetricEncryption:
cipher = TripleDES.Create();
cipher.KeySize = 192;
encryptionAlgorithmOid = Oids.Pkcs12PbeWithShaAnd3Key3Des;
Expand Down Expand Up @@ -393,12 +387,6 @@ internal static unsafe int Encrypt(
Debug.Assert(pwdTmpBytes!.Length == 0);
}

if (!Helpers.HasHMAC)
{
throw new CryptographicException(
SR.Format(SR.Cryptography_AlgorithmNotSupported, "HMAC" + prf.Name));
}

using (var pbkdf2 = new Rfc2898DeriveBytes(pwdTmpBytes, salt.ToArray(), iterationCount, prf))
{
derivedKey = pbkdf2.GetBytes(keySizeBytes);
Expand Down Expand Up @@ -540,8 +528,6 @@ private static unsafe int Pbes2Decrypt(
Rfc2898DeriveBytes pbkdf2 =
OpenPbkdf2(password, pbes2Params.KeyDerivationFunc.Parameters, out int? requestedKeyLength);

Debug.Assert(Helpers.HasHMAC);

using (pbkdf2)
{
// The biggest block size (for IV) we support is AES (128-bit / 16 byte)
Expand Down Expand Up @@ -580,12 +566,6 @@ private static SymmetricAlgorithm OpenCipher(
{
string? algId = encryptionScheme.Algorithm;

if (!Helpers.HasSymmetricEncryption)
{
throw new CryptographicException(
SR.Format(SR.Cryptography_AlgorithmNotSupported, algId));
}

if (algId == Oids.Aes128Cbc ||
algId == Oids.Aes192Cbc ||
algId == Oids.Aes256Cbc)
Expand Down Expand Up @@ -624,6 +604,12 @@ private static SymmetricAlgorithm OpenCipher(
return aes;
}

if (!Helpers.HasNonAesSymmetricEncryption)
{
throw new CryptographicException(
SR.Format(SR.Cryptography_AlgorithmNotSupported, algId));
}

if (algId == Oids.TripleDesCbc)
{
// https://tools.ietf.org/html/rfc8018#appendix-B.2.2
Expand Down Expand Up @@ -777,12 +763,6 @@ private static unsafe Rfc2898DeriveBytes OpenPbkdf2(
throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding);
}

if (!Helpers.HasHMAC)
{
throw new CryptographicException(
SR.Format(SR.Cryptography_AlgorithmNotSupported, "HMAC" + prf.Name));
}

int iterationCount = NormalizeIterationCount(pbkdf2Params.IterationCount);
ReadOnlyMemory<byte> saltMemory = pbkdf2Params.Salt.Specified.Value;

Expand Down
2 changes: 0 additions & 2 deletions src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1297,7 +1297,6 @@ protected virtual void HashCore(System.ReadOnlySpan<byte> source) { }
public override string ToString() { throw null; }
public static bool TryFromOid(string oidValue, out System.Security.Cryptography.HashAlgorithmName value) { throw null; }
}
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
public static partial class HKDF
{
public static byte[] DeriveKey(System.Security.Cryptography.HashAlgorithmName hashAlgorithmName, byte[] ikm, int outputLength, byte[]? salt = null, byte[]? info = null) { throw null; }
Expand Down Expand Up @@ -1708,7 +1707,6 @@ public RC2CryptoServiceProvider() { }
public override void GenerateIV() { }
public override void GenerateKey() { }
}
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
public partial class Rfc2898DeriveBytes : System.Security.Cryptography.DeriveBytes
{
[System.ObsoleteAttribute("The default hash algorithm and iteration counts in Rfc2898DeriveBytes constructors are outdated and insecure. Use a constructor that accepts the hash algorithm and the number of iterations.", DiagnosticId="SYSLIB0041", UrlFormat="https://aka.ms/dotnet-warnings/{0}")]
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@
<Compile Include="System\Security\Cryptography\OidLookup.NoFallback.cs" />
<Compile Include="System\Security\Cryptography\OpenSsl.NotSupported.cs" />
<Compile Include="System\Security\Cryptography\PasswordDeriveBytes.NotSupported.cs" />
<Compile Include="System\Security\Cryptography\Pbkdf2Implementation.Managed.cs" />
<Compile Include="System\Security\Cryptography\Pbkdf2Implementation.Browser.cs" />
<Compile Include="System\Security\Cryptography\RandomNumberGeneratorImplementation.Browser.cs" />
<Compile Include="System\Security\Cryptography\RC2CryptoServiceProvider.NotSupported.cs" />
<Compile Include="System\Security\Cryptography\RC2Implementation.NotSupported.cs" />
Expand Down
2 changes: 2 additions & 0 deletions ...ecurity.Cryptography/src/System/Security/Cryptography/AesSubtleCryptoTransform.Browser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ private unsafe int EncryptDecrypt(ReadOnlySpan<byte> input, Span<byte> output)
pOutput, output.Length);

if (bytesWritten < 0)
{
throw new CryptographicException(SR.Format(SR.Unknown_SubtleCrypto_Error, bytesWritten));
}

return bytesWritten;
}
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HKDF.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Diagnostics;
using System.Runtime.Versioning;
using Internal.Cryptography;

namespace System.Security.Cryptography
{
Expand All @@ -14,7 +15,6 @@ namespace System.Security.Cryptography
/// phase to be skipped, and the master key to be used directly as the pseudorandom key.
/// See <a href="https://tools.ietf.org/html/rfc5869">RFC5869</a> for more information.
/// </remarks>
[UnsupportedOSPlatform("browser")]
public static class HKDF
{
/// <summary>
Expand Down Expand Up @@ -290,7 +290,9 @@ private static int HashLength(HashAlgorithmName hashAlgorithmName)
}
else if (hashAlgorithmName == HashAlgorithmName.MD5)
{
#pragma warning disable CA1416 // HMACMD5 is unsupported on browser, throwing is handled later when making the HMAC call
return HMACMD5.HashSizeInBytes;
#pragma warning restore CA1416
}
else
{
Expand Down
2 changes: 2 additions & 0 deletions ...Security.Cryptography/src/System/Security/Cryptography/HMACHashProvider.Browser.Native.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ private static unsafe void Sign(SimpleDigest hashName, ReadOnlySpan<byte> key, R
{
int res = Interop.BrowserCrypto.Sign(hashName, k, key.Length, src, data.Length, dest, destination.Length);
if (res != 0)
{
throw new CryptographicException(SR.Format(SR.Unknown_SubtleCrypto_Error, res));
}
}
}

Expand Down
79 changes: 79 additions & 0 deletions ...em.Security.Cryptography/src/System/Security/Cryptography/Pbkdf2Implementation.Browser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using Internal.Cryptography;

using SimpleDigest = Interop.BrowserCrypto.SimpleDigest;

namespace System.Security.Cryptography
{
internal static partial class Pbkdf2Implementation
{
public static void Fill(
ReadOnlySpan<byte> password,
ReadOnlySpan<byte> salt,
int iterations,
HashAlgorithmName hashAlgorithmName,
Span<byte> destination)
{
Debug.Assert(!destination.IsEmpty);
Debug.Assert(hashAlgorithmName.Name is not null);

if (Interop.BrowserCrypto.CanUseSubtleCrypto)
{
FillSubtleCrypto(password, salt, iterations, hashAlgorithmName, destination);
}
else
{
FillManaged(password, salt, iterations, hashAlgorithmName, destination);
}
}

private static unsafe void FillSubtleCrypto(
ReadOnlySpan<byte> password,
ReadOnlySpan<byte> salt,
int iterations,
HashAlgorithmName hashAlgorithmName,
Span<byte> destination)
{
(SimpleDigest hashName, _) = SHANativeHashProvider.HashAlgorithmToPal(hashAlgorithmName.Name!);

fixed (byte* pPassword = password)
fixed (byte* pSalt = salt)
fixed (byte* pDestination = destination)
{
int result = Interop.BrowserCrypto.DeriveBits(
pPassword, password.Length,
pSalt, salt.Length,
iterations,
hashName,
pDestination, destination.Length);

if (result != 0)
{
throw new CryptographicException(SR.Format(SR.Unknown_SubtleCrypto_Error, result));
}
}
}

private static void FillManaged(
ReadOnlySpan<byte> password,
ReadOnlySpan<byte> salt,
int iterations,
HashAlgorithmName hashAlgorithmName,
Span<byte> destination)
{
using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(
password.ToArray(),
salt.ToArray(),
Copy link
Member

Choose a reason for hiding this comment

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

There's no way to stay in the span universe all the way down? Are we accepting arrays from public entrypoints, passing them around as span, and then ToArray'ing here?

Copy link
Member

@vcsjones vcsjones Jul 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It's possible but we've avoided optimizing the non-one-shot implementation of PBKDF2 when we've talked about it in the past.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this is the way the existing "mobile" platforms are implementing this functionality:

runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Pbkdf2Implementation.Managed.cs

Lines 27 to 36 in 6a02d5d

using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(
password.ToArray(),
salt.ToArray(),
iterations,
hashAlgorithmName,
clearPassword: true))
{
byte[] result = deriveBytes.GetBytes(destination.Length);
result.AsSpan().CopyTo(destination);
}

which is used by Android and non-macOS AppleCrypto (iOS, tvOS, etc.).

There's no way to stay in the span universe all the way down?

Not without some decent refactoring to Rfc2898DeriveBytes in order to make the managed algorithm completely "one shot", and then build the instance based Rfc2898DeriveBytes methods on top of it.

Are we accepting arrays from public entrypoints, passing them around as span, and then ToArray'ing here?

In one overload, yes, and in most overloads no. This method is called from these public end-points:

runtime/src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs

Lines 1734 to 1739 in 6a02d5d

public static byte[] Pbkdf2(byte[] password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; }
public static byte[] Pbkdf2(System.ReadOnlySpan<byte> password, System.ReadOnlySpan<byte> salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; }
public static void Pbkdf2(System.ReadOnlySpan<byte> password, System.ReadOnlySpan<byte> salt, System.Span<byte> destination, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
public static byte[] Pbkdf2(System.ReadOnlySpan<char> password, System.ReadOnlySpan<byte> salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; }
public static void Pbkdf2(System.ReadOnlySpan<char> password, System.ReadOnlySpan<byte> salt, System.Span<byte> destination, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm) { }
public static byte[] Pbkdf2(string password, byte[] salt, int iterations, System.Security.Cryptography.HashAlgorithmName hashAlgorithm, int outputLength) { throw null; }

Only the top overload is taking 2 byte[]s, passing them through as spans, and then ToArray'ing here. The bottom overload takes a string, which turns into UTF8 bytes in either a rented array or stackalloc'd span, and then passed in here.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be simpler to implement a one shot in managed code with IncrementalHash instead of trying to get the streaming PBKDF2 to allocate less. I have a branch with this. Happy to upstream it as a follow up to this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a branch with this. Happy to upstream it as a follow up to this.

I think that would be great!

iterations,
hashAlgorithmName,
clearPassword: true))
{
byte[] result = deriveBytes.GetBytes(destination.Length);
result.AsSpan().CopyTo(destination);
}
}
}
}
6 changes: 0 additions & 6 deletions ...em.Security.Cryptography/src/System/Security/Cryptography/Pbkdf2Implementation.Managed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ public static unsafe void Fill(
Debug.Assert(!destination.IsEmpty);
Debug.Assert(hashAlgorithmName.Name is not null);

if (!Helpers.HasHMAC)
{
throw new CryptographicException(
SR.Format(SR.Cryptography_AlgorithmNotSupported, "HMAC" + hashAlgorithmName.Name));
}

using (Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(
password.ToArray(),
salt.ToArray(),
Expand Down
Loading