Properly handle very long URL fragments captured during SAML handshake. #53505
Conversation
azasypkin
added
release_note:fix
Team:Security
|
Pinging @elastic/kibana-security (Team:Security) |
| @@ -43,11 +50,15 @@ export function defineSAMLRoutes({ router, logger, authc, csp, basePath }: Route | |||
| options: { authRequired: false }, | |||
| }, | |||
| (context, request, response) => { | |||
| // NodeJS limits the maximum size of the Request-Line + all HTTP headers to 8KB. | |||
| // See https://nodejs.org/api/cli.html#cli_max_http_header_size_size. | |||
| const maxRedirectURLSize = config.authc.saml?.maxRedirectURLSize.getValueInBytes() ?? 4096; | |||
There was a problem hiding this comment.
note: it's technically possible to call this endpoint even if SAML is not enabled (no use case for that yet), so that config isn't available. I thought not throwing error would be better, but I don't have strong opinion here.
Technically we can just hardcode any number here and not rely on config at all (since it's tied to NodeJS limit and not to what set through config). Curious what you have in mind?
💚 Build SucceededTo update your PR or re-run it, just comment with: |
|
I've been made aware that Kibana in prod is already configured to allow up to 64KB of headers incl. path. That sounds like enough to not hit this issue in real scenarios and if so we can probably wait till proper "custom HTML renderer" where we can just send entire URL fragment to a server via I'll talk to the original reporters/env maintainers to understand why they're hitting this issue on their environment. |
|
UPDATE: so we found that the root cause of the original SAML issue is in a proxy that sits in front of Kibana and has its own URL/headers size limit that's much lower than what we allow in Kibana server (64KB). Someone will bump up the limit there. |
Given this information, do you think we should still pursue this change? |
No, I'm leaning towards just closing this PR because 1. we haven't heard any "real" user complaints yet (default 64KB limit is quite large after all), 2. there is a good chance we can leverage the functionality introduced in #52161 (still needs some work) to do a proper POST call with entire fragment, 3. we have a workaround in case that actually happens in prod (set If there are no any objections I'll close this issue today/tomorrow. |
Sounds good to me! |
This PR implements the alternative workaround proposed in #53478 (comment):
Fixes: #53478
"Release note: Since Kibana 7.5 we started to fully preserve the URL user used to login with SAML, but some very long URLs managed to exceed the size that Kibana server can accept leading to failures during SAML login. With this fix we truncate user URLs before sending them to the server if they exceed the limit."