0832488 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

on master help sign in

Merge pull request #1906 from braunsonm/braunsonm/issue1897

0832488Braunson4 years ago

CHANGELOG.md+1

compare with master

unifiedsplit

@@ -11,6 +11,7 @@

11- [#1873](https://github.com/oauth2-proxy/oauth2-proxy/pull/1873) Fix empty users with some OIDC providers (@babs)

12- [#1882](https://github.com/oauth2-proxy/oauth2-proxy/pull/1882) Make `htpasswd.GetUsers` racecondition safe

13- [#1883](https://github.com/oauth2-proxy/oauth2-proxy/pull/1883) Ensure v8 manifest variant is set on docker images

14+- [#1906](https://github.com/oauth2-proxy/oauth2-proxy/pull/1906) Fix PKCE code verifier generation to never use UTF-8 characters

15# V7.4.0

16# V7.4.0

oauthproxy.go+6−5

@@ -2,7 +2,6 @@ package main

3import (

4 "context"

5− "encoding/base64"

6 "encoding/json"

5 "encoding/json"

7 "errors"

6 "errors"

8 "fmt"

7 "fmt"

@@ -737,16 +736,18 @@ func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, ove

737 extraParams := p.provider.Data().LoginURLParams(overrides)

736 extraParams := p.provider.Data().LoginURLParams(overrides)

738 prepareNoCache(rw)

737 prepareNoCache(rw)

739

738

739+ var (

740+ err error

740− var codeChallenge, codeVerifier, codeChallengeMethod string

741+ codeChallenge, codeVerifier, codeChallengeMethod string

742+ )

741 if p.provider.Data().CodeChallengeMethod != "" {

743 if p.provider.Data().CodeChallengeMethod != "" {

742 codeChallengeMethod = p.provider.Data().CodeChallengeMethod

744 codeChallengeMethod = p.provider.Data().CodeChallengeMethod

743− preEncodedCodeVerifier, err := encryption.Nonce(96)

745+ codeVerifier, err = encryption.GenerateRandomASCIIString(96)

744 if err != nil {

746 if err != nil {

745− logger.Errorf("Unable to build random string: %v", err)

747+ logger.Errorf("Unable to build random ASCII string for code verifier: %v", err)

746 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

748 p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())

747 return

749 return

748 }

750 }

749− codeVerifier = base64.RawURLEncoding.EncodeToString(preEncodedCodeVerifier)

750

751

751 codeChallenge, err = encryption.GenerateCodeChallenge(p.provider.Data().CodeChallengeMethod, codeVerifier)

752 codeChallenge, err = encryption.GenerateCodeChallenge(p.provider.Data().CodeChallengeMethod, codeVerifier)

752 if err != nil {

753 if err != nil {

pkg/encryption/utils.go+16

@@ -2,10 +2,12 @@ package encryption

3import (

4 "crypto/hmac"

5+ "crypto/rand"

5 "crypto/sha256"

6 "crypto/sha256"

6 "encoding/base64"

7 "encoding/base64"

7 "fmt"

8 "fmt"

8 "hash"

9 "hash"

10+ "math/big"

9 "net/http"

11 "net/http"

10 "strconv"

12 "strconv"

11 "strings"

13 "strings"

@@ -15,6 +17,7 @@ import (

15const (

17const (

16 CodeChallengeMethodPlain = "plain"

18 CodeChallengeMethodPlain = "plain"

17 CodeChallengeMethodS256 = "S256"

19 CodeChallengeMethodS256 = "S256"

20+ asciiCharset = " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~"

18)

21)

20// SecretBytes attempts to base64 decode the secret, if that fails it treats the secret as binary

23// SecretBytes attempts to base64 decode the secret, if that fails it treats the secret as binary

@@ -80,6 +83,19 @@ func SignedValue(seed string, key string, value []byte, now time.Time) (string,

80 return cookieVal, nil

83 return cookieVal, nil

81}

84}

86+func GenerateRandomASCIIString(length int) (string, error) {

87+ b := make([]byte, length)

88+ charsetLen := new(big.Int).SetInt64(int64(len(asciiCharset)))

89+ for i := range b {

90+ character, err := rand.Int(rand.Reader, charsetLen)

91+ if err != nil {

92+ return "", err

93+ }

94+ b[i] = asciiCharset[character.Int64()]

95+ }

96+ return string(b), nil

97+}

98+

83func GenerateCodeChallenge(method, codeVerifier string) (string, error) {

99func GenerateCodeChallenge(method, codeVerifier string) (string, error) {

84 switch method {

100 switch method {

85 case CodeChallengeMethodPlain:

101 case CodeChallengeMethodPlain:

pkg/encryption/utils_test.go+19

@@ -7,7 +7,9 @@ import (

7 "encoding/base64"

8 "fmt"

9 "io"

10+ "strings"

10 "testing"

11 "testing"

12+ "unicode"

12 "github.com/stretchr/testify/assert"

14 "github.com/stretchr/testify/assert"

13)

15)

@@ -100,3 +102,20 @@ func TestSignAndValidate(t *testing.T) {

100 assert.False(t, checkSignature(sha256sig, seed, key, "tampered", epoch))

102 assert.False(t, checkSignature(sha256sig, seed, key, "tampered", epoch))

101 assert.False(t, checkSignature(sha1sig, seed, key, "tampered", epoch))

103 assert.False(t, checkSignature(sha1sig, seed, key, "tampered", epoch))

102}

104}

105+

106+func TestGenerateRandomASCIIString(t *testing.T) {

107+ randomString, err := GenerateRandomASCIIString(96)

108+ assert.NoError(t, err)

109+

110+ // Only 8-bit characters

111+ assert.Equal(t, 96, len([]byte(randomString)))

112+

113+ // All non-ascii characters removed should still be the original string

114+ removedChars := strings.Map(func(r rune) rune {

115+ if r > unicode.MaxASCII {

116+ return -1

117+ }

118+ return r

119+ }, randomString)

120+ assert.Equal(t, removedChars, randomString)

121+}