bcgit / bc-csharp

BouncyCastle.NET Cryptography Library (Mirror)
https://www.bouncycastle.org/csharp
MIT License
1.67k stars 556 forks source link

Inconsistent scrypt output with .NET 8 #493

Open aayjaychan opened 1 year ago

aayjaychan commented 1 year ago

Program.cs:

using System;

using Org.BouncyCastle.Crypto.Generators;

byte[] secret = "secret"u8.ToArray();
byte[] salt = "salt"u8.ToArray();

for (int i = 0; i < 3; i += 1)
{
    byte[] hash = SCrypt.Generate(
        P: secret,
        S: salt,
        N: (int)Math.Pow(2, 17),
        r: 8,
        p: 1,
        dkLen: 64);

    Console.WriteLine(Convert.ToBase64String(hash));
}

ScryptTest.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="BouncyCastle.Cryptography" Version="2.2.1" />
  </ItemGroup>

</Project>

Expected Result (when <TargetFramework> is net7.0):

TQ/d9EkSgAn+BqwPDyKgp5AY4wNNEk96g6yMh3DeZ3L0f+MvHWMpgpNfH6Kmk4ItOFUSVbxY8HLUgpdPlSy++Q==
TQ/d9EkSgAn+BqwPDyKgp5AY4wNNEk96g6yMh3DeZ3L0f+MvHWMpgpNfH6Kmk4ItOFUSVbxY8HLUgpdPlSy++Q==
TQ/d9EkSgAn+BqwPDyKgp5AY4wNNEk96g6yMh3DeZ3L0f+MvHWMpgpNfH6Kmk4ItOFUSVbxY8HLUgpdPlSy++Q==

Actual result (when <TargetFramework> is net8.0):

QyWiiA4Lx83+uIa7yXoCeP5aG3MFVmlnTYQVw5Z2AJYEno1rfMpLb7+BtppRg9DO0ATcQ3xkod7B+rYXoh7xbw==
Lysd/+QvSLOlygtIDqLl++5n+4c2Yn42zHlI2DY9pvzMnHq4iLMb0rqvwhb9dEDql2h+lm4UFUVjmEHXB/EBug==
Lysd/+QvSLOlygtIDqLl++5n+4c2Yn42zHlI2DY9pvzMnHq4iLMb0rqvwhb9dEDql2h+lm4UFUVjmEHXB/EBug==

The result of the first run is different every time, and the rest doesn't match the output of .NET 7.

>dotnet --info
.NET SDK:
 Version:   8.0.100-rc.2.23502.2
 Commit:    0abacfc2b6

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.19044
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.100-rc.2.23502.2\

.NET workloads installed:
There are no installed workloads to display.

Host:
  Version:      8.0.0-rc.2.23479.6
  Architecture: x64
  Commit:       0b25e38ad3

.NET SDKs installed:
  6.0.414 [C:\Program Files\dotnet\sdk]
  7.0.111 [C:\Program Files\dotnet\sdk]
  8.0.100-rc.2.23502.2 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0-rc.2.23480.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.22 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0-rc.2.23479.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.22 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.11 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.0-rc.2.23479.10 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  None

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
peterdettman commented 9 months ago

Thanks for the report. I am able to reproduce it in small examples like this, though our scrypt tests pass as part of a full test suite run (but test vectors fail if run in isolation). The problem persists even when the library itself is compiled for net8.0. It doesn't seem to happen with debug builds.

From a brief investigation it appears related to the intrinsics-based code in Salsa20Engine.SalsaCore (commenting that out lets everything work normally). Given all the above I would suspect some sort of bug in the runtime (probably the JIT), but I will look into it a bit further.

tamasgonczi-tresorit commented 6 months ago

@peterdettman do you have any update on this? we have the same issue

adambartha commented 6 months ago

We were also able the reproduce this issue and tried to solve it via an initial static call to the generator function inside a utility class. This seems to somewhat consolidate the output, although it is still inconsistent in automated test context. We are looking forward to any update regarding this issue.