From f73fe5095e4cba1d025ec33065fcec1dfca04527 Mon Sep 17 00:00:00 2001 From: Valentin Lorentz Date: Wed, 23 Nov 2022 20:33:48 +0100 Subject: [PATCH] Replace makeExtBanmask with makeExtBanmasks Now that we can return both account extbans and regular masks, it makes sense to ban both. Otherwise, adding 'account' to supybot.protocols.irc.banmask means we banned only the account instead of the hostmask, which arguably makes the ban weaker (/NS LOGOUT to evade) --- plugins/AutoMode/plugin.py | 4 +-- plugins/Channel/plugin.py | 52 ++++++++++++++++++++++++-------------- plugins/Channel/test.py | 28 +++++++++++++------- src/commands.py | 6 ++--- src/conf.py | 37 ++++++++++++++++++++------- 5 files changed, 85 insertions(+), 42 deletions(-) diff --git a/plugins/AutoMode/plugin.py b/plugins/AutoMode/plugin.py index a3e8db67c..f11ed3238 100644 --- a/plugins/AutoMode/plugin.py +++ b/plugins/AutoMode/plugin.py @@ -163,9 +163,9 @@ class AutoMode(callbacks.Plugin): # We're not in the channel anymore. pass schedule.addEvent(unban, time.time()+period) - banmask = conf.supybot.protocols.irc.banmask.makeExtBanmask( + banmasks = conf.supybot.protocols.irc.banmask.makeExtBanmasks( msg.prefix, channel=channel, network=irc.network) - irc.queueMsg(ircmsgs.ban(channel, banmask)) + irc.queueMsg(ircmsgs.bans(channel, banmasks)) irc.queueMsg(ircmsgs.kick(channel, msg.nick)) try: diff --git a/plugins/Channel/plugin.py b/plugins/Channel/plugin.py index 815a1ef18..5d8afa167 100644 --- a/plugins/Channel/plugin.py +++ b/plugins/Channel/plugin.py @@ -368,7 +368,7 @@ class Channel(callbacks.Plugin): try: bannedHostmask = irc.state.nickToHostmask(target) banmaskstyle = conf.supybot.protocols.irc.banmask - banmask = banmaskstyle.makeExtBanmask( + banmasks = banmaskstyle.makeExtBanmasks( bannedHostmask, [o[0] for o in optlist], channel=channel, network=irc.network) except KeyError: @@ -376,12 +376,14 @@ class Channel(callbacks.Plugin): target.startswith('$'): # Select the last part, or the whole target: bannedNick = target.split(':')[-1] - banmask = bannedHostmask = target + bannedHostmask = target + banmasks = [bannedHostmask] else: irc.error(format(_('I haven\'t seen %s.'), bannedNick), Raise=True) else: bannedNick = ircutils.nickFromHostmask(target) - banmask = bannedHostmask = target + bannedHostmask = target + banmasks = [bannedHostmask] if not irc.isNick(bannedNick): self.log.warning('%q tried to kban a non nick: %q', msg.prefix, bannedNick) @@ -398,29 +400,38 @@ class Channel(callbacks.Plugin): reason = msg.nick capability = ircdb.makeChannelCapability(channel, 'op') # Check (again) that they're not trying to make us kickban ourself. - if ircutils.hostmaskPatternEqual(banmask, irc.prefix): - if ircutils.hostmaskPatternEqual(bannedHostmask, irc.prefix): - self.log.warning('%q tried to make me kban myself.',msg.prefix) - irc.error(_('I cowardly refuse to ban myself.')) - return - else: - self.log.warning('Using exact hostmask since banmask would ' - 'ban myself.') - banmask = bannedHostmask + for banmask in banmasks: + # TODO: check account ban too + if ircutils.hostmaskPatternEqual(banmask, irc.prefix): + if ircutils.hostmaskPatternEqual(bannedHostmask, irc.prefix): + self.log.warning('%q tried to make me kban myself.',msg.prefix) + irc.error(_('I cowardly refuse to ban myself.')) + return + else: + self.log.warning('Using exact hostmask since banmask would ' + 'ban myself.') + banmasks = [bannedHostmask] # Now, let's actually get to it. Check to make sure they have # #channel,op and the bannee doesn't have #channel,op; or that the # bannee and the banner are both the same person. def doBan(): if irc.state.channels[channel].isOp(bannedNick): irc.queueMsg(ircmsgs.deop(channel, bannedNick)) - irc.queueMsg(ircmsgs.ban(channel, banmask)) + irc.queueMsg(ircmsgs.bans(channel, banmasks)) if kick: irc.queueMsg(ircmsgs.kick(channel, bannedNick, reason)) if expiry > 0: def f(): - if channel in irc.state.channels and \ - banmask in irc.state.channels[channel].bans: - irc.queueMsg(ircmsgs.unban(channel, banmask)) + if channel not in irc.state.channels: + return + remaining_banmasks = [ + banmask + for banmask in banmasks + if banmask in irc.state.channels[channel].bans + ] + if remaining_banmasks: + irc.queueMsg(ircmsgs.unbans( + channel, remaining_banmasks)) schedule.addEvent(f, expiry) if bannedNick == msg.nick: doBan() @@ -591,7 +602,7 @@ class Channel(callbacks.Plugin): hostmask = wrap(hostmask, ['op', ('haveHalfop+', _('ban someone')), 'text']) @internationalizeDocstring - def add(self, irc, msg, args, channel, banmask, expires): + def add(self, irc, msg, args, channel, banmasks, expires): """[] [] If you have the #channel,op capability, this will effect a @@ -605,11 +616,14 @@ class Channel(callbacks.Plugin): channel itself. """ c = ircdb.channels.getChannel(channel) - c.addBan(banmask, expires) + if isinstance(banmasks, str): + banmasks = [banmasks] + for banmask in banmasks: + c.addBan(banmask, expires) ircdb.channels.setChannel(channel, c) irc.replySuccess() add = wrap(add, ['op', - first('hostmask', 'extbanmask'), + first('hostmask', 'extbanmasks'), additional('expiry', 0)]) @internationalizeDocstring diff --git a/plugins/Channel/test.py b/plugins/Channel/test.py index 35b71c3c8..bd716767c 100644 --- a/plugins/Channel/test.py +++ b/plugins/Channel/test.py @@ -29,6 +29,8 @@ # POSSIBILITY OF SUCH DAMAGE. ### +import itertools + from supybot.test import * import supybot.conf as conf @@ -161,9 +163,13 @@ class ChannelTestCase(ChannelPluginTestCase): self.assertTrue(m.command == 'MODE' and m.args == (self.channel, '+v', 'bar')) - def assertKban(self, query, hostmask, **kwargs): + def assertKban(self, query, *hostmasks, **kwargs): m = self.getMsg(query, **kwargs) - self.assertEqual(m, ircmsgs.ban(self.channel, hostmask)) + self.assertIn(m, [ + ircmsgs.bans(self.channel, permutation) + for permutation in itertools.permutations(hostmasks) + ]) + m = self.getMsg(' ') self.assertEqual(m.command, 'KICK') def assertBan(self, query, hostmask, **kwargs): @@ -278,25 +284,29 @@ class ChannelTestCase(ChannelPluginTestCase): for style in (['exact'], ['account', 'exact']): with conf.supybot.protocols.irc.banmask.context(style): self.assertKban('kban --account --exact foobar', - '~a:account1') + '~a:account1', 'foobar!user@host.domain.tld') join() self.assertKban('kban --account foobar', '~a:account1') join() self.assertKban('kban --account --host foobar', - '~a:account1') + '~a:account1', '*!*@host.domain.tld') join() with conf.supybot.protocols.irc.banmask.context(['account', 'exact']): self.assertKban('kban foobar', - '~a:account1') + '~a:account1', 'foobar!user@host.domain.tld') + join() + + with conf.supybot.protocols.irc.banmask.context(['account', 'host']): + self.assertKban('kban foobar', + '~a:account1', '*!*@host.domain.tld') join() def testBan(self): with conf.supybot.protocols.irc.banmask.context(['exact']): self.assertNotError('ban add foo!bar@baz') self.assertNotError('ban remove foo!bar@baz') - orig = conf.supybot.protocols.irc.strictRfc() with conf.supybot.protocols.irc.strictRfc.context(True): # something wonky is going on here. irc.error (src/Channel.py|449) # is being called but the assert is failing @@ -322,7 +332,6 @@ class ChannelTestCase(ChannelPluginTestCase): '"foobar!*@baz" (never expires)') def testIgnore(self): - orig = conf.supybot.protocols.irc.banmask() def ignore(given, expect=None): if expect is None: expect = given @@ -330,8 +339,9 @@ class ChannelTestCase(ChannelPluginTestCase): self.assertResponse('channel ignore list', "'%s'" % expect) self.assertNotError('channel ignore remove %s' % expect) self.assertRegexp('channel ignore list', 'not currently') - ignore('foo!bar@baz', '*!*@baz') - ignore('foo!*@*') + with conf.supybot.protocols.irc.banmask.context(['host']): + ignore('foo!bar@baz', '*!*@baz') + ignore('foo!*@*') with conf.supybot.protocols.irc.banmask.context(['exact']): ignore('foo!bar@baz') ignore('foo!*@*') diff --git a/src/commands.py b/src/commands.py index 2874c4692..7da905fa4 100644 --- a/src/commands.py +++ b/src/commands.py @@ -437,11 +437,11 @@ def getBanmask(irc, msg, args, state): state.args[-1] = banmaskstyle.makeBanmask(state.args[-1], channel=state.channel, network=irc.network) -def getExtBanmask(irc, msg, args, state): +def getExtBanmasks(irc, msg, args, state): getHostmask(irc, msg, args, state) getChannel(irc, msg, args, state) banmaskstyle = conf.supybot.protocols.irc.extbanmask - state.args[-1] = banmaskstyle.makeExtBanmask(state.args[-1], + state.args[-1] = banmaskstyle.makeExtBanmasks(state.args[-1], channel=state.channel, network=irc.network) def getUser(irc, msg, args, state): @@ -813,7 +813,7 @@ wrappers = ircutils.IrcDict({ 'commandName': getCommandName, 'email': getEmail, 'expiry': getExpiry, - 'extbanmask': getExtBanmask, + 'extbanmasks': getExtBanmasks, 'filename': getSomething, # XXX Check for validity. 'float': getFloat, 'glob': getGlob, diff --git a/src/conf.py b/src/conf.py index 975852c34..7c55b188d 100644 --- a/src/conf.py +++ b/src/conf.py @@ -1215,7 +1215,7 @@ class Banmask(registry.SpaceSeparatedSetOfStrings): isn't specified via options, the value of conf.supybot.protocols.irc.banmask is used. - Unlike :meth:`makeExtBanmask`, this is guaranteed to return an + Unlike :meth:`makeExtBanmasks`, this is guaranteed to return an RFC1459-like mask, suitable for ircdb's ignore lists. options - A list specifying which parts of the hostmask should @@ -1228,10 +1228,15 @@ class Banmask(registry.SpaceSeparatedSetOfStrings): options = supybot.protocols.irc.banmask.getSpecific( network, channel)() options = [option for option in options if option != 'account'] - return self.makeExtBanmask(hostmask, options, channel, network=network) + print(hostmask, options) + masks = self.makeExtBanmasks( + hostmask, options, channel, network=network) + assert len(masks) == 1, 'Unexpected number of banmasks: %r' % masks + print(masks) + return masks[0] - def makeExtBanmask(self, hostmask, options=None, channel=None, *, network): - """Create a banmask from the given hostmask. If a style of banmask + def makeExtBanmasks(self, hostmask, options=None, channel=None, *, network): + """Create banmasks from the given hostmask. If a style of banmask isn't specified via options, the value of conf.supybot.protocols.irc.banmask is used. @@ -1256,15 +1261,22 @@ class Banmask(registry.SpaceSeparatedSetOfStrings): if not options: options = supybot.protocols.irc.banmask.getSpecific( network, channel)() + + add_star_mask = False + masks = [] + for option in options: if option == 'nick': bnick = nick + add_star_mask = True elif option == 'user': buser = user + add_star_mask = True elif option == 'host': bhost = host + add_star_mask = True elif option == 'exact': - return hostmask + masks.append(hostmask) elif option == 'account': import supybot.world as world irc = world.getIrc(network) @@ -1272,11 +1284,18 @@ class Banmask(registry.SpaceSeparatedSetOfStrings): continue extban = ircutils.accountExtban(nick, irc) if extban is not None: - return extban + masks.append(extban) + + if add_star_mask and (bnick, buser, bhost) != ('*', '*', '*'): + masks.append(ircutils.joinHostmask(bnick, buser, bhost)) + if (bnick, buser, bhost) == ('*', '*', '*') and \ - ircutils.isUserHostmask(hostmask): - return hostmask - return ircutils.joinHostmask(bnick, buser, bhost) + ircutils.isUserHostmask(hostmask) and \ + masks == []: + masks.append(hostmask) + + return masks + registerChannelValue(supybot.protocols.irc, 'banmask', Banmask(['host'], _("""Determines what will be used as the