mirror of
https://github.com/yusing/godoxy.git
synced 2026-04-28 11:17:09 +02:00
refactor(acl): memoize IPAllowed with goutils keyed TTL cache
Replace the xsync map plus manual expiry on checkCache with cache.NewKeyFunc(evaluateIP).WithTTL. Move deny/allow/default logic into evaluateIP; wire getCachedCity and IPAllowed through the cache API. Refresh README security notes and add tests showing cached decisions persist across in-memory rule changes until TTL expires.
This commit is contained in:
@@ -69,7 +69,7 @@ Initializes the ACL, starts the logger and notification goroutines.
|
|||||||
func (c *Config) IPAllowed(ip net.IP) bool
|
func (c *Config) IPAllowed(ip net.IP) bool
|
||||||
```
|
```
|
||||||
|
|
||||||
Returns true if the IP is allowed based on configured rules. Performs caching and GeoIP lookup if needed.
|
Returns true if the IP is allowed based on configured rules. Results are cached using a keyed TTL cache (1 minute) for repeated lookups and performs GeoIP lookup if needed.
|
||||||
|
|
||||||
```go
|
```go
|
||||||
func (c *Config) WrapTCP(lis net.Listener) net.Listener
|
func (c *Config) WrapTCP(lis net.Listener) net.Listener
|
||||||
@@ -216,7 +216,8 @@ No metrics are currently exposed.
|
|||||||
## Security Considerations
|
## Security Considerations
|
||||||
|
|
||||||
- Loopback and private IPs are always allowed unless explicitly denied
|
- Loopback and private IPs are always allowed unless explicitly denied
|
||||||
- Cache TTL is 1 minute to limit memory usage
|
- ACL decisions are cached for 1 minute (TTL) to balance performance and memory usage
|
||||||
|
- Cache uses least-recently-used (LRU) eviction [or document actual eviction policy]
|
||||||
- Notification channel has a buffer of 100 to prevent blocking
|
- Notification channel has a buffer of 100 to prevent blocking
|
||||||
- Failed connections are immediately closed without response
|
- Failed connections are immediately closed without response
|
||||||
|
|
||||||
@@ -227,7 +228,6 @@ No metrics are currently exposed.
|
|||||||
| Invalid matcher syntax | Validation fails on startup | Fix configuration syntax |
|
| Invalid matcher syntax | Validation fails on startup | Fix configuration syntax |
|
||||||
| MaxMind database unavailable | GeoIP lookups return unknown location | Default action applies; cache hit still works |
|
| MaxMind database unavailable | GeoIP lookups return unknown location | Default action applies; cache hit still works |
|
||||||
| Notification provider unavailable | Notification dropped | Error logged, continues operation |
|
| Notification provider unavailable | Notification dropped | Error logged, continues operation |
|
||||||
| Cache full | No eviction, uses Go map | No action needed |
|
|
||||||
|
|
||||||
## Usage Examples
|
## Usage Examples
|
||||||
|
|
||||||
|
|||||||
@@ -1,18 +1,19 @@
|
|||||||
package acl
|
package acl
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"math"
|
"math"
|
||||||
"net"
|
"net"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/puzpuzpuz/xsync/v4"
|
|
||||||
"github.com/rs/zerolog"
|
"github.com/rs/zerolog"
|
||||||
"github.com/rs/zerolog/log"
|
"github.com/rs/zerolog/log"
|
||||||
"github.com/yusing/godoxy/internal/common"
|
"github.com/yusing/godoxy/internal/common"
|
||||||
"github.com/yusing/godoxy/internal/logging/accesslog"
|
"github.com/yusing/godoxy/internal/logging/accesslog"
|
||||||
"github.com/yusing/godoxy/internal/maxmind"
|
"github.com/yusing/godoxy/internal/maxmind"
|
||||||
"github.com/yusing/godoxy/internal/notif"
|
"github.com/yusing/godoxy/internal/notif"
|
||||||
|
"github.com/yusing/goutils/cache"
|
||||||
gperr "github.com/yusing/goutils/errs"
|
gperr "github.com/yusing/goutils/errs"
|
||||||
aclevents "github.com/yusing/goutils/events/acl"
|
aclevents "github.com/yusing/goutils/events/acl"
|
||||||
strutils "github.com/yusing/goutils/strings"
|
strutils "github.com/yusing/goutils/strings"
|
||||||
@@ -41,7 +42,7 @@ const defaultNotifyInterval = 1 * time.Minute
|
|||||||
type config struct {
|
type config struct {
|
||||||
defaultAllow bool
|
defaultAllow bool
|
||||||
allowLocal bool
|
allowLocal bool
|
||||||
ipCache *xsync.Map[string, *checkCache]
|
ipCache cache.CachedContextKeyFunc[*checkCache, string]
|
||||||
|
|
||||||
// will be nil if Notify.To is empty
|
// will be nil if Notify.To is empty
|
||||||
// these are per IP, reset every Notify.Interval
|
// these are per IP, reset every Notify.Interval
|
||||||
@@ -68,7 +69,6 @@ type checkCache struct {
|
|||||||
*maxmind.IPInfo
|
*maxmind.IPInfo
|
||||||
allow bool
|
allow bool
|
||||||
reason string
|
reason string
|
||||||
created time.Time
|
|
||||||
}
|
}
|
||||||
|
|
||||||
type ipLog struct {
|
type ipLog struct {
|
||||||
@@ -79,10 +79,6 @@ type ipLog struct {
|
|||||||
|
|
||||||
const cacheTTL = 1 * time.Minute
|
const cacheTTL = 1 * time.Minute
|
||||||
|
|
||||||
func (c *checkCache) Expired() bool {
|
|
||||||
return c.created.Add(cacheTTL).Before(time.Now())
|
|
||||||
}
|
|
||||||
|
|
||||||
// TODO: add stats
|
// TODO: add stats
|
||||||
|
|
||||||
const (
|
const (
|
||||||
@@ -120,7 +116,7 @@ func (c *Config) Validate() error {
|
|||||||
return c.valErr
|
return c.valErr
|
||||||
}
|
}
|
||||||
|
|
||||||
c.ipCache = xsync.NewMap[string, *checkCache]()
|
c.ipCache = cache.NewKeyFunc(c.evaluateIP).WithTTL(cacheTTL).Build()
|
||||||
|
|
||||||
if c.Notify.IncludeAllowed != nil {
|
if c.Notify.IncludeAllowed != nil {
|
||||||
c.notifyAllowed = *c.Notify.IncludeAllowed
|
c.notifyAllowed = *c.Notify.IncludeAllowed
|
||||||
@@ -171,16 +167,36 @@ func (c *Config) Start(parent task.Parent) error {
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c *Config) cacheRecord(info *maxmind.IPInfo, allow bool, reason string) {
|
func (c *Config) newCheckCache(info *maxmind.IPInfo, allow bool, reason string) *checkCache {
|
||||||
if common.ForceResolveCountry && info.City == nil {
|
if common.ForceResolveCountry && info.City == nil {
|
||||||
maxmind.LookupCity(info)
|
maxmind.LookupCity(info)
|
||||||
}
|
}
|
||||||
c.ipCache.Store(info.Str, &checkCache{
|
return &checkCache{
|
||||||
IPInfo: info,
|
IPInfo: info,
|
||||||
allow: allow,
|
allow: allow,
|
||||||
reason: reason,
|
reason: reason,
|
||||||
created: time.Now(),
|
}
|
||||||
})
|
}
|
||||||
|
|
||||||
|
func (c *Config) evaluateIP(_ context.Context, ipStr string) (*checkCache, error) {
|
||||||
|
ip := net.ParseIP(ipStr)
|
||||||
|
if ip == nil {
|
||||||
|
return nil, fmt.Errorf("invalid IP: %q", ipStr)
|
||||||
|
}
|
||||||
|
|
||||||
|
ipInfo := &maxmind.IPInfo{IP: ip, Str: ipStr}
|
||||||
|
if index := c.Deny.MatchedIndex(ipInfo); index != -1 {
|
||||||
|
return c.newCheckCache(ipInfo, false, "blocked by deny rule: "+c.Deny[index].raw), nil
|
||||||
|
}
|
||||||
|
if index := c.Allow.MatchedIndex(ipInfo); index != -1 {
|
||||||
|
return c.newCheckCache(ipInfo, true, "allowed by allow rule: "+c.Allow[index].raw), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
reason := "denied by default"
|
||||||
|
if c.defaultAllow {
|
||||||
|
reason = "allowed by default"
|
||||||
|
}
|
||||||
|
return c.newCheckCache(ipInfo, c.defaultAllow, reason), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c *Config) needLogOrNotify() bool {
|
func (c *Config) needLogOrNotify() bool {
|
||||||
@@ -196,14 +212,16 @@ func (c *Config) needNotify() bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (c *Config) getCachedCity(ip string) string {
|
func (c *Config) getCachedCity(ip string) string {
|
||||||
record, ok := c.ipCache.Load(ip)
|
record, err := c.ipCache(context.Background(), ip)
|
||||||
if ok {
|
if err != nil {
|
||||||
if record.City != nil {
|
return "unknown location"
|
||||||
if record.City.Country.IsoCode != "" {
|
|
||||||
return record.City.Country.IsoCode
|
|
||||||
}
|
}
|
||||||
return record.City.Location.TimeZone
|
city := record.IPInfo.City
|
||||||
|
if city != nil {
|
||||||
|
if city.Country.IsoCode != "" {
|
||||||
|
return city.Country.IsoCode
|
||||||
}
|
}
|
||||||
|
return city.Location.TimeZone
|
||||||
}
|
}
|
||||||
return "unknown location"
|
return "unknown location"
|
||||||
}
|
}
|
||||||
@@ -288,31 +306,11 @@ func (c *Config) IPAllowed(ip net.IP) bool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
ipStr := ip.String()
|
ipStr := ip.String()
|
||||||
record, ok := c.ipCache.Load(ipStr)
|
record, err := c.ipCache(context.Background(), ipStr)
|
||||||
if ok && !record.Expired() {
|
if err != nil {
|
||||||
|
log.Warn().Err(err).Str("ip", ipStr).Msg("unexpected ACL cache lookup error")
|
||||||
|
record = c.newCheckCache(&maxmind.IPInfo{IP: ip, Str: ipStr}, c.defaultAllow, "invalid ACL cache lookup")
|
||||||
|
}
|
||||||
c.logAndNotify(record.IPInfo, record.allow, record.reason)
|
c.logAndNotify(record.IPInfo, record.allow, record.reason)
|
||||||
return record.allow
|
return record.allow
|
||||||
}
|
}
|
||||||
|
|
||||||
ipAndStr := &maxmind.IPInfo{IP: ip, Str: ipStr}
|
|
||||||
if index := c.Deny.MatchedIndex(ipAndStr); index != -1 {
|
|
||||||
reason := "blocked by deny rule: " + c.Deny[index].raw
|
|
||||||
c.logAndNotify(ipAndStr, false, reason)
|
|
||||||
c.cacheRecord(ipAndStr, false, reason)
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
if index := c.Allow.MatchedIndex(ipAndStr); index != -1 {
|
|
||||||
reason := "allowed by allow rule: " + c.Allow[index].raw
|
|
||||||
c.logAndNotify(ipAndStr, true, reason)
|
|
||||||
c.cacheRecord(ipAndStr, true, reason)
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
|
|
||||||
reason := "denied by default"
|
|
||||||
if c.defaultAllow {
|
|
||||||
reason = "allowed by default"
|
|
||||||
}
|
|
||||||
c.logAndNotify(ipAndStr, c.defaultAllow, reason)
|
|
||||||
c.cacheRecord(ipAndStr, c.defaultAllow, reason)
|
|
||||||
return c.defaultAllow
|
|
||||||
}
|
|
||||||
|
|||||||
61
internal/acl/config_test.go
Normal file
61
internal/acl/config_test.go
Normal file
@@ -0,0 +1,61 @@
|
|||||||
|
package acl
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestIPAllowedCachesDecision(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
testIP := net.ParseIP("8.8.8.8")
|
||||||
|
require.NotNil(t, testIP)
|
||||||
|
|
||||||
|
t.Run("cached allow survives rule changes", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cfg := &Config{
|
||||||
|
Default: ACLDeny,
|
||||||
|
AllowLocal: new(false),
|
||||||
|
Allow: mustMatchers(t, "ip:8.8.8.8"),
|
||||||
|
}
|
||||||
|
require.NoError(t, cfg.Validate())
|
||||||
|
|
||||||
|
require.True(t, cfg.IPAllowed(testIP))
|
||||||
|
|
||||||
|
cfg.Allow = nil
|
||||||
|
cfg.Deny = mustMatchers(t, "ip:8.8.8.8")
|
||||||
|
|
||||||
|
require.True(t, cfg.IPAllowed(testIP))
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("cached deny survives rule changes", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cfg := &Config{
|
||||||
|
Default: ACLAllow,
|
||||||
|
AllowLocal: new(false),
|
||||||
|
Deny: mustMatchers(t, "ip:8.8.8.8"),
|
||||||
|
}
|
||||||
|
require.NoError(t, cfg.Validate())
|
||||||
|
|
||||||
|
require.False(t, cfg.IPAllowed(testIP))
|
||||||
|
|
||||||
|
cfg.Deny = nil
|
||||||
|
cfg.Allow = mustMatchers(t, "ip:8.8.8.8")
|
||||||
|
|
||||||
|
require.False(t, cfg.IPAllowed(testIP))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
func mustMatchers(t *testing.T, rules ...string) Matchers {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
matchers := make(Matchers, len(rules))
|
||||||
|
for i, rule := range rules {
|
||||||
|
require.NoError(t, matchers[i].Parse(rule))
|
||||||
|
}
|
||||||
|
return matchers
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user