Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iptables: Remove '--nowildcard' from socket match #12248

Merged
merged 3 commits into from Jun 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/datapath/iptables/iptables.go
Expand Up @@ -488,7 +488,7 @@ func (m *IptablesManager) inboundProxyRedirectRule(cmd string) []string {
return append(m.waitArgs,
"-t", "mangle",
cmd, ciliumPreMangleChain,
"-m", "socket", "--transparent", "--nowildcard",
"-m", "socket", "--transparent",
"-m", "comment", "--comment", "cilium: any->pod redirect proxied traffic to host proxy",
"-j", "MARK",
"--set-mark", toProxyMark)
Expand Down
18 changes: 18 additions & 0 deletions test/helpers/kubectl.go
Expand Up @@ -4077,3 +4077,21 @@ func logGathererSelector(allNodes bool) string {

return selector
}

// GetDNSProxyPort returns the port the Cilium DNS proxy is listening on
func (kub *Kubectl) GetDNSProxyPort(ciliumPod string) int {
const pickDNSProxyPort = `iptables-save -t mangle | sed -n '/udp.*TPROXY to host cilium-dns-egress/ s/.*--on-port \([1-9][0-9]*\).*/\1/p'`

// Find out the DNS proxy ports in use
res := kub.CiliumExecContext(context.TODO(), ciliumPod, pickDNSProxyPort)
if !res.WasSuccessful() {
ginkgoext.Failf("Cannot find DNS proxy port on %s", ciliumPod)
}
portStr := res.GetStdOut().String()
gomega.ExpectWithOffset(1, portStr).ShouldNot(gomega.BeEmpty(), "No DNS proxy port found on %s", ciliumPod)
port, err := strconv.Atoi(strings.TrimSpace(portStr))
if err != nil || port == 0 {
ginkgoext.Failf("Invalid DNS proxy port on %s: %s", ciliumPod, portStr)
}
return port
Comment on lines +4083 to +4096
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pull this from cilium status rather than the iptables rules? In future we won't rely on iptables so it'd be nice to have tests that don't rely on this implementation detail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cilium status does not currently report the actual proxy ports, so this would be a clean-up for a later PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not true :-)

$ kubectl -n kube-system exec -ti pod/cilium-6nbc5 -- cilium status -o json | jq -r '.proxy.redirects[0]."proxy-port"'
11128

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm fine to have this as an independent follow-up though)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I tried it without any policy imported (but DNS proxy still running) and in that case there are no redirects (.proxy.redirects == null), but the bug still exists as the DNS proxy is already listening.

}
68 changes: 67 additions & 1 deletion test/k8sT/Services.go
Expand Up @@ -1113,7 +1113,73 @@ var _ = Describe("K8sServicesTest", func() {
testExternalTrafficPolicyLocal()
})

SkipContextIf(helpers.RunsWithoutKubeProxy, "with L4 policy", func() {
Context("TFTP with DNS Proxy port collision", func() {
var (
demoPolicy string
ciliumPodK8s1 string
ciliumPodK8s2 string
DNSProxyPort1 int
DNSProxyPort2 int
)

BeforeAll(func() {
var err error
ciliumPodK8s1, err = kubectl.GetCiliumPodOnNodeWithLabel(helpers.CiliumNamespace, helpers.K8s1)
Expect(err).Should(BeNil(), "Cannot get cilium pod on %s", helpers.K8s1)
ciliumPodK8s2, err = kubectl.GetCiliumPodOnNodeWithLabel(helpers.CiliumNamespace, helpers.K8s2)
Expect(err).Should(BeNil(), "Cannot get cilium pod on %s", helpers.K8s2)

// Find out the DNS proxy ports in use
DNSProxyPort1 = kubectl.GetDNSProxyPort(ciliumPodK8s1)
By("DNS Proxy port in k8s1 (%s): %d", ciliumPodK8s1, DNSProxyPort1)
DNSProxyPort2 = kubectl.GetDNSProxyPort(ciliumPodK8s2)
By("DNS Proxy port in k8s2 (%s): %d", ciliumPodK8s2, DNSProxyPort2)

demoPolicy = helpers.ManifestGet(kubectl.BasePath(), "l4-policy-demo.yaml")
})

AfterAll(func() {
// Explicitly ignore result of deletion of resources to avoid incomplete
// teardown if any step fails.
_ = kubectl.Delete(demoPolicy)
})

It("Tests TFTP from DNS Proxy Port", func() {
if DNSProxyPort2 == DNSProxyPort1 {
Skip(fmt.Sprintf("TFTP source port test can not be done when both nodes have the same proxy port (%d == %d)", DNSProxyPort1, DNSProxyPort2))
}
monitorRes1, monitorCancel1 := kubectl.MonitorStart(helpers.CiliumNamespace, ciliumPodK8s1)
monitorRes2, monitorCancel2 := kubectl.MonitorStart(helpers.CiliumNamespace, ciliumPodK8s2)
defer func() {
monitorCancel1()
monitorCancel2()
helpers.WriteToReportFile(monitorRes1.CombineOutput().Bytes(), "tftp-with-l4-policy-monitor-k8s1.log")
helpers.WriteToReportFile(monitorRes2.CombineOutput().Bytes(), "tftp-with-l4-policy-monitor-k8s2.log")
}()

applyPolicy(demoPolicy)

var data v1.Service
err := kubectl.Get(helpers.DefaultNamespace, "service test-nodeport").Unmarshal(&data)
Expect(err).Should(BeNil(), "Can not retrieve service")

// Test enough times to get random backend selection from both nodes.
// The interesting case is when the backend is at k8s2.
count := 10
fails := 0
// Client from k8s1
clientPod, _ := kubectl.GetPodOnNodeWithOffset(helpers.K8s1, testDSClient, 1)
// Destination is a NodePort in k8s2, curl (in k8s1) binding to the same local port as the DNS proxy port
// in k8s2
url := getTFTPLink(k8s2IP, data.Spec.Ports[1].NodePort) + fmt.Sprintf(" --local-port %d", DNSProxyPort2)
cmd := testCommand(helpers.CurlFailNoStats(url), count, fails)
By("Making %d curl requests from %s pod to service %s using source port %d", count, clientPod, url, DNSProxyPort2)
res := kubectl.ExecPodCmd(helpers.DefaultNamespace, clientPod, cmd)
Expect(res).Should(helpers.CMDSuccess(), "Request from %s pod to service %s failed", clientPod, url)
})
})

Context("with L4 policy", func() {
var (
demoPolicy string
)
Expand Down