SEC-1373: UsernamePasswordAuthenticationToken retains password in cleartext even after authentication has succeeded

[spring-projects-issues]

Chas Emerick (Migrated from SEC-1373) said:

When using default form-based authentication, UsernamePasswordAuthenticationToken.getCredentials() (specifically, ((SecurityContextHolderAwareRequestWrapper)request).getUserPrincipal().getCredentials()) returns the user's cleartext password on all requests, even after the user's session has been authenticated.

Though I'm far from a security expert, this seems really bad. Authentication instances (in this case, UsernamePasswordAuthenticationToken) are likely being serialized to disk and databases right now by various containers -- those serializations almost certainly contain users' cleartext passwords. Clustered applications that use distributed sessions offer an even broader surface for accessing those credentials.

A localized solution would be to add a setCredentials() method to AbstractAuthenticationToken, which UsernamePasswordAuthenticationFilter.attemptAuthentication() could use to clear the credentials after a successful authentication.

More broadly: perhaps there are auth providers that do require credentials even after a session has been authenticated, or perhaps there are other use cases where having credentials around is necessary for some other purpose, but (from my naive perspective) it seems that user credentials are never needed after authentication has been completed successfully, so perhaps all authentication managers should clear credentials from Authentication instances, regardless of the type of token/provider involved?

[spring-projects-issues]

Chas Emerick said:

Sorry for the formatting -- I (incorrectly) assumed that wiki formatting was enabled here. :-(

[spring-projects-issues]

Luke Taylor said:

It's generally assumed that you have full control of the environment your application is running in and that access to the disc and network traffic is secured. Even if you have the ability to clear passwords, you have no guarantee when or if it will be garbage collected and that it won't be written to disk at some point. Similarly with other security-related data in your application, like database passwords. You should also have control over session serialization in your servlet container. You can disable it if you want to.

You can also customize the behaviour in Spring Security if you wish. You have control over the Authentication value that is returned from an AuthenticationProvider. For example, you can override DaoAuthenticationProvider.createSuccessAuthentication method. In Spring Security 3.0, you can also use a custom storage strategy for the security context by implementing SecurityContextRepository, so you are not limited to storage in the HttpSession.

[spring-projects-issues]

Chas Emerick said:

I'm not worried about in-JVM access, so GC and such isn't germane. However, serializations that include those cleartext credentials are an easy attack vector, especially for apps with clustered sessions. The notion of "full control of the environment" could be used as an argument for storing cleartext passwords in one's database, too, so I don't think that's much of a justification.

A couple of key questions:

• Are credentials required by any authentication provider after authentication has succeeded? If not, then there's no harm and unlimited upside by ensuring Authentication instances always have their credentials cleared post-authentication.
• If the answer to Q SEC-1890: make illegal arguments more obvious in BCrypt #1 is 'yes', then what would the harm be in ensuring that default form authentication machinery cleared users' credentials post-authentication?

[spring-projects-issues]

Luke Taylor said:

Even if you aren't worried about in-memory JVM access, you would also need to guarantee that your OS doesn't page the memory to disk, or that it uses a secure swap space. If people have direct access to the areas of the disk where your sessions are serialized then that is a problem. They will probably be able to directly hijack user sessions, for example, by reading the session data. In practice, replicated session clustering is not heavily used. Sticky-session load-balancer configurations are much more common and the problem is again about limiting local access.

I think we should provide some ability to allow passwords to be discarded after authentication if they are not required. This is more a defence-in-depth feature though, I don't think it is a bug. Making it the default behaviour immediately would break things for users who are currently relying on the status quo - for example, the ability to set the "authenticated" flag to false on an Authentication and have it transparently reauthenticated by the security interceptor.

[spring-projects-issues]

Chas Emerick said:

Leaving aside the semantics of enhancement vs. bug, I think that is totally backwards thinking. The point is, security breaches are hardly uncommon, so one has to assume that an attacker will have access to serialized sessions on disk or in the database (consider JDBC session stores, etc). That's bad enough for a single organization, but I'd think the last thing you want to have happen is for all of a site's users to have their cleartext passwords floating out there because a spring-security user's systems were compromised. Just to illustrate the nightmare scenario: http://www.theregister.co.uk/2009/12/16/rockyou_password_snafu/

This is why we hash and salt passwords to begin with, but the current issue makes repeats of events like the one linked above inevitable.

I'm not familiar with the concrete counter-use-case you describe, but thinking about this more brings me to the position that the risk and potentially massive consequences of allowing user passwords to be stored in cleartext in any circumstance should outweigh any other consideration.

Cheers,

/me trying to not jump up and down ;-)

[spring-projects-issues]

Luke Taylor said:

The ability to clear credentials post-authentication is now supported following the work done for SEC-1493.