ce2e92b · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Improve design of Base64Cipher wrapping other ciphers.

Have it take in a cipher init function as an argument. Remove the confusing `newCipher` method that matched legacy behavior and returns a Base64Cipher(CFBCipher) -- instead explicitly ask for that in the uses.

commitce2e92bc578452a45bedf8c6413a2d8f43367469 authorNick Meves <nick.meves@greenhouse.io> dateSun 10 May 2020, 09:44 (6 years ago) parentb6931aa

5 files changed+67−47

compare with master

unifiedsplit

pkg/apis/sessions/session_state_test.go+9−5 pkg/encryption/cipher.go+7−15 pkg/encryption/cipher_test.go+49−25 pkg/sessions/session_store_test.go+1−1 pkg/validation/options.go+1−1

pkg/apis/sessions/session_state_test.go+9−5

@@ -17,10 +17,14 @@ func timePtr(t time.Time) *time.Time {

1717 return &t

1818}

1919

20+func NewCipher(secret []byte) (encryption.Cipher, error) {

21+ return encryption.NewBase64Cipher(encryption.NewCFBCipher, secret)

22+}

23+

2024func TestSessionStateSerialization(t *testing.T) {

21− c, err := encryption.NewCipher([]byte(secret))

25+ c, err := NewCipher([]byte(secret))

2226 assert.Equal(t, nil, err)

23− c2, err := encryption.NewCipher([]byte(altSecret))

27+ c2, err := NewCipher([]byte(altSecret))

2428 assert.Equal(t, nil, err)

2529 s := &sessions.SessionState{

2630 Email: "user@domain.com",

@@ -53,9 +57,9 @@ func TestSessionStateSerialization(t *testing.T) {

5357}

5458

5559func TestSessionStateSerializationWithUser(t *testing.T) {

56− c, err := encryption.NewCipher([]byte(secret))

60+ c, err := NewCipher([]byte(secret))

5761 assert.Equal(t, nil, err)

58− c2, err := encryption.NewCipher([]byte(altSecret))

62+ c2, err := NewCipher([]byte(altSecret))

5963 assert.Equal(t, nil, err)

6064 s := &sessions.SessionState{

6165 User: "just-user",

@@ -201,7 +205,7 @@ func TestDecodeSessionState(t *testing.T) {

201205 eJSON, _ := e.MarshalJSON()

202206 eString := string(eJSON)

203207

204− c, err := encryption.NewCipher([]byte(secret))

208+ c, err := NewCipher([]byte(secret))

205209 assert.NoError(t, err)

206210

207211 testCases := []testCase{

pkg/encryption/cipher.go+7−15

@@ -135,16 +135,6 @@ func (c *DefaultCipher) DecryptInto(s *string) error {

135135 return into(c.Decrypt, s)

136136}

137137

138−// NewCipher returns a new aes Cipher for encrypting cookie values

139−// This defaults to the Base64 Cipher to align with legacy Encrypt/Decrypt functionality

140−func NewCipher(secret []byte) (Cipher, error) {

141− cfb, err := NewCFBCipher(secret)

142− if err != nil {

143− return nil, err

144− }

145− return NewBase64Cipher(cfb)

146−}

147−

148138type Base64Cipher struct {

149139 DefaultCipher

150140 Cipher Cipher

@@ -152,7 +142,11 @@ type Base64Cipher struct {

152142

153143// NewBase64Cipher returns a new AES Cipher for encrypting cookie values

154144// and wrapping them in Base64 -- Supports Legacy encryption scheme

155−func NewBase64Cipher(c Cipher) (Cipher, error) {

145+func NewBase64Cipher(initCipher func([]byte) (Cipher, error), secret []byte) (Cipher, error) {

146+ c, err := initCipher(secret)

147+ if err != nil {

148+ return nil, err

149+ }

156150 return &Base64Cipher{Cipher: c}, nil

157151}

158152

@@ -170,7 +164,7 @@ func (c *Base64Cipher) Encrypt(value []byte) ([]byte, error) {

170164func (c *Base64Cipher) Decrypt(ciphertext []byte) ([]byte, error) {

171165 encrypted, err := base64.StdEncoding.DecodeString(string(ciphertext))

172166 if err != nil {

173− return nil, fmt.Errorf("failed to decrypt cookie value %s", err)

167+ return nil, fmt.Errorf("failed to base64 decode value %s", err)

174168 }

175169

176170 return c.Cipher.Decrypt(encrypted)

@@ -206,9 +200,7 @@ func (c *CFBCipher) Encrypt(value []byte) ([]byte, error) {

206200// Decrypt an AES CFB ciphertext

207201func (c *CFBCipher) Decrypt(ciphertext []byte) ([]byte, error) {

208202 if len(ciphertext) < aes.BlockSize {

209− return nil, fmt.Errorf("encrypted value should be "+

210− "at least %d bytes, but is only %d bytes",

211− aes.BlockSize, len(ciphertext))

203+ return nil, fmt.Errorf("encrypted value should be at least %d bytes, but is only %d bytes", aes.BlockSize, len(ciphertext))

212204 }

213205

214206 iv := ciphertext[:aes.BlockSize]

pkg/encryption/cipher_test.go+49−25

@@ -102,7 +102,7 @@ func TestSignAndValidate(t *testing.T) {

102102func TestEncodeAndDecodeAccessToken(t *testing.T) {

103103 const secret = "0123456789abcdefghijklmnopqrstuv"

104104 const token = "my access token"

105− c, err := NewCipher([]byte(secret))

105+ c, err := NewBase64Cipher(NewCFBCipher, []byte(secret))

106106 assert.Equal(t, nil, err)

107107

108108 encoded, err := c.Encrypt([]byte(token))

@@ -121,7 +121,7 @@ func TestEncodeAndDecodeAccessTokenB64(t *testing.T) {

121121

122122 secret, err := base64.URLEncoding.DecodeString(secretBase64)

123123 assert.Equal(t, nil, err)

124− c, err := NewCipher([]byte(secret))

124+ c, err := NewBase64Cipher(NewCFBCipher, []byte(secret))

125125 assert.Equal(t, nil, err)

126126

127127 encoded, err := c.Encrypt([]byte(token))

@@ -135,14 +135,12 @@ func TestEncodeAndDecodeAccessTokenB64(t *testing.T) {

135135}

136136

137137func TestEncryptAndDecrypt(t *testing.T) {

138− var err error

139−

140− // Test our 3 cipher types

141− for _, initCipher := range []func([]byte) (Cipher, error){NewCipher, NewCFBCipher, NewGCMCipher} {

138+ // Test our 2 cipher types

139+ for _, initCipher := range []func([]byte) (Cipher, error){NewCFBCipher, NewGCMCipher} {

142140 // Test all 3 valid AES sizes

143141 for _, secretSize := range []int{16, 24, 32} {

144142 secret := make([]byte, secretSize)

145− _, err = io.ReadFull(rand.Reader, secret)

143+ _, err := io.ReadFull(rand.Reader, secret)

146144 assert.Equal(t, nil, err)

147145

148146 c, err := initCipher(secret)

@@ -167,27 +165,55 @@ func TestEncryptAndDecrypt(t *testing.T) {

167165 }

168166}

169167

170−func TestDecryptWrongSecret(t *testing.T) {

168+func TestEncryptAndDecryptBase64(t *testing.T) {

169+ // Test our cipher types wrapped in Base64 encoder

170+ for _, initCipher := range []func([]byte) (Cipher, error){NewCFBCipher, NewGCMCipher} {

171+ // Test all 3 valid AES sizes

172+ for _, secretSize := range []int{16, 24, 32} {

173+ secret := make([]byte, secretSize)

174+ _, err := io.ReadFull(rand.Reader, secret)

175+ assert.Equal(t, nil, err)

176+

177+ c, err := NewBase64Cipher(initCipher, secret)

178+ assert.Equal(t, nil, err)

179+

180+ // Test various sizes sessions might be

181+ for _, dataSize := range []int{10, 100, 1000, 5000, 10000} {

182+ data := make([]byte, dataSize)

183+ _, err := io.ReadFull(rand.Reader, data)

184+ assert.Equal(t, nil, err)

185+

186+ encrypted, err := c.Encrypt(data)

187+ assert.Equal(t, nil, err)

188+ assert.NotEqual(t, encrypted, data)

189+

190+ decrypted, err := c.Decrypt(encrypted)

191+ assert.Equal(t, nil, err)

192+ assert.Equal(t, data, decrypted)

193+ assert.NotEqual(t, encrypted, decrypted)

194+ }

195+ }

196+ }

197+}

198+

199+func TestDecryptCFBWrongSecret(t *testing.T) {

171200 secret1 := []byte("0123456789abcdefghijklmnopqrstuv")

172201 secret2 := []byte("9876543210abcdefghijklmnopqrstuv")

173202

174− // Test CFB & Base64 (GCM is authenticated, it errors differently)

175− for _, initCipher := range []func([]byte) (Cipher, error){NewCipher, NewCFBCipher} {

176− c1, err := initCipher(secret1)

177− assert.Equal(t, nil, err)

203+ c1, err := NewCFBCipher(secret1)

204+ assert.Equal(t, nil, err)

178205

179− c2, err := initCipher(secret2)

180− assert.Equal(t, nil, err)

206+ c2, err := NewCFBCipher(secret2)

207+ assert.Equal(t, nil, err)

181208

182− data := []byte("f3928pufm982374dj02y485dsl34890u2t9nd4028s94dm58y2394087dhmsyt29h8df")

209+ data := []byte("f3928pufm982374dj02y485dsl34890u2t9nd4028s94dm58y2394087dhmsyt29h8df")

183210

184− ciphertext, err := c1.Encrypt(data)

185− assert.Equal(t, nil, err)

211+ ciphertext, err := c1.Encrypt(data)

212+ assert.Equal(t, nil, err)

186213

187− wrongData, err := c2.Decrypt(ciphertext)

188− assert.Equal(t, nil, err)

189− assert.NotEqual(t, data, wrongData)

190− }

214+ wrongData, err := c2.Decrypt(ciphertext)

215+ assert.Equal(t, nil, err)

216+ assert.NotEqual(t, data, wrongData)

191217}

192218

193219func TestDecryptGCMWrongSecret(t *testing.T) {

@@ -211,13 +237,11 @@ func TestDecryptGCMWrongSecret(t *testing.T) {

211237}

212238

213239func TestIntermixCiphersErrors(t *testing.T) {

214− var err error

215−

216240 // Encrypt with GCM, Decrypt with CFB: Results in Garbage data

217241 // Test all 3 valid AES sizes

218242 for _, secretSize := range []int{16, 24, 32} {

219243 secret := make([]byte, secretSize)

220− _, err = io.ReadFull(rand.Reader, secret)

244+ _, err := io.ReadFull(rand.Reader, secret)

221245 assert.Equal(t, nil, err)

222246

223247 gcm, err := NewGCMCipher(secret)

@@ -248,7 +272,7 @@ func TestIntermixCiphersErrors(t *testing.T) {

248272 // Test all 3 valid AES sizes

249273 for _, secretSize := range []int{16, 24, 32} {

250274 secret := make([]byte, secretSize)

251− _, err = io.ReadFull(rand.Reader, secret)

275+ _, err := io.ReadFull(rand.Reader, secret)

252276 assert.Equal(t, nil, err)

253277

254278 gcm, err := NewGCMCipher(secret)

pkg/sessions/session_store_test.go+1−1

@@ -367,7 +367,7 @@ var _ = Describe("NewSessionStore", func() {

367367 _, err := rand.Read(secret)

368368 Expect(err).ToNot(HaveOccurred())

369369 cookieOpts.Secret = base64.URLEncoding.EncodeToString(secret)

370− cipher, err := encryption.NewCipher(encryption.SecretBytes(cookieOpts.Secret))

370+ cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))

371371 Expect(err).ToNot(HaveOccurred())

372372 Expect(cipher).ToNot(BeNil())

373373 opts.Cipher = cipher

pkg/validation/options.go+1−1

@@ -62,7 +62,7 @@ func Validate(o *options.Options) error {

6262 len(encryption.SecretBytes(o.Cookie.Secret)), suffix))

6363 } else {

6464 var err error

65− cipher, err = encryption.NewCipher(encryption.SecretBytes(o.Cookie.Secret))

65+ cipher, err = encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(o.Cookie.Secret))

6666 if err != nil {

6767 msgs = append(msgs, fmt.Sprintf("cookie-secret error: %v", err))

6868 }