From f80e093c5b40d6164ddd5ed282afc3b3c4724bdf Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 22 Jul 2026 12:18:03 -0500 Subject: [PATCH 1/2] fix(onlyfrom): match complete IP rules exactly Parse complete allowlist entries as IP addresses before falling back to textual prefix matching. Cover exact IPv4 and IPv6, CIDR, and explicit prefix behavior. --- README.md | 2 +- onlyfrom.go | 12 ++++++++++-- onlyfrom_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c263f6e..86c3dbc 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,7 @@ It prevents server crashes in case of panic in one of the controllers. OnlyFrom middleware allows access from a limited list of source IPs. Such IPs can be defined as complete ip (like 192.168.1.12), prefix (129.168.) or CIDR (192.168.0.0/16). +Complete IPs are matched exactly, while prefix rules use textual prefix matching. The middleware will respond with `StatusForbidden` (403) if the request comes from a different IP. It supports both IPv4 and IPv6 and checks the usual headers like `X-Forwarded-For` and `X-Real-IP` and the remote address. @@ -453,4 +454,3 @@ Profiler is a convenient sub-router used for mounting net/http/pprof, i.e. ``` It exposes a bunch of `/pprof/*` endpoints as well as `/vars`. Builtin support for `onlyIps` allows restricting access, which is important if it runs on a publicly exposed port. However, counting on IP check only is not that reliable way to limit request and for production use it would be better to add some sort of auth (for example provided `BasicAuth` middleware) or run with a separate http server, exposed to internal ip/port only. - diff --git a/onlyfrom.go b/onlyfrom.go index acc8ff2..0af2475 100644 --- a/onlyfrom.go +++ b/onlyfrom.go @@ -42,12 +42,20 @@ func matchSourceIP(r *http.Request, ips []string) (result bool, match string, er if err != nil { return false, "", fmt.Errorf("can't get realip: %w", err) // we can't get ip, so no match } - // check for ip prefix or CIDR + parsedIP := net.ParseIP(ip) + // check for CIDR, complete IP, or IP prefix for _, exclIP := range ips { if _, cidrnet, err := net.ParseCIDR(exclIP); err == nil { - if cidrnet.Contains(net.ParseIP(ip)) { + if cidrnet.Contains(parsedIP) { return true, ip, nil } + continue + } + if allowedIP := net.ParseIP(exclIP); allowedIP != nil { + if allowedIP.Equal(parsedIP) { + return true, ip, nil + } + continue } if strings.HasPrefix(ip, exclIP) { return true, ip, nil diff --git a/onlyfrom_test.go b/onlyfrom_test.go index ea85885..3ed1dab 100644 --- a/onlyfrom_test.go +++ b/onlyfrom_test.go @@ -99,6 +99,34 @@ func TestOnlyFromAllowedCIDR(t *testing.T) { assert.Equal(t, 403, resp.StatusCode) } +func TestMatchSourceIPRules(t *testing.T) { + tests := []struct { + name string + rule string + source string + matched bool + }{ + {name: "complete IPv4", rule: "1.2.3.4", source: "1.2.3.4", matched: true}, + {name: "complete IPv4 rejects textual prefix", rule: "1.2.3.4", source: "1.2.3.45", matched: false}, + {name: "IPv4 prefix", rule: "1.2.3.", source: "1.2.3.45", matched: true}, + {name: "IPv4 CIDR", rule: "1.2.3.0/24", source: "1.2.3.45", matched: true}, + {name: "complete IPv6 normalized", rule: "2001:db8:0:0::1", source: "2001:db8::1", matched: true}, + {name: "complete IPv6 rejects textual prefix", rule: "2001:db8::1", source: "2001:db8::10", matched: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", http.NoBody) + req.Header.Set("X-Real-IP", tt.source) + + matched, source, err := matchSourceIP(req, []string{tt.rule}) + require.NoError(t, err) + assert.Equal(t, tt.matched, matched) + assert.Equal(t, tt.source, source) + }) + } +} + func TestOnlyFromRejected(t *testing.T) { handler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, err := w.Write([]byte("blah blah")) From 834f3e621c6657018586e5a1aca9c1f9416c0514 Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 22 Jul 2026 12:26:01 -0500 Subject: [PATCH 2/2] fix(review-loop): iteration 1 addressed 6 findings - [minor] [README.md:107] clarify semantic IP matching rules - [minor] [onlyfrom.go:12] document exact IP, CIDR, and prefix behavior - [major] [onlyfrom.go:46] normalize the in-function comment - [minor] [onlyfrom_test.go:109] cover IPv6 prefix and CIDR behavior - [minor] [onlyfrom_test.go:102] cover matching a later allowlist rule - [major] [onlyfrom_test.go:109] normalize subtest messages --- README.md | 3 ++- onlyfrom.go | 5 +++-- onlyfrom_test.go | 24 ++++++++++++++++-------- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 86c3dbc..dd27fec 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,8 @@ It prevents server crashes in case of panic in one of the controllers. OnlyFrom middleware allows access from a limited list of source IPs. Such IPs can be defined as complete ip (like 192.168.1.12), prefix (129.168.) or CIDR (192.168.0.0/16). -Complete IPs are matched exactly, while prefix rules use textual prefix matching. +Complete IP rules use semantic address equality (so equivalent IPv6 spellings match), CIDRs use network containment, +and all other rules use literal textual prefix matching. The middleware will respond with `StatusForbidden` (403) if the request comes from a different IP. It supports both IPv4 and IPv6 and checks the usual headers like `X-Forwarded-For` and `X-Real-IP` and the remote address. diff --git a/onlyfrom.go b/onlyfrom.go index 0af2475..8b9400b 100644 --- a/onlyfrom.go +++ b/onlyfrom.go @@ -10,7 +10,8 @@ import ( ) // OnlyFrom middleware allows access for limited list of source IPs. -// Such IPs can be defined as complete ip (like 192.168.1.12), prefix (129.168.) or CIDR (192.168.0.0/16) +// Rules can be complete IPs (like 192.168.1.12), textual prefixes (129.168.), or CIDRs (192.168.0.0/16). +// Complete IPs use semantic address equality, CIDRs use network containment, and all other rules use prefix matching. func OnlyFrom(onlyIps ...string) func(http.Handler) http.Handler { return func(h http.Handler) http.Handler { fn := func(w http.ResponseWriter, r *http.Request) { @@ -43,7 +44,7 @@ func matchSourceIP(r *http.Request, ips []string) (result bool, match string, er return false, "", fmt.Errorf("can't get realip: %w", err) // we can't get ip, so no match } parsedIP := net.ParseIP(ip) - // check for CIDR, complete IP, or IP prefix + // check for cidr, complete ip, or ip prefix for _, exclIP := range ips { if _, cidrnet, err := net.ParseCIDR(exclIP); err == nil { if cidrnet.Contains(parsedIP) { diff --git a/onlyfrom_test.go b/onlyfrom_test.go index 3ed1dab..911c02d 100644 --- a/onlyfrom_test.go +++ b/onlyfrom_test.go @@ -102,16 +102,24 @@ func TestOnlyFromAllowedCIDR(t *testing.T) { func TestMatchSourceIPRules(t *testing.T) { tests := []struct { name string - rule string + rules []string source string matched bool }{ - {name: "complete IPv4", rule: "1.2.3.4", source: "1.2.3.4", matched: true}, - {name: "complete IPv4 rejects textual prefix", rule: "1.2.3.4", source: "1.2.3.45", matched: false}, - {name: "IPv4 prefix", rule: "1.2.3.", source: "1.2.3.45", matched: true}, - {name: "IPv4 CIDR", rule: "1.2.3.0/24", source: "1.2.3.45", matched: true}, - {name: "complete IPv6 normalized", rule: "2001:db8:0:0::1", source: "2001:db8::1", matched: true}, - {name: "complete IPv6 rejects textual prefix", rule: "2001:db8::1", source: "2001:db8::10", matched: false}, + {name: "complete ipv4", rules: []string{"1.2.3.4"}, source: "1.2.3.4", matched: true}, + {name: "complete ipv4 rejects textual prefix", rules: []string{"1.2.3.4"}, source: "1.2.3.45", matched: false}, + {name: "ipv4 prefix", rules: []string{"1.2.3."}, source: "1.2.3.45", matched: true}, + {name: "ipv4 cidr", rules: []string{"1.2.3.0/24"}, source: "1.2.3.45", matched: true}, + {name: "complete ipv6 normalized", rules: []string{"2001:db8:0:0::1"}, source: "2001:db8::1", matched: true}, + { + name: "complete ipv6 rejects textual prefix", rules: []string{"2001:db8::1"}, + source: "2001:db8::10", matched: false, + }, + {name: "ipv6 prefix", rules: []string{"2001:db8:"}, source: "2001:db8::10", matched: true}, + {name: "ipv6 prefix rejects mismatch", rules: []string{"2001:db9:"}, source: "2001:db8::10", matched: false}, + {name: "ipv6 cidr", rules: []string{"2001:db8::/32"}, source: "2001:db8::10", matched: true}, + {name: "ipv6 cidr rejects mismatch", rules: []string{"2001:db9::/32"}, source: "2001:db8::10", matched: false}, + {name: "later rule matches", rules: []string{"1.2.3.4", "5.6."}, source: "5.6.7.8", matched: true}, } for _, tt := range tests { @@ -119,7 +127,7 @@ func TestMatchSourceIPRules(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", http.NoBody) req.Header.Set("X-Real-IP", tt.source) - matched, source, err := matchSourceIP(req, []string{tt.rule}) + matched, source, err := matchSourceIP(req, tt.rules) require.NoError(t, err) assert.Equal(t, tt.matched, matched) assert.Equal(t, tt.source, source)