From 84ff797b5fc0f2c15c95fe4e35a5456b2c51e00c Mon Sep 17 00:00:00 2001 From: James Lu Date: Sat, 7 Oct 2017 20:01:53 -0700 Subject: [PATCH 1/5] clientbot: rewrite _get_UID nick collision handling to be less confusing --- protocols/clientbot.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/protocols/clientbot.py b/protocols/clientbot.py index 7e3cf92..d023f11 100644 --- a/protocols/clientbot.py +++ b/protocols/clientbot.py @@ -355,20 +355,22 @@ class ClientbotWrapperProtocol(IRCCommonProtocol): Limited (internal) nick collision checking is done here to prevent Clientbot users from being confused with virtual clients, and vice versa.""" self._check_puid_collision(nick) + idsource = self.nick_to_uid(nick) - is_internal = self.is_internal_client(idsource) - # If this sender isn't known or it is one of our virtual clients, spawn a new one. - # This also takes care of any nick collisions caused by new, Clientbot users - # taking the same nick as one of our virtual clients, and will force the virtual client to lose. - if (not idsource) or (is_internal and self.pseudoclient and idsource != self.pseudoclient.uid): - if idsource: - log.debug('(%s) Nick-colliding virtual client %s/%s', self.name, idsource, nick) - self.call_hooks([self.sid, 'CLIENTBOT_NICKCOLLIDE', {'target': idsource, 'parse_as': 'SAVE'}]) + if self.is_internal_client(idsource) and self.pseudoclient and idsource != self.pseudoclient.uid: + # We got a message from a client with the same nick as an internal client. + # Fire a virtual nick collision to prevent mixing senders. + log.debug('(%s) Nick-colliding virtual client %s/%s', self.name, idsource, nick) + self.call_hooks([self.sid, 'SAVE', {'target': idsource}]) + # Clear the UID for this nick and spawn a new client for the nick that was just freed. + idsource = None + + if idsource is None: + # If this sender doesn't already exist, spawn a new client. idsource = self.spawn_client(nick, ident or 'unknown', host or 'unknown', server=self.uplink, realname=FALLBACK_REALNAME).uid - return idsource def parse_message_tags(self, data): From d7766d54d5ffd1c71bb4ec2d8a23ceea9f18de5e Mon Sep 17 00:00:00 2001 From: James Lu Date: Sat, 7 Oct 2017 20:06:48 -0700 Subject: [PATCH 2/5] clientbot: check for nick collisions with virtual clients on NICK Closes #535. --- protocols/clientbot.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/protocols/clientbot.py b/protocols/clientbot.py index d023f11..3e7190d 100644 --- a/protocols/clientbot.py +++ b/protocols/clientbot.py @@ -977,21 +977,25 @@ class ClientbotWrapperProtocol(IRCCommonProtocol): def handle_nick(self, source, command, args): """Handles NICK changes.""" # <- :GL|!~GL@127.0.0.1 NICK :GL_ + newnick = args[0] if not self.connected.is_set(): # We haven't properly logged on yet, so any initial NICK should be treated as a forced # nick change for us. For example, this clause is used to handle forced nick changes # sent by ZNC, when the login nick and the actual IRC nick of the bouncer differ. - self.pseudoclient.nick = args[0] + self.pseudoclient.nick = newnick log.debug('(%s) Pre-auth FNC: Changing our nick to %s', self.name, args[0]) return elif source == self.pseudoclient.uid: self._nick_fails = 0 # Our last nick change succeeded. oldnick = self.users[source].nick - self.users[source].nick = args[0] - return {'newnick': args[0], 'oldnick': oldnick} + # Check for any nick collisions with existing virtual clients. + self._check_nick_collision(newnick) + self.users[source].nick = newnick + + return {'newnick': newnick, 'oldnick': oldnick} def handle_part(self, source, command, args): """ From 740b399ec2f089e6ada109f719ed53932c049061 Mon Sep 17 00:00:00 2001 From: James Lu Date: Sat, 7 Oct 2017 20:50:09 -0700 Subject: [PATCH 3/5] clientbot: block attempts from virtual clients to change to an existing nick (#535) --- protocols/clientbot.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/protocols/clientbot.py b/protocols/clientbot.py index 3e7190d..b26b1c5 100644 --- a/protocols/clientbot.py +++ b/protocols/clientbot.py @@ -253,6 +253,13 @@ class ClientbotWrapperProtocol(IRCCommonProtocol): self.send('NICK :%s' % newnick) # No state update here: the IRCd will respond with a NICK acknowledgement if the change succeeds. else: + assert source, "No source given?" + # Check that the new nick exists and isn't the same client as the sender + # (for changing nick case) + nick_uid = self.nick_to_uid(newnick) + if nick_uid and nick_uid != source: + log.warning('(%s) Blocking attempt from virtual client %s to change nick to %s (nick in use)', self.name, source, newnick) + return self.call_hooks([source, 'CLIENTBOT_NICK', {'newnick': newnick}]) self.users[source].nick = newnick From de5ab051aab8a2ce1935351dc13e49c3471baa6f Mon Sep 17 00:00:00 2001 From: James Lu Date: Sat, 7 Oct 2017 21:47:23 -0700 Subject: [PATCH 4/5] clientbot: rename cap. clear-channels-on-leave => visible-state-only (#517) --- protocols/clientbot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/clientbot.py b/protocols/clientbot.py index b26b1c5..0442f62 100644 --- a/protocols/clientbot.py +++ b/protocols/clientbot.py @@ -25,7 +25,7 @@ class ClientbotWrapperProtocol(IRCCommonProtocol): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.protocol_caps = {'clear-channels-on-leave', 'slash-in-nicks', 'slash-in-hosts', 'underscore-in-hosts'} + self.protocol_caps = {'visible-state-only', 'slash-in-nicks', 'slash-in-hosts', 'underscore-in-hosts'} self.has_eob = False From eca40a3d7c3a181277aef7700d2c34472c888716 Mon Sep 17 00:00:00 2001 From: James Lu Date: Sat, 7 Oct 2017 21:49:17 -0700 Subject: [PATCH 5/5] coremods/handlers: implement cleanup code for visible-state-only servers Closes #536. Closes #517. --- coremods/handlers.py | 26 ++++++++++++++++++++++++++ protocols/clientbot.py | 13 ++----------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/coremods/handlers.py b/coremods/handlers.py index 8e77ef5..20108b7 100644 --- a/coremods/handlers.py +++ b/coremods/handlers.py @@ -163,3 +163,29 @@ def handle_time(irc, source, command, args): timestring = time.ctime() irc.numeric(irc.sid, 391, source, '%s :%s' % (irc.hostname(), timestring)) utils.add_hook(handle_time, 'TIME') + +def _state_cleanup_core(irc, source, channel): + """ + Handles PART and KICK on clientbot-like networks (where only the users and channels we see are available) + by deleting channels when we leave and users when they leave all shared channels. + """ + if irc.has_cap('visible-state-only'): + # Delete channels that we were removed from. + if irc.pseudoclient and source == irc.pseudoclient.uid: + log.debug('(%s) state_cleanup: removing channel %s since we have left', irc.name, channel) + del irc._channels[channel] + + # Delete users no longer sharing a channel with us. + if not irc.users[source].channels: + log.debug('(%s) state_cleanup: removing user %s/%s who no longer shares a channel with us', + irc.name, source, irc.users[source].nick) + irc._remove_client(source) + +def stats_cleanup_part(irc, source, command, args): + for channel in args['channels']: + _state_cleanup_core(irc, source, channel) +utils.add_hook(stats_cleanup_part, 'PART', priority=-100) + +def stats_cleanup_kick(irc, source, command, args): + _state_cleanup_core(irc, args['target'], args['channel']) +utils.add_hook(stats_cleanup_kick, 'KICK', priority=-100) diff --git a/protocols/clientbot.py b/protocols/clientbot.py index 0442f62..828647e 100644 --- a/protocols/clientbot.py +++ b/protocols/clientbot.py @@ -916,11 +916,7 @@ class ClientbotWrapperProtocol(IRCCommonProtocol): if (not self.is_internal_client(source)) and not self.is_internal_server(source): # Don't repeat hooks if we're the kicker. - self.call_hooks([source, 'KICK', {'channel': channel, 'target': target, 'text': reason}]) - - # Delete channels that we were kicked from, for better state keeping. - if self.pseudoclient and target == self.pseudoclient.uid: - del self._channels[channel] + return {'channel': channel, 'target': target, 'text': reason} def handle_mode(self, source, command, args): """Handles MODE changes.""" @@ -1019,12 +1015,7 @@ class ClientbotWrapperProtocol(IRCCommonProtocol): self._channels[channel].remove_user(source) self.users[source].channels -= set(channels) - self.call_hooks([source, 'PART', {'channels': channels, 'text': reason}]) - - # Clear channels that are empty, or that we're parting. - for channel in channels: - if (self.pseudoclient and source == self.pseudoclient.uid) or not self._channels[channel].users: - del self._channels[channel] + return {'channels': channels, 'text': reason} def handle_ping(self, source, command, args): """