Spring Security
  1. Spring Security
  2. SEC-1330

Potential security hole in OpenID configuration

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0.RC2
    • Fix Version/s: 3.0.0
    • Component/s: OpenID
    • Labels:
      None

      Description

      Suppose that I have configured SpringSecurity with the <openid-login> and <form-login>, 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.

        Issue Links

          Activity

          Hide
          Luke Taylor added a comment -

          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 <user-service> 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.

          Show
          Luke Taylor added a comment - 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 <user-service> 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.
          Hide
          Stephen Crawley added a comment -

          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.

          Show
          Stephen Crawley added a comment - 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.
          Hide
          Luke Taylor added a comment -

          Superseded by SEC-1331.

          Show
          Luke Taylor added a comment - Superseded by SEC-1331 .

            People

            • Assignee:
              Luke Taylor
              Reporter:
              Stephen Crawley
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: