OIDC cookie: use a cryptographically secure random string

Signed-off-by: Nicola Murino <[email protected]>

internal/httpd/oauth2.go

@@ -15,8 +15,6 @@
 package httpd
 
 import (
-	"crypto/sha256"
-	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"sync"
@@ -53,10 +51,8 @@ type oauth2PendingAuth struct {
 }
 
 func newOAuth2PendingAuth(provider int, redirectURL, clientID string, clientSecret *kms.Secret) oauth2PendingAuth {
-	state := sha256.Sum256(util.GenerateRandomBytes(32))
-
 	return oauth2PendingAuth{
-		State:        hex.EncodeToString(state[:]),
+		State:        util.GenerateOpaqueString(),
 		Provider:     provider,
 		ClientID:     clientID,
 		ClientSecret: clientSecret,

internal/httpd/oidc.go

@@ -16,8 +16,6 @@ package httpd
 
 import (
 	"context"
-	"crypto/sha256"
-	"encoding/hex"
 	"errors"
 	"fmt"
 	"net/http"
@@ -204,12 +202,9 @@ type oidcPendingAuth struct {
 }
 
 func newOIDCPendingAuth(audience tokenAudience) oidcPendingAuth {
-	state := sha256.Sum256(util.GenerateRandomBytes(32))
-	nonce := util.GenerateUniqueID()
-
 	return oidcPendingAuth{
-		State:    hex.EncodeToString(state[:]),
-		Nonce:    nonce,
+		State:    util.GenerateOpaqueString(),
+		Nonce:    util.GenerateOpaqueString(),
 		Audience: audience,
 		IssuedAt: util.GetTimeAsMsSinceEpoch(time.Now()),
 	}
@@ -684,7 +679,7 @@ func (s *httpdServer) handleOIDCRedirect(w http.ResponseWriter, r *http.Request)
 		RefreshToken: oauth2Token.RefreshToken,
 		IDToken:      rawIDToken,
 		Nonce:        idToken.Nonce,
-		Cookie:       xid.New().String(),
+		Cookie:       util.GenerateOpaqueString(),
 	}
 	if !oauth2Token.Expiry.IsZero() {
 		token.ExpiresAt = util.GetTimeAsMsSinceEpoch(oauth2Token.Expiry)

internal/httpd/oidc_test.go

@@ -152,8 +152,8 @@ func TestOIDCLoginLogout(t *testing.T) {
 	assert.Contains(t, rr.Body.String(), util.I18nInvalidAuth)
 
 	expiredAuthReq := oidcPendingAuth{
-		State:    xid.New().String(),
-		Nonce:    xid.New().String(),
+		State:    util.GenerateOpaqueString(),
+		Nonce:    util.GenerateOpaqueString(),
 		Audience: tokenAudienceWebClient,
 		IssuedAt: util.GetTimeAsMsSinceEpoch(time.Now().Add(-10 * time.Minute)),
 	}
@@ -564,7 +564,7 @@ func TestOIDCRefreshToken(t *testing.T) {
 	r, err := http.NewRequest(http.MethodGet, webUsersPath, nil)
 	assert.NoError(t, err)
 	token := oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		TokenType:   "Bearer",
 		ExpiresAt:   util.GetTimeAsMsSinceEpoch(time.Now().Add(-1 * time.Minute)),
@@ -668,7 +668,7 @@ func TestOIDCRefreshToken(t *testing.T) {
 
 func TestOIDCRefreshUser(t *testing.T) {
 	token := oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		TokenType:   "Bearer",
 		ExpiresAt:   util.GetTimeAsMsSinceEpoch(time.Now().Add(1 * time.Minute)),
@@ -782,7 +782,7 @@ func TestValidateOIDCToken(t *testing.T) {
 		},
 	}
 	token := oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		ExpiresAt:   util.GetTimeAsMsSinceEpoch(time.Now().Add(-2 * time.Minute)),
 	}
@@ -798,8 +798,8 @@ func TestValidateOIDCToken(t *testing.T) {
 
 	server.tokenAuth = jwtauth.New("PS256", util.GenerateRandomBytes(32), nil)
 	token = oidcToken{
-		Cookie:      xid.New().String(),
-		AccessToken: xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
+		AccessToken: util.GenerateUniqueID(),
 	}
 	oidcMgr.addToken(token)
 	rr = httptest.NewRecorder()
@@ -813,7 +813,7 @@ func TestValidateOIDCToken(t *testing.T) {
 	assert.Len(t, oidcMgr.tokens, 0)
 
 	token = oidcToken{
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		AccessToken: xid.New().String(),
 		Role:        "admin",
 	}
@@ -1107,7 +1107,7 @@ func TestMemoryOIDCManager(t *testing.T) {
 		AccessToken: xid.New().String(),
 		Nonce:       xid.New().String(),
 		SessionID:   xid.New().String(),
-		Cookie:      xid.New().String(),
+		Cookie:      util.GenerateOpaqueString(),
 		Username:    xid.New().String(),
 		Role:        "admin",
 		Permissions: []string{dataprovider.PermAdminAny},
@@ -1157,7 +1157,7 @@ func TestMemoryOIDCManager(t *testing.T) {
 	token.UsedAt = usedAt
 	oidcMgr.tokens[token.Cookie] = token
 	newToken := oidcToken{
-		Cookie: xid.New().String(),
+		Cookie: util.GenerateOpaqueString(),
 	}
 	oidcMgr.addToken(newToken)
 	oidcMgr.cleanup()
@@ -1663,7 +1663,7 @@ func TestDbOIDCManager(t *testing.T) {
 	}
 
 	token := oidcToken{
-		Cookie:       xid.New().String(),
+		Cookie:       util.GenerateOpaqueString(),
 		AccessToken:  xid.New().String(),
 		TokenType:    "Bearer",
 		RefreshToken: xid.New().String(),

internal/util/util.go

@@ -22,8 +22,10 @@ import (
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/rsa"
+	"crypto/sha256"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/hex"
 	"encoding/json"
 	"encoding/pem"
 	"errors"
@@ -550,7 +552,7 @@ func createDirPathIfMissing(file string, perm os.FileMode) error {
 	return nil
 }
 
-// GenerateRandomBytes generates the secret to use for JWT auth
+// GenerateRandomBytes generates random bytes with the specified length
 func GenerateRandomBytes(length int) []byte {
 	b := make([]byte, length)
 	_, err := io.ReadFull(rand.Reader, b)
@@ -560,6 +562,12 @@ func GenerateRandomBytes(length int) []byte {
 	return b
 }
 
+// GenerateOpaqueString generates a cryptographically secure opaque string
+func GenerateOpaqueString() string {
+	randomBytes := sha256.Sum256(GenerateRandomBytes(32))
+	return hex.EncodeToString(randomBytes[:])
+}
+
 // GenerateUniqueID returns an unique ID
 func GenerateUniqueID() string {
 	u, err := uuid.NewRandom()