Make private Envoy sockets localhost only #24011

meyskens · 2023-02-24T13:33:10Z

This change makes parts of Envoy that do not need public access bind to
localhost only. This was previously not possible as it could not bind
on both ipv4 and v6. In Envoy 1.24 multi listeners were added making
this improvement possible.

~~Depends on #23940~~ (merged)

Fixes a part of #23353 (there are more cilium agent ports which are not managed by envoy)

Make Envoy sockets for tproxy and the xDS API and bind to localhost only

jrajahalme

Nice, some early comments below.

pkg/envoy/server.go

meyskens · 2023-02-28T06:20:15Z

/test

jrajahalme

Small nits only. Use of local only ports on DNS proxy may warrant further discussion, though.

bpf/lib/proxy.h

pkg/proxy/proxy.go

pkg/proxy/proxy_test.go

tommyp1ckles · 2023-03-08T22:40:52Z

pkg/envoy/ciliumenvoyconfig.go

@@ -361,7 +361,7 @@ func ParseResources(cecNamespace string, cecName string, anySlice []cilium_v2.XD
 	// Do this only after all other possible error cases.
 	for _, listener := range resources.Listeners {
 		if listener.GetAddress() == nil {
-			port, err := portAllocator.AllocateProxyPort(listener.Name, false)
+			port, err := portAllocator.AllocateProxyPort(listener.Name, false, true)


Was this meant to ever be configurable?

It kind of is already, you can specify an address and port in the Listener spec. This path is only executed when there is no address in the Listener spec, meaning that "let each Cilium Agent pick a local port". With this change that now includes the fact the locally allocated port is listened only on localhost interfaces. The locally allocated port is essentially random, so it is different on each node and is not exposed in any user facing API. Given this it would be hard to send traffic to that port from the outside of the node anyway, so listening on localhost only does not change much. Other than have one less open port on external interfaces potentially vulnerable for port scanning.

This change makes parts of Envoy that do not need public access bind to localhost only. This was previously not possible as it could not bind on both ipv4 and v6. In Envoy 1.24 multi listeners were added making this improvement possible. Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>

This change adds the ability to make proxy rules run over the local interface. To do this it adds the underlying IP to the iptable ruleset for --on-ip instead ot it defaulting to any ip. Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>

This change adds a lookup to the localhost IP in the tproxy code. It will first look for a localhost socket, failing that it will fall back to the previous behaviour of 0.0.0.0/:: Co-authored-by: Jarno Rajahalme <jarno@isovalent.com> Signed-off-by: Maartje Eyskens <maartje.eyskens@isovalent.com>

jrajahalme · 2023-03-20T15:55:57Z

The loader area change is a trivial interface function prototype change, no point asking a review from the loader area, so marking this as ready-to-merge.

jrajahalme · 2023-03-20T15:57:44Z

/test

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentChaosTest Connectivity demo application Endpoint can still connect while Cilium is not running

Failure Output

FAIL: Timed out after 240.000s.

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.25-kernel-4.19 so I can create one.

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks N/S loadbalancing Tests with XDP, direct routing, Hybrid and Maglev

Failure Output

FAIL: Timed out after 240.000s.

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

jrajahalme · 2023-03-20T15:58:26Z

Noticed ginkgo tests had not run after recent force pushes, running the tests now.

jrajahalme · 2023-03-22T14:11:52Z

The two test fails are for unrelated image pulling errors. Images exist but Cilium pods as in image-pull-backoff.

meyskens requested a review from jrajahalme February 24, 2023 13:33

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 24, 2023

jrajahalme reviewed Feb 24, 2023

View reviewed changes

pkg/envoy/server.go Outdated Show resolved Hide resolved

pkg/envoy/server.go Outdated Show resolved Hide resolved

meyskens force-pushed the meyskens/envoy-listen-local branch from 7d26d66 to 2913bb1 Compare February 24, 2023 13:52

meyskens added kind/cleanup This includes no functional changes. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Feb 24, 2023

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 24, 2023

meyskens force-pushed the meyskens/envoy-listen-local branch 4 times, most recently from 0ebe11b to 5f5852e Compare February 27, 2023 17:15

meyskens marked this pull request as ready for review February 28, 2023 06:19

meyskens requested review from a team as code owners February 28, 2023 06:19

meyskens requested review from tommyp1ckles and rgo3 February 28, 2023 06:19

meyskens requested a review from jrajahalme February 28, 2023 08:19

meyskens added the area/servicemesh GH issues or PRs regarding servicemesh label Mar 1, 2023

meyskens mentioned this pull request Mar 3, 2023

gha: Clean-up Ingress/GatewayAPI Conformance tests #24025

Merged

jrajahalme approved these changes Mar 8, 2023

View reviewed changes

bpf/lib/proxy.h Outdated Show resolved Hide resolved

bpf/lib/proxy.h Outdated Show resolved Hide resolved

pkg/proxy/proxy.go Show resolved Hide resolved

pkg/proxy/proxy_test.go Show resolved Hide resolved

tommyp1ckles reviewed Mar 8, 2023

View reviewed changes

meyskens force-pushed the meyskens/envoy-listen-local branch from 5f5852e to 879026c Compare March 9, 2023 14:49

meyskens mentioned this pull request Mar 9, 2023

[pkg/proxy] Make change of values in AllocateProxyPort() predictable #24264

Closed

tommyp1ckles approved these changes Mar 14, 2023

View reviewed changes

meyskens force-pushed the meyskens/envoy-listen-local branch from 879026c to 6bf753b Compare March 20, 2023 09:05

meyskens added 2 commits March 20, 2023 14:41

meyskens force-pushed the meyskens/envoy-listen-local branch from 6bf753b to e180093 Compare March 20, 2023 13:41

jrajahalme added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 20, 2023

jrajahalme removed the request for review from rgo3 March 20, 2023 15:56

jrajahalme removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 20, 2023

meyskens added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 22, 2023

borkmann merged commit e3af70e into cilium:master Mar 22, 2023
41 checks passed

meyskens deleted the meyskens/envoy-listen-local branch March 27, 2023 08:20

meyskens mentioned this pull request Mar 27, 2023

Cilium agent opens ports in all interfaces (maybe unnecessarily?) #23353

Closed

2 tasks

jrajahalme mentioned this pull request Sep 4, 2023

Fix minor bug where the previous Cilium proxy port was not reused #27634

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make private Envoy sockets localhost only #24011

Make private Envoy sockets localhost only #24011

meyskens commented Feb 24, 2023 •

edited

jrajahalme left a comment

meyskens commented Feb 28, 2023

jrajahalme left a comment •

edited

tommyp1ckles Mar 8, 2023

jrajahalme Mar 9, 2023

jrajahalme commented Mar 20, 2023

jrajahalme commented Mar 20, 2023 •

edited by maintainer-s-little-helper bot

Test Name

Failure Output

Test Name

Failure Output

jrajahalme commented Mar 20, 2023

jrajahalme commented Mar 22, 2023

Make private Envoy sockets localhost only #24011

Make private Envoy sockets localhost only #24011

Conversation

meyskens commented Feb 24, 2023 • edited

jrajahalme left a comment

Choose a reason for hiding this comment

meyskens commented Feb 28, 2023

jrajahalme left a comment • edited

Choose a reason for hiding this comment

tommyp1ckles Mar 8, 2023

Choose a reason for hiding this comment

jrajahalme Mar 9, 2023

Choose a reason for hiding this comment

jrajahalme commented Mar 20, 2023

jrajahalme commented Mar 20, 2023 • edited by maintainer-s-little-helper bot

Test Name

Failure Output

Test Name

Failure Output

jrajahalme commented Mar 20, 2023

jrajahalme commented Mar 22, 2023

meyskens commented Feb 24, 2023 •

edited

jrajahalme left a comment •

edited

jrajahalme commented Mar 20, 2023 •

edited by maintainer-s-little-helper bot