From 2658d0668247e1840255cedb6b4a943b3d0e5e63 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 20 Nov 2024 18:26:23 +0100 Subject: [PATCH] IDC 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 f01e9aa9..93068387 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" @@ -203,12 +201,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()), } @@ -668,7 +663,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 86dffdb2..636c9cfa 100644 --- a/internal/httpd/oidc_test.go +++ b/internal/httpd/oidc_test.go @@ -151,8 +151,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)), } @@ -561,7 +561,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)), @@ -665,7 +665,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)), @@ -779,7 +779,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)), } @@ -795,8 +795,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() @@ -810,7 +810,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", } @@ -1104,7 +1104,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}, @@ -1154,7 +1154,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 c8053815..fc61c00d 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" @@ -560,7 +562,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) @@ -570,6 +572,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 retuens an unique ID func GenerateUniqueID() string { u, err := uuid.NewRandom()