From f30a9a2095bf90c0661b04fe038e3b7efc788bc6 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 20 Nov 2024 18:28:43 +0100 Subject: [PATCH] OIDC cookie: use a cryptographically secure random string Signed-off-by: Nicola Murino --- internal/httpd/oauth2.go | 6 +----- internal/httpd/oidc.go | 11 +++-------- internal/httpd/oidc_test.go | 22 +++++++++++----------- internal/util/util.go | 10 +++++++++- 4 files changed, 24 insertions(+), 25 deletions(-) diff --git a/internal/httpd/oauth2.go b/internal/httpd/oauth2.go index 49459161..88a599d3 100644 --- a/internal/httpd/oauth2.go +++ b/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, diff --git a/internal/httpd/oidc.go b/internal/httpd/oidc.go index 228eed91..1bdaccab 100644 --- a/internal/httpd/oidc.go +++ b/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) diff --git a/internal/httpd/oidc_test.go b/internal/httpd/oidc_test.go index b84926d4..9a098b36 100644 --- a/internal/httpd/oidc_test.go +++ b/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(), diff --git a/internal/util/util.go b/internal/util/util.go index b439b385..9f4f14dc 100644 --- a/internal/util/util.go +++ b/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()