IdentityServer / IdentityServer3.AccessTokenValidation

OWIN Middleware to validate access tokens from IdentityServer3
Apache License 2.0
91 stars 149 forks source link

Security concerns regarding cache #75

Closed bureus closed 8 years ago

bureus commented 8 years ago

Isn't it a security concern that the key in the cache is the plain token?... Should it not be encrypted before its stored as the cache key? Otherwise I can just read the keys of the cache and act as someone else?

leastprivilege commented 8 years ago

storing the token hashed would be probably better. wanna PR?

leastprivilege commented 8 years ago

or would that be rather the job of the cache implementation?? I have no concerns storing the tokens in-mem in plain text.

What's your scenario?

bureus commented 8 years ago

Hi,

Thanks for the quick reply. Im thinking of security concerns related to attackers targeting the cache. We are using Redis cache atm, and if i then can access the cache i will have all the users current access tokens there in plain text. So i can then just copy the cache key (the token) and then call my resource server and act as someone else. The same concern that you have if you print tokens in the log for example.

So instead of doing like this if (_options.EnableValidationResultCache) { await _options.ValidationResultCache.AddAsync(context.Token, claims); } do something like if (_options.EnableValidationResultCache) { await _options.ValidationResultCache.AddAsync(encrypt(context.Token), claims); }

bureus commented 8 years ago

but as you say, you could add the "encryption" of the key inside the cache implementation. But then it could be missed easily...

leastprivilege commented 8 years ago

You are right - it definitely shields the unsuspecting developer ;)

leastprivilege commented 8 years ago

see https://github.com/IdentityServer/IdentityServer3.AccessTokenValidation/issues/77

leastprivilege commented 8 years ago

can you give it a try?

brockallen commented 8 years ago

Any update @bureus?