Vulnerability when caching is enabled

[strieflin]

I use the Stormpath Shiro plugin to implement authentication / authorization in a REST API implementation with HTTP Basic Auth. When I disable caching everything works fine. However, when it is enabled a login attempt with a wrong password succeeds when it is preceded by a successful login attempt.

Looking at the Shiro / Plugin code I think that the problem is that the principal (username) is used as the cache key (which translates into the valid AuthenticationInfo object from the last successful login). However, as a AllowAllCredentialMatcher is used by default, the authentication attempt succeeds even though the password is wrong. Configuring the system to use a SimpleCredentialMatcher does not work either, as the credentials are not stored in the AuthenticationInfo object. As a consequence every login attempt, even those with correct credentials, fails. Hopefully, I'm doing something terribly wrong. My configuration is as follows:

[main]
# Configure the cache manager used to cache authentication results
cacheManager = org.apache.shiro.cache.MemoryConstrainedCacheManager
securityManager.cacheManager = $cacheManager

# Configure the Stormpath client used to access the Stormpath Cloud service.
# Set a Client factory, that configures a proxy and sets the path to the API key
stormpathClient = package.StormpathClientFactory
stormpathClient.cacheManager = $cacheManager

# Configure and set the Stormpath realm
stormpathRealm = com.stormpath.shiro.realm.ApplicationRealm
stormpathRealm.client = $stormpathClient

stormpathRealm.applicationRestUrl = my-url
stormpathRealm.authenticationCachingEnabled = true

securityManager.realm = $stormpathRealm

[omgitstom]

Hey @strieflin - we have an engineering looking at this now, we have reproduced. We will communicate updates through the support issue you created and update this github issue accordingly. Thanks!

[mrioan]

Hi @strieflin - The issue can be resolved as explained here: http://shiro.apache.org/static/current/apidocs/org/apache/shiro/realm/AuthenticatingRealm.html (see "Authentication Cache Invalidation on Logout" section). A fix has already been implemented. We first need to get it reviewed before we can release it. In case you want to take a look at it, the proposed fix is in branch "cached_login_eviction_fix". Thanks!

[strieflin]

Hi @antollinim - thank you for looking into the issue. However, I am not really sure that what you proposed really solves my problem.

What I want to have is that I don't have to hit the Stormpath servers for every request against my REST API. Hence, I don't want to have the cache evicted, since that would make a round trip to the Stormpath servers necessary again. My current strategy is simply not to perfom a logout after processing the request.

From my point of view the real problem is that when accessing the cache the cached credentials are not compared to the provided credentials (due to the AllowAllCredentialMatcher). The consequence of this is that you can provide even a wrong password and the login still succeeds.

[mrioan]

HI @strieflin I understand what you are saying. However, take a look at the javadoc of AuthenticatingRealm (http://shiro.apache.org/static/1.2.2/apidocs/org/apache/shiro/realm/AuthenticatingRealm.html). It stays that "If authentication caching is enabled, this implementation will attempt to evict (remove) cached authentication data for an account during logout". So, what you reported unveiled and issue in our plugin code because getAuthenticationCacheKey and getAuthenticationCacheKey methods were returning different values. So, we indeed had an issue that we needed to fix.

Now, going back to your intention of not having the cache evicted after logout, what you are describing is a new functionality not currently supported by AuthenticationRealm (Shiro code). So, could you please create a new issue here in order to request the addition of such new feature in the plugin?

Thanks!
Mario

[strieflin]

Well, still I think what you solved is not exactly the issue that I filed (compare "[...] when it is enabled a login attempt with a wrong password succeeds when it is preceded by a successful login attempt [...]"). Nevertheless I have opened another issue (see #7).

[mrioan]

Reopening because we still need to validate that the fix really solves the issue.

This is the issue: "[...] when it is enabled a login attempt with a wrong password succeeds when it is preceded by a successful login attempt [...]"

This is the fix: https://github.com/stormpath/stormpath-shiro/tree/cached_login_eviction_fix. PR not yet created since there are other pending PRs previous to this one that will cause merge conflicts.

[mrioan]

Issue has been resolved in this release https://github.com/stormpath/stormpath-shiro/releases/tag/stormpath-shiro-root-0.6.0

[lhazlewood]

@antollinim please assign this to the 0.6.0 milestone. All closed issues should be assigned to a milestone.

[mrioan]

Done. I did not know I could assign this issue to an already closed milestone without re-opening it. I just found out how...