-
Notifications
You must be signed in to change notification settings - Fork 6k
SEC-1374: CAS ServiceProperties getters marked final/Can't dynamically manipulate ServiceProperties #1617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Scott Battaglia said: 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. |
Luke Taylor said: I guess that'll be a "won't fix" then :) |
Stephen Todd said: 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. |
Luke Taylor said: 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. |
Stephen Todd said: 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. |
Scott Battaglia said: 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 ;-)) |
Luke Taylor said: 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). |
Stephen Todd (Migrated from SEC-1374) said:
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...
The text was updated successfully, but these errors were encountered: