From 0dceedfad77ee0ce44d8bb54b0f399494d1c9f50 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Fri, 30 Apr 2021 00:24:28 -0300 Subject: [PATCH] Remove localhost calls from external names Signed-off-by: Ricardo Pchevuzinske Katz --- .github/workflows/ci.yaml | 2 +- internal/ingress/controller/endpoints.go | 6 +++ internal/ingress/controller/endpoints_test.go | 48 +++++++++++++++++++ rootfs/etc/nginx/lua/balancer.lua | 8 ++++ rootfs/etc/nginx/lua/tcp_udp_balancer.lua | 8 ++++ rootfs/etc/nginx/template/nginx.tmpl | 3 ++ 6 files changed, 74 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1db87e791c..612bd4b387 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -159,7 +159,7 @@ jobs: strategy: matrix: - k8s: [v1.16.15, v1.17.17, v1.18.19, v1.19.11, v1.20.7, v1.21.0] + k8s: [v1.16.15, v1.17.17, v1.18.15, v1.19.7, v1.20.2] steps: diff --git a/internal/ingress/controller/endpoints.go b/internal/ingress/controller/endpoints.go index 7164614dbe..bdddcb0a08 100644 --- a/internal/ingress/controller/endpoints.go +++ b/internal/ingress/controller/endpoints.go @@ -50,6 +50,12 @@ func getEndpoints(s *corev1.Service, port *corev1.ServicePort, proto corev1.Prot // ExternalName services if s.Spec.Type == corev1.ServiceTypeExternalName { + if ip := net.ParseIP(s.Spec.ExternalName); s.Spec.ExternalName == "localhost" || + (ip != nil && ip.IsLoopback()) { + klog.Errorf("Invalid attempt to use localhost name %s in %q", s.Spec.ExternalName, svcKey) + return upsServers + } + klog.V(3).Infof("Ingress using Service %q of type ExternalName.", svcKey) targetPort := port.TargetPort.IntValue() // if the externalName is not an IP address we need to validate is a valid FQDN diff --git a/internal/ingress/controller/endpoints_test.go b/internal/ingress/controller/endpoints_test.go index 6dad1b55c3..20d53c5265 100644 --- a/internal/ingress/controller/endpoints_test.go +++ b/internal/ingress/controller/endpoints_test.go @@ -78,6 +78,54 @@ func TestGetEndpoints(t *testing.T) { }, []ingress.Endpoint{}, }, + { + "a service type ServiceTypeExternalName service with localhost in name should return 0 endpoint", + &corev1.Service{ + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeExternalName, + ExternalName: "localhost", + Ports: []corev1.ServicePort{ + { + Name: "default", + TargetPort: intstr.FromInt(443), + }, + }, + }, + }, + &corev1.ServicePort{ + Name: "default", + TargetPort: intstr.FromInt(80), + }, + corev1.ProtocolTCP, + func(string) (*corev1.Endpoints, error) { + return &corev1.Endpoints{}, nil + }, + []ingress.Endpoint{}, + }, + { + "a service type ServiceTypeExternalName service with 127.0.0.1 in name should return 0 endpoint", + &corev1.Service{ + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeExternalName, + ExternalName: "127.0.0.1", + Ports: []corev1.ServicePort{ + { + Name: "default", + TargetPort: intstr.FromInt(443), + }, + }, + }, + }, + &corev1.ServicePort{ + Name: "default", + TargetPort: intstr.FromInt(80), + }, + corev1.ProtocolTCP, + func(string) (*corev1.Endpoints, error) { + return &corev1.Endpoints{}, nil + }, + []ingress.Endpoint{}, + }, { "a service type ServiceTypeExternalName with a valid port should return one endpoint", &corev1.Service{ diff --git a/rootfs/etc/nginx/lua/balancer.lua b/rootfs/etc/nginx/lua/balancer.lua index ad91b0041c..afcfebb67a 100644 --- a/rootfs/etc/nginx/lua/balancer.lua +++ b/rootfs/etc/nginx/lua/balancer.lua @@ -35,6 +35,9 @@ local IMPLEMENTATIONS = { ewma = ewma, } +local PROHIBITED_LOCALHOST_PORT = configuration.prohibited_localhost_port or '10246' +local PROHIBITED_PEER_PATTERN = "^127.*:" .. PROHIBITED_LOCALHOST_PORT .. "$" + local _M = {} local balancers = {} local backends_with_external_name = {} @@ -317,6 +320,11 @@ function _M.balance() return end + if peer:match(PROHIBITED_PEER_PATTERN) then + ngx.log(ngx.ERR, "attempted to proxy to self, balancer: ", balancer.name, ", peer: ", peer) + return + end + ngx_balancer.set_more_tries(1) local ok, err = ngx_balancer.set_current_peer(peer) diff --git a/rootfs/etc/nginx/lua/tcp_udp_balancer.lua b/rootfs/etc/nginx/lua/tcp_udp_balancer.lua index a2a491af8e..4a9694d09f 100644 --- a/rootfs/etc/nginx/lua/tcp_udp_balancer.lua +++ b/rootfs/etc/nginx/lua/tcp_udp_balancer.lua @@ -25,6 +25,9 @@ local IMPLEMENTATIONS = { round_robin = round_robin } +local PROHIBITED_LOCALHOST_PORT = configuration.prohibited_localhost_port or '10246' +local PROHIBITED_PEER_PATTERN = "^127.*:" .. PROHIBITED_LOCALHOST_PORT .. "$" + local _M = {} local balancers = {} local backends_with_external_name = {} @@ -181,6 +184,11 @@ function _M.balance() return end + if peer:match(PROHIBITED_PEER_PATTERN) then + ngx.log(ngx.ERR, "attempted to proxy to self, balancer: ", balancer.name, ", peer: ", peer) + return + end + ngx_balancer.set_more_tries(1) local ok, err = ngx_balancer.set_current_peer(peer) diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 57f8bb3d4b..45c261f83b 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -84,6 +84,7 @@ http { error("require failed: " .. tostring(res)) else configuration = res + configuration.prohibited_localhost_port = '{{ .StatusPort }}' end ok, res = pcall(require, "balancer") @@ -713,6 +714,8 @@ stream { error("require failed: " .. tostring(res)) else tcp_udp_configuration = res + tcp_udp_configuration.prohibited_localhost_port = '{{ .StatusPort }}' + end ok, res = pcall(require, "tcp_udp_balancer")