Reduce SessionState size better with MessagePack + LZ4 (#632)

* Encode sessions with MsgPack + LZ4

Assumes ciphers are now mandatory per #414. Cookie & Redis sessions
can fallback to V5 style JSON in error cases. TODO: session_state.go
unit tests & new unit tests for Legacy fallback scenarios.

* Only compress encoded sessions with Cookie Store

* Cleanup msgpack + lz4 error handling

* Change NewBase64Cipher to take in an existing Cipher

* Add msgpack & lz4 session state tests

* Add required options for oauthproxy tests

More aggressively assert.NoError on all
validation.Validate(opts) calls to enforce legal
options in all our tests.
Add additional NoError checks wherever error return
values were ignored.

* Remove support for uncompressed session state fields

* Improve error verbosity & add session state tests

* Ensure all marshalled sessions are valid

Invalid CFB decryptions can result in garbage data
that 1/100 times might cause message pack unmarshal
to not fail and instead return an empty session.
This adds more rigor to make sure legacy sessions
cause appropriate errors.

* Add tests for legacy V5 session decoding

Refactor common legacy JSON test cases to a
legacy helpers area under session store tests.

* Make ValidateSession a struct method & add CHANGELOG entry

* Improve SessionState error & comments verbosity

* Move legacy session test helpers to sessions pkg

Placing these helpers under the sessions pkg removed
all the circular import uses in housing it under the
session store area.

* Improve SignatureAuthenticator test helper formatting

* Make redis.legacyV5DecodeSession internal

* Make LegacyV5TestCase test table public for linter

pkg/sessions/cookie/session_store.go

@@ -60,7 +60,7 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) {
 		return nil, errors.New("cookie signature not valid")
 	}
 
-	session, err := sessionFromCookie(string(val), s.CookieCipher)
+	session, err := sessionFromCookie(val, s.CookieCipher)
 	if err != nil {
 		return nil, err
 	}
@@ -85,17 +85,26 @@ func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error {
 }
 
 // cookieForSession serializes a session state for storage in a cookie
