From 5fc685aaeb22cac18f78f16a0486a2c8358282f8 Mon Sep 17 00:00:00 2001
From: Jan Garaj <info@monitoringartist.com>
Date: Sun, 8 Oct 2017 14:38:17 +0200
Subject: [PATCH] Use divided cookies for large access/refresh token cookie -
fix #278 (#279)
---
README.md | 13 +++++++++++++
cookies.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
session.go | 23 +++++++++++++++++++++--
3 files changed, 87 insertions(+), 4 deletions(-)
diff --git a/README.md b/README.md
index 2f72508..ad1624b 100644
--- a/README.md
+++ b/README.md
@@ -535,6 +535,19 @@ You can control the upstream endpoint via the --upstream-url option. Both http a
Assuming the *--enable-metrics* has been set, a Prometheus endpoint can be found on */oauth/metrics*; at present the only metric being exposed is a counter per http code.
+### Limitations
+
+Keep in mind [browser cookie limits](http://browsercookielimits.squawky.net/), if you use access or
+refresh tokens in the browser cookie. Keycloak-proxy divides cookie automatically if your cookie
+is longer than 4093 bytes. Real size of the cookie depends on the content of the issued access token.
+Also, encryption might add additional bytes to the cookie size. If you have large cookies (>200 KB),
+you might reach browser cookie limits.
+
+All cookies are part of the header request, so you might find a problem with the max headers size
+limits in your infrastructure (some load balancers have very low this value, such as 8 KB). Be
+sure that all network devices have sufficient header size limits. Otherwise, your users won't be
+able to obtain access token.
+
### **Contribution Guidelines**
----
diff --git a/cookies.go b/cookies.go
index a852d1b..a257a12 100644
--- a/cookies.go
+++ b/cookies.go
@@ -17,6 +17,7 @@ package main
import (
"net/http"
+ "strconv"
"strings"
"time"
)
@@ -45,12 +46,42 @@ func (r *oauthProxy) dropCookie(w http.ResponseWriter, host, name, value string,
// dropAccessTokenCookie drops a access token cookie into the response
func (r *oauthProxy) dropAccessTokenCookie(req *http.Request, w http.ResponseWriter, value string, duration time.Duration) {
- r.dropCookie(w, req.Host, r.config.CookieAccessName, value, duration)
+ // also cookie name is included in the cookie length; cookie name suffix "-xxx"
+ maxCookieLenght := 4089 - len(r.config.CookieAccessName)
+
+ if len(value) <= maxCookieLenght {
+ r.dropCookie(w, req.Host, r.config.CookieAccessName, value, duration)
+ } else {
+ // write divided cookies because payload is too long for single cookie
+ r.dropCookie(w, req.Host, r.config.CookieAccessName, value[0:maxCookieLenght], duration)
+ for i := maxCookieLenght; i < len(value); i += maxCookieLenght {
+ end := i + maxCookieLenght
+ if end > len(value) {
+ end = len(value)
+ }
+ r.dropCookie(w, req.Host, r.config.CookieAccessName+"-"+strconv.Itoa(i/maxCookieLenght), value[i:end], duration)
+ }
+ }
}
// dropRefreshTokenCookie drops a refresh token cookie into the response
func (r *oauthProxy) dropRefreshTokenCookie(req *http.Request, w http.ResponseWriter, value string, duration time.Duration) {
- r.dropCookie(w, req.Host, r.config.CookieRefreshName, value, duration)
+ // also cookie name is included in the cookie length; cookie name suffix "-xxx"
+ maxCookieLenght := 4089 - len(r.config.CookieRefreshName)
+
+ if len(value) <= maxCookieLenght {
+ r.dropCookie(w, req.Host, r.config.CookieRefreshName, value, duration)
+ } else {
+ // write divided cookies because payload is too long for single cookie
+ r.dropCookie(w, req.Host, r.config.CookieRefreshName, value[0:maxCookieLenght], duration)
+ for i := maxCookieLenght; i < len(value); i += maxCookieLenght {
+ end := i + maxCookieLenght
+ if end > len(value) {
+ end = len(value)
+ }
+ r.dropCookie(w, req.Host, r.config.CookieRefreshName+"-"+strconv.Itoa(i/maxCookieLenght), value[i:end], duration)
+ }
+ }
}
// clearAllCookies is just a helper function for the below
@@ -62,9 +93,29 @@ func (r *oauthProxy) clearAllCookies(req *http.Request, w http.ResponseWriter) {
// clearRefreshSessionCookie clears the session cookie
func (r *oauthProxy) clearRefreshTokenCookie(req *http.Request, w http.ResponseWriter) {
r.dropCookie(w, req.Host, r.config.CookieRefreshName, "", -10*time.Hour)
+
+ // clear divided cookies
+ for i := 1; i < 600; i++ {
+ var _, err = req.Cookie(r.config.CookieRefreshName + "-" + strconv.Itoa(i))
+ if err == nil {
+ r.dropCookie(w, req.Host, r.config.CookieRefreshName+"-"+strconv.Itoa(i), "", -10*time.Hour)
+ } else {
+ break
+ }
+ }
}
// clearAccessTokenCookie clears the session cookie
func (r *oauthProxy) clearAccessTokenCookie(req *http.Request, w http.ResponseWriter) {
r.dropCookie(w, req.Host, r.config.CookieAccessName, "", -10*time.Hour)
+
+ // clear divided cookies
+ for i := 1; i < len(req.Cookies()); i++ {
+ var _, err = req.Cookie(r.config.CookieAccessName + "-" + strconv.Itoa(i))
+ if err == nil {
+ r.dropCookie(w, req.Host, r.config.CookieAccessName+"-"+strconv.Itoa(i), "", -10*time.Hour)
+ } else {
+ break
+ }
+ }
}
diff --git a/session.go b/session.go
index 6bb11f9..bd47ff9 100644
--- a/session.go
+++ b/session.go
@@ -16,7 +16,9 @@ limitations under the License.
package main
import (
+ "bytes"
"net/http"
+ "strconv"
"strings"
"github.com/gambol99/go-oidc/jose"
@@ -100,8 +102,25 @@ func getTokenInBearer(req *http.Request) (string, error) {
// getTokenInCookie retrieves the access token from the request cookies
func getTokenInCookie(req *http.Request, name string) (string, error) {
+ var token bytes.Buffer
+
if cookie := findCookie(name, req.Cookies()); cookie != nil {
- return cookie.Value, nil
+ token.WriteString(cookie.Value)
+ }
+
+ // add also divided cookies
+ for i := 1; i < 600; i++ {
+ cookie := findCookie(name+"-"+strconv.Itoa(i), req.Cookies())
+ if cookie == nil {
+ break
+ } else {
+ token.WriteString(cookie.Value)
+ }
}
- return "", ErrSessionNotFound
+
+ if token.Len() == 0 {
+ return "", ErrSessionNotFound
+ }
+
+ return token.String(), nil
}
--
GitLab