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

@@ -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)
+}

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)
+	}
+}

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 == "" {

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,

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)
 }

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>
+// 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 [<mechanism>|<data>|*]
 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"))

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 <target>
+
+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 <target> <text to be sent>
 

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())
 }

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)
 }

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)