From 84e3b5d77b907a562e1fa6fcb4d7992cdda6369a Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Tue, 8 Dec 2020 22:01:23 -0500 Subject: [PATCH] stop autocreating d-lines for throttle violations This didn't work correctly for IPv6 or custom nets. /UNDLINE IP can temporarily be used to reset the throttle. --- default.yaml | 3 -- irc/client.go | 3 +- irc/connection_limits/limiter.go | 58 ++++++++++++------------- irc/connection_limits/limiter_test.go | 22 ++++------ irc/connection_limits/throttler_test.go | 19 ++++---- irc/dline.go | 3 +- irc/flatip/adhoc.go | 33 ++++++++++++++ irc/flatip/flatip.go | 47 +++++++------------- irc/gateways.go | 3 +- irc/handlers.go | 6 +++ irc/server.go | 25 ++++------- traditional.yaml | 3 -- 12 files changed, 113 insertions(+), 112 deletions(-) create mode 100644 irc/flatip/adhoc.go diff --git a/default.yaml b/default.yaml index 4a394bcb..1ab18be0 100644 --- a/default.yaml +++ b/default.yaml @@ -247,9 +247,6 @@ server: window: 10m # 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 - throttle-ban-duration: 10m # how wide the CIDR should be for IPv4 (a /32 is a fully specified IPv4 address) cidr-len-ipv4: 32 diff --git a/irc/client.go b/irc/client.go index 38b7287e..68bc680b 100644 --- a/irc/client.go +++ b/irc/client.go @@ -21,6 +21,7 @@ import ( ident "github.com/oragono/go-ident" "github.com/oragono/oragono/irc/caps" "github.com/oragono/oragono/irc/connection_limits" + "github.com/oragono/oragono/irc/flatip" "github.com/oragono/oragono/irc/history" "github.com/oragono/oragono/irc/modes" "github.com/oragono/oragono/irc/sno" @@ -1477,7 +1478,7 @@ func (client *Client) destroy(session *Session) { if session.proxiedIP != nil { ip = session.proxiedIP } - client.server.connectionLimiter.RemoveClient(ip) + client.server.connectionLimiter.RemoveClient(flatip.FromNetIP(ip)) source = ip.String() } client.server.logger.Info("connect-ip", fmt.Sprintf("disconnecting session of %s from %s", details.nick, source)) diff --git a/irc/connection_limits/limiter.go b/irc/connection_limits/limiter.go index 377ec69d..867cb0f8 100644 --- a/irc/connection_limits/limiter.go +++ b/irc/connection_limits/limiter.go @@ -7,7 +7,6 @@ import ( "crypto/md5" "errors" "fmt" - "net" "sync" "time" @@ -48,8 +47,7 @@ type rawLimiterConfig struct { Throttle bool Window time.Duration - MaxPerWindow int `yaml:"max-connections-per-window"` - BanDuration time.Duration `yaml:"throttle-ban-duration"` + MaxPerWindow int `yaml:"max-connections-per-window"` CidrLenIPv4 int `yaml:"cidr-len-ipv4"` CidrLenIPv6 int `yaml:"cidr-len-ipv6"` @@ -126,44 +124,49 @@ type Limiter struct { // addrToKey canonicalizes `addr` to a string key, and returns // the relevant connection limit and throttle max-per-window values -func (cl *Limiter) addrToKey(flat flatip.IP) (key limiterKey, limit int, throttle int) { +func (cl *Limiter) addrToKey(addr flatip.IP) (key limiterKey, limit int, throttle int) { for _, custom := range cl.config.customLimits { for _, net := range custom.nets { - if net.Contains(flat) { + if net.Contains(addr) { return limiterKey{maskedIP: custom.name, prefixLen: 0}, custom.maxConcurrent, custom.maxPerWindow } } } var prefixLen int - if flat.IsIPv4() { + if addr.IsIPv4() { prefixLen = cl.config.CidrLenIPv4 - flat = flat.Mask(prefixLen, 32) + addr = addr.Mask(prefixLen, 32) prefixLen += 96 } else { prefixLen = cl.config.CidrLenIPv6 - flat = flat.Mask(prefixLen, 128) + addr = addr.Mask(prefixLen, 128) } - return limiterKey{maskedIP: flat, prefixLen: uint8(prefixLen)}, cl.config.MaxConcurrent, cl.config.MaxPerWindow + return limiterKey{maskedIP: addr, prefixLen: uint8(prefixLen)}, cl.config.MaxConcurrent, cl.config.MaxPerWindow } // AddClient adds a client to our population if possible. If we can't, throws an error instead. -func (cl *Limiter) AddClient(addr net.IP) error { - flat := flatip.FromNetIP(addr) - +func (cl *Limiter) AddClient(addr flatip.IP) error { cl.Lock() defer cl.Unlock() // we don't track populations for exempted addresses or nets - this is by design - if flatip.IPInNets(flat, cl.config.exemptedNets) { + if flatip.IPInNets(addr, cl.config.exemptedNets) { return nil } - addrString, maxConcurrent, maxPerWindow := cl.addrToKey(flat) + addrString, maxConcurrent, maxPerWindow := cl.addrToKey(addr) + + // check limiter + var count int + if cl.config.Count { + count = cl.limiter[addrString] + 1 + if count > maxConcurrent { + return ErrLimitExceeded + } + } - // 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 @@ -175,16 +178,13 @@ func (cl *Limiter) AddClient(addr net.IP) error { throttled, _ := g.Touch() // actually check the limit cl.throttler[addrString] = g.ThrottleDetails // store modified mutable state if throttled { + // back out the limiter add return ErrThrottleExceeded } } - // now check limiter + // success, record in limiter if cl.config.Count { - count := cl.limiter[addrString] + 1 - if count > maxConcurrent { - return ErrLimitExceeded - } cl.limiter[addrString] = count } @@ -192,17 +192,15 @@ func (cl *Limiter) AddClient(addr net.IP) error { } // RemoveClient removes the given address from our population -func (cl *Limiter) RemoveClient(addr net.IP) { - flat := flatip.FromNetIP(addr) - +func (cl *Limiter) RemoveClient(addr flatip.IP) { cl.Lock() defer cl.Unlock() - if !cl.config.Count || flatip.IPInNets(flat, cl.config.exemptedNets) { + if !cl.config.Count || flatip.IPInNets(addr, cl.config.exemptedNets) { return } - addrString, _, _ := cl.addrToKey(flat) + addrString, _, _ := cl.addrToKey(addr) count := cl.limiter[addrString] count -= 1 if count < 0 { @@ -212,17 +210,15 @@ func (cl *Limiter) RemoveClient(addr net.IP) { } // ResetThrottle resets the throttle count for an IP -func (cl *Limiter) ResetThrottle(addr net.IP) { - flat := flatip.FromNetIP(addr) - +func (cl *Limiter) ResetThrottle(addr flatip.IP) { cl.Lock() defer cl.Unlock() - if !cl.config.Throttle || flatip.IPInNets(flat, cl.config.exemptedNets) { + if !cl.config.Throttle || flatip.IPInNets(addr, cl.config.exemptedNets) { return } - addrString, _, _ := cl.addrToKey(flat) + addrString, _, _ := cl.addrToKey(addr) delete(cl.throttler, addrString) } diff --git a/irc/connection_limits/limiter_test.go b/irc/connection_limits/limiter_test.go index 819da408..3bc0b39e 100644 --- a/irc/connection_limits/limiter_test.go +++ b/irc/connection_limits/limiter_test.go @@ -5,26 +5,20 @@ package connection_limits import ( "crypto/md5" - "net" "testing" "time" "github.com/oragono/oragono/irc/flatip" ) -func easyParseIP(ipstr string) (result net.IP) { - result = net.ParseIP(ipstr) - if result == nil { - panic(ipstr) +func easyParseIP(ipstr string) (result flatip.IP) { + result, err := flatip.ParseIP(ipstr) + if err != nil { + panic(err) } return } -func easyParseFlat(ipstr string) (result flatip.IP) { - r1 := easyParseIP(ipstr) - return flatip.FromNetIP(r1) -} - var baseConfig = LimiterConfig{ rawLimiterConfig: rawLimiterConfig{ Count: true, @@ -56,20 +50,20 @@ func TestKeying(t *testing.T) { limiter.ApplyConfig(&config) // an ipv4 /32 looks like a /128 to us after applying the 4-in-6 mapping - key, maxConc, maxWin := limiter.addrToKey(easyParseFlat("1.1.1.1")) + key, maxConc, maxWin := limiter.addrToKey(easyParseIP("1.1.1.1")) assertEqual(key.prefixLen, uint8(128), t) assertEqual(key.maskedIP[12:], []byte{1, 1, 1, 1}, t) assertEqual(maxConc, 4, t) assertEqual(maxWin, 8, t) - testIPv6 := easyParseFlat("2607:5301:201:3100::7426") + testIPv6 := easyParseIP("2607:5301:201:3100::7426") key, maxConc, maxWin = limiter.addrToKey(testIPv6) assertEqual(key.prefixLen, uint8(64), t) - assertEqual(key.maskedIP[:], []byte(easyParseIP("2607:5301:201:3100::")), t) + assertEqual(flatip.IP(key.maskedIP), easyParseIP("2607:5301:201:3100::"), t) assertEqual(maxConc, 4, t) assertEqual(maxWin, 8, t) - key, maxConc, maxWin = limiter.addrToKey(easyParseFlat("8.8.4.4")) + key, maxConc, maxWin = limiter.addrToKey(easyParseIP("8.8.4.4")) assertEqual(key.prefixLen, uint8(0), t) assertEqual([16]byte(key.maskedIP), md5.Sum([]byte("google")), t) assertEqual(maxConc, 128, t) diff --git a/irc/connection_limits/throttler_test.go b/irc/connection_limits/throttler_test.go index 093c02bf..d44ec860 100644 --- a/irc/connection_limits/throttler_test.go +++ b/irc/connection_limits/throttler_test.go @@ -4,7 +4,6 @@ package connection_limits import ( - "net" "reflect" "testing" "time" @@ -83,7 +82,7 @@ func makeTestThrottler(v4len, v6len int) *Limiter { func TestConnectionThrottle(t *testing.T) { throttler := makeTestThrottler(32, 64) - addr := net.ParseIP("8.8.8.8") + addr := easyParseIP("8.8.8.8") for i := 0; i < 3; i += 1 { err := throttler.AddClient(addr) @@ -97,14 +96,14 @@ func TestConnectionThrottleIPv6(t *testing.T) { throttler := makeTestThrottler(32, 64) var err error - err = throttler.AddClient(net.ParseIP("2001:0db8::1")) + err = throttler.AddClient(easyParseIP("2001:0db8::1")) assertEqual(err, nil, t) - err = throttler.AddClient(net.ParseIP("2001:0db8::2")) + err = throttler.AddClient(easyParseIP("2001:0db8::2")) assertEqual(err, nil, t) - err = throttler.AddClient(net.ParseIP("2001:0db8::3")) + err = throttler.AddClient(easyParseIP("2001:0db8::3")) assertEqual(err, nil, t) - err = throttler.AddClient(net.ParseIP("2001:0db8::4")) + err = throttler.AddClient(easyParseIP("2001:0db8::4")) assertEqual(err, ErrThrottleExceeded, t) } @@ -112,13 +111,13 @@ func TestConnectionThrottleIPv4(t *testing.T) { throttler := makeTestThrottler(24, 64) var err error - err = throttler.AddClient(net.ParseIP("192.168.1.101")) + err = throttler.AddClient(easyParseIP("192.168.1.101")) assertEqual(err, nil, t) - err = throttler.AddClient(net.ParseIP("192.168.1.102")) + err = throttler.AddClient(easyParseIP("192.168.1.102")) assertEqual(err, nil, t) - err = throttler.AddClient(net.ParseIP("192.168.1.103")) + err = throttler.AddClient(easyParseIP("192.168.1.103")) assertEqual(err, nil, t) - err = throttler.AddClient(net.ParseIP("192.168.1.104")) + err = throttler.AddClient(easyParseIP("192.168.1.104")) assertEqual(err, ErrThrottleExceeded, t) } diff --git a/irc/dline.go b/irc/dline.go index 3d70e903..3de5a621 100644 --- a/irc/dline.go +++ b/irc/dline.go @@ -226,8 +226,7 @@ func (dm *DLineManager) RemoveIP(addr net.IP) error { } // CheckIP returns whether or not an IP address was banned, and how long it is banned for. -func (dm *DLineManager) CheckIP(netAddr net.IP) (isBanned bool, info IPBanInfo) { - addr := flatip.FromNetIP(netAddr) +func (dm *DLineManager) CheckIP(addr flatip.IP) (isBanned bool, info IPBanInfo) { if addr.IsLoopback() { return // #671 } diff --git a/irc/flatip/adhoc.go b/irc/flatip/adhoc.go new file mode 100644 index 00000000..6c994c56 --- /dev/null +++ b/irc/flatip/adhoc.go @@ -0,0 +1,33 @@ +// Copyright 2020 Shivaram Lingamneni +// Released under the MIT license + +package flatip + +// begin ad-hoc utilities + +// ParseToNormalizedNet attempts to interpret a string either as an IP +// network in CIDR notation, returning an IPNet, or as an IP address, +// returning an IPNet that contains only that address. +func ParseToNormalizedNet(netstr string) (ipnet IPNet, err error) { + _, ipnet, err = ParseCIDR(netstr) + if err == nil { + return + } + ip, err := ParseIP(netstr) + if err == nil { + ipnet.IP = ip + ipnet.PrefixLen = 128 + } + return +} + +// IPInNets is a convenience function for testing whether an IP is contained +// in any member of a slice of IPNet's. +func IPInNets(addr IP, nets []IPNet) bool { + for _, net := range nets { + if net.Contains(addr) { + return true + } + } + return false +} diff --git a/irc/flatip/flatip.go b/irc/flatip/flatip.go index 39bf4314..7ebdbb50 100644 --- a/irc/flatip/flatip.go +++ b/irc/flatip/flatip.go @@ -1,5 +1,6 @@ // Copyright 2020 Shivaram Lingamneni // Copyright 2009 The Go Authors +// Released under the MIT license package flatip @@ -13,6 +14,8 @@ var ( v4InV6Prefix = []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff} IPv6loopback = IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1} + IPv6zero = IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} + IPv4zero = IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0, 0, 0, 0} ErrInvalidIPString = errors.New("String could not be interpreted as an IP address") ) @@ -20,8 +23,8 @@ var ( // packed versions of net.IP and net.IPNet; these are pure value types, // so they can be compared with == and used as map keys. -// IP is the 128-bit representation of the IPv6 address, using the 4-in-6 mapping -// if necessary: +// IP is a 128-bit representation of an IP address, using the 4-in-6 mapping +// to represent IPv4 addresses. type IP [16]byte // IPNet is a IP network. In a valid value, all bits after PrefixLen are zeroes. @@ -95,6 +98,10 @@ func (ip IP) IsLoopback() bool { } } +func (ip IP) IsUnspecified() bool { + return ip == IPv4zero || ip == IPv6zero +} + func rawCidrMask(length int) (m IP) { n := uint(length) for i := 0; i < 16; i++ { @@ -176,6 +183,13 @@ func (cidr IPNet) String() string { return ipnet.String() } +// IsZero tests whether ipnet is the zero value of an IPNet, 0::0/0. +// Although this is a valid subnet, it can still be used as a sentinel +// value in some contexts. +func (ipnet IPNet) IsZero() bool { + return ipnet == IPNet{} +} + // ParseCIDR parses a string representation of an IP network in CIDR notation, // then returns it as an IPNet (along with the original, unmasked address). func ParseCIDR(netstr string) (ip IP, ipnet IPNet, err error) { @@ -186,32 +200,3 @@ func ParseCIDR(netstr string) (ip IP, ipnet IPNet, err error) { } return FromNetIP(nip), FromNetIPNet(*nipnet), nil } - -// begin ad-hoc utilities - -// ParseToNormalizedNet attempts to interpret a string either as an IP -// network in CIDR notation, returning an IPNet, or as an IP address, -// returning an IPNet that contains only that address. -func ParseToNormalizedNet(netstr string) (ipnet IPNet, err error) { - _, ipnet, err = ParseCIDR(netstr) - if err == nil { - return - } - ip, err := ParseIP(netstr) - if err == nil { - ipnet.IP = ip - ipnet.PrefixLen = 128 - } - return -} - -// IPInNets is a convenience function for testing whether an IP is contained -// in any member of a slice of IPNet's. -func IPInNets(addr IP, nets []IPNet) bool { - for _, net := range nets { - if net.Contains(addr) { - return true - } - } - return false -} diff --git a/irc/gateways.go b/irc/gateways.go index 388687bc..e7dbbecf 100644 --- a/irc/gateways.go +++ b/irc/gateways.go @@ -9,6 +9,7 @@ import ( "errors" "net" + "github.com/oragono/oragono/irc/flatip" "github.com/oragono/oragono/irc/modes" "github.com/oragono/oragono/irc/utils" ) @@ -87,7 +88,7 @@ func (client *Client) ApplyProxiedIP(session *Session, proxiedIP net.IP, tls boo } // successfully added a limiter entry for the proxied IP; // remove the entry for the real IP if applicable (#197) - client.server.connectionLimiter.RemoveClient(session.realIP) + client.server.connectionLimiter.RemoveClient(flatip.FromNetIP(session.realIP)) // given IP is sane! override the client's current IP client.server.logger.Info("connect-ip", "Accepted proxy IP for client", proxiedIP.String()) diff --git a/irc/handlers.go b/irc/handlers.go index 8bf96e35..a03d2368 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -24,6 +24,7 @@ import ( "github.com/goshuirc/irc-go/ircmsg" "github.com/oragono/oragono/irc/caps" "github.com/oragono/oragono/irc/custime" + "github.com/oragono/oragono/irc/flatip" "github.com/oragono/oragono/irc/history" "github.com/oragono/oragono/irc/jwt" "github.com/oragono/oragono/irc/modes" @@ -2798,6 +2799,11 @@ func unDLineHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *R // get host hostString := msg.Params[0] + // TODO(#1447) consolidate this into the "unban" command + if flatip, ipErr := flatip.ParseIP(hostString); ipErr == nil { + server.connectionLimiter.ResetThrottle(flatip) + } + // check host hostNet, err := utils.NormalizedNetFromString(hostString) diff --git a/irc/server.go b/irc/server.go index 1f11c122..397ba1d1 100644 --- a/irc/server.go +++ b/irc/server.go @@ -23,6 +23,7 @@ import ( "github.com/oragono/oragono/irc/caps" "github.com/oragono/oragono/irc/connection_limits" + "github.com/oragono/oragono/irc/flatip" "github.com/oragono/oragono/irc/history" "github.com/oragono/oragono/irc/logger" "github.com/oragono/oragono/irc/modes" @@ -160,31 +161,23 @@ func (server *Server) checkBans(config *Config, ipaddr net.IP, checkScripts bool } } + flat := flatip.FromNetIP(ipaddr) + // check DLINEs - isBanned, info := server.dlines.CheckIP(ipaddr) + isBanned, info := server.dlines.CheckIP(flat) if isBanned { - server.logger.Info("connect-ip", fmt.Sprintf("Client from %v rejected by d-line", ipaddr)) + server.logger.Info("connect-ip", "Client rejected by d-line", ipaddr.String()) return true, false, info.BanMessage("You are banned from this server (%s)") } // check connection limits - err := server.connectionLimiter.AddClient(ipaddr) + err := server.connectionLimiter.AddClient(flat) if err == connection_limits.ErrLimitExceeded { // too many connections from one client, tell the client and close the connection - server.logger.Info("connect-ip", fmt.Sprintf("Client from %v rejected for connection limit", ipaddr)) + server.logger.Info("connect-ip", "Client rejected for connection limit", ipaddr.String()) return true, false, "Too many clients from your network" } else if err == connection_limits.ErrThrottleExceeded { - duration := config.Server.IPLimits.BanDuration - if duration != 0 { - 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.connectionLimiter.ResetThrottle(ipaddr) - } - server.logger.Info( - "connect-ip", - fmt.Sprintf("Client from %v exceeded connection throttle, d-lining for %v", ipaddr, duration)) + server.logger.Info("connect-ip", "Client exceeded connection throttle", ipaddr.String()) return true, false, throttleMessage } else if err != nil { server.logger.Warning("internal", "unexpected ban result", err.Error()) @@ -211,7 +204,7 @@ func (server *Server) checkBans(config *Config, ipaddr net.IP, checkScripts bool } if output.Result == IPBanned { // XXX roll back IP connection/throttling addition for the IP - server.connectionLimiter.RemoveClient(ipaddr) + server.connectionLimiter.RemoveClient(flat) server.logger.Info("connect-ip", "Rejected client due to ip-check-script", ipaddr.String()) return true, false, output.BanMessage } else if output.Result == IPRequireSASL { diff --git a/traditional.yaml b/traditional.yaml index 227ed980..5f7c79aa 100644 --- a/traditional.yaml +++ b/traditional.yaml @@ -220,9 +220,6 @@ server: window: 10m # 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 - throttle-ban-duration: 10m # how wide the CIDR should be for IPv4 (a /32 is a fully specified IPv4 address) cidr-len-ipv4: 32