-
Notifications
You must be signed in to change notification settings - Fork 669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Identd: fix various issues #4872
Conversation
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 <juerd@tnx.nl>
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.
@Juerd I've added you as co-authored-by, hope that's ok but if not ping me and I'll amend the commit |
connection.socket.remotePort === fport && | ||
connection.socket.localPort === lport && | ||
socket.remoteAddress === connection.socket.remoteAddress && | ||
socket.localAddress === connection.socket.localAddress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ignores ipv4 in ipv6 handling...
Meaning if for some reason our ident server listens on ipv6 (dual stack) but the IRC connection is in ipv4 we will wrongly send an not found response.
Considering that you have to try rather hard to get to that edge case, I chose not to care.
Reto skribis 2024-05-09 10:06 (-0700):
@Juerd I've added you as co-authored-by, hope that's ok but if not ping me and I'll amend the commit
That is fine, thanks for confirming this with me.
|
There's a bunch of sub optimal behavior from our ident server.
For one, it allows user enumeration which we don't really want and it doesn't clean up connections that don't send any data.
Fix that