Spring Security
  1. Spring Security
  2. SEC-1374

CAS ServiceProperties getters marked final/Can't dynamically manipulate ServiceProperties

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: 3.0.0
    • Fix Version/s: 3.0.2
    • Component/s: CAS
    • Labels:
      None

      Description

      I use spring security with the CAS client and I have an app that is deployed with a wild card domain. Users are given a subdomain to reach their services. So if my username is foo, I would reach my area by accessing foo.example.com. Another user named bar accesses their area at bar.example.com.

      With spring 2.5.6, I implemented this by subclassing ServiceProperties and using the RequestContextHolder to get access to the request information. In my subclass, getService() builds and returns the service identifier with the matching subdomain so that the user would get redirected back to the right place with the right session after signing in with CAS.

      In spring 3.0.0, all methods are marked final preventing me from making my dynamic ServiceProperties. I know I can get around this by subclassing the CasAuthenticationProvider and CasEntryPoint, but that seems a little dumb considering I would just be swapping the regular ServiceProperties with my own. So I'm hoping that you will just remove the final markers...

        Activity

        Hide
        Scott Battaglia added a comment -

        Its a security hazard to build the service url from the host portion of the request object since Host headers are set by the user. This is why Spring Security and the basic CAS client DO NOT do it.

        Show
        Scott Battaglia added a comment - Its a security hazard to build the service url from the host portion of the request object since Host headers are set by the user. This is why Spring Security and the basic CAS client DO NOT do it.
        Hide
        Luke Taylor added a comment -

        I guess that'll be a "won't fix" then

        Show
        Luke Taylor added a comment - I guess that'll be a "won't fix" then
        Hide
        Stephen Todd added a comment -

        Scott I appreciate your response. I hope you don't mind if I dig a little deeper. I'm hoping to find out in what ways it would be hazardous and what ways I could make it not hazardous. Could you give me an example of an attack or breach that could be occur when the Host header is maliciously altered? I have been trying to go through various scenarios myself and I am having a hard time coming up with some. I realize you may be very busy, but I would appreciate your input.

        Show
        Stephen Todd added a comment - Scott I appreciate your response. I hope you don't mind if I dig a little deeper. I'm hoping to find out in what ways it would be hazardous and what ways I could make it not hazardous. Could you give me an example of an attack or breach that could be occur when the Host header is maliciously altered? I have been trying to go through various scenarios myself and I am having a hard time coming up with some. I realize you may be very busy, but I would appreciate your input.
        Hide
        Luke Taylor added a comment -

        There is an entry in the CAS FAQ on why the host header isn't used in determining the service URL and you can find other references on the CAS web site and mailing list archives. e.g.

        http://www.mail-archive.com/cas@tp.its.yale.edu/msg03003.html

        Show
        Luke Taylor added a comment - There is an entry in the CAS FAQ on why the host header isn't used in determining the service URL and you can find other references on the CAS web site and mailing list archives. e.g. http://www.mail-archive.com/cas@tp.its.yale.edu/msg03003.html
        Hide
        Stephen Todd added a comment -

        Ok, thanks guys. So after reading through various the referenced I see how the vulnerability plays out. As I understand it, service A needs to self identify to prevent another malicious service B from using the ticket granted against B to authenticate service B to service A. The vulnerability is caused by tricking A to present itself as B when validating the ticket.

        That being said, I think that with modification I can still securely make what I want to work. The solution looks similar to the one I initially described with the addition of checking the hostname against my database of valid domains. The domains for users are defined in a database, the domains are all subdomains of a domain that I control, and authentication alone does not provide authorization for access to any of the services under that domain.

        So using the email in Luke's comment as a reference. Alice is redirected to Eve's cheating service with a service ticket. Eve's app crafts a request to Bob's web app that is located on alice.files.bob.com (alice is the name of the share in this case, but the app responds to all .files.bob.com). *Bob looks up "evil.eve.com" and determines that it is not a valid domain for the app and does not proceed to authenticate against the request against CAS.

        I think the main point is that the concept of a service having more than one access URL is currently not supported. Granted, I can see someone not understanding how this works and subclassing and implementing a dynamic version of ServiceProperties and opening themselves up to vulnerabilities (much like I did), however, a big fat warning in the javadoc may be better than just making everything final.

        Anyway, I know you guys definitely know the issues better than I do. I appreciate your time.

        Show
        Stephen Todd added a comment - Ok, thanks guys. So after reading through various the referenced I see how the vulnerability plays out. As I understand it, service A needs to self identify to prevent another malicious service B from using the ticket granted against B to authenticate service B to service A. The vulnerability is caused by tricking A to present itself as B when validating the ticket. That being said, I think that with modification I can still securely make what I want to work. The solution looks similar to the one I initially described with the addition of checking the hostname against my database of valid domains. The domains for users are defined in a database, the domains are all subdomains of a domain that I control, and authentication alone does not provide authorization for access to any of the services under that domain. So using the email in Luke's comment as a reference. Alice is redirected to Eve's cheating service with a service ticket. Eve's app crafts a request to Bob's web app that is located on alice.files.bob.com (alice is the name of the share in this case, but the app responds to all .files.bob.com). *Bob looks up "evil.eve.com" and determines that it is not a valid domain for the app and does not proceed to authenticate against the request against CAS. I think the main point is that the concept of a service having more than one access URL is currently not supported. Granted, I can see someone not understanding how this works and subclassing and implementing a dynamic version of ServiceProperties and opening themselves up to vulnerabilities (much like I did), however, a big fat warning in the javadoc may be better than just making everything final. Anyway, I know you guys definitely know the issues better than I do. I appreciate your time.
        Hide
        Scott Battaglia added a comment -

        If I did anything I would add an interface. The way ServiceProperties is set now is the correct behavior. Final prevents you from introducing the vulnerability. The CAS Client for Java also prevents you from introducing the vulnerability.

        The default class should not allow you to introduce the vulnerability.

        That said, if people want to, I can create an interface (i.e. CasServiceProperties) with the default implementation of (ServiceProperties) and allow people to create their own versions (i.e. IAmAVulnerableImplemenationServiceProperties )

        Show
        Scott Battaglia added a comment - If I did anything I would add an interface. The way ServiceProperties is set now is the correct behavior. Final prevents you from introducing the vulnerability. The CAS Client for Java also prevents you from introducing the vulnerability. The default class should not allow you to introduce the vulnerability. That said, if people want to, I can create an interface (i.e. CasServiceProperties) with the default implementation of (ServiceProperties) and allow people to create their own versions (i.e. IAmAVulnerableImplemenationServiceProperties )
        Hide
        Luke Taylor added a comment -

        Closing as "won't fix". If someone really wants to do this then they can probably get round the default behaviour using other means. But as Scott says, I don't think the default implementation should allow this (in the same way that the standard CAS client doesn't).

        Show
        Luke Taylor added a comment - Closing as "won't fix". If someone really wants to do this then they can probably get round the default behaviour using other means. But as Scott says, I don't think the default implementation should allow this (in the same way that the standard CAS client doesn't).

          People

          • Assignee:
            Scott Battaglia
            Reporter:
            Stephen Todd
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: