From bade7461ca294af88555ecfec50ff3389aa32c4f Mon Sep 17 00:00:00 2001 From: Bobby DeSimone Date: Fri, 12 Jul 2019 15:39:03 -0700 Subject: [PATCH] internal/sessions: add cookie chunking --- CHANGELOG.md | 5 +- internal/sessions/cookie_store.go | 69 ++++++++++++++++++++++++- internal/sessions/session_state.go | 5 -- internal/sessions/session_state_test.go | 3 +- 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abaefb71c..872b5a1b4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/internal/sessions/cookie_store.go b/internal/sessions/cookie_store.go index aeceb07d5..4492312e4 100644 --- a/internal/sessions/cookie_store.go +++ b/internal/sessions/cookie_store.go @@ -9,11 +9,27 @@ import ( "time" "github.com/pomerium/pomerium/internal/cryptutil" + "github.com/pomerium/pomerium/internal/log" ) // 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 +127,40 @@ 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) + } else { + chunks := chunk(cookie.Value, MaxChunkSize) + for i, c := range chunks { + // 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) + } + log.Info().Interface("new cookie", nc).Msg("SetCookie: chunked") + 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 +182,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 +191,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 } diff --git a/internal/sessions/session_state.go b/internal/sessions/session_state.go index 6750c5dcc..41e9df2b5 100644 --- a/internal/sessions/session_state.go +++ b/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 } diff --git a/internal/sessions/session_state_test.go b/internal/sessions/session_state_test.go index 28144fb1e..19277c7c0 100644 --- a/internal/sessions/session_state_test.go +++ b/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) {