From 4050b6571a04afbf6a6b83b4e123e64363e18dc4 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 18 Nov 2019 01:42:48 -0500 Subject: [PATCH 1/5] fix #646 Includes a partially backwards-incompatible config change --- irc/config.go | 21 +-- irc/connection_limits/limiter.go | 220 +++++++++++++++++------- irc/connection_limits/limiter_test.go | 87 ++++++++++ irc/connection_limits/throttler.go | 127 -------------- irc/connection_limits/throttler_test.go | 30 ++-- irc/connection_limits/tor.go | 6 - irc/server.go | 89 +++++----- oragono.yaml | 71 ++++---- 8 files changed, 332 insertions(+), 319 deletions(-) create mode 100644 irc/connection_limits/limiter_test.go diff --git a/irc/config.go b/irc/config.go index 42be0a1d..2d4fcb65 100644 --- a/irc/config.go +++ b/irc/config.go @@ -308,12 +308,11 @@ type Config struct { forceTrailing bool SendUnprefixedSasl bool `yaml:"send-unprefixed-sasl"` } - isupport isupport.List - ConnectionLimiter connection_limits.LimiterConfig `yaml:"connection-limits"` - ConnectionThrottler connection_limits.ThrottlerConfig `yaml:"connection-throttling"` - Cloaks cloaks.CloakConfig `yaml:"ip-cloaking"` - supportedCaps *caps.Set - capValues caps.Values + isupport isupport.List + IPLimits connection_limits.LimiterConfig `yaml:"ip-limits"` + Cloaks cloaks.CloakConfig `yaml:"ip-cloaking"` + supportedCaps *caps.Set + capValues caps.Values } Languages struct { @@ -633,16 +632,6 @@ func LoadConfig(filename string) (config *Config, err error) { // set this even if STS is disabled config.Server.capValues[caps.STS] = config.Server.STS.Value() - if config.Server.ConnectionThrottler.Enabled { - config.Server.ConnectionThrottler.Duration, err = time.ParseDuration(config.Server.ConnectionThrottler.DurationString) - if err != nil { - return nil, fmt.Errorf("Could not parse connection-throttle duration: %s", err.Error()) - } - config.Server.ConnectionThrottler.BanDuration, err = time.ParseDuration(config.Server.ConnectionThrottler.BanDurationString) - if err != nil { - return nil, fmt.Errorf("Could not parse connection-throttle ban-duration: %s", err.Error()) - } - } // process webirc blocks var newWebIRC []webircConfig for _, webirc := range config.Server.WebIRC { diff --git a/irc/connection_limits/limiter.go b/irc/connection_limits/limiter.go index f9f912cd..c8aea7b0 100644 --- a/irc/connection_limits/limiter.go +++ b/irc/connection_limits/limiter.go @@ -8,69 +8,161 @@ import ( "fmt" "net" "sync" + "time" "github.com/oragono/oragono/irc/utils" ) -// LimiterConfig controls the automated connection limits. -type LimiterConfig struct { - Enabled bool - CidrLenIPv4 int `yaml:"cidr-len-ipv4"` - CidrLenIPv6 int `yaml:"cidr-len-ipv6"` - ConnsPerSubnet int `yaml:"connections-per-subnet"` - IPsPerSubnet int `yaml:"ips-per-subnet"` // legacy name for ConnsPerSubnet - Exempted []string +var ( + ErrLimitExceeded = errors.New("too many concurrent connections") + ErrThrottleExceeded = errors.New("too many recent connection attempts") +) + +type CustomLimitConfig struct { + MaxConcurrent int `yaml:"max-concurrent-connections"` + MaxPerWindow int `yaml:"max-connections-per-window"` } -var ( - errTooManyClients = errors.New("Too many clients in subnet") -) +// tuples the key-value pair of a CIDR and its custom limit/throttle values +type customLimit struct { + CustomLimitConfig + ipNet net.IPNet +} + +// LimiterConfig controls the automated connection limits. +// RawLimiterConfig contains all the YAML-visible fields; +// LimiterConfig contains additional denormalized private fields +type RawLimiterConfig struct { + Limit bool + MaxConcurrent int `yaml:"max-concurrent-connections"` + + Throttle bool + Window time.Duration + MaxPerWindow int `yaml:"max-connections-per-window"` + BanDuration time.Duration `yaml:"throttle-ban-duration"` + + CidrLenIPv4 int `yaml:"cidr-len-ipv4"` + CidrLenIPv6 int `yaml:"cidr-len-ipv6"` + + Exempted []string + + CustomLimits map[string]CustomLimitConfig `yaml:"custom-limits"` +} + +type LimiterConfig struct { + RawLimiterConfig + + ipv4Mask net.IPMask + ipv6Mask net.IPMask + exemptedNets []net.IPNet + customLimits []customLimit +} + +func (config *LimiterConfig) UnmarshalYAML(unmarshal func(interface{}) error) (err error) { + if err = unmarshal(&config.RawLimiterConfig); err != nil { + return err + } + return config.postprocess() +} + +func (config *LimiterConfig) postprocess() (err error) { + config.exemptedNets, err = utils.ParseNetList(config.Exempted) + if err != nil { + return fmt.Errorf("Could not parse limiter exemption list: %v", err.Error()) + } + + for netStr, customLimitConf := range config.CustomLimits { + normalizedNet, err := utils.NormalizedNetFromString(netStr) + if err != nil { + return fmt.Errorf("Could not parse custom limit specification: %v", err.Error()) + } + config.customLimits = append(config.customLimits, customLimit{ + CustomLimitConfig: customLimitConf, + ipNet: normalizedNet, + }) + } + + config.ipv4Mask = net.CIDRMask(config.CidrLenIPv4, 32) + config.ipv6Mask = net.CIDRMask(config.CidrLenIPv6, 128) + + return nil +} // Limiter manages the automated client connection limits. type Limiter struct { sync.Mutex - enabled bool - ipv4Mask net.IPMask - ipv6Mask net.IPMask - // subnetLimit is the maximum number of clients per subnet - subnetLimit int - // population holds IP -> count of clients connected from there - population map[string]int + config *LimiterConfig - // exemptedNets holds networks that are exempt from limits - exemptedNets []net.IPNet + // IP/CIDR -> count of clients connected from there: + limiter map[string]int + // IP/CIDR -> throttle state: + throttler map[string]ThrottleDetails } -// addrToKey canonicalizes `addr` to a string key. -func addrToKey(addr net.IP, v4Mask net.IPMask, v6Mask net.IPMask) string { - if addr.To4() != nil { - addr = addr.Mask(v4Mask) // IP.Mask() handles the 4-in-6 mapping for us - } else { - addr = addr.Mask(v6Mask) +// addrToKey canonicalizes `addr` to a string key, and returns +// the relevant connection limit and throttle max-per-window values +func (cl *Limiter) addrToKey(addr net.IP) (key string, limit int, throttle int) { + // `key` will be a CIDR string like "8.8.8.8/32" or "2001:0db8::/32" + for _, custom := range cl.config.customLimits { + if custom.ipNet.Contains(addr) { + return custom.ipNet.String(), custom.MaxConcurrent, custom.MaxPerWindow + } } - return addr.String() + + var ipNet net.IPNet + addrv4 := addr.To4() + if addrv4 != nil { + ipNet = net.IPNet{ + IP: addrv4.Mask(cl.config.ipv4Mask), + Mask: cl.config.ipv4Mask, + } + } else { + ipNet = net.IPNet{ + IP: addr.Mask(cl.config.ipv6Mask), + Mask: cl.config.ipv6Mask, + } + } + return ipNet.String(), cl.config.MaxConcurrent, cl.config.MaxPerWindow } // 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 *Limiter) AddClient(addr net.IP, force bool) error { +func (cl *Limiter) AddClient(addr net.IP) error { cl.Lock() defer cl.Unlock() // we don't track populations for exempted addresses or nets - this is by design - if !cl.enabled || utils.IPInNets(addr, cl.exemptedNets) { + if utils.IPInNets(addr, cl.config.exemptedNets) { return nil } - // check population - addrString := addrToKey(addr, cl.ipv4Mask, cl.ipv6Mask) + addrString, maxConcurrent, maxPerWindow := cl.addrToKey(addr) - if cl.population[addrString]+1 > cl.subnetLimit && !force { - return errTooManyClients + // XXX check throttle first; if we checked limit first and then checked throttle, + // we'd have to decrement the limit on an unsuccessful throttle check + if cl.config.Throttle { + details := cl.throttler[addrString] // retrieve mutable throttle state from the map + // add in constant state to process the limiting operation + g := GenericThrottle{ + ThrottleDetails: details, + Duration: cl.config.Window, + Limit: maxPerWindow, + } + throttled, _ := g.Touch() // actually check the limit + cl.throttler[addrString] = g.ThrottleDetails // store modified mutable state + if throttled { + return ErrThrottleExceeded + } } - cl.population[addrString] = cl.population[addrString] + 1 + // now check limiter + if cl.config.Limit { + count := cl.limiter[addrString] + 1 + if count > maxConcurrent { + return ErrLimitExceeded + } + cl.limiter[addrString] = count + } return nil } @@ -80,45 +172,43 @@ func (cl *Limiter) RemoveClient(addr net.IP) { cl.Lock() defer cl.Unlock() - if !cl.enabled || utils.IPInNets(addr, cl.exemptedNets) { + if !cl.config.Limit || utils.IPInNets(addr, cl.config.exemptedNets) { return } - addrString := addrToKey(addr, cl.ipv4Mask, cl.ipv6Mask) - cl.population[addrString] = cl.population[addrString] - 1 - - // safety limiter - if cl.population[addrString] < 0 { - cl.population[addrString] = 0 + addrString, _, _ := cl.addrToKey(addr) + count := cl.limiter[addrString] + count -= 1 + if count < 0 { + count = 0 } + cl.limiter[addrString] = count } -// ApplyConfig atomically applies a config update to a connection limit handler -func (cl *Limiter) ApplyConfig(config LimiterConfig) error { - // assemble exempted nets - exemptedNets, err := utils.ParseNetList(config.Exempted) - if err != nil { - return fmt.Errorf("Could not parse limiter exemption list: %v", err.Error()) - } - +// ResetThrottle resets the throttle count for an IP +func (cl *Limiter) ResetThrottle(addr net.IP) { cl.Lock() defer cl.Unlock() - if cl.population == nil { - cl.population = make(map[string]int) + if !cl.config.Throttle || utils.IPInNets(addr, cl.config.exemptedNets) { + return } - 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.ConnsPerSubnet - // but: check if the current key was left unset, but the legacy was set: - if cl.subnetLimit == 0 && config.IPsPerSubnet != 0 { - cl.subnetLimit = config.IPsPerSubnet - } - cl.exemptedNets = exemptedNets - - return nil + addrString, _, _ := cl.addrToKey(addr) + delete(cl.throttler, addrString) +} + +// ApplyConfig atomically applies a config update to a connection limit handler +func (cl *Limiter) ApplyConfig(config *LimiterConfig) { + cl.Lock() + defer cl.Unlock() + + if cl.limiter == nil { + cl.limiter = make(map[string]int) + } + if cl.throttler == nil { + cl.throttler = make(map[string]ThrottleDetails) + } + + cl.config = config } diff --git a/irc/connection_limits/limiter_test.go b/irc/connection_limits/limiter_test.go new file mode 100644 index 00000000..dfe9139f --- /dev/null +++ b/irc/connection_limits/limiter_test.go @@ -0,0 +1,87 @@ +// Copyright (c) 2018 Shivaram Lingamneni +// released under the MIT license + +package connection_limits + +import ( + "net" + "testing" + "time" +) + +func easyParseIP(ipstr string) (result net.IP) { + result = net.ParseIP(ipstr) + if result == nil { + panic(ipstr) + } + return +} + +var baseConfig = LimiterConfig{ + RawLimiterConfig: RawLimiterConfig{ + Limit: true, + MaxConcurrent: 4, + + Throttle: true, + Window: time.Second * 600, + MaxPerWindow: 8, + + CidrLenIPv4: 32, + CidrLenIPv6: 64, + + Exempted: []string{"localhost"}, + + CustomLimits: map[string]CustomLimitConfig{ + "8.8.0.0/16": { + MaxConcurrent: 128, + MaxPerWindow: 256, + }, + }, + }, +} + +func TestKeying(t *testing.T) { + config := baseConfig + config.postprocess() + var limiter Limiter + limiter.ApplyConfig(&config) + + key, maxConc, maxWin := limiter.addrToKey(easyParseIP("1.1.1.1")) + assertEqual(key, "1.1.1.1/32", t) + assertEqual(maxConc, 4, t) + assertEqual(maxWin, 8, t) + + key, maxConc, maxWin = limiter.addrToKey(easyParseIP("2607:5301:201:3100::7426")) + assertEqual(key, "2607:5301:201:3100::/64", t) + assertEqual(maxConc, 4, t) + assertEqual(maxWin, 8, t) + + key, maxConc, maxWin = limiter.addrToKey(easyParseIP("8.8.4.4")) + assertEqual(key, "8.8.0.0/16", t) + assertEqual(maxConc, 128, t) + assertEqual(maxWin, 256, t) +} + +func TestLimits(t *testing.T) { + regularIP := easyParseIP("2607:5301:201:3100::7426") + config := baseConfig + config.postprocess() + var limiter Limiter + limiter.ApplyConfig(&config) + + for i := 0; i < 4; i++ { + err := limiter.AddClient(regularIP) + if err != nil { + t.Errorf("ip should not be blocked, but %v", err) + } + } + err := limiter.AddClient(regularIP) + if err != ErrLimitExceeded { + t.Errorf("ip should be blocked, but %v", err) + } + limiter.RemoveClient(regularIP) + err = limiter.AddClient(regularIP) + if err != nil { + t.Errorf("ip should not be blocked, but %v", err) + } +} diff --git a/irc/connection_limits/throttler.go b/irc/connection_limits/throttler.go index 99a785ba..53fe3749 100644 --- a/irc/connection_limits/throttler.go +++ b/irc/connection_limits/throttler.go @@ -4,28 +4,9 @@ package connection_limits import ( - "fmt" - "net" - "sync" "time" - - "github.com/oragono/oragono/irc/utils" ) -// ThrottlerConfig controls the automated connection throttling. -type ThrottlerConfig struct { - Enabled bool - CidrLenIPv4 int `yaml:"cidr-len-ipv4"` - CidrLenIPv6 int `yaml:"cidr-len-ipv6"` - ConnectionsPerCidr int `yaml:"max-connections"` - DurationString string `yaml:"duration"` - Duration time.Duration `yaml:"duration-time"` - BanDurationString string `yaml:"ban-duration"` - BanDuration time.Duration - BanMessage string `yaml:"ban-message"` - Exempted []string -} - // ThrottleDetails holds the connection-throttling details for a subnet/IP. type ThrottleDetails struct { Start time.Time @@ -68,111 +49,3 @@ func (g *GenericThrottle) touch(now time.Time) (throttled bool, remainingTime ti return false, 0 } } - -// Throttler manages automated client connection throttling. -type Throttler struct { - sync.RWMutex - - enabled bool - ipv4Mask net.IPMask - ipv6Mask net.IPMask - subnetLimit int - duration time.Duration - population map[string]ThrottleDetails - - // used by the server to ban clients that go over this limit - banDuration time.Duration - banMessage string - - // exemptedNets holds networks that are exempt from limits - exemptedNets []net.IPNet -} - -// ResetFor removes any existing count for the given address. -func (ct *Throttler) ResetFor(addr net.IP) { - ct.Lock() - defer ct.Unlock() - - if !ct.enabled { - return - } - - // remove - addrString := addrToKey(addr, ct.ipv4Mask, ct.ipv6Mask) - delete(ct.population, addrString) -} - -// AddClient introduces a new client connection if possible. If we can't, throws an error instead. -func (ct *Throttler) AddClient(addr net.IP) error { - ct.Lock() - defer ct.Unlock() - - if !ct.enabled { - return nil - } - - // check exempted lists - if utils.IPInNets(addr, ct.exemptedNets) { - return nil - } - - // check throttle - addrString := addrToKey(addr, ct.ipv4Mask, ct.ipv6Mask) - - details := ct.population[addrString] // retrieve mutable throttle state from the map - // add in constant state to process the limiting operation - g := GenericThrottle{ - ThrottleDetails: details, - Duration: ct.duration, - Limit: ct.subnetLimit, - } - throttled, _ := g.Touch() // actually check the limit - ct.population[addrString] = g.ThrottleDetails // store modified mutable state - - if throttled { - return errTooManyClients - } else { - return nil - } -} - -func (ct *Throttler) BanDuration() time.Duration { - ct.RLock() - defer ct.RUnlock() - - return ct.banDuration -} - -func (ct *Throttler) BanMessage() string { - ct.RLock() - defer ct.RUnlock() - - return ct.banMessage -} - -// ApplyConfig atomically applies a config update to a throttler -func (ct *Throttler) ApplyConfig(config ThrottlerConfig) error { - // assemble exempted nets - exemptedNets, err := utils.ParseNetList(config.Exempted) - if err != nil { - return fmt.Errorf("Could not parse throttle exemption list: %v", err.Error()) - } - - ct.Lock() - defer ct.Unlock() - - if ct.population == nil { - ct.population = make(map[string]ThrottleDetails) - } - - 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.exemptedNets = exemptedNets - - return nil -} diff --git a/irc/connection_limits/throttler_test.go b/irc/connection_limits/throttler_test.go index 08a79f89..e8fc8687 100644 --- a/irc/connection_limits/throttler_test.go +++ b/irc/connection_limits/throttler_test.go @@ -62,19 +62,23 @@ func TestGenericThrottleDisabled(t *testing.T) { } } -func makeTestThrottler(v4len, v6len int) *Throttler { +func makeTestThrottler(v4len, v6len int) *Limiter { minute, _ := time.ParseDuration("1m") maxConnections := 3 - config := ThrottlerConfig{ - Enabled: true, - CidrLenIPv4: v4len, - CidrLenIPv6: v6len, - ConnectionsPerCidr: maxConnections, - Duration: minute, + config := LimiterConfig{ + RawLimiterConfig: RawLimiterConfig{ + Limit: false, + Throttle: true, + CidrLenIPv4: v4len, + CidrLenIPv6: v6len, + MaxPerWindow: maxConnections, + Window: minute, + }, } - var throttler Throttler - throttler.ApplyConfig(config) - return &throttler + config.postprocess() + var limiter Limiter + limiter.ApplyConfig(&config) + return &limiter } func TestConnectionThrottle(t *testing.T) { @@ -86,7 +90,7 @@ func TestConnectionThrottle(t *testing.T) { assertEqual(err, nil, t) } err := throttler.AddClient(addr) - assertEqual(err, errTooManyClients, t) + assertEqual(err, ErrThrottleExceeded, t) } func TestConnectionThrottleIPv6(t *testing.T) { @@ -101,7 +105,7 @@ func TestConnectionThrottleIPv6(t *testing.T) { assertEqual(err, nil, t) err = throttler.AddClient(net.ParseIP("2001:0db8::4")) - assertEqual(err, errTooManyClients, t) + assertEqual(err, ErrThrottleExceeded, t) } func TestConnectionThrottleIPv4(t *testing.T) { @@ -116,5 +120,5 @@ func TestConnectionThrottleIPv4(t *testing.T) { assertEqual(err, nil, t) err = throttler.AddClient(net.ParseIP("192.168.1.104")) - assertEqual(err, errTooManyClients, t) + assertEqual(err, ErrThrottleExceeded, t) } diff --git a/irc/connection_limits/tor.go b/irc/connection_limits/tor.go index 87081465..643f79cf 100644 --- a/irc/connection_limits/tor.go +++ b/irc/connection_limits/tor.go @@ -4,16 +4,10 @@ package connection_limits import ( - "errors" "sync" "time" ) -var ( - ErrLimitExceeded = errors.New("too many concurrent connections") - ErrThrottleExceeded = errors.New("too many recent connection attempts") -) - // TorLimiter is a combined limiter and throttler for use on connections // proxied from a Tor hidden service (so we don't have meaningful IPs, // a notion of CIDR width, etc.) diff --git a/irc/server.go b/irc/server.go index 31b8b24e..516df040 100644 --- a/irc/server.go +++ b/irc/server.go @@ -46,6 +46,8 @@ var ( // we only have standard channels for now. TODO: any updates to this // will also need to be reflected in CasefoldChannel chanTypes = "#" + + throttleMessage = "You have attempted to connect too many times within a short duration. Wait a while, and you will be able to connect." ) // ListenerWrapper wraps a listener so it can be safely reconfigured or stopped @@ -59,34 +61,33 @@ type ListenerWrapper struct { // Server is the main Oragono server. type Server struct { - accounts AccountManager - channels ChannelManager - channelRegistry ChannelRegistry - clients ClientManager - config unsafe.Pointer - configFilename string - connectionLimiter connection_limits.Limiter - connectionThrottler connection_limits.Throttler - ctime time.Time - dlines *DLineManager - helpIndexManager HelpIndexManager - klines *KLineManager - listeners map[string]*ListenerWrapper - logger *logger.Manager - monitorManager MonitorManager - name string - nameCasefolded string - rehashMutex sync.Mutex // tier 4 - rehashSignal chan os.Signal - pprofServer *http.Server - resumeManager ResumeManager - signals chan os.Signal - snomasks SnoManager - store *buntdb.DB - torLimiter connection_limits.TorLimiter - whoWas WhoWasList - stats Stats - semaphores ServerSemaphores + accounts AccountManager + channels ChannelManager + channelRegistry ChannelRegistry + clients ClientManager + config unsafe.Pointer + configFilename string + connectionLimiter connection_limits.Limiter + ctime time.Time + dlines *DLineManager + helpIndexManager HelpIndexManager + klines *KLineManager + listeners map[string]*ListenerWrapper + logger *logger.Manager + monitorManager MonitorManager + name string + nameCasefolded string + rehashMutex sync.Mutex // tier 4 + rehashSignal chan os.Signal + pprofServer *http.Server + resumeManager ResumeManager + signals chan os.Signal + snomasks SnoManager + store *buntdb.DB + torLimiter connection_limits.TorLimiter + whoWas WhoWasList + stats Stats + semaphores ServerSemaphores } var ( @@ -214,32 +215,28 @@ func (server *Server) checkBans(ipaddr net.IP) (banned bool, message string) { } // check connection limits - err := server.connectionLimiter.AddClient(ipaddr, false) - if err != nil { + err := server.connectionLimiter.AddClient(ipaddr) + if err == connection_limits.ErrLimitExceeded { // too many connections from one client, tell the client and close the connection server.logger.Info("localconnect-ip", fmt.Sprintf("Client from %v rejected for connection limit", ipaddr)) return true, "Too many clients from your network" - } - - // check connection throttle - err = server.connectionThrottler.AddClient(ipaddr) - if err != nil { - // too many connections too quickly from client, tell them and close the connection - duration := server.connectionThrottler.BanDuration() + } else if err == connection_limits.ErrThrottleExceeded { + duration := server.Config().Server.IPLimits.BanDuration if duration == 0 { return false, "" } - server.dlines.AddIP(ipaddr, duration, server.connectionThrottler.BanMessage(), "Exceeded automated connection throttle", "auto.connection.throttler") - + server.dlines.AddIP(ipaddr, duration, throttleMessage, "Exceeded automated connection throttle", "auto.connection.throttler") // 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.connectionThrottler.ResetFor(ipaddr) + server.connectionLimiter.ResetThrottle(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 server.logger.Info( "localconnect-ip", fmt.Sprintf("Client from %v exceeded connection throttle, d-lining for %v", ipaddr, duration)) - return true, server.connectionThrottler.BanMessage() + return true, throttleMessage + } else if err != nil { + server.logger.Warning("internal", "unexpected ban result", err.Error()) } return false, "" @@ -604,15 +601,7 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) { // first, reload config sections for functionality implemented in subpackages: - err = server.connectionLimiter.ApplyConfig(config.Server.ConnectionLimiter) - if err != nil { - return err - } - - err = server.connectionThrottler.ApplyConfig(config.Server.ConnectionThrottler) - if err != nil { - return err - } + server.connectionLimiter.ApplyConfig(&config.Server.IPLimits) tlConf := &config.Server.TorListeners server.torLimiter.Configure(tlConf.MaxConnections, tlConf.ThrottleDuration, tlConf.MaxConnectionsPerDuration) diff --git a/oragono.yaml b/oragono.yaml index d6716738..1523eed0 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -144,53 +144,42 @@ server: # this works around that bug, allowing them to use SASL. send-unprefixed-sasl: true - # maximum number of connections per subnet - connection-limits: - # whether to enforce connection limits or not - enabled: true + # IP-based DoS protection + ip-limits: + # whether to enforce limits on the total number of concurrent connections per IP / CIDR + limit: true + # maximum concurrent connections per subnet + max-concurrent-connections: 16 - # how wide the cidr should be for IPv4 + # whether to restrict the rate of new connections per IP / CIDR + throttle: true + # how long to keep track of connections for + window: 10m + # maximum number of connections, per subnet, within the given duration + max-connections-per-window: 32 + # how long to ban offenders for + # after banning them, the number of connections is reset (which lets you use UNDLINE to unban people) + throttle-ban-duration: 10m + + # how wide the CIDR should be for IPv4 (a /32 is a fully specified IPv4 address) cidr-len-ipv4: 32 - - # how wide the cidr should be for IPv6 + # how wide the cidr should be for IPv6 (a /64 is the typical prefix assigned + # by an ISP to an individual customer for their LAN) cidr-len-ipv6: 64 - # maximum concurrent connections per subnet (defined above by the cidr length) - connections-per-subnet: 16 - # IPs/networks which are exempted from connection limits exempted: - "localhost" # - "192.168.1.1" # - "2001:0db8::/32" - # automated connection throttling - connection-throttling: - # whether to throttle connections or not - enabled: true - - # how wide the cidr should be for IPv4 - cidr-len-ipv4: 32 - - # how wide the cidr should be for IPv6 - cidr-len-ipv6: 64 - - # how long to keep track of connections for - duration: 10m - - # maximum number of connections, per subnet, within the given duration - max-connections: 32 - - # how long to ban offenders for, and the message to use - # after banning them, the number of connections is reset (which lets you use UNDLINE to unban people) - ban-duration: 10m - ban-message: You have attempted to connect too many times within a short duration. Wait a while, and you will be able to connect. - - # IPs/networks which are exempted from connection throttling - exempted: - - "localhost" - # - "192.168.1.1" - # - "2001:0db8::/32" + # custom connection limits for certain IPs/networks. Note that CIDR + # widths defined here override the default CIDR width --- the limit + # will apply to the entire CIDR no matter how large or small it is + custom-limits: + # "8.8.0.0/16": + # max-concurrent-connections: 128 + # max-connections-per-window: 1024 # IP cloaking hides users' IP addresses from other users and from channel admins # (but not from server admins), while still allowing channel admins to ban @@ -214,13 +203,11 @@ server: # the cloaked hostname is derived only from the CIDR (most significant bits # of the IP address), up to a configurable number of bits. this is the - # granularity at which bans will take effect for ipv4 (a /32 is a fully - # specified IP address). note that changing this value will invalidate - # any stored bans. + # granularity at which bans will take effect for IPv4. Note that changing + # this value will invalidate any stored bans. cidr-len-ipv4: 32 - # analogous value for ipv6 (an ipv6 /64 is the typical prefix assigned - # by an ISP to an individual customer for their LAN) + # analogous granularity for IPv6 cidr-len-ipv6: 64 # number of bits of hash output to include in the cloaked hostname. From 42db1778ac20f7d0e3cfb0e8c6079b8785bc883c Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 18 Nov 2019 17:30:54 -0500 Subject: [PATCH 2/5] unexport rawLimiterConfig --- irc/connection_limits/limiter.go | 8 ++++---- irc/connection_limits/limiter_test.go | 2 +- irc/connection_limits/throttler_test.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/irc/connection_limits/limiter.go b/irc/connection_limits/limiter.go index c8aea7b0..646aa798 100644 --- a/irc/connection_limits/limiter.go +++ b/irc/connection_limits/limiter.go @@ -30,9 +30,9 @@ type customLimit struct { } // LimiterConfig controls the automated connection limits. -// RawLimiterConfig contains all the YAML-visible fields; +// rawLimiterConfig contains all the YAML-visible fields; // LimiterConfig contains additional denormalized private fields -type RawLimiterConfig struct { +type rawLimiterConfig struct { Limit bool MaxConcurrent int `yaml:"max-concurrent-connections"` @@ -50,7 +50,7 @@ type RawLimiterConfig struct { } type LimiterConfig struct { - RawLimiterConfig + rawLimiterConfig ipv4Mask net.IPMask ipv6Mask net.IPMask @@ -59,7 +59,7 @@ type LimiterConfig struct { } func (config *LimiterConfig) UnmarshalYAML(unmarshal func(interface{}) error) (err error) { - if err = unmarshal(&config.RawLimiterConfig); err != nil { + if err = unmarshal(&config.rawLimiterConfig); err != nil { return err } return config.postprocess() diff --git a/irc/connection_limits/limiter_test.go b/irc/connection_limits/limiter_test.go index dfe9139f..c789f2ae 100644 --- a/irc/connection_limits/limiter_test.go +++ b/irc/connection_limits/limiter_test.go @@ -18,7 +18,7 @@ func easyParseIP(ipstr string) (result net.IP) { } var baseConfig = LimiterConfig{ - RawLimiterConfig: RawLimiterConfig{ + rawLimiterConfig: rawLimiterConfig{ Limit: true, MaxConcurrent: 4, diff --git a/irc/connection_limits/throttler_test.go b/irc/connection_limits/throttler_test.go index e8fc8687..313a55f0 100644 --- a/irc/connection_limits/throttler_test.go +++ b/irc/connection_limits/throttler_test.go @@ -66,7 +66,7 @@ func makeTestThrottler(v4len, v6len int) *Limiter { minute, _ := time.ParseDuration("1m") maxConnections := 3 config := LimiterConfig{ - RawLimiterConfig: RawLimiterConfig{ + rawLimiterConfig: rawLimiterConfig{ Limit: false, Throttle: true, CidrLenIPv4: v4len, From 41497c1b5128282a9487013567dac877ae64a4f4 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sat, 23 Nov 2019 21:07:45 -0500 Subject: [PATCH 3/5] review fixes --- oragono.yaml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/oragono.yaml b/oragono.yaml index 1523eed0..1f27b060 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -146,24 +146,24 @@ server: # IP-based DoS protection ip-limits: - # whether to enforce limits on the total number of concurrent connections per IP / CIDR + # whether to enforce limits on the total number of concurrent connections per IP/CIDR limit: true - # maximum concurrent connections per subnet + # maximum concurrent connections per IP/CIDR max-concurrent-connections: 16 - # whether to restrict the rate of new connections per IP / CIDR + # whether to restrict the rate of new connections per IP/CIDR throttle: true # how long to keep track of connections for window: 10m - # maximum number of connections, per subnet, within the given duration + # maximum number of new connections per IP/CIDR within the given duration max-connections-per-window: 32 - # how long to ban offenders for - # after banning them, the number of connections is reset (which lets you use UNDLINE to unban people) + # how long to ban offenders for. after banning them, the number of connections is + # reset, which lets you use /UNDLINE to unban people throttle-ban-duration: 10m # how wide the CIDR should be for IPv4 (a /32 is a fully specified IPv4 address) cidr-len-ipv4: 32 - # how wide the cidr should be for IPv6 (a /64 is the typical prefix assigned + # how wide the CIDR should be for IPv6 (a /64 is the typical prefix assigned # by an ISP to an individual customer for their LAN) cidr-len-ipv6: 64 @@ -173,7 +173,7 @@ server: # - "192.168.1.1" # - "2001:0db8::/32" - # custom connection limits for certain IPs/networks. Note that CIDR + # custom connection limits for certain IPs/networks. note that CIDR # widths defined here override the default CIDR width --- the limit # will apply to the entire CIDR no matter how large or small it is custom-limits: From 2d456c2106652dc9c8bd9a562a954bc4e62f362a Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sat, 23 Nov 2019 21:09:31 -0500 Subject: [PATCH 4/5] review feedback: rename a key --- irc/connection_limits/limiter.go | 6 +++--- irc/connection_limits/limiter_test.go | 2 +- irc/connection_limits/throttler_test.go | 2 +- oragono.yaml | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/irc/connection_limits/limiter.go b/irc/connection_limits/limiter.go index 646aa798..208eb4cf 100644 --- a/irc/connection_limits/limiter.go +++ b/irc/connection_limits/limiter.go @@ -33,7 +33,7 @@ type customLimit struct { // rawLimiterConfig contains all the YAML-visible fields; // LimiterConfig contains additional denormalized private fields type rawLimiterConfig struct { - Limit bool + Count bool MaxConcurrent int `yaml:"max-concurrent-connections"` Throttle bool @@ -156,7 +156,7 @@ func (cl *Limiter) AddClient(addr net.IP) error { } // now check limiter - if cl.config.Limit { + if cl.config.Count { count := cl.limiter[addrString] + 1 if count > maxConcurrent { return ErrLimitExceeded @@ -172,7 +172,7 @@ func (cl *Limiter) RemoveClient(addr net.IP) { cl.Lock() defer cl.Unlock() - if !cl.config.Limit || utils.IPInNets(addr, cl.config.exemptedNets) { + if !cl.config.Count || utils.IPInNets(addr, cl.config.exemptedNets) { return } diff --git a/irc/connection_limits/limiter_test.go b/irc/connection_limits/limiter_test.go index c789f2ae..8ef74c14 100644 --- a/irc/connection_limits/limiter_test.go +++ b/irc/connection_limits/limiter_test.go @@ -19,7 +19,7 @@ func easyParseIP(ipstr string) (result net.IP) { var baseConfig = LimiterConfig{ rawLimiterConfig: rawLimiterConfig{ - Limit: true, + Count: true, MaxConcurrent: 4, Throttle: true, diff --git a/irc/connection_limits/throttler_test.go b/irc/connection_limits/throttler_test.go index 313a55f0..093c02bf 100644 --- a/irc/connection_limits/throttler_test.go +++ b/irc/connection_limits/throttler_test.go @@ -67,7 +67,7 @@ func makeTestThrottler(v4len, v6len int) *Limiter { maxConnections := 3 config := LimiterConfig{ rawLimiterConfig: rawLimiterConfig{ - Limit: false, + Count: false, Throttle: true, CidrLenIPv4: v4len, CidrLenIPv6: v6len, diff --git a/oragono.yaml b/oragono.yaml index 1f27b060..34430483 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -146,8 +146,8 @@ server: # IP-based DoS protection ip-limits: - # whether to enforce limits on the total number of concurrent connections per IP/CIDR - limit: true + # whether to limit the total count of concurrent connections per IP/CIDR + count: true # maximum concurrent connections per IP/CIDR max-concurrent-connections: 16 From 6ee083b6c567f9d1c84a2ba438380a8aca1a1f41 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sat, 23 Nov 2019 22:06:51 -0500 Subject: [PATCH 5/5] review fix --- oragono.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oragono.yaml b/oragono.yaml index 34430483..215b606e 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -146,7 +146,7 @@ server: # IP-based DoS protection ip-limits: - # whether to limit the total count of concurrent connections per IP/CIDR + # whether to limit the total number of concurrent connections per IP/CIDR count: true # maximum concurrent connections per IP/CIDR max-concurrent-connections: 16