1683aa5 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Merge pull request #601 from oauth2-proxy/fix-fallback

Ensure decrypted user/email are valid UTF8

commit1683aa59780cd910e49561e7c046fe1782649fc8 authorJoel Speed <Joel.speed@hotmail.co.uk> dateFri 12 Jun 2020, 15:47 (6 years ago) parent2c851fc 808084b

3 files changed+19−17

compare with master

unifiedsplit

CHANGELOG.md+1 pkg/apis/sessions/session_state.go+8 pkg/apis/sessions/session_state_test.go+10−17

CHANGELOG.md+1

@@ -55,6 +55,7 @@

5555

5656## Changes since v5.1.1

5757

58+- [#601](https://github.com/oauth2-proxy/oauth2-proxy/pull/601) Ensure decrypted user/email are valid UTF8 (@JoelSpeed)

5859- [#560](https://github.com/oauth2-proxy/oauth2-proxy/pull/560) Fallback to UserInfo is User ID claim not present (@JoelSpeed)

5960- [#598](https://github.com/oauth2-proxy/oauth2-proxy/pull/598) acr_values no longer sent to IdP when empty (@ScottGuymer)

6061- [#548](https://github.com/oauth2-proxy/oauth2-proxy/pull/548) Separate logging options out of main options structure (@JoelSpeed)

pkg/apis/sessions/session_state.go+8

@@ -2,8 +2,10 @@ package sessions

33import (

44 "encoding/json"

5+ "errors"

56 "fmt"

67 "time"

8+ "unicode/utf8"

810 "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"

911)

@@ -106,6 +108,9 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {

106108 if ss.Email != "" {

107109 decryptedEmail, errEmail := c.Decrypt(ss.Email)

108110 if errEmail == nil {

111+ if !utf8.ValidString(decryptedEmail) {

112+ return nil, errors.New("invalid value for decrypted email")

113+ }

109114 ss.Email = decryptedEmail

110115 }

111116 }

@@ -113,6 +118,9 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {

113118 if ss.User != "" {

114119 decryptedUser, errUser := c.Decrypt(ss.User)

115120 if errUser == nil {

121+ if !utf8.ValidString(decryptedUser) {

122+ return nil, errors.New("invalid value for decrypted user")

123+ }

116124 ss.User = decryptedUser

117125 }

118126 }

pkg/apis/sessions/session_state_test.go+10−17

@@ -49,15 +49,7 @@ func TestSessionStateSerialization(t *testing.T) {

4949 // ensure a different cipher can't decode properly (ie: it gets gibberish)

5050 ss, err = sessions.DecodeSessionState(encoded, c2)

5151 t.Logf("%#v", ss)

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

53− assert.NotEqual(t, "user@domain.com", ss.User)

54− assert.NotEqual(t, s.Email, ss.Email)

55− assert.NotEqual(t, s.PreferredUsername, ss.PreferredUsername)

56− assert.Equal(t, s.CreatedAt.Unix(), ss.CreatedAt.Unix())

57− assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix())

58− assert.NotEqual(t, s.AccessToken, ss.AccessToken)

59− assert.NotEqual(t, s.IDToken, ss.IDToken)

60− assert.NotEqual(t, s.RefreshToken, ss.RefreshToken)

52+ assert.NotEqual(t, nil, err)

6153}

6254

6355func TestSessionStateSerializationWithUser(t *testing.T) {

@@ -91,14 +83,7 @@ func TestSessionStateSerializationWithUser(t *testing.T) {

9183 // ensure a different cipher can't decode properly (ie: it gets gibberish)

9284 ss, err = sessions.DecodeSessionState(encoded, c2)

9385 t.Logf("%#v", ss)

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

95− assert.NotEqual(t, s.User, ss.User)

96− assert.NotEqual(t, s.Email, ss.Email)

97− assert.NotEqual(t, s.PreferredUsername, ss.PreferredUsername)

98− assert.Equal(t, s.CreatedAt.Unix(), ss.CreatedAt.Unix())

99− assert.Equal(t, s.ExpiresOn.Unix(), ss.ExpiresOn.Unix())

100− assert.NotEqual(t, s.AccessToken, ss.AccessToken)

101− assert.NotEqual(t, s.RefreshToken, ss.RefreshToken)

86+ assert.NotEqual(t, nil, err)

10287}

10388

10489func TestSessionStateSerializationNoCipher(t *testing.T) {

@@ -278,6 +263,14 @@ func TestDecodeSessionState(t *testing.T) {

278263 Cipher: c,

279264 Error: true,

280265 },

266+ {

267+ SessionState: sessions.SessionState{

268+ Email: "user@domain.com",

269+ User: "YmFzZTY0LWVuY29kZWQtdXNlcgo=", // Base64 encoding of base64-encoded-user

270+ },

271+ Error: true,

272+ Cipher: c,

273+ },

281274 }

282275

283276 for i, tc := range testCases {