From b4530b92927d23c3df986e4b25b6e2046a047f61 Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Mon, 4 May 2020 11:34:01 -0700 Subject: [PATCH] Allow binary values in signed cookies Make signedValue & Validate operate on []byte by default and not assume/cast string. Any casting will be done from callers. --- pkg/encryption/cipher.go | 8 ++++---- pkg/sessions/cookie/session_store.go | 4 ++-- pkg/sessions/redis/redis_store.go | 8 ++++---- pkg/sessions/session_store_test.go | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/encryption/cipher.go b/pkg/encryption/cipher.go index dfdf036b..d23dd206 100644 --- a/pkg/encryption/cipher.go +++ b/pkg/encryption/cipher.go @@ -39,7 +39,7 @@ func SecretBytes(secret string) []byte { // additionally, the 'value' is encrypted so it's opaque to the browser // Validate ensures a cookie is properly signed -func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value string, t time.Time, ok bool) { +func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value []byte, t time.Time, ok bool) { // value, timestamp, sig parts := strings.Split(cookie.Value, "|") if len(parts) != 3 { @@ -59,7 +59,7 @@ func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value // it's a valid cookie. now get the contents rawValue, err := base64.URLEncoding.DecodeString(parts[0]) if err == nil { - value = string(rawValue) + value = rawValue ok = true return } @@ -69,8 +69,8 @@ func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value } // SignedValue returns a cookie that is signed and can later be checked with Validate -func SignedValue(seed string, key string, value string, now time.Time) string { - encodedValue := base64.URLEncoding.EncodeToString([]byte(value)) +func SignedValue(seed string, key string, value []byte, now time.Time) string { + encodedValue := base64.URLEncoding.EncodeToString(value) timeStr := fmt.Sprintf("%d", now.Unix()) sig := cookieSignature(sha256.New, seed, key, encodedValue, timeStr) cookieVal := fmt.Sprintf("%s|%s|%s", encodedValue, timeStr, sig) diff --git a/pkg/sessions/cookie/session_store.go b/pkg/sessions/cookie/session_store.go index 1b88e027..65fd237c 100644 --- a/pkg/sessions/cookie/session_store.go +++ b/pkg/sessions/cookie/session_store.go @@ -59,7 +59,7 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) { return nil, errors.New("cookie signature not valid") } - session, err := sessionFromCookie(val, s.CookieCipher) + session, err := sessionFromCookie(string(val), s.CookieCipher) if err != nil { return nil, err } @@ -104,7 +104,7 @@ func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Reques // authentication details func (s *SessionStore) makeSessionCookie(req *http.Request, value string, now time.Time) []*http.Cookie { if value != "" { - value = encryption.SignedValue(s.CookieOptions.Secret, s.CookieOptions.Name, value, now) + value = encryption.SignedValue(s.CookieOptions.Secret, s.CookieOptions.Name, []byte(value), now) } c := s.makeCookie(req, s.CookieOptions.Name, value, s.CookieOptions.Expire, now) if len(c.Value) > 4096-len(s.CookieOptions.Name) { diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 472ac3c9..51f31d51 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/pkg/sessions/redis/redis_store.go @@ -175,7 +175,7 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro return nil, fmt.Errorf("cookie signature not valid") } ctx := req.Context() - session, err := store.loadSessionFromString(ctx, val) + session, err := store.loadSessionFromString(ctx, string(val)) if err != nil { return nil, fmt.Errorf("error loading session: %s", err) } @@ -237,7 +237,7 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro // We only return an error if we had an issue with redis // If there's an issue decoding the ticket, ignore it - ticket, _ := decodeTicket(store.CookieOptions.Name, val) + ticket, _ := decodeTicket(store.CookieOptions.Name, string(val)) if ticket != nil { ctx := req.Context() err := store.Client.Del(ctx, ticket.asHandle(store.CookieOptions.Name)) @@ -251,7 +251,7 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro // makeCookie makes a cookie, signing the value if present func (store *SessionStore) makeCookie(req *http.Request, value string, expires time.Duration, now time.Time) *http.Cookie { if value != "" { - value = encryption.SignedValue(store.CookieOptions.Secret, store.CookieOptions.Name, value, now) + value = encryption.SignedValue(store.CookieOptions.Secret, store.CookieOptions.Name, []byte(value), now) } return cookies.MakeCookieFromOptions( req, @@ -302,7 +302,7 @@ func (store *SessionStore) getTicket(requestCookie *http.Cookie) (*TicketData, e } // Valid cookie, decode the ticket - ticket, err := decodeTicket(store.CookieOptions.Name, val) + ticket, err := decodeTicket(store.CookieOptions.Name, string(val)) if err != nil { // If we can't decode the ticket we have to create a new one return newTicket() diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index 60a86cef..f1919eb0 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -170,7 +170,7 @@ var _ = Describe("NewSessionStore", func() { BeforeEach(func() { By("Using a valid cookie with a different providers session encoding") broken := "BrokenSessionFromADifferentSessionImplementation" - value := encryption.SignedValue(cookieOpts.Secret, cookieOpts.Name, broken, time.Now()) + value := encryption.SignedValue(cookieOpts.Secret, cookieOpts.Name, []byte(broken), time.Now()) cookie := cookiesapi.MakeCookieFromOptions(request, cookieOpts.Name, value, cookieOpts, cookieOpts.Expire, time.Now()) request.AddCookie(cookie)