-func cookieForSession(s *sessions.SessionState, c encryption.Cipher) (string, error) {
-	return s.EncodeSessionState(c)
+func cookieForSession(s *sessions.SessionState, c encryption.Cipher) ([]byte, error) {
+	return s.EncodeSessionState(c, true)
 }
 
 // sessionFromCookie deserializes a session from a cookie value
-func sessionFromCookie(v string, c encryption.Cipher) (s *sessions.SessionState, err error) {
-	return sessions.DecodeSessionState(v, c)
+func sessionFromCookie(v []byte, c encryption.Cipher) (s *sessions.SessionState, err error) {
+	ss, err := sessions.DecodeSessionState(v, c, true)
+	// If anything fails (Decrypt, LZ4, MessagePack), try legacy JSON decode
+	// LZ4 will likely fail for wrong header after AES-CFB spits out garbage
+	// data from trying to decrypt JSON it things is ciphertext
+	if err != nil {
+		// Legacy used Base64 + AES CFB
+		legacyCipher := encryption.NewBase64Cipher(c)
+		return sessions.LegacyV5DecodeSessionState(string(v), legacyCipher)
+	}
+	return ss, nil
 }
 
 // setSessionCookie adds the user's session cookie to the response
-func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Request, val string, created time.Time) {
+func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Request, val []byte, created time.Time) {
 	for _, c := range s.makeSessionCookie(req, val, created) {
 		http.SetCookie(rw, c)
 	}
@@ -103,12 +112,12 @@ func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Reques
 
 // makeSessionCookie creates an http.Cookie containing the authenticated user's
 // authentication details
-func (s *SessionStore) makeSessionCookie(req *http.Request, value string, now time.Time) []*http.Cookie {
-	if value != "" {
-		value = encryption.SignedValue(s.Cookie.Secret, s.Cookie.Name, []byte(value), now)
+func (s *SessionStore) makeSessionCookie(req *http.Request, value []byte, now time.Time) []*http.Cookie {
+	strValue := string(value)
+	if strValue != "" {
+		strValue = encryption.SignedValue(s.Cookie.Secret, s.Cookie.Name, value, now)
 	}
-	c := s.makeCookie(req, s.Cookie.Name, value, s.Cookie.Expire, now)
-
+	c := s.makeCookie(req, s.Cookie.Name, strValue, s.Cookie.Expire, now)
 	if len(c.String()) > maxCookieLength {
 		return splitCookie(c)
 	}
@@ -129,7 +138,7 @@ func (s *SessionStore) makeCookie(req *http.Request, name string, value string,
 // NewCookieSessionStore initialises a new instance of the SessionStore from
 // the configuration given
 func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessions.SessionStore, error) {
-	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))
+	cipher, err := encryption.NewCFBCipher(encryption.SecretBytes(cookieOpts.Secret))
 	if err != nil {
 		return nil, fmt.Errorf("error initialising cipher: %v", err)
 	}

pkg/sessions/redis/redis_store.go

@@ -40,7 +40,7 @@ type SessionStore struct {
 // NewRedisSessionStore initialises a new instance of the SessionStore from
 // the configuration given
 func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cookie) (sessions.SessionStore, error) {
-	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))
+	cfbCipher, err := encryption.NewCFBCipher(encryption.SecretBytes(cookieOpts.Secret))
 	if err != nil {
 		return nil, fmt.Errorf("error initialising cipher: %v", err)
 	}
@@ -52,7 +52,7 @@ func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cook
 
 	rs := &SessionStore{
 		Client:       client,
-		CookieCipher: cipher,
+		CookieCipher: cfbCipher,
 		Cookie:       cookieOpts,
 	}
 	return rs, nil
@@ -146,12 +146,8 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
 	// Old sessions that we are refreshing would have a request cookie
 	// New sessions don't, so we ignore the error. storeValue will check requestCookie
 	requestCookie, _ := req.Cookie(store.Cookie.Name)
-	value, err := s.EncodeSessionState(store.CookieCipher)
-	if err != nil {
-		return err
-	}
 	ctx := req.Context()
-	ticketString, err := store.storeValue(ctx, value, store.Cookie.Expire, requestCookie)
+	ticketString, err := store.saveSession(ctx, s, store.Cookie.Expire, requestCookie)
 	if err != nil {
 		return err
 	}
@@ -180,40 +176,13 @@ 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, string(val))
+	session, err := store.loadSessionFromTicket(ctx, string(val))
 	if err != nil {
 		return nil, fmt.Errorf("error loading session: %s", err)
 	}
 	return session, nil
 }
 
-// loadSessionFromString loads the session based on the ticket value
-func (store *SessionStore) loadSessionFromString(ctx context.Context, value string) (*sessions.SessionState, error) {
-	ticket, err := decodeTicket(store.Cookie.Name, value)
-	if err != nil {
-		return nil, err
-	}
-
-	resultBytes, err := store.Client.Get(ctx, ticket.asHandle(store.Cookie.Name))
-	if err != nil {
-		return nil, err
-	}
-
-	block, err := aes.NewCipher(ticket.Secret)
-	if err != nil {
-		return nil, err
-	}
-	// Use secret as the IV too, because each entry has it's own key
-	stream := cipher.NewCFBDecrypter(block, ticket.Secret)
-	stream.XORKeyStream(resultBytes, resultBytes)
-
-	session, err := sessions.DecodeSessionState(string(resultBytes), store.CookieCipher)
-	if err != nil {
-		return nil, err
-	}
-	return session, nil
-}
-
 // Clear clears any saved session information for a given ticket cookie
 // from redis, and then clears the session
 func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error {
@@ -253,6 +222,80 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro
 	return nil
 }
 
+// saveSession encodes a session with a GCM cipher & saves the data into Redis
+func (store *SessionStore) saveSession(ctx context.Context, s *sessions.SessionState, expiration time.Duration, requestCookie *http.Cookie) (string, error) {
+	ticket, err := store.getTicket(requestCookie)
+	if err != nil {
+		return "", fmt.Errorf("error getting ticket: %v", err)
+	}
+
+	c, err := encryption.NewGCMCipher(ticket.Secret)
+	if err != nil {
+		return "", fmt.Errorf("error initiating cipher block %s", err)
+	}
+
+	// Use AES-GCM since it provides authenticated encryption
+	// AES-CFB used in cookies has the cookie signing SHA to get around the lack of
+	// authentication in AES-CFB
+	ciphertext, err := s.EncodeSessionState(c, false)
+	if err != nil {
+		return "", err
+	}
+
+	handle := ticket.asHandle(store.Cookie.Name)
+	err = store.Client.Set(ctx, handle, ciphertext, expiration)
+	if err != nil {
+		return "", err
+	}
+	return ticket.encodeTicket(store.Cookie.Name), nil
+}
+
+// loadSessionFromTicket loads the session based on the ticket value
+func (store *SessionStore) loadSessionFromTicket(ctx context.Context, value string) (*sessions.SessionState, error) {
+	ticket, err := decodeTicket(store.Cookie.Name, value)
+	if err != nil {
+		return nil, err
+	}
+
+	resultBytes, err := store.Client.Get(ctx, ticket.asHandle(store.Cookie.Name))
+	if err != nil {
+		return nil, err
+	}
+
+	c, err := encryption.NewGCMCipher(ticket.Secret)
+	if err != nil {
+		return nil, err
+	}
+
+	session, err := sessions.DecodeSessionState(resultBytes, c, false)
+	if err != nil {
+		// The GCM cipher will error due to a legacy JSON payload not passing
+		// the authentication check part of AES GCM encryption.
+		// In that case, we can attempt to fallback to try a legacy load
+		legacyCipher := encryption.NewBase64Cipher(store.CookieCipher)
+		return legacyV5DecodeSession(resultBytes, ticket, legacyCipher)
+	}
+	return session, nil
+}
+
+// legacyV5DecodeSession loads the session based on the ticket value
+// This fallback uses V5 style encryption of Base64 + AES CFB
+func legacyV5DecodeSession(resultBytes []byte, ticket *TicketData, c encryption.Cipher) (*sessions.SessionState, error) {
+	block, err := aes.NewCipher(ticket.Secret)
+	if err != nil {
+		return nil, err
+	}
+	// Use secret as the IV too, because each entry has it's own key
+	stream := cipher.NewCFBDecrypter(block, ticket.Secret)
+	stream.XORKeyStream(resultBytes, resultBytes)
+
+	session, err := sessions.LegacyV5DecodeSessionState(string(resultBytes), c)
+	if err != nil {
+		return nil, err
+	}
+	return session, nil
+}
+
 // 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 != "" {
@@ -268,30 +311,6 @@ func (store *SessionStore) makeCookie(req *http.Request, value string, expires t
 	)
 }
 
