Skip to content
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

SEC-1658: JA-SIG CAS Single Sign Out feature conflict with session-fixation protection #1897

Closed
spring-projects-issues opened this issue Jan 17, 2011 · 13 comments
Assignees
Labels
for: external-project For an external project and not something we can fix in: cas An issue in spring-security-cas type: enhancement A general enhancement type: jira An issue that was migrated from JIRA

Comments

@spring-projects-issues
Copy link

DUBOIS Fabrice (Migrated from SEC-1658) said:

I run into problems with CAS Single Sign Out feature working with session-fixation protection feature.

Indeed, Spring Security ConcurrentSessionControlStrategy always invalidate the existing session (call of session.invalidate( )) and create a new one to prevent from session-fixation attacks.

So, after CAS login process, SingleSignOutHttpSessionListener.sessionDestroyed( ) is called causing "ST to original session mapping" to be removed from SingleSignOutFilter's SESSION_MAPPING_STORAGE member.

When a single sign out request is posted, the new session isn't invalidated because "ST to new session mapping" was never been registered in SingleSingOutFilter's SESSION_MAPPING_STORAGE member.

@spring-projects-issues
Copy link
Author

Luke Taylor said:

I don't really think this is a major bug in Spring Security's current code. What you are saying is that if you choose to use session-fixation protection then this is not compatible with the use of CAS single sign-out which assumes that the session is never changed after authenticating.

Any external service could implement HttpSessionListener.sessionDestroyed(), making the assumption that the event means the user has logged out. In each case, this would require custom behaviour to be injected to override that assumption. In this case, it would probably mean calling the SessionMappingStorage instance used by the SSO filter in order to add the new session.

@spring-projects-issues
Copy link
Author

DUBOIS Fabrice said:

If I understand, this means that I should intercept sessionCreated() event, request another Service Ticket (ST) from CAS and store in SessionMappingStorage the "ST to new session mapping" ?

For the moment, SingleSignOutHttpSessionListener's sessionCreated() method does nothing :

public final class SingleSignOutHttpSessionListener implements HttpSessionListener {

private SessionMappingStorage SESSION_MAPPING_STORAGE;

public void sessionCreated(final HttpSessionEvent event) {
    // nothing to do at the moment
}

....

@spring-projects-issues
Copy link
Author

Luke Taylor said:

I don't think it should be necessary to request another ST. The existing ST should be available as the credentials property of the CasAuthenticationToken, so if you customize the SessionAuthenticationStrategy to do something like

sessionMappingStorage.addSessionById((String)authentication.getCredentials(), newSession);

This should make sure that the new session is stored in the SSO filter's registry, and when a logout request arrives for that ST, it will invalidate the new session.

@spring-projects-issues
Copy link
Author

DUBOIS Fabrice said:

I agree with you but this solution implies that :

  • my customization version of SessionAuthenticationStrategy is tight to CAS spring-security-cas-client.jar
  • my customization version of SessionAuthenticationStrategy must place 'ticket' request parameter because SingleSignOutFilter store it in SessionMappingStorage (this member is private and can't be accessed directly).
  • SingleSignOutFilter must be registred after SessionManagementFilter in filter chain.

@spring-projects-issues
Copy link
Author

Luke Taylor said:

I'm not really sure what you mean here - SessionMappingStorage is a static variable and is accessible through SingleSignOutFilter.getSessionMappingStorage(). So you have full access to it.

Alternatively you can inject the implementation into the filter yourself and retain a reference for use elsewhere.

I would expect that the filter would come before the Spring Security filters. The sequence would be

  1. It eagerly creates a session if one doesn't already exist and adds an entry using the ticket Id and the current session
  2. Spring Security's CasAuthenticationFilter authenticates using the ticket
  3. The SessionAuthenticationStrategy is called
  4. It invalidates the session (causing it to be removed from the store). Either that or you could do this explicitly yourself, rather than relying on the listener.
  5. It creates a new session.
  6. You call addSessionById to store the new session.

@spring-projects-issues
Copy link
Author

DUBOIS Fabrice said:

Just to be sure (as far I could understand) :

  1. It = SingleSigneOutFilter
  2. It = SessionManagementFilter in relation with SessionAuthenticationStrategy
  3. It = SessionManagementFilter in relation with SessionAuthenticationStrategy
  4. SessionAuthenticationStrategy

Right ?

@spring-projects-issues
Copy link
Author

Luke Taylor said:

No. SessionManagementFilter would not be involved, since a redirect is performed from the authentication filter. 4,5 and 6 are all part of your SessionAuthenticationStrategy .

@spring-projects-issues
Copy link
Author

roll tide said:

Hi Dubois - Did this work for you with the suggestions provided? I am running to this issue as well so wanted to check if suggested fix worked for you?

@spring-projects-issues
Copy link
Author

Mike Argyriou said:

@dubois Fabrice: I have faced the same problem in my application using CAS (Jasig) (not spring or spring security involved). I am just commenting to thank you for your observation; same happened on my custom app. On login action (using struts) there was a line session.invalidate() which fired an event that called org.jasig.cas.client.session.SingleSignOutHttpSessionListener#sessionDestroyed() which in turn called sessionMappingStorage.removeBySessionById(session.getId()). Therefore org.jasig.cas.client.session#handler#sessionMappingStorage was always empty!
The solution was to comment (or remove) session.invalidate() in login action.

@spring-projects-issues
Copy link
Author

Hugo Francisco Baes Junior said:

I managed to solve this issue using a custom SSO Filter that checks if the request is a ticket validation (like SSOHandler.isTokenRequest) before delegating to SSOHandler. If it is, then I call the filterChain.doFilter() first, so CasAuthenticationFilter/SuccessHandler may renew the SessionID , and after that I let SSOHandler proccess properly the request with the actual SessionID.

@spring-projects-issues spring-projects-issues added in: cas An issue in spring-security-cas Open type: enhancement A general enhancement type: jira An issue that was migrated from JIRA labels Feb 5, 2016
@jgribonvald
Copy link

Thanks for the tip this helped me a lot !

Just one other tip : you need to update authentication details on implementing a custom SessionFixatgionProtectionStrategy else you keep in authentication object the old sessionId (not totaly usefull, it's only for logging event that used this object)

@rwinch rwinch removed the Open label May 3, 2019
@rwinch
Copy link
Member

rwinch commented May 24, 2021

This should be changed in CAS rather than Spring Security

@rwinch rwinch closed this as completed May 24, 2021
@rwinch rwinch self-assigned this May 24, 2021
@rwinch rwinch added the for: external-project For an external project and not something we can fix label May 24, 2021
@nchava1
Copy link

nchava1 commented Feb 26, 2024

Hi. How this was fixed? I see same issue on spring 5.x and cas server 6.5.4 and cas client 3.1.12.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix in: cas An issue in spring-security-cas type: enhancement A general enhancement type: jira An issue that was migrated from JIRA
Projects
None yet
Development

No branches or pull requests

4 participants