diff --git a/server/home.go b/server/home.go index ee0c496a..168b70e0 100644 --- a/server/home.go +++ b/server/home.go @@ -2,6 +2,7 @@ package server import ( "context" + "crypto/subtle" "errors" "fmt" "net/http" @@ -24,7 +25,7 @@ func (s *Server) handleHome(w http.ResponseWriter, r *http.Request) { if cookie, err := r.Cookie(s.sessionConfig.CookieName); err == nil && cookie.Value != "" { if userID, connectorID, nonce, err := parseSessionCookie(cookie.Value, s.sessionConfig.CookieEncryptionKey); err == nil { session, err := s.storage.GetAuthSession(ctx, userID, connectorID) - if err == nil && session.Nonce == nonce { + if err == nil && subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) == 1 { data.LoggedIn = true data.IPAddress = session.IPAddress data.UserAgent = session.UserAgent diff --git a/server/logout.go b/server/logout.go index c3a0699f..d48f7a59 100644 --- a/server/logout.go +++ b/server/logout.go @@ -2,6 +2,7 @@ package server import ( "context" + "crypto/subtle" "errors" "net/http" "net/url" @@ -94,7 +95,7 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) { if cookie, err := r.Cookie(s.sessionConfig.CookieName); err == nil && cookie.Value != "" { if uid, cid, nonce, err := parseSessionCookie(cookie.Value, s.sessionConfig.CookieEncryptionKey); err == nil { // Verify the session exists and nonce matches before trusting the cookie. - if session, err := s.storage.GetAuthSession(ctx, uid, cid); err == nil && session.Nonce == nonce { + if session, err := s.storage.GetAuthSession(ctx, uid, cid); err == nil && subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) == 1 { userID = uid connectorID = cid s.logger.DebugContext(ctx, "logout: identified user from session cookie", @@ -178,7 +179,7 @@ func (s *Server) handleLogoutCallback(w http.ResponseWriter, r *http.Request) { return } - if session.Nonce != nonce { + if subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) != 1 { s.renderError(r, w, http.StatusBadRequest, "Invalid session.") return } diff --git a/server/session.go b/server/session.go index e4753b47..b5adbeb9 100644 --- a/server/session.go +++ b/server/session.go @@ -5,6 +5,7 @@ import ( "crypto/aes" "crypto/cipher" "crypto/rand" + "crypto/subtle" "encoding/base64" "errors" "fmt" @@ -178,7 +179,9 @@ func (s *Server) getValidSession(ctx context.Context, w http.ResponseWriter, r * } // Verify nonce to prevent cookie forgery. - if session.Nonce != nonce { + // Use constant-time comparison to prevent timing attacks that could + // allow an attacker to recover the nonce byte-by-byte. + if subtle.ConstantTimeCompare([]byte(session.Nonce), []byte(nonce)) != 1 { s.logger.DebugContext(ctx, "auth session nonce mismatch") s.clearSessionCookie(w) return nil diff --git a/server/session_test.go b/server/session_test.go index 91253e8a..155b91a7 100644 --- a/server/session_test.go +++ b/server/session_test.go @@ -1432,6 +1432,56 @@ func TestFinishSessionLogin_MFA(t *testing.T) { }) } +// TestNonceVerificationRejectsForgedCookie verifies that a session cookie +// with a valid (userID, connectorID) but wrong nonce is rejected. +// The nonce comparison uses constant-time comparison to prevent timing attacks. +func TestNonceVerificationRejectsForgedCookie(t *testing.T) { + ctx := t.Context() + s := newTestSessionServer(t) + now := s.now() + + require.NoError(t, s.storage.CreateAuthSession(ctx, storage.AuthSession{ + UserID: "user-1", ConnectorID: "mock", Nonce: "real-nonce", + CreatedAt: now.Add(-10 * time.Minute), LastActivity: now.Add(-1 * time.Minute), + AbsoluteExpiry: now.Add(24 * time.Hour), IdleExpiry: now.Add(59 * time.Minute), + })) + + tests := []struct { + name string + nonce string + }{ + {"wrong nonce", "wrong-nonce"}, + {"empty nonce", ""}, + {"prefix of real nonce", "real"}, + {"real nonce with suffix", "real-nonce-extra"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + r := sessionCookieRequest("user-1", "mock", tc.nonce) + w := httptest.NewRecorder() + + session := s.getValidSession(ctx, w, r) + assert.Nil(t, session, "session with forged nonce %q should be rejected", tc.nonce) + + // Cookie should be cleared on nonce mismatch. + for _, c := range w.Result().Cookies() { + if c.Name == "dex_session" { + assert.Equal(t, -1, c.MaxAge, "cookie should be cleared") + } + } + }) + } + + t.Run("correct nonce accepted", func(t *testing.T) { + r := sessionCookieRequest("user-1", "mock", "real-nonce") + w := httptest.NewRecorder() + + session := s.getValidSession(ctx, w, r) + require.NotNil(t, session) + assert.Equal(t, "user-1", session.UserID) + }) +} + // TestPromptNone tests the prompt=none silent authentication scenarios. // These verify the code paths in handleConnectorLogin (handlers.go:444-457) // where prompt=none requires session-based login without any UI.