kmaragon / Konscious.Security.Cryptography

MIT License
201 stars 20 forks source link

It does not work in IIS express. #11

Closed ghost closed 7 years ago

ghost commented 7 years ago

It is very strange... inside console app it works fine. But in IIS Express it is simply timed out.

WouterJanson commented 7 years ago

I had the same sort of issues, where the code would lock and it would never return a hash.

A simple temporary workaround would be to put the code in a separate method and call it as a task with TaskCreationOptions.LongRunning.

For example:

Task.Factory.StartNew(() => { hash = YourCryptMethod(password, salt); },TaskCreationOptions.LongRunning).Wait();

I've made a PR (#10) which could fix these problems.

ghost commented 7 years ago

I had the same sort of issues, where the code would lock and it would never return a hash. Absolutely agree...

It was a reason why PDKDF2 - HMACSHA512 with very big amount of iterations is finally used instead of Argon2.

Task.WhenAll(segment).ConfigureAwait(false);

Why we cannot restore context? And what does it mean from point of view of implementation of Argon2 algorithm? I guess you made it work... but are you believe it is the correct fix?

smarts commented 7 years ago

@hardhub can I ask why you're using IIS Express for .NET Core development instead of Kestrel?

ghost commented 7 years ago

@smarts

can I ask why you're using IIS Express for .NET Core development instead of Kestrel?

That project is not .Net Core project.
Kestrel is not available option in IDE.
And IIS Express is default and it fully meets development needs.

smarts commented 7 years ago

Ah ok, thanks for clarifying. ASP.NET has its own way of doing stuff in the background. This article sums it up pretty well (in addition to linking to other article(s) and libraries that can help). That said, I'm not convinced the pull request implementation is the right way to go. It's basically trying to solve a problem that exists for a compatibility issue between the Task Parallel Library (TPL) and a particular hosting environment (ASP.NET). IMHO the better solution is to leave this library alone and change the invoking code to perform the background tasks in a way that works for the desired hosting environment.

kmaragon commented 7 years ago

I merged the PR for now. I think I could give an easy way to avert the problem in .NET 4.5+ by exposing this all in an async method. Then the calling code could use that in an async / await context and expose it as such all the way up at the controller level. As I understand it, that should also help the issue as the initial thread from IIS can get back to doing work for IIS.

At the very least, that'd let the user set up the ConfigureAwait call rather than having to use it explicitly in the library and risk breaking other purposes - like a UI app from the UI thread

kmaragon commented 7 years ago

So this update might break you a bit. But if you switch from GetBytes to GetBytesAsync you can ConfigureAwait directly on that. Alternatively, you could expose the endpoint on the controller as async, using await on the GetBytesAsync result and IIS (or really ASP.NET inside of IIS) should handle the Context issues.

kmaragon commented 7 years ago

I'm going to close this issue as I haven't heard anything since the update