search

spring-attic / spring-security-saml

SAML extension for the Spring Security project
Other
419 stars 482 forks source link

SAMLUtil.isDateTimeSkewValid having int overflow #478

Closed wojciech-piotrowiak closed 4 years ago

wojciech-piotrowiak commented 4 years ago

Recently noticed such error in our app:

Caused by: java.lang.IllegalArgumentException: The end instant must be greater than the start instant at org.joda.time.base.AbstractInterval.checkInterval(AbstractInterval.java:63) at org.joda.time.base.BaseInterval.(BaseInterval.java:94) at org.joda.time.Interval.(Interval.java:199) at org.springframework.security.saml.util.SAMLUtil.isDateTimeSkewValid(SAMLUtil.java:531) at org.springframework.security.saml.websso.WebSSOProfileConsumerImpl.verifyAuthenticationStatement(WebSSOProfileConsumerImpl.java:573) at org.springframework.security.saml.websso.WebSSOProfileConsumerImpl.verifyAssertion(WebSSOProfileConsumerImpl.java:344) at org.springframework.security.saml.websso.WebSSOProfileConsumerImpl.processAuthenticationResponse(WebSSOProfileConsumerImpl.java:250)

After debugging, realized that it happen when our application having maximum timeout available, which is Integer.MAX_VALUE. It is possible, because WebSSOProfileConsumerImpl accept long as maxAuthenticationAge param. So default skewInSec plus maximum forwardInterval will not fit into integer and it makes a problem for Interval class, as below:

Screen Shot 2020-05-04 at 3 31 15 PM

Used latest spring-security-saml2-core:1.0.10.RELEASE, previously used 1.0.3 and that version accepted Integer.MAX_VALUE as timeout.

wojciech-piotrowiak commented 4 years ago

@rwinch @jzheaux Is this defect have a chance to be fixed and released? If yes, can I try and open pull request ?

jzheaux commented 4 years ago

Yes, @wojciech-piotrowiak, thank you for offering! A pull request would be welcome.

wojciech-piotrowiak commented 4 years ago

@rwinch @jzheaux PR opened: https://github.com/spring-projects/spring-security-saml/pull/481