From b077097a33b17c64a24294f1531bcdeb807f6543 Mon Sep 17 00:00:00 2001 From: olcxja Date: Wed, 26 Aug 2026 00:11:06 +0200 Subject: [PATCH] security --- LarpixServer/Account/Requests.cs | 40 +++++++++---- LarpixServer/Account/Utils.cs | 45 ++++++++++++++- LarpixServer/Program.cs | 7 +++ LarpixServer/Room/Groups.cs | 71 +++++++++++++++++------ LarpixServer/Room/Requests.cs | 83 ++++++++++++++++---------- LarpixServer/Room/Shared.cs | 99 +++++++++++++++++++++++++++++--- LarpixServer/Utils/Utils.cs | 26 ++++++++- LarpixVoice/Requests.cs | 9 +++ 8 files changed, 309 insertions(+), 71 deletions(-) diff --git a/LarpixServer/Account/Requests.cs b/LarpixServer/Account/Requests.cs index dd08533..1ee4f03 100644 --- a/LarpixServer/Account/Requests.cs +++ b/LarpixServer/Account/Requests.cs @@ -307,7 +307,8 @@ public class Requests } createHolder.TryRemove(serialized.idKey, out _); - await Utils.UpdateLastLogin(id.ToString()); + //już trzymamy userLock(id) - rdzeń bez ponownego Wejścia + await Utils.UpdateLastLoginLocked(id.ToString()); } finally { @@ -465,19 +466,30 @@ public class Requests { nonceHolder.TryRemove(kvp.Key, out _); } - else if (kvp.Key == id) + } + + //Get-or-add w pętli: wcześniej enumerate+warunkowy return+TryAdd był + //check-then-act - dwa równoległe żądania generowały różne nonce, drugi + //TryAdd przegrywał po cichu, a klient dostawał "obcy" nonce i kolejny + //auth kończył się error:invalid.nonce (audyt S3) + while (true) + { + if (nonceHolder.TryGetValue(id, out var existing)) { - await context.Response.WriteAsync(Encryption.Encryption.EncryptForClient(kvp.Value.Item1, + await context.Response.WriteAsync(Encryption.Encryption.EncryptForClient(existing.Item1, plainPass)); return; } - } - string nonce = Encryption.Encryption.GetRandomString(64); - nonceHolder.TryAdd(id, (nonce, DateTimeOffset.UtcNow)); - await context.Response.WriteAsync(Encryption.Encryption.EncryptForClient(nonce, - plainPass)); - return; + string nonce = Encryption.Encryption.GetRandomString(64); + if (nonceHolder.TryAdd(id, (nonce, DateTimeOffset.UtcNow))) + { + await context.Response.WriteAsync(Encryption.Encryption.EncryptForClient(nonce, + plainPass)); + return; + } + //ktoś wstawił wpis w międzyczasie - pętla odczyta jego wartość + } } public static async Task ChangePassword(HttpContext context, Func next, IQueryCollection query, @@ -1140,7 +1152,8 @@ public class Requests serializedBody.string2, AppJsonSerializerContext.Default.Universal2String ); - await Account.Utils.UpdateUserDm(id, readGet.string1, "true", ""); + //dispatch trzyma już userLock(id) - rdzeń bez ponownego Wejścia + await Account.Utils.UpdateUserDmLocked(id, readGet.string1, "true", ""); await context.Response.WriteAsync(Encryption.Encryption.EncryptForClient("success:dm.read", password)); break; } @@ -1150,7 +1163,8 @@ public class Requests serializedBody.string2, AppJsonSerializerContext.Default.Universal2String ); - await Room.Groups.UpdateUserGroup(id, readGet.string1, "true", ""); + //dispatch trzyma już userLock(id) - rdzeń bez ponownego Wejścia + await Room.Groups.UpdateUserGroupLocked(id, readGet.string1, "true", ""); await context.Response.WriteAsync(Encryption.Encryption.EncryptForClient("success:group.read", password)); break; } @@ -1259,7 +1273,9 @@ public class Requests string id = Utils.GetValidIdOrZero(idQuery.ToString()); string password = await Utils.GetPassword(id); - string secretStr = await Utils.NonceDecryptBody(id, password, secret, false); + //S9: jednorazowa akcja zapisu - konsumuj nonce (delEntry:false pozwalał + //na nieograniczony replay przechwyconej pary secret+body) + string secretStr = await Utils.NonceDecryptBody(id, password, secret, true); string auth = await Utils.Auth(id, secretStr, password); if (auth != Utils.LOGIN_SUCCESS) diff --git a/LarpixServer/Account/Utils.cs b/LarpixServer/Account/Utils.cs index 169e578..8acd79f 100644 --- a/LarpixServer/Account/Utils.cs +++ b/LarpixServer/Account/Utils.cs @@ -210,11 +210,31 @@ public class Utils } public static async Task UpdateLastLogin(string id) + { + //Wrapper blokujący GetUserLock(id); rdzeń w UpdateLastLoginLocked. + //Uwaga: register woła rdzeń bezpośrednio - już trzyma ten lock. + SemaphoreSlim userLock = GetUserLock(id); + await userLock.WaitAsync(); + try + { + await UpdateLastLoginLocked(id); + } + finally + { + userLock.Release(); + } + } + + //Rdzeń UpdateLastLogin - WYMAGA trzymanego GetUserLock(id) przez wołającego. + public static async Task UpdateLastLoginLocked(string id) { if (!Fs.Exists($"{ACCOUNTS_DATA_DIR}/{id}/secret")) { return; } + //Pod GetUserLock: BackgroundDelete decyduje o usunięciu konta na podstawie + //lastlogin (createLock -> userLock) - niezablokowany zapis mógł wyścigowo + //odtworzyć plik w usuwanym drzewie albo przeoczyć świeży login await Fs.WriteFile($"{ACCOUNTS_DATA_DIR}/{id}/lastlogin", Encoding.UTF8.GetBytes(DateTimeOffset.UtcNow.ToUnixTimeSeconds().ToString())); } @@ -482,7 +502,30 @@ public class Utils public static async Task UpdateUserDm(string id, string dmId, string isRead = "false", string timestamp = "") { - + //Wrapper blokujący GetUserLock(id). RDZENIEK jest UpdateUserDmLocked - + //ścieżki wołane spod już trzymanego userLock(id) MUSZĄ wołać wariant + //Locked bezpośrednio (SemaphoreSlim nie jest reentrantny - podwójne + //WaitAsync na tym samym pasku = samoczynny deadlock) + SemaphoreSlim userLock = GetUserLock(id); + await userLock.WaitAsync(); + try + { + await UpdateUserDmLocked(id, dmId, isRead, timestamp); + } + finally + { + userLock.Release(); + } + } + + //Rdzeń UpdateUserDm - WYMAGA trzymanego GetUserLock(id) przez wołającego. + public static async Task UpdateUserDmLocked(string id, string dmId, string isRead = "false", string timestamp = "") + { + //dmId trafia prosto do ścieżki pliku. Bez walidacji "dm/read" pozwalał + //zapisać plik członkostwa w dowolnym pokoju przez "../../.." (path + //traversal -> samodzielne nadanie sobie członkostwa / zapis gdziekolwiek) + if (!IsSafePathComponent(dmId)) return; + if (await Account.Utils.NameFromId(id) == "") //if user account just got deleted { return; diff --git a/LarpixServer/Program.cs b/LarpixServer/Program.cs index dd0258a..3601ce7 100644 --- a/LarpixServer/Program.cs +++ b/LarpixServer/Program.cs @@ -15,6 +15,13 @@ public class Program { await LoadEnv(); + //S18: część operacji Fs (Exists/ReadDirectory/Delete) jest synchroniczna + //i blokuje wątek poola. Przy tysiącach równoległych żądań pool dokłada + //wątki tylko ~1/s co kończy się załamaniem latencji zanim zdąży nadążyć. + //Podniesienie minimum usuwa ten ramp-up przy zerowym ryzyku zmian w kodzie. + int workers = Environment.ProcessorCount; + ThreadPool.SetMinThreads(workers * 16, workers * 16); + // Voice ticket sweeper: issued tickets live for 60s; remove expired // ones a minute later so nothing lingers in memory. Also prunes stale // per-IP auth rate-limit counters (they are written pre-HMAC-check, diff --git a/LarpixServer/Room/Groups.cs b/LarpixServer/Room/Groups.cs index b11953e..ab9396c 100644 --- a/LarpixServer/Room/Groups.cs +++ b/LarpixServer/Room/Groups.cs @@ -23,23 +23,46 @@ public class Groups } public static async Task UpdateUserGroup(string id, string groupId, string isRead = "false", string timestamp = "") + { + //Wrapper blokujący GetUserLock(id). Rdzeń: UpdateUserGroupLocked - + //ścieżki spod trzymanego userLock(id) wołają wariant Locked bezpośrednio + //(SemaphoreSlim nie jest reentrantny) + SemaphoreSlim userLock = Account.Utils.GetUserLock(id); + await userLock.WaitAsync(); + try + { + await UpdateUserGroupLocked(id, groupId, isRead, timestamp); + } + finally + { + userLock.Release(); + } + } + + //Rdzeń UpdateUserGroup - WYMAGA trzymanego GetUserLock(id) przez wołającego. + public static async Task UpdateUserGroupLocked(string id, string groupId, string isRead = "false", string timestamp = "") { if (await Account.Utils.NameFromId(id) == "") //if user account just got deleted { return; } - + string groupDir = $"{ACCOUNTS_DATA_DIR}/{id}/groups"; if (!Fs.Exists(groupDir)) { Fs.CreateDirectory(groupDir); } - + if (!groupId.Contains(';')) { groupId = $"{groupId};{Utils.Utils.DOMAIN}"; } + //groupId trafia prosto do ścieżki pliku - bez walidacji "group/read" + //pozwalał zapisać plik członkostwa w dowolnej grupie przez "../../.." + //(path traversal -> samodzielne nadanie sobie członkostwa / zapis gdziekolwiek) + if (!IsSafeGroupId(groupId)) return; + string groupPath = $"{groupDir}/{groupId}"; Universal3String fileGroup = new Universal3String(); if (Fs.Exists(groupPath)) @@ -141,7 +164,7 @@ public class Groups JsonSerializer.Serialize(startingMessage, AppJsonSerializerContext.Default.Message) )); - await UpdateUserGroup(id, groupId.ToString(), "true", startingMessage.timestamp); + await UpdateUserGroupLocked(id, groupId.ToString(), "true", startingMessage.timestamp); return $"success:{groupId}"; } @@ -416,6 +439,11 @@ public class Groups await userLock2.WaitAsync(); try { + //Jeśli zaproszenie zostało właśnie zaakceptowane (członkostwo + //istnieje), NIE odtwarzaj pliku invite - asynchroniczny zapis + //PO akceptacji tworzyłby zombie-invite na liście na zawsze + if (Fs.Exists($"{groupPath}/members/{id2};{DOMAIN}")) return; + Fs.CreateDirectory(ACCOUNTS_DATA_DIR + $"/{id2}/groupinvites/recv"); await Fs.WriteFile(inviteFile, inviteBytes); } @@ -493,12 +521,14 @@ public class Groups string inviteFile = ACCOUNTS_DATA_DIR + $"/{id}/groupinvites/recv/{groupId};{iId}"; string groupPath = GetGroupPath(groupId); - if (!Fs.Exists(groupPath)) return "error:group.not.found"; SemaphoreSlim roomLock = Room.Requests.GetRoomLock(groupId); await roomLock.WaitAsync(); try { + //Istnienie grupy sprawdzane POD lockiem - GroupDelete mógł usunąć + //grupę między checkiem a joinem (powstawał katalog bez ownera/nazwy) + if (!Fs.Exists(groupPath)) return "error:group.not.found"; if (!Fs.Exists(inviteFile)) return "error:no.invite"; string inviteContent = Encoding.UTF8.GetString(await Fs.ReadFile(inviteFile)); @@ -591,7 +621,8 @@ public class Groups )); await Fs.WriteFile($"{groupPath}/messages/last", Encoding.UTF8.GetBytes(msgId.ToString())); - await UpdateUserGroup(id, groupId, "true", timestamp); + //dispatch trzyma już userLock(id) - rdzeń bez ponownego Wejścia + await UpdateUserGroupLocked(id, groupId, "true", timestamp); // Notify all members (including the new one) via WebSocket string[] currentMembers = Fs.ReadDirectory($"{groupPath}/members"); @@ -978,13 +1009,8 @@ public class Groups { if (!IsSafeGroupId(groupId)) return "error:group.not.found"; string groupPath = GetGroupPath(groupId); - if (!Fs.Exists(groupPath)) return "error:group.not.found"; - - string ownerPath = $"{groupPath}/owner"; - if (!Fs.Exists(ownerPath)) return "error:forbidden"; - string currentOwner = Encoding.UTF8.GetString(await Fs.ReadFile(ownerPath)); - if (currentOwner != $"{id};{DOMAIN}") return "error:forbidden"; + string ownerPath = $"{groupPath}/owner"; string tId = Account.Utils.GetValidIdOrZero(targetId); if (tId == "0") return "error:invalid.user"; @@ -996,6 +1022,12 @@ public class Groups await roomLock.WaitAsync(); try { + //Check ownera POD lockiem - poza nim ex-owner wykonywał kick + //wyścigowo z równoległym transferem własności + if (!Fs.Exists(ownerPath)) return "error:forbidden"; + string currentOwner = Encoding.UTF8.GetString(await Fs.ReadFile(ownerPath)); + if (currentOwner != $"{id};{DOMAIN}") return "error:forbidden"; + if (!Fs.Exists(memberPath)) return "error:not.a.member"; if (tId == id) return "error:cannot.kick.self"; @@ -1068,13 +1100,8 @@ public class Groups { if (!IsSafeGroupId(groupId)) return "error:group.not.found"; string groupPath = GetGroupPath(groupId); - if (!Fs.Exists(groupPath)) return "error:group.not.found"; - - string ownerPath = $"{groupPath}/owner"; - if (!Fs.Exists(ownerPath)) return "error:forbidden"; - string currentOwner = Encoding.UTF8.GetString(await Fs.ReadFile(ownerPath)); - if (currentOwner != $"{id};{DOMAIN}") return "error:forbidden"; + string ownerPath = $"{groupPath}/owner"; string tId = Account.Utils.GetValidIdOrZero(targetId); if (tId == "0") return "error:invalid.user"; @@ -1086,6 +1113,11 @@ public class Groups await roomLock.WaitAsync(); try { + //Check ownera POD lockiem (j.w.) - okno TOCTOU z kickiem/transferem + if (!Fs.Exists(ownerPath)) return "error:forbidden"; + string currentOwner = Encoding.UTF8.GetString(await Fs.ReadFile(ownerPath)); + if (currentOwner != $"{id};{DOMAIN}") return "error:forbidden"; + if (!Fs.Exists(memberPath)) return "error:not.a.member"; await Fs.WriteFile(ownerPath, Encoding.UTF8.GetBytes($"{tId};{tDomain}")); @@ -1167,7 +1199,8 @@ public class Groups { if (memberId == userId) { - await UpdateUserGroup(memberId, groupId, "true", timestamp); + //dispatch trzyma już userLock(userId) - rdzeń bez ponownego Wejścia + await UpdateUserGroupLocked(memberId, groupId, "true", timestamp); await Utils.Utils.SendWsMessage(memberId, $"group_message:{groupId}:read"); } else @@ -1178,7 +1211,7 @@ public class Groups await userLock.WaitAsync(); try { - await UpdateUserGroup(memberId, groupId, "false", timestamp); + await UpdateUserGroupLocked(memberId, groupId, "false", timestamp); } finally { diff --git a/LarpixServer/Room/Requests.cs b/LarpixServer/Room/Requests.cs index 274f546..e610b9f 100644 --- a/LarpixServer/Room/Requests.cs +++ b/LarpixServer/Room/Requests.cs @@ -80,6 +80,11 @@ public class Requests { start = s; } + //Clamp do [0, last] - startOffset spoza zakresu (np. long.MaxValue przy + //mniejszej historii) dekrementowałby od góry pętlę robiącą Fs.Exists na + //każdą iterację = trwały DoS wątków poola jednym żądaniem + if (start > last) start = last; + if (start < 0) start = 0; int count = 50; if (!string.IsNullOrEmpty(countStr) && int.TryParse(countStr, out int c)) @@ -217,6 +222,12 @@ public class Requests await userLock2.WaitAsync(); try { + //Jeśli zaproszenie zostało właśnie zaakceptowane (pokój istnieje), + //NIE odtwarzaj pliku invite - pisany asynchronicznie PO akceptacji + //tworzyłby zombie-invite wiszący na liście na zawsze + string dmIdCheck = Account.Utils.GetDmId(id, DOMAIN, id2, domain); + if (Fs.Exists($"{ROOMS_DIR}/dms/{DOMAIN}/{dmIdCheck}/members")) return; + await Fs.WriteFile(inviteFile, timestamp); } finally @@ -402,7 +413,7 @@ public class Requests JsonSerializer.Serialize(startingMessage, AppJsonSerializerContext.Default.Message) )); - await Account.Utils.UpdateUserDm(id, dmId, "false", + await Account.Utils.UpdateUserDmLocked(id, dmId, "false", startingMessage.timestamp); if (isUserLocal) @@ -413,7 +424,7 @@ public class Requests await userLock.WaitAsync(); try { - await Account.Utils.UpdateUserDm(id2, dmId, "false", + await Account.Utils.UpdateUserDmLocked(id2, dmId, "false", startingMessage.timestamp); } finally @@ -547,7 +558,8 @@ public class Requests { if (memberId == id) { - await Account.Utils.UpdateUserDm(memberId, dmId, "true", msg.timestamp); + //dispatch trzyma już userLock(id) - rdzeń bez ponownego Wejścia + await Account.Utils.UpdateUserDmLocked(memberId, dmId, "true", msg.timestamp); await Utils.Utils.SendWsMessage(memberId, $"dm_message:{dmId}:read"); } else @@ -558,7 +570,7 @@ public class Requests await userLock.WaitAsync(); try { - await Account.Utils.UpdateUserDm(memberId, dmId, "false", msg.timestamp); + await Account.Utils.UpdateUserDmLocked(memberId, dmId, "false", msg.timestamp); } finally { @@ -584,35 +596,46 @@ public class Requests if (!IsSafeMsgId(msgId)) return "error:invalid.msg.id"; if (!await IsMemberOfDm(id, dmId)) return "error:forbidden"; - string msgPath = $"{ROOMS_DIR}/dms/{DOMAIN}/{dmId}/messages/{msgId}"; - if (!Fs.Exists(msgPath)) return "error:message.not.found"; - - Message msg = JsonSerializer.Deserialize( - Encoding.UTF8.GetString(await Fs.ReadFile(msgPath)), - AppJsonSerializerContext.Default.Message - ); - - if (msg.author != id) return "error:forbidden"; - if (msg.type == "larp.redacted") return "error:message.already.deleted"; - - msg.type = "larp.redacted"; - msg.reactions = ""; - msg.content = "{blah(larp.redacted)}"; - msg.attachment = ""; - msg.key = ""; - - await Fs.WriteFile(msgPath, Encoding.UTF8.GetBytes(JsonSerializer.Serialize(msg, AppJsonSerializerContext.Default.Message))); - - string reactDir = $"{ROOMS_DIR}/dms/{DOMAIN}/{dmId}/reactions/{msgId}"; - if (Fs.Exists(reactDir)) + //W3: jedyna mutacja DM bez room-locka - wyścig z DmMessageReactAdd + //(licznik reakcji vs DeleteDirectory) i z podwójnym delete'em + SemaphoreSlim roomLock = GetRoomLock(dmId); + await roomLock.WaitAsync(); + try { - Fs.DeleteDirectory(reactDir); + string msgPath = $"{ROOMS_DIR}/dms/{DOMAIN}/{dmId}/messages/{msgId}"; + if (!Fs.Exists(msgPath)) return "error:message.not.found"; + + Message msg = JsonSerializer.Deserialize( + Encoding.UTF8.GetString(await Fs.ReadFile(msgPath)), + AppJsonSerializerContext.Default.Message + ); + + if (msg.author != id) return "error:forbidden"; + if (msg.type == "larp.redacted") return "error:message.already.deleted"; + + msg.type = "larp.redacted"; + msg.reactions = ""; + msg.content = "{blah(larp.redacted)}"; + msg.attachment = ""; + msg.key = ""; + + await Fs.WriteFile(msgPath, Encoding.UTF8.GetBytes(JsonSerializer.Serialize(msg, AppJsonSerializerContext.Default.Message))); + + string reactDir = $"{ROOMS_DIR}/dms/{DOMAIN}/{dmId}/reactions/{msgId}"; + if (Fs.Exists(reactDir)) + { + Fs.DeleteDirectory(reactDir); + } + + string attDir = $"{ROOMS_DIR}/dms/{DOMAIN}/{dmId}/attachments/{msgId}"; + if (Fs.Exists(attDir)) + { + Fs.DeleteDirectory(attDir); + } } - - string attDir = $"{ROOMS_DIR}/dms/{DOMAIN}/{dmId}/attachments/{msgId}"; - if (Fs.Exists(attDir)) + finally { - Fs.DeleteDirectory(attDir); + roomLock.Release(); } string[] members = Fs.ReadDirectory($"{ROOMS_DIR}/dms/{DOMAIN}/{dmId}/members"); diff --git a/LarpixServer/Room/Shared.cs b/LarpixServer/Room/Shared.cs index d2db31f..6fb3cda 100644 --- a/LarpixServer/Room/Shared.cs +++ b/LarpixServer/Room/Shared.cs @@ -69,15 +69,29 @@ public class Shared public static async Task IsMemberOfRoom(string id, string roomType, string roomId) { + //Whitelist roomType - trafia do GetRoomPath bezpośrednio, a pochodzi + //od użytkownika (audyt S7: "../.."-style wartości przekierowywały + //proby ścieżek poza katalog pokoi) + if (!IsSupportedRoomType(roomType)) return false; if (!IsSafeId(roomId)) return false; string memberPath = $"{GetRoomPath(roomType, roomId)}/members/{id};{DOMAIN}"; return Fs.Exists(memberPath); } + public static bool IsSupportedRoomType(string roomType) + { + return roomType == "dms" || roomType == "groups"; + } + public static async Task GetUserRole(string id, string roomType, string roomId, string targetUserId) { if (!await IsMemberOfRoom(id, roomType, roomId)) return "error:forbidden"; + //S8: targetUserId trafia do ścieżki pliku - bez walidacji odpowiedź + //"not.a.member" vs rola działała jako oracle istnienia dowolnych plików + //(probe "../../../.."). Pełny format "id;domena" zostaje zachowany + if (string.IsNullOrEmpty(targetUserId) || !Account.Utils.IsSafePathComponent(targetUserId)) return "error:not.a.member"; + string targetMemberPath = $"{GetRoomPath(roomType, roomId)}/members/{targetUserId}"; if (!Fs.Exists(targetMemberPath)) return "error:not.a.member"; @@ -141,6 +155,10 @@ public class Shared { start = s; } + //Clamp do [0, last] - patrz GetDmMessages (audyt W1: nieskończona + //pętla Fs.Exists przy startOffset poza zakresem) + if (start > last) start = last; + if (start < 0) start = 0; int count = 50; if (!string.IsNullOrEmpty(countStr) && int.TryParse(countStr, out int c)) @@ -320,8 +338,10 @@ public class Shared { if (memberId == id) { - if (roomType == "dms") await Account.Utils.UpdateUserDm(memberId, roomId, "true", msg.timestamp); - if (roomType == "groups") await Room.Groups.UpdateUserGroup(memberId, roomId, "true", msg.timestamp); + //dispatch trzyma już userLock(memberId==id) - rdzenie bez + //ponownego Wejścia w nie-reentrantny semaphore + if (roomType == "dms") await Account.Utils.UpdateUserDmLocked(memberId, roomId, "true", msg.timestamp); + if (roomType == "groups") await Room.Groups.UpdateUserGroupLocked(memberId, roomId, "true", msg.timestamp); await Utils.Utils.SendWsMessage(memberId, $"{GetRoomEventPrefix(roomType)}_message:{roomId}:read"); } @@ -333,8 +353,8 @@ public class Shared await userLock.WaitAsync(); try { - if (roomType == "dms") await Account.Utils.UpdateUserDm(memberId, roomId, "false", msg.timestamp); - if (roomType == "groups") await Room.Groups.UpdateUserGroup(memberId, roomId, "false", msg.timestamp); + if (roomType == "dms") await Account.Utils.UpdateUserDmLocked(memberId, roomId, "false", msg.timestamp); + if (roomType == "groups") await Room.Groups.UpdateUserGroupLocked(memberId, roomId, "false", msg.timestamp); } finally { @@ -953,9 +973,15 @@ public class Shared string roomType = serializedBody.string1; string roomId = serializedBody.string2; + //S7: wcześniej brak jakiejkolwiek weryfikacji - każdy zalogowany mógł + //"wyjść" z cudzych rozmów (usuwał uczestników z ActiveCalls dowolnego + //pokoju i generował broadcasty na podstawie craftowanych ścieżek) + if (!await IsMemberOfRoom(id, roomType, roomId)) return "error:forbidden"; + if (ActiveCalls.TryGetValue(roomId, out var callList)) { bool removed = false; + bool removeEntry = false; lock (callList) { if (callList.Contains(id)) @@ -963,6 +989,7 @@ public class Shared callList.Remove(id); removed = true; } + if (callList.Count == 0) removeEntry = true; } if (removed) @@ -980,6 +1007,15 @@ public class Shared } } } + + if (removeEntry) + { + //S1: puste wpisy muszą znikać - inaczej dictionary rósł bez granic, + //a disconnect cleanup skanował wszystkie historyczne pokoje. + //Ewentualny wyścig z równoległym GetOrAdd nowego joinera kończy się + //co najwyżej jednym pominiętym broadcastem - następny join/leave to naprawia + ActiveCalls.TryRemove(roomId, out _); + } } return "success"; } @@ -1011,6 +1047,10 @@ public class Shared return JsonSerializer.Serialize(participants, AppJsonSerializerContext.Default.ListString); } + //S12: sygnalizacja głosowa - limit rozmiaru payloadu (setupy klucza mieszczą + //się w kilobajtach; bez capu członek pokoju amplifikowałby ruch do wszystkich) + private const int MAX_SIGNAL_PAYLOAD_SIZE = 16 * 1024; + public static async Task RoomSignal(string id, string body) { Universal3String serializedBody = JsonSerializer.Deserialize( @@ -1023,13 +1063,56 @@ public class Shared if (!await IsMemberOfRoom(id, roomType, roomId)) return "error:forbidden"; - string[] members = Fs.ReadDirectory($"{GetRoomPath(roomType, roomId)}/members"); - foreach(string memberWD in members) + if (string.IsNullOrEmpty(signalPayload) || signalPayload.Length > MAX_SIGNAL_PAYLOAD_SIZE) { - if (Account.Utils.IsUserLocal(memberWD, out string _)) + return "error:invalid.signal"; + } + + //S12: pole 'from' jest nadpisywane uwierzytelnionym nadawcą - klient nie + //może podszyć się pod innego użytkownika (wcześniej odbiorca instalował + //klucz rozmowy wg client-asserted 'from') + string stamped; + string toId; + try + { + if (System.Text.Json.Nodes.JsonNode.Parse(signalPayload) is not System.Text.Json.Nodes.JsonObject signalObj) { + return "error:invalid.signal"; + } + signalObj["from"] = id; + toId = Account.Utils.GetValidIdOrZero(signalObj["to"]?.ToString() ?? ""); + stamped = signalObj.ToJsonString(); + } + catch + { + return "error:invalid.signal"; + } + + //S21: doręczenie do adresata zamiast broadcastu do wszystkich członków - + //sygnalizacja była O(N²): każdy target = request, serwer rozsyłał do + //wszystkich i klient filtrował lokalnie. Gdy 'to' nie pasuje do nikogo + //(kompatybilność/stare klienty), zostaje broadcast jak dotychczas + string[] members = Fs.ReadDirectory($"{GetRoomPath(roomType, roomId)}/members"); + bool targeted = false; + foreach (string memberWD in members) + { + if (!Account.Utils.IsUserLocal(memberWD, out string _)) continue; + string memberId = Account.Utils.GetValidIdOrZero(memberWD); + if (memberId != "0" && memberId == toId) + { + targeted = true; + _ = Utils.Utils.SendWsMessage(memberId, $"signal:{roomId}:{stamped}"); + break; + } + } + + if (!targeted) + { + foreach (string memberWD in members) + { + if (!Account.Utils.IsUserLocal(memberWD, out string _)) continue; string memberId = Account.Utils.GetValidIdOrZero(memberWD); - _ = Utils.Utils.SendWsMessage(memberId, $"signal:{roomId}:{signalPayload}"); + _ = Utils.Utils.SendWsMessage(memberId, $"signal:{roomId}:{stamped}"); } } return "success"; diff --git a/LarpixServer/Utils/Utils.cs b/LarpixServer/Utils/Utils.cs index fa972c0..ff751c9 100644 --- a/LarpixServer/Utils/Utils.cs +++ b/LarpixServer/Utils/Utils.cs @@ -236,6 +236,7 @@ public class Utils foreach (var kvp in ActiveCalls) { bool removed = false; + bool removeEntry = false; lock (kvp.Value) { if (kvp.Value.Contains(id)) @@ -243,6 +244,7 @@ public class Utils kvp.Value.Remove(id); removed = true; } + if (kvp.Value.Count == 0) removeEntry = true; } if (removed) @@ -263,6 +265,14 @@ public class Utils } } } + + if (removeEntry) + { + //S1: przycinaj puste wpisy - bez tego dictionary rósł o każdy + //pokój, w którym kiedykolwiek była rozmowa, a każdy disconnect + //skanował wszystkie historyczne wpisy + ActiveCalls.TryRemove(kvp.Key, out _); + } } } } @@ -271,6 +281,13 @@ public class Utils //(równoległe SendAsync na tym samym sockecie rzucają InvalidOperationException). public static readonly ConcurrentDictionary WsSendLocks = new(); + //Timeout pojedynczej wysyłki WS. Bez niego klient, który uwierzytelnił się + //a potem przestał odbierać TCP (zero-window), zawiesza SendAsync w + //nieskończoność: semafor per-socket zajęty na zawsze => wieczne waitery + //(pamięć), a await siedzi wewnątrz userLock/roomLock i blokuje całe ścieżki + //żądań tego użytkownika/pokoju (audyt W2) + public static readonly TimeSpan WsSendTimeout = TimeSpan.FromMinutes(1); + private static SemaphoreSlim GetWsSendLock(System.Net.WebSockets.WebSocket ws) { return WsSendLocks.GetOrAdd(ws, _ => new SemaphoreSlim(1, 1)); @@ -303,7 +320,14 @@ public class Utils { try { - await ws.SendAsync(new ArraySegment(bytes), System.Net.WebSockets.WebSocketMessageType.Text, true, CancellationToken.None); + using var sendCts = new System.Threading.CancellationTokenSource(WsSendTimeout); + await ws.SendAsync(new ArraySegment(bytes), System.Net.WebSockets.WebSocketMessageType.Text, true, sendCts.Token); + } + catch (OperationCanceledException) + { + //Timeout wysyłki = socket przestał odbierać - usuwamy go + //z aktywnych, żeby nie blokował semafora na zawsze + remove = true; } catch { diff --git a/LarpixVoice/Requests.cs b/LarpixVoice/Requests.cs index a73d909..3313649 100644 --- a/LarpixVoice/Requests.cs +++ b/LarpixVoice/Requests.cs @@ -90,6 +90,15 @@ public class Requests } currentSession = new Session(webSocket); + if (oldSession != null && !ReferenceEquals(oldSession, currentSession)) + { + //W4: nadpisujemy wpis w mapie - stara sesja nigdy nie przejdzie przez + //swoją gałąź finally (guard session==currentSession), więc jej kolejkę + //trzeba zamknąć TU. Bez tego task-pump starej sesji wisi wiecznie na + //ReadAllAsync = leak taska + kanału + buforów ArrayPool przy każdym + //reconnectcie + oldSession.SendQueue.Writer.TryComplete(); + } roomClients[clientId] = currentSession; _ = Task.Run(async () =>