-func (store *SessionStore) storeValue(ctx context.Context, value string, expiration time.Duration, requestCookie *http.Cookie) (string, error) {
-	ticket, err := store.getTicket(requestCookie)
-	if err != nil {
-		return "", fmt.Errorf("error getting ticket: %v", err)
-	}
-
-	ciphertext := make([]byte, len(value))
-	block, err := aes.NewCipher(ticket.Secret)
-	if err != nil {
-		return "", fmt.Errorf("error initiating cipher block %s", err)
-	}
-
-	// Use secret as the Initialization Vector too, because each entry has it's own key
-	stream := cipher.NewCFBEncrypter(block, ticket.Secret)
-	stream.XORKeyStream(ciphertext, []byte(value))
-
-	handle := ticket.asHandle(store.Cookie.Name)
-	err = store.Client.Set(ctx, handle, ciphertext, expiration)
-	if err != nil {
-		return "", err
-	}
-	return ticket.encodeTicket(store.Cookie.Name), nil
-}
-
 // getTicket retrieves an existing ticket from the cookie if present,
 // or creates a new ticket
 func (store *SessionStore) getTicket(requestCookie *http.Cookie) (*TicketData, error) {

pkg/sessions/redis/redis_store_test.go

@@ -1,6 +1,11 @@
 package redis
 
 import (
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/rand"
+	"fmt"
+	"io"
 	"log"
 	"os"
 	"testing"
@@ -17,6 +22,65 @@ import (
 	. "github.com/onsi/gomega"
 )
 
+// TestLegacyV5DecodeSession tests the fallback to LegacyV5DecodeSession
+// when a V5 encoded session is in Redis
+//
+// TODO: Remove when this is deprecated (likely V7)
+func Test_legacyV5DecodeSession(t *testing.T) {
+	testCases, _, legacyCipher := sessionsapi.CreateLegacyV5TestCases(t)
+
+	for testName, tc := range testCases {
+		t.Run(testName, func(t *testing.T) {
+			g := NewWithT(t)
+
+			secret := make([]byte, aes.BlockSize)
+			_, err := io.ReadFull(rand.Reader, secret)
+			g.Expect(err).ToNot(HaveOccurred())
+			ticket := &TicketData{
+				TicketID: "",
+				Secret:   secret,
+			}
+
+			encrypted, err := legacyStoreValue(tc.Input, ticket.Secret)
+			g.Expect(err).ToNot(HaveOccurred())
+
+			ss, err := legacyV5DecodeSession(encrypted, ticket, legacyCipher)
+			if tc.Error {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(ss).To(BeNil())
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+
+			// Compare sessions without *time.Time fields
+			exp := *tc.Output
+			exp.CreatedAt = nil
+			exp.ExpiresOn = nil
+			act := *ss
+			act.CreatedAt = nil
+			act.ExpiresOn = nil
+			g.Expect(exp).To(Equal(act))
+		})
+	}
+}
+
+// legacyStoreValue implements the legacy V5 Redis store AES-CFB value encryption
+//
+// TODO: Remove when this is deprecated (likely V7)
+func legacyStoreValue(value string, ticketSecret []byte) ([]byte, error) {
+	ciphertext := make([]byte, len(value))
+	block, err := aes.NewCipher(ticketSecret)
+	if err != nil {
+		return nil, fmt.Errorf("error initiating cipher block: %v", err)
+	}
+
+	// Use secret as the Initialization Vector too, because each entry has it's own key
+	stream := cipher.NewCFBEncrypter(block, ticketSecret)
+	stream.XORKeyStream(ciphertext, []byte(value))
+
+	return ciphertext, nil
+}
+
 func TestSessionStore(t *testing.T) {
 	logger.SetOutput(GinkgoWriter)