search

spring-attic / spring-security-saml

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

change default for LogoutResponse #442

Open strehle opened 5 years ago

strehle commented 5 years ago

according to SAML2 spec http://docs.oasis-open.org/security/saml/v2.0/saml-profiles-2.0-os.pdf lines 1276-1277 logout messages must be signed in case of front channel. see also https://github.com/spring-projects/spring-security-saml/issues/145

pivotal-issuemaster commented 5 years ago

@strehle Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-issuemaster commented 5 years ago

@strehle Thank you for signing the Contributor License Agreement!

strehle commented 5 years ago

Hi @fhanik , The issue is with UAA if there are more than 1 UAAs connected to an IDP and one starts the SLO. The first one gets logout, but then the IDP (from a customer) stops with error message and the 2nd UAA is not proceeding the logout. regards, -markus

fhanik commented 5 years ago

Hi @strehle , Got a question for you.

A change like this would break existing implementations for a branch that is only released when critical security fixes are needed. Can the UAA not simply call setRequireLogoutResponseSigned(true)?

strehle commented 5 years ago

Hi @fhanik

I would have set this in UAA if possible, but due to issue https://github.com/spring-projects/spring-security-saml/issues/145 e.g. boolean signMessage = context.getPeerExtendedMetadata().isRequireLogoutResponseSigned() is always true.

The workaround we did, we patched the class in spring-security-saml and this fixed the issue and does not break any existing scenarios because the signature is part of the standard and IDPs handle an existing signature. As mentioned in https://github.com/spring-projects/spring-security-saml/pull/442#issue-292281724 the signature in this part is a SAML standard MUST.

The issue is not visible if you only have a 1 : 1 relation , IDP : SP, but if there are several SPs conected to an IDP the signature in the logout is a MUST to logout all SPs.

So if you fix issue #145 then this would also solve this here.