diff --git a/server/internal/wsapi/chat_test.go b/server/internal/wsapi/chat_test.go index d0f6b8a..4246882 100644 --- a/server/internal/wsapi/chat_test.go +++ b/server/internal/wsapi/chat_test.go @@ -315,10 +315,12 @@ func TestBotRoomHasNoChat(t *testing.T) { // TestChatDoesNotKeepARoomAlive: talking is not playing. Without this a room // is held open for the life of the process by one message every nine minutes. func TestChatDoesNotKeepARoomAlive(t *testing.T) { - api, url := newTestServer(t, chainDict(), Config{IdleFor: 300 * time.Millisecond}) + const idleFor = 300 * time.Millisecond + api, url := newTestServer(t, chainDict(), Config{IdleFor: idleFor}) host := dial(t, url) host.hello("Chủ phòng") + start := time.Now() host.createRoom() host.await("room_state") @@ -331,6 +333,11 @@ func TestChatDoesNotKeepARoomAlive(t *testing.T) { if got := host.await("error").GetError().GetCode(); got != "room_idle_closed" { t.Errorf("a chatted-in room closed with %q, want room_idle_closed", got) } + // Closing on time is the assertion: a chat that restarted the clock would + // still close the room, but a window later than this one. + if elapsed := time.Since(start); elapsed > idleFor+200*time.Millisecond { + t.Errorf("the room closed after %v, want about %v: chat restarted the idle clock", elapsed, idleFor) + } awaitNoRooms(t, api, "a room held open by chat") } diff --git a/server/internal/wsapi/convert_test.go b/server/internal/wsapi/convert_test.go index 306cfa7..7181b26 100644 --- a/server/internal/wsapi/convert_test.go +++ b/server/internal/wsapi/convert_test.go @@ -1,6 +1,7 @@ package wsapi import ( + "strings" "testing" noituv1 "github.com/tiennm99dev/noitu/server/gen/noitu/v1" @@ -157,6 +158,9 @@ func TestPointKindMappingIsExhaustive(t *testing.T) { t.Errorf("game.PointKind(%d) %q has no wire mapping", int(k), k) continue } + if want := "POINT_KIND_" + strings.ToUpper(k.String()); got.String() != want { + t.Errorf("game.PointKind %q maps to %v, want %s", k, got, want) + } if prev, dup := seen[got]; dup { t.Errorf("%v is produced by both %q and %q", got, prev, k) } diff --git a/server/internal/wsapi/corpuslog.go b/server/internal/wsapi/corpuslog.go new file mode 100644 index 0000000..6089ddc --- /dev/null +++ b/server/internal/wsapi/corpuslog.go @@ -0,0 +1,55 @@ +package wsapi + +import ( + "log/slog" + "sync/atomic" + "time" +) + +// Corpus log lines are word_rejected and word_reported: the feedback loop the +// dictionary is tuned by. Each session is already bounded on how many it can +// cause, but a session is free to reconnect, so the per-session bounds do not +// bound the process. This is the bound that does. +const ( + // corpusLogPerSecond and corpusLogBurst are the process-wide budget for + // those lines. Far above what real play produces — a rejection is a typo + // or a missing word, a few a minute per room — so only a flood is sampled. + corpusLogPerSecond = 20 + corpusLogBurst = 100 +) + +// corpusLogger writes the corpus lines through one process-wide bucket. The +// counters that mirror them stay exact; only the log is sampled, so a flood +// cannot fill the disk or rotate the real signal out. +type corpusLogger struct { + bucket *bucket + suppressed atomic.Int64 + // out writes one line that got through; slog.Info outside of tests. + out func(msg string, args ...any) +} + +func newCorpusLogger(now time.Time) *corpusLogger { + return &corpusLogger{ + bucket: newBucket(corpusLogPerSecond, corpusLogBurst, now), + out: slog.Info, + } +} + +// corpusLog is the one instance every corpus line goes through. +var corpusLog = newCorpusLogger(time.Now()) + +// info logs one corpus line, or counts it as suppressed when the budget is +// spent. The first line to get through after a suppressed run carries how many +// were dropped, so the gap is visible in the log itself and not only in the +// noitu_corpus_log_suppressed counter. +func (c *corpusLogger) info(now time.Time, msg string, args ...any) { + if !c.bucket.allow(now) { + c.suppressed.Add(1) + metrics.corpusLogSuppressed.Add(1) + return + } + if n := c.suppressed.Swap(0); n > 0 { + args = append(args, "suppressed_before", n) + } + c.out(msg, args...) +} diff --git a/server/internal/wsapi/dispatch.go b/server/internal/wsapi/dispatch.go index 2aa4a87..7e927d2 100644 --- a/server/internal/wsapi/dispatch.go +++ b/server/internal/wsapi/dispatch.go @@ -111,11 +111,12 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { }) case *noituv1.ClientMessage_Resign: - // No budget of its own beyond the frame limiter: only one can ever be - // accepted per game, and a refusal goes to the sender alone. A silently - // dropped resignation leaves the player staring at a board they thought - // they had left. - s.toRoom(nil, codeNotInAGame, codeGameAlreadyOver, func(id game.PlayerID) any { + // Only one can ever be accepted per game, but every refusal is still an + // input the room has to drain, and a lobby full of refused resignations + // would crowd out the notices that matter. It shares the submission + // budget for that reason. A silently dropped resignation leaves the + // player staring at a board they thought they had left. + s.toRoom(s.submitLimiter, codeNotInAGame, codeGameAlreadyOver, func(id game.PlayerID) any { return resignInput{sess: s, player: id} }) @@ -154,6 +155,11 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { case *noituv1.ClientMessage_Ping: s.send(pongMsg(p.Ping.GetClientTimeMs(), time.Now().UnixMilli())) + + default: + // An empty payload, or one from a client newer than this server. Saying + // nothing leaves the sender waiting on an answer that is not coming. + s.send(errorMsg(codeUnknownMessage)) } return nil } @@ -161,8 +167,12 @@ func (s *session) dispatch(msg *noituv1.ClientMessage) error { // allowRoom charges the room budget, answering too_many_rooms when it is // spent. Every path that mints a room — a bot game, a code, a quick match — // holds a goroutine and an engine, so all three share it. +// +// Two budgets apply: the connection's own, and the client address's, which a +// reconnect does not reset. func (s *session) allowRoom() bool { - if s.roomLimiter.allow(time.Now()) { + now := time.Now() + if s.roomLimiter.allow(now) && s.hub.roomLimiter.allow(s.remoteIP, now) { return true } s.send(errorMsg(codeTooManyRooms)) @@ -259,6 +269,9 @@ func (s *session) handleHello(h *noituv1.Hello) error { return errors.New("wsapi: repeated hello") } s.greeted = true + if s.helloTimer != nil { + s.helloTimer.Stop() + } s.setNickname(sanitizeNickname(h.GetNickname())) s.hub.register(s) @@ -294,7 +307,13 @@ func (s *session) resumeFrom(prior *session) { return } if !r.send(resumeInput{player: id, sess: s, prior: prior}) { - s.send(errorMsg(codeGameAlreadyOver)) + // A room that has finished says so; one that is only busy is still + // there, and calling it over would be a false statement. + if r.ctx.Err() != nil { + s.send(errorMsg(codeGameAlreadyOver)) + } else { + s.send(errorMsg(codeBusy)) + } return } // Deliberately no attach and no close here. The room has not decided yet, @@ -341,16 +360,20 @@ func (s *session) handleReportWord(m *noituv1.ReportWord) { } metrics.wordsReported.Add(1) - slog.Info("word_reported", "word", word, "link", "", "mode", "none", "room", "") + corpusLog.info(time.Now(), "word_reported", "word", word, "link", "", "mode", "none", "room", "") s.send(wordReportedMsg(word)) } // leaveRoom tells the room this connection is gone, so the seat enters its // grace window rather than the game simply stalling. +// +// Delivered reliably: this runs as the session goroutine ends, where waiting +// for room is harmless, and a notice lost to a full inbox would leave a dead +// socket bound to the seat, counted as connected and never expiring. func (s *session) leaveRoom() { r, id := s.currentRoom() if r == nil { return } - r.send(disconnectInput{player: id, sess: s}) + r.sendReliably(disconnectInput{player: id, sess: s}) } diff --git a/server/internal/wsapi/errcodes.go b/server/internal/wsapi/errcodes.go index 82b45e0..4178593 100644 --- a/server/internal/wsapi/errcodes.go +++ b/server/internal/wsapi/errcodes.go @@ -45,6 +45,7 @@ const ( codeTooFast errCode = "too_fast" codeTooManyAttempts errCode = "too_many_attempts" codeTooManyRooms errCode = "too_many_rooms" + codeUnknownMessage errCode = "unknown_message" codeUnknownDifficulty errCode = "unknown_difficulty" codeWordReportLimit errCode = "word_report_limit" codeWordReportRefused errCode = "word_report_refused" diff --git a/server/internal/wsapi/hardening_test.go b/server/internal/wsapi/hardening_test.go new file mode 100644 index 0000000..b43d678 --- /dev/null +++ b/server/internal/wsapi/hardening_test.go @@ -0,0 +1,533 @@ +package wsapi + +import ( + "context" + "net/http" + "net/http/httptest" + "slices" + "testing" + "time" + + "github.com/coder/websocket" + noituv1 "github.com/tiennm99dev/noitu/server/gen/noitu/v1" + "google.golang.org/protobuf/proto" +) + +// Lifecycle and abuse-surface guarantees at the edges of the room actor: what +// a seat id means after its seat is freed, what happens to inputs queued +// behind a room's last one, and what one address may hold. + +// handlerRoom builds a room that is driven by direct handler calls, without +// its goroutine running, for the guarantees that live below the transport. +func handlerRoom(t *testing.T) *room { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + h := newHub(ctx, chainDict(), time.Second, time.Second, time.Minute, 10) + return newRoom(h, "AAAAAA", time.Second, time.Second, time.Minute, roomModePvP) +} + +// awaitFrame blocks on a session's outbox until a frame satisfies want, so a +// handler-level test waits on the event itself rather than on a sleep. +func awaitFrame(t *testing.T, s *session, want func(*noituv1.ServerMessage) bool) *noituv1.ServerMessage { + t.Helper() + timeout := time.After(5 * time.Second) + for { + select { + case raw := <-s.out: + var m noituv1.ServerMessage + if err := proto.Unmarshal(raw, &m); err != nil { + t.Fatalf("decode: %v", err) + } + if want(&m) { + return &m + } + case <-timeout: + t.Fatal("the awaited frame never reached the session") + } + } +} + +func errorCode(m *noituv1.ServerMessage) string { return m.GetError().GetCode() } + +// TestKickedPlayersTokenCannotResumeIntoAReusedSeat: seat ids are reused. A +// player who dropped, was kicked, and comes back inside their grace window +// must be refused, not dropped into whoever holds the id now. +func TestKickedPlayersTokenCannotResumeIntoAReusedSeat(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{GraceFor: 10 * time.Second}) + + host := dial(t, url) + host.hello("Chủ phòng") + host.createRoom() + code := host.await("room_state").GetRoomState().GetRoomCode() + + alice := dial(t, url) + aliceWelcome := alice.hello("Alice") + alice.joinRoom(code) + host.await("room_state") + + _ = alice.conn.Close(websocket.StatusAbnormalClosure, "") + host.await("room_state") // Alice is away + + host.kickPlayer("p2") + host.await("room_state") // her seat is free + + carol := dial(t, url) + carolWelcome := carol.hello("Carol") + carol.joinRoom(code) + host.await("room_state") + carol.say("carol-only secret") + host.await("chat_message") + + _ = carol.conn.Close(websocket.StatusAbnormalClosure, "") + host.await("room_state") // Carol is away, on the seat Alice used to hold + + stale := resumeAs(t, url, "Alice", aliceWelcome.GetResumeToken()) + if got := stale.await("error").GetError().GetCode(); got != "session_not_resumable" { + t.Fatalf("a kicked player's token got %q, want session_not_resumable", got) + } + + // The seat still belongs to its real occupant. + back := resumeAs(t, url, "Carol", carolWelcome.GetResumeToken()) + history := back.await("chat_history").GetChatHistory().GetMessages() + if len(history) != 1 || !history[0].GetFromMe() { + t.Errorf("the real occupant's resume did not restore their own seat: %+v", history) + } +} + +// TestRoomExitAnswersInputsQueuedBehindTheLastOne: the owner leaves an empty +// room while a friend's join is already queued behind it. The room is gone by +// the time the join is reached, and the friend must be told so rather than +// waiting forever. +func TestRoomExitAnswersInputsQueuedBehindTheLastOne(t *testing.T) { + r := handlerRoom(t) + host, friend := offlineSession(t, 64), offlineSession(t, 64) + + r.inputs <- createInput{sess: host} + r.inputs <- lobbyInput{sess: host, player: "p1", action: lobbyLeave} + r.inputs <- joinInput{sess: friend} + r.inputs <- resumeInput{player: "p1", sess: friend, prior: host} + r.run() + + got := queued(t, friend) + var codes []string + for _, m := range got { + codes = append(codes, errorCode(m)) + } + if !slices.Contains(codes, "room_not_found") || !slices.Contains(codes, "session_not_resumable") { + t.Errorf("inputs queued behind the room's last were answered with %v, want room_not_found and session_not_resumable", codes) + } +} + +// TestDisconnectNoticeSurvivesAFullInbox: the notice that a socket died is +// what starts its seat's reconnect window. Dropped by a full inbox, the seat +// stays bound to a dead connection and counts as connected forever. +func TestDisconnectNoticeSurvivesAFullInbox(t *testing.T) { + r := handlerRoom(t) + host := offlineSession(t, 4*roomInputCap) + host.room, host.playerID = r, "p1" + + r.inputs <- createInput{sess: host} + for len(r.inputs) < cap(r.inputs) { + r.inputs <- lobbyInput{sess: host, player: "p1", action: lobbyReady, ready: true} + } + + left := make(chan struct{}) + go func() { + host.leaveRoom() + close(left) + }() + go r.run() + + select { + case <-left: + case <-time.After(5 * time.Second): + t.Fatal("leaveRoom never returned") + } + + // FIFO: this is handled after the disconnect, so a seat that entered its + // grace window refuses it, and one still bound to the dead connection + // delivers it. + r.sendReliably(chatInput{sess: host, player: "p1", text: "still here?"}) + got := awaitFrame(t, host, func(m *noituv1.ServerMessage) bool { + return m.GetChatMessage() != nil || errorCode(m) == "not_your_seat" + }) + if errorCode(got) != "not_your_seat" { + t.Errorf("the seat is still bound to the disconnected session: got %s", describe(got)) + } +} + +// TestBusyRoomIsNotReportedAsOver: a resume that finds the room's inbox full +// is being told the room is busy; game_already_over would be a false +// statement about a room that is alive. +func TestBusyRoomIsNotReportedAsOver(t *testing.T) { + r := handlerRoom(t) + prior, next := offlineSession(t, 8), offlineSession(t, 8) + prior.room, prior.playerID = r, "p1" + + for len(r.inputs) < cap(r.inputs) { + r.inputs <- lobbyInput{} + } + next.resumeFrom(prior) + if got := queued(t, next); len(got) != 1 || errorCode(got[0]) != "busy" { + t.Errorf("resume into a busy room got %v, want busy", got) + } + + r.cancel() + next.resumeFrom(prior) + if got := queued(t, next); len(got) != 1 || errorCode(got[0]) != "game_already_over" { + t.Errorf("resume into a finished room got %v, want game_already_over", got) + } +} + +// TestRefusedResignationsAreRateLimited: a resignation costs the room an +// input even when it is refused, so it shares the submission budget instead +// of being free. +func TestRefusedResignationsAreRateLimited(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{}) + host := dial(t, url) + host.hello("Chủ phòng") + host.createRoom() + host.await("room_state") + + const sent = submitBurst + 5 + for range sent { + host.resign() + } + throttled := 0 + for range sent { + if host.await("error").GetError().GetCode() == "too_fast" { + throttled++ + } + } + if throttled == 0 { + t.Error("no refused resignation was rate limited") + } +} + +// TestQuickMatchAutoStartDoesNotOutliveItsPairing: the pairing's joiner died +// before it was seated. The room must not start a game for whoever fills the +// second seat afterwards, because nobody readied for it. +func TestQuickMatchAutoStartDoesNotOutliveItsPairing(t *testing.T) { + r := handlerRoom(t) + waiter, partner, friend := offlineSession(t, 64), offlineSession(t, 64), offlineSession(t, 64) + + r.handleCreate(createInput{sess: waiter, autoStart: true}) + partner.cancel() // gone between the pairing and the seating + r.handleJoin(joinInput{sess: partner}) + r.vacate(r.seatOf("p2")) + + r.handleJoin(joinInput{sess: friend}) + if r.engine != nil { + t.Error("a game started for a joiner nobody had readied with") + } +} + +// TestResumeLeavesTheQuickMatchQueue: a connection that resumes into a seat +// while queued must not stay in the queue, or a later pairing pulls it out of +// the game it is playing. +func TestResumeLeavesTheQuickMatchQueue(t *testing.T) { + r := handlerRoom(t) + prior, next := offlineSession(t, 64), offlineSession(t, 64) + r.handleCreate(createInput{sess: prior}) + r.holdSeat(r.seatOf("p1")) + + r.hub.waiting = []*session{next} + r.handleResume(resumeInput{player: "p1", sess: next, prior: prior}) + + if len(r.hub.waiting) != 0 { + t.Error("a resumed connection is still queued for a quick match") + } +} + +// TestQuickMatchIsRefusedWhileDraining: a player queued now would wait for a +// stranger until the shutdown. +func TestQuickMatchIsRefusedWhileDraining(t *testing.T) { + api, url := newTestServer(t, chainDict(), Config{}) + c := dial(t, url) + c.hello("Người chơi") + + api.StartDraining() + c.quickMatch() + if got := c.await("error").GetError().GetCode(); got != "server_restarting" { + t.Errorf("quick match while draining got %q, want server_restarting", got) + } + if n := len(api.hub.waiting); n != 0 { + t.Errorf("%d sessions queued while draining", n) + } +} + +// TestEmptyMessageIsAnswered: a frame with no payload, or one from a newer +// client, is told it was not understood instead of being dropped. +func TestEmptyMessageIsAnswered(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{}) + c := dial(t, url) + c.hello("Người chơi") + + c.send(&noituv1.ClientMessage{}) + if got := c.await("error").GetError().GetCode(); got != "unknown_message" { + t.Errorf("an empty message got %q, want unknown_message", got) + } +} + +// TestRefusedActionsDoNotKeepALobbyAlive: the idle window restarts when the +// room changes, not when somebody pokes it. A refused action every 100ms must +// not hold a 300ms lobby open. +func TestRefusedActionsDoNotKeepALobbyAlive(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{IdleFor: 300 * time.Millisecond}) + host := dial(t, url) + host.hello("Chủ phòng") + host.createRoom() + host.await("room_state") + + // The owner has no readiness, so each of these is refused and changes + // nothing. + const pokes = 9 + for range pokes { + host.setReady(true) + time.Sleep(100 * time.Millisecond) + } + + refusedBeforeClose := 0 + for { + m := host.recv() + switch errorCode(m) { + case "owner_needs_no_ready": + refusedBeforeClose++ + continue + case "room_idle_closed": + default: + continue + } + break + } + if refusedBeforeClose == pokes { + t.Error("the lobby outlived the whole run of refused actions") + } +} + +// TestReadyToggleKeepsALobbyAlive: a readiness change is the room changing, +// so it restarts the idle window. +func TestReadyToggleKeepsALobbyAlive(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{IdleFor: 400 * time.Millisecond}) + host, guest, _ := pvpLobby(t, url) + + start := time.Now() + for i := range 4 { + guest.setReady(i%2 == 0) + host.await("room_state") + time.Sleep(150 * time.Millisecond) + } + if got := host.await("error").GetError().GetCode(); got != "room_idle_closed" { + t.Fatalf("got %q, want room_idle_closed", got) + } + if elapsed := time.Since(start); elapsed < 700*time.Millisecond { + t.Errorf("the lobby closed after %v, though readiness changed for ~600ms", elapsed) + } +} + +// TestSilentSocketIsClosed: a socket that never says Hello holds a slot of the +// global connection cap for nothing, so it is closed after the handshake +// deadline. A greeted connection is left alone. +func TestSilentSocketIsClosed(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{}, func(s *Server) { + s.hub.helloTimeout = 200 * time.Millisecond + }) + + greeted := dial(t, url) + greeted.hello("Người chơi") + + silent := dial(t, url) + opened := time.Now() + for { + if _, err := silent.read(); err != nil { + // read gives up after five seconds on its own, which would look + // the same as a close; the server's deadline is a fraction of that. + if time.Since(opened) > 3*time.Second { + t.Fatalf("the silent socket was never closed by the server: %v", err) + } + break + } + } + + // The silent socket outlived its deadline, and the greeted one started + // earlier, so its deadline has passed too. + greeted.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_Ping{Ping: &noituv1.Ping{ClientTimeMs: 7}}}) + if got := greeted.await("pong").GetPong().GetClientTimeMs(); got != 7 { + t.Errorf("pong echoed %d, want 7", got) + } +} + +// TestRoomBudgetIsPerAddress: reconnecting mints a fresh per-connection +// budget, so the address is what has to hold the line. +func TestRoomBudgetIsPerAddress(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{}, func(s *Server) { + s.hub.roomLimiter = newKeyedLimiter(0.001, roomBurst, time.Hour) + }) + + first := dial(t, url) + first.hello("Một") + for range roomBurst { + first.createRoom() + first.await("room_state") + } + + // A new connection has a full budget of its own, but the same address. + second := dial(t, url) + second.hello("Hai") + second.createRoom() + if got := second.await("error").GetError().GetCode(); got != "too_many_rooms" { + t.Errorf("a reconnect got %q, want too_many_rooms", got) + } +} + +// TestLimiterKeyFoldsAddressesToWhatOneClientOwns: an IPv4-mapped address is +// its IPv4 host, and an IPv6 client owns a whole /64. +func TestLimiterKeyFoldsAddressesToWhatOneClientOwns(t *testing.T) { + cases := []struct{ in, want string }{ + {"203.0.113.9", "203.0.113.9"}, + {"::ffff:203.0.113.9", "203.0.113.9"}, + {"2001:db8:1:2:aaaa:bbbb:cccc:dddd", "2001:db8:1:2::/64"}, + {"2001:db8:1:2:1::1", "2001:db8:1:2::/64"}, + {"2001:db8:1:3::1", "2001:db8:1:3::/64"}, + {"fe80::1%eth0", "fe80::/64"}, + {"not-an-address", "not-an-address"}, + {"", ""}, + } + for _, tc := range cases { + if got := limiterKey(tc.in); got != tc.want { + t.Errorf("limiterKey(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +// TestCorpusLogIsBoundedProcessWide: only a flood is sampled, and the gap is +// counted and reported on the next line through. +func TestCorpusLogIsBoundedProcessWide(t *testing.T) { + type line struct { + msg string + args []any + } + var lines []line + now := time.Now() + c := newCorpusLogger(now) + c.out = func(msg string, args ...any) { lines = append(lines, line{msg, args}) } + + before := metrics.corpusLogSuppressed.Value() + const total = corpusLogBurst + 50 + for range total { + c.info(now, "word_rejected", "word", "x") + } + if len(lines) != corpusLogBurst { + t.Fatalf("%d lines got through at one instant, want the burst of %d", len(lines), corpusLogBurst) + } + if got := metrics.corpusLogSuppressed.Value() - before; got != 50 { + t.Errorf("suppressed counter rose by %d, want 50", got) + } + + c.info(now.Add(time.Second), "word_rejected", "word", "x") + last := lines[len(lines)-1] + if len(last.args) < 2 || last.args[len(last.args)-2] != "suppressed_before" || last.args[len(last.args)-1] != int64(50) { + t.Errorf("the line after the gap did not report it: %v", last.args) + } +} + +// TestResponsesCarrySecurityHeaders covers the small text endpoints and an +// unknown path alike: the headers are set before routing. +func TestResponsesCarrySecurityHeaders(t *testing.T) { + api, _ := newTestServer(t, chainDict(), Config{}) + want := map[string]string{ + "X-Content-Type-Options": "nosniff", + "Content-Security-Policy": "frame-ancestors 'self'", + "Referrer-Policy": "strict-origin-when-cross-origin", + } + for _, path := range []string{"/healthz", "/version", "/no/such/page"} { + rec := httptest.NewRecorder() + api.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil)) + for name, value := range want { + if got := rec.Header().Get(name); got != value { + t.Errorf("GET %s: %s = %q, want %q", path, name, got, value) + } + } + } +} + +// TestResumeIntoTheLobbyReplaysAMissedGameOver: a player who was away when the +// game ended comes back to the lobby it returned to, and is owed the result, +// after the room state. +func TestResumeIntoTheLobbyReplaysAMissedGameOver(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{TurnLimit: time.Second, GraceFor: 10 * time.Second}) + + host := dial(t, url) + host.hello("Chủ phòng") + host.createRoom() + code := host.await("room_state").GetRoomState().GetRoomCode() + + guest := dial(t, url) + guestWelcome := guest.hello("Khách") + guest.joinRoom(code) + readyAndStart(t, host, guest) + hostStart := host.await("game_started").GetGameStarted() + guest.await("game_started") + + _ = guest.conn.Close(websocket.StatusAbnormalClosure, "") + host.await("room_state") // the guest is away + + // The game ends while the guest is gone: the host gives up on their turn, + // or the absent guest runs out the clock on theirs. + if hostStart.GetMyTurn() { + host.resign() + } + host.await("game_over") + + back := resumeAs(t, url, "Khách", guestWelcome.GetResumeToken()) + var order []string + var over *noituv1.GameOver + for over == nil { + m := back.recv() + order = append(order, payloadCase(m)) + over = m.GetGameOver() + if len(order) > 6 { + t.Fatalf("no game_over replayed after the resume: %v", order) + } + } + if order[len(order)-2] != "room_state" { + t.Errorf("frames = %v, want game_over right after room_state", order) + } + // The guest's own version: the host resigned exactly when it was theirs to + // act, which hands the guest the win, and otherwise the guest lost the clock. + if over.GetIWon() != hostStart.GetMyTurn() { + t.Errorf("i_won = %v, want %v", over.GetIWon(), hostStart.GetMyTurn()) + } + if len(over.GetStandings()) != 2 || !slices.ContainsFunc(over.GetStandings(), (*noituv1.PlayerScore).GetIsMe) { + t.Errorf("standings are not rendered for the resuming seat: %+v", over.GetStandings()) + } +} + +// TestResumeReplaysNothingForAGameSeenLive: a player who was connected when +// the game ended already has the result, so a later refresh in the lobby is +// answered with the lobby alone. +func TestResumeReplaysNothingForAGameSeenLive(t *testing.T) { + _, url := newTestServer(t, chainDict(), Config{}) + + host := dial(t, url) + welcome := host.hello("Chủ phòng") + host.createRoom() + code := host.await("room_state").GetRoomState().GetRoomCode() + guest := dial(t, url) + guest.hello("Khách") + guest.joinRoom(code) + readyAndStart(t, host, guest) + start := host.await("game_started").GetGameStarted() + guest.await("game_started") + resignAndSettle(t, host, guest, start) + + _ = host.conn.Close(websocket.StatusAbnormalClosure, "") + back := resumeAs(t, url, "Chủ phòng", welcome.GetResumeToken()) + back.await("room_state") + + back.send(&noituv1.ClientMessage{Payload: &noituv1.ClientMessage_Ping{Ping: &noituv1.Ping{ClientTimeMs: 1}}}) + if m := back.recv(); payloadCase(m) != "pong" { + t.Errorf("after the lobby state the next frame was %s, want nothing replayed", describe(m)) + } +} diff --git a/server/internal/wsapi/hub.go b/server/internal/wsapi/hub.go index e0adcbe..93cd3f1 100644 --- a/server/internal/wsapi/hub.go +++ b/server/internal/wsapi/hub.go @@ -69,6 +69,14 @@ type hub struct { waiting []*session joinLimiter *keyedLimiter + // roomLimiter is the room-creation budget per client address, alongside + // the per-connection one: reconnecting mints a fresh connection, and the + // address is what it cannot shed. + roomLimiter *keyedLimiter + + // helloTimeout overrides the package default for how long a socket may stay + // silent before its Hello. Zero means the default; only tests set it. + helloTimeout time.Duration // draining refuses every new room once set, so a creator is told to come // back rather than being seated in a room the shutdown below is about to @@ -98,6 +106,7 @@ func newHub(ctx context.Context, dict Dictionary, turnLimit, graceFor, idleFor t rooms: map[string]*room{}, sessions: map[string]*session{}, joinLimiter: newKeyedLimiter(joinsPerSecond, joinBurst, limiterIdleFor), + roomLimiter: newKeyedLimiter(addressRoomsPerSecond, addressRoomBurst, limiterIdleFor), } } @@ -170,6 +179,12 @@ func (h *hub) createRoom(s *session) error { // claiming "queued". The enqueue path sends its own status for the same // reason, symmetry, and because the caller has nobody else to hear from. func (h *hub) quickMatch(s *session) error { + // Refused before queueing, as a new room is: a lone player queued now would + // be told to wait for a stranger the shutdown is about to end. + if h.draining.Load() { + return errDraining + } + h.mu.Lock() for _, w := range h.waiting { if w == s { diff --git a/server/internal/wsapi/limits_test.go b/server/internal/wsapi/limits_test.go index 48c219a..85ff0d1 100644 --- a/server/internal/wsapi/limits_test.go +++ b/server/internal/wsapi/limits_test.go @@ -145,7 +145,9 @@ func TestFrameFloodClosesTheConnection(t *testing.T) { return // closed on us mid-flood, which is the point } } - for range 5 { + // Each frame within the burst is answered as unknown, so the close arrives + // after those replies rather than in place of them. + for range 2 * (frameBurst + 20) { _, _, err := c.conn.Read(c.ctx) if err != nil { return @@ -189,6 +191,10 @@ func TestClientIPTrustsOnlyConfiguredProxies(t *testing.T) { {"127.0.0.1:5000", "not an ip", "127.0.0.1"}, // An IPv4-mapped peer still matches its IPv4 prefix. {"[::ffff:127.0.0.1]:5000", "198.51.100.7", "198.51.100.7"}, + // A client's key is the address it owns, whichever way it arrived. + {"[::ffff:203.0.113.9]:4000", "", "203.0.113.9"}, + {"[2001:db8:1:2:aaaa::1]:4000", "", "2001:db8:1:2::/64"}, + {"127.0.0.1:5000", "2001:db8:1:2:bbbb::7", "2001:db8:1:2::/64"}, } for _, tc := range cases { if got := s.clientIP(req(tc.remote, tc.xff)); got != tc.want { @@ -215,6 +221,13 @@ func TestIdleRoomReleasesItsSeats(t *testing.T) { t.Fatalf("%d rooms still registered after the idle close", n) } + // Released, not merely tolerated: it must not still point at the dead room, + // which would answer this as busy instead of as no room at all. + host.say("x") + if got := host.await("error").GetError().GetCode(); got != "not_in_a_room" { + t.Errorf("chat after the idle close returned %q, want not_in_a_room", got) + } + // The connection is free again: a second room opens and seats it. host.createRoom() if host.await("room_state").GetRoomState().GetRoomCode() == "" { diff --git a/server/internal/wsapi/lobby_test.go b/server/internal/wsapi/lobby_test.go index 2080028..66e756f 100644 --- a/server/internal/wsapi/lobby_test.go +++ b/server/internal/wsapi/lobby_test.go @@ -472,20 +472,7 @@ func TestOneConnectionCannotStrandRooms(t *testing.T) { _ = c.conn.Close(websocket.StatusNormalClosure, "") - deadline := time.Now().Add(5 * time.Second) - for { - api.hub.mu.Lock() - left := len(api.hub.rooms) - api.hub.mu.Unlock() - - if left == 0 { - return - } - if time.Now().After(deadline) { - t.Fatalf("%d of %d rooms outlived the only connection that was ever in them", left, rooms) - } - time.Sleep(20 * time.Millisecond) - } + awaitNoRooms(t, api, "rooms that outlived the only connection that was ever in them") } // TestNoOtherRoomMidGame guards a player walking out of a running game by diff --git a/server/internal/wsapi/metrics.go b/server/internal/wsapi/metrics.go index f50bdc6..b514502 100644 --- a/server/internal/wsapi/metrics.go +++ b/server/internal/wsapi/metrics.go @@ -68,6 +68,11 @@ type metricSet struct { quickMatchQueued *expvar.Int quickMatchCancelled *expvar.Int quickMatchMatched *expvar.Int + + // corpusLogSuppressed counts word_rejected and word_reported lines the + // process-wide log budget dropped. The counters above stay exact; only the + // log is sampled. + corpusLogSuppressed *expvar.Int } // metrics is the one instance every call site writes through. Built at @@ -99,5 +104,7 @@ func newMetricSet() *metricSet { quickMatchQueued: expvar.NewInt("noitu_quick_match_queued"), quickMatchCancelled: expvar.NewInt("noitu_quick_match_cancelled"), quickMatchMatched: expvar.NewInt("noitu_quick_match_matched"), + + corpusLogSuppressed: expvar.NewInt("noitu_corpus_log_suppressed"), } } diff --git a/server/internal/wsapi/nickname.go b/server/internal/wsapi/nickname.go index aa039f9..ad3709f 100644 --- a/server/internal/wsapi/nickname.go +++ b/server/internal/wsapi/nickname.go @@ -59,12 +59,19 @@ func sanitizeText(raw string, maxRunes, maxMarks int) string { // Drop anything non-printing. Format characters (Cf) are the important // case: zero-width joiners and bidi overrides are invisible, so they can // pad text past a visual check or reverse how it renders. + // + // Every kind of space becomes a plain one first — tab, newline, no-break + // and the wide and thin spaces IMEs and paste introduce — and is collapsed + // below. Deleting them instead would glue "ngữ pháp" typed with a + // no-break space into one syllable. s = strings.Map(func(r rune) rune { switch { - case r == '\t' || r == '\n' || r == '\r': - return ' ' // collapsed below + case unicode.IsSpace(r): + return ' ' case unicode.IsControl(r), unicode.Is(unicode.Cf, r): return -1 + case blankLetters[r]: + return -1 case !unicode.IsPrint(r): return -1 } @@ -83,6 +90,20 @@ func sanitizeText(raw string, maxRunes, maxMarks int) string { return s } +// blankLetters are printable characters that draw nothing: Hangul fillers, +// the braille blank and Khmer inherent vowels. IsPrint accepts them, so +// without this list a name made of them looks empty, or is "Người chơi" plus +// invisible padding that makes it a different string. +var blankLetters = map[rune]bool{ + 0x115F: true, // HANGUL CHOSEONG FILLER + 0x1160: true, // HANGUL JUNGSEONG FILLER + 0x17B4: true, // KHMER VOWEL INHERENT AQ + 0x17B5: true, // KHMER VOWEL INHERENT AA + 0x2800: true, // BRAILLE PATTERN BLANK + 0x3164: true, // HANGUL FILLER + 0xFFA0: true, // HALFWIDTH HANGUL FILLER +} + // capMarks limits how many combining marks may follow one base rune. // // The filter above cannot catch these: a combining mark is printable, is not a diff --git a/server/internal/wsapi/nickname_test.go b/server/internal/wsapi/nickname_test.go index 8213258..7a7bead 100644 --- a/server/internal/wsapi/nickname_test.go +++ b/server/internal/wsapi/nickname_test.go @@ -80,6 +80,9 @@ func TestSanitizeNickname(t *testing.T) { {"whitespace collapsed", " Minh Nguyen ", "Minh Nguyen"}, {"newlines become spaces", "Minh\nNguyen", "Minh Nguyen"}, {"over length truncated", strings.Repeat("a", 40), strings.Repeat("a", maxNicknameRunes)}, + {"no-break space separates", "Minh\u00a0Nguyen", "Minh Nguyen"}, + {"blank-rendering letters alone fall back", "\u3164\u115f\u1160\u2800\uffa0", defaultNickname}, + {"blank-rendering letters stripped from a name", "Mi\u3164nh\u2800", "Minh"}, } for _, tc := range tests { @@ -131,3 +134,26 @@ func TestDistinguishSeparatesIdenticalNames(t *testing.T) { t.Errorf("distinguished name is %d runes, over the %d cap", len([]rune(got)), maxNicknameRunes) } } + +// TestSanitizeTextMapsEverySpaceToASpace: an IME or a paste can put a +// no-break, thin or ideographic space between syllables, and deleting it +// would hand the engine "ngữpháp" for "ngữ pháp". +func TestSanitizeTextMapsEverySpaceToASpace(t *testing.T) { + for _, sp := range []string{"\u00a0", "\u2009", "\u202f", "\u3000", "\u2003", "\u0085", "\u2028", "\v", "\f"} { + in := "ngữ" + sp + "pháp" + if got := sanitizeText(in, maxWordRunes, maxNicknameMarks); got != "ngữ pháp" { + t.Errorf("sanitizeText(%q) = %q, want two syllables", in, got) + } + } +} + +// TestBlankLettersAreDropped covers the printable characters that draw +// nothing, which IsPrint accepts. +func TestBlankLettersAreDropped(t *testing.T) { + for r := range blankLetters { + in := "a" + string(r) + "b" + if got := sanitizeText(in, maxChatRunes, maxChatMarks); got != "ab" { + t.Errorf("sanitizeText(%q) = %q, want the %U dropped", in, got, r) + } + } +} diff --git a/server/internal/wsapi/room.go b/server/internal/wsapi/room.go index 325d66d..4e56535 100644 --- a/server/internal/wsapi/room.go +++ b/server/internal/wsapi/room.go @@ -112,6 +112,16 @@ type seat struct { // out of it, and zero while they are connected. Per seat rather than per // room because any number of them can be waiting at once. graceUntil time.Time + // heldFor is the connection whose drop opened graceUntil. A resume is + // honoured only for that connection's token: seat ids are reused, and a + // seat that was vacated and refilled is a new *seat, so a token from the + // player who used to hold the id can never match it. + heldFor *session + // missedResult is the GameOver this seat was owed when the last game ended + // while nobody was connected to it, rendered for this seat. It is replayed + // once to the connection that resumes into the lobby, and dropped when the + // next game begins. + missedResult *noituv1.ServerMessage } // room owns one game. @@ -149,8 +159,9 @@ type room struct { // autoStart marks a room opened by a quick match. Once both seats are // filled and connected it begins its own first game — see handleJoin — - // and is cleared right there, so every later game in the room is agreed - // with readiness and StartGame like any other. + // and the first join the room handles consumes it, whether or not a game + // started, so every later game is agreed with readiness and StartGame like + // any other. autoStart bool seats [maxPlayers]*seat @@ -187,6 +198,12 @@ type room struct { // RoomState broadcast per input, which is why no handler has to remember // to send one. lobbyChanged bool + + // resumeReplays are seats whose resume has been accepted and that are owed + // their missed result. It waits here instead of being sent from the + // handler because the RoomState it must follow is broadcast by the run + // loop, after the handler returns. + resumeReplays []*seat } // Dictionary is everything the transport layer needs from the wordlist: the @@ -257,9 +274,24 @@ func (r *room) send(msg any) bool { } } +// sendReliably hands a message to the room, waiting for space in its inbox. +// +// For the notices whose loss is never recoverable — a connection has gone, so +// its seat must start its reconnect window. Only for callers that may block +// safely: a session goroutine that is ending, or one spawned for the purpose. +// It gives up when the room is finished, since nothing will read it then. +func (r *room) sendReliably(msg any) { + select { + case r.inputs <- msg: + case <-r.ctx.Done(): + } +} + // run is the room goroutine. It is the only place the engine is touched. func (r *room) run() { - defer r.cancel() + // Registered first, so it runs last: after cancel, when nothing new can be + // accepted, it answers whatever was still queued behind the final input. + defer r.refusePending() defer r.hub.evict(r.code) defer metrics.roomsLive.Add(r.mode, -1) // Catches a room cancelled with a game still running — drain forcing the @@ -271,6 +303,9 @@ func (r *room) run() { // A session that keeps a dead room would answer every later action with // "not in a room" and could never be seated anywhere else cleanly. defer r.detachAll() + // Registered last, so it runs first: from here on send refuses, and the + // queue refusePending drains can only shrink. + defer r.cancel() var turnTimer, graceTimer, idleTimer *time.Timer stop := func(t *time.Timer) { @@ -325,10 +360,6 @@ func (r *room) run() { } for { - // Reset by every input except chat: talking is not playing, and a room - // must not be holdable open forever by typing into it once a minute. - idleActivity := true - var turnC, graceC, idleC <-chan time.Time if turnTimer != nil { turnC = turnTimer.C @@ -360,14 +391,12 @@ func (r *room) run() { r.handleLobby(m) case chatInput: r.handleChat(m) - idleActivity = false case resignInput: r.handleResign(m) case claimDeadEndInput: r.handleClaimDeadEnd(m) case reportWordInput: r.handleReportWord(m) - idleActivity = false case disconnectInput: // A dropped connection is not a player leaving: the seat is // held for the reconnect window whether a game is running or @@ -410,6 +439,13 @@ func (r *room) run() { return } + // Whether the room changed is what keeps a lobby alive, so it is read + // before the broadcast below clears it. An input that was refused, or + // that only talked, changed nothing and leaves the idle window running; + // otherwise a refused action every few minutes would hold a lobby open + // for good. + changed := r.lobbyChanged + // One broadcast per input, from the one place that knows the input is // finished. A kick, a grace window running out and a game ending all // leave the room in the same state — a lobby — and this is where that @@ -417,6 +453,9 @@ func (r *room) run() { if r.strategy == nil && r.lobbyChanged { r.lobbyChanged = false r.broadcastRoomState() + // After the state, so a resumed client is shown the lobby before + // the result of the game that ended in its absence. + r.flushResumeReplays() } // A bot room is its game: there is no lobby to return to and nobody to @@ -430,12 +469,52 @@ func (r *room) run() { if !r.occupied() { return } - if idleActivity { + // The window also has to start or stop when the room crosses between + // lobby and game, which a game beginning does without marking the + // lobby changed. + if wantIdle := r.strategy == nil && r.inLobby(); changed || wantIdle != (idleTimer != nil) { resetIdleTimer() } } } +// refusePending answers every input still queued as the room exits. Each one +// came from a caller waiting on a reply — a join that lost the race with the +// last player leaving, a resume behind the last grace timer — and without an +// answer that caller waits forever. Called once the room is cancelled, so +// send already refuses new input and the drain is finite. +func (r *room) refusePending() { + for { + select { + case msg := <-r.inputs: + switch m := msg.(type) { + case createInput: + m.sess.send(errorMsg(codeRoomStartFailed)) + case startBotInput: + m.sess.send(errorMsg(codeRoomStartFailed)) + case joinInput: + m.sess.send(errorMsg(codeRoomNotFound)) + case resumeInput: + m.sess.send(errorMsg(codeSessionNotResumable)) + case submitInput: + m.sess.send(errorMsg(codeNotInAGame)) + case resignInput: + m.sess.send(errorMsg(codeNotInAGame)) + case claimDeadEndInput: + m.sess.send(errorMsg(codeNotInAGame)) + case lobbyInput: + m.sess.send(errorMsg(codeNotInARoom)) + case chatInput: + m.sess.send(errorMsg(codeNotInARoom)) + case reportWordInput: + m.sess.send(errorMsg(codeNotInARoom)) + } + default: + return + } + } +} + // occupies reports whether this connection is the one seated at p. // // The seat, not the claimed id, is the authority: a session that was never diff --git a/server/internal/wsapi/room_game.go b/server/internal/wsapi/room_game.go index 49dfff3..23b46f4 100644 --- a/server/internal/wsapi/room_game.go +++ b/server/internal/wsapi/room_game.go @@ -166,6 +166,7 @@ func (r *room) beginGame() error { for _, s := range r.seats { if s != nil { s.ready = false + s.missedResult = nil } } @@ -300,7 +301,7 @@ func (r *room) recordRejection(reason game.RejectReason, raw string) { word = string(runes[:maxWordRunes]) } - slog.Info("word_rejected", + corpusLog.info(time.Now(), "word_rejected", "reason", reason.String(), "word", word, "link", r.engine.Current(), @@ -323,7 +324,7 @@ func (r *room) handleReportWord(m reportWordInput) { link = r.engine.Current() } metrics.wordsReported.Add(1) - slog.Info("word_reported", "word", m.word, "link", link, "mode", r.mode, "room", r.code) + corpusLog.info(time.Now(), "word_reported", "word", m.word, "link", link, "mode", r.mode, "room", r.code) m.sess.send(wordReportedMsg(m.word)) } @@ -568,15 +569,26 @@ func (r *room) broadcastGameOver(state game.State) { order = append(order, standing.Player) } - for s := range r.connected() { - s.sess.send(&noituv1.ServerMessage{Payload: &noituv1.ServerMessage_GameOver{ + for _, s := range r.seats { + if s == nil || s.id == botPlayerID { + continue + } + msg := &noituv1.ServerMessage{Payload: &noituv1.ServerMessage_GameOver{ GameOver: &noituv1.GameOver{ IWon: state.Winner == s.id, Reason: reason, ChainLength: uint32(state.ChainLength), Standings: r.scoreRows(order, state, s.id, ranks), }, - }}) + }} + if s.sess == nil { + // Nobody is connected to hear it. Keep this seat's own version for + // the connection that resumes into the lobby this game returns to. + s.missedResult = msg + continue + } + s.missedResult = nil + s.sess.send(msg) } // A finished game is a return to the lobby, and the run loop reports the // state they are returning to. diff --git a/server/internal/wsapi/room_lobby.go b/server/internal/wsapi/room_lobby.go index bf3c6b9..42d6e44 100644 --- a/server/internal/wsapi/room_lobby.go +++ b/server/internal/wsapi/room_lobby.go @@ -39,6 +39,13 @@ func (r *room) handleCreate(m createInput) { // holding a seat, and every later Submit or Resign it sent would be applied to // the real player sitting there. func (r *room) handleJoin(m joinInput) { + // The first join a quick-matched room handles is the pairing itself, so it + // consumes the flag whatever comes of it. One left armed after a pairing + // that never started — the partner gone before seating — would start a + // game nobody readied for the next time a second seat filled. + autoStart := r.autoStart + r.autoStart = false + free := r.freeSeat() if free < 0 || !r.occupied() { metrics.joinsRefused.Add("room_full", 1) @@ -75,7 +82,7 @@ func (r *room) handleJoin(m joinInput) { // first, with both seats filled, so the wait ends on an ordinary room a // beat before GameStarted rather than jumping straight into one with no // seating frame behind it. - if r.autoStart && r.seatedCount() >= minPlayers && r.allConnected() { + if autoStart && r.seatedCount() >= minPlayers && r.allConnected() { if r.hub.isDraining() { // The second seat filled after the drain decision. There is no // owner here to answer with server_restarting the way lobbyStart @@ -84,7 +91,6 @@ func (r *room) handleJoin(m joinInput) { r.broadcastError(codeServerRestarting) return } - r.autoStart = false r.lobbyChanged = false r.broadcastRoomState() if err := r.beginGame(); err != nil { diff --git a/server/internal/wsapi/room_presence.go b/server/internal/wsapi/room_presence.go index 60f7294..d59a5d0 100644 --- a/server/internal/wsapi/room_presence.go +++ b/server/internal/wsapi/room_presence.go @@ -24,6 +24,7 @@ import ( // allConnected() would report true and quick match's own auto-start could // begin a game against a socket nobody is behind. func (r *room) holdSeat(s *seat) { + s.heldFor = s.sess s.sess = nil s.graceUntil = time.Now().Add(r.graceFor) // Presence is part of the room's state, and the run loop is what sends it. @@ -116,7 +117,12 @@ func (r *room) handleResume(m resumeInput) { // both find it registered before either resume lands. Only the first may // take the seat: the second would displace it and leave that connection // attached to a seat that is no longer its own. - if s == nil || (s.sess != nil && s.sess != m.prior) { + // + // A seat in its grace window is resumable only by the connection that + // dropped out of it. Seat ids are reused, so a seat vacated while its + // player was away and refilled since carries the same id but was held for + // somebody else; the old token must not open it. + if s == nil || (s.sess != m.prior && (s.sess != nil || s.heldFor != m.prior)) { m.sess.send(errorMsg(codeSessionNotResumable)) return } @@ -126,11 +132,15 @@ func (r *room) handleResume(m resumeInput) { // let a third connection claim the same seat. metrics.resumesSucceeded.Add(1) m.sess.attach(r, m.player) + // Every other way into a seat leaves the quick-match queue (takeSeat); a + // resume must too, or a seated player could be paired out of a game. + r.hub.cancelQuickMatch(m.sess) if m.prior != nil { r.hub.unregister(m.prior.resumeToken) m.prior.close() } s.sess = m.sess + s.heldFor = nil s.graceUntil = time.Time{} // The seat keeps the name it was given. Re-reading it from the new // connection would let a reconnect rename a player mid-game, including @@ -159,6 +169,12 @@ func (r *room) handleResume(m resumeInput) { // Resumed between games, or before the first one. The lobby state above is // the whole answer; there is no position to replay. if r.inLobby() { + // A game that ended while this seat was away is owed its result: the + // lobby alone would leave the player unaware of how it went. Queued + // rather than sent, to follow the RoomState the run loop broadcasts. + if s.missedResult != nil { + r.resumeReplays = append(r.resumeReplays, s) + } return } state := r.engine.Snapshot() @@ -168,6 +184,19 @@ func (r *room) handleResume(m resumeInput) { } } +// flushResumeReplays sends each resumed seat the result it missed, and +// forgets it, so a result is replayed once. A seat whose connection has gone +// again since keeps the result for the next resume. +func (r *room) flushResumeReplays() { + for _, s := range r.resumeReplays { + if s.sess != nil && s.missedResult != nil { + s.sess.send(s.missedResult) + s.missedResult = nil + } + } + r.resumeReplays = r.resumeReplays[:0] +} + // detachAll releases every connection still bound to this room as it exits. func (r *room) detachAll() { for s := range r.connected() { diff --git a/server/internal/wsapi/server.go b/server/internal/wsapi/server.go index 7a2f834..4d1dc71 100644 --- a/server/internal/wsapi/server.go +++ b/server/internal/wsapi/server.go @@ -135,7 +135,17 @@ func NewServer(ctx context.Context, dict Dictionary, cfg Config) *Server { return s } -func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.mux.ServeHTTP(w, r) } +// ServeHTTP sets the response headers every reply carries, then routes. They +// go on all of them, the app shell included, because the shell is the page a +// hostile site would try to frame. HSTS is left to the proxy that terminates +// TLS. +func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { + h := w.Header() + h.Set("X-Content-Type-Options", "nosniff") + h.Set("Content-Security-Policy", "frame-ancestors 'self'") + h.Set("Referrer-Policy", "strict-origin-when-cross-origin") + s.mux.ServeHTTP(w, r) +} // Shutdown tells live games why they are ending, then stops the hub. func (s *Server) Shutdown() { @@ -256,7 +266,30 @@ func underRoot(root, path string) bool { return rel == "." || (!strings.HasPrefix(rel, "..") && !filepath.IsAbs(rel)) } -// clientIP is the key the join limiter counts against. +// clientIP is the key every per-address limit counts against: the client's +// address reduced by limiterKey. +func (s *Server) clientIP(r *http.Request) string { + return limiterKey(s.clientAddr(r)) +} + +// limiterKey folds an address into what one client owns. An IPv4-mapped IPv6 +// address is the same host as its IPv4 form, and a single IPv6 customer +// controls a whole /64, so both would otherwise split one client across +// buckets: 2^64 of them for the second, which makes any per-address limit +// free to walk around. Anything that does not parse is returned as it came. +func limiterKey(host string) string { + addr, err := netip.ParseAddr(host) + if err != nil { + return host + } + addr = addr.WithZone("").Unmap() + if addr.Is6() { + return netip.PrefixFrom(addr, 64).Masked().String() + } + return addr.String() +} + +// clientAddr is the address the request came from, before limiterKey. // // RemoteAddr is the default and the only source when no proxy is trusted: // X-Forwarded-For is attacker-controlled unless the proxy is known to append @@ -265,7 +298,7 @@ func underRoot(root, path string) bool { // the header is walked from the right and the first address that is not // itself a trusted proxy is the client — the entries a client could have // forged all sit to the left of the one the proxy appended. -func (s *Server) clientIP(r *http.Request) string { +func (s *Server) clientAddr(r *http.Request) string { peer := remoteHost(r.RemoteAddr) if len(s.proxies) == 0 || !s.trusted(peer) { return peer @@ -376,6 +409,7 @@ func (s *Server) sweepLimiters(ctx context.Context) { return case now := <-ticker.C: s.hub.joinLimiter.sweep(now) + s.hub.roomLimiter.sweep(now) } } } diff --git a/server/internal/wsapi/session.go b/server/internal/wsapi/session.go index 8d14ad1..5a348cf 100644 --- a/server/internal/wsapi/session.go +++ b/server/internal/wsapi/session.go @@ -73,6 +73,28 @@ const ( roomBurst = 5 limiterIdleFor = 5 * time.Minute + // addressRoomsPerSecond and addressRoomBurst are the same budget kept per + // client address, because the per-connection one above resets with every + // reconnect and so bounds nothing about who is opening rooms. + // + // Sized like the join budget, for the same reason: without a trusted proxy + // that unmasks the real client — production today runs without + // NOITU_TRUSTED_PROXIES — every player shares one address, and a NAT or + // CGNAT egress is many real people at once. Thirty rooms up front and one + // every two seconds after is far above what any crowd creates by hand, yet + // it keeps one address from holding more than about a third of the default + // room ceiling for a whole idle window (0.5/s over ten minutes is 300). + addressRoomsPerSecond = 0.5 + addressRoomBurst = 30 + + // helloTimeout is how long a socket may stay open without saying Hello. + // A connection that has not greeted is not in the hub, is in no room and + // has no resume token, yet it counts against the global connection cap; + // an honest client sends Hello the instant the socket opens, so a silent + // one is only ever holding a slot. The keepalive cannot catch it, since + // every WebSocket stack answers pings on its own. + helloTimeout = 10 * time.Second + // framesPerSecond bounds every frame a connection sends, before it is // routed. The per-action limiters above only meter the actions they know // about; a Ping, or a ClientMessage with no payload set, matched none of @@ -150,6 +172,12 @@ type session struct { // Hello would re-register the session and rewrite its nickname mid-game. // Touched only from dispatch, like reportedWords, so it needs no lock. greeted bool + + // helloTimer closes the connection if the handshake never arrives. Armed + // by run before the first read and stopped by handleHello, both on the + // reader goroutine, so it needs no lock either. Nil for a session that + // was never run. + helloTimer *time.Timer } func newSession(ctx context.Context, conn *websocket.Conn, h *hub, remoteIP string) *session { @@ -200,8 +228,13 @@ func (s *session) attach(r *room, id game.PlayerID) { // unreleased room parks in select forever, holding a goroutine and a room // code for the life of the process. One connection asking for several rooms // is all it takes. + // + // Delivered reliably, and from its own goroutine: this runs on the goroutine + // of the room being entered, and a room waiting on another room's full + // inbox from there could wait on each other. A dropped notice would leave + // the old seat bound to this connection for good. if previous != nil && previous != r { - previous.send(disconnectInput{player: previousID, sess: s}) + go previous.sendReliably(disconnectInput{player: previousID, sess: s}) } } @@ -297,6 +330,16 @@ func (s *session) run() { s.conn.SetReadLimit(maxFrameBytes) + timeout := helloTimeout + if s.hub != nil && s.hub.helloTimeout > 0 { + timeout = s.hub.helloTimeout + } + s.helloTimer = time.AfterFunc(timeout, func() { + s.send(errorMsg(codeHandshakeRequired)) + s.close() + }) + defer s.helloTimer.Stop() + var wg sync.WaitGroup wg.Add(3) go func() { defer wg.Done(); s.writeLoop() }() diff --git a/server/internal/wsapi/wsapi_test.go b/server/internal/wsapi/wsapi_test.go index 58fe6e6..5f8470f 100644 --- a/server/internal/wsapi/wsapi_test.go +++ b/server/internal/wsapi/wsapi_test.go @@ -150,7 +150,9 @@ type testClient struct { ctx context.Context } -func newTestServer(t *testing.T, dict Dictionary, cfg Config) (*Server, string) { +// newTestServer starts a server on a real socket. opts adjust the *Server +// before it accepts a connection, for the few knobs Config does not expose. +func newTestServer(t *testing.T, dict Dictionary, cfg Config, opts ...func(*Server)) (*Server, string) { t.Helper() ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -168,6 +170,9 @@ func newTestServer(t *testing.T, dict Dictionary, cfg Config) (*Server, string) } api := NewServer(ctx, dict, cfg) + for _, opt := range opts { + opt(api) + } hs := httptest.NewServer(api) t.Cleanup(func() { api.Shutdown()