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.

Sign up for GitHub

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

Jump to bottom

OpenSaml4LogoutRequestResolver calls setIssueInstant with wrong type #10547

Closed
scho opened this issue Nov 23, 2021 · 6 comments
Closed

OpenSaml4LogoutRequestResolver calls setIssueInstant with wrong type #10547

scho opened this issue Nov 23, 2021 · 6 comments
Assignees
@marcusdacoregio
Labels
in: saml2 An issue in SAML2 modules status: invalid An issue that we don't feel is valid

Comments

@scho
Copy link

scho commented Nov 23, 2021 •
edited
Loading

Description of the bug
When the Saml2 Single Logout feature is used, a logout request is resolved using OpenSaml4LogoutRequestResolver.
When resolving, the issue instant of the logout request is set by calling:

logoutRequest.setIssueInstant(Instant.now(this.clock));

The parameter of org.opensaml.saml.saml2.core.RequestAbstractType#setIssueInstant is of type org.joda.time.DateTime, but Instant.now(this.clock) returns a java.time.Instant.

This results in the following error being logged:

Nov. 23, 2021 3:50:39 PM org.apache.catalina.core.StandardWrapperValve invoke
SCHWERWIEGEND: Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Filter execution threw an exception] with root cause
java.lang.NoSuchMethodError: 'void org.opensaml.saml.saml2.core.LogoutRequest.setIssueInstant(java.time.Instant)'
	at org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml4LogoutRequestResolver.lambda$resolve$1(OpenSaml4LogoutRequestResolver.java:62)
	at org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSamlLogoutRequestResolver.resolve(OpenSamlLogoutRequestResolver.java:122)
	at org.springframework.security.saml2.provider.service.web.authentication.logout.OpenSaml4LogoutRequestResolver.resolve(OpenSaml4LogoutRequestResolver.java:61)
	at org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2RelyingPartyInitiatedLogoutSuccessHandler.onLogoutSuccess(Saml2RelyingPartyInitiatedLogoutSuccessHandler.java:80)
	at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:100)
	at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:89)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutResponseFilter.doFilterInternal(Saml2LogoutResponseFilter.java:98)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutRequestFilter.doFilterInternal(Saml2LogoutRequestFilter.java:105)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:90)
	at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:75)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:110)
	at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:80)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:55)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:211)
	at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:183)
	at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:358)
	at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:271)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:96)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:197)
	at org.apache.catalina.core.StandardContextValve.__invoke(StandardContextValve.java:97)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:41002)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:540)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:135)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:357)
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:382)
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:893)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1726)
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	at java.base/java.lang.Thread.run(Thread.java:833)

To Reproduce
Use Spring Security 5.6.0 and the SAML2 Single Logout feature by sending a POST request to /logout.

Expected behavior
No error should occur.

Sample
Since this is probably a version issue (org.opensaml:opensaml-saml-api:4.0.0 might not use yoda time?), I omitted proving a sample. If you need one, let me know and I can see what I can do.
@scho scho added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 23, 2021
@marcusdacoregio
Copy link
Contributor

Hi @scho, thanks for reaching out.

The newest version of the SAML 2.0 Service Provider uses org.opensaml:opensaml-saml-api:4.1.0. Did you try bumping your version to be aligned with what Spring Security uses?

@marcusdacoregio marcusdacoregio self-assigned this Nov 25, 2021
@marcusdacoregio marcusdacoregio added in: saml2 An issue in SAML2 modules and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Nov 25, 2021
@scho
Copy link
Author

scho commented Nov 29, 2021

Hi @marcusdacoregio,

thanks for your quick answer!
I bumped the version to 4.0.1 and I got rid of the error.

But this manual bumping should not be necessary and I believe, there are two issues:

Shouldn't both versions be set to 4.0.1?

@scho
Copy link
Author

scho commented Nov 29, 2021 •
edited
Loading

Bumping the version to 4.0.1 resolves the error mentioned above, but creates other NoSuchMethodErrors, as the API of 4.0.1 is different:

