Skip to content
This repository has been archived by the owner on Dec 14, 2017. It is now read-only.

Performance Query #39

Closed
parkinsona opened this issue Feb 15, 2016 · 11 comments
Closed

Performance Query #39

parkinsona opened this issue Feb 15, 2016 · 11 comments
Labels

Comments

@parkinsona
Copy link

We have just implemented an API that uses the Resource Owner Flow. As part of testing, we discovered that the retrieval of the AccessToken is quite slow compared to the Client Credentials Flow. About 4-5s Vs 0.5s.

I understand that it will be longer for Resource Owner because it needs to make a few DB calls.
I had a look through the logs, and it appears as though the longest part of the login is the call to Verify Password.

2016-02-15 10:56:15,128 [21] INFO  Sema.IdentityServer.MembershipReboot - [UserAccountService.VerifyPassword] called for accountID: ae790c39-c351-4360-a14b-608919f30100

2016-02-15 10:56:19,910 [21] DEBUG Sema.IdentityServer.MembershipReboot - [UserAccountService.VerifyPassword] success

The call to VerifyPassword seems to take about 4.5 seconds.

the above times are from my DEV environment. Test is about 3-4 seconds, and PROD is 1-2 seconds. I haven't checked UAT.

Is VerifyPassword likely to be slower because of the Database hits or the hashing of the password? I'm just wondering how we can speed this up if possible.

@brockallen
Copy link
Member

  1. You should explicitly configure the password hashing to be a number that's good for your scenario -- most hardware isn't keeping up with moore's law these days.
  2. As for DB query performance, if you need an index then set one in the DB.

@parkinsona
Copy link
Author

I know it's not DB query performance as I only have about 100 users in DEV at the moment. I also didn't notice a change when we did our bulk loads into DEV.

What I did notice in the bulk loads was the password hashing, so that was at the back of my mind as a possible cause.

I know I haven't altered the hashing count. From memory your hashing algorithm is based on the current year? That might explain why its appear "slower" in the past few weeks. My only requirement is to be OWASP compliant, so I will revist thoses specs.

@brockallen
Copy link
Member

From memory your hashing algorithm is based on the current year? That might explain why its appear "slower" in the past few weeks.

Yes. I'd pick ~50K to ~100K depending on how slow it is for a user to login.

@parkinsona
Copy link
Author

If I change the hashing count, will I need to update the stored passwords at all?

@brockallen
Copy link
Member

No -- the count is stored per-account, so you can change it without problems.

@brockallen
Copy link
Member

I had to double check if the passwords are re-hashed if the count in config is different than the stored password, and they're not. So if you have some passwords using a higher iteration and want to lower them, then you could handle an event from MR to re-hash and re-store the password.

@parkinsona
Copy link
Author

Where does it store the hash iteration count? I can't see that in the UserAccounts table.

hmmm I'll have to think about if I'll go ahead with the change or not.
Looks like it would have changed from 128K iterations to 256K when we went from 2015 to 2016.
It'll be the same for next year, so this may be alright.

One question though.... if I create a user in 2015 and and they're hashed with 128K iterations and they login in 2016, their hash iteration count would be different, wouldn't I have seen issues if we had to re-hash passwords when iteration counts changed?

@brockallen
Copy link
Member

It's prefixed on the hash (in hex).

256K is a lot, to be honest. 100K feels about right these days, but still might be too slow for some.

@parkinsona
Copy link
Author

Ohhhh I see.
I just had a look at VerifyHashedPassword and saw that it uses that prefix to perform the iterations when verifying an entered password.

Is the config setting for the Password Hashing Iteration Count only used when changing someone's password?

If so, then I should be right to change my iteration count in the configuration without affecting my existing users. When my existing users change their password, they will then be given a new hashing prefix.

@brockallen
Copy link
Member

Yes

@parkinsona
Copy link
Author

Ok then, well it should be fairly easy to reduce the iteration count back down to 100K.
Thanks for your help on this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants