diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 6aadab48e..60fe66fab 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -45,6 +45,7 @@ import ( "k8s.io/ingress-nginx/internal/ingress/controller/config" ing_net "k8s.io/ingress-nginx/internal/net" "k8s.io/ingress-nginx/pkg/apis/ingress" + iSets "k8s.io/ingress-nginx/pkg/util/sets" ) const ( @@ -234,6 +235,8 @@ var ( "extractHostPort": extractHostPort, "changeHostPort": changeHostPort, "buildProxyPass": buildProxyPass, + "buildDenylists": buildDenylists, + "buildWhitelists": buildWhitelists, "filterRateLimits": filterRateLimits, "buildRateLimitZones": buildRateLimitZones, "buildRateLimit": buildRateLimit, @@ -790,6 +793,105 @@ rewrite "(?i)%s" %s break; return defProxyPass } +type accessListVariable struct { + ID string `json:"id"` + AccessList []string `json:"accessList"` +} + +func (gv1 *accessListVariable) Equal(gv2 *accessListVariable) bool { + if gv1 == gv2 { + return true + } + if gv1 == nil || gv2 == nil { + return false + } + + if len(gv1.AccessList) != len(gv2.AccessList) { + return false + } + return iSets.StringElementsMatch(gv1.AccessList, gv2.AccessList) +} + +func buildDenylists(input interface{}) []accessListVariable { + var variables []accessListVariable + + servers, ok := input.([]*ingress.Server) + if !ok { + klog.Errorf("expected a '[]*ingress.Server' type but %T was returned", input) + return variables + } + + for _, server := range servers { + for _, loc := range server.Locations { + if len(loc.Denylist.CIDR) == 0 { + continue + } + + list := accessListVariable{ + ID: fmt.Sprintf("%d", len(variables)), + AccessList: loc.Denylist.CIDR, + } + + foundID := "" + for _, list2 := range variables { + if list.Equal(&list2) { + foundID = list2.ID + continue + } + } + + if foundID == "" { + foundID = list.ID + variables = append(variables, list) + } + + loc.DenylistID = foundID + } + } + + return variables +} + +func buildWhitelists(input interface{}) []accessListVariable { + var variables []accessListVariable + + servers, ok := input.([]*ingress.Server) + if !ok { + klog.Errorf("expected a '[]*ingress.Server' type but %T was returned", input) + return variables + } + + for _, server := range servers { + for _, loc := range server.Locations { + if len(loc.Whitelist.CIDR) == 0 { + continue + } + + list := accessListVariable{ + ID: fmt.Sprintf("%d", len(variables)), + AccessList: loc.Whitelist.CIDR, + } + + foundID := "" + for _, list2 := range variables { + if list.Equal(&list2) { + foundID = list2.ID + continue + } + } + + if foundID == "" { + foundID = list.ID + variables = append(variables, list) + } + + loc.WhitelistID = foundID + } + } + + return variables +} + func filterRateLimits(input interface{}) []ratelimit.Config { ratelimits := []ratelimit.Config{} found := sets.Set[string]{} diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 1980d7e52..568785f75 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -20,6 +20,8 @@ import ( "bytes" "encoding/base64" "fmt" + "k8s.io/ingress-nginx/internal/ingress/annotations/ipdenylist" + "k8s.io/ingress-nginx/internal/ingress/annotations/ipwhitelist" "net" "os" "path" @@ -1006,6 +1008,130 @@ func TestFilterRateLimits(t *testing.T) { } } +func TestBuildDenylists(t *testing.T) { + servers := []*ingress.Server{ + { + Locations: []*ingress.Location{ + { + Denylist: ipdenylist.SourceRange{ + CIDR: []string{"8.8.8.8/32"}, + }, + }, + { + Denylist: ipdenylist.SourceRange{ + CIDR: []string{"4.4.4.4/32"}, + }, + }, + { + Denylist: ipdenylist.SourceRange{ + CIDR: []string{"8.8.8.8/32"}, + }, + }, + { + Denylist: ipdenylist.SourceRange{ + CIDR: []string{"4.4.4.4/32", "8.8.8.8/32"}, + }, + }, + }, + }, + } + + expected := []accessListVariable{ + { + ID: "0", + AccessList: []string{"8.8.8.8/32"}, + }, + { + ID: "1", + AccessList: []string{"4.4.4.4/32"}, + }, + { + ID: "2", + AccessList: []string{"4.4.4.4/32", "8.8.8.8/32"}, + }, + } + actual := buildDenylists(servers) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + if servers[0].Locations[0].DenylistID != "0" { + t.Errorf("Expected '%v' but returned '%v'", "0", servers[0].Locations[0].DenylistID) + } + if servers[0].Locations[1].DenylistID != "1" { + t.Errorf("Expected '%v' but returned '%v'", "1", servers[0].Locations[1].DenylistID) + } + if servers[0].Locations[2].DenylistID != "0" { + t.Errorf("Expected '%v' but returned '%v'", "0", servers[0].Locations[2].DenylistID) + } + if servers[0].Locations[3].DenylistID != "2" { + t.Errorf("Expected '%v' but returned '%v'", "2", servers[0].Locations[3].DenylistID) + } +} + +func TestBuildWhitelists(t *testing.T) { + servers := []*ingress.Server{ + { + Locations: []*ingress.Location{ + { + Whitelist: ipwhitelist.SourceRange{ + CIDR: []string{"8.8.8.8/32"}, + }, + }, + { + Whitelist: ipwhitelist.SourceRange{ + CIDR: []string{"4.4.4.4/32"}, + }, + }, + { + Whitelist: ipwhitelist.SourceRange{ + CIDR: []string{"8.8.8.8/32"}, + }, + }, + { + Whitelist: ipwhitelist.SourceRange{ + CIDR: []string{"4.4.4.4/32", "8.8.8.8/32"}, + }, + }, + }, + }, + } + + expected := []accessListVariable{ + { + ID: "0", + AccessList: []string{"8.8.8.8/32"}, + }, + { + ID: "1", + AccessList: []string{"4.4.4.4/32"}, + }, + { + ID: "2", + AccessList: []string{"4.4.4.4/32", "8.8.8.8/32"}, + }, + } + actual := buildWhitelists(servers) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf("Expected '%v' but returned '%v'", expected, actual) + } + + if servers[0].Locations[0].WhitelistID != "0" { + t.Errorf("Expected '%v' but returned '%v'", "0", servers[0].Locations[0].WhitelistID) + } + if servers[0].Locations[1].WhitelistID != "1" { + t.Errorf("Expected '%v' but returned '%v'", "1", servers[0].Locations[1].WhitelistID) + } + if servers[0].Locations[2].WhitelistID != "0" { + t.Errorf("Expected '%v' but returned '%v'", "0", servers[0].Locations[2].WhitelistID) + } + if servers[0].Locations[3].WhitelistID != "2" { + t.Errorf("Expected '%v' but returned '%v'", "2", servers[0].Locations[3].WhitelistID) + } +} + func TestBuildAuthSignURL(t *testing.T) { cases := map[string]struct { Input, RedirectParam, Output string diff --git a/pkg/apis/ingress/types.go b/pkg/apis/ingress/types.go index e50666c18..eeef7a6b8 100644 --- a/pkg/apis/ingress/types.go +++ b/pkg/apis/ingress/types.go @@ -298,10 +298,16 @@ type Location struct { // addresses or networks are allowed. // +optional Denylist ipdenylist.SourceRange `json:"denylist,omitempty"` + // Denylist ID is a unique variable index + // +optional + DenylistID string // Whitelist indicates only connections from certain client // addresses or networks are allowed. // +optional Whitelist ipwhitelist.SourceRange `json:"whitelist,omitempty"` + // Whitelist ID is a unique variable index + // +optional + WhitelistID string // Proxy contains information about timeouts and buffer sizes // to be used in connections against endpoints // +optional diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index a49989f30..b6d2801e7 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -540,6 +540,34 @@ http { } {{ end }} + {{ range $variable := (buildDenylists $servers) }} + geo $denied_{{ $variable.ID }} { + default "false"; + + {{- range $trustedIP := $cfg.ProxyRealIPCIDR }} + proxy {{ $trustedIP }}; + {{- end }} + + {{- range $denylistIP := $variable.AccessList }} + {{ $denylistIP }} "true"; + {{- end }} + } + {{ end }} + + {{ range $variable := (buildWhitelists $servers) }} + geo $allowed_{{ $variable.ID }} { + default "false"; + + {{- range $trustedIP := $cfg.ProxyRealIPCIDR }} + proxy {{ $trustedIP }}; + {{- end }} + + {{- range $whitelistIP := $variable.AccessList }} + {{ $whitelistIP }} "true"; + {{- end }} + } + {{ end }} + {{/* build all the required rate limit zones. Each annotation requires a dedicated zone */}} {{/* 1MB -> 16 thousand 64-byte states or about 8 thousand 128-byte states */}} {{ range $zone := (buildRateLimitZones $servers) }} @@ -1287,14 +1315,15 @@ stream { {{ buildModSecurityForLocation $all.Cfg $location }} {{ if isLocationAllowed $location }} - {{ if gt (len $location.Denylist.CIDR) 0 }} - {{ range $ip := $location.Denylist.CIDR }} - deny {{ $ip }};{{ end }} + {{ if gt (len $location.DenylistID) 0 }} + if ($denied_{{ $location.DenylistID }} = "true") { + return 403; + } {{ end }} - {{ if gt (len $location.Whitelist.CIDR) 0 }} - {{ range $ip := $location.Whitelist.CIDR }} - allow {{ $ip }};{{ end }} - deny all; + {{ if gt (len $location.WhitelistID) 0 }} + if ($allowed_{{ $location.WhitelistID }} = "false") { + return 403; + } {{ end }} {{ if $location.CorsConfig.CorsEnabled }} diff --git a/test/e2e/annotations/ipdenylist.go b/test/e2e/annotations/ipdenylist.go index 9c1d45cf5..938090ff9 100644 --- a/test/e2e/annotations/ipdenylist.go +++ b/test/e2e/annotations/ipdenylist.go @@ -17,10 +17,9 @@ limitations under the License. package annotations import ( - "net/http" - "strings" - "github.com/onsi/ginkgo/v2" + "net/http" + "regexp" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -51,11 +50,21 @@ var _ = framework.DescribeAnnotation("denylist-source-range", func() { f.EnsureIngress(ing) + f.WaitForNginxConfiguration( + func(cfg string) bool { + return regexp.MustCompile( + `geo \$denied_0 \{\s+` + + `default "false";\s+` + + `proxy 0.0.0.0/0;\s+` + + `18.0.0.0/8 "true";\s+` + + `56.0.0.1 "true";\s+` + + `}`, + ).MatchString(cfg) + }) + f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, "deny 18.0.0.0/8;") && - strings.Contains(server, "deny 56.0.0.1;") && - !strings.Contains(server, "deny all;") + return regexp.MustCompile(`if \(\$denied_0 = "true"\) \{\s+return 403;\s+}`).MatchString(server) }) ginkgo.By("sending request from an explicitly denied IP range") @@ -103,13 +112,30 @@ var _ = framework.DescribeAnnotation("denylist-source-range", func() { f.EnsureIngress(ing) + f.WaitForNginxConfiguration( + func(cfg string) bool { + return regexp.MustCompile( + `geo \$denied_0 \{\s+`+ + `default "false";\s+`+ + `proxy 0.0.0.0/0;\s+`+ + `18.1.0.0/16 "true";\s+`+ + `56.0.0.0/8 "true";\s+`+ + `}`, + ).MatchString(cfg) && + regexp.MustCompile( + `geo \$allowed_0 \{\s+`+ + `default "false";\s+`+ + `proxy 0.0.0.0/0;\s+`+ + `18.0.0.0/8 "true";\s+`+ + `55.0.0.0/8 "true";\s+`+ + `}`, + ).MatchString(cfg) + }) + f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, "deny 18.1.0.0/16;") && - strings.Contains(server, "deny 56.0.0.0/8;") && - strings.Contains(server, "allow 18.0.0.0/8;") && - strings.Contains(server, "allow 55.0.0.0/8;") && - strings.Contains(server, "deny all;") + return regexp.MustCompile(`if \(\$denied_0 = "true"\) \{\s+return 403;\s+}`).MatchString(server) && + regexp.MustCompile(`if \(\$allowed_0 = "false"\) \{\s+return 403;\s+}`).MatchString(server) }) ginkgo.By("sending request from an explicitly denied IP range") diff --git a/test/e2e/annotations/ipwhitelist.go b/test/e2e/annotations/ipwhitelist.go index 71f026c7f..d9df8fe49 100644 --- a/test/e2e/annotations/ipwhitelist.go +++ b/test/e2e/annotations/ipwhitelist.go @@ -17,9 +17,8 @@ limitations under the License. package annotations import ( - "strings" - "github.com/onsi/ginkgo/v2" + "regexp" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -42,11 +41,21 @@ var _ = framework.DescribeAnnotation("whitelist-source-range", func() { ing := framework.NewSingleIngress(host, "/", host, nameSpace, framework.EchoService, 80, annotations) f.EnsureIngress(ing) + f.WaitForNginxConfiguration( + func(cfg string) bool { + return regexp.MustCompile( + `geo \$allowed_0 \{\s+` + + `default "false";\s+` + + `proxy 0.0.0.0/0;\s+` + + `18.0.0.0/8 "true";\s+` + + `56.0.0.0/8 "true";\s+` + + `}`, + ).MatchString(cfg) + }) + f.WaitForNginxServer(host, func(server string) bool { - return strings.Contains(server, "allow 18.0.0.0/8;") && - strings.Contains(server, "allow 56.0.0.0/8;") && - strings.Contains(server, "deny all;") + return regexp.MustCompile(`if \(\$allowed_0 = "false"\) \{\s+return 403;\s+}`).MatchString(server) }) }) }) diff --git a/test/e2e/settings/configmap_change.go b/test/e2e/settings/configmap_change.go index 3e37b62cd..07f55b25d 100644 --- a/test/e2e/settings/configmap_change.go +++ b/test/e2e/settings/configmap_change.go @@ -54,7 +54,7 @@ var _ = framework.DescribeSetting("Configmap change", func() { checksum = match[1] } - return strings.Contains(cfg, "allow 1.1.1.1;") + return strings.Contains(cfg, `1.1.1.1 "true";`) }) assert.NotEmpty(ginkgo.GinkgoT(), checksum)