kmaragon / Konscious.Security.Cryptography

MIT License
202 stars 20 forks source link

GetBytesAsync method on IIS seems not to work #25

Closed ghost closed 5 years ago

ghost commented 5 years ago

I'm having a very troubling issue trying to get the asynchronous task of Argon2 to work, as the synchronous task does not work on IIS.

I've followed all the advice and code given in this issue : https://github.com/kmaragon/Konscious.Security.Cryptography/issues/22

but the asynchronous task still will not complete, with this message:

Id = 74, Status = WaitingForActivation, Method = "{null}", Result = "{Not yet computed}"

The link provided shows all the code I've tested which has still resulted in exceptions thrown or asynchronous tasks not completing:

https://forums.asp.net/p/2146399/6227943.aspx?p=True&t=636715616764988597

kmaragon commented 5 years ago

In your code sample, try to use ConfigureAwait(false) on the awaited task that you have there (This will replicate the non-Async version of GetBytes() as of the latest version.

That is, change:

    byte[] h = await a.GetBytesAsync(64);

to

    byte[] h = await a.GetBytesAsync(64).ConfigureAwait(false);
kmaragon commented 5 years ago

Also, if you're using the latest version, you should be able to just call .GetBytes which just does the same thing.

kmaragon commented 5 years ago

If that doesn't do it, I'd be interested in knowing what parameters you're using. Namely DegreeOfParallelism, Iterations, and MemorySize. In your post, the last example you have appears to show that the hash is in fact being calculated, but it might just be taking a really long time. And if you tune these numbers correctly, you can set it up so that calculating your hash will take longer than the years either of us have left on earth. So it's possible that of all your attempts, the last one actually worked, but you're just generating an extraordinarily expensive hash.

ghost commented 5 years ago

Hi @kmaragon,

Thanks for your replies. I actually checked one of my methods that does that hashing and I did as you suggested in your first comment, adding the .ConfigureAwait(false), but this still does not compute the hash: Id = 90, Status = WaitingForActivation, Method = "{null}", Result = "{Not yet computed}"

I tried using GetBytes before, and although it did work with my ASP.NET MVC application, it did not work when I connected my application to IIS, as it gave me a NullReferenceException so I thought the solution was to switch over to the asynchronous method.

I'm still testing my application, so I set the parameters to pretty much the lowest just so that I could ensure that it hashes the password in its input fine. The values are listed below:

DegreeOfParallelism = 1 Iterations = 1 *MemorySize = 8

I also tried it with these values which still yields the same error:

DegreeOfParallelism = 2 Iterations = 40 *MemorySize = 8192

kmaragon commented 5 years ago

NullReferenceException. Wow. Can you post the exception backtrace on that?

ghost commented 5 years ago

Sure thing:

System.AggregateException
  HResult=0x80131500
  Message=One or more errors occurred.
  Source=MVCApp
  StackTrace:
   at MVCApp.Security.Password.Test(Byte[] password) in c:\users\danoded\source\repos\MVCApp\MVCApp\Security\Password.cs:line 181
   at MVCApp.Security.Password.PasswordToBytes(String password, String& PASSWORDSALT, String& SALT, String& UUID) in c:\users\danoded\source\repos\MVCApp\MVCApp\Security\Password.cs:line 133
   at MVCApp.Controllers.UserAccountController.SignUpNewCustomer(Customer signUp) in c:\users\danoded\source\repos\MVCApp\MVCApp\Controllers\UserAccountController.cs:line 381
   at System.Web.Mvc.ActionMethodDispatcher.Execute(ControllerBase controller, Object[] parameters)
   at System.Web.Mvc.ReflectedActionDescriptor.Execute(ControllerContext controllerContext, IDictionary`2 parameters)
   at System.Web.Mvc.ControllerActionInvoker.InvokeActionMethod(ControllerContext controllerContext, ActionDescriptor actionDescriptor, IDictionary`2 parameters)
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.<>c.<BeginInvokeSynchronousActionMethod>b__9_0(IAsyncResult asyncResult, ActionInvocation innerInvokeState)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncResult`2.CallEndDelegate(IAsyncResult asyncResult)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncResultBase`1.End()
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.EndInvokeActionMethod(IAsyncResult asyncResult)
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.AsyncInvocationWithFilters.<InvokeActionMethodFilterAsynchronouslyRecursive>b__11_0()
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.AsyncInvocationWithFilters.<>c__DisplayClass11_1.<InvokeActionMethodFilterAsynchronouslyRecursive>b__2()

Inner Exception 1:
NullReferenceException: Object reference not set to an instance of an object.
kmaragon commented 5 years ago

Is it possible to catch that Aggregate exception and rethrow the inner exception via: ExceptionDispatchInfo.Capture(e.InnerExceptions[0]).Throw();

I'm hoping the lack of stack trace there is something about the logger and not that the NullReferenceException was thrown from inside of some unmanaged IIS code with no .NET stack (which seems like wouldn't be able to throw an exception).

