Found while reviewing #1136 (review by @maximthomas, reply: #1136 (comment)). #1136 leaves this path as it is, on purpose, so that it changes no configuration behaviour.
What happens
In the standalone IdP discovery WAR, CookieUtils.addCookieToResponse sends the preferred-IdP cookie _saml_idp through response.addCookie when HttpOnly is off and no SameSite is set:
|
if (!isCookieHttpOnly() && getCookieSameSite() == null) { |
|
response.addCookie(cookie); |
|
return; |
|
} |
HttpOnly off is the configurator default (Configurator.jsp:211). The WAR's libIDPDiscoveryConfig.properties template has no SameSite key, so a deployment with default settings takes this path. Tomcat's Rfc6265CookieProcessor then throws IllegalArgumentException for two cookies the configurator lets a deployment produce:
- A cookie domain with a leading dot, e.g.
.example.com, entered in the free-text Cookie Domain field (Configurator.jsp:187): An invalid domain [.example.com] was specified for this cookie. Every _saml_idp write fails.
- An unencoded preferred-IdP list with two or more IdPs (Encode Cookie = False,
Configurator.jsp:201). resetPreferredIDPCookie joins the IdPs with PREFERRED_COOKIE_SEPERATOR = " " (CookieWriterServlet.java:368, IDPDiscoveryConstants.java:46), and newCookie passes the value through unchanged when encoding is off: An invalid character [32] was present in the Cookie value. The first IdP is written, and the write for the second one fails.
CookieWriterServlet.doGetPost catches only IOException (:306), so the exception escapes and /saml2writer answers HTTP 500. The user is not redirected back to the RelayState.
With HttpOnly on, or with SameSite set, the same cookies go out as a hand-built Set-Cookie header, which the container does not validate, so they work. The failure is specific to the response.addCookie path.
Reproduction
Checked directly against the cookie processor on tomcat-embed-core 10.1.48 and 11.0.20:
Cookie c = new Cookie("_saml_idp", "aWRw");
c.setPath("/");
c.setDomain(".example.com");
new Rfc6265CookieProcessor().generateHeader(c, null);
// IllegalArgumentException: An invalid domain [.example.com] was specified for this cookie
new Rfc6265CookieProcessor().generateHeader(new Cookie("_saml_idp", "aWRw aWRwMg=="), null);
// IllegalArgumentException: An invalid character [32] was present in the Cookie value
example.com without the dot and a URL-encoded value (aWRw+aWRwMg%3D%3D) both pass.
End to end: deploy the IdP discovery WAR on Tomcat 10.1+ and configure it with Cookie Domain .example.com and HTTP-Only Cookie False. Then call /saml2writer?_saml_idp=<idp>&RelayState=<same-origin URL>, and the response is 500.
Possible directions
- Strip a leading dot from the configured domain in
newCookie. RFC 6265 §5.2.3 ignores it anyway, so .example.com and example.com mean the same to a browser.
- Keep
response.addCookie, and fall back to the hand-built header when it throws IllegalArgumentException. This also covers the unencoded list, whose separator cannot change without breaking the cookies browsers already hold.
- Catch the exception in
CookieWriterServlet and answer with the error page instead of a bare 500.
Found while reviewing #1136 (review by @maximthomas, reply: #1136 (comment)). #1136 leaves this path as it is, on purpose, so that it changes no configuration behaviour.
What happens
In the standalone IdP discovery WAR,
CookieUtils.addCookieToResponsesends the preferred-IdP cookie_saml_idpthroughresponse.addCookiewhen HttpOnly is off and no SameSite is set:OpenAM/openam-federation/openam-idpdiscovery/src/main/java/com/sun/identity/saml2/idpdiscovery/CookieUtils.java
Lines 487 to 490 in 1ceea86
HttpOnly off is the configurator default (
Configurator.jsp:211). The WAR'slibIDPDiscoveryConfig.propertiestemplate has no SameSite key, so a deployment with default settings takes this path. Tomcat'sRfc6265CookieProcessorthen throwsIllegalArgumentExceptionfor two cookies the configurator lets a deployment produce:.example.com, entered in the free-text Cookie Domain field (Configurator.jsp:187):An invalid domain [.example.com] was specified for this cookie. Every_saml_idpwrite fails.Configurator.jsp:201).resetPreferredIDPCookiejoins the IdPs withPREFERRED_COOKIE_SEPERATOR = " "(CookieWriterServlet.java:368,IDPDiscoveryConstants.java:46), andnewCookiepasses the value through unchanged when encoding is off:An invalid character [32] was present in the Cookie value. The first IdP is written, and the write for the second one fails.CookieWriterServlet.doGetPostcatches onlyIOException(:306), so the exception escapes and/saml2writeranswers HTTP 500. The user is not redirected back to the RelayState.With HttpOnly on, or with SameSite set, the same cookies go out as a hand-built
Set-Cookieheader, which the container does not validate, so they work. The failure is specific to theresponse.addCookiepath.Reproduction
Checked directly against the cookie processor on tomcat-embed-core 10.1.48 and 11.0.20:
example.comwithout the dot and a URL-encoded value (aWRw+aWRwMg%3D%3D) both pass.End to end: deploy the IdP discovery WAR on Tomcat 10.1+ and configure it with Cookie Domain
.example.comand HTTP-Only Cookie False. Then call/saml2writer?_saml_idp=<idp>&RelayState=<same-origin URL>, and the response is 500.Possible directions
newCookie. RFC 6265 §5.2.3 ignores it anyway, so.example.comandexample.commean the same to a browser.response.addCookie, and fall back to the hand-built header when it throwsIllegalArgumentException. This also covers the unencoded list, whose separator cannot change without breaking the cookies browsers already hold.CookieWriterServletand answer with the error page instead of a bare 500.