1
0
mirror of https://github.com/oauth2-proxy/oauth2-proxy.git synced 2025-06-08 23:56:36 +02:00

Count complete cookie content in byte splitting

This commit is contained in:
Nick Meves 2020-07-03 23:41:08 -07:00
parent c6f1daba2f
commit 48a2aaadc1
No known key found for this signature in database
GPG Key ID: 93BA8A3CEDCDD1CF
2 changed files with 53 additions and 49 deletions

View File

@ -15,6 +15,13 @@ import (
"github.com/oauth2-proxy/oauth2-proxy/pkg/logger" "github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
) )
const (
// Cookies are limited to 4kb for all parts
// including the cookie name, value, attributes; IE (http.cookie).String()
// Most browsers' max is 4096 -- but we give ourselves some leeway
maxCookieLength = 4000
)
// Ensure CookieSessionStore implements the interface // Ensure CookieSessionStore implements the interface
var _ sessions.SessionStore = &SessionStore{} var _ sessions.SessionStore = &SessionStore{}
@ -102,9 +109,7 @@ func (s *SessionStore) makeSessionCookie(req *http.Request, value string, now ti
} }
c := s.makeCookie(req, s.CookieOptions.Name, value, s.CookieOptions.Expire, now) c := s.makeCookie(req, s.CookieOptions.Name, value, s.CookieOptions.Expire, now)
// Cookies are limited to 4kb including the length of the cookie name, if len(c.String()) > maxCookieLength {
// the cookie name can be up to 256 bytes
if len(c.Value) > 4096-len(s.CookieOptions.Name) {
return splitCookie(c) return splitCookie(c)
} }
return []*http.Cookie{c} return []*http.Cookie{c}
@ -139,7 +144,7 @@ func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.Coo
// it into a slice of cookies which fit within the 4kb cookie limit indexing // it into a slice of cookies which fit within the 4kb cookie limit indexing
// the cookies from 0 // the cookies from 0
func splitCookie(c *http.Cookie) []*http.Cookie { func splitCookie(c *http.Cookie) []*http.Cookie {
if len(c.Value) < 4096-len(c.Name) { if len(c.String()) < maxCookieLength {
return []*http.Cookie{c} return []*http.Cookie{c}
} }
@ -153,13 +158,16 @@ func splitCookie(c *http.Cookie) []*http.Cookie {
newCookie.Name = splitCookieName(c.Name, count) newCookie.Name = splitCookieName(c.Name, count)
count++ count++
maxCookieLength := 4096 - len(newCookie.Name)
if len(valueBytes) < maxCookieLength {
newCookie.Value = string(valueBytes) newCookie.Value = string(valueBytes)
cookieLength := len(newCookie.String())
if cookieLength <= maxCookieLength {
valueBytes = []byte{} valueBytes = []byte{}
} else { } else {
newValue := valueBytes[:maxCookieLength] overflow := cookieLength - maxCookieLength
valueBytes = valueBytes[maxCookieLength:] valueSize := len(valueBytes) - overflow
newValue := valueBytes[:valueSize]
valueBytes = valueBytes[valueSize:]
newCookie.Value = string(newValue) newCookie.Value = string(newValue)
} }
cookies = append(cookies, newCookie) cookies = append(cookies, newCookie)

View File

@ -54,59 +54,55 @@ func Test_copyCookie(t *testing.T) {
} }
func Test_splitCookie(t *testing.T) { func Test_splitCookie(t *testing.T) {
testCases := map[string]struct { testCases := map[string]*http.Cookie{
Cookie *http.Cookie
ExpectedSizes []int
}{
"Short cookie name": { "Short cookie name": {
Cookie: &http.Cookie{
Name: "short", Name: "short",
Value: strings.Repeat("v", 10000), Value: strings.Repeat("v", 10000),
}, },
ExpectedSizes: []int{4089, 4089, 1822},
},
"Long cookie name": { "Long cookie name": {
Cookie: &http.Cookie{
Name: strings.Repeat("n", 251), Name: strings.Repeat("n", 251),
Value: strings.Repeat("a", 10000), Value: strings.Repeat("a", 10000),
}, },
ExpectedSizes: []int{3843, 3843, 2314},
},
"Max cookie name": { "Max cookie name": {
Cookie: &http.Cookie{
Name: strings.Repeat("n", 256), Name: strings.Repeat("n", 256),
Value: strings.Repeat("a", 10000), Value: strings.Repeat("a", 10000),
}, },
ExpectedSizes: []int{3840, 3840, 2320},
},
"Suffix overflow cookie name": { "Suffix overflow cookie name": {
Cookie: &http.Cookie{
Name: strings.Repeat("n", 255), Name: strings.Repeat("n", 255),
Value: strings.Repeat("a", 10000), Value: strings.Repeat("a", 10000),
}, },
ExpectedSizes: []int{3840, 3840, 2320}, "Double digit suffix cookie name overflow": {
},
"Double digit suffix cookie name results in different sizes": {
Cookie: &http.Cookie{
Name: strings.Repeat("n", 253), Name: strings.Repeat("n", 253),
Value: strings.Repeat("a", 50000), Value: strings.Repeat("a", 50000),
}, },
ExpectedSizes: []int{3841, 3841, 3841, 3841, 3841, 3841, 3841, 3841, 3841, 3841, "With short name and attributes": {
3840, 3840, 3840, 70}, Name: "short",
Value: strings.Repeat("v", 10000),
Path: "/path",
Domain: "x.y.z",
Secure: true,
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
}, },
"Double digit suffix overflow cookie name results in same sizes": { "With max length name and attributes": {
Cookie: &http.Cookie{ Name: strings.Repeat("n", 256),
Name: strings.Repeat("n", 254), Value: strings.Repeat("v", 10000),
Value: strings.Repeat("a", 50000), Path: "/path",
}, Domain: "x.y.z",
ExpectedSizes: []int{3840, 3840, 3840, 3840, 3840, 3840, 3840, 3840, 3840, 3840, Secure: true,
3840, 3840, 3840, 80}, HttpOnly: true,
SameSite: http.SameSiteLaxMode,
}, },
} }
for testName, tc := range testCases { for testName, tc := range testCases {
t.Run(testName, func(t *testing.T) { t.Run(testName, func(t *testing.T) {
for i, cookie := range splitCookie(tc.Cookie) { splitCookies := splitCookie(tc)
assert.Equal(t, tc.ExpectedSizes[i], len(cookie.Value)) for i, cookie := range splitCookies {
if i < len(splitCookies)-1 {
assert.Equal(t, 4000, len(cookie.String()))
} else {
assert.GreaterOrEqual(t, 4000, len(cookie.String()))
}
} }
}) })
} }