ghost commented 5 years ago

Yeah sure here:

System.NullReferenceException
  HResult=0x80004003
  Message=Object reference not set to an instance of an object.
  Source=mscorlib
  StackTrace:
   at System.Security.Cryptography.HMAC.get_Key()
   at Konscious.Security.Cryptography.HMACBlake2B.get_Key()
   at Konscious.Security.Cryptography.HMACBlake2B.Initialize()
   at Konscious.Security.Cryptography.Argon2Core.Initialize(Byte[] password)
   at Konscious.Security.Cryptography.Argon2Core.<InitializeLanes>d__32.MoveNext()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Konscious.Security.Cryptography.Argon2Core.<Hash>d__27.MoveNext()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Konscious.Security.Cryptography.Argon2.<>c__DisplayClass2_0.<<GetBytes>b__0>d.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at MVCApp.Security.Password.Test(Byte[] password) in C:\Users\danoded\source\repos\MVCApp\MVCApp\Security\Password.cs:line 182
   at MVCApp.Security.Password.PasswordToBytes(String password, String& PASSWORDSALT, String& SALT, String& UUID) in C:\Users\danoded\source\repos\MVCApp\MVCApp\Security\Password.cs:line 134
   at MVCApp.Controllers.UserAccountController.SignUpNewCustomer(Customer signUp) in C:\Users\danoded\source\repos\MVCApp\MVCApp\Controllers\UserAccountController.cs:line 381
   at System.Web.Mvc.ActionMethodDispatcher.Execute(ControllerBase controller, Object[] parameters)
   at System.Web.Mvc.ReflectedActionDescriptor.Execute(ControllerContext controllerContext, IDictionary`2 parameters)
   at System.Web.Mvc.ControllerActionInvoker.InvokeActionMethod(ControllerContext controllerContext, ActionDescriptor actionDescriptor, IDictionary`2 parameters)
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.<>c.<BeginInvokeSynchronousActionMethod>b__9_0(IAsyncResult asyncResult, ActionInvocation innerInvokeState)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncResult`2.CallEndDelegate(IAsyncResult asyncResult)
   at System.Web.Mvc.Async.AsyncResultWrapper.WrappedAsyncResultBase`1.End()
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.EndInvokeActionMethod(IAsyncResult asyncResult)
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.AsyncInvocationWithFilters.<InvokeActionMethodFilterAsynchronouslyRecursive>b__11_0()
   at System.Web.Mvc.Async.AsyncControllerActionInvoker.AsyncInvocationWithFilters.<>c__DisplayClass11_1.<InvokeActionMethodFilterAsynchronouslyRecursive>b__2()
kmaragon commented 5 years ago

There we go... This is probably un related to any of the threading issues. Did you construct the Hash with a non-null password? Either way, getting that in an aggregate exception is crappy. I'll update to throw that exception early in the GetBytes call without the async context so that it's not so hidden and cryptic.

ghost commented 5 years ago

Yeah my password is definitely not null, hashed perfectly fine with the built in SHA function (just used as a test to see if the password would hash).

kmaragon commented 5 years ago

Ok - I went through the reference source at https://referencesource.microsoft.com/#mscorlib/system/security/cryptography/hmac.cs,574972c57157be33

I assume this is .NET non-Core? Looking at .NET 4.x code, I can see how this can happen. I'll try a fix right now.

ghost commented 5 years ago

Yeah this is just .NET Framework 4.6.1.

kmaragon commented 5 years ago

Ok - I'm waiting for nuget to re-index. See if Argon2 1.1.3 + Blake2 1.0.8 fix the NullReferenceException when you can get them. (Actually Argon2 has no real fixes, I just cleaned up how exceptions are thrown so that if there is something wrong it doesn't land in an AggregateException and is just reported upfront).

ghost commented 5 years ago

Sure thing, thanks again for your help.

ghost commented 5 years ago

Just updated the package and tried the .GetBytes() method and it worked perfectly, thanks again. If anything else somehow manages to come up I'll reopen the issue.

kmaragon commented 5 years ago

Woot! Looks like it's been a bug in the pre .NET core implementation all along.

kmaragon commented 5 years ago

Thanks for helping me debug that. I have no access to a Windows machine and don't have the incentive to use the money, disk space, and time to install it since 100% of my work both public and private is on Linux... Using .NET core when applicable.

ghost commented 5 years ago

No problem, thanks for the amazing library.