git.iko.soy/typeable/oauth2-proxy
on master help sign in
Refactor encryption.Cipher to be an Encrypt/Decrypt Interface

All Encrypt/Decrypt Cipher implementations will now take and return []byte to set up usage in future binary compatible encoding schemes to fix issues with bloat encrypting to strings (which requires base64ing adding 33% size)

commitf7cca1d0b3a2de56ec2121eff3ffaf8cf76bc176 authorNick Meves <nick.meves@greenhouse.io> dateSat 9 May 2020, 17:01 (6 years ago) parentb4530b9
8 files changed+187−44
compare with master
unifiedsplit
pkg/apis/options/sessions.go+1−1 pkg/apis/sessions/session_state.go+13−4 pkg/apis/sessions/session_state_test.go+1−1 pkg/encryption/cipher.go+60−27 pkg/encryption/cipher_test.go+107−6 pkg/sessions/cookie/session_store.go+3−3 pkg/sessions/redis/redis_store.go+1−1 pkg/validation/options.go+1−1
pkg/apis/options/sessions.go+1−1
@@ -5,7 +5,7 @@ import "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
55// SessionOptions contains configuration options for the SessionStore providers.
66type SessionOptions struct {
77 Type string `flag:"session-store-type" cfg:"session_store_type"`
8− Cipher *encryption.Cipher `cfg:",internal"`
8+ Cipher encryption.Cipher `cfg:",internal"`
99 Redis RedisStoreOptions `cfg:",squash"`
1010}
1111
pkg/apis/sessions/session_state.go+13−4
@@ -60,7 +60,7 @@ func (s *SessionState) String() string {
6060}
6161
6262// EncodeSessionState returns string representation of the current session
63−func (s *SessionState) EncodeSessionState(c *encryption.Cipher) (string, error) {
63+func (s *SessionState) EncodeSessionState(c encryption.Cipher) (string, error) {
6464 var ss SessionState
6565 if c == nil {
6666 // Store only Email and User when cipher is unavailable
@@ -89,7 +89,7 @@ func (s *SessionState) EncodeSessionState(c *encryption.Cipher) (string, error)
8989}
9090
9191// DecodeSessionState decodes the session cookie string into a SessionState
92−func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
92+func DecodeSessionState(v string, c encryption.Cipher) (*SessionState, error) {
9393 var ss SessionState
9494 err := json.Unmarshal([]byte(v), &ss)
9595 if err != nil {
@@ -106,7 +106,7 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
106106 } else {
107107 // Backward compatibility with using unencrypted Email
108108 if ss.Email != "" {
109− decryptedEmail, errEmail := c.Decrypt(ss.Email)
109+ decryptedEmail, errEmail := stringDecrypt(ss.Email, c)
110110 if errEmail == nil {
111111 if !utf8.ValidString(decryptedEmail) {
112112 return nil, errors.New("invalid value for decrypted email")
@@ -116,7 +116,7 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
116116 }
117117 // Backward compatibility with using unencrypted User
118118 if ss.User != "" {
119− decryptedUser, errUser := c.Decrypt(ss.User)
119+ decryptedUser, errUser := stringDecrypt(ss.User, c)
120120 if errUser == nil {
121121 if !utf8.ValidString(decryptedUser) {
122122 return nil, errors.New("invalid value for decrypted user")
@@ -139,3 +139,12 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
139139 }
140140 return &ss, nil
141141}
142+
143+// stringDecrypt wraps a Base64Cipher to make it string => string
144+func stringDecrypt(ciphertext string, c encryption.Cipher) (string, error) {
145+ value, err := c.Decrypt([]byte(ciphertext))
146+ if err != nil {
147+ return "", err
148+ }
149+ return string(value), nil
150+}
pkg/apis/sessions/session_state_test.go+1−1
@@ -145,7 +145,7 @@ func TestExpired(t *testing.T) {
145145type testCase struct {
146146 sessions.SessionState
147147 Encoded string
148− Cipher *encryption.Cipher
148+ Cipher encryption.Cipher
149149 Error bool
150150}
151151
pkg/encryption/cipher.go+60−27
@@ -109,47 +109,79 @@ func checkHmac(input, expected string) bool {
109109 return false
110110}
111111
112−// Cipher provides methods to encrypt and decrypt cookie values
113−type Cipher struct {
114− cipher.Block
112+// Cipher provides methods to encrypt and decrypt
113+type Cipher interface {
114+ Encrypt(value []byte) ([]byte, error)
115+ Decrypt(ciphertext []byte) ([]byte, error)
116+ EncryptInto(s *string) error
117+ DecryptInto(s *string) error
115118}
116119
117120// NewCipher returns a new aes Cipher for encrypting cookie values
118−func NewCipher(secret []byte) (*Cipher, error) {
119− c, err := aes.NewCipher(secret)
121+// This defaults to the Base64 Cipher to align with legacy Encrypt/Decrypt functionality
122+func NewCipher(secret []byte) (*Base64Cipher, error) {
123+ cfb, err := NewCFBCipher(secret)
120124 if err != nil {
121125 return nil, err
122126 }
123− return &Cipher{Block: c}, err
127+ return NewBase64Cipher(cfb)
128+}
129+
130+type Base64Cipher struct {
131+ Cipher Cipher
132+}
133+
134+// NewBase64Cipher returns a new AES CFB Cipher for encrypting cookie values
135+// And wrapping them in Base64 -- Supports Legacy encryption scheme
136+func NewBase64Cipher(c Cipher) (*Base64Cipher, error) {
137+ return &Base64Cipher{Cipher: c}, nil
124138}
125139
126−// Encrypt a value for use in a cookie
127−func (c *Cipher) Encrypt(value string) (string, error) {
128− encrypted, err := c.EncryptCFB([]byte(value))
140+// Encrypt encrypts a value with AES CFB & base64 encodes it
141+func (c *Base64Cipher) Encrypt(value []byte) ([]byte, error) {
142+ encrypted, err := c.Cipher.Encrypt([]byte(value))
129143 if err != nil {
130− return "", err
144+ return nil, err
131145 }
132146
133− return base64.StdEncoding.EncodeToString(encrypted), nil
147+ return []byte(base64.StdEncoding.EncodeToString(encrypted)), nil
134148}
135149
136−// Decrypt a value from a cookie to it's original string
137−func (c *Cipher) Decrypt(s string) (string, error) {
138− encrypted, err := base64.StdEncoding.DecodeString(s)
150+// Decrypt Base64 decodes a value & decrypts it with AES CFB
151+func (c *Base64Cipher) Decrypt(ciphertext []byte) ([]byte, error) {
152+ encrypted, err := base64.StdEncoding.DecodeString(string(ciphertext))
139153 if err != nil {
140− return "", fmt.Errorf("failed to decrypt cookie value %s", err)
154+ return nil, fmt.Errorf("failed to decrypt cookie value %s", err)
141155 }
142156
143− decrypted, err := c.DecryptCFB(encrypted)
157+ return c.Cipher.Decrypt(encrypted)
158+}
159+
160+// EncryptInto encrypts the value and stores it back in the string pointer
161+func (c *Base64Cipher) EncryptInto(s *string) error {
162+ return into(c.Encrypt, s)
163+}
164+
165+// DecryptInto decrypts the value and stores it back in the string pointer
166+func (c *Base64Cipher) DecryptInto(s *string) error {
167+ return into(c.Decrypt, s)
168+}
169+
170+type CFBCipher struct {
171+ cipher.Block
172+}
173+
174+// NewCFBCipher returns a new AES CFB Cipher
175+func NewCFBCipher(secret []byte) (*CFBCipher, error) {
176+ c, err := aes.NewCipher(secret)
144177 if err != nil {
145− return "", err
178+ return nil, err
146179 }
147−
148− return string(decrypted), nil
180+ return &CFBCipher{Block: c}, err
149181}
150182
151−// Encrypt with AES CFB on raw bytes
152−func (c *Cipher) EncryptCFB(value []byte) ([]byte, error) {
183+// Encrypt with AES CFB
184+func (c *CFBCipher) Encrypt(value []byte) ([]byte, error) {
153185 ciphertext := make([]byte, aes.BlockSize+len(value))
154186 iv := ciphertext[:aes.BlockSize]
155187 if _, err := io.ReadFull(rand.Reader, iv); err != nil {
@@ -162,7 +194,7 @@ func (c *Cipher) EncryptCFB(value []byte) ([]byte, error) {
162194}
163195
164196// Decrypt a AES CFB ciphertext
165−func (c *Cipher) DecryptCFB(ciphertext []byte) ([]byte, error) {
197+func (c *CFBCipher) Decrypt(ciphertext []byte) ([]byte, error) {
166198 if len(ciphertext) < aes.BlockSize {
167199 return nil, fmt.Errorf("encrypted value should be "+
168200 "at least %d bytes, but is only %d bytes",
@@ -178,17 +210,18 @@ func (c *Cipher) DecryptCFB(ciphertext []byte) ([]byte, error) {
178210}
179211
180212// EncryptInto encrypts the value and stores it back in the string pointer
181−func (c *Cipher) EncryptInto(s *string) error {
213+func (c *CFBCipher) EncryptInto(s *string) error {
182214 return into(c.Encrypt, s)
183215}
184216
185217// DecryptInto decrypts the value and stores it back in the string pointer
186−func (c *Cipher) DecryptInto(s *string) error {
218+func (c *CFBCipher) DecryptInto(s *string) error {
187219 return into(c.Decrypt, s)
188220}
189221
190222// codecFunc is a function that takes a string and encodes/decodes it
191−type codecFunc func(string) (string, error)
223+type codecFunc func([]byte) ([]byte, error)
224+
192225
193226func into(f codecFunc, s *string) error {
194227 // Do not encrypt/decrypt nil or empty strings
@@ -196,10 +229,10 @@ func into(f codecFunc, s *string) error {
196229 return nil
197230 }
198231
199− d, err := f(*s)
232+ d, err := f([]byte(*s))
200233 if err != nil {
201234 return err
202235 }
203− *s = d
236+ *s = string(d)
204237 return nil
205238}
pkg/encryption/cipher_test.go+107−6
@@ -105,14 +105,14 @@ func TestEncodeAndDecodeAccessToken(t *testing.T) {
105105 c, err := NewCipher([]byte(secret))
106106 assert.Equal(t, nil, err)
107107
108− encoded, err := c.Encrypt(token)
108+ encoded, err := c.Encrypt([]byte(token))
109109 assert.Equal(t, nil, err)
110110
111111 decoded, err := c.Decrypt(encoded)
112112 assert.Equal(t, nil, err)
113113
114− assert.NotEqual(t, token, encoded)
115− assert.Equal(t, token, decoded)
114+ assert.NotEqual(t, []byte(token), encoded)
115+ assert.Equal(t, []byte(token), decoded)
116116}
117117
118118func TestEncodeAndDecodeAccessTokenB64(t *testing.T) {
@@ -124,14 +124,115 @@ func TestEncodeAndDecodeAccessTokenB64(t *testing.T) {
124124 c, err := NewCipher([]byte(secret))
125125 assert.Equal(t, nil, err)
126126
127− encoded, err := c.Encrypt(token)
127+ encoded, err := c.Encrypt([]byte(token))
128128 assert.Equal(t, nil, err)
129129
130130 decoded, err := c.Decrypt(encoded)
131131 assert.Equal(t, nil, err)
132132
133− assert.NotEqual(t, token, encoded)
134− assert.Equal(t, token, decoded)
133+ assert.NotEqual(t, []byte(token), encoded)
134+ assert.Equal(t, []byte(token), decoded)
135+}
136+
137+func TestEncryptAndDecryptBase64(t *testing.T) {
138+ var err error
139+
140+ // Test all 3 valid AES sizes
141+ for _, secretSize := range []int{16, 24, 32} {
142+ secret := make([]byte, secretSize)
143+ _, err = io.ReadFull(rand.Reader, secret)
144+ assert.Equal(t, nil, err)
145+
146+ // NewCipher creates a Base64 wrapper of CFBCipher
147+ c, err := NewCipher(secret)
148+ assert.Equal(t, nil, err)
149+
150+ // Test various sizes sessions might be
151+ for _, dataSize := range []int{10, 100, 1000, 5000, 10000} {
152+ data := make([]byte, dataSize)
153+ _, err := io.ReadFull(rand.Reader, data)
154+ assert.Equal(t, nil, err)
155+
156+ encrypted, err := c.Encrypt(data)
157+ assert.Equal(t, nil, err)
158+
159+ decrypted, err := c.Decrypt(encrypted)
160+ assert.Equal(t, nil, err)
161+ assert.Equal(t, data, decrypted)
162+ }
163+ }
164+}
165+
166+func TestDecryptBase64WrongSecret(t *testing.T) {
167+ var err error
168+
169+ secret1 := []byte("0123456789abcdefghijklmnopqrstuv")
170+ secret2 := []byte("9876543210abcdefghijklmnopqrstuv")
171+
172+ c1, err := NewCipher(secret1)
173+ assert.Equal(t, nil, err)
174+
175+ c2, err := NewCipher(secret2)
176+ assert.Equal(t, nil, err)
177+
178+ data := []byte("f3928pufm982374dj02y485dsl34890u2t9nd4028s94dm58y2394087dhmsyt29h8df")
179+
180+ ciphertext, err := c1.Encrypt(data)
181+ assert.Equal(t, nil, err)
182+
183+ wrongData, err := c2.Decrypt(ciphertext)
184+ assert.Equal(t, nil, err)
185+ assert.NotEqual(t, data, wrongData)
186+}
187+
188+func TestEncryptAndDecryptCFB(t *testing.T) {
189+ var err error
190+
191+ // Test all 3 valid AES sizes
192+ for _, secretSize := range []int{16, 24, 32} {
193+ secret := make([]byte, secretSize)
194+ _, err = io.ReadFull(rand.Reader, secret)
195+ assert.Equal(t, nil, err)
196+
197+ c, err := NewCFBCipher(secret)
198+ assert.Equal(t, nil, err)
199+
200+ // Test various sizes sessions might be
201+ for _, dataSize := range []int{10, 100, 1000, 5000, 10000} {
202+ data := make([]byte, dataSize)
203+ _, err := io.ReadFull(rand.Reader, data)
204+ assert.Equal(t, nil, err)
205+
206+ encrypted, err := c.Encrypt(data)
207+ assert.Equal(t, nil, err)
208+
209+ decrypted, err := c.Decrypt(encrypted)
210+ assert.Equal(t, nil, err)
211+ assert.Equal(t, data, decrypted)
212+ }
213+ }
214+}
215+
216+func TestDecryptCFBWrongSecret(t *testing.T) {
217+ var err error
218+
219+ secret1 := []byte("0123456789abcdefghijklmnopqrstuv")
220+ secret2 := []byte("9876543210abcdefghijklmnopqrstuv")
221+
222+ c1, err := NewCFBCipher(secret1)
223+ assert.Equal(t, nil, err)
224+
225+ c2, err := NewCFBCipher(secret2)
226+ assert.Equal(t, nil, err)
227+
228+ data := []byte("f3928pufm982374dj02y485dsl34890u2t9nd4028s94dm58y2394087dhmsyt29h8df")
229+
230+ ciphertext, err := c1.Encrypt(data)
231+ assert.Equal(t, nil, err)
232+
233+ wrongData, err := c2.Decrypt(ciphertext)
234+ assert.Equal(t, nil, err)
235+ assert.NotEqual(t, data, wrongData)
135236}
136237
137238func TestEncodeIntoAndDecodeIntoAccessToken(t *testing.T) {
pkg/sessions/cookie/session_store.go+3−3
@@ -28,7 +28,7 @@ var _ sessions.SessionStore = &SessionStore{}
2828// interface that stores sessions in client side cookies
2929type SessionStore struct {
3030 CookieOptions *options.CookieOptions
31− CookieCipher *encryption.Cipher
31+ CookieCipher encryption.Cipher
3232}
3333
3434// Save takes a sessions.SessionState and stores the information from it
@@ -84,12 +84,12 @@ func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error {
8484}
8585
8686// cookieForSession serializes a session state for storage in a cookie
87−func cookieForSession(s *sessions.SessionState, c *encryption.Cipher) (string, error) {
87+func cookieForSession(s *sessions.SessionState, c encryption.Cipher) (string, error) {
8888 return s.EncodeSessionState(c)
8989}
9090
9191// sessionFromCookie deserializes a session from a cookie value
92−func sessionFromCookie(v string, c *encryption.Cipher) (s *sessions.SessionState, err error) {
92+func sessionFromCookie(v string, c encryption.Cipher) (s *sessions.SessionState, err error) {
9393 return sessions.DecodeSessionState(v, c)
9494}
9595
pkg/sessions/redis/redis_store.go+1−1
@@ -32,7 +32,7 @@ type TicketData struct {
3232// SessionStore is an implementation of the sessions.SessionStore
3333// interface that stores sessions in redis
3434type SessionStore struct {
35− CookieCipher *encryption.Cipher
35+ CookieCipher encryption.Cipher
3636 CookieOptions *options.CookieOptions
3737 Client Client
3838}
pkg/validation/options.go+1−1
@@ -38,7 +38,7 @@ func Validate(o *options.Options) error {
3838
3939 msgs := make([]string, 0)
4040
41− var cipher *encryption.Cipher
41+ var cipher encryption.Cipher
4242 if o.Cookie.Secret == "" {
4343 msgs = append(msgs, "missing setting: cookie-secret")
4444 } else {