Skipper: Cookies set by oauthOidc -filters are occasionally too big

[josafat]

Describe the bug
oauthOidc* -filters like oauthOidcAnyClaims set a cookie from the client. The cookie size reaches easily the maximum size that the browsers accept (4096 bytes on Chrome?) causing the cookie to be dropped. With a basic OIDC setup e.g. with Keycloak, the size of the cookie easily reaches 5100bytes.

To Reproduce

1. Any route with oauthOidcAnyClaims -filter, which passes OIDC auth.

test:
      PathSubtree("/test/")
      -> oauthOidcAnyClaims("https://test.net/auth/realms/testrealm", "test", "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxx", "https://test.net/test/", "email profile", "name email")
      -> "http://test:9912";

Callback request sent after successful authentication to oidcp

Request URL: https://test.net/test/?state=e30123f4c2164...(truncated)...d791826ebce&session_state=614e2f07-8c31-...(truncated)...-d215d8569e70
Request Method: GET
Status Code: 307 

Response headers:

content-length: 0
date: Thu, 06 Jun 2019 09:14:34 GMT
location: /test
server: Skipper
set-cookie: skipperOauthOidcec3dfd82=d6a93c3bc69cae1201503...(truncated 5.75KB of data)...1a1c0b72ddd32577ed2b443d990891558930364288da0835; Path=/; HttpOnly; MaxAge=3600; Domain=test.net
status: 307

Expected behavior
The cookie(s) fit to browsers. E.g. the big ones are split into smaller cookies and merged when read. As an example keycloak-gatekeeper, which acts in a similar role from OIDC point of view splits the big cookies into smaller ones https://github.com/keycloak/keycloak-gatekeeper/blob/master/cookies.go

Observed behavior
Cookies are rejected by Chrome, Safari due to too big size.

[szuecs]

@josafat thanks for the detailed explanation!

@arjunrn do you want to work on a fix?

[arjunrn]

@szuecs yes I will assign the issue to myself

[universam1]

Looks like we are hitting the same issue

[szuecs]

It would be nice to have a test bed here, such that we can fix it more easy.

@universam1 Do you want to work on a fix for the oidc filter? Not sure if splitting the cookie or if gzip the cookie content could help. Split would be a fix that clearly works. What do you think?

[universam1]

Chrome says "Malformed Response Cookies: This set-cookie had invalid syntax"

Example truncated:

Set-Cookie: skipperOauthOidcb70dad48=98c8e57ca6...................adf43cd78a8e22893b4; Path=/; Secure; HttpOnly; MaxAge=3600; Domain=o.....org

[universam1]

Made a test for my idP token, they are 14.4k plain and 5.5k using simple gzip compression. Guess the idea to gzip @szuecs is an interesting one. Nevertheless, splitting must happen as well.
If you like @szuecs I could work on an implementation of that

[szuecs]

I am happy to review your pr or if you need guidance let us know!

[universam1]

Ok sounds good- is there any policy or suggestions from your side that would be valuable to know for an acceptable implementation?

[szuecs]

Normal Go style, tests that show the problem and that your fix work. Simple is better. :)
Sign off your commits is mandatory.

I guess, you will add a function, that checks cookie content size, gzip if too big, split into chunks of cookieMaxSize - len(cookieConfig) and append all the cookies and have a counter in the name. And to check the cookie you combine them again in another function. And add tests for both functions.
I hope this helps.

[universam1]

@szuecs Please see my first part of the proposed solution in #1289 . I like to split the chunking and the compressing into two PR as they are not directly tied and it will allow you possibly an easier review.

[szuecs]

Thanks @universam1 your PR triggered the release: https://github.com/zalando/skipper/releases/tag/v0.11.30

[universam1]

@szuecs Please see the second part #1293 to properly deal with large JWT token and cookies by compressing the cookie. Your idea is a great one - it actually not only solves our issues with ALB limits but also improves the overall latency on the deployments quite a bit!