Merge pull request #222 from desimone/bug/support-large-cookies

internal/sessions: add (chocolate chip) cookie chunks

CHANGELOG.md

@@ -4,10 +4,10 @@
 
 ### New
 
-
 ### Changed
 
 - GRPC Metrics Implementation [GH-218]
+
   - Additional GRPC server metrics and request sizes
   - Improved GRPC metrics implementation internals
   - The GRPC method label is now 'grpc_method' and GRPC status is now `grpc_client_status` and `grpc_server_status`
@@ -15,10 +15,13 @@
 - GRPC version upgraded to v1.22 [GH-219]
 
 - HTTP Metrics Implementation [GH-220]
+
   - Support HTTP request sizes on client and server side of proxy
   - Improved HTTP metrics implementation internals
   - The HTTP method label is now `http_method`, and HTTP status label is now `http_status`
 
+- Add support for large cookie sessions by chunking. [GH-211]
+
 ## v0.1.0
 
 ### NEW

internal/sessions/cookie_store.go

@@ -14,6 +14,21 @@ import (
 // ErrInvalidSession is an error for invalid sessions.
 var ErrInvalidSession = errors.New("internal/sessions: invalid session")
 
+// ChunkedCanaryByte is the byte value used as a canary prefix to distinguish if
+// the cookie is multi-part or not. This constant *should not* be valid
+// base64. It's important this byte is ASCII to avoid UTF-8 variable sized runes.
+// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#Directives
+const ChunkedCanaryByte byte = '%'
+
+// MaxChunkSize sets the upper bound on a cookie chunks payload value.
+// Note, this should be lower than the actual cookie's max size (4096 bytes)
+// which includes metadata.
+const MaxChunkSize = 3800
+
+// MaxNumChunks limits the number of chunks to iterate through. Conservatively
+// set to prevent any abuse.
+const MaxNumChunks = 5
+
 // CSRFStore has the functions for setting, getting, and clearing the CSRF cookie
 type CSRFStore interface {
 	SetCSRF(http.ResponseWriter, *http.Request, string)
@@ -111,6 +126,38 @@ func (s *CookieStore) makeCSRFCookie(req *http.Request, value string, expiration
 	return s.makeCookie(req, s.csrfName(), value, expiration, now)
 }
 
+func (s *CookieStore) setCookie(w http.ResponseWriter, cookie *http.Cookie) {
+	if len(cookie.String()) <= MaxChunkSize {
+		http.SetCookie(w, cookie)
+		return
+	}
+	for i, c := range chunk(cookie.Value, MaxChunkSize) {
+		// start with a copy of our original cookie
+		nc := *cookie
+		if i == 0 {
+			// if this is the first cookie, add our canary byte
+			nc.Value = fmt.Sprintf("%s%s", string(ChunkedCanaryByte), c)
+		} else {
+			// subsequent parts will be postfixed with their part number
+			nc.Name = fmt.Sprintf("%s_%d", cookie.Name, i)
+			nc.Value = fmt.Sprintf("%s", c)
+		}
+		http.SetCookie(w, &nc)
+	}
+
+}
+
+func chunk(s string, size int) []string {
+	ss := make([]string, 0, len(s)/size+1)
+	for len(s) > 0 {
+		if len(s) < size {
+			size = len(s)
+		}
+		ss, s = append(ss, s[:size]), s[size:]
+	}
+	return ss
+}
+
 // ClearCSRF clears the CSRF cookie from the request
 func (s *CookieStore) ClearCSRF(w http.ResponseWriter, req *http.Request) {
 	http.SetCookie(w, s.makeCSRFCookie(req, "", time.Hour*-1, time.Now()))
@@ -132,7 +179,7 @@ func (s *CookieStore) ClearSession(w http.ResponseWriter, req *http.Request) {
 }
 
 func (s *CookieStore) setSessionCookie(w http.ResponseWriter, req *http.Request, val string) {
-	http.SetCookie(w, s.makeSessionCookie(req, val, s.CookieExpire, time.Now()))
+	s.setCookie(w, s.makeSessionCookie(req, val, s.CookieExpire, time.Now()))
 }
 
 // LoadSession returns a SessionState from the cookie in the request.
@@ -141,7 +188,22 @@ func (s *CookieStore) LoadSession(req *http.Request) (*SessionState, error) {
 	if err != nil {
 		return nil, err // http.ErrNoCookie
 	}
-	session, err := UnmarshalSession(c.Value, s.CookieCipher)
+	cipherText := c.Value
+
+	// if the first byte is our canary byte, we need to handle the multipart bit
+	if []byte(c.Value)[0] == ChunkedCanaryByte {
+		var b strings.Builder
+		fmt.Fprintf(&b, "%s", cipherText[1:])
+		for i := 1; i < MaxNumChunks; i++ {
+			next, err := req.Cookie(fmt.Sprintf("%s_%d", s.Name, i))
+			if err != nil {
+				break // break if we can't find the next cookie
+			}
+			fmt.Fprintf(&b, "%s", next.Value)
+		}
+		cipherText = b.String()
+	}
+	session, err := UnmarshalSession(cipherText, s.CookieCipher)
 	if err != nil {
 		return nil, ErrInvalidSession
 	}

internal/sessions/cookie_store_test.go

@@ -1,7 +1,9 @@
 package sessions
 
 import (
+	"crypto/rand"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"reflect"
@@ -204,6 +206,10 @@ func TestCookieStore_SaveSession(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	hugeString := make([]byte, 4097)
+	if _, err := rand.Read(hugeString); err != nil {
+		t.Fatal(err)
+	}
 	tests := []struct {
 		name         string
 		sessionState *SessionState
@@ -211,22 +217,9 @@ func TestCookieStore_SaveSession(t *testing.T) {
 		wantErr      bool
 		wantLoadErr  bool
 	}{
-		{"good",
-			&SessionState{
-				AccessToken:     "token1234",
-				RefreshToken:    "refresh4321",
-				RefreshDeadline: time.Now().Add(1 * time.Hour).Truncate(time.Second).UTC(),
-				Email:           "user@domain.com",
-				User:            "user",
-			}, cipher, false, false},
-		{"bad cipher",
-			&SessionState{
-				AccessToken:     "token1234",
-				RefreshToken:    "refresh4321",
-				RefreshDeadline: time.Now().Add(1 * time.Hour).Truncate(time.Second).UTC(),
-				Email:           "user@domain.com",
-				User:            "user",
-			}, mockCipher{}, true, true},
+		{"good", &SessionState{AccessToken: "token1234", RefreshToken: "refresh4321", RefreshDeadline: time.Now().Add(1 * time.Hour).Truncate(time.Second).UTC(), Email: "user@domain.com", User: "user"}, cipher, false, false},
+		{"bad cipher", &SessionState{AccessToken: "token1234", RefreshToken: "refresh4321", RefreshDeadline: time.Now().Add(1 * time.Hour).Truncate(time.Second).UTC(), Email: "user@domain.com", User: "user"}, mockCipher{}, true, true},
+		{"huge cookie", &SessionState{AccessToken: fmt.Sprintf("%x", hugeString), RefreshToken: "refresh4321", RefreshDeadline: time.Now().Add(1 * time.Hour).Truncate(time.Second).UTC(), Email: "user@domain.com", User: "user"}, cipher, false, false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

internal/sessions/session_state.go

@@ -11,8 +11,6 @@ import (
 	"github.com/pomerium/pomerium/internal/cryptutil"
 )
 
-const MaxCookieSize = 4096
-
 var (
 	// ErrLifetimeExpired is an error for the lifetime deadline expiring
 	ErrLifetimeExpired = errors.New("user lifetime expired")
@@ -93,9 +91,6 @@ func MarshalSession(s *SessionState, c cryptutil.Cipher) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	if len(v) >= MaxCookieSize {
-		return "", fmt.Errorf("session too large, got %d bytes", len(v))
-	}
 	return v, nil
 }
 

internal/sessions/session_state_test.go

@@ -2,6 +2,7 @@ package sessions
 
 import (
 	"crypto/rand"
+	"fmt"
 	"reflect"
 	"testing"
 	"time"
@@ -157,7 +158,7 @@ func TestMarshalSession(t *testing.T) {
 		wantErr bool
 	}{
 		{"simple", &SessionState{}, false},
-		{"too big", &SessionState{AccessToken: string(hugeString)}, true},
+		{"too big", &SessionState{AccessToken: fmt.Sprintf("%x", hugeString)}, false},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {