Check real client IP for access when behind a proxy

The module ngx_http_access_module allows limiting access to certain
*connecting* client addresses. When using it behind a proxy it would
then use the proxy IP instead of the real client IP, which is ultimately
the desired outcome.

By using the module ngx_http_geo_module we can explicitly specify what
are the proxy IPs thus allowing us to check the real client IP directly.

This implementation also creates unique geo variables to be used by
locations having the same set of access list IPs. This can result in a
much smaller configuration file.

The nginx documentation also recommends the usage of geo variables when
having a lot of rules:
> In case of a lot of rules, the use of the ngx_http_geo_module module variables is preferable.

See:
- http://nginx.org/en/docs/http/ngx_http_access_module.html
- http://nginx.org/en/docs/http/ngx_http_geo_module.html
This commit is contained in:
Renan Gonçalves 2022-03-09 15:13:10 +01:00 committed by Ruud van der Weijde
parent e09e40af1a
commit f910799647
No known key found for this signature in database
GPG key ID: E209DF6E958981C9
7 changed files with 322 additions and 24 deletions

View file

@ -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]{}

View file

@ -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

View file

@ -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

View file

@ -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 }}

View file

@ -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")

View file

@ -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)
})
})
})

View file

@ -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)