Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

change default for LogoutResponse #442

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

strehle
Copy link

@strehle strehle commented Jun 27, 2019

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 #145

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 spring-attic#145
@pivotal-issuemaster
Copy link

@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
Copy link

@strehle Thank you for signing the Contributor License Agreement!

@strehle
Copy link
Author

strehle commented Jun 27, 2019

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
Copy link
Contributor

fhanik commented Jul 8, 2019

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)?

@fhanik fhanik self-assigned this Jul 8, 2019
@strehle
Copy link
Author

strehle commented Jul 11, 2019

Hi @fhanik

I would have set this in UAA if possible, but due to issue
#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 #442 (comment) 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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants