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

Line breaks in Base64 encoded LogoutResponse cause an IllegalArgumentException #10923

Closed
chschu opened this issue Mar 1, 2022 · 2 comments
Closed
Assignees
Labels
in: saml2 An issue in SAML2 modules status: feedback-provided Feedback has been provided type: bug A general bug
Milestone

Comments

@chschu
Copy link

chschu commented Mar 1, 2022

Describe the bug
If the SAMLResponse parameter for Single Logout contains line breaks, Base64 decoding fails with an IllegalArgumentException:

java.lang.IllegalArgumentException: Illegal base64 character d
  at java.base/java.util.Base64$Decoder.decode0(Base64.java:847)
  at java.base/java.util.Base64$Decoder.decode(Base64.java:566)
  at java.base/java.util.Base64$Decoder.decode(Base64.java:589)
  at org.springframework.security.saml2.provider.service.authentication.logout.Saml2Utils.samlDecode(Saml2Utils.java:47)
  at org.springframework.security.saml2.provider.service.authentication.logout.OpenSamlLogoutResponseValidator.validate(OpenSamlLogoutResponseValidator.java:77)
  at org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutResponseFilter.doFilterInternal(Saml2LogoutResponseFilter.java:141)

Because the same Saml2Utils class is used to decode the LogoutRequest, the issue should also occur there.

During authentication, the SAMLResponse is Base64-decoded using org.apache.commons.codec.binary.Base64, and line breaks are not an issue there.

To Reproduce
Capture the POST to /logout/saml2/slo, add some %0D and/or %0A to the SAMLResponse parameter, and submit the POST request. Alternatively, have an asserting party that produces a SAMLResponse containing line breaks.

Expected behavior
Single Logout should accept newlines in the SAMLResponse request parameter.

According to https://docs.oasis-open.org/security/saml/v2.0/saml-bindings-2.0-os.pdf, line 793: "The base64-encoded value MAY be line-wrapped at a reasonable length in accordance with common practice."

The document is not clear about the specific Base64 format to be used in this context, but it refers to RFC2045 in some other contexts. RFC2045 is implemented by java.util.Base64.getMimeEncoder().

@chschu chschu added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Mar 1, 2022
@sjohnr sjohnr added the in: saml2 An issue in SAML2 modules label Mar 1, 2022
@jzheaux jzheaux added this to the 5.6.3 milestone Mar 1, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Mar 1, 2022

Thanks, @chschu, for reporting this. I agree that the spec in various places indicates RFC2045 must be used and in one place it loosely says that lines can be wrapped at "a reasonable length". These two are somewhat in conflict since RFC2045 line-wraps at a specific length.

I agree that one practical way to address this is to change Saml2Utils to use the MIME encoder. Would that address the issues you are experiencing?

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 1, 2022
@chschu
Copy link
Author

chschu commented Mar 2, 2022

Yes, the MIME decoder would be able to decode a SAMLResponse with line breaks, because it ignores anything that is not in the "base64 alphabet" (RFC2045, top of page 26).

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 2, 2022
jzheaux added a commit that referenced this issue Mar 2, 2022
jzheaux added a commit that referenced this issue Mar 2, 2022
@jzheaux jzheaux closed this as completed in 0b59e77 Mar 2, 2022
jzheaux added a commit that referenced this issue Mar 2, 2022
jzheaux added a commit that referenced this issue Mar 2, 2022
jzheaux added a commit that referenced this issue Mar 2, 2022
jzheaux added a commit that referenced this issue Mar 3, 2022
@jzheaux jzheaux modified the milestones: 5.6.3, 5.7.0-M3 Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: saml2 An issue in SAML2 modules status: feedback-provided Feedback has been provided type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants