LocalLogin Timing attack vulnerability

[kezakez]

• [x] I read and understood how to enable [logging]

  Question / Issue

  The LoginLocal route can potentially give away what accounts exist when used with AspNetIdentityUserService. When using a bad password entering an account that exists responds more quickly than an account that doesn't.

It is possible to fix the issue in the userservice implementation, but fixing higher up protects all implementations.

[kezakez]

An example of simple fix https://github.com/IdentityServer/IdentityServer3/pull/3423

[brockallen]

I'd suggest putting this in your identity management library, not in IdentityServer. Or add a time-constant middleware in front. I don't want IdentityServer to mandate this since we are deliberately separate from the identity management implementation.

[kezakez]

True the problem is specific to AspNetIdentity, but the fix is not AspNetIdentity specific. I had a look at trying to put the fix in https://github.com/IdentityServer/IdentityServer3.AspNetIdentity/blob/master/source/IdentityServer3.AspNetIdentity/IdentityServer3.AspNetIdentity.cs but anyone that overrides the AuthenticateLocalAsync function can easily become vulnerable again, also putting it in the framework protects all implementations. By default they fall in the pit of success ;)

[leastprivilege]

IdentityServer is also deployed to environments where timing attacks are not relevant.  What you really should do, is open an issue at the asp.net identity repo. 

---

Dominick Baier

On Wed, Dec 21, 2016 at 12:25 AM +0100, "Keiran McDonald" notifications@github.com wrote:

True the problem is specific to AspNetIdentity, but the fix is not AspNetIdentity specific. I had a look at trying to put the fix in https://github.com/IdentityServer/IdentityServer3.AspNetIdentity/blob/master/source/IdentityServer3.AspNetIdentity/IdentityServer3.AspNetIdentity.cs but anyone that overrides the AuthenticateLocalAsync function can easily become vulnerable again, also putting it in the framework protects all implementations. By default they fall in the pit of success ;)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[kezakez]

What is an example of an environment where a timing attack is not relevant?

I opened this issue https://github.com/IdentityServer/IdentityServer3.AspNetIdentity/issues/78

[leastprivilege]

e.g. Internal company deployments

I actually meant to open an issue with Microsoft - asp.net identity is on github. Curious to see what their response is.

[kezakez]

The deployment I am working on is internal, I still don't want it to be vulnerable. ;)

I opened issue https://github.com/aspnet/Identity/issues/1063 for CheckPasswordAsync

The issue a layer up https://github.com/IdentityServer/IdentityServer3.AspNetIdentity/issues/78 will still be a problem even if CheckPasswordAsync is fixed due to the if test on https://github.com/IdentityServer/IdentityServer3.AspNetIdentity/blob/master/source/IdentityServer3.AspNetIdentity/IdentityServer3.AspNetIdentity.cs#L204

[leastprivilege]

That's totally fine - then fix it in your user service. IdentityServer is used in many different ways - as Brock said this is too specific to put it in the core library.