From a2ac4eeef9451d5f405a19d56a242478afd2c309 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 9 Oct 2017 01:47:04 -0400 Subject: [PATCH] refactor limits and throttling --- irc/client.go | 2 -- irc/connection_limits.go | 46 +++++++++++++++++------- irc/connection_throttling.go | 69 ++++++++++++++++++++++++++---------- irc/server.go | 52 ++++++++------------------- 4 files changed, 100 insertions(+), 69 deletions(-) diff --git a/irc/client.go b/irc/client.go index 241c8a51..340c298c 100644 --- a/irc/client.go +++ b/irc/client.go @@ -542,9 +542,7 @@ func (client *Client) destroy() { ipaddr := client.IP() // this check shouldn't be required but eh if ipaddr != nil { - client.server.connectionLimitsMutex.Lock() client.server.connectionLimits.RemoveClient(ipaddr) - client.server.connectionLimitsMutex.Unlock() } // alert monitors diff --git a/irc/connection_limits.go b/irc/connection_limits.go index cc1ba34f..207c51f9 100644 --- a/irc/connection_limits.go +++ b/irc/connection_limits.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net" + "sync" ) var ( @@ -15,6 +16,8 @@ var ( // ConnectionLimits manages the automated client connection limits. type ConnectionLimits struct { + sync.Mutex + enabled bool ipv4Mask net.IPMask ipv6Mask net.IPMask @@ -45,6 +48,9 @@ func (cl *ConnectionLimits) maskAddr(addr net.IP) net.IP { // AddClient adds a client to our population if possible. If we can't, throws an error instead. // 'force' is used to add already-existing clients (i.e. ones that are already on the network). func (cl *ConnectionLimits) AddClient(addr net.IP, force bool) error { + cl.Lock() + defer cl.Unlock() + if !cl.enabled { return nil } @@ -75,6 +81,9 @@ func (cl *ConnectionLimits) AddClient(addr net.IP, force bool) error { // RemoveClient removes the given address from our population func (cl *ConnectionLimits) RemoveClient(addr net.IP) { + cl.Lock() + defer cl.Unlock() + if !cl.enabled { return } @@ -89,34 +98,47 @@ func (cl *ConnectionLimits) RemoveClient(addr net.IP) { } // NewConnectionLimits returns a new connection limit handler. -func NewConnectionLimits(config ConnectionLimitsConfig) (*ConnectionLimits, error) { +// The handler is functional, but disabled; it can be enabled via `ApplyConfig`. +func NewConnectionLimits() *ConnectionLimits { var cl ConnectionLimits - cl.enabled = config.Enabled + // initialize empty population; all other state is configurable cl.population = make(map[string]int) - cl.exemptedIPs = make(map[string]bool) - cl.ipv4Mask = net.CIDRMask(config.CidrLenIPv4, 32) - cl.ipv6Mask = net.CIDRMask(config.CidrLenIPv6, 128) - // subnetLimit is explicitly NOT capped at a minimum of one. - // this is so that CL config can be used to allow ONLY clients from exempted IPs/nets - cl.subnetLimit = config.IPsPerCidr + return &cl +} +// ApplyConfig atomically applies a config update to a connection limit handler +func (cl *ConnectionLimits) ApplyConfig(config ConnectionLimitsConfig) error { // assemble exempted nets + exemptedIPs := make(map[string]bool) + var exemptedNets []net.IPNet for _, cidr := range config.Exempted { ipaddr := net.ParseIP(cidr) _, netaddr, err := net.ParseCIDR(cidr) if ipaddr == nil && err != nil { - return nil, fmt.Errorf("Could not parse exempted IP/network [%s]", cidr) + return fmt.Errorf("Could not parse exempted IP/network [%s]", cidr) } if ipaddr != nil { - cl.exemptedIPs[ipaddr.String()] = true + exemptedIPs[ipaddr.String()] = true } else { - cl.exemptedNets = append(cl.exemptedNets, *netaddr) + exemptedNets = append(exemptedNets, *netaddr) } } - return &cl, nil + cl.Lock() + defer cl.Unlock() + + cl.enabled = config.Enabled + cl.ipv4Mask = net.CIDRMask(config.CidrLenIPv4, 32) + cl.ipv6Mask = net.CIDRMask(config.CidrLenIPv6, 128) + // subnetLimit is explicitly NOT capped at a minimum of one. + // this is so that CL config can be used to allow ONLY clients from exempted IPs/nets + cl.subnetLimit = config.IPsPerCidr + cl.exemptedIPs = exemptedIPs + cl.exemptedNets = exemptedNets + + return nil } diff --git a/irc/connection_throttling.go b/irc/connection_throttling.go index c5d7cb84..f2737481 100644 --- a/irc/connection_throttling.go +++ b/irc/connection_throttling.go @@ -6,6 +6,7 @@ package irc import ( "fmt" "net" + "sync" "time" ) @@ -17,6 +18,8 @@ type ThrottleDetails struct { // ConnectionThrottle manages automated client connection throttling. type ConnectionThrottle struct { + sync.RWMutex + enabled bool ipv4Mask net.IPMask ipv6Mask net.IPMask @@ -25,9 +28,8 @@ type ConnectionThrottle struct { population map[string]ThrottleDetails // used by the server to ban clients that go over this limit - BanDuration time.Duration - BanMessage string - BanMessageBytes []byte + banDuration time.Duration + banMessage string // exemptedIPs holds IPs that are exempt from limits exemptedIPs map[string]bool @@ -50,6 +52,9 @@ func (ct *ConnectionThrottle) maskAddr(addr net.IP) net.IP { // ResetFor removes any existing count for the given address. func (ct *ConnectionThrottle) ResetFor(addr net.IP) { + ct.Lock() + defer ct.Unlock() + if !ct.enabled { return } @@ -62,6 +67,9 @@ func (ct *ConnectionThrottle) ResetFor(addr net.IP) { // AddClient introduces a new client connection if possible. If we can't, throws an error instead. func (ct *ConnectionThrottle) AddClient(addr net.IP) error { + ct.Lock() + defer ct.Unlock() + if !ct.enabled { return nil } @@ -97,38 +105,63 @@ func (ct *ConnectionThrottle) AddClient(addr net.IP) error { return nil } +func (ct *ConnectionThrottle) BanDuration() time.Duration { + ct.RLock() + defer ct.RUnlock() + + return ct.banDuration +} + +func (ct *ConnectionThrottle) BanMessage() string { + ct.RLock() + defer ct.RUnlock() + + return ct.banMessage +} + // NewConnectionThrottle returns a new client connection throttler. -func NewConnectionThrottle(config ConnectionThrottleConfig) (*ConnectionThrottle, error) { +// The throttler is functional, but disabled; it can be enabled via `ApplyConfig`. +func NewConnectionThrottle() *ConnectionThrottle { var ct ConnectionThrottle - ct.enabled = config.Enabled + // initialize empty population; all other state is configurable ct.population = make(map[string]ThrottleDetails) - ct.exemptedIPs = make(map[string]bool) - ct.ipv4Mask = net.CIDRMask(config.CidrLenIPv4, 32) - ct.ipv6Mask = net.CIDRMask(config.CidrLenIPv6, 128) - ct.subnetLimit = config.ConnectionsPerCidr - - ct.duration = config.Duration - - ct.BanDuration = config.BanDuration - ct.BanMessage = config.BanMessage + return &ct +} +// ApplyConfig atomically applies a config update to a throttler +func (ct *ConnectionThrottle) ApplyConfig(config ConnectionThrottleConfig) error { // assemble exempted nets + exemptedIPs := make(map[string]bool) + var exemptedNets []net.IPNet for _, cidr := range config.Exempted { ipaddr := net.ParseIP(cidr) _, netaddr, err := net.ParseCIDR(cidr) if ipaddr == nil && err != nil { - return nil, fmt.Errorf("Could not parse exempted IP/network [%s]", cidr) + return fmt.Errorf("Could not parse exempted IP/network [%s]", cidr) } if ipaddr != nil { - ct.exemptedIPs[ipaddr.String()] = true + exemptedIPs[ipaddr.String()] = true } else { - ct.exemptedNets = append(ct.exemptedNets, *netaddr) + exemptedNets = append(exemptedNets, *netaddr) } } - return &ct, nil + ct.Lock() + defer ct.Unlock() + + ct.enabled = config.Enabled + ct.ipv4Mask = net.CIDRMask(config.CidrLenIPv4, 32) + ct.ipv6Mask = net.CIDRMask(config.CidrLenIPv6, 128) + ct.subnetLimit = config.ConnectionsPerCidr + ct.duration = config.Duration + ct.banDuration = config.BanDuration + ct.banMessage = config.BanMessage + ct.exemptedIPs = exemptedIPs + ct.exemptedNets = exemptedNets + + return nil } diff --git a/irc/server.go b/irc/server.go index e3f3c704..5062ce79 100644 --- a/irc/server.go +++ b/irc/server.go @@ -87,9 +87,7 @@ type Server struct { configFilename string configurableStateMutex sync.RWMutex // generic protection for server state modified by rehash() connectionLimits *ConnectionLimits - connectionLimitsMutex sync.Mutex // used when affecting the connection limiter, to make sure rehashing doesn't make things go out-of-whack connectionThrottle *ConnectionThrottle - connectionThrottleMutex sync.Mutex // used when affecting the connection limiter, to make sure rehashing doesn't make things go out-of-whack ctime time.Time defaultChannelModes Modes dlines *DLineManager @@ -149,6 +147,8 @@ func NewServer(config *Config, logger *logger.Manager) (*Server, error) { channels: *NewChannelNameMap(), clients: NewClientLookupSet(), commands: make(chan Command), + connectionLimits: connection_limiting.NewConnectionLimits(), + connectionThrottle: connection_limiting.NewConnectionThrottle(), listeners: make(map[string]*ListenerWrapper), logger: logger, monitorManager: NewMonitorManager(), @@ -301,32 +301,29 @@ func (server *Server) checkBans(ipaddr net.IP) (banned bool, message string) { } // check connection limits - server.connectionLimitsMutex.Lock() err := server.connectionLimits.AddClient(ipaddr, false) - server.connectionLimitsMutex.Unlock() if err != nil { // too many connections from one client, tell the client and close the connection return true, "Too many clients from your network" } // check connection throttle - server.connectionThrottleMutex.Lock() err = server.connectionThrottle.AddClient(ipaddr) - server.connectionThrottleMutex.Unlock() if err != nil { // too many connections too quickly from client, tell them and close the connection + duration := server.connectionThrottle.BanDuration() length := &IPRestrictTime{ - Duration: server.connectionThrottle.BanDuration, - Expires: time.Now().Add(server.connectionThrottle.BanDuration), + Duration: duration, + Expires: time.Now().Add(duration), } - server.dlines.AddIP(ipaddr, length, server.connectionThrottle.BanMessage, "Exceeded automated connection throttle") + server.dlines.AddIP(ipaddr, length, server.connectionThrottle.BanMessage(), "Exceeded automated connection throttle") // they're DLINE'd for 15 minutes or whatever, so we can reset the connection throttle now, // and once their temporary DLINE is finished they can fill up the throttler again server.connectionThrottle.ResetFor(ipaddr) // this might not show up properly on some clients, but our objective here is just to close it out before it has a load impact on us - return true, server.connectionThrottle.BanMessage + return true, server.connectionThrottle.BanMessage() } return false, "" @@ -1229,18 +1226,6 @@ func (server *Server) applyConfig(config *Config, initial bool) error { return fmt.Errorf("Server name isn't valid [%s]: %s", config.Server.Name, err.Error()) } - // confirm connectionLimits are fine - connectionLimits, err := NewConnectionLimits(config.Server.ConnectionLimits) - if err != nil { - return fmt.Errorf("Error rehashing config file connection-limits: %s", err.Error()) - } - - // confirm connectionThrottler is fine - connectionThrottle, err := NewConnectionThrottle(config.Server.ConnectionThrottle) - if err != nil { - return fmt.Errorf("Error rehashing config file connection-throttle: %s", err.Error()) - } - // confirm operator stuff all exists and is fine operclasses, err := config.OperatorClasses() if err != nil { @@ -1272,22 +1257,15 @@ func (server *Server) applyConfig(config *Config, initial bool) error { // apply new PROXY command restrictions server.proxyAllowedFrom = config.Server.ProxyAllowedFrom - // apply new connectionlimits - server.connectionLimitsMutex.Lock() - server.connectionLimits = connectionLimits - server.connectionThrottleMutex.Lock() - server.connectionThrottle = connectionThrottle - - server.clients.ByNickMutex.RLock() - for _, client := range server.clients.ByNick { - ipaddr := client.IP() - if ipaddr != nil { - server.connectionLimits.AddClient(ipaddr, true) - } + err = server.connectionLimits.ApplyConfig(config.Server.ConnectionLimits) + if err != nil { + return err + } + + err = server.connectionThrottle.ApplyConfig(config.Server.ConnectionThrottle) + if err != nil { + return err } - server.clients.ByNickMutex.RUnlock() - server.connectionThrottleMutex.Unlock() - server.connectionLimitsMutex.Unlock() // setup new and removed caps addedCaps := caps.NewSet()