Fix routing for TPROXY mode #28363

gmemcc · 2020-10-28T14:16:17Z

This PR fixes #23369

In the old versions, envoy sent upstream requests to 127.0.0.1, but now it sends to original dest ip ( pod ip), which causes TPROXY to not work properly.

this PR will:

ensure correct routing for traffic from workload to envoy
prevent infinite redirecting
change TPROXY target port from 15001 to 15006

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[X] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

istio-policy-bot · 2020-10-28T14:16:19Z

🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test?

Courtesy of your friendly test nag.

istio-policy-bot · 2020-10-28T14:16:20Z

😊 Welcome @gmemcc! This is either your first contribution to the Istio istio repo, or it's been
awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

istio-testing · 2020-10-28T14:16:33Z

Hi @gmemcc. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

howardjohn · 2020-10-28T14:54:22Z

cc @istio/wg-networking-maintainers and @istio/technical-oversight-committee

We need to either add proper tests and docs for this feature or consider removal. Otherwise we end up in permanent experimental state which leads to a lack of user trust

rlenglet · 2020-10-29T00:29:09Z

/ok-to-test

rlenglet · 2020-10-29T00:31:23Z

pilot/pkg/xds/filters/filters.go

@@ -104,7 +105,9 @@ var (
 	OriginalSrc = &listener.ListenerFilter{
 		Name: OriginalSrcFilterName,
 		ConfigType: &listener.ListenerFilter_TypedConfig{
-			TypedConfig: util.MessageToAny(&originalsrc.OriginalSrc{}),
+			TypedConfig: util.MessageToAny(&originalsrc.OriginalSrc{
+				Mark: uint32(os.Getegid()),


I don't understand why this is used as the mark?!

Should this be set as 1337?

pod template provided by configmap istio-sidecar-injector declares runAsGroup: 1337 for TPROXY mode:

privileged: {{ .Values.global.proxy.privileged }} readOnlyRootFilesystem: {{ not .Values.global.proxy.enableCoreDump }} runAsGroup: 1337 fsGroup: 1337 {{ if or (eq (annotation .ObjectMeta `sidecar.istio.io/interceptionMode` .ProxyConfig.InterceptionMode) `TPROXY`) (eq (annotation .ObjectMeta `sidecar.istio.io/capNetBindService` .Values.global.proxy.capNetBindService) `true`) -}} runAsNonRoot: false runAsUser: 0 {{- else -}} runAsNonRoot: true runAsUser: 1337 {{- end }}

I think getting the mark dynamically is more portable

I did not realize that the istiod is run as 1337, @howardjohn I remember you said there is a case we run istiod locally without in k8s.

we should not assume istiod uid/gid correlates. we set it to 1337 only out of convenience

my misreading... should we introduce an environment variable or just hardcode 1337?

hzxuzhonghu · 2020-10-29T01:43:02Z

@gmemcc Can you also take the outside traffic through ingress gateway into consideration? We have use case to get the the client source ip in this case.

hzxuzhonghu · 2020-10-29T08:43:39Z

I have tested it. To make it work: we need to make several other changes too.

virtualInbound listener filters orders should be changed. If not, then when i access with podip directly, the original src filter does not take effect, i guess it is influenced by the tls inspector. After i moved it before, everything works
Original Src filter mark should be set 1337
run echo 1 > /proc/sys/net/ipv4/conf/eth0/route_localnet

hzxuzhonghu · 2020-10-29T08:48:54Z

tools/istio-iptables/pkg/cmd/run.go

-			"-j", constants.MARK, "--set-mark", iptConfigurator.cfg.InboundTProxyMark)
+			"-p", constants.TCP, "-m", "connmark", "--mark", iptConfigurator.cfg.InboundTProxyMark, "-j", "CONNMARK", "--restore-mark")
+		// prevent infinite redirect
+		iptConfigurator.iptables.InsertRuleV4(constants.ISTIOINBOUND, constants.MANGLE, 1,


This rule may be unnecessary, it works without it

Yes, I also found that the last rule is not necessary in default configuration, but without that rule, after applying a Sidecar object:

apiVersion: networking.istio.io/v1alpha3 kind: Sidecar metadata: name: default namespace: default spec: egress: - hosts: - "default/*"

infinite redirection appeared again

gmemcc · 2020-10-29T13:24:30Z

@gmemcc Can you also take the outside traffic through ingress gateway into consideration? We have use case to get the the client source ip in this case.

If ingress gateway acts as a transparent proxy, it will send packets whose src ip is the real client ip to upstream workload. how to fix routing for replying packets may be environment dependent, usually cannot be done via iptables rules.

istio-testing · 2020-10-29T13:53:40Z

@gmemcc: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
release-notes_istio	`dfb3216`	link	`/test release-notes_istio`
lint_istio	`dfb3216`	link	`/test lint_istio`
unit-tests_istio	`dfb3216`	link	`/test unit-tests_istio`
gencheck_istio	`dfb3216`	link	`/test gencheck_istio`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

gmemcc · 2020-10-29T14:02:35Z

@hzxuzhonghu in my cluster, leaving /proc/sys/net/ipv4/conf/eth0/route_localnet as its default value 0 is ok, may be this is kernel-dependent ? my kernel version is 4.4.0-186

hzxuzhonghu · 2020-10-30T01:33:53Z

Mine is 4.4.0-142-generic

hzxuzhonghu · 2020-10-30T01:35:45Z

If ingress gateway acts as a transparent proxy, it will send packets whose src ip is the real client ip to upstream workload. how to fix routing for replying packets may be environment dependent, usually cannot be done via iptables rules.

maybe via configuring ingress gateway to send proxy protocol, I will investigate it

gmemcc · 2020-11-02T04:31:37Z

see #28457

Fix routing for TPROXY mode

dfb3216

gmemcc requested a review from a team as a code owner October 28, 2020 14:16

istio-policy-bot added the area/networking label Oct 28, 2020

google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Oct 28, 2020

istio-testing added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-ok-to-test labels Oct 28, 2020

istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Oct 29, 2020

rlenglet reviewed Oct 29, 2020

View reviewed changes

hzxuzhonghu reviewed Oct 29, 2020

View reviewed changes

gmemcc closed this Oct 29, 2020

gmemcc reopened this Oct 29, 2020

hzxuzhonghu mentioned this pull request Nov 2, 2020

Preserve original src ip #28457

Merged

gmemcc closed this Nov 2, 2020

Fix routing for TPROXY mode #28363

Fix routing for TPROXY mode #28363

Uh oh!

Conversation

gmemcc commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

istio-policy-bot commented Oct 28, 2020

Uh oh!

istio-policy-bot commented Oct 28, 2020

Uh oh!

istio-testing commented Oct 28, 2020

Uh oh!

howardjohn commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rlenglet commented Oct 29, 2020

Uh oh!

rlenglet Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

gmemcc Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu Oct 30, 2020

Choose a reason for hiding this comment

Uh oh!

howardjohn Oct 30, 2020

Choose a reason for hiding this comment

Uh oh!

gmemcc Oct 30, 2020

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu commented Oct 29, 2020

Uh oh!

hzxuzhonghu commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hzxuzhonghu Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

gmemcc Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

gmemcc commented Oct 29, 2020

Uh oh!

istio-testing commented Oct 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmemcc commented Oct 29, 2020

Uh oh!

hzxuzhonghu commented Oct 30, 2020

Uh oh!

hzxuzhonghu commented Oct 30, 2020

Uh oh!

gmemcc commented Nov 2, 2020

Uh oh!

Uh oh!

gmemcc commented Oct 28, 2020 •

edited

Loading

howardjohn commented Oct 28, 2020 •

edited

Loading

gmemcc Oct 29, 2020 •

edited

Loading

hzxuzhonghu commented Oct 29, 2020 •

edited

Loading

istio-testing commented Oct 29, 2020 •

edited

Loading