diff --git a/packages/transfer-server/src/JsBridgeE2EEServer.ts b/packages/transfer-server/src/JsBridgeE2EEServer.ts index 69492b7..ec4d942 100644 --- a/packages/transfer-server/src/JsBridgeE2EEServer.ts +++ b/packages/transfer-server/src/JsBridgeE2EEServer.ts @@ -10,6 +10,7 @@ import type { IJsBridgeMessagePayload, IJsonRpcRequest, } from '@onekeyfe/cross-inpage-provider-types'; +import type { RoomManager } from './roomManager'; import type { Socket } from 'socket.io'; const logger = createModuleLogger('jsBridge'); @@ -46,11 +47,43 @@ const CLIENT_TO_CLIENT_RATE_LIMIT_ERROR_CODE = -387_155_488; // Rate limiting whitelist - methods that are exempt from rate limiting const RATE_LIMIT_WHITELIST = new Set([ 'changeTransferDirection', - 'getRoomUsers', 'leaveRoom', 'cancelTransfer', ]); +/** + * Per-method rate limit windows, overriding RATE_LIMIT_INTERVAL_MS. + * + * This table is for legitimate high-frequency callers that a shipped client + * already depends on. It is not a way to opt out of rate limiting: a method + * absent from here is limited at RATE_LIMIT_INTERVAL_MS, and that default is + * the point - a newly added method is protected without anyone having to + * remember to protect it. + * + * Adding an entry means stating, on that entry, which caller needs it, at what + * frequency, and why the default window cannot serve it. An entry is a + * compatibility shim and has a lifetime: remove it once its caller no longer + * needs it, rather than leaving a permanent hole behind. + * + * A window also has to stay meaningfully below the caller's polling interval. + * setInterval fixes the interval at which requests are *sent*; network latency + * shifts every request by roughly the same amount, so it cancels out of the gap + * the server observes, and only jitter moves that gap - in both directions. A + * 1000ms window against a 1000ms poll therefore sits exactly on the threshold + * rather than safely above it: measured over localhost, where latency is under + * a millisecond and stable, timer drift alone still pushed one poll in 30 below + * it and into a rejection. + */ +const METHOD_RATE_LIMIT_INTERVAL_MS = new Map([ + // CLI, once per second for the whole pairing phase + // (ROOM_USERS_POLL_INTERVAL_MS in transfer-receiver-adapter.ts): it had no + // user-joined push to wait on, so it polls to notice a peer arriving. The + // default 3s window would reject two of every three polls. Remove this entry + // once the CLI consumes the user-joined event instead - the poll and this + // shim go together. + ['getRoomUsers', 800], +]); + const SUPPORTED_MESSAGE_TYPES: ReadonlySet = new Set([ IJsBridgeMessageTypes.REQUEST, IJsBridgeMessageTypes.RESPONSE, @@ -94,15 +127,21 @@ function checkBridgePayload( export class JsBridgeE2EEServer extends JsBridgeBase { constructor( config: IJsBridgeConfig, - { socketClient }: { socketClient: Socket }, + { + socketClient, + roomManager, + }: { socketClient: Socket; roomManager: RoomManager }, ) { super(config); this.socketClient = socketClient; + this.roomManager = roomManager; this.setup(); } private socketClient: Socket; + private roomManager: RoomManager; + /** * Rate limit state, scoped to this connection rather than kept in a * module-level map that lived for the lifetime of the process. @@ -257,8 +296,10 @@ export class JsBridgeE2EEServer extends JsBridgeBase { const now = Date.now(); const lastTime = this.rateLimitState.get(rateLimitKey); + const interval = + METHOD_RATE_LIMIT_INTERVAL_MS.get(method) ?? RATE_LIMIT_INTERVAL_MS; - if (lastTime !== undefined && now - lastTime < RATE_LIMIT_INTERVAL_MS) { + if (lastTime !== undefined && now - lastTime < interval) { sendErrorResponse(); return true; } @@ -298,7 +339,14 @@ export class JsBridgeE2EEServer extends JsBridgeBase { */ private pruneRateLimitState(now: number): void { for (const [key, time] of this.rateLimitState) { - if (now - time >= RATE_LIMIT_INTERVAL_MS) { + // Expire each entry against its own window, not the default one: a + // per-method window longer than the default would otherwise be dropped + // while still live, which is exactly the rate limit reset this function + // is written to prevent. + const method = key.slice(key.indexOf(':') + 1); + const interval = + METHOD_RATE_LIMIT_INTERVAL_MS.get(method) ?? RATE_LIMIT_INTERVAL_MS; + if (now - time >= interval) { this.rateLimitState.delete(key); } } @@ -430,6 +478,16 @@ export class JsBridgeE2EEServer extends JsBridgeBase { return undefined; } + // `socket.to(roomId)` is a delivery operator: it reads the membership of the + // recipients and never checks the sender's. Without this, any connected + // socket that knows a roomId can inject client-to-client calls into a room + // it never joined - bypassing the room-slot invariant the pairing flow + // relies on. Membership is authoritative in RoomManager, so ask it. + if (!this.roomManager.isUserInRoom(roomId, this.socketClient.id).isInRoom) { + this.logInvalidPayload(eventName, payload, 'sender is not a room member'); + return undefined; + } + return { payload: checked.payload, roomId }; } } diff --git a/packages/transfer-server/src/e2eeServerApi.ts b/packages/transfer-server/src/e2eeServerApi.ts index 32815a0..604a367 100644 --- a/packages/transfer-server/src/e2eeServerApi.ts +++ b/packages/transfer-server/src/e2eeServerApi.ts @@ -70,6 +70,7 @@ function createBridgeE2EEServer({ }, { socketClient, + roomManager, }, ); } diff --git a/packages/transfer-server/src/roomManager.ts b/packages/transfer-server/src/roomManager.ts index 508cd03..88617b5 100644 --- a/packages/transfer-server/src/roomManager.ts +++ b/packages/transfer-server/src/roomManager.ts @@ -46,6 +46,17 @@ export class RoomManager { /** * Create new room * @returns Room information (room ID and encryption key) + * + * NOTE on `encryptionKey` (returned here and as `roomKey` from joinRoom): + * this key is NOT used by the OneKey client, and it is not what protects + * transferred data. The client derives its own end-to-end key locally from + * material this server never sees - the pairing code shown in the QR code + * (only its roomId prefix ever reaches the server), an ECDHE shared secret + * negotiated directly between the two devices, and the room's user list. + * A malicious or compromised server therefore still cannot decrypt anything, + * because it holds none of that material. This server never encrypts or + * decrypts payloads with this key either - it only relays them. The field is + * kept for wire compatibility with existing clients. */ @e2eeApiMethod() async createRoom(): Promise<{ roomId: string; encryptionKey: string }> { @@ -101,6 +112,11 @@ export class RoomManager { * @param encryptionKey Encryption key * @param socketId User's Socket ID * @returns Join result + * + * The returned `roomKey` is server-generated and unused by the client - see + * the note on createRoom(). It is not part of the end-to-end encryption + * scheme; the client derives its own key from the pairing code and an ECDHE + * exchange this server is not party to. */ @e2eeApiMethod() async joinRoom( @@ -186,6 +202,20 @@ export class RoomManager { await context?.socketClient.join(roomId); + // Tell the members already in the room that someone joined. The server + // never pushed this before, so a peer that needed to know had to poll + // getRoomUsers instead - which is why that method carries a high call rate. + // + // Emitted from the joining socket rather than the server, so the joiner is + // excluded (it does not need to be told about itself), and only after + // join() resolves, so the membership the event describes is already in + // effect if a receiver immediately calls getRoomUsers. + context?.socketClient.to(roomId).emit("user-joined", { + roomId, + userId, + userCount: room.users.size, + }); + return { success: true, userId, @@ -310,18 +340,26 @@ export class RoomManager { ); } logger.debug({ roomId }, "room.getRoomUsers"); - const room = this.rooms.get(roomId); - if (!room) { - logger.debug({ roomId }, "room.getRoomUsersNotFound"); - return []; - } - // Validate that the socket is in the room + + // A room the caller cannot see must be indistinguishable from a room that + // does not exist. Returning [] for a missing room while throwing for a room + // the caller is not in turned this into a room existence oracle - and this + // method is exempt from rate limiting, so it could be probed at line speed. + // isUserInRoom() already reports false for a missing room, so both cases + // fail here identically, with the same error code and message. const socketValidation = this.isUserInRoom(roomId, context.socketClient.id); if (!socketValidation.isInRoom) { - throw new E2eeError( - E2eeErrorCode.SOCKET_NOT_IN_ROOM, - "Socket must be in the room to set transfer direction" + logger.debug( + { roomId, roomExists: this.rooms.has(roomId) }, + "room.getRoomUsersDenied" ); + throw new E2eeError(E2eeErrorCode.ROOM_NOT_FOUND, "Room not found"); + } + + // isInRoom implies the room exists. + const room = this.rooms.get(roomId); + if (!room) { + throw new E2eeError(E2eeErrorCode.ROOM_NOT_FOUND, "Room not found"); } const users: IE2EESocketUserInfo[] = sortBy( diff --git a/packages/transfer-server/src/types.ts b/packages/transfer-server/src/types.ts index 9c2a21f..0baadf3 100644 --- a/packages/transfer-server/src/types.ts +++ b/packages/transfer-server/src/types.ts @@ -8,8 +8,19 @@ export { E2eeError, E2eeErrorCode } from './errors'; export interface IServerToClientEvents { 'room-created': (data: { roomId: string; encryptionKey: string }) => void; 'room-joined': (data: { roomId: string; userId: string }) => void; - 'user-joined': (data: { userId: string; userCount: number }) => void; - 'user-left': (data: { userId: string; userCount: number }) => void; + // roomId is part of both payloads: a client multiplexes every room over one + // socket, so it needs to tell which room an event refers to. `user-left` has + // always been emitted with it - the declaration was simply out of date. + 'user-joined': (data: { + roomId: string; + userId: string; + userCount: number; + }) => void; + 'user-left': (data: { + roomId: string; + userId: string; + userCount: number; + }) => void; 'encrypted-data': (data: { encryptedData: string; senderId: string; @@ -48,6 +59,11 @@ export interface ISocketData { // Room data structure export interface IRoom { id: string; + // Server-generated key handed to clients on create/join. The OneKey client + // does not consume it, and this server never encrypts with it - payloads are + // relayed as-is. Real end-to-end protection comes from a key the clients + // derive themselves (pairing code + ECDHE shared secret + room user list), + // none of which this server holds. See RoomManager.createRoom(). encryptionKey: string; users: Map; transferDirection?: