From 4819406af577d1c8fbe08bc8b1b0addef5437087 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Thu, 9 May 2024 17:24:53 +0200 Subject: [PATCH 1/4] ident: order imports --- server/identification.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/identification.ts b/server/identification.ts index 1c01d793..96648083 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -1,9 +1,9 @@ -import log from "./log"; import fs from "fs"; import net, {Socket} from "net"; import colors from "chalk"; import Helper from "./helper"; import Config from "./config"; +import log from "./log"; type Connection = { socket: Socket; From 0e48014d5a7a8a584cf007b3c3017e0fa43c9976 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Thu, 9 May 2024 17:41:12 +0200 Subject: [PATCH 2/4] ident: only respond if the ip,port tuples match Per RFC 1413, The uniquely identifying tuple includes not only the ports, but also both addresses. If multiple connections happen to use the same local port number (which is possible if the addresses differ), the username of the first is returned for all, resulting in the wrong ident for all but the first. By not checking the connection address, the information becomes public. Because there is only relatively small number of local ports, and the remote ports are likely to be either 6667 or 6697, it becomes trivial to enumerate all the users. Co-Authored-By: Juerd Waalboer --- server/identification.ts | 42 ++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/server/identification.ts b/server/identification.ts index 96648083..44dff894 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -73,24 +73,41 @@ class Identification { } respondToIdent(socket: Socket, buffer: Buffer) { + if (!socket.remoteAddress) { + log.warn("identd: no remote address"); + return; + } + const data = buffer.toString().split(","); const lport = parseInt(data[0], 10) || 0; const fport = parseInt(data[1], 10) || 0; if (lport < 1 || fport < 1 || lport > 65535 || fport > 65535) { + log.warn(`identd: bogus request from ${socket.remoteAddress}`); return; } + log.debug(`identd: remote ${socket.remoteAddress} query ${lport}, ${fport}`); + for (const connection of this.connections.values()) { - if (connection.socket.remotePort === fport && connection.socket.localPort === lport) { - return socket.write( - `${lport}, ${fport} : USERID : TheLounge : ${connection.user}\r\n` - ); + // we only want to respond if all the ip,port tuples match, to avoid user enumeration + if ( + connection.socket.remotePort === fport && + connection.socket.localPort === lport && + socket.remoteAddress === connection.socket.remoteAddress && + socket.localAddress === connection.socket.localAddress + ) { + const reply = `${lport}, ${fport} : USERID : TheLounge : ${connection.user}\r\n`; + log.debug(`identd: reply is ${reply.trimEnd()}`); + socket.write(reply); + return; } } - socket.write(`${lport}, ${fport} : ERROR : NO-USER\r\n`); + const reply = `${lport}, ${fport} : ERROR : NO-USER\r\n`; + log.debug(`identd: reply is ${reply.trimEnd()}`); + socket.write(reply); } addSocket(socket: Socket, user: string) { @@ -127,8 +144,21 @@ class Identification { return; } + if (!connection.socket.remoteAddress) { + log.warn(`oidentd: socket has no remote address, will not respond to queries`); + return; + } + + if (!connection.socket.localAddress) { + log.warn(`oidentd: socket has no local address, will not respond to queries`); + return; + } + + // we only want to respond if all the ip,port tuples match, to avoid user enumeration file += - `fport ${connection.socket.remotePort}` + + `to ${connection.socket.remoteAddress}` + + ` fport ${connection.socket.remotePort}` + + ` from ${connection.socket.localAddress}` + ` lport ${connection.socket.localPort}` + ` { reply "${connection.user}" }\n`; }); From 12679081c848ee2a1fc4acb894f81b77c97b56aa Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Thu, 9 May 2024 17:44:58 +0200 Subject: [PATCH 3/4] ident: fix write after connection end We only respond once to data, then half-close the connection. Hence, we should only listen to a single data event as well, else if the remote doesn't stop sending data we keep trying to write to the closed write end of the pipe. --- server/identification.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/identification.ts b/server/identification.ts index 44dff894..2fc1f758 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -66,7 +66,7 @@ class Identification { serverConnection(socket: Socket) { socket.on("error", (err: string) => log.error(`Identd socket error: ${err}`)); - socket.on("data", (data) => { + socket.once("data", (data) => { this.respondToIdent(socket, data); socket.end(); }); From 29fcc2da053b659ea187f8a9e90d0128c5a60779 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Thu, 9 May 2024 18:35:40 +0200 Subject: [PATCH 4/4] ident: close connections if they don't send data --- server/identification.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/identification.ts b/server/identification.ts index 2fc1f758..03a29687 100644 --- a/server/identification.ts +++ b/server/identification.ts @@ -66,6 +66,14 @@ class Identification { serverConnection(socket: Socket) { socket.on("error", (err: string) => log.error(`Identd socket error: ${err}`)); + socket.setTimeout(5000, () => { + log.warn( + `identd: no data received, closing connection to ${ + socket.remoteAddress || "undefined" + }` + ); + socket.destroy(); + }); socket.once("data", (data) => { this.respondToIdent(socket, data); socket.end();