Skip to content

IdP discovery: /saml2writer answers 500 for a leading-dot cookie domain or an unencoded list of two IdPs #1145

Description

@vharseko

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:

  1. 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.
  2. 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugsamlSAML / SAML2 federation

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions