kmaragon / Konscious.Security.Cryptography

MIT License
201 stars 20 forks source link

When used on a .Net Core Console app, a simple test throws exceptions on GetBytes() #3

Closed SparkDustJoe closed 7 years ago

SparkDustJoe commented 7 years ago

An unhandled exception of type 'System.AggregateException' occurred in System.Private.CoreLib.ni.dll

Additional information: One or more errors occurred.

Inner Exception: {System.DivideByZeroException: Attempted to divide by zero. at Konscious.Security.Cryptography.Argon2Core.d32.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at Konscious.Security.Cryptography.Argon2Core.d27.MoveNext()}

Code: string testPwd = "B@ll0ck$"; Guid salt = Guid.NewGuid(); myArg = new Argon2d(UTF8Encoding.UTF8.GetBytes(testPwd)); myArg.Salt = salt.ToByteArray(); byte[] hashResult = myArg.GetBytes(32); <----Thrown here Console.WriteLine(Convert.ToString(hashResult)); Console.ReadKey(true);

kmaragon commented 7 years ago

What are the values for Iterations, MemorySize, and DegreeOfParallelism?

kmaragon commented 7 years ago

Fwiw an outstanding issue is that if any of these are 0 it won't throw an exception saying it can't be zero. Except sometimes in the form you describe. Which if that's what you're facing, sorry for the crappy user experience. I recently fixed a similar issue in the Blake2B library

SparkDustJoe commented 7 years ago

I didn't initialize those thinking that they had defaults, I can try setting those and checking again tomorrow. And as a fellow developer, I've been bitten countless times by the "Wait, he did what?" errors from users finding new and exciting ways to break my code. :) All part of the process.

On Aug 7, 2016 00:57, "Keef Aragon" notifications@github.com wrote:

Fwiw an outstanding issue is that if any of these are 0 it won't throw an exception saying it can't be zero. Except sometimes in the form you describe. Which if that's what you're facing, sorry for the crappy user experience. I recently fixed a similar issue in the Blake2B library

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/kmaragon/Konscious.Security.Cryptography/issues/3#issuecomment-238063712, or mute the thread https://github.com/notifications/unsubscribe-auth/AIl7mAq3jdDmnxgO7qgY7gTSXM6sKw09ks5qdWWngaJpZM4Jea9_ .

kmaragon commented 7 years ago

Yeah - that'd probably be it. There are no sensible defaults because sensible totally is subjective and entirely dependent on the use case. But there's also no "InvalidOperationException { Message: You can't hash data with 0 iterations using 0 memory and 0 threads. }" which there should be. And I will add that in the next package release. It's all in the docs but asking someone to go read the docs (when the nuget package is right there and the package is essentially just two visible classes) is really crummy. A simple exception would have been better. Sorry it wasn't there in this case.

kmaragon commented 7 years ago

With the latest package, you should get an exception that is informative rather than divide by 0. Let me know if that works out better.

SparkDustJoe commented 7 years ago

MUCH better :+1: :-) I'll close out this issue and bang against it more.