kmaragon / Konscious.Security.Cryptography

MIT License
202 stars 20 forks source link

Getting OutOfMemory-Exception #56

Open vveena12 opened 10 months ago

vveena12 commented 10 months ago

Hi,

I am using Argon2i hashing for storing user password. Thus, also using it while doing login into application. When many users simultanesously login into system, the GetBytes(int) method throws an OutOfMemoryException.

For identifying the problem, I did hashing parallerly on a thread. After about 35 or 40 occurrances, it starts throwing OutOfMemoryException. Below is my test code.

            System.Threading.Tasks.Parallel.For(0, 42, i =>
            {
                byte[] buf = new byte[10];
                (new RNGCryptoServiceProvider()).GetBytes(buf);
                var salstr = Convert.ToBase64String(buf);

                var argon2 = new Argon2id(Encoding.UTF8.GetBytes(password));
                argon2.Salt = Convert.FromBase64String(saltstr);
                argon2.DegreeOfParallelism = 8;
                argon2.Iterations = 2;
                argon2.MemorySize = 256000;
                var hash = argon2.GetBytes(32);
            });

Here's the exception:

--- Message: One or more errors occurred. --- Stack Trace: at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions) at System.Threading.Tasks.Task1.GetResultCore(Boolean waitCompletionNotification) at System.Threading.Tasks.Task1.get_Result() at Konscious.Security.Cryptography.Argon2.GetBytes(Int32 bc) in C:\projects\konscious-security-cryptography\Konscious.Security.Cryptography.Argon2\Argon2.cs:line 40 at ArgonHashingTest.KonsciousHash.<>cDisplayClass1_0.b0(Int32 i) in C:\Users\veenakh\source\repos\ArgonHashingTest\ArgonHashingTest\KonsciousHash.cs:line 107 at System.Threading.Tasks.Parallel.<>cDisplayClass17_0`1.b1() at System.Threading.Tasks.Task.InnerInvoke() at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask) at System.Threading.Tasks.Task.<>c__DisplayClass176_0.b__0(Object )

--- Message: Exception of type 'System.OutOfMemoryException' was thrown. --- Stack Trace: at Konscious.Security.Cryptography.Argon2Lane..ctor(Int32 blockCount) in C:\projects\konscious-security-cryptography\Konscious.Security.Cryptography.Argon2\Argon2Lane.cs:line 9 at Konscious.Security.Cryptography.Argon2Core.d32.MoveNext() in C:\projects\konscious-security-cryptography\Konscious.Security.Cryptography.Argon2\Argon2Core.cs:line 163 at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Konscious.Security.Cryptography.Argon2Core.d27.MoveNext() in C:\projects\konscious-security-cryptography\Konscious.Security.Cryptography.Argon2\Argon2Core.cs:line 34 at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Konscious.Security.Cryptography.Argon2.<>c__DisplayClass2_0.<b__0>d.MoveNext() in C:\projects\konscious-security-cryptography\Konscious.Security.Cryptography.Argon2\Argon2.cs:line 40

There was in past similar issue https://github.com/kmaragon/Konscious.Security.Cryptography/issues/12

Please help me resolve this issue, so that we can fix this problem in our application ASAP.

bobarune commented 10 months ago

Your issue appears to be the same as that previous issue: allocating unmanaged resources without disposing of them when you're done. Two of the items you are instantiating implement the interface IDisposable through inherited classes.

System.Security.Cryptography.RNGCryptoServiceProvider -> System.Security.Cryptography.RandomNumberGenerator -> System.IDisposable

Konscious.Security.Cryptography.Argon2id -> Konscious.Security.Cryptography.Argon2 -> System.Security.Cryptography.DeriveBytes -> System.IDisposable

Use a using statement.

vveena12 commented 9 months ago

Thanks for your reply. As per your suggestion, I changed my code to use "using", It did improved a bit but not much. Infact, when I used "using" for RNGCryptoServiceProvider, it improved a bit (i.e. error starts coming after 45 occurrences), but after that change for Argon2 does not have any impact.

           System.Threading.Tasks.Parallel.For(0, cnt, i =>
            {
                byte[] buf = null;
                using (RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider())
                {
                    buf = new byte[10];
                    rng.GetBytes(buf);
                }

                var saltstr = Convert.ToBase64String(buf);

                byte[] hash = null;
                using (Argon2id argon2 = new Argon2id(Encoding.UTF8.GetBytes(password)))
                {
                    argon2.Salt = Convert.FromBase64String(saltstr);
                    argon2.DegreeOfParallelism = 8;
                    argon2.Iterations = 2;
                    argon2.MemorySize = 256000;
                    hash = argon21.GetBytes(32);
                }
            });
SparkDustJoe commented 9 months ago

Is it possible that you're using settings that are just aggressively consuming memory? If I'm remembering correctly from the standard (C++ implementation) that Parallelism/Lanes uses the Memory Size TIMES Lanes, AND Memory Size is in Kibibytes, so in your case, 256000kib X 8 Lanes. If I'm understanding the parameters correctly, that's approx. 1.95Gibibytes PER INSTANCE! @kmaragon am I remembering that correctly? Based on a cursory look through the code, it's an array of 256000 (your value) * 128 ulong's (64 bits each), which is 250Mibibytes right there, then 8 lanes of that (based on the initialization of the ArgonLane class). If 45 people rapidly log in and memory isn't immediately released by the Garbage Collector, that's up to 88Gibibytes right there depending on how far apart they log in.

NOTE: Changing any parameters WILL change the resulting hash result, so that would cause future login attempts by established users to fail!