Nov. 29, 2021 10:16:44 AM org.apache.catalina.core.StandardWrapperValve invoke
SCHWERWIEGEND: Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Filter execution threw an exception] with root cause
java.lang.NoSuchMethodError: 'void org.opensaml.saml.saml2.assertion.SAML20AssertionValidator.<init>(java.util.Collection, java.util.Collection, java.util.Collection, org.opensaml.saml.saml2.assertion.AssertionValidator, org.opensaml.xmlsec.signature.support.SignatureTrustEngine, org.opensaml.xmlsec.signature.support.SignaturePrevalidator)'
	at org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider$SAML20AssertionValidators$3.<init>(OpenSaml4AuthenticationProvider.java:732)
	at org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider$SAML20AssertionValidators.<clinit>(OpenSaml4AuthenticationProvider.java:731)
	at org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider.lambda$createDefaultAssertionSignatureValidator$8(OpenSaml4AuthenticationProvider.java:572)
	at org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider.lambda$createAssertionValidator$11(OpenSaml4AuthenticationProvider.java:654)
	at org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider.process(OpenSaml4AuthenticationProvider.java:495)
	at org.springframework.security.saml2.provider.service.authentication.OpenSaml4AuthenticationProvider.authenticate(OpenSaml4AuthenticationProvider.java:448)
	at org.springframework.security.authentication.ProviderManager.authenticate(ProviderManager.java:182)
	at org.springframework.security.saml2.provider.service.servlet.filter.Saml2WebSsoAuthenticationFilter.attemptAuthentication(Saml2WebSsoAuthenticationFilter.java:113)
	at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:223)
	at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:213)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.saml2.provider.service.web.Saml2MetadataFilter.doFilterInternal(Saml2MetadataFilter.java:86)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.saml2.provider.service.servlet.filter.Saml2WebSsoAuthenticationRequestFilter.doFilterInternal(Saml2WebSsoAuthenticationRequestFilter.java:179)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:103)
	at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:89)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:103)
	at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:89)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutResponseFilter.doFilterInternal(Saml2LogoutResponseFilter.java:98)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutRequestFilter.doFilterInternal(Saml2LogoutRequestFilter.java:105)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.header.HeaderWriterFilter.doHeadersAfter(HeaderWriterFilter.java:90)
	at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:75)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:110)
	at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:80)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:55)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:336)
	at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:211)
	at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:183)
	at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:358)
	at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:271)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.boot.actuate.metrics.web.servlet.WebMvcMetricsFilter.doFilterInternal(WebMvcMetricsFilter.java:96)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:197)
	at org.apache.catalina.core.StandardContextValve.__invoke(StandardContextValve.java:97)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:41002)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:540)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:135)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:357)
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:382)
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:893)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1726)
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	at java.base/java.lang.Thread.run(Thread.java:833)

Edit: This issue is that same as #10539, using 4.1.0 and the shibboleth maven repository fixes it.

@marcusdacoregio marcusdacoregio mentioned this issue Nov 29, 2021
Closed
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Nov 29, 2021 •
edited
Loading

Bumping the version to 4.0.1 resolves the error mentioned above, but creates other NoSuchMethodErrors, as the API of 4.0.1 is different:

I mean, you should update to 4.1.0 since it's the version that Spring Security uses.

Shouldn't both versions be set to 4.0.1?

Not right now, because Spring Security supports OpenSAML3 and OpenSAML4, so we have to support the two versions for now since it would be a breaking change. I've opened #10556 for that.

I'm closing this as solved but feel free to continue discussing.

@marcusdacoregio marcusdacoregio added the status: invalid An issue that we don't feel is valid label Nov 29, 2021
@scho
Copy link
Author

scho commented Nov 30, 2021 •
edited
Loading

Hi @marcusdacoregio,

using 4.1.0 fixed this issue indeed and therefore, I'm fine with closing this issue.

-But shouldn't there be some warning or even a compile time check, that you can not use the Single Logout feature unless you use Open SAML 4.1.0?-

Edit: In case somebody is on Open SAML3, using OpenSaml3LogoutResponseResolver and OpenSaml3LogoutRequestResolver would resolve the issue above.

But my first point stands:
Don't let the application start, when your configuration is invalid, would probably save a lot of people quite some time.

@marcusdacoregio marcusdacoregio mentioned this issue Nov 30, 2021
Closed
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Nov 30, 2021 •
edited
Loading

Hi @scho. That is a good idea. We could check inside the OpenSamlX constructors the opensaml version.

I've created #10567 to discuss that.
Also, if you like to contribute with a PR, feel free to reach out on that issue.

@marcusdacoregio marcusdacoregio mentioned this issue Aug 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marcusdacoregio marcusdacoregio

Labels
in: saml2 An issue in SAML2 modules status: invalid An issue that we don't feel is valid
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@scho @marcusdacoregio