From c5579a6a345fb4694a8bc08bd2516f7061466c77 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 5 May 2022 22:34:43 -0400 Subject: [PATCH] fix #1688 * Add ACCEPT-tracking functionality (authorizing users to send DMs despite +R or other applicable restrictions) * Sending a DM automatically accepts the recipient * Add explicit ACCEPT command --- irc/accept.go | 76 +++++++++++++++++++++++++++++++++++++++ irc/accept_test.go | 87 +++++++++++++++++++++++++++++++++++++++++++++ irc/client.go | 3 +- irc/commands.go | 4 +++ irc/fakelag_test.go | 20 +++++------ irc/handlers.go | 41 ++++++++++++++++++++- irc/help.go | 7 ++++ irc/misc_test.go | 20 +++++------ irc/modes_test.go | 25 ++++++------- irc/server.go | 2 ++ 10 files changed, 250 insertions(+), 35 deletions(-) create mode 100644 irc/accept.go create mode 100644 irc/accept_test.go diff --git a/irc/accept.go b/irc/accept.go new file mode 100644 index 00000000..e54bd2e4 --- /dev/null +++ b/irc/accept.go @@ -0,0 +1,76 @@ +package irc + +import ( + "sync" + + "github.com/ergochat/ergo/irc/utils" +) + +// tracks ACCEPT relationships, i.e., `accepter` is willing to receive DMs from +// `accepted` despite some restriction (currently the only relevant restriction +// is that `accepter` is +R and `accepted` is not logged in) + +type AcceptManager struct { + sync.RWMutex + + // maps recipient -> whitelist of permitted senders: + // this is what we actually check + clientToAccepted map[*Client]utils.HashSet[*Client] + // this is the reverse mapping, it's needed so we can + // clean up the forward mapping during (*Client).destroy(): + clientToAccepters map[*Client]utils.HashSet[*Client] +} + +func (am *AcceptManager) Initialize() { + am.clientToAccepted = make(map[*Client]utils.HashSet[*Client]) + am.clientToAccepters = make(map[*Client]utils.HashSet[*Client]) +} + +func (am *AcceptManager) MaySendTo(sender, recipient *Client) (result bool) { + am.RLock() + defer am.RUnlock() + return am.clientToAccepted[recipient].Has(sender) +} + +func (am *AcceptManager) Accept(accepter, accepted *Client) { + am.Lock() + defer am.Unlock() + + var m utils.HashSet[*Client] + + m = am.clientToAccepted[accepter] + if m == nil { + m = make(utils.HashSet[*Client]) + am.clientToAccepted[accepter] = m + } + m.Add(accepted) + + m = am.clientToAccepters[accepted] + if m == nil { + m = make(utils.HashSet[*Client]) + am.clientToAccepters[accepted] = m + } + m.Add(accepter) +} + +func (am *AcceptManager) Unaccept(accepter, accepted *Client) { + am.Lock() + defer am.Unlock() + + delete(am.clientToAccepted[accepter], accepted) + delete(am.clientToAccepters[accepted], accepter) +} + +func (am *AcceptManager) Remove(client *Client) { + am.Lock() + defer am.Unlock() + + for accepter := range am.clientToAccepters[client] { + delete(am.clientToAccepted[accepter], client) + } + for accepted := range am.clientToAccepted[client] { + delete(am.clientToAccepters[accepted], client) + } + delete(am.clientToAccepters, client) + delete(am.clientToAccepted, client) +} diff --git a/irc/accept_test.go b/irc/accept_test.go new file mode 100644 index 00000000..bf2e1022 --- /dev/null +++ b/irc/accept_test.go @@ -0,0 +1,87 @@ +package irc + +import ( + "testing" +) + +func TestAccept(t *testing.T) { + var am AcceptManager + am.Initialize() + + alice := new(Client) + bob := new(Client) + eve := new(Client) + + assertEqual(am.MaySendTo(alice, bob), false) + assertEqual(am.MaySendTo(bob, alice), false) + assertEqual(am.MaySendTo(alice, eve), false) + assertEqual(am.MaySendTo(eve, alice), false) + assertEqual(am.MaySendTo(bob, eve), false) + assertEqual(am.MaySendTo(eve, bob), false) + + am.Accept(alice, bob) + + assertEqual(am.MaySendTo(alice, bob), false) + assertEqual(am.MaySendTo(bob, alice), true) + assertEqual(am.MaySendTo(alice, eve), false) + assertEqual(am.MaySendTo(eve, alice), false) + assertEqual(am.MaySendTo(bob, eve), false) + assertEqual(am.MaySendTo(eve, bob), false) + + am.Accept(bob, alice) + + assertEqual(am.MaySendTo(alice, bob), true) + assertEqual(am.MaySendTo(bob, alice), true) + assertEqual(am.MaySendTo(alice, eve), false) + assertEqual(am.MaySendTo(eve, alice), false) + assertEqual(am.MaySendTo(bob, eve), false) + assertEqual(am.MaySendTo(eve, bob), false) + + am.Accept(bob, eve) + + assertEqual(am.MaySendTo(alice, bob), true) + assertEqual(am.MaySendTo(bob, alice), true) + assertEqual(am.MaySendTo(alice, eve), false) + assertEqual(am.MaySendTo(eve, alice), false) + assertEqual(am.MaySendTo(bob, eve), false) + assertEqual(am.MaySendTo(eve, bob), true) + + am.Remove(alice) + + assertEqual(am.MaySendTo(alice, bob), false) + assertEqual(am.MaySendTo(bob, alice), false) + assertEqual(am.MaySendTo(alice, eve), false) + assertEqual(am.MaySendTo(eve, alice), false) + assertEqual(am.MaySendTo(bob, eve), false) + assertEqual(am.MaySendTo(eve, bob), true) + + am.Remove(bob) + + assertEqual(am.MaySendTo(alice, bob), false) + assertEqual(am.MaySendTo(bob, alice), false) + assertEqual(am.MaySendTo(alice, eve), false) + assertEqual(am.MaySendTo(eve, alice), false) + assertEqual(am.MaySendTo(bob, eve), false) + assertEqual(am.MaySendTo(eve, bob), false) +} + +func TestAcceptInternal(t *testing.T) { + var am AcceptManager + am.Initialize() + + alice := new(Client) + bob := new(Client) + eve := new(Client) + + am.Accept(alice, bob) + am.Accept(bob, alice) + am.Accept(bob, eve) + am.Remove(alice) + am.Remove(bob) + + // assert that there is no memory leak + for _, client := range []*Client{alice, bob, eve} { + assertEqual(len(am.clientToAccepted[client]), 0) + assertEqual(len(am.clientToAccepters[client]), 0) + } +} diff --git a/irc/client.go b/irc/client.go index 62ce70c3..7e2ead9c 100644 --- a/irc/client.go +++ b/irc/client.go @@ -1318,8 +1318,7 @@ func (client *Client) destroy(session *Session) { // clean up server client.server.clients.Remove(client) - - // clean up self + client.server.accepts.Remove(client) client.server.accounts.Logout(client) if quitMessage == "" { diff --git a/irc/commands.go b/irc/commands.go index adc2d8ef..1d456d1a 100644 --- a/irc/commands.go +++ b/irc/commands.go @@ -75,6 +75,10 @@ var Commands map[string]Command func init() { Commands = map[string]Command{ + "ACCEPT": { + handler: acceptHandler, + minParams: 1, + }, "AMBIANCE": { handler: sceneHandler, minParams: 2, diff --git a/irc/fakelag_test.go b/irc/fakelag_test.go index eb13a5c7..bc1ff45b 100644 --- a/irc/fakelag_test.go +++ b/irc/fakelag_test.go @@ -125,31 +125,31 @@ func TestFakelag(t *testing.T) { func TestSuspend(t *testing.T) { window, _ := time.ParseDuration("1s") fl, _ := newFakelagForTesting(window, 3, 2, window) - assertEqual(fl.config.Enabled, true, t) + assertEqual(fl.config.Enabled, true) // suspend idempotently disables fl.Suspend() - assertEqual(fl.config.Enabled, false, t) + assertEqual(fl.config.Enabled, false) fl.Suspend() - assertEqual(fl.config.Enabled, false, t) + assertEqual(fl.config.Enabled, false) // unsuspend idempotently enables fl.Unsuspend() - assertEqual(fl.config.Enabled, true, t) + assertEqual(fl.config.Enabled, true) fl.Unsuspend() - assertEqual(fl.config.Enabled, true, t) + assertEqual(fl.config.Enabled, true) fl.Suspend() - assertEqual(fl.config.Enabled, false, t) + assertEqual(fl.config.Enabled, false) fl2, _ := newFakelagForTesting(window, 3, 2, window) fl2.config.Enabled = false // if we were never enabled, suspend and unsuspend are both no-ops fl2.Suspend() - assertEqual(fl2.config.Enabled, false, t) + assertEqual(fl2.config.Enabled, false) fl2.Suspend() - assertEqual(fl2.config.Enabled, false, t) + assertEqual(fl2.config.Enabled, false) fl2.Unsuspend() - assertEqual(fl2.config.Enabled, false, t) + assertEqual(fl2.config.Enabled, false) fl2.Unsuspend() - assertEqual(fl2.config.Enabled, false, t) + assertEqual(fl2.config.Enabled, false) } diff --git a/irc/handlers.go b/irc/handlers.go index 0b5e470c..860411df 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -145,6 +145,41 @@ func (server *Server) sendLoginSnomask(nickMask, accountName string) { server.snomasks.Send(sno.LocalAccounts, fmt.Sprintf(ircfmt.Unescape("Client $c[grey][$r%s$c[grey]] logged into account $c[grey][$r%s$c[grey]]"), nickMask, accountName)) } +// ACCEPT +// nicklist is a comma-delimited list of nicknames; each may be prefixed with - +// to indicate that it should be removed from the list +func acceptHandler(server *Server, client *Client, msg ircmsg.Message, rb *ResponseBuffer) bool { + for _, tNick := range strings.Split(msg.Params[0], ",") { + add := true + if strings.HasPrefix(tNick, "-") { + add = false + tNick = strings.TrimPrefix(tNick, "-") + } + + target := server.clients.Get(tNick) + if target == nil { + rb.Add(nil, server.name, "FAIL", "ACCEPT", "INVALID_USER", utils.SafeErrorParam(tNick), client.t("No such user")) + continue + } + + if add { + server.accepts.Accept(client, target) + } else { + server.accepts.Unaccept(client, target) + } + + // https://github.com/solanum-ircd/solanum/blob/main/doc/features/modeg.txt + // Charybdis/Solanum define various error numerics that could be sent here, + // but this doesn't seem important to me. One thing to note is that we are not + // imposing an upper bound on the size of the accept list, since in our + // implementation you can only ACCEPT clients who are actually present, + // and an attacker attempting to DoS has much easier resource exhaustion + // strategies available (for example, channel history buffers). + } + + return false +} + // AUTHENTICATE [||*] func authenticateHandler(server *Server, client *Client, msg ircmsg.Message, rb *ResponseBuffer) bool { session := rb.session @@ -2284,10 +2319,14 @@ func dispatchMessageToTarget(client *Client, tags map[string]string, histType hi return } // restrict messages appropriately when +R is set - if details.account == "" && user.HasMode(modes.RegisteredOnly) { + if details.account == "" && user.HasMode(modes.RegisteredOnly) && !server.accepts.MaySendTo(client, user) { rb.Add(nil, server.name, ERR_NEEDREGGEDNICK, client.Nick(), tnick, client.t("You must be registered to send a direct message to this user")) return } + if client.HasMode(modes.RegisteredOnly) && tDetails.account == "" { + // #1688: auto-ACCEPT on DM + server.accepts.Accept(client, user) + } if !client.server.Config().Server.Compatibility.allowTruncation { if !validateSplitMessageLen(histType, client.NickMaskString(), tnick, message) { rb.Add(nil, server.name, ERR_INPUTTOOLONG, client.Nick(), client.t("Line too long to be relayed without truncation")) diff --git a/irc/help.go b/irc/help.go index f8aa895b..3f9271a1 100644 --- a/irc/help.go +++ b/irc/help.go @@ -110,6 +110,13 @@ For instance, this would set the kill, oper, account and xline snomasks on dan: // Help contains the help strings distributed with the IRCd. var Help = map[string]HelpEntry{ // Commands + "accept": { + text: `ACCEPT + +ACCEPT allows the target user to send you direct messages, overriding any +restrictions that might otherwise prevent this. Currently, the only +applicable restriction is the +R registered-only mode.`, + }, "ambiance": { text: `AMBIANCE diff --git a/irc/misc_test.go b/irc/misc_test.go index 09dfca02..4b7ec0e1 100644 --- a/irc/misc_test.go +++ b/irc/misc_test.go @@ -9,14 +9,14 @@ import ( ) func TestZncTimestampParser(t *testing.T) { - assertEqual(zncWireTimeToTime("1558338348.988"), time.Unix(1558338348, 988000000).UTC(), t) - assertEqual(zncWireTimeToTime("1558338348.9"), time.Unix(1558338348, 900000000).UTC(), t) - assertEqual(zncWireTimeToTime("1558338348"), time.Unix(1558338348, 0).UTC(), t) - assertEqual(zncWireTimeToTime("1558338348.99999999999999999999999999999"), time.Unix(1558338348, 999999999).UTC(), t) - assertEqual(zncWireTimeToTime("1558338348.999999999111111111"), time.Unix(1558338348, 999999999).UTC(), t) - assertEqual(zncWireTimeToTime("1558338348.999999991111111111"), time.Unix(1558338348, 999999991).UTC(), t) - assertEqual(zncWireTimeToTime(".988"), time.Unix(0, 988000000).UTC(), t) - assertEqual(zncWireTimeToTime("0"), time.Unix(0, 0).UTC(), t) - assertEqual(zncWireTimeToTime("garbage"), time.Unix(0, 0).UTC(), t) - assertEqual(zncWireTimeToTime(""), time.Unix(0, 0).UTC(), t) + assertEqual(zncWireTimeToTime("1558338348.988"), time.Unix(1558338348, 988000000).UTC()) + assertEqual(zncWireTimeToTime("1558338348.9"), time.Unix(1558338348, 900000000).UTC()) + assertEqual(zncWireTimeToTime("1558338348"), time.Unix(1558338348, 0).UTC()) + assertEqual(zncWireTimeToTime("1558338348.99999999999999999999999999999"), time.Unix(1558338348, 999999999).UTC()) + assertEqual(zncWireTimeToTime("1558338348.999999999111111111"), time.Unix(1558338348, 999999999).UTC()) + assertEqual(zncWireTimeToTime("1558338348.999999991111111111"), time.Unix(1558338348, 999999991).UTC()) + assertEqual(zncWireTimeToTime(".988"), time.Unix(0, 988000000).UTC()) + assertEqual(zncWireTimeToTime("0"), time.Unix(0, 0).UTC()) + assertEqual(zncWireTimeToTime("garbage"), time.Unix(0, 0).UTC()) + assertEqual(zncWireTimeToTime(""), time.Unix(0, 0).UTC()) } diff --git a/irc/modes_test.go b/irc/modes_test.go index 94eca03c..4c3579d8 100644 --- a/irc/modes_test.go +++ b/irc/modes_test.go @@ -4,6 +4,7 @@ package irc import ( + "fmt" "reflect" "testing" @@ -74,21 +75,21 @@ func TestUmodeGreaterThan(t *testing.T) { } } -func assertEqual(supplied, expected interface{}, t *testing.T) { - if !reflect.DeepEqual(supplied, expected) { - t.Errorf("expected %v but got %v", expected, supplied) +func assertEqual(found, expected interface{}) { + if !reflect.DeepEqual(found, expected) { + panic(fmt.Sprintf("found %#v, expected %#v", found, expected)) } } func TestChannelUserModeHasPrivsOver(t *testing.T) { - assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Halfop), false, t) - assertEqual(channelUserModeHasPrivsOver(modes.Mode(0), modes.Halfop), false, t) - assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Mode(0)), false, t) - assertEqual(channelUserModeHasPrivsOver(modes.ChannelAdmin, modes.ChannelAdmin), false, t) - assertEqual(channelUserModeHasPrivsOver(modes.Halfop, modes.Halfop), false, t) - assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Voice), false, t) + assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Halfop), false) + assertEqual(channelUserModeHasPrivsOver(modes.Mode(0), modes.Halfop), false) + assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Mode(0)), false) + assertEqual(channelUserModeHasPrivsOver(modes.ChannelAdmin, modes.ChannelAdmin), false) + assertEqual(channelUserModeHasPrivsOver(modes.Halfop, modes.Halfop), false) + assertEqual(channelUserModeHasPrivsOver(modes.Voice, modes.Voice), false) - assertEqual(channelUserModeHasPrivsOver(modes.Halfop, modes.Voice), true, t) - assertEqual(channelUserModeHasPrivsOver(modes.ChannelFounder, modes.ChannelAdmin), true, t) - assertEqual(channelUserModeHasPrivsOver(modes.ChannelOperator, modes.ChannelOperator), true, t) + assertEqual(channelUserModeHasPrivsOver(modes.Halfop, modes.Voice), true) + assertEqual(channelUserModeHasPrivsOver(modes.ChannelFounder, modes.ChannelAdmin), true) + assertEqual(channelUserModeHasPrivsOver(modes.ChannelOperator, modes.ChannelOperator), true) } diff --git a/irc/server.go b/irc/server.go index 2097a393..7d8b96d6 100644 --- a/irc/server.go +++ b/irc/server.go @@ -61,6 +61,7 @@ var ( // Server is the main Oragono server. type Server struct { + accepts AcceptManager accounts AccountManager channels ChannelManager channelRegistry ChannelRegistry @@ -104,6 +105,7 @@ func NewServer(config *Config, logger *logger.Manager) (*Server, error) { defcon: 5, } + server.accepts.Initialize() server.clients.Initialize() server.semaphores.Initialize() server.whoWas.Initialize(config.Limits.WhowasEntries)