SEC-1330: Potential security hole in OpenID configuration #1576

Closed
spring-issuemaster opened this Issue Dec 15, 2009 · 3 comments

1 participant

@spring-issuemaster

Stephen Crawley (Migrated from SEC-1330) said:

Suppose that I have configured SpringSecurity with the and , and created a user-details-service entry like:

<user name="http://jimi.hendrix.myopenid.com/" password="notused" 
      authorities="ROLE_USER" />

This allows me to perform an OpenID login after entering my password with "myopenid.com" into their login page (or whatever). But it ALSO allows me to login by entering "http://jimi.hendrix.myopenid.com/" as my username and "notused" as my password. Indeed, if I'd left the password blank, ...

This is unexpected, to say the least!

At the very least, the manual should contain a stern warning to the effect that the password MAY be used, depending on your configuration.

But I think that the root problem is that the current UserDetailsService API is too limited for OpenID use-cases. For example, I'd like a separate API method named (say) loadUserByIdentityUri that the OpenIDAuthenticationProvider calls passing the identityURL AND the OpenID response attributes. Then I could implement my user details service to return an unguessable (random) password in the OpenID case. And I could do things such as allow someone with OpenID credentials but no local account access with default authorities.

@spring-issuemaster

Luke Taylor said:

I don't think this is a bug. A form-login authentication requries an AuthenticationProvider, whereas OpenID only requires a UserDetailsService. If you define a user-service in the application context, in the same way as the OpenID sample, then you will not be able to authenticate against it using form login. You will get a message along the lines of

"No AuthenticationProvider found for org.springframework.security.authentication.UsernamePasswordAuthenticationToken"

If on the other hand you define an authentication-provider, containing the user-service, and you add a form-login element to the configuration, then that will be used to authenticate against. You say that is "unexpected", but my question would probably then be "What did you expect the form-login mechanism to use for authentication?". There is nothing that ties the user-service or authentication-provider definition to OpenID explicitly.

You are already free to return any password value you want from your UserDetailsService, so there is nothing to stop you returning a random value for OpenID users. In production, it would also normally be the case that you would have a separate UserDetailsService for use by OpenID and specify it using the user-service-ref attribute.

The element is really only intended for defining test data and not for production use, as storing user data within the application context isn't ultimately a practical option. One enhancement we could make though, which would clarify the fact that certain users are not there for authenticating against, would be to make the password field optional and have the namespace parser generate a string of random data when it was missing. This would prevent someone from accidentally using one for authentication.

I'll add this feature and modify the sample and namespace chapter to use this syntax.

@spring-issuemaster

Stephen Crawley said:

You are correct that is not strictly a bug. But IMO it is still a "hole", in the sense that a novice can easily mess up security by using the same UserDetailsStore for form-based and open id login without realising the consequences.

Your proposed enhancement, together with some text in the manual to point out the "trap" should be sufficient, I think.

@spring-issuemaster

Luke Taylor said:

Superseded by SEC-1331.

@spring-issuemaster spring-issuemaster added this to the 3.0.0 milestone Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment