Fix PKCE code verifier generation to never use UTF-8 characters

- This could result in intermittent/random failures of PKCE enabled IdP's

CHANGELOG.md

@@ -11,6 +11,7 @@
 - [#1873](https://github.com/oauth2-proxy/oauth2-proxy/pull/1873) Fix empty users with some OIDC providers (@babs)
 - [#1882](https://github.com/oauth2-proxy/oauth2-proxy/pull/1882) Make `htpasswd.GetUsers` racecondition safe
 - [#1883](https://github.com/oauth2-proxy/oauth2-proxy/pull/1883) Ensure v8 manifest variant is set on docker images
+- [#1906](https://github.com/oauth2-proxy/oauth2-proxy/pull/1906) Fix PKCE code verifier generation to never use UTF-8 characters
 
 # V7.4.0
 

oauthproxy.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"context"
-	"encoding/base64"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -737,16 +736,18 @@ func (p *OAuthProxy) doOAuthStart(rw http.ResponseWriter, req *http.Request, ove
 	extraParams := p.provider.Data().LoginURLParams(overrides)
 	prepareNoCache(rw)
 
-	var codeChallenge, codeVerifier, codeChallengeMethod string
+	var (
+		err                                              error
+		codeChallenge, codeVerifier, codeChallengeMethod string
+	)
 	if p.provider.Data().CodeChallengeMethod != "" {
 		codeChallengeMethod = p.provider.Data().CodeChallengeMethod
-		preEncodedCodeVerifier, err := encryption.Nonce(96)
+		codeVerifier, err = encryption.GenerateRandomASCIIString(96)
 		if err != nil {
-			logger.Errorf("Unable to build random string: %v", err)
+			logger.Errorf("Unable to build random ASCII string for code verifier: %v", err)
 			p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
 			return
 		}
-		codeVerifier = base64.RawURLEncoding.EncodeToString(preEncodedCodeVerifier)
 
 		codeChallenge, err = encryption.GenerateCodeChallenge(p.provider.Data().CodeChallengeMethod, codeVerifier)
 		if err != nil {

pkg/encryption/utils.go

@@ -2,10 +2,12 @@ package encryption
 
 import (
 	"crypto/hmac"
+	"crypto/rand"
 	"crypto/sha256"
 	"encoding/base64"
 	"fmt"
 	"hash"
+	"math/big"
 	"net/http"
 	"strconv"
 	"strings"
@@ -15,6 +17,7 @@ import (
 const (
 	CodeChallengeMethodPlain = "plain"
 	CodeChallengeMethodS256  = "S256"
+	asciiCharset             = " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~"
 )
 
 // 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,
 	return cookieVal, nil
 }
 
+func GenerateRandomASCIIString(length int) (string, error) {
+	b := make([]byte, length)
+	charsetLen := new(big.Int).SetInt64(int64(len(asciiCharset)))
+	for i := range b {
+		character, err := rand.Int(rand.Reader, charsetLen)
+		if err != nil {
+			return "", err
+		}
+		b[i] = asciiCharset[character.Int64()]
+	}
+	return string(b), nil
+}
+
 func GenerateCodeChallenge(method, codeVerifier string) (string, error) {
 	switch method {
 	case CodeChallengeMethodPlain:

pkg/encryption/utils_test.go

@@ -7,7 +7,9 @@ import (
 	"encoding/base64"
 	"fmt"
 	"io"
+	"strings"
 	"testing"
+	"unicode"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -100,3 +102,20 @@ func TestSignAndValidate(t *testing.T) {
 	assert.False(t, checkSignature(sha256sig, seed, key, "tampered", epoch))
 	assert.False(t, checkSignature(sha1sig, seed, key, "tampered", epoch))
 }
+
+func TestGenerateRandomASCIIString(t *testing.T) {
+	randomString, err := GenerateRandomASCIIString(96)
+	assert.NoError(t, err)
+
+	// Only 8-bit characters
+	assert.Equal(t, 96, len([]byte(randomString)))
+
+	// All non-ascii characters removed should still be the original string
+	removedChars := strings.Map(func(r rune) rune {
+		if r > unicode.MaxASCII {
+			return -1
+		}
+		return r
+	}, randomString)
+	assert.Equal(t, removedChars, randomString)
+}